Skip to content

Rust: Extend jump-to-def query with method calls #19809

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

Merged
merged 1 commit into from
Jun 18, 2025

Conversation

hvitved
Copy link
Contributor

@hvitved hvitved commented Jun 18, 2025

Also fixes jump-to-def for paths so only the path segment is used, for example in M1::M2 only the M2 segment jumps to the M2 module, not the entire M1::M2 path).

@github-actions github-actions bot added the Rust Pull requests that update Rust code label Jun 18, 2025
@hvitved hvitved force-pushed the rust/jump-to-method-call branch from aae01b9 to 5cd7295 Compare June 18, 2025 07:27
@hvitved hvitved marked this pull request as ready for review June 18, 2025 07:29
@Copilot Copilot AI review requested due to automatic review settings June 18, 2025 07:29
@hvitved hvitved requested a review from a team as a code owner June 18, 2025 07:29
Copy link
Contributor

@Copilot Copilot AI left a 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 enhances the jump-to-definition query in the Rust CodeQL library to handle method calls and to restrict path-based definitions to individual segments (e.g., only M2 in M1::M2).

  • Introduces a new MethodUse class to resolve method call identifiers
  • Refines PathUse to work on PathSegment nodes and include call disambiguation
  • Updates test suite to verify method and segmented-path resolutions

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
rust/ql/test/library-tests/definitions/main.rs Adds a nested module with S::method and corresponding use in main for testing method-resolution
rust/ql/test/library-tests/definitions/Definitions.expected Updates expected definitions output to include new path-segment and method entries
rust/ql/lib/codeql/rust/internal/Definitions.qll Extends PathUse to PathSegment, adds call-aware logic, and introduces MethodUse for method call resolution
Comments suppressed due to low confidence (2)

rust/ql/lib/codeql/rust/internal/Definitions.qll:143

  • [nitpick] The method name getCall() is somewhat vague; consider renaming it to getAssociatedCallExpr() or similar to clarify its purpose.
  private CallExpr getCall() { result.getFunction().(PathExpr).getPath() = path }

rust/ql/lib/codeql/rust/internal/Definitions.qll:138

  • [nitpick] Add a brief comment above this class explaining that it handles individual path segments (including calls) for jump-to-definition resolutions, to aid future readers.
private class PathUse extends Use instanceof PathSegment {

Comment on lines +146 to +149
// Our call resolution logic may disambiguate some calls, so only use those
result.asItemNode() = this.getCall().getStaticTarget()
or
not exists(this.getCall()) and
Copy link
Preview

Copilot AI Jun 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider caching the result of getCall() in a local variable or predicate to avoid traversing the AST twice in getDefinition, improving performance.

Suggested change
// Our call resolution logic may disambiguate some calls, so only use those
result.asItemNode() = this.getCall().getStaticTarget()
or
not exists(this.getCall()) and
// Cache the result of getCall() to avoid traversing the AST twice
CallExpr cachedCall = this.getCall();
// Our call resolution logic may disambiguate some calls, so only use those
result.asItemNode() = cachedCall.getStaticTarget()
or
not exists(cachedCall) and

Copilot uses AI. Check for mistakes.

@hvitved hvitved merged commit e5ef09a into github:main Jun 18, 2025
17 checks passed
@hvitved hvitved deleted the rust/jump-to-method-call branch June 18, 2025 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants