- Notifications
You must be signed in to change notification settings - Fork1.7k
Rust: Rename type inference test inline expectation tag#20037
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Rust: Rename type inference test inline expectation tag#20037
Uh oh!
There was an error while loading.Please reload this page.
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
This PR renames the inline expectation tag from "method" to "target" in the Rust type inference test to better reflect the change made in#19993, wheregetStaticTarget was introduced to annotate all function calls (not just method calls). The tag name "target" is more accurate since it now covers both method calls and function calls.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| rust/ql/test/library-tests/type-inference/type-inference.ql | Updates the query to use "target" tag instead of "method" in relevant tag definitions |
| rust/ql/test/library-tests/type-inference/type-inference.expected | Updates test expectations to reflect the tag name change |
| rust/ql/test/library-tests/type-inference/pattern_matching.rs | Updates inline comments to use "target" tag instead of "method" |
| rust/ql/test/library-tests/type-inference/main.rs | Updates inline comments to use "target" tag instead of "method" |
| rust/ql/test/library-tests/type-inference/loop/main.rs | Updates inline comment to use "target" tag |
| rust/ql/test/library-tests/type-inference/dereference.rs | Updates inline comments to use "target" tag instead of "method" |
| rust/ql/lib/utils/test/internal/InlineExpectationsTestImpl.qll | Adds constructor check to ExpectationComment class |
| @@ -4,6 +4,8 @@ private import codeql.util.test.InlineExpectationsTest | |||
| module Impl implements InlineExpectationsTestSig { | |||
| class ExpectationComment extends R::Comment { | |||
| ExpectationComment() { this.fromSource() } | |||
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.
There was a comment inside the standard library starting with$target, so I had to limit expectations comments to those inside the source, which seems right in any case.
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.
Yep this is better, LGTM.
I've merged |
87a8dcc intogithub:mainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
#19993 changed the inline expectations in the type inference test such that all function calls (and not just method calls) are annotated by using
getStaticTargetin the tests. This makes the tag namemethoda bit inaccurate.This PR changes the tag name to
target.