Skip to content

Rust: add flag to turn off extractor path resolution #18813

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 4 commits into from
Mar 10, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
4 changes: 4 additions & 0 deletions rust/codeql-extractor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -78,3 +78,7 @@ options:
Collect flame graph data using the `tracing-flame` crate. To render a flame graph
or chart, run the `inferno-flamegraph` command. See also: https://crates.io/crates/tracing-flame
type: string
skip_path_resolution:
title: Skip path resolution
description: >
Skip path resolution. This is experimental, while we move path resolution from the extractor to the QL library.
1 change: 1 addition & 0 deletions rust/extractor/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ pub struct Config {
pub qltest: bool,
pub qltest_cargo_check: bool,
pub qltest_dependencies: Vec<String>,
pub skip_path_resolution: bool,
}

impl Config {
Expand Down
22 changes: 18 additions & 4 deletions rust/extractor/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::diagnostics::{emit_extraction_diagnostics, ExtractionStep};
use crate::rust_analyzer::path_to_file_id;
use crate::translate::ResolvePaths;
use crate::trap::TrapId;
use anyhow::Context;
use archive::Archiver;
Expand Down Expand Up @@ -43,7 +44,7 @@ impl<'a> Extractor<'a> {
}
}

fn extract(&mut self, rust_analyzer: &rust_analyzer::RustAnalyzer, file: &std::path::Path) {
fn extract(&mut self, rust_analyzer: &RustAnalyzer, file: &Path, resolve_paths: ResolvePaths) {
self.archiver.archive(file);

let before_parse = Instant::now();
Expand All @@ -66,6 +67,7 @@ impl<'a> Extractor<'a> {
label,
line_index,
semantics_info.as_ref().ok(),
resolve_paths,
);

for err in errors {
Expand Down Expand Up @@ -102,12 +104,17 @@ impl<'a> Extractor<'a> {
file: &Path,
semantics: &Semantics<'_, RootDatabase>,
vfs: &Vfs,
resolve_paths: ResolvePaths,
) {
self.extract(&RustAnalyzer::new(vfs, semantics), file);
self.extract(&RustAnalyzer::new(vfs, semantics), file, resolve_paths);
}

pub fn extract_without_semantics(&mut self, file: &Path, reason: &str) {
self.extract(&RustAnalyzer::WithoutSemantics { reason }, file);
self.extract(
&RustAnalyzer::WithoutSemantics { reason },
file,
ResolvePaths::No,
);
}

pub fn load_manifest(
Expand Down Expand Up @@ -236,12 +243,19 @@ fn main() -> anyhow::Result<()> {
extractor.extract_without_semantics(file, "no manifest found");
}
let cargo_config = cfg.to_cargo_config(&cwd()?);
let resolve_paths = if cfg.skip_path_resolution {
ResolvePaths::No
} else {
ResolvePaths::Yes
};
for (manifest, files) in map.values().filter(|(_, files)| !files.is_empty()) {
if let Some((ref db, ref vfs)) = extractor.load_manifest(manifest, &cargo_config) {
let semantics = Semantics::new(db);
for file in files {
match extractor.load_source(file, &semantics, vfs) {
Ok(()) => extractor.extract_with_semantics(file, &semantics, vfs),
Ok(()) => {
extractor.extract_with_semantics(file, &semantics, vfs, resolve_paths)
}
Err(reason) => extractor.extract_without_semantics(file, &reason),
};
}
Expand Down
2 changes: 1 addition & 1 deletion rust/extractor/src/translate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ mod base;
mod generated;
mod mappings;

pub use base::Translator;
pub use base::{ResolvePaths, Translator};
21 changes: 21 additions & 0 deletions rust/extractor/src/translate/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,20 @@ macro_rules! dispatch_to_tracing {
};
}

#[derive(Copy, Clone, PartialEq, Eq)]
pub enum ResolvePaths {
Yes,
No,
}

pub struct Translator<'a> {
pub trap: TrapFile,
path: &'a str,
label: Label<generated::File>,
line_index: LineIndex,
file_id: Option<EditionedFileId>,
pub semantics: Option<&'a Semantics<'a, RootDatabase>>,
resolve_paths: ResolvePaths,
}

const UNKNOWN_LOCATION: (LineCol, LineCol) =
Expand All @@ -102,6 +109,7 @@ impl<'a> Translator<'a> {
label: Label<generated::File>,
line_index: LineIndex,
semantic_info: Option<&FileSemanticInformation<'a>>,
resolve_paths: ResolvePaths,
) -> Translator<'a> {
Translator {
trap,
Expand All @@ -110,6 +118,7 @@ impl<'a> Translator<'a> {
line_index,
file_id: semantic_info.map(|i| i.file_id),
semantics: semantic_info.map(|i| i.semantics),
resolve_paths,
}
}
fn location(&self, range: TextRange) -> Option<(LineCol, LineCol)> {
Expand Down Expand Up @@ -497,6 +506,9 @@ impl<'a> Translator<'a> {
item: &T,
label: Label<generated::Addressable>,
) {
if self.resolve_paths == ResolvePaths::No {
return;
}
(|| {
let sema = self.semantics.as_ref()?;
let def = T::Hir::try_from_source(item, sema)?;
Expand All @@ -517,6 +529,9 @@ impl<'a> Translator<'a> {
item: &ast::Variant,
label: Label<generated::Variant>,
) {
if self.resolve_paths == ResolvePaths::No {
return;
}
(|| {
let sema = self.semantics.as_ref()?;
let def = sema.to_enum_variant_def(item)?;
Expand All @@ -537,6 +552,9 @@ impl<'a> Translator<'a> {
item: &impl PathAst,
label: Label<generated::Resolvable>,
) {
if self.resolve_paths == ResolvePaths::No {
return;
}
(|| {
let path = item.path()?;
let sema = self.semantics.as_ref()?;
Expand All @@ -557,6 +575,9 @@ impl<'a> Translator<'a> {
item: &ast::MethodCallExpr,
label: Label<generated::MethodCallExpr>,
) {
if self.resolve_paths == ResolvePaths::No {
return;
}
(|| {
let sema = self.semantics.as_ref()?;
let resolved = sema.resolve_method_call_fallback(item)?;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// would prefer to write `include!("../canonical_paths/anonymous.rs");`
// but `include!` does not work with out-of-dir files

use super::regular::Trait;

fn canonicals() {
struct OtherStruct;

trait OtherTrait {
fn g(&self);
}

impl OtherTrait for OtherStruct {
fn g(&self) {}
}

impl OtherTrait for crate::regular::Struct {
fn g(&self) {}
}

impl crate::regular::Trait for OtherStruct {
fn f(&self) {}
}

fn nested() {
struct OtherStruct;
}

fn usage() {
let s = OtherStruct {};
s.f();
s.g();
nested();
}
}

fn other() {
struct OtherStruct;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
canonicalPaths
| anonymous.rs:4:1:4:26 | Use | None | None |
| anonymous.rs:6:1:35:1 | fn canonicals | None | None |
| anonymous.rs:7:5:7:23 | struct OtherStruct | None | None |
| anonymous.rs:9:5:11:5 | trait OtherTrait | None | None |
| anonymous.rs:10:9:10:20 | fn g | None | None |
| anonymous.rs:13:5:15:5 | impl OtherTrait for OtherStruct { ... } | None | None |
| anonymous.rs:14:9:14:22 | fn g | None | None |
| anonymous.rs:17:5:19:5 | impl OtherTrait for ...::Struct { ... } | None | None |
| anonymous.rs:18:9:18:22 | fn g | None | None |
| anonymous.rs:21:5:23:5 | impl ...::Trait for OtherStruct { ... } | None | None |
| anonymous.rs:22:9:22:22 | fn f | None | None |
| anonymous.rs:25:5:27:5 | fn nested | None | None |
| anonymous.rs:26:9:26:27 | struct OtherStruct | None | None |
| anonymous.rs:29:5:34:5 | fn usage | None | None |
| anonymous.rs:37:1:39:1 | fn other | None | None |
| anonymous.rs:38:5:38:23 | struct OtherStruct | None | None |
| lib.rs:1:1:1:14 | mod anonymous | None | None |
| lib.rs:2:1:2:12 | mod regular | None | None |
| regular.rs:4:1:5:18 | struct Struct | None | None |
| regular.rs:7:1:9:1 | trait Trait | None | None |
| regular.rs:8:5:8:16 | fn f | None | None |
| regular.rs:11:1:13:1 | impl Trait for Struct { ... } | None | None |
| regular.rs:12:5:12:18 | fn f | None | None |
| regular.rs:15:1:17:1 | impl Struct { ... } | None | None |
| regular.rs:16:5:16:18 | fn g | None | None |
| regular.rs:19:1:21:1 | trait TraitWithBlanketImpl | None | None |
| regular.rs:20:5:20:16 | fn h | None | None |
| regular.rs:23:1:25:1 | impl TraitWithBlanketImpl for T { ... } | None | None |
| regular.rs:24:5:24:18 | fn h | None | None |
| regular.rs:27:1:27:12 | fn free | None | None |
| regular.rs:29:1:35:1 | fn usage | None | None |
| regular.rs:37:1:41:1 | enum MyEnum | None | None |
| regular.rs:43:1:49:1 | fn enum_qualified_usage | None | None |
| regular.rs:51:1:58:1 | fn enum_unqualified_usage | None | None |
| regular.rs:54:5:54:18 | Use | None | None |
| regular.rs:60:1:66:1 | fn enum_match | None | None |
resolvedPaths
| anonymous.rs:30:17:30:30 | OtherStruct {...} | None | None |
| anonymous.rs:31:9:31:9 | s | None | None |
| anonymous.rs:31:9:31:13 | s.f(...) | None | None |
| anonymous.rs:32:9:32:9 | s | None | None |
| anonymous.rs:32:9:32:13 | s.g(...) | None | None |
| anonymous.rs:33:9:33:14 | nested | None | None |
| regular.rs:30:13:30:21 | Struct {...} | None | None |
| regular.rs:31:5:31:5 | s | None | None |
| regular.rs:31:5:31:9 | s.f(...) | None | None |
| regular.rs:32:5:32:5 | s | None | None |
| regular.rs:32:5:32:9 | s.g(...) | None | None |
| regular.rs:33:5:33:5 | s | None | None |
| regular.rs:33:5:33:9 | s.h(...) | None | None |
| regular.rs:34:5:34:8 | free | None | None |
| regular.rs:44:9:44:26 | ...::None::<...> | None | None |
| regular.rs:45:9:45:20 | ...::Some | None | None |
| regular.rs:46:9:46:24 | ...::Variant1 | None | None |
| regular.rs:47:9:47:24 | ...::Variant2 | None | None |
| regular.rs:48:9:48:33 | ...::Variant3 {...} | None | None |
| regular.rs:52:9:52:18 | None::<...> | None | None |
| regular.rs:53:9:53:12 | Some | None | None |
| regular.rs:55:9:55:16 | Variant1 | None | None |
| regular.rs:56:9:56:16 | Variant2 | None | None |
| regular.rs:57:9:57:25 | Variant3 {...} | None | None |
| regular.rs:61:11:61:11 | e | None | None |
| regular.rs:62:9:62:24 | ...::Variant1 | None | None |
| regular.rs:63:9:63:27 | ...::Variant2(...) | None | None |
| regular.rs:64:9:64:31 | ...::Variant3 {...} | None | None |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
extractor-tests/canonical_path/canonical_paths.ql

Check warning

Code scanning / CodeQL

Query test without inline test expectations Warning test

Query test does not use inline test expectations.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
skip_path_resolution: true
66 changes: 66 additions & 0 deletions rust/ql/test/extractor-tests/canonical_path_disabled/regular.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// would prefer to write `include!("../canonical_path/regular.rs");
// but `include!` does not work with out-of-dir files

#[derive(Eq, PartialEq)]
pub struct Struct;

pub trait Trait {
fn f(&self);
}

impl Trait for Struct {
fn f(&self) {}
}

impl Struct {
fn g(&self) {}
}

trait TraitWithBlanketImpl {
fn h(&self);
}

impl<T: Eq> TraitWithBlanketImpl for T {
fn h(&self) {}
}

fn free() {}

fn usage() {
let s = Struct {};
s.f();
s.g();
s.h();
free();
}

enum MyEnum {
Variant1,
Variant2(usize),
Variant3 { x: usize },
}

fn enum_qualified_usage() {
_ = Option::None::<()>;
_ = Option::Some(0);
_ = MyEnum::Variant1;
_ = MyEnum::Variant2(0);
_ = MyEnum::Variant3 { x: 1 };
}

fn enum_unqualified_usage() {
_ = None::<()>;
_ = Some(0);
use MyEnum::*;
_ = Variant1;
_ = Variant2(0);
_ = Variant3 { x: 1 };
}

fn enum_match(e: MyEnum) {
match e {
MyEnum::Variant1 => {}
MyEnum::Variant2(_) => {}
MyEnum::Variant3 { .. } => {}
}
}