-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Rust: Update PoemHandlerParam to use getCanonicalPath #19801
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?
Conversation
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
Updates the PoemHandlerParam predicate to use getCanonicalPath
on an inferred type rather than the deprecated getResolvedPath
.
- Added imports for
TypeInference
andType
- Switched from matching
getResolvedPath
to inferring the type and checking its canonical path - Refactored the
exists
clause to bind the inferred type and perform the new path comparison
Comments suppressed due to low confidence (3)
rust/ql/lib/codeql/rust/frameworks/Poem.qll:16
- [nitpick] The variable name
t
is ambiguous; consider renaming it toparamType
orinferredType
for clarity.
exists(TupleStructPat param, Type t |
rust/ql/lib/codeql/rust/frameworks/Poem.qll:12
- The doc comment still refers to handler parameters in terms of path resolution; update it to mention canonical path checking via type inference.
* Parameters of a handler function
rust/ql/lib/codeql/rust/frameworks/Poem.qll:16
- Add unit tests for
PoemHandlerParam
to cover bothQuery
andPath
cases with the new canonical path logic.
exists(TupleStructPat param, Type t |
t = inferType(param) and | ||
t.(StructType).asItemNode().(Addressable).getCanonicalPath() = |
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 think it would be better to add this predicate to TupleStructPatImpl.qll
:
/** Gets the struct matched by this pattern. */
Struct getStruct() { result = PathResolution::resolvePath(this.getPath()) }
and then change the above to param.getStruct().getCanonicalPath() = ...
.
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.
Updated.
this.asPat().getPat() = param.getAField() | ||
exists(TupleStructPat param, Type t | | ||
this.asPat().getPat() = param.getAField() and | ||
t = inferType(param) and |
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.
Should now be removed (as well as the two imports).
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.
Sorry, the heat must be getting to me. Now done.
Update
PoemHandlerParam
to usegetCanonicalPath
rather thangetResolvedPath
.@hvitved