ide: improve ReferenceCategoryType

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.
rust-lang · Apr 16, 2024 · 15dd02f · 15dd02f
1 parent 1179c3e
commit 15dd02f
Show file tree

Hide file tree

Showing 9 changed files with 167 additions and 132 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/crates/ide-assists/src/handlers/extract_function.rs b/crates/ide-assists/src/handlers/extract_function.rs
@@ -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;
     }
 

diff --git a/crates/ide-assists/src/handlers/remove_unused_imports.rs b/crates/ide-assists/src/handlers/remove_unused_imports.rs
@@ -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 {

diff --git a/crates/ide-db/Cargo.toml b/crates/ide-db/Cargo.toml
@@ -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

diff --git a/crates/ide-db/src/search.rs b/crates/ide-db/src/search.rs
@@ -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)]
@@ -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.
@@ -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)
             }
@@ -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)
             }
@@ -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)
             }
@@ -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)
             }
@@ -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)
             }
@@ -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)
             }
@@ -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)
             }
@@ -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))
+        return result | mode;
     }
 }