Skip to content

Commit

Permalink
Rust: Exclude results inside macro expansions from unused entity queries
Browse files Browse the repository at this point in the history
  • Loading branch information
hvitved committed Oct 29, 2024
1 parent 40dcc0e commit 072b836
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 16 deletions.
20 changes: 14 additions & 6 deletions rust/ql/lib/codeql/rust/elements/internal/AstNodeImpl.qll
Original file line number Diff line number Diff line change
Expand Up @@ -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 getParent() { 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.getParent() |
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.getParent().isInMacroExpansion()
}
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions rust/ql/src/queries/unusedentities/UnreachableCode.ql
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ predicate hiddenNode(AstNode n) {
not succ(_, n)
or
n instanceof ControlFlowGraphImpl::PostOrderTree // location is counter-intuitive
or
n.isInMacroExpansion()
}

/**
Expand Down
3 changes: 2 additions & 1 deletion rust/ql/src/queries/unusedentities/UnusedValue.ql
Original file line number Diff line number Diff line change
Expand Up @@ -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."
1 change: 1 addition & 0 deletions rust/ql/src/queries/unusedentities/UnusedVariable.qll
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
4 changes: 0 additions & 4 deletions rust/ql/test/query-tests/unusedentities/UnusedValue.expected
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
Expand Down
8 changes: 4 additions & 4 deletions rust/ql/test/query-tests/unusedentities/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ fn loops() {
}

for x in 1..10 {

Check notice

Code scanning / CodeQL

Unused variable Note test

Variable 'x' is not used.
_ = format!("x is {x}"); // $ SPURIOUS: Alert[rust/unused-value]
_ = format!("x is {x}");
}

for x in 1..10 {

Check notice

Code scanning / CodeQL

Unused variable Note test

Variable 'x' is not used.
Expand All @@ -203,11 +203,11 @@ fn loops() {
}

for x in 1..10 {

Check notice

Code scanning / CodeQL

Unused variable Note test

Variable is not used.
assert_eq!(x, 1); // $ SPURIOUS: Alert[rust/unused-value]
assert_eq!(x, 1);
}

for x in 1..10 {

Check notice

Code scanning / CodeQL

Unused variable Note test

Variable is not used.
assert_eq!(id(x), id(1)); // $ SPURIOUS: Alert[rust/unused-value]
assert_eq!(id(x), id(1));
}
}

Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 072b836

Please sign in to comment.