Rust: Exclude results inside macro expansions from unused entity queries

Author: hvitved

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

@@ -26,21 +26,29 @@ module Impl {
     result = getImmediateParent(e)
   }
 
-  /** Gets the nearest enclosing parent of `ast` that is an `AstNode`. */
-  private AstNode getParentOfAst(AstNode ast) {
-    result = getParentOfAstStep*(getImmediateParent(ast))
-  }
-
   class AstNode extends Generated::AstNode {
+    /**
+     * Gets the nearest enclosing parent of this node, which is also an `AstNode`,
+     * if any.
+     */
+    AstNode getParentNode() { result = getParentOfAstStep*(getImmediateParent(this)) }
+
     /** Gets the immediately enclosing callable of this node, if any. */
     cached
     Callable getEnclosingCallable() {
-      exists(AstNode p | p = getParentOfAst(this) |
+      exists(AstNode p | p = this.getParentNode() |
         result = p
         or
         not p instanceof Callable and
         result = p.getEnclosingCallable()
       )
     }
+
+    /** Holds if this node is inside a macro expansion. */
+    predicate isInMacroExpansion() {
+      this = any(MacroCall mc).getExpanded()
+      or
+      this.getParentNode().isInMacroExpansion()
+    }
   }
 }

rust/ql/src/queries/unusedentities/UnreachableCode.ql

@@ -29,7 +29,10 @@ predicate hiddenNode(AstNode n) {
   not succ(n, _) and
   not succ(_, n)
   or
-  n instanceof ControlFlowGraphImpl::PostOrderTree // location is counter-intuitive
+  n instanceof ControlFlowGraphImpl::PostOrderTree and // location is counter-intuitive
+  not n instanceof MacroExpr
+  or
+  n.isInMacroExpansion()
 }
 
 /**

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

@@ -20,5 +20,6 @@ where
   not write = any(Ssa::WriteDefinition def).getWriteAccess().getAstNode() and
   // avoid overlap with the unused variable query
   not isUnused(v) and
-  not v instanceof DiscardVariable
+  not v instanceof DiscardVariable and
+  not write.isInMacroExpansion()
 select write, "Variable '" + v + "' is assigned a value that is never used."

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

@@ -10,5 +10,6 @@ predicate isUnused(Variable v) {
   not exists(v.getAnAccess()) and
   not exists(v.getInitializer()) and
   not v instanceof DiscardVariable and
+  not v.getPat().isInMacroExpansion() and
   exists(File f | f.getBaseName() = "main.rs" | v.getLocation().getFile() = f) // temporarily severely limit results
 }

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

@@ -3,8 +3,8 @@
 | unreachable.rs:31:9:31:23 | ExprStmt | This code is never reached. |
 | unreachable.rs:38:9:38:23 | ExprStmt | This code is never reached. |
 | unreachable.rs:59:5:59:19 | ExprStmt | This code is never reached. |
-| unreachable.rs:106:13:106:20 | ExprStmt | This code is never reached. |
-| unreachable.rs:115:13:115:20 | ExprStmt | This code is never reached. |
+| unreachable.rs:106:13:106:20 | MacroExpr | This code is never reached. |
+| unreachable.rs:115:13:115:20 | MacroExpr | This code is never reached. |
 | unreachable.rs:141:5:141:19 | ExprStmt | This code is never reached. |
 | unreachable.rs:148:9:148:23 | ExprStmt | This code is never reached. |
 | unreachable.rs:157:13:157:27 | ExprStmt | This code is never reached. |

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

@@ -11,11 +11,7 @@
 | main.rs:87:9:87:9 | a | Variable 'a' is assigned a value that is never used. |
 | main.rs:108:9:108:10 | is | Variable 'is' is assigned a value that is never used. |
 | main.rs:131:13:131:17 | total | Variable 'total' is assigned a value that is never used. |
-| main.rs:194:13:194:31 | res | Variable 'res' is assigned a value that is never used. |
-| main.rs:206:9:206:24 | kind | Variable 'kind' is assigned a value that is never used. |
-| main.rs:210:9:210:32 | kind | Variable 'kind' is assigned a value that is never used. |
 | main.rs:266:13:266:17 | total | Variable 'total' is assigned a value that is never used. |
-| main.rs:334:5:334:39 | kind | Variable 'kind' is assigned a value that is never used. |
 | main.rs:359:9:359:9 | x | Variable 'x' is assigned a value that is never used. |
 | main.rs:367:17:367:17 | x | Variable 'x' is assigned a value that is never used. |
 | more.rs:24:9:24:11 | val | Variable 'val' is assigned a value that is never used. |

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

@@ -191,7 +191,7 @@ fn loops() {
     }
 
     for x in 1..10 {
-        _ = format!("x is {x}"); // $ SPURIOUS: Alert[rust/unused-value]
+        _ = format!("x is {x}");
     }
 
     for x in 1..10 {
@@ -203,11 +203,11 @@ fn loops() {
     }
 
     for x in 1..10 {
-        assert_eq!(x, 1); // $ SPURIOUS: Alert[rust/unused-value]
+        assert_eq!(x, 1);
     }
 
     for x in 1..10 {
-        assert_eq!(id(x), id(1)); // $ SPURIOUS: Alert[rust/unused-value]
+        assert_eq!(id(x), id(1));
     }
 }
 
@@ -331,7 +331,7 @@ fn if_lets_matches() {
     }
 
     let duration1 = std::time::Duration::new(10, 0); // ten seconds
-    assert_eq!(duration1.as_secs(), 10); // $ SPURIOUS: Alert[rust/unused-value]
+    assert_eq!(duration1.as_secs(), 10);
 
     let duration2: Result<std::time::Duration, String> = Ok(std::time::Duration::new(10, 0));
     match duration2 {