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 all 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
@@ -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
@@ -66,6 +66,7 @@ pub struct Config {
pub build_script_command: Vec<String>,
pub extra_includes: Vec<PathBuf>,
pub proc_macro_server: Option<PathBuf>,
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;
@@ -44,7 +45,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();
@@ -67,6 +68,7 @@ impl<'a> Extractor<'a> {
label,
line_index,
semantics_info.as_ref().ok(),
resolve_paths,
);

for err in errors {
@@ -103,12 +105,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(
@@ -239,14 +246,21 @@ fn main() -> anyhow::Result<()> {
}
let cwd = cwd()?;
let (cargo_config, load_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, &load_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
@@ -86,13 +86,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) =
@@ -105,6 +112,7 @@ impl<'a> Translator<'a> {
label: Label<generated::File>,
line_index: LineIndex,
semantic_info: Option<&FileSemanticInformation<'a>>,
resolve_paths: ResolvePaths,
) -> Translator<'a> {
Translator {
trap,
@@ -113,6 +121,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)> {
@@ -500,6 +509,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)?;
@@ -520,6 +532,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)?;
@@ -540,6 +555,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()?;
@@ -560,6 +578,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!("../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 { .. } => {}
}
}