Merge pull request github#17718 from geoffw0/unusedvar4

Rust: Restrict variables to lowercase (for now).
jessejay-ch · Oct 10, 2024 · 25b3d76 · 25b3d76
2 parents 09c2f90 + 5ac9c2d
commit 25b3d76
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 12 deletions.
diff --git a/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll b/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll
@@ -78,7 +78,12 @@ module Impl {
       not exists(getOutermostEnclosingOrPat(p)) and
       definingNode = p.getName()
     ) and
-    name = p.getName().getText()
+    name = p.getName().getText() and
+    // exclude for now anything starting with an uppercase character, which may be a reference to
+    // an enum constant (e.g. `None`). This excludes static and constant variables (UPPERCASE),
+    // which we don't appear to recognize yet anyway. This also assumes programmers follow the
+    // naming guidelines, which they generally do, but they're not enforced.
+    not name.charAt(0).isUppercase()
   }
 
   /** A variable. */

diff --git a/rust/ql/test/library-tests/variables/variables.expected b/rust/ql/test/library-tests/variables/variables.expected
@@ -24,7 +24,6 @@ variable
 | variables.rs:100:9:100:10 | x6 |
 | variables.rs:101:9:101:10 | y1 |
 | variables.rs:105:14:105:15 | y1 |
-| variables.rs:110:9:110:12 | None |
 | variables.rs:117:9:117:15 | numbers |
 | variables.rs:121:13:121:17 | first |
 | variables.rs:122:13:122:17 | third |

diff --git a/rust/ql/test/query-tests/unusedentities/UnusedVariable.expected b/rust/ql/test/query-tests/unusedentities/UnusedVariable.expected
@@ -8,15 +8,10 @@
 | main.rs:202:17:202:17 | a | Variable is not used. |
 | main.rs:210:20:210:22 | val | Variable is not used. |
 | main.rs:223:14:223:16 | val | Variable is not used. |
-| main.rs:225:9:225:12 | None | Variable is not used. |
-| main.rs:234:9:234:12 | None | Variable is not used. |
 | main.rs:240:22:240:24 | val | Variable is not used. |
 | main.rs:248:24:248:26 | val | Variable is not used. |
 | main.rs:257:13:257:15 | num | Variable is not used. |
-| main.rs:268:9:268:11 | Yes | Variable is not used. |
-| main.rs:269:9:269:10 | No | Variable is not used. |
 | main.rs:272:12:272:12 | j | Variable is not used. |
-| main.rs:282:12:282:14 | Yes | Variable is not used. |
 | main.rs:294:25:294:25 | y | Variable is not used. |
 | main.rs:298:28:298:28 | a | Variable is not used. |
 | main.rs:302:9:302:9 | p | Variable is not used. |

diff --git a/rust/ql/test/query-tests/unusedentities/main.rs b/rust/ql/test/query-tests/unusedentities/main.rs
@@ -222,7 +222,7 @@ fn if_lets_matches() {
     match c {
         Some(val) => { // BAD: unused variable
         }
-        None => { // SPURIOUS: unused variable 'None'
+        None => {
         }
     }
 
@@ -231,7 +231,7 @@ fn if_lets_matches() {
         Some(val) => {
             total += val;
         }
-        None => { // SPURIOUS: unused variable 'None'
+        None => {
         }
     }
 
@@ -265,8 +265,8 @@ fn if_lets_matches() {
 
     let i = Yes;
     match i {
-        Yes => {} // SPURIOUS: unused variable 'Yes'
-        No => {} // SPURIOUS: unused variable 'No'
+        Yes => {}
+        No => {}
     }
 
     if let j = Yes { // BAD: unused variable
@@ -279,7 +279,7 @@ fn if_lets_matches() {
     }
 
     let l = Yes;
-    if let Yes = l { // SPURIOUS: unused variable 'Yes'
+    if let Yes = l {
     }
 
     match 1 {