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

[geoffw0]

Improve rust/unused-variable and rust/unused-value, by excluding results in functions that contain unexpanded macros. In these functions, there may be and often are accesses of the supposedly unused variables / values that we simply can't see.

In my local testing this removed about 80% of results, for rust/unused-variable the quality of remaining results seemed much better (on average), though for rust/unused-value even the remaining results were still mostly false positives (note that the latter query has a lower @precision).

DCA will confirm.

[Copilot]

PR Overview

This PR improves the filtering of unused variable and unused value alerts in Rust by excluding results in functions that contain unexpanded macros. Key changes include:

• Addition of new test functions (macros1–macros6, hygiene_mismatch) to cover macro expansion scenarios.
• Adjustments in test annotations to identify missing and spurious alerts.
• A configuration update in options.yml to disable Cargo checking.

Reviewed Changes

File	Description
rust/ql/test/query-tests/unusedentities/main.rs	Added and updated test cases for macros and hygiene scenarios.
rust/ql/test/query-tests/unusedentities/options.yml	Updated testing option to disable cargo check.

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

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

• The assignment inside the block in macros6 does not trigger the expected 'unused-value' alert. Verify if the macro expansion behavior should indeed trigger an unused value alert or update the test annotation accordingly.

x = 10; // $ MISSING: Alert[rust/unused-value]

Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more

[aibaars]

Looks good to me

[aibaars]

Some expected output still needs an update:

Cannot find ExtractionConsistency.expected file.
--- expected
+++ actual
@@ -1,1 +1,4 @@
-
+extractionWarning
+| main.rs:512:5:512:28 | macro expansion failed: could not resolve macro 'undefined_macro_call' |
+| main.rs:516:5:516:28 | macro expansion failed: could not resolve macro 'undefined_macro_call' |
+| main.rs:522:13:522:36 | macro expansion failed: could not resolve macro 'undefined_macro_call' |
Error: [373/1788 comp 15.9s eval 421ms] FAILED(RESULT) /home/runner/work/semmle-code/semmle-code/ql/rust/ql/test/query-tests/unusedentities/CONSISTENCY/ExtractionConsistency.ql

[geoffw0]

DCA shows 1509 less results for rust/unused-variable, fairly evenly across all projects. It's too many to display, but I've now checked a sample of results for four of these projects locally and both queries, I'm happy with the result changes. There's a lot less noise (but still some) now.

[paldepind]

This seems like a great way to reduce the number of FPs. I've left one comment and otherwise it looks great.

[hvitved]

While this works, it would perhaps be more correct to only exclude results when there is an unexpandable macro in the scope that the variable is defined. However, I realize that the concept of variable scope is not currently exposed, so perhaps that is better done follow-up.

[geoffw0]

What is required to expose the concept of a variable scope?

I've added a test that exposes the situation I think we're talking about here.

[hvitved]

We would have to expose the VariableScope class from VariableImpl.qll.

[geoffw0]

OK, I've added it to the existing improvements issue for rust/unused-variable. It sounds like a straightforward follow-up, apart from the need to go through another cycle of DCA / testing.

Is there any reason VariableScope shouldn't be exposed publically?

[hvitved]

Is there any reason VariableScope shouldn't be exposed publically?

Only that we haven't needed it before. We may also have to change it slightly, for example right now x and y are considered to be in the same scope in

let x = ...;
let y = ...;