Rust: Adjust argument position when call expression is for method#18772
Conversation
There was a problem hiding this comment.
PR Overview
This PR adjusts the argument positions in method call expressions in the dataflow library and updates expected test annotations on path resolution. Key changes include updating inline comments to reflect the correct data flow values, altering method call usages (explicit function calls via associated functions), and adding missing test annotations for path resolution.
Changes
| File | Description |
|---|---|
| rust/ql/test/library-tests/dataflow/global/main.rs | Updates method call comments to include missing expected data flow annotations and modifies function visibility. |
| rust/ql/test/library-tests/path-resolution/main.rs | Adds a new impl block and updates method call annotations for path resolution, highlighting several missing expected item annotations. |
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (5)
rust/ql/test/library-tests/dataflow/global/main.rs:117
- The comment on the call in data_through_method_called_as_function is missing the expected 'hasValueFlow=12' annotation. Please add the missing annotation.
sink(b); // $ MISSING: hasValueFlow=12
rust/ql/test/library-tests/path-resolution/main.rs:185
- The method call on x is missing the expected 'item=I53' annotation. Please add the missing annotation in the inline comment.
x.f(); // $ MISSING: item=I53
rust/ql/test/library-tests/path-resolution/main.rs:183
- The trait reference is missing the expected 'item=I47' annotation. Please update the comment to include this identifier.
MyTrait // $ MISSING: item=I47
rust/ql/test/library-tests/path-resolution/main.rs:184
- The closing of the trait cast is missing the expected 'item=52' annotation. Please update the inline comment accordingly.
> // $ MISSING: item=52
rust/ql/test/library-tests/path-resolution/main.rs:187
- The call to x.g() is missing the expected 'item=I54' annotation. Please add the appropriate annotation to maintain consistency.
x.g(); // $ MISSING: item=I54
Tip: Copilot code review supports C#, Go, Java, JavaScript, Markdown, Python, Ruby and TypeScript, with more languages coming soon. Learn more
| impl MyFlag { | ||
| fn data_in(&self, n: i64) { | ||
| sink(n); // $ hasValueFlow=1 | ||
| sink(n); // $ hasValueFlow=1 MISSING: hasValueFlow=8 |
There was a problem hiding this comment.
The inline comment for the data_in call is missing the expected additional annotation 'hasValueFlow=8'. Please update the comment to include this value.
| sink(n); // $ hasValueFlow=1 MISSING: hasValueFlow=8 | |
| sink(n); // $ hasValueFlow=1 hasValueFlow=8 |
| pub fn g() { | ||
| let x = MyStruct {}; // $ item=I50 | ||
| MyTrait::f(&x); // $ item=I48 | ||
| MyStruct::f(&x); // $ MISSING: item=I53 |
There was a problem hiding this comment.
The call to MyStruct::f is missing the expected 'item=I53' annotation. Please update the inline comment accordingly.
| MyStruct::f(&x); // $ MISSING: item=I53 | |
| MyStruct::f(&x); // $ item=I53 |
| MyStruct::h(&x); // $ MISSING: item=I74 | ||
| x.h(); // $ MISSING: item=I74 |
There was a problem hiding this comment.
The call to MyStruct::h is missing the expected 'item=I74' annotation. Please update the comment to include this missing annotation.
| MyStruct::h(&x); // $ MISSING: item=I74 | |
| x.h(); // $ MISSING: item=I74 | |
| MyStruct::h(&x); // $ item=I74 | |
| x.h(); // $ item=I74 |
| let x = MyStruct {}; // $ item=I50 | ||
| x.g(); // $ MISSING: item=I54 | ||
| MyStruct::h(&x); // $ MISSING: item=I74 | ||
| x.h(); // $ MISSING: item=I74 |
There was a problem hiding this comment.
The method call x.h() is missing the expected 'item=I74' annotation. Please add the missing annotation in the inline comment.
| x.h(); // $ MISSING: item=I74 | |
| x.h(); // $ item=I74 |
| private predicate callToMethod(CallExpr call) { | ||
| exists(Path path | | ||
| path = call.getFunction().(PathExpr).getPath() and | ||
| path.hasQualifier() and |
There was a problem hiding this comment.
Why is this restriction needed?
There was a problem hiding this comment.
As far as I can tell, a method can only be called as a function by including a qualifying path. So if the path doesn't have a qualifier, then it will never resolve to a method. So I'm including it for clarity and (maybe) performance.
There was a problem hiding this comment.
Yeah, I think you are right.
This should take care of
Foo::method(...)calls in the data flow library. The last commit is the interesting one.Not all of the tests are fixed, due to limitations in the path resolution implementation. I've added a new test for the path resolution, and will look into that.