Rust: Implement UnusedValue.ql

github · Oct 9, 2024 · 2665f0c · 2665f0c
1 parent ce5cccc
commit 2665f0c
Show file tree

Hide file tree

Showing 6 changed files with 111 additions and 66 deletions.
diff --git a/rust/ql/src/queries/unusedentities/UnusedValue.ql b/rust/ql/src/queries/unusedentities/UnusedValue.ql
@@ -9,7 +9,16 @@
  */
 
 import rust
+import codeql.rust.dataflow.Ssa
+import codeql.rust.dataflow.internal.SsaImpl
+import UnusedVariable
 
-from Locatable e
-where none() // TODO: implement query
-select e, "Variable is assigned a value that is never used."
+from AstNode write, Ssa::Variable v
+where
+  variableWrite(write, v) and
+  // SSA definitions are only created for live writes
+  not write = any(Ssa::WriteDefinition def).getWriteAccess().getAstNode() and
+  // avoid overlap with the unused variable query
+  not isUnused(v) and
+  not v instanceof DiscardVariable
+select write, "Variable is assigned a value that is never used."
diff --git a/rust/ql/src/queries/unusedentities/UnusedVariable.ql b/rust/ql/src/queries/unusedentities/UnusedVariable.ql
@@ -9,11 +9,8 @@
  */
 
 import rust
+import UnusedVariable
 
 from Variable v
-where
-  not exists(v.getAnAccess()) and
-  not exists(v.getInitializer()) and
-  not v.getName().charAt(0) = "_" and
-  exists(File f | f.getBaseName() = "main.rs" | v.getLocation().getFile() = f) // temporarily severely limit results
+where isUnused(v)
 select v, "Variable is not used."
diff --git a/rust/ql/src/queries/unusedentities/UnusedVariable.qll b/rust/ql/src/queries/unusedentities/UnusedVariable.qll
@@ -0,0 +1,14 @@
+import rust
+
+/** A deliberately unused variable. */
+class DiscardVariable extends Variable {
+  DiscardVariable() { this.getName().charAt(0) = "_" }
+}
+
+/** Holds if variable `v` is unused. */
+predicate isUnused(Variable v) {
+  not exists(v.getAnAccess()) and
+  not exists(v.getInitializer()) and
+  not v instanceof DiscardVariable and
+  exists(File f | f.getBaseName() = "main.rs" | v.getLocation().getFile() = f) // temporarily severely limit results
+}
diff --git a/rust/ql/test/query-tests/unusedentities/UnusedValue.expected b/rust/ql/test/query-tests/unusedentities/UnusedValue.expected
@@ -0,0 +1,29 @@
+| main.rs:6:9:6:9 | a | Variable is assigned a value that is never used. |
+| main.rs:7:9:7:9 | b | Variable is assigned a value that is never used. |
+| main.rs:8:9:8:9 | c | Variable is assigned a value that is never used. |
+| main.rs:9:9:9:9 | d | Variable is assigned a value that is never used. |
+| main.rs:10:9:10:9 | e | Variable is assigned a value that is never used. |
+| main.rs:11:9:11:9 | f | Variable is assigned a value that is never used. |
+| main.rs:35:5:35:5 | b | Variable is assigned a value that is never used. |
+| main.rs:37:5:37:5 | c | Variable is assigned a value that is never used. |
+| main.rs:38:5:38:5 | c | Variable is assigned a value that is never used. |
+| main.rs:40:5:40:5 | c | Variable is assigned a value that is never used. |
+| main.rs:42:5:42:5 | d | Variable is assigned a value that is never used. |
+| main.rs:44:9:44:9 | d | Variable is assigned a value that is never used. |
+| main.rs:45:9:45:9 | d | Variable is assigned a value that is never used. |
+| main.rs:50:5:50:5 | e | Variable is assigned a value that is never used. |
+| main.rs:52:9:52:9 | e | Variable is assigned a value that is never used. |
+| main.rs:54:9:54:9 | e | Variable is assigned a value that is never used. |
+| main.rs:61:5:61:5 | f | Variable is assigned a value that is never used. |
+| main.rs:63:5:63:5 | f | Variable is assigned a value that is never used. |
+| main.rs:65:5:65:5 | g | Variable is assigned a value that is never used. |
+| main.rs:67:5:67:5 | i | Variable is assigned a value that is never used. |
+| main.rs:87:9:87:9 | a | Variable is assigned a value that is never used. |
+| main.rs:88:9:88:9 | b | Variable is assigned a value that is never used. |
+| main.rs:89:9:89:9 | c | Variable is assigned a value that is never used. |
+| main.rs:108:9:108:10 | is | Variable is assigned a value that is never used. |
+| main.rs:109:9:109:10 | js | Variable is assigned a value that is never used. |
+| main.rs:133:13:133:17 | total | Variable is assigned a value that is never used. |
+| main.rs:233:13:233:17 | total | Variable is assigned a value that is never used. |
+| main.rs:304:9:304:9 | x | Variable is assigned a value that is never used. |
+| main.rs:312:17:312:17 | x | Variable is assigned a value that is never used. |
diff --git a/rust/ql/test/query-tests/unusedentities/UnusedVariable.expected b/rust/ql/test/query-tests/unusedentities/UnusedVariable.expected
@@ -7,18 +7,18 @@
 | main.rs:174:9:174:9 | x | Variable is not used. |
 | 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:224:14:224:16 | val | Variable is not used. |
+| main.rs:226:9:226:12 | None | Variable is not used. |
+| main.rs:235:9:235:12 | None | Variable is not used. |
+| main.rs:241:22:241: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. |
-| main.rs:309:13:309:13 | y | Variable is not used. |
-| main.rs:317:21:317:21 | y | Variable is not used. |
+| main.rs:256:13:256:15 | num | Variable is not used. |
+| main.rs:267:9:267:11 | Yes | Variable is not used. |
+| main.rs:268:9:268:10 | No | Variable is not used. |
+| main.rs:271:12:271:12 | j | Variable is not used. |
+| main.rs:281:12:281:14 | Yes | Variable is not used. |
+| main.rs:292:25:292:25 | y | Variable is not used. |
+| main.rs:295:28:295:28 | a | Variable is not used. |
+| main.rs:298:9:298:9 | p | Variable is not used. |
+| main.rs:305:13:305:13 | y | Variable is not used. |
+| main.rs:313:21:313:21 | y | Variable is not used. |
diff --git a/rust/ql/test/query-tests/unusedentities/main.rs b/rust/ql/test/query-tests/unusedentities/main.rs
@@ -3,12 +3,12 @@
 // --- locals ---
 
 fn locals_1() {
-    let a = 1; // BAD: unused value [NOT DETECTED]
-    let b = 1;
-    let c = 1;
-    let d = String::from("a"); // BAD: unused value [NOT DETECTED]
-    let e = String::from("b");
-    let f = 1;
+    let a = 1; // BAD: unused value
+    let b = 1; // SPURIOUS: unused value [macros not yet supported]
+    let c = 1; // SPURIOUS: unused value [macros not yet supported]
+    let d = String::from("a"); // BAD: unused value
+    let e = String::from("b"); // SPURIOUS: unused value [macros not yet supported]
+    let f = 1; // SPURIOUS: unused value [macros not yet supported]
     let _ = 1; // (deliberately unused)
 
     println!("use {}", b);
@@ -32,39 +32,39 @@ fn locals_2() {
     let h: i32;
     let i: i32;
 
-    b = 1; // BAD: unused value [NOT DETECTED]
+    b = 1; // BAD: unused value
 
-    c = 1; // BAD: unused value [NOT DETECTED]
-    c = 2;
+    c = 1; // BAD: unused value
+    c = 2; // SPURIOUS: unused value [macros not yet supported]
     println!("use {}", c);
-    c = 3; // BAD: unused value [NOT DETECTED]
+    c = 3; // BAD: unused value
 
-    d = 1;
+    d = 1; // SPURIOUS: unused value [macros not yet supported]
     if cond() {
-        d = 2; // BAD: unused value [NOT DETECTED]
-        d = 3;
+        d = 2; // BAD: unused value
+        d = 3; // SPURIOUS: unused value [macros not yet supported]
     } else {
     }
     println!("use {}", d);
 
-    e = 1; // BAD: unused value [NOT DETECTED]
+    e = 1; // BAD: unused value
     if cond() {
-        e = 2;
+        e = 2; // SPURIOUS: unused value [macros not yet supported]
     } else {
-        e = 3;
+        e = 3; // SPURIOUS: unused value [macros not yet supported]
     }
     println!("use {}", e);
 
     f = 1;
     f += 1;
     println!("use {}", f);
-    f += 1; // BAD: unused value [NOT DETECTED]
+    f += 1; // BAD: unused value
     f = 1;
-    f += 1; // BAD: unused value [NOT DETECTED]
+    f += 1; // BAD: unused value
 
-    g = if cond() { 1 } else { 2 }; // BAD: unused value (x2) [NOT DETECTED]
+    g = if cond() { 1 } else { 2 }; // BAD: unused value
     h = if cond() { 3 } else { 4 };
-    i = if cond() { h } else { 5 };
+    i = if cond() { h } else { 5 }; // SPURIOUS: unused value [macros not yet supported]
     println!("use {}", i);
 
     _ = 1; // (deliberately unused) [NOT DETECTED]
@@ -84,9 +84,9 @@ impl MyStruct {
 }
 
 fn structs() {
-    let a = MyStruct { val: 1 }; // BAD: unused value [NOT DETECTED]
-    let b = MyStruct { val: 2 };
-    let c = MyStruct { val: 3 };
+    let a = MyStruct { val: 1 }; // BAD: unused value
+    let b = MyStruct { val: 2 }; // SPURIOUS: unused value [macros not yet supported]
+    let c = MyStruct { val: 3 }; // SPURIOUS: unused value [macros not yet supported]
     let mut d: MyStruct; // BAD: unused variable
     let mut e: MyStruct;
     let mut f: MyStruct;
@@ -105,8 +105,8 @@ fn structs() {
 // --- arrays ---
 
 fn arrays() {
-    let is = [1, 2, 3]; // BAD: unused values (x3) [NOT DETECTED]
-    let js = [1, 2, 3];
+    let is = [1, 2, 3]; // BAD: unused values (x3)
+    let js = [1, 2, 3]; // SPURIOUS: unused value [macros not yet supported]
     let ks = [1, 2, 3];
 
     println!("lets use {:?}", js);
@@ -130,7 +130,7 @@ fn statics() {
     static mut STAT4: i32 = 0; // BAD: unused value [NOT DETECTED]
 
     unsafe {
-        let total = CON1 + STAT1 + STAT3;
+        let total = CON1 + STAT1 + STAT3; // BAD: unused value
     }
 }
 
@@ -139,7 +139,7 @@ fn statics() {
 fn parameters(
     x: i32,
     y: i32,  // BAD: unused variable
-    _z: i32, // (`_` is asking the compiler, and by extension us, to not warn that this is unused)
+    _z: i32, // (`_` is asking the compiler, and by extension us, to not warn that this is unused) TODO
 ) -> i32 {
     return x;
 }
@@ -189,7 +189,7 @@ enum YesOrNo {
     No,
 }
 
-use YesOrNo::{Yes, No}; // allows `Yes`, `No` to be accessed without qualifiers.
+use YesOrNo::{No, Yes}; // allows `Yes`, `No` to be accessed without qualifiers.
 
 struct MyPoint {
     x: i64,
@@ -207,7 +207,8 @@ fn if_lets_matches() {
     }
 
     let mut next = Some(30);
-    while let Some(val) = next // BAD: unused variable
+    while let Some(val) = // BAD: unused variable
+        next
     {
         next = None;
     }
@@ -229,7 +230,7 @@ fn if_lets_matches() {
     let d = Some(70);
     match d {
         Some(val) => {
-            total += val;
+            total += val; // BAD: unused variable
         }
         None => { // SPURIOUS: unused variable 'None'
         }
@@ -239,8 +240,7 @@ fn if_lets_matches() {
     match e {
         Option::Some(val) => { // BAD: unused variable
         }
-        Option::None => {
-        }
+        Option::None => {}
     }
 
     let f = MyOption::Some(90);
@@ -250,10 +250,9 @@ fn if_lets_matches() {
         MyOption::None => {}
     }
 
-    let g : Result<i64, i64> = Ok(100);
+    let g: Result<i64, i64> = Ok(100);
     match g {
-        Ok(_) => {
-        }
+        Ok(_) => {}
         Err(num) => {} // BAD: unused variable
     }
 
@@ -266,7 +265,7 @@ fn if_lets_matches() {
     let i = Yes;
     match i {
         Yes => {} // SPURIOUS: unused variable 'Yes'
-        No => {} // SPURIOUS: unused variable 'No'
+        No => {}  // SPURIOUS: unused variable 'No'
     }
 
     if let j = Yes { // BAD: unused variable
@@ -289,31 +288,28 @@ fn if_lets_matches() {
 
     let p1 = MyPoint { x: 1, y: 2 };
     match p1 {
-        MyPoint { x: 0, y: 0 } => {
-        }
+        MyPoint { x: 0, y: 0 } => {}
         MyPoint { x: 1, y } => { // BAD: unused variable
         }
-        MyPoint { x: 2, y: _ } => {
-        }
+        MyPoint { x: 2, y: _ } => {}
         MyPoint { x: 3, y: a } => { // BAD: unused variable
         }
-        MyPoint { x: 4, .. } => {
-        }
+        MyPoint { x: 4, .. } => {}
         p => { // BAD: unused variable
         }
     }
 }
 
 fn shadowing() -> i32 {
-    let x = 1; // BAD: unused value [NOT DETECTED]
+    let x = 1; // BAD: unused value
     let mut y: i32; // BAD: unused variable
 
     {
         let x = 2;
         let mut y: i32;
 
         {
-            let x = 3; // BAD: unused value [NOT DETECTED]
+            let x = 3; // BAD: unused value
             let mut y: i32; // BAD: unused variable
         }