Rust: Check call arities in path resolution #20502
Conversation
hvitved
force-pushed
the
rust/path-resolution-check-arity
branch
2 times, most recently
from
569e231 to
d4bb0c6
Compare
hvitved
force-pushed
the
rust/path-resolution-check-arity
branch
from
d4bb0c6 to
e6b1e8e
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR improves path resolution in Rust analysis by introducing call arity checks to better distinguish between callable items (functions, structs, variants). It adds helper predicates getSelfParam and hasSelfParam to the Callable class and extends path resolution to consider the number of arguments when resolving function calls.
Key changes:
- Added helper methods for self parameter handling on callable items
- Introduced
CallableItemNodeabstract class with arity checks for structs, variants, and functions - Enhanced path resolution to match call expressions with their targets based on parameter count
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| HardcodedCryptographicValue.expected | Test expectation updates reflecting improved path resolution eliminating duplicates |
| PathResolutionConsistency.expected | Consistency test results showing reduced multiple call target issues |
| TypeInference.qll | Updated to use new getSelfParam() and hasSelfParam() helper methods |
| PathResolution.qll | Core changes adding CallableItemNode class and arity-based call resolution |
| VariableImpl.qll | Updated to use new getSelfParam() helper method |
| CallableImpl.qll | Added getSelfParam() and hasSelfParam() helper methods to Callable class |
| CallImpl.qll | Updated to use new getSelfParam() helper method |
| ControlFlowGraphImpl.qll | Updated to use new getSelfParam() helper method |
paldepind
left a comment
paldepind
left a comment
There was a problem hiding this comment.
Nice reduction in path resolution inconsistencies 👍
| arr = super.getNumberOfParams() and | ||
| if super.hasSelfParam() then result = arr + 1 else result = arr | ||
| ) | ||
| } |
There was a problem hiding this comment.
What about adding this to Callable instead as it's not tied to path resolution?
There was a problem hiding this comment.
Ok; calling it getNumberOfParamsInclSelf?
There was a problem hiding this comment.
That sounds very clear 👍
|
|
||
| private class VariantItemNode extends ItemNode instanceof Variant { | ||
| /** An item that can be called with arguments. */ | ||
| abstract class CallableItemNode extends ItemNode { |
There was a problem hiding this comment.
I think it's a bit confusing that CallableItemNode is a function or a struct, when we already have Callable which is a function or a closure. Could we name it something different? Maybe HasParameters?
2 participants
Also introduces helper predicates
getSelfParamandhasSelfParamon theCallableclass.DCA shows a reduction in path resolution inconsistencies, which is expected.