From c1df8d0e1382076b67a74ebc900c276a19b86caa Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Wed, 19 Feb 2025 09:44:09 +0100 Subject: [PATCH] Rust: add flag to turn off extractor path resolution --- rust/codeql-extractor.yml | 4 ++ rust/extractor/src/config.rs | 1 + rust/extractor/src/main.rs | 22 +++++-- rust/extractor/src/translate.rs | 2 +- rust/extractor/src/translate/base.rs | 21 ++++++ .../canonical_path/disabled/anonymous.rs | 39 +++++++++++ .../disabled/canonical_paths.expected | 66 +++++++++++++++++++ .../disabled/canonical_paths.qlref | 1 + .../canonical_path/disabled/options.yml | 1 + .../canonical_path/disabled/regular.rs | 66 +++++++++++++++++++ 10 files changed, 218 insertions(+), 5 deletions(-) create mode 100644 rust/ql/test/extractor-tests/canonical_path/disabled/anonymous.rs create mode 100644 rust/ql/test/extractor-tests/canonical_path/disabled/canonical_paths.expected create mode 100644 rust/ql/test/extractor-tests/canonical_path/disabled/canonical_paths.qlref create mode 100644 rust/ql/test/extractor-tests/canonical_path/disabled/options.yml create mode 100644 rust/ql/test/extractor-tests/canonical_path/disabled/regular.rs diff --git a/rust/codeql-extractor.yml b/rust/codeql-extractor.yml index 27f423134a6..1c73a070e58 100644 --- a/rust/codeql-extractor.yml +++ b/rust/codeql-extractor.yml @@ -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. diff --git a/rust/extractor/src/config.rs b/rust/extractor/src/config.rs index 82568b64553..aec443f6d87 100644 --- a/rust/extractor/src/config.rs +++ b/rust/extractor/src/config.rs @@ -57,6 +57,7 @@ pub struct Config { pub qltest: bool, pub qltest_cargo_check: bool, pub qltest_dependencies: Vec, + pub skip_path_resolution: bool, } impl Config { diff --git a/rust/extractor/src/main.rs b/rust/extractor/src/main.rs index 04aaf23c652..9d70956ab18 100644 --- a/rust/extractor/src/main.rs +++ b/rust/extractor/src/main.rs @@ -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), }; } diff --git a/rust/extractor/src/translate.rs b/rust/extractor/src/translate.rs index 4e9cda3e2aa..c74652628f8 100644 --- a/rust/extractor/src/translate.rs +++ b/rust/extractor/src/translate.rs @@ -2,4 +2,4 @@ mod base; mod generated; mod mappings; -pub use base::Translator; +pub use base::{ResolvePaths, Translator}; diff --git a/rust/extractor/src/translate/base.rs b/rust/extractor/src/translate/base.rs index fe13de6498e..19c86374a45 100644 --- a/rust/extractor/src/translate/base.rs +++ b/rust/extractor/src/translate/base.rs @@ -83,6 +83,12 @@ 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, @@ -90,6 +96,7 @@ pub struct Translator<'a> { line_index: LineIndex, file_id: Option, 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, 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, ) { + 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, ) { + 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, ) { + 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, ) { + if self.resolve_paths == ResolvePaths::No { + return; + } (|| { let sema = self.semantics.as_ref()?; let resolved = sema.resolve_method_call_fallback(item)?; diff --git a/rust/ql/test/extractor-tests/canonical_path/disabled/anonymous.rs b/rust/ql/test/extractor-tests/canonical_path/disabled/anonymous.rs new file mode 100644 index 00000000000..d26ea6b992d --- /dev/null +++ b/rust/ql/test/extractor-tests/canonical_path/disabled/anonymous.rs @@ -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; +} diff --git a/rust/ql/test/extractor-tests/canonical_path/disabled/canonical_paths.expected b/rust/ql/test/extractor-tests/canonical_path/disabled/canonical_paths.expected new file mode 100644 index 00000000000..6a19610d76f --- /dev/null +++ b/rust/ql/test/extractor-tests/canonical_path/disabled/canonical_paths.expected @@ -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 | diff --git a/rust/ql/test/extractor-tests/canonical_path/disabled/canonical_paths.qlref b/rust/ql/test/extractor-tests/canonical_path/disabled/canonical_paths.qlref new file mode 100644 index 00000000000..f40a74f5aed --- /dev/null +++ b/rust/ql/test/extractor-tests/canonical_path/disabled/canonical_paths.qlref @@ -0,0 +1 @@ +extractor-tests/canonical_path/canonical_paths.ql diff --git a/rust/ql/test/extractor-tests/canonical_path/disabled/options.yml b/rust/ql/test/extractor-tests/canonical_path/disabled/options.yml new file mode 100644 index 00000000000..baee4ba5afe --- /dev/null +++ b/rust/ql/test/extractor-tests/canonical_path/disabled/options.yml @@ -0,0 +1 @@ +skip_path_resolution: true diff --git a/rust/ql/test/extractor-tests/canonical_path/disabled/regular.rs b/rust/ql/test/extractor-tests/canonical_path/disabled/regular.rs new file mode 100644 index 00000000000..44483645485 --- /dev/null +++ b/rust/ql/test/extractor-tests/canonical_path/disabled/regular.rs @@ -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 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 { .. } => {} + } +}