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.

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

Jump to bottom

ide: improve ReferenceCategoryType #17083

Merged
merged 2 commits into from
Apr 16, 2024
Merged

ide: improve ReferenceCategoryType #17083

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9bd8eee
ide: improve ReferenceCategoryType
matklad Apr 16, 2024
f852a6f
ide: add a new test
matklad Apr 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 7 additions & 1 deletion crates/ide-assists/src/handlers/extract_function.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1149,8 +1149,14 @@ fn reference_is_exclusive(
node: &dyn HasTokenAtOffset,
ctx: &AssistContext<'_>,
) -> bool {
// FIXME: this quite an incorrect way to go about doing this :-)
// `FileReference` is an IDE-type --- it encapsulates data communicated to the human,
// but doesn't necessary fully reflect all the intricacies of the underlying language semantics
// The correct approach here would be to expose this entire analysis as a method on some hir
// type. Something like `body.free_variables(statement_range)`.

// we directly modify variable with set: `n = 0`, `n += 1`
if reference.category == Some(ReferenceCategory::Write) {
if reference.category.contains(ReferenceCategory::WRITE) {
return true;
}

Expand Down
2 changes: 1 addition & 1 deletion crates/ide-assists/src/handlers/remove_unused_imports.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ fn used_once_in_scope(ctx: &AssistContext<'_>, def: Definition, scopes: &Vec<Sea
for scope in scopes {
let mut search_non_import = |_, r: FileReference| {
// The import itself is a use; we must skip that.
if r.category != Some(ReferenceCategory::Import) {
if !r.category.contains(ReferenceCategory::IMPORT) {
found = true;
true
} else {
Expand Down
1 change: 1 addition & 0 deletions crates/ide-db/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ indexmap.workspace = true
memchr = "2.6.4"
triomphe.workspace = true
nohash-hasher.workspace = true
bitflags.workspace = true

# local deps
base-db.workspace = true
Expand Down
85 changes: 46 additions & 39 deletions crates/ide-db/src/search.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ pub struct FileReference {
pub range: TextRange,
/// The node of the reference in the (macro-)file
pub name: FileReferenceNode,
pub category: Option<ReferenceCategory>,
pub category: ReferenceCategory,
}

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -124,17 +124,16 @@ impl FileReferenceNode {
}
}

#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
pub enum ReferenceCategory {
// FIXME: Add this variant and delete the `retain_adt_literal_usages` function.
// Create
Write,
Read,
Import,
// FIXME: Some day should be able to search in doc comments. Would probably
// need to switch from enum to bitflags then?
// DocComment
Test,
bitflags::bitflags! {
#[derive(Copy, Clone, Default, PartialEq, Eq, Hash, Debug)]
pub struct ReferenceCategory: u8 {
// FIXME: Add this variant and delete the `retain_adt_literal_usages` function.
// const CREATE = 1 << 0;
const WRITE = 1 << 0;
const READ = 1 << 1;
const IMPORT = 1 << 2;
const TEST = 1 << 3;
}
}

/// Generally, `search_scope` returns files that might contain references for the element.
Expand Down Expand Up @@ -660,7 +659,7 @@ impl<'a> FindUsages<'a> {
let reference = FileReference {
range,
name: FileReferenceNode::NameRef(name_ref.clone()),
category: None,
category: ReferenceCategory::empty(),
};
sink(file_id, reference)
}
Expand All @@ -676,10 +675,15 @@ impl<'a> FindUsages<'a> {
match NameRefClass::classify(self.sema, name_ref) {
Some(NameRefClass::Definition(def @ Definition::Module(_))) if def == self.def => {
let FileRange { file_id, range } = self.sema.original_range(name_ref.syntax());
let category = if is_name_ref_in_import(name_ref) {
ReferenceCategory::IMPORT
} else {
ReferenceCategory::empty()
};
let reference = FileReference {
range,
name: FileReferenceNode::NameRef(name_ref.clone()),
category: is_name_ref_in_import(name_ref).then_some(ReferenceCategory::Import),
category,
};
sink(file_id, reference)
}
Expand All @@ -700,7 +704,7 @@ impl<'a> FindUsages<'a> {
let reference = FileReference {
range,
name: FileReferenceNode::FormatStringEntry(token, range),
category: Some(ReferenceCategory::Read),
category: ReferenceCategory::READ,
};
sink(file_id, reference)
}
Expand All @@ -719,7 +723,7 @@ impl<'a> FindUsages<'a> {
let reference = FileReference {
range,
name: FileReferenceNode::Lifetime(lifetime.clone()),
category: None,
category: ReferenceCategory::empty(),
};
sink(file_id, reference)
}
Expand Down Expand Up @@ -817,7 +821,7 @@ impl<'a> FindUsages<'a> {
range,
name: FileReferenceNode::Name(name.clone()),
// FIXME: mutable patterns should have `Write` access
category: Some(ReferenceCategory::Read),
category: ReferenceCategory::READ,
};
sink(file_id, reference)
}
Expand All @@ -826,7 +830,7 @@ impl<'a> FindUsages<'a> {
let reference = FileReference {
range,
name: FileReferenceNode::Name(name.clone()),
category: None,
category: ReferenceCategory::empty(),
};
sink(file_id, reference)
}
Expand All @@ -851,7 +855,7 @@ impl<'a> FindUsages<'a> {
let reference = FileReference {
range,
name: FileReferenceNode::Name(name.clone()),
category: None,
category: ReferenceCategory::empty(),
};
sink(file_id, reference)
}
Expand All @@ -875,38 +879,41 @@ impl ReferenceCategory {
sema: &Semantics<'_, RootDatabase>,
def: &Definition,
r: &ast::NameRef,
) -> Option<ReferenceCategory> {
) -> ReferenceCategory {
let mut result = ReferenceCategory::empty();
if is_name_ref_in_test(sema, r) {
return Some(ReferenceCategory::Test);
result |= ReferenceCategory::TEST;
}

// Only Locals and Fields have accesses for now.
if !matches!(def, Definition::Local(_) | Definition::Field(_)) {
return is_name_ref_in_import(r).then_some(ReferenceCategory::Import);
if is_name_ref_in_import(r) {
result |= ReferenceCategory::IMPORT;
}
return result;
}

let mode = r.syntax().ancestors().find_map(|node| {
match_ast! {
match node {
ast::BinExpr(expr) => {
if matches!(expr.op_kind()?, ast::BinaryOp::Assignment { .. }) {
// If the variable or field ends on the LHS's end then it's a Write (covers fields and locals).
// FIXME: This is not terribly accurate.
if let Some(lhs) = expr.lhs() {
if lhs.syntax().text_range().end() == r.syntax().text_range().end() {
return Some(ReferenceCategory::Write);
match_ast! {
match node {
ast::BinExpr(expr) => {
if matches!(expr.op_kind()?, ast::BinaryOp::Assignment { .. }) {
// If the variable or field ends on the LHS's end then it's a Write
// (covers fields and locals). FIXME: This is not terribly accurate.
if let Some(lhs) = expr.lhs() {
if lhs.syntax().text_range().end() == r.syntax().text_range().end() {
return Some(ReferenceCategory::WRITE)
}
}
}
}
Some(ReferenceCategory::Read)
},
_ => None
Some(ReferenceCategory::READ)
},
_ => None,
}
}
}
});
}).unwrap_or(ReferenceCategory::READ);

// Default Locals and Fields to read
mode.or(Some(ReferenceCategory::Read))
result | mode
}
}

Expand Down