Merge pull request #18952 from geoffw0/unusedvarfix

Rust: Improve rust/unused-variable and rust/unused-value

Author: geoffw0

rust/ql/src/queries/unusedentities/UnusedValue.ql

@@ -16,10 +16,11 @@ import UnusedVariable
 from AstNode write, Ssa::Variable v
 where
   variableWrite(write, v) and
+  not v instanceof DiscardVariable and
+  not write.isInMacroExpansion() and
+  not isAllowableUnused(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 and
-  not write.isInMacroExpansion()
+  not isUnused(v)
 select write, "Variable $@ is assigned a value that is never used.", v, v.getText()

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

@@ -16,6 +16,20 @@ predicate isUnused(Variable v) {
   not exists(v.getInitializer())
 }
 
+/**
+ * A callable for which we have incomplete information, for example because an unexpanded
+ * macro call is present. These callables are prone to false positive results from unused
+ * entities queries, unless they are excluded from results.
+ */
+class IncompleteCallable extends Callable {
+  IncompleteCallable() {
+    exists(MacroExpr me |
+      me.getEnclosingCallable() = this and
+      not me.getMacroCall().hasExpanded()
+    )
+  }
+}
+
 /**
  * Holds if variable `v` is in a context where we may not find a use for it,
  * but that's expected and should not be considered a problem.
@@ -24,6 +38,9 @@ predicate isAllowableUnused(Variable v) {
   // in a macro expansion
   v.getPat().isInMacroExpansion()
   or
+  // declared in an incomplete callable
+  v.getEnclosingCfgScope() instanceof IncompleteCallable
+  or
   // a 'self' variable
   v.getText() = "self"
 }

rust/ql/test/query-tests/unusedentities/CONSISTENCY/ExtractionConsistency.expected

@@ -0,0 +1,2 @@
+extractionWarning
+| undefined_macros/main.rs:1:1:1:1 | semantic analyzer unavailable (not included as a module) |

rust/ql/test/query-tests/unusedentities/UnusedValue.expected

@@ -14,8 +14,8 @@
 | main.rs:284:13:284:17 | total | Variable $@ is assigned a value that is never used. | main.rs:252:13:252:17 | total | total |
 | main.rs:377:9:377:9 | x | Variable $@ is assigned a value that is never used. | main.rs:377:9:377:9 | x | x |
 | main.rs:385:17:385:17 | x | Variable $@ is assigned a value that is never used. | main.rs:385:17:385:17 | x | x |
-| main.rs:486:9:486:9 | c | Variable $@ is assigned a value that is never used. | main.rs:486:9:486:9 | c | c |
-| main.rs:519:9:519:20 | var_in_macro | Variable $@ is assigned a value that is never used. | main.rs:519:9:519:20 | var_in_macro | var_in_macro |
+| main.rs:531:9:531:20 | var_in_macro | Variable $@ is assigned a value that is never used. | main.rs:531:9:531:20 | var_in_macro | var_in_macro |
+| main.rs:540:9:540:9 | c | Variable $@ is assigned a value that is never used. | main.rs:540:9:540:9 | c | c |
 | more.rs:44:9:44:14 | a_ptr4 | Variable $@ is assigned a value that is never used. | more.rs:44:9:44:14 | a_ptr4 | a_ptr4 |
 | more.rs:59:9:59:13 | d_ptr | Variable $@ is assigned a value that is never used. | more.rs:59:9:59:13 | d_ptr | d_ptr |
 | more.rs:65:13:65:17 | f_ptr | Variable $@ is assigned a value that is never used. | more.rs:65:13:65:17 | f_ptr | f_ptr |

rust/ql/test/query-tests/unusedentities/main.rs

@@ -468,16 +468,70 @@ fn traits() {
 
 // --- macros ---
 
-fn macros() {
+macro_rules! set_value {
+    ($x:expr,$y:expr) => {
+        $x = $y
+    };
+}
+
+macro_rules! use_value {
+    ($x:expr) => {
+        println!("{}", $x)
+    };
+}
+
+fn macros1() {
+    let a: u16;
+    let b: u16 = 2;
+    set_value!(a, 1);
+    use_value!(b);
+
+    match std::env::args().nth(1).unwrap().parse::<u16>() {
+        Ok(n) => {
+            use_value!(n);
+        }
+        _ => {}
+    }
+}
+
+fn macros2() {
+    let a: u16 = 3;
+    println!("{}", a);
+
+    match std::env::args().nth(1).unwrap().parse::<u16>() {
+        Ok(n) => {
+            println!("{}", n);
+        }
+        _ => {}
+    }
+}
+
+fn macros3() {
     let x;
     println!(
         "The value of x is {}",
         ({
             x = 10; // $ MISSING: Alert[rust/unused-value]
             10
         })
-    )
+    );
+}
+
+macro_rules! let_in_macro {
+    ($e:expr) => {{
+        let var_in_macro = 0;
+        $e
+    }};
 }
+
+// Our analysis does not currently respect the hygiene rules of Rust macros
+// (https://veykril.github.io/tlborm/decl-macros/minutiae/hygiene.html), because
+// all we have access to is the expanded AST
+fn hygiene_mismatch() {
+    let var_in_macro = 0; // $ SPURIOUS: Alert[rust/unused-value]
+    let_in_macro!(var_in_macro);
+}
+
 // --- references ---
 
 fn references() {
@@ -505,21 +559,6 @@ trait MyTrait {
     fn my_func2(&self, x: i32) -> i32;
 }
 
-macro_rules! let_in_macro {
-    ($e:expr) => {{
-        let var_in_macro = 0;
-        $e
-    }};
-}
-
-// Our analysis does not currently respect the hygiene rules of Rust macros
-// (https://veykril.github.io/tlborm/decl-macros/minutiae/hygiene.html), because
-// all we have access to is the expanded AST
-fn hygiene_mismatch() {
-    let var_in_macro = 0; // $ SPURIOUS: Alert[rust/unused-value]
-    let_in_macro!(var_in_macro);
-}
-
 // --- main ---
 
 fn main() {
@@ -534,7 +573,10 @@ fn main() {
     shadowing();
     func_ptrs();
     folds_and_closures();
-    macros();
+    macros1();
+    macros2();
+    macros3();
+    hygiene_mismatch();
     references();
 
     generics();

rust/ql/test/query-tests/unusedentities/undefined_macros/main.rs

@@ -0,0 +1,33 @@
+
+// --- undefined macro calls ---
+
+fn undefined_macros1() {
+    let a: u16;
+
+    undefined_macro_call!(d);
+}
+
+fn undefined_macros2() {
+    {
+        let a: u16 = 1; // $ MISSING: Alert[rust/unused-value]
+    }
+
+    undefined_macro_call!(5);
+
+    let b: u16; // $ MISSING: Alert[rust/unused-variable]
+}
+
+fn undefined_macros3() {
+    match std::env::args().nth(1).unwrap().parse::<u16>() {
+        Ok(n) => {
+            undefined_macro_call!(n);
+        }
+        _ => {}
+    }
+}
+
+fn main() {
+    undefined_macros1();
+    undefined_macros2();
+    undefined_macros3();
+}

rust/ql/test/query-tests/unusedentities/undefined_macros/options.yml

@@ -0,0 +1 @@
+qltest_cargo_check: false