Merge pull request #17773 from geoffw0/unusedval2

Rust: Implement UnusedValue.ql (2)
2026-04-24 16:25:15 +02:00 · 2024-10-17 21:03:24 +02:00
parent b07c788a08 7e2542bd74
commit 6e197b5802
8 changed files with 246 additions and 78 deletions
--- 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."
--- 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."
--- 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
+}
--- a/rust/ql/test/query-tests/unusedentities/CONSISTENCY/CfgConsistency.expected
+++ b/rust/ql/test/query-tests/unusedentities/CONSISTENCY/CfgConsistency.expected
@@ -1,11 +1,17 @@
 multipleSuccessors
-| main.rs:428:17:428:17 | 0 | successor | main.rs:428:9:428:10 | a2 |
-| main.rs:428:17:428:17 | 0 | successor | main.rs:428:20:428:62 | ClosureExpr |
-| main.rs:431:17:431:17 | 0 | successor | main.rs:431:9:431:10 | a3 |
-| main.rs:431:17:431:17 | 0 | successor | main.rs:431:20:431:41 | ClosureExpr |
-| main.rs:434:17:434:17 | 0 | successor | main.rs:434:9:434:10 | a4 |
-| main.rs:434:17:434:17 | 0 | successor | main.rs:434:20:434:35 | ClosureExpr |
-| main.rs:437:17:437:17 | 0 | successor | main.rs:437:9:437:10 | a5 |
-| main.rs:437:17:437:17 | 0 | successor | main.rs:437:20:437:35 | ClosureExpr |
-| main.rs:441:17:441:17 | 0 | successor | main.rs:441:9:441:10 | a6 |
-| main.rs:441:17:441:17 | 0 | successor | main.rs:441:20:441:46 | ClosureExpr |
+| main.rs:421:17:421:17 | 0 | successor | main.rs:421:9:421:10 | a2 |
+| main.rs:421:17:421:17 | 0 | successor | main.rs:421:20:421:62 | ClosureExpr |
+| main.rs:424:17:424:17 | 0 | successor | main.rs:424:9:424:10 | a3 |
+| main.rs:424:17:424:17 | 0 | successor | main.rs:424:20:424:41 | ClosureExpr |
+| main.rs:427:17:427:17 | 0 | successor | main.rs:427:9:427:10 | a4 |
+| main.rs:427:17:427:17 | 0 | successor | main.rs:427:20:427:35 | ClosureExpr |
+| main.rs:430:17:430:17 | 0 | successor | main.rs:430:9:430:10 | a5 |
+| main.rs:430:17:430:17 | 0 | successor | main.rs:430:20:430:35 | ClosureExpr |
+| main.rs:434:17:434:17 | 0 | successor | main.rs:434:9:434:10 | a6 |
+| main.rs:434:17:434:17 | 0 | successor | main.rs:434:20:434:46 | ClosureExpr |
+| more.rs:32:17:32:17 | a | successor | more.rs:32:5:32:5 | i |
+| more.rs:32:17:32:17 | a | successor | more.rs:32:20:32:20 | b |
+deadEnd
+| more.rs:9:9:9:19 | Param |
+| more.rs:38:23:38:28 | Param |
+| more.rs:42:19:42:24 | Param |
--- a/rust/ql/test/query-tests/unusedentities/UnusedValue.expected
+++ b/rust/ql/test/query-tests/unusedentities/UnusedValue.expected
@@ -0,0 +1,30 @@
+| main.rs:6:9:6:9 | a | 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: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:40:5:40:5 | c | 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:50:5:50:5 | 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:87:9:87:9 | a | 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:133:13:133:17 | total | Variable is assigned a value that is never used. |
+| main.rs:203:13:203:31 | res | Variable is assigned a value that is never used. |
+| main.rs:218:9:218:24 | kind | Variable is assigned a value that is never used. |
+| main.rs:223:9:223:32 | kind | Variable is assigned a value that is never used. |
+| main.rs:280:13:280:17 | total | Variable is assigned a value that is never used. |
+| main.rs:348:5:348:39 | kind | Variable is assigned a value that is never used. |
+| main.rs:370:9:370:9 | x | Variable is assigned a value that is never used. |
+| main.rs:378:17:378:17 | x | Variable is assigned a value that is never used. |
+| main.rs:432:9:432:10 | i6 | Variable is assigned a value that is never used. |
+| more.rs:8:9:8:13 | times | Variable is assigned a value that is never used. |
+| more.rs:9:9:9:14 | unused | Variable is assigned a value that is never used. |
+| more.rs:21:9:21:14 | unused | Variable is assigned a value that is never used. |
+| more.rs:38:23:38:25 | val | Variable is assigned a value that is never used. |
+| more.rs:42:19:42:21 | val | Variable is assigned a value that is never used. |
+| more.rs:58:9:58:11 | val | Variable is assigned a value that is never used. |
+| more.rs:80:9:80:14 | a_ptr4 | Variable is assigned a value that is never used. |
+| more.rs:95:9:95:13 | d_ptr | Variable is assigned a value that is never used. |
+| more.rs:101:9:101:17 | f_ptr | Variable is assigned a value that is never used. |
--- a/rust/ql/test/query-tests/unusedentities/UnusedVariable.expected
+++ b/rust/ql/test/query-tests/unusedentities/UnusedVariable.expected
@@ -6,17 +6,17 @@
 | main.rs:201:9:201:9 | x | Variable is not used. |
 | main.rs:250:17:250:17 | a | Variable is not used. |
 | main.rs:258:20:258:22 | val | Variable is not used. |
-| main.rs:271:14:271:16 | val | Variable is not used. |
-| main.rs:288:22:288:24 | val | Variable is not used. |
-| main.rs:296:24:296:26 | val | Variable is not used. |
-| main.rs:305:13:305:15 | num | Variable is not used. |
-| main.rs:320:12:320:12 | j | Variable is not used. |
-| main.rs:342:25:342:25 | y | Variable is not used. |
-| main.rs:346:28:346:28 | a | Variable is not used. |
-| main.rs:350:9:350:9 | p | Variable is not used. |
-| main.rs:365:9:365:13 | right | Variable is not used. |
-| main.rs:371:9:371:14 | right2 | Variable is not used. |
-| main.rs:378:13:378:13 | y | Variable is not used. |
-| main.rs:386:21:386:21 | y | Variable is not used. |
-| main.rs:434:27:434:29 | val | Variable is not used. |
-| main.rs:437:22:437:24 | acc | Variable is not used. |
+| main.rs:272:14:272:16 | val | Variable is not used. |
+| main.rs:287:22:287:24 | val | Variable is not used. |
+| main.rs:294:24:294:26 | val | Variable is not used. |
+| main.rs:302:13:302:15 | num | Variable is not used. |
+| main.rs:317:12:317:12 | j | Variable is not used. |
+| main.rs:337:25:337:25 | y | Variable is not used. |
+| main.rs:340:28:340:28 | a | Variable is not used. |
+| main.rs:343:9:343:9 | p | Variable is not used. |
+| main.rs:358:9:358:13 | right | Variable is not used. |
+| main.rs:364:9:364:14 | right2 | Variable is not used. |
+| main.rs:371:13:371:13 | y | Variable is not used. |
+| main.rs:379:21:379:21 | y | Variable is not used. |
+| main.rs:427:27:427:29 | val | Variable is not used. |
+| main.rs:430:22:430:24 | acc | Variable is not used. |
--- a/rust/ql/test/query-tests/unusedentities/main.rs
+++ b/rust/ql/test/query-tests/unusedentities/main.rs
@@ -3,10 +3,10 @@
 // --- locals ---

 fn locals_1() {
-    let a = 1; // BAD: unused value [NOT DETECTED]
+    let a = 1; // BAD: unused value
    let b = 1;
    let c = 1;
-    let d = String::from("a"); // BAD: unused value [NOT DETECTED]
+    let d = String::from("a"); // BAD: unused value
    let e = String::from("b");
    let f = 1;
    let _ = 1; // (deliberately unused)
@@ -32,22 +32,22 @@ 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 = 1; // BAD: unused value
    c = 2;
    println!("use {}", c);
-    c = 3; // BAD: unused value [NOT DETECTED]
+    c = 3; // BAD: unused value

    d = 1;
    if cond() {
-        d = 2; // BAD: unused value [NOT DETECTED]
+        d = 2; // BAD: unused value
        d = 3;
    } else {
    }
    println!("use {}", d);

-    e = 1; // BAD: unused value [NOT DETECTED]
+    e = 1; // BAD: unused value
    if cond() {
        e = 2;
    } else {
@@ -58,16 +58,16 @@ fn locals_2() {
    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 };
    println!("use {}", i);

-    _ = 1; // (deliberately unused) [NOT DETECTED]
+    _ = 1; // GOOD (deliberately unused)
 }

 // --- structs ---
@@ -84,7 +84,7 @@ impl MyStruct {
 }

 fn structs() {
-    let a = MyStruct { val: 1 }; // BAD: unused value [NOT DETECTED]
+    let a = MyStruct { val: 1 }; // BAD: unused value
    let b = MyStruct { val: 2 };
    let c = MyStruct { val: 3 };
    let mut d: MyStruct; // BAD: unused variable
@@ -105,7 +105,7 @@ fn structs() {
 // --- arrays ---

 fn arrays() {
-    let is = [1, 2, 3]; // BAD: unused values (x3) [NOT DETECTED]
+    let is = [1, 2, 3]; // BAD: unused value
    let js = [1, 2, 3];
    let ks = [1, 2, 3];

@@ -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
    }
 }

@@ -200,7 +200,7 @@ fn loops() {

    for x // SPURIOUS: unused variable
    in 1..10 {
-        _ = format!("x is {x}");
+        _ = format!("x is {x}"); // SPURIOUS: unused value `res`
    }

    for x
@@ -215,12 +215,12 @@ fn loops() {

    for x
    in 1..10 {
-        assert_eq!(x, 1);
+        assert_eq!(x, 1); // SPURIOUS: unused value `kind`
    }

    for x
    in 1..10 {
-        assert_eq!(id(x), id(1));
+        assert_eq!(id(x), id(1)); // SPURIOUS: unused value `kind`
    }

 }
@@ -255,7 +255,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;
    }
@@ -270,25 +271,22 @@ fn if_lets_matches() {
    match c {
        Some(val) => { // BAD: unused variable
        }
-        None => {
-        }
+        None => {}
    }

    let d = Some(70);
    match d {
        Some(val) => {
-            total += val;
-        }
-        None => {
+            total += val; // BAD: unused value
        }
+        None => {}
    }

    let e = Option::Some(80);
    match e {
        Option::Some(val) => { // BAD: unused variable
        }
-        Option::None => {
-        }
+        Option::None => {}
    }

    let f = MyOption::Some(90);
@@ -298,10 +296,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
    }

@@ -327,8 +324,7 @@ fn if_lets_matches() {
    }

    let l = Yes;
-    if let Yes = l {
-    }
+    if let Yes = l {}

    match 1 {
        1 => {}
@@ -337,22 +333,19 @@ 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
        }
    }

    let duration1 = std::time::Duration::new(10, 0); // ten seconds
-    assert_eq!(duration1.as_secs(), 10);
+    assert_eq!(duration1.as_secs(), 10); // SPURIOUS: unused value `kind`

    let duration2:Result<std::time::Duration, String> =
        Ok(std::time::Duration::new(10, 0));
@@ -374,7 +367,7 @@ fn if_lets_matches() {
 }

 fn shadowing() -> i32 {
-    let x = 1; // BAD: unused value [NOT DETECTED]
+    let x = 1; // BAD: unused value
    let mut y: i32; // BAD: unused variable

    {
@@ -382,7 +375,7 @@ fn shadowing() -> i32 {
        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
        }

@@ -436,7 +429,7 @@ fn folds_and_closures() {
    let a5 = 1..10;
    _ = a5.fold(0, | acc, val | val); // BAD: unused variable

-    let i6 = 1;
+    let i6 = 1; // SPURIOUS: unused value
    let a6 = 1..10;
    _ = a6.fold(0, | acc, val | acc + val + i6);
 }
@@ -449,16 +442,16 @@ fn main() {
    structs();
    arrays();
    statics();
-	println!("lets use result {}", parameters(1, 2, 3));
+    println!("lets use result {}", parameters(1, 2, 3));
    loops();
    if_lets_matches();
    shadowing();
    func_ptrs();
    folds_and_closures();

-	unreachable_if();
-	unreachable_panic();
-	unreachable_match();
-	unreachable_loop();
-	unreachable_paren();
+    unreachable_if();
+    unreachable_panic();
+    unreachable_match();
+    unreachable_loop();
+    unreachable_paren();
 }
--- a/rust/ql/test/query-tests/unusedentities/more.rs
+++ b/rust/ql/test/query-tests/unusedentities/more.rs
@@ -0,0 +1,119 @@
+
+
+// --- traits ---
+
+trait Incrementable {
+    fn increment(
+        &mut self,
+        times: i32, // SPURIOUS: unused value
+        unused: i32 // SPURIOUS: unused value
+    );
+}
+
+struct MyValue {
+    value: i32,
+}
+
+impl Incrementable for MyValue {
+    fn increment(
+        &mut self,
+        times: i32,
+        unused: i32 // BAD: unused variable [NOT DETECTED] SPURIOUS: unused value
+    ) {
+        self.value += times;
+    }
+}
+
+fn traits() {
+    let mut i = MyValue { value: 0 };
+    let a = 1;
+    let b = 2;
+
+    i.increment(a, b);
+}
+
+// --- generics ---
+
+trait MySettable<T> {
+    fn set(&mut self, val: T); // SPURIOUS: unused value
+}
+
+trait MyGettable<T> {
+    fn get(&self, val: T) -> &T; // SPURIOUS: unused value
+}
+
+struct MyContainer<T> {
+    val: T
+}
+
+impl<T> MySettable<T> for MyContainer<T> {
+    fn set(&mut self, val: T) {
+        self.val = val;
+    }
+}
+
+impl<T> MyGettable<T> for MyContainer<T> {
+    fn get(
+        &self,
+        val: T // BAD: unused variable [NOT DETECTED] SPURIOUS: unused value
+    ) -> &T {
+        return &(self.val);
+    }
+}
+
+fn generics() {
+    let mut a = MyContainer { val: 1 }; // BAD: unused value [NOT DETECTED]
+    let b = MyContainer { val: 2 };
+
+    a.set(
+        *b.get(3)
+    );
+}
+
+// --- pointers ---
+
+fn pointers() {
+    let a = 1;
+    let a_ptr1 = &a;
+    let a_ptr2 = &a;
+    let a_ptr3 = &a; // BAD: unused value [NOT DETECTED]
+    let a_ptr4 = &a; // BAD: unused value
+    println!("{}", *a_ptr1);
+    println!("{}", a_ptr2);
+    println!("{}", &a_ptr3);
+
+    let b = 2; // BAD: unused value [NOT DETECTED]
+    let b_ptr = &b;
+    println!("{}", b_ptr);
+
+    let c = 3;
+    let c_ptr = &c;
+    let c_ptr_ptr = &c_ptr;
+    println!("{}", **c_ptr_ptr);
+
+    let d = 4;
+    let d_ptr = &d; // BAD: unused value
+    let d_ptr_ptr = &&d;
+    println!("{}", **d_ptr_ptr);
+
+    let e = 5; // BAD: unused value [NOT DETECTED]
+    let f = 6;
+    let mut f_ptr = &e; // BAD: unused value
+    f_ptr = &f;
+    println!("{}", *f_ptr);
+
+    let mut g = 7; // BAD: unused value [NOT DETECTED]
+    let g_ptr = &mut g;
+    *g_ptr = 77; // BAD: unused value [NOT DETECTED]
+
+    let mut h = 8; // BAD: unused value [NOT DETECTED]
+    let h_ptr = &mut h;
+    *h_ptr = 88;
+    println!("{}", h);
+
+    let mut i = 9; // BAD: unused value [NOT DETECTED]
+    let i_ptr = &mut i;
+    *i_ptr = 99;
+    let i_ptr2 = &mut i;
+    println!("{}", *i_ptr2);
+}