Skip to content

Commit

Permalink
ide: improve ReferenceCategoryType
Browse files Browse the repository at this point in the history
It is bitset semantically --- many categorical things can be true about
a reference at the same time.

In parciular, a reference can be a "test" and a "write" at the same
time.
  • Loading branch information
matklad committed Apr 16, 2024
1 parent 1179c3e commit 9bd8eee
Show file tree
Hide file tree
Showing 9 changed files with 167 additions and 132 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

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
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
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
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
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
Loading

0 comments on commit 9bd8eee

Please sign in to comment.