-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Rust: Remove elements superseded by attribute macro expansions #20770
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Rust: Remove elements superseded by attribute macro expansions #20770
Conversation
e305f21 to
20bb58d
Compare
20bb58d to
c81f5f5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors macro expansion handling in the Rust QL library by consolidating logic into a central MacroExpansion module and introducing the isFromMacroExpansion() predicate to better distinguish between macro arguments and macro-generated code. The changes improve code organization, reduce duplication, and update test expectations to reflect the new behavior.
- Centralized macro expansion logic from
MacroCallImplandPathResolutionintoElementImpl::MacroExpansion - Introduced
isFromMacroExpansion()to exclude macro arguments from expansion checks - Updated queries to use the new predicate where appropriate
- Added custom
toString()for items with attribute macro expansions
Reviewed Changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
rust/ql/lib/codeql/rust/elements/internal/ElementImpl.qll |
Adds centralized MacroExpansion module with logic for classifying macro-related elements |
rust/ql/lib/codeql/rust/elements/internal/AstNodeImpl.qll |
Updates isInMacroExpansion() and isFromMacroExpansion() to delegate to MacroExpansion module |
rust/ql/lib/codeql/rust/elements/internal/MacroCallImpl.qll |
Removes isInMacroExpansion() and getATokenTreeNode() logic, now in ElementImpl |
rust/ql/lib/codeql/rust/elements/internal/ItemImpl.qll |
Adds custom toString() for items with attribute macro expansions |
rust/ql/lib/codeql/rust/elements/internal/LocatableImpl.qll |
Updates to use new getImmediatelyEnclosingMacroInvocation() helper |
rust/ql/lib/codeql/rust/internal/PathResolution.qll |
Simplifies ItemNode constructor by removing supersededByAttributeMacroExpansion() |
rust/ql/src/queries/unusedentities/UnusedValue.ql |
Updates to use isFromMacroExpansion() instead of isInMacroExpansion() |
rust/ql/src/queries/unusedentities/UnreachableCode.ql |
Updates to use isFromMacroExpansion() instead of isInMacroExpansion() |
rust/ql/src/queries/unusedentities/UnusedVariable.qll |
Adds TODO comment noting future migration to isFromMacroExpansion() |
rust/ql/lib/utils/test/PathResolutionInlineExpectationsTest.qll |
Simplifies itemAt() by removing unused inMacro parameter |
rust/ql/test/library-tests/variables/variables.ql |
Updates to use isFromMacroExpansion() instead of isInMacroExpansion() |
| Test expectation files | Updates expected test outputs to reflect deduplication and new behavior |
.gitattributes and .generated.list |
Removes ItemImpl.qll from generated file list |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
| let a: u16; | ||
| let b: u16 = 2; | ||
| set_value!(a, 1); | ||
| set_value!(a, 1); // $ Alert[rust/unused-value] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
I'm happy with all of the changes to test results in this PR, and I've made a comment in the DCA run about the results there. I haven't reviewed much of the actual QL (yet).
Follows up on #20454 and removes elements superseded by attribute macro expansions entirely from the
Elementtype. We only ever care about the expanded version, which will typically contain a copy of the original definition.DCA looks good: we reduce inconsistencies, increase call resolution rates, and eliminate
rust/unused-variablefalse positives.