From 46eb03d99aa68ec7cbe03bce8a82780b21971907 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 2 Oct 2021 12:18:18 +0300 Subject: [PATCH 1/2] internal: use naming that matches intended use-case --- crates/ide/src/highlight_related.rs | 10 +-- crates/ide/src/lib.rs | 2 +- crates/ide/src/references.rs | 12 ++-- .../src/handlers/extract_function.rs | 4 +- crates/ide_db/src/search.rs | 69 +++++++++++-------- crates/rust-analyzer/src/to_proto.rs | 10 +-- 6 files changed, 61 insertions(+), 46 deletions(-) diff --git a/crates/ide/src/highlight_related.rs b/crates/ide/src/highlight_related.rs index 6b557bc92d2..ee504ffe44a 100644 --- a/crates/ide/src/highlight_related.rs +++ b/crates/ide/src/highlight_related.rs @@ -3,7 +3,7 @@ base_db::FilePosition, defs::{Definition, NameClass, NameRefClass}, helpers::{for_each_break_expr, for_each_tail_expr, node_ext::walk_expr, pick_best_token}, - search::{FileReference, ReferenceAccess, SearchScope}, + search::{FileReference, ReferenceCategory, SearchScope}, RootDatabase, }; use rustc_hash::FxHashSet; @@ -19,7 +19,7 @@ #[derive(PartialEq, Eq, Hash)] pub struct HighlightedRange { pub range: TextRange, - pub access: Option, + pub access: Option, } #[derive(Default, Clone)] @@ -87,7 +87,7 @@ fn highlight_references( .remove(&file_id) }) .flatten() - .map(|FileReference { access, range, .. }| HighlightedRange { range, access }); + .map(|FileReference { category: access, range, .. }| HighlightedRange { range, access }); let declarations = defs.iter().flat_map(|def| { match def { @@ -355,8 +355,8 @@ fn check_with_config(ra_fixture: &str, config: HighlightRelatedConfig) { hl.range, hl.access.map(|it| { match it { - ReferenceAccess::Read => "read", - ReferenceAccess::Write => "write", + ReferenceCategory::Read => "read", + ReferenceCategory::Write => "write", } .to_string() }), diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index dbfa99bdf24..5c472279907 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -108,7 +108,7 @@ macro_rules! eprintln { call_info::CallInfo, label::Label, line_index::{LineCol, LineColUtf16, LineIndex}, - search::{ReferenceAccess, SearchScope}, + search::{ReferenceCategory, SearchScope}, source_change::{FileSystemEdit, SourceChange}, symbol_index::Query, RootDatabase, SymbolKind, diff --git a/crates/ide/src/references.rs b/crates/ide/src/references.rs index 3ebec4dd8fb..993f212544e 100644 --- a/crates/ide/src/references.rs +++ b/crates/ide/src/references.rs @@ -16,7 +16,7 @@ use ide_db::{ base_db::FileId, defs::{Definition, NameClass, NameRefClass}, - search::{ReferenceAccess, SearchScope, UsageSearchResult}, + search::{ReferenceCategory, SearchScope, UsageSearchResult}, RootDatabase, }; use rustc_hash::FxHashMap; @@ -31,13 +31,13 @@ #[derive(Debug, Clone)] pub struct ReferenceSearchResult { pub declaration: Option, - pub references: FxHashMap)>>, + pub references: FxHashMap)>>, } #[derive(Debug, Clone)] pub struct Declaration { pub nav: NavigationTarget, - pub access: Option, + pub access: Option, } // Feature: Find All References @@ -102,7 +102,7 @@ pub(crate) fn find_all_refs( ( file_id, refs.into_iter() - .map(|file_ref| (file_ref.range, file_ref.access)) + .map(|file_ref| (file_ref.range, file_ref.category)) .collect(), ) }) @@ -149,7 +149,7 @@ pub(crate) fn decl_access( def: &Definition, syntax: &SyntaxNode, range: TextRange, -) -> Option { +) -> Option { match def { Definition::Local(_) | Definition::Field(_) => {} _ => return None, @@ -160,7 +160,7 @@ pub(crate) fn decl_access( let pat = stmt.pat()?; if let ast::Pat::IdentPat(it) = pat { if it.mut_token().is_some() { - return Some(ReferenceAccess::Write); + return Some(ReferenceCategory::Write); } } } diff --git a/crates/ide_assists/src/handlers/extract_function.rs b/crates/ide_assists/src/handlers/extract_function.rs index 4b11c5da193..585ef31daf2 100644 --- a/crates/ide_assists/src/handlers/extract_function.rs +++ b/crates/ide_assists/src/handlers/extract_function.rs @@ -6,7 +6,7 @@ use ide_db::{ defs::{Definition, NameRefClass}, helpers::node_ext::{preorder_expr, walk_expr, walk_pat, walk_patterns_in_expr}, - search::{FileReference, ReferenceAccess, SearchScope}, + search::{FileReference, ReferenceCategory, SearchScope}, RootDatabase, }; use itertools::Itertools; @@ -877,7 +877,7 @@ fn reference_is_exclusive( ctx: &AssistContext, ) -> bool { // we directly modify variable with set: `n = 0`, `n += 1` - if reference.access == Some(ReferenceAccess::Write) { + if reference.category == Some(ReferenceCategory::Write) { return true; } diff --git a/crates/ide_db/src/search.rs b/crates/ide_db/src/search.rs index 00acfde243e..70732327297 100644 --- a/crates/ide_db/src/search.rs +++ b/crates/ide_db/src/search.rs @@ -58,13 +58,18 @@ fn into_iter(self) -> Self::IntoIter { pub struct FileReference { pub range: TextRange, pub name: ast::NameLike, - pub access: Option, + pub category: Option, } #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] -pub enum ReferenceAccess { - Read, +pub enum ReferenceCategory { + // FIXME: Add this variant and delete the `retain_adt_literal_usages` function. + // Create Write, + Read, + // FIXME: Some day should be able to search in doc comments. Would probably + // need to switch from enum to bitflags then? + // DocComment } /// Generally, `search_scope` returns files that might contain references for the element. @@ -472,7 +477,7 @@ fn found_self_ty_name_ref( let reference = FileReference { range, name: ast::NameLike::NameRef(name_ref.clone()), - access: None, + category: None, }; sink(file_id, reference) } @@ -491,7 +496,7 @@ fn found_self_module_name_ref( let reference = FileReference { range, name: ast::NameLike::NameRef(name_ref.clone()), - access: None, + category: None, }; sink(file_id, reference) } @@ -510,7 +515,7 @@ fn found_lifetime( let reference = FileReference { range, name: ast::NameLike::Lifetime(lifetime.clone()), - access: None, + category: None, }; sink(file_id, reference) } @@ -529,7 +534,7 @@ fn found_name_ref( let reference = FileReference { range, name: ast::NameLike::NameRef(name_ref.clone()), - access: reference_access(&def, name_ref), + category: ReferenceCategory::new(&def, name_ref), }; sink(file_id, reference) } @@ -539,7 +544,7 @@ fn found_name_ref( let reference = FileReference { range, name: ast::NameLike::NameRef(name_ref.clone()), - access: reference_access(&def, name_ref), + category: ReferenceCategory::new(&def, name_ref), }; sink(file_id, reference) } else { @@ -550,14 +555,19 @@ fn found_name_ref( let field = Definition::Field(field); let FileRange { file_id, range } = self.sema.original_range(name_ref.syntax()); let access = match self.def { - Definition::Field(_) if field == self.def => reference_access(&field, name_ref), + Definition::Field(_) if field == self.def => { + ReferenceCategory::new(&field, name_ref) + } Definition::Local(l) if local == l => { - reference_access(&Definition::Local(local), name_ref) + ReferenceCategory::new(&Definition::Local(local), name_ref) } _ => return false, }; - let reference = - FileReference { range, name: ast::NameLike::NameRef(name_ref.clone()), access }; + let reference = FileReference { + range, + name: ast::NameLike::NameRef(name_ref.clone()), + category: access, + }; sink(file_id, reference) } _ => false, @@ -580,14 +590,17 @@ fn found_name( range, name: ast::NameLike::Name(name.clone()), // FIXME: mutable patterns should have `Write` access - access: Some(ReferenceAccess::Read), + category: Some(ReferenceCategory::Read), }; sink(file_id, reference) } Some(NameClass::ConstReference(def)) if self.def == def => { let FileRange { file_id, range } = self.sema.original_range(name.syntax()); - let reference = - FileReference { range, name: ast::NameLike::Name(name.clone()), access: None }; + let reference = FileReference { + range, + name: ast::NameLike::Name(name.clone()), + category: None, + }; sink(file_id, reference) } // Resolve trait impl function definitions to the trait definition's version if self.def is the trait definition's @@ -611,7 +624,7 @@ fn found_name( let reference = FileReference { range, name: ast::NameLike::Name(name.clone()), - access: None, + category: None, }; sink(file_id, reference) }) @@ -642,13 +655,14 @@ fn def_to_ty(sema: &Semantics, def: &Definition) -> Option Option { - // Only Locals and Fields have accesses for now. - if !matches!(def, Definition::Local(_) | Definition::Field(_)) { - return None; - } +impl ReferenceCategory { + fn new(def: &Definition, r: &ast::NameRef) -> Option { + // Only Locals and Fields have accesses for now. + if !matches!(def, Definition::Local(_) | Definition::Field(_)) { + return None; + } - let mode = name_ref.syntax().ancestors().find_map(|node| { + let mode = r.syntax().ancestors().find_map(|node| { match_ast! { match (node) { ast::BinExpr(expr) => { @@ -656,18 +670,19 @@ fn reference_access(def: &Definition, name_ref: &ast::NameRef) -> Option None } } }); - // Default Locals and Fields to read - mode.or(Some(ReferenceAccess::Read)) + // Default Locals and Fields to read + mode.or(Some(ReferenceCategory::Read)) + } } diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs index e18298fadee..59a768397fe 100644 --- a/crates/rust-analyzer/src/to_proto.rs +++ b/crates/rust-analyzer/src/to_proto.rs @@ -9,7 +9,7 @@ Annotation, AnnotationKind, Assist, AssistKind, CallInfo, Cancellable, CompletionItem, CompletionItemKind, CompletionRelevance, Documentation, FileId, FileRange, FileSystemEdit, Fold, FoldKind, Highlight, HlMod, HlOperator, HlPunct, HlRange, HlTag, Indel, InlayHint, - InlayKind, Markup, NavigationTarget, ReferenceAccess, RenameError, Runnable, Severity, + InlayKind, Markup, NavigationTarget, ReferenceCategory, RenameError, Runnable, Severity, SourceChange, StructureNodeKind, SymbolKind, TextEdit, TextRange, TextSize, }; use itertools::Itertools; @@ -75,11 +75,11 @@ pub(crate) fn structure_node_kind(kind: StructureNodeKind) -> lsp_types::SymbolK } pub(crate) fn document_highlight_kind( - reference_access: ReferenceAccess, + category: ReferenceCategory, ) -> lsp_types::DocumentHighlightKind { - match reference_access { - ReferenceAccess::Read => lsp_types::DocumentHighlightKind::Read, - ReferenceAccess::Write => lsp_types::DocumentHighlightKind::Write, + match category { + ReferenceCategory::Read => lsp_types::DocumentHighlightKind::Read, + ReferenceCategory::Write => lsp_types::DocumentHighlightKind::Write, } } From 12103b16de2012988cc4ebcb025f7149a7ee6336 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 2 Oct 2021 13:02:06 +0300 Subject: [PATCH 2/2] internal: untangle usages of ReferenceCategory somewhat Not everything that can be read or write is a reference, let's try to use more precise types. --- crates/ide/src/highlight_related.rs | 40 ++++++++++++++++++---------- crates/ide/src/references.rs | 34 +++++++++-------------- crates/rust-analyzer/src/handlers.rs | 4 +-- 3 files changed, 41 insertions(+), 37 deletions(-) diff --git a/crates/ide/src/highlight_related.rs b/crates/ide/src/highlight_related.rs index ee504ffe44a..f910edebf11 100644 --- a/crates/ide/src/highlight_related.rs +++ b/crates/ide/src/highlight_related.rs @@ -19,7 +19,10 @@ #[derive(PartialEq, Eq, Hash)] pub struct HighlightedRange { pub range: TextRange, - pub access: Option, + // FIXME: This needs to be more precise. Reference category makes sense only + // for references, but we also have defs. And things like exit points are + // neither. + pub category: Option, } #[derive(Default, Clone)] @@ -87,7 +90,10 @@ fn highlight_references( .remove(&file_id) }) .flatten() - .map(|FileReference { category: access, range, .. }| HighlightedRange { range, access }); + .map(|FileReference { category: access, range, .. }| HighlightedRange { + range, + category: access, + }); let declarations = defs.iter().flat_map(|def| { match def { @@ -99,8 +105,12 @@ fn highlight_references( .filter(|decl| decl.file_id == file_id) .and_then(|decl| { let range = decl.focus_range?; - let access = references::decl_access(&def, syntax, range); - Some(HighlightedRange { range, access }) + let category = if references::decl_mutability(&def, syntax, range) { + Some(ReferenceCategory::Write) + } else { + None + }; + Some(HighlightedRange { range, category }) }) }); @@ -125,18 +135,20 @@ fn hl( walk_expr(&body, &mut |expr| match expr { ast::Expr::ReturnExpr(expr) => { if let Some(token) = expr.return_token() { - highlights.push(HighlightedRange { access: None, range: token.text_range() }); + highlights.push(HighlightedRange { category: None, range: token.text_range() }); } } ast::Expr::TryExpr(try_) => { if let Some(token) = try_.question_mark_token() { - highlights.push(HighlightedRange { access: None, range: token.text_range() }); + highlights.push(HighlightedRange { category: None, range: token.text_range() }); } } ast::Expr::MethodCallExpr(_) | ast::Expr::CallExpr(_) | ast::Expr::MacroCall(_) => { if sema.type_of_expr(&expr).map_or(false, |ty| ty.original.is_never()) { - highlights - .push(HighlightedRange { access: None, range: expr.syntax().text_range() }); + highlights.push(HighlightedRange { + category: None, + range: expr.syntax().text_range(), + }); } } _ => (), @@ -154,7 +166,7 @@ fn hl( .map_or_else(|| tail.syntax().text_range(), |tok| tok.text_range()), _ => tail.syntax().text_range(), }; - highlights.push(HighlightedRange { access: None, range }) + highlights.push(HighlightedRange { category: None, range }) }); } Some(highlights) @@ -187,13 +199,13 @@ fn hl( token.map(|tok| tok.text_range()), label.as_ref().map(|it| it.syntax().text_range()), ); - highlights.extend(range.map(|range| HighlightedRange { access: None, range })); + highlights.extend(range.map(|range| HighlightedRange { category: None, range })); for_each_break_expr(label, body, &mut |break_| { let range = cover_range( break_.break_token().map(|it| it.text_range()), break_.lifetime().map(|it| it.syntax().text_range()), ); - highlights.extend(range.map(|range| HighlightedRange { access: None, range })); + highlights.extend(range.map(|range| HighlightedRange { category: None, range })); }); Some(highlights) } @@ -241,13 +253,13 @@ fn hl( body: Option, ) -> Option> { let mut highlights = - vec![HighlightedRange { access: None, range: async_token?.text_range() }]; + vec![HighlightedRange { category: None, range: async_token?.text_range() }]; if let Some(body) = body { walk_expr(&body, &mut |expr| { if let ast::Expr::AwaitExpr(expr) = expr { if let Some(token) = expr.await_token() { highlights - .push(HighlightedRange { access: None, range: token.text_range() }); + .push(HighlightedRange { category: None, range: token.text_range() }); } } }); @@ -353,7 +365,7 @@ fn check_with_config(ra_fixture: &str, config: HighlightRelatedConfig) { .map(|hl| { ( hl.range, - hl.access.map(|it| { + hl.category.map(|it| { match it { ReferenceCategory::Read => "read", ReferenceCategory::Write => "write", diff --git a/crates/ide/src/references.rs b/crates/ide/src/references.rs index 993f212544e..fd36cfc44de 100644 --- a/crates/ide/src/references.rs +++ b/crates/ide/src/references.rs @@ -37,7 +37,7 @@ pub struct ReferenceSearchResult { #[derive(Debug, Clone)] pub struct Declaration { pub nav: NavigationTarget, - pub access: Option, + pub is_mut: bool, } // Feature: Find All References @@ -88,7 +88,7 @@ pub(crate) fn find_all_refs( .map(|nav| { let decl_range = nav.focus_or_full_range(); Declaration { - access: decl_access(&def, sema.parse(nav.file_id).syntax(), decl_range), + is_mut: decl_mutability(&def, sema.parse(nav.file_id).syntax(), decl_range), nav, } }); @@ -145,27 +145,19 @@ pub(crate) fn find_defs<'a>( }) } -pub(crate) fn decl_access( - def: &Definition, - syntax: &SyntaxNode, - range: TextRange, -) -> Option { +pub(crate) fn decl_mutability(def: &Definition, syntax: &SyntaxNode, range: TextRange) -> bool { match def { Definition::Local(_) | Definition::Field(_) => {} - _ => return None, + _ => return false, }; - let stmt = find_node_at_offset::(syntax, range.start())?; - if stmt.initializer().is_some() { - let pat = stmt.pat()?; - if let ast::Pat::IdentPat(it) = pat { - if it.mut_token().is_some() { - return Some(ReferenceCategory::Write); - } - } + match find_node_at_offset::(syntax, range.start()) { + Some(stmt) if stmt.initializer().is_some() => match stmt.pat() { + Some(ast::Pat::IdentPat(it)) => it.mut_token().is_some(), + _ => false, + }, + _ => false, } - - None } /// Filter out all non-literal usages for adt-defs @@ -283,7 +275,7 @@ fn is_lit_name_ref(name_ref: &ast::NameRef) -> bool { #[cfg(test)] mod tests { use expect_test::{expect, Expect}; - use ide_db::base_db::FileId; + use ide_db::{base_db::FileId, search::ReferenceCategory}; use stdx::format_to; use crate::{fixture, SearchScope}; @@ -1095,8 +1087,8 @@ fn check_with_scope(ra_fixture: &str, search_scope: Option, expect: if let Some(decl) = refs.declaration { format_to!(actual, "{}", decl.nav.debug_render()); - if let Some(access) = decl.access { - format_to!(actual, " {:?}", access) + if decl.is_mut { + format_to!(actual, " {:?}", ReferenceCategory::Write) } actual += "\n\n"; } diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs index 6b8f1f6d1a9..e62bb9499fa 100644 --- a/crates/rust-analyzer/src/handlers.rs +++ b/crates/rust-analyzer/src/handlers.rs @@ -1182,9 +1182,9 @@ pub(crate) fn handle_document_highlight( }; let res = refs .into_iter() - .map(|ide::HighlightedRange { range, access }| lsp_types::DocumentHighlight { + .map(|ide::HighlightedRange { range, category }| lsp_types::DocumentHighlight { range: to_proto::range(&line_index, range), - kind: access.map(to_proto::document_highlight_kind), + kind: category.map(to_proto::document_highlight_kind), }) .collect(); Ok(Some(res))