Rust: Add Operation class #19454
Rust: Add Operation class #19454
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a unified Operation class in the Rust QL library, letting all unary, binary, assignment, and logical expressions share a consistent interface with getOperatorName() and getAnOperand().
- Defined
OperationImpl::Operationand aliased it asOperation - Updated
PrefixExprandBinaryExprimplementations to mix inOperation - Added imports of
Operationin query modules and extended tests to exercise the new API
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| rust/ql/lib/codeql/rust/elements/Operation.qll | Added the abstract Operation class and alias |
| rust/ql/lib/codeql/rust/elements/internal/PrefixExprImpl.qll | Extended PrefixExpr to implement Operation |
| rust/ql/lib/codeql/rust/elements/internal/BinaryExprImpl.qll | Extended BinaryExpr to implement Operation |
| rust/ql/test/library-tests/operations/test.rs | Expanded inline tests for various operators |
Comments suppressed due to low confidence (1)
rust/ql/test/library-tests/operations/test.rs:54
- No inline expectation is provided for the
?try operator. Consider adding an expectation comment, for example:// $ Operation Op=? Operands=1 PrefixExprto validate its classification and operands.
res?;
|
DCA shows nothing interesting, which is what we'd expect for a change like this. |
|
I wonder if we should use the term "operator" instead of "operation"? I think that's a more precise term for this. |
Perhaps. I think of the "operator" as the Another naming issue is that we currently have |
Co-authored-by: Simon Friis Vindum <paldepind@github.com>
If other languages uses "operation" then we should probably just stick to that yes. But as someone not familiar with these conventions I find "operator" much clearer than "operation". For the distinction between and operator and its application something like
Something along those lines sounds like a good idea to me. Would renaming |
I can see that.
Yes I think that will be the right way to do it. |
Unify various classes representing operations under a single
Operationclass, providing a single consistent interface (withgetOperatorName()andgetAnOperand()) for reasoning about them.The idea is to get things in order before I follow up with another PR that will add new subclasses, allowing us to find things like the dereference expression (
*) and comparison operations more easily.