Merge pull request #17656 from geoffw0/unusedvar2

Rust: Diagnose unused variable false positives

Author: geoffw0

rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll

@@ -218,6 +218,13 @@ module Impl {
         scope = ce.getBody() and
         scope.getLocation().hasLocationInfo(_, line, column, _, _)
       )
+      or
+      exists(WhileExpr we, LetExpr let |
+        let.getPat() = pat and
+        we.getCondition() = let and
+        scope = we.getLoopBody() and
+        scope.getLocation().hasLocationInfo(_, line, column, _, _)
+      )
     )
   }
 

rust/ql/src/queries/unusedentities/UnusedVariable.ql

@@ -14,5 +14,6 @@ from Variable v
 where
   not exists(v.getAnAccess()) and
   not exists(v.getInitializer()) and
-  not v.getName().charAt(0) = "_"
+  not v.getName().charAt(0) = "_" and
+  exists(File f | f.getBaseName() = "main.rs" | v.getLocation().getFile() = f) // temporarily severely limit results
 select v, "Variable is not used."

rust/ql/test/library-tests/variables/Cfg.expected

@@ -85,6 +85,15 @@ fn let_pattern4() {
     print_str(x5); // $ access=x5
 }
 
+fn let_pattern5() {
+    let s1 = Some(String::from("Hello!")); // s1
+
+    while let Some(ref s2) // s2
+        = s1 { // $ access=s1
+        print_str(s2); // $ access=s2
+    }
+}
+
 fn match_pattern1() {
     let x6 = Some(5); // x6
     let y1 = 10; // y1_1

rust/ql/test/library-tests/variables/variables.expected

@@ -1,4 +1,13 @@
-| main.rs:23:9:23:9 | a | Variable is not used. |
-| main.rs:88:13:88:13 | d | Variable is not used. |
-| main.rs:112:9:112:9 | k | Variable is not used. |
-| main.rs:139:5:139:5 | y | Variable is not used. |
+| main.rs:25:9:25:9 | a | Variable is not used. |
+| main.rs:90:13:90:13 | d | Variable is not used. |
+| main.rs:114:9:114:9 | k | Variable is not used. |
+| main.rs:141:5:141:5 | y | Variable is not used. |
+| main.rs:164:9:164:9 | x | Variable is not used. |
+| main.rs:169:9:169:9 | x | Variable is not used. |
+| main.rs:174:9:174:9 | x | Variable is not used. |
+| main.rs:195:17:195:17 | a | Variable is not used. |
+| main.rs:203:20:203:22 | val | Variable is not used. |
+| main.rs:216:14:216:16 | val | Variable is not used. |
+| main.rs:218:9:218:12 | None | Variable is not used. |
+| main.rs:227:9:227:12 | None | Variable is not used. |
+| main.rs:233:24:233:26 | val | Variable is not used. |

rust/ql/test/library-tests/variables/variables.rs

@@ -8,6 +8,7 @@ fn locals_1() {
     let c = 1;
     let d = String::from("a"); // BAD: unused value [NOT DETECTED]
     let e = String::from("b");
+    let f = 1;
     let _ = 1; // (deliberately unused)
 
     println!("use {}", b);
@@ -17,6 +18,7 @@ fn locals_1() {
     }
 
     println!("use {}", e);
+    assert!(f == 1);
 }
 
 fn locals_2() {
@@ -142,11 +144,112 @@ fn parameters(
     return x;
 }
 
+// --- loops ---
+
+fn loops() {
+    let mut a: i64 = 10;
+    let b: i64 = 20;
+    let c: i64 = 30;
+    let d: i64 = 40;
+    let mut e: i64 = 50;
+
+    while a < b {
+        a += 1;
+    }
+
+    for x in c..d {
+        e += x;
+    }
+
+    for x in 1..10 { // BAD: unused variable
+    }
+
+    for _ in 1..10 {}
+
+    for x // SPURIOUS: unused variable [macros not yet supported]
+    in 1..10 {    
+        println!("x is {}", x);
+    }
+
+    for x // SPURIOUS: unused variable [macros not yet supported]
+    in 1..10 {   
+        assert!(x != 11);
+    }
+}
+
+// --- lets ---
+
+enum MyOption<T> {
+    None,
+    Some(T),
+}
+
+enum YesOrNo {
+    Yes,
+    No,
+}
+
+fn if_lets() {
+    let mut total: i64 = 0;
+
+    if let Some(a) = Some(10) { // BAD: unused variable
+    }
+
+    if let Some(b) = Some(20) {
+        total += b;
+    }
+
+    let mut next = Some(30);
+    while let Some(val) = next // BAD: unused variable
+    {
+        next = None;
+    }
+
+    let mut next2 = Some(40);
+    while let Some(val) = next2 {
+        total += val;
+        next2 = None;
+    }
+
+    let c = Some(60);
+    match c {
+        Some(val) => { // BAD: unused variable
+        }
+        None => { // SPURIOUS: unused variable 'None'
+        }
+    }
+
+    let d = Some(70);
+    match d {
+        Some(val) => {
+            total += val;
+        }
+        None => { // SPURIOUS: unused variable 'None'
+        }
+    }
+
+    let e = MyOption::Some(80);
+    match e {
+        MyOption::Some(val) => { // BAD: unused variable
+        }
+        MyOption::None => {}
+    }
+
+    let f = YesOrNo::Yes;
+    match f {
+        YesOrNo::Yes => {}
+        YesOrNo::No => {}
+    }
+}
+
 fn main() {
     locals_1();
     locals_2();
     structs();
     arrays();
     statics();
+    loops();
+    if_lets();
+
     println!("lets use result {}", parameters(1, 2, 3));
 }