Rust: Clean up code / clarify responsibilities and fix the issue in rust/unused-value as well.

Author: geoffw0

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

@@ -12,5 +12,8 @@ import rust
 import UnusedVariable
 
 from Variable v
-where isUnused(v)
+where
+  isUnused(v) and
+  not isAllowableUnused(v) and
+  not v instanceof DiscardVariable
 select v, "Variable '" + v + "' is not used."

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

@@ -1,18 +1,28 @@
 import rust
 
-/** A deliberately unused variable. */
+/**
+ * A deliberately unused variable, for example `_` or `_x`.
+ */
 class DiscardVariable extends Variable {
   DiscardVariable() { this.getName().charAt(0) = "_" }
 }
 
-/** Holds if variable `v` is unused. */
+/**
+ * Holds if variable `v` is unused.
+ */
 predicate isUnused(Variable v) {
   // variable is accessed or initialized
   not exists(v.getAnAccess()) and
-  not exists(v.getInitializer()) and
-  // variable is intentionally unused
-  not v instanceof DiscardVariable and
-  // variable is in a context where is may not have a use
-  not v.getPat().isInMacroExpansion() and
-  not exists(FnPtrType fp | fp.getParamList().getParam(_).getPat() = v.getPat())
+  not exists(v.getInitializer())
+}
+
+/**
+ * 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.
+ */
+predicate isAllowableUnused(Variable v) {
+  // in a macro expansion
+  v.getPat().isInMacroExpansion() or
+  // function pointer parameters
+  exists(FnPtrType fp | fp.getParamList().getParam(_).getPat() = v.getPat())
 }

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

@@ -15,10 +15,6 @@
 | main.rs:373:9:373:9 | x | Variable $@ is assigned a value that is never used. | main.rs:373:9:373:9 | x | x |
 | main.rs:381:17:381:17 | x | Variable $@ is assigned a value that is never used. | main.rs:381:17:381:17 | x | x |
 | main.rs:482:9:482:9 | c | Variable $@ is assigned a value that is never used. | main.rs:482:9:482:9 | c | c |
-| main.rs:494:16:494:16 | x | Variable $@ is assigned a value that is never used. | main.rs:494:16:494:16 | x | x |
-| main.rs:495:16:495:16 | y | Variable $@ is assigned a value that is never used. | main.rs:495:16:495:16 | y | y |
-| main.rs:496:12:496:12 | z | Variable $@ is assigned a value that is never used. | main.rs:496:12:496:12 | z | z |
-| main.rs:499:18:499:18 | x | Variable $@ is assigned a value that is never used. | main.rs:499:18:499:18 | x | x |
 | 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:9: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

@@ -491,12 +491,12 @@ fn references() {
 
 pub struct my_declaration {
     field1: fn(i32) -> i32,
-    field2: fn(x: i32) -> i32, // $ SPURIOUS: Alert[rust/unused-value]
-    field3: fn(y: // $ SPURIOUS: Alert[rust/unused-value]
-        fn(z: i32) -> i32) -> i32, // $ SPURIOUS: Alert[rust/unused-value]
+    field2: fn(x: i32) -> i32,
+    field3: fn(y:
+        fn(z: i32) -> i32) -> i32,
 }
 
-type MyType = fn(x: i32) -> i32; // $ SPURIOUS: Alert[rust/unused-value]
+type MyType = fn(x: i32) -> i32;
 
 trait MyTrait {
     fn my_func2(&self, x: i32) -> i32;