From decfd28bd14b56befa17257694caacc57a090939 Mon Sep 17 00:00:00 2001 From: Ekaterina Babshukova Date: Tue, 22 Oct 2019 23:46:53 +0300 Subject: some fixes, add docs --- crates/ra_ide_api/src/goto_definition.rs | 12 ++-- crates/ra_ide_api/src/references.rs | 39 ++++++----- crates/ra_ide_api/src/references/classify.rs | 8 ++- .../ra_ide_api/src/references/name_definition.rs | 5 +- crates/ra_ide_api/src/references/search_scope.rs | 81 ++++++++++------------ 5 files changed, 73 insertions(+), 72 deletions(-) diff --git a/crates/ra_ide_api/src/goto_definition.rs b/crates/ra_ide_api/src/goto_definition.rs index d14908b25..1f3fa6c57 100644 --- a/crates/ra_ide_api/src/goto_definition.rs +++ b/crates/ra_ide_api/src/goto_definition.rs @@ -206,7 +206,7 @@ fn named_target(file_id: FileId, node: &SyntaxNode) -> Option #[cfg(test)] mod tests { - // use test_utils::covers; + use test_utils::covers; use crate::mock_analysis::analysis_and_position; @@ -274,7 +274,7 @@ mod tests { #[test] fn goto_definition_works_for_macros() { - // covers!(goto_definition_works_for_macros); + covers!(goto_definition_works_for_macros); check_goto( " //- /lib.rs @@ -294,7 +294,7 @@ mod tests { #[test] fn goto_definition_works_for_macros_from_other_crates() { - // covers!(goto_definition_works_for_macros); + covers!(goto_definition_works_for_macros); check_goto( " //- /lib.rs @@ -317,7 +317,7 @@ mod tests { #[test] fn goto_definition_works_for_methods() { - // covers!(goto_definition_works_for_methods); + covers!(goto_definition_works_for_methods); check_goto( " //- /lib.rs @@ -336,7 +336,7 @@ mod tests { #[test] fn goto_definition_works_for_fields() { - // covers!(goto_definition_works_for_fields); + covers!(goto_definition_works_for_fields); check_goto( " //- /lib.rs @@ -354,7 +354,7 @@ mod tests { #[test] fn goto_definition_works_for_record_fields() { - // covers!(goto_definition_works_for_record_fields); + covers!(goto_definition_works_for_record_fields); check_goto( " //- /lib.rs diff --git a/crates/ra_ide_api/src/references.rs b/crates/ra_ide_api/src/references.rs index aadd52616..f35d835ac 100644 --- a/crates/ra_ide_api/src/references.rs +++ b/crates/ra_ide_api/src/references.rs @@ -1,4 +1,13 @@ -//! FIXME: write short doc here +//! This module implements a reference search. +//! First, the element at the cursor position must be either an `ast::Name` +//! or `ast::NameRef`. If it's a `ast::NameRef`, at the classification step we +//! try to resolve the direct tree parent of this element, otherwise we +//! already have a definition and just need to get its HIR together with +//! some information that is needed for futher steps of searching. +//! After that, we collect files that might contain references and look +//! for text occurrences of the identifier. If there's an `ast::NameRef` +//! at the index that the match starts at and its tree parent is +//! resolved to the search element definition, we get a reference. mod classify; mod name_definition; @@ -9,7 +18,7 @@ use once_cell::unsync::Lazy; use ra_db::{SourceDatabase, SourceDatabaseExt}; use ra_syntax::{algo::find_node_at_offset, ast, AstNode, SourceFile, SyntaxNode, TextUnit}; -use crate::{db::RootDatabase, FileId, FilePosition, FileRange, NavigationTarget, RangeInfo}; +use crate::{db::RootDatabase, FilePosition, FileRange, NavigationTarget, RangeInfo}; pub(crate) use self::{ classify::{classify_name, classify_name_ref}, @@ -102,16 +111,7 @@ fn process_definition(db: &RootDatabase, def: NameDefinition, name: String) -> V let scope = def.search_scope(db); let mut refs = vec![]; - let is_match = |file_id: FileId, name_ref: &ast::NameRef| -> bool { - let classified = classify_name_ref(db, file_id, &name_ref); - if let Some(d) = classified { - d == def - } else { - false - } - }; - - for (file_id, text_range) in scope { + for (file_id, search_range) in scope { let text = db.file_text(file_id); let parse = Lazy::new(|| SourceFile::parse(&text)); @@ -122,19 +122,20 @@ fn process_definition(db: &RootDatabase, def: NameDefinition, name: String) -> V find_node_at_offset::(parse.tree().syntax(), offset) { let range = name_ref.syntax().text_range(); - - if let Some(text_range) = text_range { - if range.is_subrange(&text_range) && is_match(file_id, &name_ref) { + if let Some(search_range) = search_range { + if !range.is_subrange(&search_range) { + continue; + } + } + if let Some(d) = classify_name_ref(db, file_id, &name_ref) { + if d == def { refs.push(FileRange { file_id, range }); } - } else if is_match(file_id, &name_ref) { - refs.push(FileRange { file_id, range }); } } } } - - return refs; + refs } #[cfg(test)] diff --git a/crates/ra_ide_api/src/references/classify.rs b/crates/ra_ide_api/src/references/classify.rs index 3beab9861..c8daff9b1 100644 --- a/crates/ra_ide_api/src/references/classify.rs +++ b/crates/ra_ide_api/src/references/classify.rs @@ -1,8 +1,9 @@ -//! FIXME: write short doc here +//! Functions that are used to classify an element from its definition or reference. use hir::{Either, FromSource, Module, ModuleSource, Path, PathResolution, Source, SourceAnalyzer}; use ra_db::FileId; use ra_syntax::{ast, match_ast, AstNode, AstPtr}; +use test_utils::tested_by; use super::{ name_definition::{from_assoc_item, from_module_def, from_pat, from_struct_field}, @@ -111,18 +112,21 @@ pub(crate) fn classify_name_ref( let analyzer = SourceAnalyzer::new(db, file_id, name_ref.syntax(), None); if let Some(method_call) = ast::MethodCallExpr::cast(parent.clone()) { + tested_by!(goto_definition_works_for_methods); if let Some(func) = analyzer.resolve_method_call(&method_call) { return Some(from_assoc_item(db, func.into())); } } if let Some(field_expr) = ast::FieldExpr::cast(parent.clone()) { + tested_by!(goto_definition_works_for_fields); if let Some(field) = analyzer.resolve_field(&field_expr) { return Some(from_struct_field(db, field)); } } if let Some(record_field) = ast::RecordField::cast(parent.clone()) { + tested_by!(goto_definition_works_for_record_fields); if let Some(record_lit) = record_field.syntax().ancestors().find_map(ast::RecordLit::cast) { let variant_def = analyzer.resolve_record_literal(&record_lit)?; let hir_path = Path::from_name_ref(name_ref); @@ -139,6 +143,7 @@ pub(crate) fn classify_name_ref( let visibility = None; if let Some(macro_call) = parent.ancestors().find_map(ast::MacroCall::cast) { + tested_by!(goto_definition_works_for_macros); if let Some(macro_def) = analyzer.resolve_macro_call(db, ¯o_call) { let kind = NameKind::Macro(macro_def); return Some(NameDefinition { kind, container, visibility }); @@ -152,7 +157,6 @@ pub(crate) fn classify_name_ref( AssocItem(item) => Some(from_assoc_item(db, item)), LocalBinding(Either::A(pat)) => from_pat(db, file_id, pat), LocalBinding(Either::B(par)) => { - // Not really supported let kind = NameKind::SelfParam(par); Some(NameDefinition { kind, container, visibility }) } diff --git a/crates/ra_ide_api/src/references/name_definition.rs b/crates/ra_ide_api/src/references/name_definition.rs index 723d97237..4580bc789 100644 --- a/crates/ra_ide_api/src/references/name_definition.rs +++ b/crates/ra_ide_api/src/references/name_definition.rs @@ -1,4 +1,7 @@ -//! FIXME: write short doc here +//! `NameDefinition` keeps information about the element we want to search references for. +//! The element is represented by `NameKind`. It's located inside some `container` and +//! has a `visibility`, which defines a search scope. +//! Note that the reference search is possible for not all of the classified items. use hir::{ db::AstDatabase, Adt, AssocItem, DefWithBody, FromSource, HasSource, HirFileId, MacroDef, diff --git a/crates/ra_ide_api/src/references/search_scope.rs b/crates/ra_ide_api/src/references/search_scope.rs index 8495a92a5..5cb69b8fc 100644 --- a/crates/ra_ide_api/src/references/search_scope.rs +++ b/crates/ra_ide_api/src/references/search_scope.rs @@ -1,72 +1,66 @@ -//! FIXME: write short doc here - -use std::collections::HashSet; +//! Generally, `search_scope` returns files that might contain references for the element. +//! For `pub(crate)` things it's a crate, for `pub` things it's a crate and dependant crates. +//! In some cases, the location of the references is known to within a `TextRange`, +//! e.g. for things like local variables. use hir::{DefWithBody, HasSource, ModuleSource}; use ra_db::{FileId, SourceDatabase, SourceDatabaseExt}; use ra_syntax::{AstNode, TextRange}; +use rustc_hash::FxHashSet; use crate::db::RootDatabase; use super::{NameDefinition, NameKind}; impl NameDefinition { - pub(crate) fn search_scope(&self, db: &RootDatabase) -> HashSet<(FileId, Option)> { + pub(crate) fn search_scope(&self, db: &RootDatabase) -> FxHashSet<(FileId, Option)> { let module_src = self.container.definition_source(db); let file_id = module_src.file_id.original_file(db); if let NameKind::Pat((def, _)) = self.kind { + let mut res = FxHashSet::default(); let range = match def { DefWithBody::Function(f) => f.source(db).ast.syntax().text_range(), DefWithBody::Const(c) => c.source(db).ast.syntax().text_range(), DefWithBody::Static(s) => s.source(db).ast.syntax().text_range(), }; - return [(file_id, Some(range))].iter().cloned().collect(); + res.insert((file_id, Some(range))); + return res; } - if let Some(ref vis) = self.visibility { - let vis = vis.syntax().to_string(); - - // FIXME: add "pub(in path)" - - if vis.as_str() == "pub(super)" { - if let Some(parent_module) = self.container.parent(db) { - let mut files = HashSet::new(); + let vis = + self.visibility.as_ref().map(|v| v.syntax().to_string()).unwrap_or("".to_string()); - let parent_src = parent_module.definition_source(db); - let file_id = parent_src.file_id.original_file(db); + if vis.as_str() == "pub(super)" { + if let Some(parent_module) = self.container.parent(db) { + let mut files = FxHashSet::default(); + let parent_src = parent_module.definition_source(db); + let file_id = parent_src.file_id.original_file(db); - match parent_src.ast { - ModuleSource::Module(m) => { - let range = Some(m.syntax().text_range()); - files.insert((file_id, range)); - } - ModuleSource::SourceFile(_) => { - files.insert((file_id, None)); - files.extend( - parent_module - .children(db) - .map(|m| { - let src = m.definition_source(db); - (src.file_id.original_file(db), None) - }) - .collect::>(), - ); - } + match parent_src.ast { + ModuleSource::Module(m) => { + let range = Some(m.syntax().text_range()); + files.insert((file_id, range)); + } + ModuleSource::SourceFile(_) => { + files.insert((file_id, None)); + files.extend(parent_module.children(db).map(|m| { + let src = m.definition_source(db); + (src.file_id.original_file(db), None) + })); } - return files; - } else { - let range = match module_src.ast { - ModuleSource::Module(m) => Some(m.syntax().text_range()), - ModuleSource::SourceFile(_) => None, - }; - return [(file_id, range)].iter().cloned().collect(); } + return files; } + } + if vis.as_str() != "" { let source_root_id = db.file_source_root(file_id); let source_root = db.source_root(source_root_id); - let mut files = source_root.walk().map(|id| (id.into(), None)).collect::>(); + let mut files = + source_root.walk().map(|id| (id.into(), None)).collect::>(); + + // FIXME: add "pub(in path)" if vis.as_str() == "pub(crate)" { return files; @@ -74,10 +68,8 @@ impl NameDefinition { if vis.as_str() == "pub" { let krate = self.container.krate(db).unwrap(); let crate_graph = db.crate_graph(); - for crate_id in crate_graph.iter() { let mut crate_deps = crate_graph.dependencies(crate_id); - if crate_deps.any(|dep| dep.crate_id() == krate.crate_id()) { let root_file = crate_graph.crate_root(crate_id); let source_root_id = db.file_source_root(root_file); @@ -85,15 +77,16 @@ impl NameDefinition { files.extend(source_root.walk().map(|id| (id.into(), None))); } } - return files; } } + let mut res = FxHashSet::default(); let range = match module_src.ast { ModuleSource::Module(m) => Some(m.syntax().text_range()), ModuleSource::SourceFile(_) => None, }; - [(file_id, range)].iter().cloned().collect() + res.insert((file_id, range)); + res } } -- cgit v1.2.3