Skip to content
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

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
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
Next Next commit
Rust: add flag to turn off extractor path resolution
  • Loading branch information
Paolo Tranquilli committed Feb 19, 2025
commit c1df8d0e1382076b67a74ebc900c276a19b86caa
4 changes: 4 additions & 0 deletions rust/codeql-extractor.yml
Original file line number Diff line number Diff line change
@@ -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
@@ -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 {
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;
@@ -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();
@@ -66,6 +67,7 @@ impl<'a> Extractor<'a> {
label,
line_index,
semantics_info.as_ref().ok(),
resolve_paths,
);

for err in errors {
@@ -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(
@@ -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),
};
}
2 changes: 1 addition & 1 deletion rust/extractor/src/translate.rs
Original file line number Diff line number Diff line change
@@ -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
@@ -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) =
@@ -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,
@@ -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)> {
@@ -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)?;
@@ -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)?;
@@ -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()?;
@@ -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)?;
39 changes: 39 additions & 0 deletions rust/ql/test/extractor-tests/canonical_path/disabled/anonymous.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// would prefer to write `include!("../anonymous.rs");`
// but it seems `include!` does not work in rust-analyzer/our extractor

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
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!("../regular.rs");`
// but it seems `include!` does not work in rust-analyzer/our extractor

#[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 { .. } => {}
}
}