Unnamed repository; edit this file 'description' to name the repository.
| -rw-r--r-- | crates/hir-ty/src/display.rs | 42 | ||||
| -rw-r--r-- | crates/hir/src/display.rs | 17 | ||||
| -rw-r--r-- | crates/hir/src/lib.rs | 3 | ||||
| -rw-r--r-- | crates/ide-db/src/defs.rs | 58 | ||||
| -rw-r--r-- | crates/ide/src/lib.rs | 4 | ||||
| -rw-r--r-- | crates/ide/src/moniker.rs | 343 | ||||
| -rw-r--r-- | crates/ide/src/static_index.rs | 4 | ||||
| -rw-r--r-- | crates/rust-analyzer/src/cli/lsif.rs | 7 | ||||
| -rw-r--r-- | crates/rust-analyzer/src/cli/scip.rs | 314 |
9 files changed, 522 insertions, 270 deletions
diff --git a/crates/hir-ty/src/display.rs b/crates/hir-ty/src/display.rs index de8ce56df6..d8db473682 100644 --- a/crates/hir-ty/src/display.rs +++ b/crates/hir-ty/src/display.rs @@ -474,7 +474,7 @@ impl HirDisplay for ProjectionTy { let trait_ref = self.trait_ref(f.db); write!(f, "<")?; - fmt_trait_ref(f, &trait_ref, true)?; + fmt_trait_ref(f, &trait_ref, TraitRefFormat::SelfAsTrait)?; write!( f, ">::{}", @@ -1775,21 +1775,34 @@ fn write_bounds_like_dyn_trait( Ok(()) } +#[derive(Clone, Copy)] +pub enum TraitRefFormat { + SelfAsTrait, + SelfImplementsTrait, + OnlyTrait, +} + fn fmt_trait_ref( f: &mut HirFormatter<'_>, tr: &TraitRef, - use_as: bool, + format: TraitRefFormat, ) -> Result<(), HirDisplayError> { if f.should_truncate() { return write!(f, "{TYPE_HINT_TRUNCATION}"); } - tr.self_type_parameter(Interner).hir_fmt(f)?; - if use_as { - write!(f, " as ")?; - } else { - write!(f, ": ")?; + match format { + TraitRefFormat::SelfAsTrait => { + tr.self_type_parameter(Interner).hir_fmt(f)?; + write!(f, " as ")?; + } + TraitRefFormat::SelfImplementsTrait => { + tr.self_type_parameter(Interner).hir_fmt(f)?; + write!(f, ": ")?; + } + TraitRefFormat::OnlyTrait => {} } + let trait_ = tr.hir_trait_id(); f.start_location_link(trait_.into()); write!(f, "{}", f.db.trait_data(trait_).name.display(f.db.upcast(), f.edition()))?; @@ -1798,9 +1811,14 @@ fn fmt_trait_ref( hir_fmt_generics(f, &substs[1..], None, substs[0].ty(Interner)) } -impl HirDisplay for TraitRef { +pub struct TraitRefDisplayWrapper { + pub trait_ref: TraitRef, + pub format: TraitRefFormat, +} + +impl HirDisplay for TraitRefDisplayWrapper { fn hir_fmt(&self, f: &mut HirFormatter<'_>) -> Result<(), HirDisplayError> { - fmt_trait_ref(f, self, false) + fmt_trait_ref(f, &self.trait_ref, self.format) } } @@ -1811,10 +1829,12 @@ impl HirDisplay for WhereClause { } match self { - WhereClause::Implemented(trait_ref) => trait_ref.hir_fmt(f)?, + WhereClause::Implemented(trait_ref) => { + fmt_trait_ref(f, trait_ref, TraitRefFormat::SelfImplementsTrait)?; + } WhereClause::AliasEq(AliasEq { alias: AliasTy::Projection(projection_ty), ty }) => { write!(f, "<")?; - fmt_trait_ref(f, &projection_ty.trait_ref(f.db), true)?; + fmt_trait_ref(f, &projection_ty.trait_ref(f.db), TraitRefFormat::SelfAsTrait)?; write!(f, ">::",)?; let type_alias = from_assoc_type_id(projection_ty.associated_ty_id); f.start_location_link(type_alias.into()); diff --git a/crates/hir/src/display.rs b/crates/hir/src/display.rs index 959d62d595..0d97585476 100644 --- a/crates/hir/src/display.rs +++ b/crates/hir/src/display.rs @@ -22,7 +22,7 @@ use itertools::Itertools; use crate::{ Adt, AsAssocItem, AssocItem, AssocItemContainer, Const, ConstParam, Enum, ExternCrateDecl, Field, Function, GenericParam, HasCrate, HasVisibility, Impl, LifetimeParam, Macro, Module, - SelfParam, Static, Struct, Trait, TraitAlias, TupleField, TyBuilder, Type, TypeAlias, + SelfParam, Static, Struct, Trait, TraitAlias, TraitRef, TupleField, TyBuilder, Type, TypeAlias, TypeOrConstParam, TypeParam, Union, Variant, }; @@ -743,6 +743,21 @@ impl HirDisplay for Static { } } +pub struct TraitRefDisplayWrapper { + pub trait_ref: TraitRef, + pub format: hir_ty::display::TraitRefFormat, +} + +impl HirDisplay for TraitRefDisplayWrapper { + fn hir_fmt(&self, f: &mut HirFormatter<'_>) -> Result<(), HirDisplayError> { + hir_ty::display::TraitRefDisplayWrapper { + format: self.format, + trait_ref: self.trait_ref.trait_ref.clone(), + } + .hir_fmt(f) + } +} + impl HirDisplay for Trait { fn hir_fmt(&self, f: &mut HirFormatter<'_>) -> Result<(), HirDisplayError> { write_trait_header(self, f)?; diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index ac8a62ee85..d0fd2e8e05 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -96,6 +96,7 @@ use crate::db::{DefDatabase, HirDatabase}; pub use crate::{ attrs::{resolve_doc_path_on, HasAttrs}, diagnostics::*, + display::TraitRefDisplayWrapper, has_source::HasSource, semantics::{ PathResolution, Semantics, SemanticsImpl, SemanticsScope, TypeInfo, VisibleTraits, @@ -148,7 +149,7 @@ pub use { hir_ty::{ consteval::ConstEvalError, diagnostics::UnsafetyReason, - display::{ClosureStyle, HirDisplay, HirDisplayError, HirWrite}, + display::{ClosureStyle, HirDisplay, HirDisplayError, HirWrite, TraitRefFormat}, dyn_compatibility::{DynCompatibilityViolation, MethodViolationCode}, layout::LayoutError, mir::{MirEvalError, MirLowerError}, diff --git a/crates/ide-db/src/defs.rs b/crates/ide-db/src/defs.rs index 73b73736ce..91d5cf2945 100644 --- a/crates/ide-db/src/defs.rs +++ b/crates/ide-db/src/defs.rs @@ -13,10 +13,10 @@ use either::Either; use hir::{ Adt, AsAssocItem, AsExternAssocItem, AssocItem, AttributeTemplate, BuiltinAttr, BuiltinType, Const, Crate, DefWithBody, DeriveHelper, DocLinkDef, ExternAssocItem, ExternCrateDecl, Field, - Function, GenericParam, GenericSubstitution, HasVisibility, HirDisplay, Impl, InlineAsmOperand, - Label, Local, Macro, Module, ModuleDef, Name, PathResolution, Semantics, Static, - StaticLifetime, Struct, ToolModule, Trait, TraitAlias, TupleField, TypeAlias, Variant, - VariantDef, Visibility, + Function, GenericDef, GenericParam, GenericSubstitution, HasContainer, HasVisibility, + HirDisplay, Impl, InlineAsmOperand, ItemContainer, Label, Local, Macro, Module, ModuleDef, + Name, PathResolution, Semantics, Static, StaticLifetime, Struct, ToolModule, Trait, TraitAlias, + TupleField, TypeAlias, Variant, VariantDef, Visibility, }; use span::Edition; use stdx::{format_to, impl_from}; @@ -98,8 +98,30 @@ impl Definition { pub fn enclosing_definition(&self, db: &RootDatabase) -> Option<Definition> { match self { + Definition::Macro(it) => Some(it.module(db).into()), + Definition::Module(it) => it.parent(db).map(Definition::Module), + Definition::Field(it) => Some(it.parent_def(db).into()), + Definition::Function(it) => it.container(db).try_into().ok(), + Definition::Adt(it) => Some(it.module(db).into()), + Definition::Const(it) => it.container(db).try_into().ok(), + Definition::Static(it) => it.container(db).try_into().ok(), + Definition::Trait(it) => it.container(db).try_into().ok(), + Definition::TraitAlias(it) => it.container(db).try_into().ok(), + Definition::TypeAlias(it) => it.container(db).try_into().ok(), + Definition::Variant(it) => Some(Adt::Enum(it.parent_enum(db)).into()), + Definition::SelfType(it) => Some(it.module(db).into()), Definition::Local(it) => it.parent(db).try_into().ok(), - _ => None, + Definition::GenericParam(it) => Some(it.parent().into()), + Definition::Label(it) => it.parent(db).try_into().ok(), + Definition::ExternCrateDecl(it) => it.container(db).try_into().ok(), + Definition::DeriveHelper(it) => Some(it.derive().module(db).into()), + Definition::InlineAsmOperand(it) => it.parent(db).try_into().ok(), + Definition::BuiltinAttr(_) + | Definition::BuiltinType(_) + | Definition::BuiltinLifetime(_) + | Definition::TupleField(_) + | Definition::ToolModule(_) + | Definition::InlineAsmRegOrRegClass(_) => None, } } @@ -932,3 +954,29 @@ impl TryFrom<DefWithBody> for Definition { } } } + +impl TryFrom<ItemContainer> for Definition { + type Error = (); + fn try_from(container: ItemContainer) -> Result<Self, Self::Error> { + match container { + ItemContainer::Trait(it) => Ok(it.into()), + ItemContainer::Impl(it) => Ok(it.into()), + ItemContainer::Module(it) => Ok(it.into()), + ItemContainer::ExternBlock() | ItemContainer::Crate(_) => Err(()), + } + } +} + +impl From<GenericDef> for Definition { + fn from(def: GenericDef) -> Self { + match def { + GenericDef::Function(it) => it.into(), + GenericDef::Adt(it) => it.into(), + GenericDef::Trait(it) => it.into(), + GenericDef::TraitAlias(it) => it.into(), + GenericDef::TypeAlias(it) => it.into(), + GenericDef::Impl(it) => it.into(), + GenericDef::Const(it) => it.into(), + } + } +} diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index fe2760d2ba..1637d578fd 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -96,8 +96,8 @@ pub use crate::{ join_lines::JoinLinesConfig, markup::Markup, moniker::{ - MonikerDescriptorKind, MonikerKind, MonikerResult, PackageInformation, - SymbolInformationKind, + Moniker, MonikerDescriptorKind, MonikerIdentifier, MonikerKind, MonikerResult, + PackageInformation, SymbolInformationKind, }, move_item::Direction, navigation_target::{NavigationTarget, TryToNav, UpmappingResult}, diff --git a/crates/ide/src/moniker.rs b/crates/ide/src/moniker.rs index 14781b2129..adb4850338 100644 --- a/crates/ide/src/moniker.rs +++ b/crates/ide/src/moniker.rs @@ -3,7 +3,10 @@ use core::fmt; -use hir::{Adt, AsAssocItem, AssocItemContainer, Crate, MacroKind, Semantics}; +use hir::{ + Adt, AsAssocItem, Crate, HirDisplay, MacroKind, Semantics, TraitRefDisplayWrapper, + TraitRefFormat, +}; use ide_db::{ base_db::{CrateOrigin, LangCrateOrigin}, defs::{Definition, IdentClass}, @@ -11,6 +14,7 @@ use ide_db::{ FilePosition, RootDatabase, }; use itertools::Itertools; +use span::Edition; use syntax::{AstNode, SyntaxKind::*, T}; use crate::{doc_links::token_as_doc_comment, parent_module::crates_for, RangeInfo}; @@ -57,8 +61,8 @@ pub enum SymbolInformationKind { impl From<SymbolInformationKind> for MonikerDescriptorKind { fn from(value: SymbolInformationKind) -> Self { match value { - SymbolInformationKind::AssociatedType => Self::TypeParameter, - SymbolInformationKind::Attribute => Self::Macro, + SymbolInformationKind::AssociatedType => Self::Type, + SymbolInformationKind::Attribute => Self::Meta, SymbolInformationKind::Constant => Self::Term, SymbolInformationKind::Enum => Self::Type, SymbolInformationKind::EnumMember => Self::Type, @@ -70,7 +74,7 @@ impl From<SymbolInformationKind> for MonikerDescriptorKind { SymbolInformationKind::Parameter => Self::Parameter, SymbolInformationKind::SelfParameter => Self::Parameter, SymbolInformationKind::StaticMethod => Self::Method, - SymbolInformationKind::StaticVariable => Self::Meta, + SymbolInformationKind::StaticVariable => Self::Term, SymbolInformationKind::Struct => Self::Type, SymbolInformationKind::Trait => Self::Type, SymbolInformationKind::TraitMethod => Self::Method, @@ -109,10 +113,12 @@ pub enum MonikerKind { } #[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub struct MonikerResult { - pub identifier: MonikerIdentifier, - pub kind: MonikerKind, - pub package_information: PackageInformation, +pub enum MonikerResult { + /// Uniquely identifies a definition. + Moniker(Moniker), + /// Specifies that the definition is a local, and so does not have a unique identifier. Provides + /// a unique identifier for the container. + Local { enclosing_moniker: Option<Moniker> }, } impl MonikerResult { @@ -121,6 +127,15 @@ impl MonikerResult { } } +/// Information which uniquely identifies a definition which might be referenceable outside of the +/// source file. Visibility declarations do not affect presence. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct Moniker { + pub identifier: MonikerIdentifier, + pub kind: MonikerKind, + pub package_information: PackageInformation, +} + #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct PackageInformation { pub name: String, @@ -232,157 +247,129 @@ pub(crate) fn def_to_kind(db: &RootDatabase, def: Definition) -> SymbolInformati } } +/// Computes a `MonikerResult` for a definition. Result cases: +/// +/// `Some(MonikerResult::Moniker(_))` provides a unique `Moniker` which refers to a definition. +/// +/// `Some(MonikerResult::Local { .. })` provides a `Moniker` for the definition enclosing a local. +/// +/// `None` is returned in the following cases: +/// +/// * Inherent impl definitions, as they cannot be uniquely identified (multiple are allowed for the +/// same type). +/// +/// * Definitions which are not in a module: `BuiltinAttr`, `BuiltinType`, `BuiltinLifetime`, +/// `TupleField`, `ToolModule`, and `InlineAsmRegOrRegClass`. TODO: it might be sensible to +/// provide monikers that refer to some non-existent crate of compiler builtin definitions. pub(crate) fn def_to_moniker( db: &RootDatabase, - def: Definition, + definition: Definition, from_crate: Crate, ) -> Option<MonikerResult> { - if matches!( - def, - Definition::GenericParam(_) - | Definition::Label(_) - | Definition::DeriveHelper(_) - | Definition::BuiltinAttr(_) - | Definition::ToolModule(_) - ) { - return None; + match definition { + // Not possible to give sensible unique symbols for inherent impls, as multiple can be + // defined for the same type. + Definition::SelfType(impl_) if impl_.trait_(db).is_none() => { + return None; + } + Definition::Local(_) | Definition::Label(_) | Definition::GenericParam(_) => { + return Some(MonikerResult::Local { + enclosing_moniker: enclosing_def_to_moniker(db, definition, from_crate), + }); + } + _ => {} } + Some(MonikerResult::Moniker(def_to_non_local_moniker(db, definition, from_crate)?)) +} - let module = def.module(db)?; - let krate = module.krate(); - let edition = krate.edition(db); - let mut description = vec![]; - description.extend(module.path_to_root(db).into_iter().filter_map(|x| { - Some(MonikerDescriptor { - name: x.name(db)?.display(db, edition).to_string(), - desc: def_to_kind(db, x.into()).into(), - }) - })); - - // Handle associated items within a trait - if let Some(assoc) = def.as_assoc_item(db) { - let container = assoc.container(db); - match container { - AssocItemContainer::Trait(trait_) => { - // Because different traits can have functions with the same name, - // we have to include the trait name as part of the moniker for uniqueness. - description.push(MonikerDescriptor { - name: trait_.name(db).display(db, edition).to_string(), - desc: def_to_kind(db, trait_.into()).into(), - }); - } - AssocItemContainer::Impl(impl_) => { - // Because a struct can implement multiple traits, for implementations - // we add both the struct name and the trait name to the path - if let Some(adt) = impl_.self_ty(db).as_adt() { - description.push(MonikerDescriptor { - name: adt.name(db).display(db, edition).to_string(), - desc: def_to_kind(db, adt.into()).into(), - }); - } - - if let Some(trait_) = impl_.trait_(db) { - description.push(MonikerDescriptor { - name: trait_.name(db).display(db, edition).to_string(), - desc: def_to_kind(db, trait_.into()).into(), - }); - } - } +fn enclosing_def_to_moniker( + db: &RootDatabase, + mut def: Definition, + from_crate: Crate, +) -> Option<Moniker> { + loop { + let enclosing_def = def.enclosing_definition(db)?; + if let Some(enclosing_moniker) = def_to_non_local_moniker(db, enclosing_def, from_crate) { + return Some(enclosing_moniker); } + def = enclosing_def; } +} - if let Definition::Field(it) = def { - description.push(MonikerDescriptor { - name: it.parent_def(db).name(db).display(db, edition).to_string(), - desc: def_to_kind(db, it.parent_def(db).into()).into(), - }); +fn def_to_non_local_moniker( + db: &RootDatabase, + definition: Definition, + from_crate: Crate, +) -> Option<Moniker> { + match definition { + // Not possible to give sensible unique symbols for inherent impls, as multiple can be + // defined for the same type. + Definition::SelfType(impl_) if impl_.trait_(db).is_none() => { + return None; + } + _ => {} } - // Qualify locals/parameters by their parent definition name. - if let Definition::Local(it) = def { - let parent = Definition::try_from(it.parent(db)).ok(); - if let Some(parent) = parent { - let parent_name = parent.name(db); - if let Some(name) = parent_name { - description.push(MonikerDescriptor { - name: name.display(db, edition).to_string(), - desc: def_to_kind(db, parent).into(), + let module = definition.module(db)?; + let krate = module.krate(); + let edition = krate.edition(db); + + // Add descriptors for this definition and every enclosing definition. + let mut reverse_description = vec![]; + let mut def = definition; + loop { + match def { + Definition::SelfType(impl_) => { + if let Some(trait_ref) = impl_.trait_ref(db) { + // Trait impls use `trait_type` constraint syntax for the 2nd parameter. + let trait_ref_for_display = + TraitRefDisplayWrapper { trait_ref, format: TraitRefFormat::OnlyTrait }; + reverse_description.push(MonikerDescriptor { + name: display(db, edition, module, trait_ref_for_display), + desc: MonikerDescriptorKind::TypeParameter, + }); + } + // Both inherent and trait impls use `self_type` as the first parameter. + reverse_description.push(MonikerDescriptor { + name: display(db, edition, module, impl_.self_ty(db)), + desc: MonikerDescriptorKind::TypeParameter, + }); + reverse_description.push(MonikerDescriptor { + name: "impl".to_owned(), + desc: MonikerDescriptorKind::Type, }); } - } - } - - let desc = def_to_kind(db, def).into(); - - let name_desc = match def { - // These are handled by top-level guard (for performance). - Definition::GenericParam(_) - | Definition::Label(_) - | Definition::DeriveHelper(_) - | Definition::BuiltinLifetime(_) - | Definition::BuiltinAttr(_) - | Definition::ToolModule(_) - | Definition::InlineAsmRegOrRegClass(_) - | Definition::InlineAsmOperand(_) => return None, - - Definition::Local(local) => { - if !local.is_param(db) { - return None; + _ => { + if let Some(name) = def.name(db) { + reverse_description.push(MonikerDescriptor { + name: name.display(db, edition).to_string(), + desc: def_to_kind(db, def).into(), + }); + } else if reverse_description.is_empty() { + // Don't allow the last descriptor to be absent. + return None; + } else { + match def { + Definition::Module(module) if module.is_crate_root() => {} + _ => { + tracing::error!( + "Encountered enclosing definition with no name: {:?}", + def + ); + } + } + } } - - MonikerDescriptor { name: local.name(db).display(db, edition).to_string(), desc } - } - Definition::Macro(m) => { - MonikerDescriptor { name: m.name(db).display(db, edition).to_string(), desc } - } - Definition::Function(f) => { - MonikerDescriptor { name: f.name(db).display(db, edition).to_string(), desc } - } - Definition::Variant(v) => { - MonikerDescriptor { name: v.name(db).display(db, edition).to_string(), desc } - } - Definition::Const(c) => { - MonikerDescriptor { name: c.name(db)?.display(db, edition).to_string(), desc } - } - Definition::Trait(trait_) => { - MonikerDescriptor { name: trait_.name(db).display(db, edition).to_string(), desc } - } - Definition::TraitAlias(ta) => { - MonikerDescriptor { name: ta.name(db).display(db, edition).to_string(), desc } - } - Definition::TypeAlias(ta) => { - MonikerDescriptor { name: ta.name(db).display(db, edition).to_string(), desc } - } - Definition::Module(m) => { - MonikerDescriptor { name: m.name(db)?.display(db, edition).to_string(), desc } } - Definition::BuiltinType(b) => { - MonikerDescriptor { name: b.name().display(db, edition).to_string(), desc } - } - Definition::SelfType(imp) => MonikerDescriptor { - name: imp.self_ty(db).as_adt()?.name(db).display(db, edition).to_string(), - desc, - }, - Definition::Field(it) => { - MonikerDescriptor { name: it.name(db).display(db, edition).to_string(), desc } - } - Definition::TupleField(it) => { - MonikerDescriptor { name: it.name().display(db, edition).to_string(), desc } - } - Definition::Adt(adt) => { - MonikerDescriptor { name: adt.name(db).display(db, edition).to_string(), desc } - } - Definition::Static(s) => { - MonikerDescriptor { name: s.name(db).display(db, edition).to_string(), desc } - } - Definition::ExternCrateDecl(m) => { - MonikerDescriptor { name: m.name(db).display(db, edition).to_string(), desc } - } - }; - - description.push(name_desc); + let Some(next_def) = def.enclosing_definition(db) else { + break; + }; + def = next_def; + } + reverse_description.reverse(); + let description = reverse_description; - Some(MonikerResult { + Some(Moniker { identifier: MonikerIdentifier { crate_name: krate.display_name(db)?.crate_name().to_string(), description, @@ -417,17 +404,58 @@ pub(crate) fn def_to_moniker( }) } +fn display<T: HirDisplay>( + db: &RootDatabase, + edition: Edition, + module: hir::Module, + it: T, +) -> String { + match it.display_source_code(db, module.into(), true) { + Ok(result) => result, + // Fallback on display variant that always succeeds + Err(_) => { + let fallback_result = it.display(db, edition).to_string(); + tracing::error!( + "display_source_code failed. Falling back to using display, which has result: {}", + fallback_result + ); + fallback_result + } + } +} + #[cfg(test)] mod tests { - use crate::fixture; + use crate::{fixture, MonikerResult}; use super::MonikerKind; + #[allow(dead_code)] #[track_caller] fn no_moniker(ra_fixture: &str) { let (analysis, position) = fixture::position(ra_fixture); if let Some(x) = analysis.moniker(position).unwrap() { - assert_eq!(x.info.len(), 0, "Moniker founded but no moniker expected: {x:?}"); + assert_eq!(x.info.len(), 0, "Moniker found but no moniker expected: {x:?}"); + } + } + + #[track_caller] + fn check_local_moniker(ra_fixture: &str, identifier: &str, package: &str, kind: MonikerKind) { + let (analysis, position) = fixture::position(ra_fixture); + let x = analysis.moniker(position).unwrap().expect("no moniker found").info; + assert_eq!(x.len(), 1); + match x.into_iter().next().unwrap() { + MonikerResult::Local { enclosing_moniker: Some(x) } => { + assert_eq!(identifier, x.identifier.to_string()); + assert_eq!(package, format!("{:?}", x.package_information)); + assert_eq!(kind, x.kind); + } + MonikerResult::Local { enclosing_moniker: None } => { + panic!("Unexpected local with no enclosing moniker"); + } + MonikerResult::Moniker(_) => { + panic!("Unexpected non-local moniker"); + } } } @@ -436,10 +464,16 @@ mod tests { let (analysis, position) = fixture::position(ra_fixture); let x = analysis.moniker(position).unwrap().expect("no moniker found").info; assert_eq!(x.len(), 1); - let x = x.into_iter().next().unwrap(); - assert_eq!(identifier, x.identifier.to_string()); - assert_eq!(package, format!("{:?}", x.package_information)); - assert_eq!(kind, x.kind); + match x.into_iter().next().unwrap() { + MonikerResult::Local { enclosing_moniker } => { + panic!("Unexpected local enclosed in {:?}", enclosing_moniker); + } + MonikerResult::Moniker(x) => { + assert_eq!(identifier, x.identifier.to_string()); + assert_eq!(package, format!("{:?}", x.package_information)); + assert_eq!(kind, x.kind); + } + } } #[test] @@ -538,15 +572,13 @@ pub mod module { pub trait MyTrait { pub fn func() {} } - struct MyStruct {} - impl MyTrait for MyStruct { pub fn func$0() {} } } "#, - "foo::module::MyStruct::MyTrait::func", + "foo::module::impl::MyStruct::MyTrait::func", r#"PackageInformation { name: "foo", repo: Some("https://a.b/foo.git"), version: Some("0.1.0") }"#, MonikerKind::Export, ); @@ -573,8 +605,8 @@ pub struct St { } #[test] - fn no_moniker_for_local() { - no_moniker( + fn local() { + check_local_moniker( r#" //- /lib.rs crate:main deps:foo use foo::module::func; @@ -588,6 +620,9 @@ pub mod module { } } "#, + "foo::module::func", + r#"PackageInformation { name: "foo", repo: Some("https://a.b/foo.git"), version: Some("0.1.0") }"#, + MonikerKind::Export, ); } } diff --git a/crates/ide/src/static_index.rs b/crates/ide/src/static_index.rs index 53eeffaf97..700e166b23 100644 --- a/crates/ide/src/static_index.rs +++ b/crates/ide/src/static_index.rs @@ -48,7 +48,6 @@ pub struct TokenStaticData { pub references: Vec<ReferenceData>, pub moniker: Option<MonikerResult>, pub display_name: Option<String>, - pub enclosing_moniker: Option<MonikerResult>, pub signature: Option<String>, pub kind: SymbolInformationKind, } @@ -225,9 +224,6 @@ impl StaticIndex<'_> { display_name: def .name(self.db) .map(|name| name.display(self.db, edition).to_string()), - enclosing_moniker: current_crate - .zip(def.enclosing_definition(self.db)) - .and_then(|(cc, enclosing_def)| def_to_moniker(self.db, enclosing_def, cc)), signature: Some(def.label(self.db, edition)), kind: def_to_kind(self.db, def), }); diff --git a/crates/rust-analyzer/src/cli/lsif.rs b/crates/rust-analyzer/src/cli/lsif.rs index 33c4f31fbe..eb5c44418b 100644 --- a/crates/rust-analyzer/src/cli/lsif.rs +++ b/crates/rust-analyzer/src/cli/lsif.rs @@ -4,8 +4,9 @@ use std::env; use std::time::Instant; use ide::{ - Analysis, AnalysisHost, FileId, FileRange, MonikerKind, PackageInformation, RootDatabase, - StaticIndex, StaticIndexedFile, TokenId, TokenStaticData, VendoredLibrariesConfig, + Analysis, AnalysisHost, FileId, FileRange, MonikerKind, MonikerResult, PackageInformation, + RootDatabase, StaticIndex, StaticIndexedFile, TokenId, TokenStaticData, + VendoredLibrariesConfig, }; use ide_db::{line_index::WideEncoding, LineIndexDatabase}; use load_cargo::{load_workspace, LoadCargoConfig, ProcMacroServerChoice}; @@ -167,7 +168,7 @@ impl LsifManager<'_, '_> { out_v: result_set_id.into(), })); } - if let Some(moniker) = token.moniker { + if let Some(MonikerResult::Moniker(moniker)) = token.moniker { let package_id = self.get_package_id(moniker.package_information); let moniker_id = self.add_vertex(lsif::Vertex::Moniker(lsp_types::Moniker { scheme: "rust-analyzer".to_owned(), diff --git a/crates/rust-analyzer/src/cli/scip.rs b/crates/rust-analyzer/src/cli/scip.rs index ff009e6954..6b0aafc248 100644 --- a/crates/rust-analyzer/src/cli/scip.rs +++ b/crates/rust-analyzer/src/cli/scip.rs @@ -3,8 +3,9 @@ use std::{path::PathBuf, time::Instant}; use ide::{ - AnalysisHost, LineCol, MonikerDescriptorKind, MonikerResult, StaticIndex, StaticIndexedFile, - SymbolInformationKind, TextRange, TokenId, VendoredLibrariesConfig, + AnalysisHost, LineCol, Moniker, MonikerDescriptorKind, MonikerIdentifier, MonikerResult, + StaticIndex, StaticIndexedFile, SymbolInformationKind, TextRange, TokenId, TokenStaticData, + VendoredLibrariesConfig, }; use ide_db::LineIndexDatabase; use load_cargo::{load_workspace_at, LoadCargoConfig, ProcMacroServerChoice}; @@ -85,19 +86,13 @@ impl flags::Scip { }; let mut documents = Vec::new(); - let mut symbols_emitted: FxHashSet<TokenId> = FxHashSet::default(); - let mut tokens_to_symbol: FxHashMap<TokenId, String> = FxHashMap::default(); - let mut tokens_to_enclosing_symbol: FxHashMap<TokenId, Option<String>> = - FxHashMap::default(); + let mut token_ids_emitted: FxHashSet<TokenId> = FxHashSet::default(); + let mut global_symbols_emitted: FxHashSet<String> = FxHashSet::default(); + let mut duplicate_symbols: Vec<(String, String)> = Vec::new(); + let mut symbol_generator = SymbolGenerator::new(); for StaticIndexedFile { file_id, tokens, .. } in si.files { - let mut local_count = 0; - let mut new_local_symbol = || { - let new_symbol = scip::types::Symbol::new_local(local_count); - local_count += 1; - - new_symbol - }; + symbol_generator.clear_document_local_state(); let relative_path = match get_relative_filepath(&vfs, &root, file_id) { Some(relative_path) => relative_path, @@ -116,39 +111,23 @@ impl flags::Scip { tokens.into_iter().for_each(|(text_range, id)| { let token = si.tokens.get(id).unwrap(); - let range = text_range_to_scip_range(&line_index, text_range); - let symbol = tokens_to_symbol - .entry(id) - .or_insert_with(|| { - let symbol = token - .moniker - .as_ref() - .map(moniker_to_symbol) - .unwrap_or_else(&mut new_local_symbol); - scip::symbol::format_symbol(symbol) - }) - .clone(); - let enclosing_symbol = tokens_to_enclosing_symbol - .entry(id) - .or_insert_with(|| { - token - .enclosing_moniker - .as_ref() - .map(moniker_to_symbol) - .map(scip::symbol::format_symbol) - }) - .clone(); - - let mut symbol_roles = Default::default(); - - if let Some(def) = token.definition { - // if the range of the def and the range of the token are the same, this must be the definition. - // they also must be in the same file. See https://github.com/rust-lang/rust-analyzer/pull/17988 - if def.file_id == file_id && def.range == text_range { - symbol_roles |= scip_types::SymbolRole::Definition as i32; - } - - if symbols_emitted.insert(id) { + let (symbol, enclosing_symbol) = + if let Some(TokenSymbols { symbol, enclosing_symbol }) = + symbol_generator.token_symbols(id, token) + { + (symbol, enclosing_symbol) + } else { + ("".to_owned(), None) + }; + + if !symbol.is_empty() && token_ids_emitted.insert(id) { + if !symbol.starts_with("local ") + && !global_symbols_emitted.insert(symbol.clone()) + { + let source_location = + text_range_to_string(relative_path.as_str(), &line_index, text_range); + duplicate_symbols.push((source_location, symbol.clone())); + } else { let documentation = match &token.documentation { Some(doc) => vec![doc.as_str().to_owned()], None => vec![], @@ -179,8 +158,18 @@ impl flags::Scip { } } + // If the range of the def and the range of the token are the same, this must be the definition. + // they also must be in the same file. See https://github.com/rust-lang/rust-analyzer/pull/17988 + let mut symbol_roles = Default::default(); + match token.definition { + Some(def) if def.file_id == file_id && def.range == text_range => { + symbol_roles |= scip_types::SymbolRole::Definition as i32; + } + _ => {} + }; + occurrences.push(scip_types::Occurrence { - range, + range: text_range_to_scip_range(&line_index, text_range), symbol, symbol_roles, override_documentation: Vec::new(), @@ -215,6 +204,15 @@ impl flags::Scip { special_fields: Default::default(), }; + if !duplicate_symbols.is_empty() { + eprintln!("{}", DUPLICATE_SYMBOLS_MESSAGE); + for (source_location, symbol) in duplicate_symbols { + eprintln!("{}", source_location); + eprintln!(" Duplicate symbol: {}", symbol); + eprintln!(); + } + } + let out_path = self.output.unwrap_or_else(|| PathBuf::from(r"index.scip")); scip::write_message_to_file(out_path, index) .map_err(|err| anyhow::format_err!("Failed to write scip to file: {}", err))?; @@ -224,6 +222,23 @@ impl flags::Scip { } } +// TODO: Fix the known buggy cases described here. +const DUPLICATE_SYMBOLS_MESSAGE: &str = " +Encountered duplicate scip symbols, indicating an internal rust-analyzer bug. These duplicates are +included in the output, but this causes information lookup to be ambiguous and so information about +these symbols presented by downstream tools may be incorrect. + +Known cases that can cause this: + + * Definitions in crate example binaries which have the same symbol as definitions in the library + or some other example. + + * When a struct/enum/const/static/impl is defined with a function, it erroneously appears to be + defined at the same level as the function. + +Duplicate symbols encountered: +"; + fn get_relative_filepath( vfs: &vfs::Vfs, rootpath: &vfs::AbsPathBuf, @@ -247,6 +262,13 @@ fn text_range_to_scip_range(line_index: &LineIndex, range: TextRange) -> Vec<i32 } } +fn text_range_to_string(relative_path: &str, line_index: &LineIndex, range: TextRange) -> String { + let LineCol { line: start_line, col: start_col } = line_index.index.line_col(range.start()); + let LineCol { line: end_line, col: end_col } = line_index.index.line_col(range.end()); + + format!("{relative_path}:{start_line}:{start_col}-{end_line}:{end_col}") +} + fn new_descriptor_str( name: &str, suffix: scip_types::descriptor::Suffix, @@ -259,14 +281,6 @@ fn new_descriptor_str( } } -fn new_descriptor(name: &str, suffix: scip_types::descriptor::Suffix) -> scip_types::Descriptor { - if name.contains('\'') { - new_descriptor_str(&format!("`{name}`"), suffix) - } else { - new_descriptor_str(name, suffix) - } -} - fn symbol_kind(kind: SymbolInformationKind) -> scip_types::symbol_information::Kind { use scip_types::symbol_information::Kind as ScipKind; match kind { @@ -295,17 +309,79 @@ fn symbol_kind(kind: SymbolInformationKind) -> scip_types::symbol_information::K } } -fn moniker_to_symbol(moniker: &MonikerResult) -> scip_types::Symbol { - use scip_types::descriptor::Suffix::*; +#[derive(Clone)] +struct TokenSymbols { + symbol: String, + /// Definition that contains this one. Only set when `symbol` is local. + enclosing_symbol: Option<String>, +} + +struct SymbolGenerator { + token_to_symbols: FxHashMap<TokenId, Option<TokenSymbols>>, + local_count: usize, +} + +impl SymbolGenerator { + fn new() -> Self { + SymbolGenerator { token_to_symbols: FxHashMap::default(), local_count: 0 } + } + + fn clear_document_local_state(&mut self) { + self.local_count = 0; + } + + fn token_symbols(&mut self, id: TokenId, token: &TokenStaticData) -> Option<TokenSymbols> { + let mut local_count = self.local_count; + let token_symbols = self + .token_to_symbols + .entry(id) + .or_insert_with(|| { + Some(match token.moniker.as_ref()? { + MonikerResult::Moniker(moniker) => TokenSymbols { + symbol: scip::symbol::format_symbol(moniker_to_symbol(moniker)), + enclosing_symbol: None, + }, + MonikerResult::Local { enclosing_moniker } => { + let local_symbol = scip::types::Symbol::new_local(local_count); + local_count += 1; + TokenSymbols { + symbol: scip::symbol::format_symbol(local_symbol), + enclosing_symbol: enclosing_moniker + .as_ref() + .map(moniker_to_symbol) + .map(scip::symbol::format_symbol), + } + } + }) + }) + .clone(); + self.local_count = local_count; + token_symbols + } +} - let package_name = moniker.package_information.name.clone(); - let version = moniker.package_information.version.clone(); - let descriptors = moniker - .identifier +fn moniker_to_symbol(moniker: &Moniker) -> scip_types::Symbol { + scip_types::Symbol { + scheme: "rust-analyzer".into(), + package: Some(scip_types::Package { + manager: "cargo".to_owned(), + name: moniker.package_information.name.clone(), + version: moniker.package_information.version.clone().unwrap_or_else(|| ".".to_owned()), + special_fields: Default::default(), + }) + .into(), + descriptors: moniker_descriptors(&moniker.identifier), + special_fields: Default::default(), + } +} + +fn moniker_descriptors(identifier: &MonikerIdentifier) -> Vec<scip_types::Descriptor> { + use scip_types::descriptor::Suffix::*; + identifier .description .iter() .map(|desc| { - new_descriptor( + new_descriptor_str( &desc.name, match desc.desc { MonikerDescriptorKind::Namespace => Namespace, @@ -319,27 +395,13 @@ fn moniker_to_symbol(moniker: &MonikerResult) -> scip_types::Symbol { }, ) }) - .collect(); - - scip_types::Symbol { - scheme: "rust-analyzer".into(), - package: Some(scip_types::Package { - manager: "cargo".to_owned(), - name: package_name, - version: version.unwrap_or_else(|| ".".to_owned()), - special_fields: Default::default(), - }) - .into(), - descriptors, - special_fields: Default::default(), - } + .collect() } #[cfg(test)] mod test { use super::*; use ide::{FilePosition, TextSize}; - use scip::symbol::format_symbol; use test_fixture::ChangeFixture; use vfs::VfsPath; @@ -376,7 +438,21 @@ mod test { for &(range, id) in &file.tokens { if range.contains(offset - TextSize::from(1)) { let token = si.tokens.get(id).unwrap(); - found_symbol = token.moniker.as_ref().map(moniker_to_symbol); + found_symbol = match token.moniker.as_ref() { + None => None, + Some(MonikerResult::Moniker(moniker)) => { + Some(scip::symbol::format_symbol(moniker_to_symbol(moniker))) + } + Some(MonikerResult::Local { enclosing_moniker: Some(moniker) }) => { + Some(format!( + "local enclosed by {}", + scip::symbol::format_symbol(moniker_to_symbol(moniker)) + )) + } + Some(MonikerResult::Local { enclosing_moniker: None }) => { + Some("unenclosed local".to_owned()) + } + }; break; } } @@ -388,9 +464,7 @@ mod test { } assert!(found_symbol.is_some(), "must have one symbol {found_symbol:?}"); - let res = found_symbol.unwrap(); - let formatted = format_symbol(res); - assert_eq!(formatted, expected); + assert_eq!(found_symbol.unwrap(), expected); } #[test] @@ -467,8 +541,7 @@ pub mod module { } } "#, - // "foo::module::MyTrait::MyType", - "rust-analyzer cargo foo 0.1.0 module/MyTrait#[MyType]", + "rust-analyzer cargo foo 0.1.0 module/MyTrait#MyType#", ); } @@ -489,8 +562,7 @@ pub mod module { } } "#, - // "foo::module::MyStruct::MyTrait::func", - "rust-analyzer cargo foo 0.1.0 module/MyStruct#MyTrait#func().", + "rust-analyzer cargo foo 0.1.0 module/impl#[MyStruct][MyTrait]func().", ); } @@ -526,7 +598,7 @@ pub mod example_mod { pub fn func(x$0: usize) {} } "#, - "rust-analyzer cargo foo 0.1.0 example_mod/func().(x)", + "local enclosed by rust-analyzer cargo foo 0.1.0 example_mod/func().", ); } @@ -546,7 +618,7 @@ pub mod example_mod { } } "#, - "rust-analyzer cargo foo 0.1.0 example_mod/func().(x)", + "local enclosed by rust-analyzer cargo foo 0.1.0 example_mod/func().", ); } @@ -566,7 +638,7 @@ pub mod example_mod { } } "#, - "", + "local enclosed by rust-analyzer cargo foo 0.1.0 module/func().", ); } @@ -609,7 +681,7 @@ pub mod example_mod { } #[test] - fn symbol_for_for_type_alias() { + fn symbol_for_type_alias() { check_symbol( r#" //- /workspace/lib.rs crate:main @@ -619,6 +691,70 @@ pub mod example_mod { ); } + // TODO: This test represents current misbehavior. + #[test] + fn symbol_for_nested_function() { + check_symbol( + r#" + //- /workspace/lib.rs crate:main + pub fn func() { + pub fn inner_func$0() {} + } + "#, + "rust-analyzer cargo main . inner_func().", + // TODO: This should be a local: + // "local enclosed by rust-analyzer cargo main . func().", + ); + } + + // TODO: This test represents current misbehavior. + #[test] + fn symbol_for_struct_in_function() { + check_symbol( + r#" + //- /workspace/lib.rs crate:main + pub fn func() { + struct SomeStruct$0 {} + } + "#, + "rust-analyzer cargo main . SomeStruct#", + // TODO: This should be a local: + // "local enclosed by rust-analyzer cargo main . func().", + ); + } + + // TODO: This test represents current misbehavior. + #[test] + fn symbol_for_const_in_function() { + check_symbol( + r#" + //- /workspace/lib.rs crate:main + pub fn func() { + const SOME_CONST$0: u32 = 1; + } + "#, + "rust-analyzer cargo main . SOME_CONST.", + // TODO: This should be a local: + // "local enclosed by rust-analyzer cargo main . func().", + ); + } + + // TODO: This test represents current misbehavior. + #[test] + fn symbol_for_static_in_function() { + check_symbol( + r#" + //- /workspace/lib.rs crate:main + pub fn func() { + static SOME_STATIC$0: u32 = 1; + } + "#, + "rust-analyzer cargo main . SOME_STATIC.", + // TODO: This should be a local: + // "local enclosed by rust-analyzer cargo main . func().", + ); + } + #[test] fn documentation_matches_doc_comment() { let s = "/// foo\nfn bar() {}"; |