Skip to content

Rust: Jump-to-def for operations and indexing #20900

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+197 −48
Open

Rust: Jump-to-def for operations and indexing #20900

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ee06364
Rust: Add jump-to-def tests for operations and indexing
hvitved Nov 24, 2025
c81a1c2
Rust: Jump-to-def for operations and indexing
hvitved Nov 24, 2025
f04c359
Rust: Add more jump-to-def test cases and refine logic
hvitved Nov 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 73 additions & 0 deletions rust/ql/lib/codeql/rust/internal/Definitions.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,19 @@ abstract class Use extends Locatable {
* Gets the type of use.
*/
abstract string getUseType();

/**
* Holds if this element is at the specified location.
* The location spans column `startcolumn` of line `startline` to
* column `endcolumn` of line `endline` in file `filepath`.
* For more information, see
* [Providing locations in CodeQL queries](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
*/
predicate hasLocationInfo(
string filepath, int startline, int startcolumn, int endline, int endcolumn
) {
this.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
}
}

cached
Expand Down Expand Up @@ -166,6 +179,66 @@ private class MethodUse extends Use instanceof NameRef {
override string getUseType() { result = "method" }
}

// We don't have entities for the operator symbols, so we approximate a location.
// The location spans are chosen so that they align with rust-analyzer's jump-to-def
// behavior in VS Code, which means using weird locations where the end column is
// before the start column in the case of unary prefix operations.
private predicate operatorHasLocationInfo(
Operation o, string filepath, int startline, int startcolumn, int endline, int endcolumn
) {
o =
// `-x`; placing the cursor before `-` jumps to `neg`, placing it after jumps to `x`
any(PrefixExpr pe |
pe.getLocation().hasLocationInfo(filepath, startline, startcolumn, _, _) and
endline = startline and
endcolumn = startcolumn - 1
Copy link
Contributor

@geoffw0 geoffw0 Nov 26, 2025

Choose a reason for hiding this comment

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

I think this would fix the backwards-location issue on test line 67:

Suggested change
endcolumn = startcolumn - 1
endcolumn = startcolumn

Though I'm starting to get the impression you maybe want the location starting at column 5, ending at column 4 for some reason. If that's true you're going to have to explain why - it just seems invalid to me.

)
or
o =
// `x + y`: placing the cursor before or after `+` jumps to `add`
// `x+ y`: placing the cursor before `+` jumps to `x`, after `+` jumps to `add`
any(BinaryExpr be |
be.getLhs().getLocation().hasLocationInfo(filepath, _, _, startline, startcolumn - 2) and
be.getRhs().getLocation().hasLocationInfo(filepath, endline, endcolumn + 2, _, _) and
Comment on lines +201 to +202
Copy link
Contributor

@geoffw0 geoffw0 Nov 26, 2025

Choose a reason for hiding this comment

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

Is there any reason we don't take all the space between the LHS and RHS?

Suggested change
be.getLhs().getLocation().hasLocationInfo(filepath, _, _, startline, startcolumn - 2) and
be.getRhs().getLocation().hasLocationInfo(filepath, endline, endcolumn + 2, _, _) and
be.getLhs().getLocation().hasLocationInfo(filepath, _, _, startline, startcolumn - 1) and
be.getRhs().getLocation().hasLocationInfo(filepath, endline, endcolumn + 1, _, _) and

(
startline < endline
or
endcolumn = startcolumn
Copy link
Contributor

@geoffw0 geoffw0 Nov 26, 2025

Choose a reason for hiding this comment

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

I think this would be sufficient:

Suggested change
endcolumn = startcolumn
startcolumn <= endcolumn

Though it seems like this check is only needed because you offset by 2 places instead of 1, above.

)
)
}

private class OperationUse extends Use instanceof Operation {
OperationUse() { operatorHasLocationInfo(this, _, _, _, _, _) }

override Definition getDefinition() { result.asItemNode() = this.(Call).getStaticTarget() }

override string getUseType() { result = "method" }

override predicate hasLocationInfo(
string filepath, int startline, int startcolumn, int endline, int endcolumn
) {
operatorHasLocationInfo(this, filepath, startline, startcolumn, endline, endcolumn)
}
}

private class IndexExprUse extends Use instanceof IndexExpr {
override Definition getDefinition() { result.asItemNode() = this.(Call).getStaticTarget() }

override string getUseType() { result = "method" }

// We don't have entities for the bracket symbols, so approximate a location
// The location spans are chosen so that they align with rust-analyzer's jump-to-def
// behavior in VS Code.
override predicate hasLocationInfo(
string filepath, int startline, int startcolumn, int endline, int endcolumn
) {
// `x[y]`: placing the cursor after `]` jumps to `index`
super.getIndex().getLocation().hasLocationInfo(filepath, _, _, startline, startcolumn - 2) and
Copy link
Contributor

@geoffw0 geoffw0 Nov 26, 2025

Choose a reason for hiding this comment

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

Suggested change
super.getIndex().getLocation().hasLocationInfo(filepath, _, _, startline, startcolumn - 2) and
super.getIndex().getLocation().hasLocationInfo(filepath, _, _, startline, startcolumn - 1) and

?

this.getLocation().hasLocationInfo(_, _, _, endline, endcolumn)
Comment on lines +237 to +238
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The location calculation results in inverted columns where end column is before start column. For an index expression like S[0] on line 69, the expected test output shows 69:9:69:8 (start column 9, end column 8).

Looking at the logic:

  • Line 214: super.getIndex().getLocation().hasLocationInfo(filepath, _, _, startline, startcolumn - 2) - This tries to get a location where the index ends at startcolumn - 2, which would place startcolumn approximately at the opening bracket [
  • Line 215: this.getLocation().hasLocationInfo(_, _, _, endline, endcolumn) - This gets the end of the entire IndexExpr

The issue is that getIndex() returns the index expression (e.g., 0), and this code assumes its end location minus 2 gives the bracket position. However, the arithmetic appears incorrect, resulting in endcolumn < startcolumn.

Consider revising the location calculation to ensure endcolumn >= startcolumn, or compute the bracket positions more accurately.

Suggested change
super.getIndex().getLocation().hasLocationInfo(filepath, _, _, startline, startcolumn - 2) and
this.getLocation().hasLocationInfo(_, _, _, endline, endcolumn)
this.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)

Copilot uses AI. Check for mistakes.
}
}

private class FileUse extends Use instanceof Name {
override Definition getDefinition() {
exists(Module m |
Expand Down
133 changes: 85 additions & 48 deletions rust/ql/test/library-tests/definitions/Definitions.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,48 +1,85 @@
| main.rs:3:5:3:7 | lib | lib.rs:1:1:1:1 | SourceFile | file |
| main.rs:9:14:9:14 | S | main.rs:7:9:7:21 | struct S | path |
| main.rs:10:36:10:39 | Self | main.rs:7:9:7:21 | struct S | path |
| main.rs:11:17:11:17 | S | main.rs:7:9:7:21 | struct S | path |
| main.rs:16:22:16:22 | T | main.rs:16:19:16:19 | T | path |
| main.rs:18:13:18:14 | S2 | main.rs:16:5:16:24 | struct S2 | path |
| main.rs:18:16:18:16 | T | main.rs:18:10:18:10 | T | path |
| main.rs:19:23:19:23 | T | main.rs:18:10:18:10 | T | path |
| main.rs:19:29:19:32 | Self | main.rs:16:5:16:24 | struct S2 | path |
| main.rs:20:13:20:14 | S2 | main.rs:16:5:16:24 | struct S2 | path |
| main.rs:20:16:20:16 | x | main.rs:19:20:19:20 | x | local variable |
| main.rs:29:5:29:11 | println | {EXTERNAL LOCATION} | MacroRules | path |
| main.rs:29:22:29:26 | value | main.rs:29:50:29:54 | value | format argument |
| main.rs:29:29:29:33 | width | main.rs:26:9:26:13 | width | local variable |
| main.rs:29:36:29:44 | precision | main.rs:27:9:27:17 | precision | local variable |
| main.rs:30:5:30:11 | println | {EXTERNAL LOCATION} | MacroRules | path |
| main.rs:30:22:30:22 | 0 | main.rs:30:34:30:38 | value | format argument |
| main.rs:30:25:30:25 | 1 | main.rs:30:41:30:45 | width | format argument |
| main.rs:30:28:30:28 | 2 | main.rs:30:48:30:56 | precision | format argument |
| main.rs:30:34:30:38 | value | main.rs:28:9:28:13 | value | local variable |
| main.rs:30:41:30:45 | width | main.rs:26:9:26:13 | width | local variable |
| main.rs:30:48:30:56 | precision | main.rs:27:9:27:17 | precision | local variable |
| main.rs:31:5:31:11 | println | {EXTERNAL LOCATION} | MacroRules | path |
| main.rs:31:21:31:22 | {} | main.rs:31:29:31:33 | value | format argument |
| main.rs:31:24:31:25 | {} | main.rs:31:36:31:40 | width | format argument |
| main.rs:31:29:31:33 | value | main.rs:28:9:28:13 | value | local variable |
| main.rs:31:36:31:40 | width | main.rs:26:9:26:13 | width | local variable |
| main.rs:33:5:33:11 | println | {EXTERNAL LOCATION} | MacroRules | path |
| main.rs:33:22:33:27 | people | main.rs:32:9:32:14 | people | local variable |
| main.rs:34:5:34:11 | println | {EXTERNAL LOCATION} | MacroRules | path |
| main.rs:34:16:34:16 | 1 | main.rs:34:34:34:34 | 2 | format argument |
| main.rs:34:19:34:20 | {} | main.rs:34:31:34:31 | 1 | format argument |
| main.rs:34:23:34:23 | 0 | main.rs:34:31:34:31 | 1 | format argument |
| main.rs:34:26:34:27 | {} | main.rs:34:34:34:34 | 2 | format argument |
| main.rs:35:5:35:13 | assert_eq | {EXTERNAL LOCATION} | MacroRules | path |
| main.rs:35:16:35:21 | format | {EXTERNAL LOCATION} | MacroRules | path |
| main.rs:35:31:35:35 | {:<5} | main.rs:35:40:35:42 | "x" | format argument |
| main.rs:36:13:36:13 | S | main.rs:1:1:1:9 | struct S | path |
| main.rs:37:13:37:14 | M1 | main.rs:5:1:23:1 | mod M1 | path |
| main.rs:37:17:37:18 | M2 | main.rs:6:5:14:5 | mod M2 | path |
| main.rs:37:21:37:21 | S | main.rs:7:9:7:21 | struct S | path |
| main.rs:38:5:38:5 | s | main.rs:37:9:37:9 | s | local variable |
| main.rs:38:7:38:12 | method | main.rs:10:13:12:13 | fn method | method |
| main.rs:39:5:39:6 | M1 | main.rs:5:1:23:1 | mod M1 | path |
| main.rs:39:9:39:10 | S2 | main.rs:16:5:16:24 | struct S2 | path |
| main.rs:39:14:39:14 | S | main.rs:1:1:1:9 | struct S | path |
| main.rs:39:18:39:20 | new | main.rs:19:9:21:9 | fn new | path |
| main.rs:39:22:39:22 | S | main.rs:1:1:1:9 | struct S | path |
| main.rs:1:5:1:7 | std | {EXTERNAL LOCATION} | Crate([email protected]) | path |
| main.rs:1:10:1:12 | ops | {EXTERNAL LOCATION} | mod ops | path |
| main.rs:3:3:3:8 | derive | {EXTERNAL LOCATION} | MacroDef | path |
| main.rs:6:5:6:7 | lib | lib.rs:1:1:1:1 | SourceFile | file |
| main.rs:8:6:8:8 | Neg | {EXTERNAL LOCATION} | trait Neg | path |
| main.rs:8:14:8:14 | S | main.rs:3:1:4:9 | struct S | path |
| main.rs:9:19:9:22 | Self | main.rs:3:1:4:9 | struct S | path |
| main.rs:11:21:11:24 | Self | main.rs:3:1:4:9 | struct S | path |
| main.rs:12:9:12:9 | S | main.rs:3:1:4:9 | struct S | path |
| main.rs:16:6:16:8 | Add | {EXTERNAL LOCATION} | trait Add | path |
| main.rs:16:14:16:14 | S | main.rs:3:1:4:9 | struct S | path |
| main.rs:17:19:17:22 | Self | main.rs:3:1:4:9 | struct S | path |
| main.rs:19:23:19:26 | Self | main.rs:3:1:4:9 | struct S | path |
| main.rs:19:32:19:35 | Self | main.rs:3:1:4:9 | struct S | path |
| main.rs:20:9:20:9 | S | main.rs:3:1:4:9 | struct S | path |
| main.rs:24:6:24:10 | Index | {EXTERNAL LOCATION} | trait Index | path |
| main.rs:24:12:24:16 | usize | {EXTERNAL LOCATION} | struct usize | path |
| main.rs:24:23:24:23 | S | main.rs:3:1:4:9 | struct S | path |
| main.rs:25:19:25:19 | S | main.rs:3:1:4:9 | struct S | path |
| main.rs:27:28:27:32 | usize | {EXTERNAL LOCATION} | struct usize | path |
| main.rs:27:39:27:42 | Self | main.rs:24:1:30:1 | impl Index::<...> for S { ... } | path |
| main.rs:27:45:27:50 | Output | main.rs:25:5:25:20 | type Output | path |
| main.rs:28:10:28:10 | S | main.rs:3:1:4:9 | struct S | path |
| main.rs:36:14:36:14 | S | main.rs:34:9:34:21 | struct S | path |
| main.rs:37:36:37:39 | Self | main.rs:34:9:34:21 | struct S | path |
| main.rs:38:17:38:17 | S | main.rs:34:9:34:21 | struct S | path |
| main.rs:43:22:43:22 | T | main.rs:43:19:43:19 | T | path |
| main.rs:45:13:45:14 | S2 | main.rs:43:5:43:24 | struct S2 | path |
| main.rs:45:16:45:16 | T | main.rs:45:10:45:10 | T | path |
| main.rs:46:23:46:23 | T | main.rs:45:10:45:10 | T | path |
| main.rs:46:29:46:32 | Self | main.rs:43:5:43:24 | struct S2 | path |
| main.rs:47:13:47:14 | S2 | main.rs:43:5:43:24 | struct S2 | path |
| main.rs:47:16:47:16 | x | main.rs:46:20:46:20 | x | local variable |
| main.rs:56:5:56:11 | println | {EXTERNAL LOCATION} | MacroRules | path |
| main.rs:56:22:56:26 | value | main.rs:56:50:56:54 | value | format argument |
| main.rs:56:29:56:33 | width | main.rs:53:9:53:13 | width | local variable |
| main.rs:56:36:56:44 | precision | main.rs:54:9:54:17 | precision | local variable |
| main.rs:57:5:57:11 | println | {EXTERNAL LOCATION} | MacroRules | path |
| main.rs:57:22:57:22 | 0 | main.rs:57:34:57:38 | value | format argument |
| main.rs:57:25:57:25 | 1 | main.rs:57:41:57:45 | width | format argument |
| main.rs:57:28:57:28 | 2 | main.rs:57:48:57:56 | precision | format argument |
| main.rs:57:34:57:38 | value | main.rs:55:9:55:13 | value | local variable |
| main.rs:57:41:57:45 | width | main.rs:53:9:53:13 | width | local variable |
| main.rs:57:48:57:56 | precision | main.rs:54:9:54:17 | precision | local variable |
| main.rs:58:5:58:11 | println | {EXTERNAL LOCATION} | MacroRules | path |
| main.rs:58:21:58:22 | {} | main.rs:58:29:58:33 | value | format argument |
| main.rs:58:24:58:25 | {} | main.rs:58:36:58:40 | width | format argument |
| main.rs:58:29:58:33 | value | main.rs:55:9:55:13 | value | local variable |
| main.rs:58:36:58:40 | width | main.rs:53:9:53:13 | width | local variable |
| main.rs:60:5:60:11 | println | {EXTERNAL LOCATION} | MacroRules | path |
| main.rs:60:22:60:27 | people | main.rs:59:9:59:14 | people | local variable |
| main.rs:61:5:61:11 | println | {EXTERNAL LOCATION} | MacroRules | path |
| main.rs:61:16:61:16 | 1 | main.rs:61:34:61:34 | 2 | format argument |
| main.rs:61:19:61:20 | {} | main.rs:61:31:61:31 | 1 | format argument |
| main.rs:61:23:61:23 | 0 | main.rs:61:31:61:31 | 1 | format argument |
| main.rs:61:26:61:27 | {} | main.rs:61:34:61:34 | 2 | format argument |
| main.rs:62:5:62:13 | assert_eq | {EXTERNAL LOCATION} | MacroRules | path |
| main.rs:62:16:62:21 | format | {EXTERNAL LOCATION} | MacroRules | path |
| main.rs:62:31:62:35 | {:<5} | main.rs:62:40:62:42 | "x" | format argument |
| main.rs:63:13:63:13 | S | main.rs:3:1:4:9 | struct S | path |
| main.rs:64:13:64:14 | M1 | main.rs:32:1:50:1 | mod M1 | path |
| main.rs:64:17:64:18 | M2 | main.rs:33:5:41:5 | mod M2 | path |
| main.rs:64:21:64:21 | S | main.rs:34:9:34:21 | struct S | path |
| main.rs:65:5:65:5 | s | main.rs:64:9:64:9 | s | local variable |
| main.rs:65:7:65:12 | method | main.rs:37:13:39:13 | fn method | method |
| main.rs:66:5:66:6 | M1 | main.rs:32:1:50:1 | mod M1 | path |
| main.rs:66:9:66:10 | S2 | main.rs:43:5:43:24 | struct S2 | path |
| main.rs:66:14:66:14 | S | main.rs:3:1:4:9 | struct S | path |
| main.rs:66:18:66:20 | new | main.rs:46:9:48:9 | fn new | path |
| main.rs:66:22:66:22 | S | main.rs:3:1:4:9 | struct S | path |
| main.rs:67:5:67:4 | - ... | main.rs:11:5:13:5 | fn neg | method |
Copy link
Contributor

@geoffw0 geoffw0 Nov 25, 2025

Choose a reason for hiding this comment

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

Shouldn't this be main.rs:67:5:67:5? i.e. endcolumn + 2 in the logic should be endcolumn + 1.

Copy link
Contributor Author

@hvitved hvitved Nov 25, 2025

Choose a reason for hiding this comment

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

That's what I did initially, but that means putting the cursor between - and S will jump to both neg and S, and I want to avoid overlapping targets. With the above, only putting the cursor before - will jump to neg.

Copy link
Contributor

@geoffw0 geoffw0 Nov 25, 2025

Choose a reason for hiding this comment

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

That's odd - S is clearly (from the line below) character 6, while - is character 5.

I'm not sure what you mean about "putting the cursor between" two characters - surely the granularity here is which single character you're pointing at?

Copy link
Contributor

@geoffw0 geoffw0 Nov 26, 2025

Choose a reason for hiding this comment

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

putting the cursor between - and S

I still don't know what you mean here. Surely your mouse is either over the - or over the S?

| main.rs:67:6:67:6 | S | main.rs:3:1:4:9 | struct S | path |
| main.rs:68:5:68:5 | S | main.rs:3:1:4:9 | struct S | path |
| main.rs:68:7:68:7 | ... + ... | main.rs:19:5:21:5 | fn add | method |
| main.rs:68:9:68:9 | S | main.rs:3:1:4:9 | struct S | path |
| main.rs:70:13:70:13 | S | main.rs:3:1:4:9 | struct S | path |
| main.rs:70:15:70:15 | S | main.rs:3:1:4:9 | struct S | path |
| main.rs:72:13:72:13 | S | main.rs:3:1:4:9 | struct S | path |
| main.rs:72:15:73:4 | ... + ... | main.rs:19:5:21:5 | fn add | method |
| main.rs:73:6:73:6 | S | main.rs:3:1:4:9 | struct S | path |
| main.rs:75:13:75:13 | S | main.rs:3:1:4:9 | struct S | path |
| main.rs:75:15:77:3 | ... + ... | main.rs:19:5:21:5 | fn add | method |
| main.rs:77:5:77:5 | S | main.rs:3:1:4:9 | struct S | path |
| main.rs:78:5:78:5 | S | main.rs:3:1:4:9 | struct S | path |
| main.rs:78:9:78:8 | S[0] | main.rs:27:5:29:5 | fn index | method |
39 changes: 39 additions & 0 deletions rust/ql/test/library-tests/definitions/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,34 @@
use std::ops::*;

#[derive(Debug, Copy, Clone)]
struct S;

mod lib;

impl Neg for S {
type Output = Self;

fn neg(self) -> Self {
S
}
}

impl Add for S {
type Output = Self;

fn add(self, rhs: Self) -> Self {
S
}
}

impl Index<usize> for S {
type Output = S;

fn index(&self, index: usize) -> &Self::Output {
&S
}
}

mod M1 {
pub mod M2 {
pub struct S;
Expand Down Expand Up @@ -37,4 +64,16 @@ fn main() {
let s = M1::M2::S;
s.method();
M1::S2::<S>::new(S);
-S;
S + S;
Copy link
Contributor

@geoffw0 geoffw0 Nov 25, 2025

Choose a reason for hiding this comment

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

I think there would be an issue if you'd wrote S+S here. We might be better off accepting that we can't assume spaces are present, and just include them in the location range of the +? i.e. replace startcolumn - 2 with startcolumn - 1 and endcolumn + 2 with endcolumn + 1.

Copy link
Contributor Author

@hvitved hvitved Nov 26, 2025

Choose a reason for hiding this comment

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

I have added more tests, refined the logic, and added more comments.

#[rustfmt::skip]
let x = S+S;
#[rustfmt::skip]
let x = S
+S;
#[rustfmt::skip]
let x = S
+
S;
S[0];
}