8000 Rust: Extend jump-to-def to include paths and `mod file;` imports by hvitved · Pull Request #19605 · github/codeql · GitHub
[go: up one dir, main page]
Skip to content

Rust: Extend jump-to-def to include paths and mod file; imports #19605

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

Merged
merged 2 commits into from
Jun 3, 2025
Merged

Rust: Extend jump-to-def to include paths and mod file; imports #19605

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d74e95f
Rust: Extend jump-to-def to include paths and `mod file;` imports
hvitved May 28, 2025
3781de7
Rust: Reorder columns in `Definitions.ql` test
hvitved Jun 3, 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
33 changes: 30 additions & 3 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 @@ -3,6 +3,7 @@
* in the code viewer.
*/

private import rust
Copy link
Preview
Copilot AI May 28, 2025

Choose a reason for hiding this comment

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

[nitpick] The private import rust line does not appear to be used by this module; you may remove it to avoid unnecessary imports.

Suggested change
private import rust

Copilot uses AI. Check for mistakes.

private import codeql.rust.elements.Variable
private import codeql.rust.elements.Locatable
private import codeql.rust.elements.FormatArgsExpr
Expand All @@ -12,9 +13,12 @@ private import codeql.rust.elements.MacroCall
private import codeql.rust.elements.NamedFormatArgument
private import codeql.rust.elements.PositionalFormatArgument
private import codeql.Locations
private import codeql.rust.internal.PathResolution

/** An element with an associated definition. */
abstract class Use extends Locatable {
Use() { not this.(AstNode).isFromMacroExpansion() }

/** Gets the definition associated with this element. */
abstract Definition getDefinition();

Expand All @@ -30,7 +34,8 @@ private module Cached {
newtype TDef =
TVariable(Variable v) or
TFormatArgsArgName(Name name) { name = any(FormatArgsArg a).getName() } or
TFormatArgsArgIndex(Expr e) { e = any(FormatArgsArg a).getExpr() }
TFormatArgsArgIndex(Expr e) { e = any(FormatArgsArg a).getExpr() } or
TItemNode(ItemNode i)

/**
* Gets an element, of kind `kind`, that element `use` uses, if any.
Expand All @@ -51,7 +56,8 @@ class Definition extends Cached::TDef {
Location getLocation() {
result = this.asVariable().getLocation() or
result = this.asName().getLocation() or
result = this.asExpr().getLocation()
result = this.asExpr().getLocation() or
result = this.asItemNode().getLocation()
}

/** Gets this definition as a `Variable` */
Expand All @@ -63,11 +69,15 @@ class Definition extends Cached::TDef {
/** Gets this definition as an `Expr` */
Expr asExpr() { this = Cached::TFormatArgsArgIndex(result) }

/** Gets this definition as an `ItemNode` */
ItemNode asItemNode() { this = Cached::TItemNode(result) }

/** Gets the string representation of this element. */
string toString() {
result = this.asExpr().toString() or
result = this.asVariable().toString() or
result = this.asName().getText()
result = this.asName().getText() or
result = this.asItemNode().toString()
}
}

Expand Down Expand Up @@ -124,3 +134,20 @@ private class PositionalFormatArgumentUse extends Use instanceof PositionalForma

override string getUseType() { result = "format argument" }
}

private class PathUse extends Use instanceof Path {
override Definition getDefinition() { result.asItemNode() = resolvePath(this) }

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

private class FileUse extends Use instanceof Name {
override Definition getDefinition() {
exists(Module m |
this = m.getName() and
fileImport(m, result.asItemNode())
)
}

override string getUseType() { result = "file" }
}
3 changes: 2 additions & 1 deletion rust/ql/lib/codeql/rust/internal/PathResolution.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1021,7 +1021,8 @@ private predicate pathAttrImport(Folder f, Module m, string relativePath) {
private predicate shouldAppend(Folder f, string relativePath) { pathAttrImport(f, _, relativePath) }

/** Holds if `m` is a `mod name;` item importing file `f`. */
private predicate fileImport(Module m, SourceFile f) {
pragma[nomagic]
Copy link
Preview
Copilot AI May 28, 2025

Choose a reason for hiding this comment

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

[nitpick] After marking fileImport with pragma[nomagic], consider re-adding the private qualifier (e.g. pragma[nomagic] private predicate fileImport…) to keep its visibility scope intentional.

Suggested change
pragma[nomagic]
pragma[nomagic] private

Copilot uses AI. Check for mistakes.

predicate fileImport(Module m, SourceFile f) {
exists(string name, Folder parent |
modImport0(m, name, _) and
fileModule(f, name, parent)
Expand Down
40 changes: 21 additions & 19 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,19 +1,21 @@
| main.rs:2:9:2:13 | width | main.rs:5:29:5:33 | width | local variable |
| main.rs:2:9:2:13 | width | main.rs:6:41:6:45 | width | local variable |
| main.rs:2:9:2:13 | width | main.rs:7:36:7:40 | width | local variable |
| main.rs:3:9:3:17 | precision | main.rs:5:36:5:44 | precision | local variable |
| main.rs:3:9:3:17 | precision | main.rs:6:48:6:56 | precision | local variable |
| main.rs:4:9:4:13 | value | main.rs:6:34:6:38 | value | local variable |
| main.rs:4:9:4:13 | value | main.rs:7:29:7:33 | value | local variable |
| main.rs:5:50:5:54 | value | main.rs:5:22:5:26 | value | format argument |
| main.rs:6:34:6:38 | value | main.rs:6:22:6:22 | 0 | format argument |
| main.rs:6:41:6:45 | width | main.rs:6:25:6:25 | 1 | format argument |
| main.rs:6:48:6:56 | precision | main.rs:6:28:6:28 | 2 | format argument |
| main.rs:7:29:7:33 | value | main.rs:7:21:7:22 | {} | format argument |
| main.rs:7:36:7:40 | width | main.rs:7:24:7:25 | {} | format argument |
| main.rs:8:9:8:14 | people | main.rs:9:22:9:27 | people | local variable |
| main.rs:10:31:10:31 | 1 | main.rs:10:19:10:20 | {} | format argument |
| main.rs:10:31:10:31 | 1 | main.rs:10:23:10:23 | 0 | format argument |
| main.rs:10:34:10:34 | 2 | main.rs:10:16:10:16 | 1 | format argument |
| main.rs:10:34:10:34 | 2 | main.rs:10:26:10:27 | {} | format argument |
| main.rs:11:40:11:42 | "x" | main.rs:11:31:11:35 | {:<5} | format argument |
| lib.rs:1:1:1:1 | SourceFile | main.rs:3:5:3:7 | lib | file |
| main.rs:1:1:1:9 | struct S | main.rs:16:13:16:13 | S | path |
| main.rs:6:9:6:13 | width | main.rs:9:29:9:33 | width | local variable |
| main.rs:6:9:6:13 | width | main.rs:10:41:10:45 | width | local variable |
| main.rs:6:9:6:13 | width | main.rs:11:36:11:40 | width | local variable |
| main.rs:7:9:7:17 | precision | main.rs:9:36:9:44 | precision | local variable |
| main.rs:7:9:7:17 | precision | main.rs:10:48:10:56 | precision | local variable |
| main.rs:8:9:8:13 | value | main.rs:10:34:10:38 | value | local variable |
| main.rs:8:9:8:13 | value | main.rs:11:29:11:33 | value | local variable |
| main.rs:9:50:9:54 | value | main.rs:9:22:9:26 | value | format argument |
| main.rs:10:34:10:38 | value | main.rs:10:22:10:22 | 0 | format argument |
| main.rs:10:41:10:45 | width | main.rs:10:25:10:25 | 1 | format argument |
| main.rs:10:48:10:56 | precision | main.rs:10:28:10:28 | 2 | format argument |
| main.rs:11:29:11:33 | value | main.rs:11:21:11:22 | {} | format argument |
| main.rs:11:36:11:40 | width | main.rs:11:24:11:25 | {} | format argument |
| main.rs:12:9:12:14 | people | main.rs:13:22:13:27 | people | local variable |
| main.rs:14:31:14:31 | 1 | main.rs:14:19:14:20 | {} | format argument |
| main.rs:14:31:14:31 | 1 | main.rs:14:23:14:23 | 0 | format argument |
| main.rs:14:34:14:34 | 2 | main.rs:14:16:14:16 | 1 | format argument |
| main.rs:14:34:14:34 | 2 | main.rs:14:26:14:27 | {} | format argument |
| main.rs:15:40:15:42 | "x" | main.rs:15:31:15:35 | {:<5} | format argument |
Copy link
Contributor

Choose a reason for hiding this comment

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

Nothing on main.rs lines 3 or 16?

Copy link
Contributor Author
@hvitved hvitved Jun 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

They are there, it's just the output that is confusing (def, use instead of use, def); I'll push a commit to change that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, thanks for making it clearer!

4 changes: 3 additions & 1 deletion rust/ql/test/library-tests/definitions/Definitions.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import codeql.rust.internal.Definitions

from Definition def, Use use, string kind
where def = definitionOf(use, kind)
where
def = definitionOf(use, kind) and
use.fromSource()
select def, use, kind
5 changes: 5 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,3 +1,7 @@
struct S;

mod lib;

fn main() {
let width = 4;
let precision = 2;
Expand All @@ -9,4 +13,5 @@ fn main() {
println!("Hello {people}!");
println!("{1} {} {0} {}", 1, 2);
assert_eq!(format!("Hello {:<5}!", "x"), "Hello x !");
let x = S;
}
0