Skip to content

Commit b1c091c

Browse files
committed
Improve SCIP symbols
In particular, the symbol generation before this change creates a lot of symbols with the same name for different definitions. This change makes progress on symbol uniqueness, but does not fix a couple cases where it was unclear to me how to fix (see TODOs in `scip.rs`) Behavior changes: * `scip` command now reports symbol information omitted due to symbol collisions. Iterating with this on a large codebase (Zed!) resulted in the other improvements in this change. * Generally fixes providing the path to nested definitions in symbols. Instead of having special cases for a couple limited cases of nesting, implements `Definition::enclosing_definition` and uses this to walk definitions. * Parameter variables are now treated like locals. - This fixes a bug where closure captures also received symbols scoped to the containing function. To bring back parameter symbols I would want a way to filter these out, since they can cause symbol collisions. - Having symbols for them seems to be intentional in 27e2eea, but no particular use is specified there. For the typical indexing purposes of SCIP I don't see why parameter symbols are useful or sensible, as function parameters are not referencable by anything but position. I can imagine they might be useful in representing diagnostics or something. * Inherent impls are now represented as `impl#[SelfType]` - a type named `impl` which takes a single type parameter. * Trait impls are now represented as `impl#[SelfType][TraitType]` - a type named `impl` which takes two type parameters. * Associated types in traits and impls are now treated like types instead of type parameters, and so are now suffixed with `#` instead of wrapped with `[]`. Treating them as type parameters seems to have been intentional in 73d9c77 but it doesn't make sense to me, so changing it. * Static variables are now treated as terms instead of `Meta`, and so receive `.` suffix instead of `:`. * Attributes are now treated as `Meta` instead of `Macro`, and so receive `:` suffix instead of `!`. * `enclosing_symbol` is now provided for labels and generic params, which are local symbols. * Fixes a bug where presence of `'` causes a descriptor name to get double wrapped in backticks, since both `fn new_descriptor` and `scip::symbol::format_symbol` have logic for wrapping in backticks. Solution is to simply delete the redundant logic. * Deletes a couple tests in moniker.rs because the cases are adequeately covered in scip.rs and the format for identifiers used in moniker.rs is clunky with the new representation for trait impls
1 parent 022bece commit b1c091c

File tree

9 files changed

+522
-270
lines changed

9 files changed

+522
-270
lines changed

src/tools/rust-analyzer/crates/hir-ty/src/display.rs

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -474,7 +474,7 @@ impl HirDisplay for ProjectionTy {
474474

475475
let trait_ref = self.trait_ref(f.db);
476476
write!(f, "<")?;
477-
fmt_trait_ref(f, &trait_ref, true)?;
477+
fmt_trait_ref(f, &trait_ref, TraitRefFormat::SelfAsTrait)?;
478478
write!(
479479
f,
480480
">::{}",
@@ -1775,21 +1775,34 @@ fn write_bounds_like_dyn_trait(
17751775
Ok(())
17761776
}
17771777

1778+
#[derive(Clone, Copy)]
1779+
pub enum TraitRefFormat {
1780+
SelfAsTrait,
1781+
SelfImplementsTrait,
1782+
OnlyTrait,
1783+
}
1784+
17781785
fn fmt_trait_ref(
17791786
f: &mut HirFormatter<'_>,
17801787
tr: &TraitRef,
1781-
use_as: bool,
1788+
format: TraitRefFormat,
17821789
) -> Result<(), HirDisplayError> {
17831790
if f.should_truncate() {
17841791
return write!(f, "{TYPE_HINT_TRUNCATION}");
17851792
}
17861793

1787-
tr.self_type_parameter(Interner).hir_fmt(f)?;
1788-
if use_as {
1789-
write!(f, " as ")?;
1790-
} else {
1791-
write!(f, ": ")?;
1794+
match format {
1795+
TraitRefFormat::SelfAsTrait => {
1796+
tr.self_type_parameter(Interner).hir_fmt(f)?;
1797+
write!(f, " as ")?;
1798+
}
1799+
TraitRefFormat::SelfImplementsTrait => {
1800+
tr.self_type_parameter(Interner).hir_fmt(f)?;
1801+
write!(f, ": ")?;
1802+
}
1803+
TraitRefFormat::OnlyTrait => {}
17921804
}
1805+
17931806
let trait_ = tr.hir_trait_id();
17941807
f.start_location_link(trait_.into());
17951808
write!(f, "{}", f.db.trait_data(trait_).name.display(f.db.upcast(), f.edition()))?;
@@ -1798,9 +1811,14 @@ fn fmt_trait_ref(
17981811
hir_fmt_generics(f, &substs[1..], None, substs[0].ty(Interner))
17991812
}
18001813

1801-
impl HirDisplay for TraitRef {
1814+
pub struct TraitRefDisplayWrapper {
1815+
pub trait_ref: TraitRef,
1816+
pub format: TraitRefFormat,
1817+
}
1818+
1819+
impl HirDisplay for TraitRefDisplayWrapper {
18021820
fn hir_fmt(&self, f: &mut HirFormatter<'_>) -> Result<(), HirDisplayError> {
1803-
fmt_trait_ref(f, self, false)
1821+
fmt_trait_ref(f, &self.trait_ref, self.format)
18041822
}
18051823
}
18061824

@@ -1811,10 +1829,12 @@ impl HirDisplay for WhereClause {
18111829
}
18121830

18131831
match self {
1814-
WhereClause::Implemented(trait_ref) => trait_ref.hir_fmt(f)?,
1832+
WhereClause::Implemented(trait_ref) => {
1833+
fmt_trait_ref(f, trait_ref, TraitRefFormat::SelfImplementsTrait)?;
1834+
}
18151835
WhereClause::AliasEq(AliasEq { alias: AliasTy::Projection(projection_ty), ty }) => {
18161836
write!(f, "<")?;
1817-
fmt_trait_ref(f, &projection_ty.trait_ref(f.db), true)?;
1837+
fmt_trait_ref(f, &projection_ty.trait_ref(f.db), TraitRefFormat::SelfAsTrait)?;
18181838
write!(f, ">::",)?;
18191839
let type_alias = from_assoc_type_id(projection_ty.associated_ty_id);
18201840
f.start_location_link(type_alias.into());

src/tools/rust-analyzer/crates/hir/src/display.rs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use itertools::Itertools;
2222
use crate::{
2323
Adt, AsAssocItem, AssocItem, AssocItemContainer, Const, ConstParam, Enum, ExternCrateDecl,
2424
Field, Function, GenericParam, HasCrate, HasVisibility, Impl, LifetimeParam, Macro, Module,
25-
SelfParam, Static, Struct, Trait, TraitAlias, TupleField, TyBuilder, Type, TypeAlias,
25+
SelfParam, Static, Struct, Trait, TraitAlias, TraitRef, TupleField, TyBuilder, Type, TypeAlias,
2626
TypeOrConstParam, TypeParam, Union, Variant,
2727
};
2828

@@ -743,6 +743,21 @@ impl HirDisplay for Static {
743743
}
744744
}
745745

746+
pub struct TraitRefDisplayWrapper {
747+
pub trait_ref: TraitRef,
748+
pub format: hir_ty::display::TraitRefFormat,
749+
}
750+
751+
impl HirDisplay for TraitRefDisplayWrapper {
752+
fn hir_fmt(&self, f: &mut HirFormatter<'_>) -> Result<(), HirDisplayError> {
753+
hir_ty::display::TraitRefDisplayWrapper {
754+
format: self.format,
755+
trait_ref: self.trait_ref.trait_ref.clone(),
756+
}
757+
.hir_fmt(f)
758+
}
759+
}
760+
746761
impl HirDisplay for Trait {
747762
fn hir_fmt(&self, f: &mut HirFormatter<'_>) -> Result<(), HirDisplayError> {
748763
write_trait_header(self, f)?;

src/tools/rust-analyzer/crates/hir/src/lib.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ use crate::db::{DefDatabase, HirDatabase};
9696
pub use crate::{
9797
attrs::{resolve_doc_path_on, HasAttrs},
9898
diagnostics::*,
99+
display::TraitRefDisplayWrapper,
99100
has_source::HasSource,
100101
semantics::{
101102
PathResolution, Semantics, SemanticsImpl, SemanticsScope, TypeInfo, VisibleTraits,
@@ -148,7 +149,7 @@ pub use {
148149
hir_ty::{
149150
consteval::ConstEvalError,
150151
diagnostics::UnsafetyReason,
151-
display::{ClosureStyle, HirDisplay, HirDisplayError, HirWrite},
152+
display::{ClosureStyle, HirDisplay, HirDisplayError, HirWrite, TraitRefFormat},
152153
dyn_compatibility::{DynCompatibilityViolation, MethodViolationCode},
153154
layout::LayoutError,
154155
mir::{MirEvalError, MirLowerError},

src/tools/rust-analyzer/crates/ide-db/src/defs.rs

Lines changed: 53 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@ use either::Either;
1313
use hir::{
1414
Adt, AsAssocItem, AsExternAssocItem, AssocItem, AttributeTemplate, BuiltinAttr, BuiltinType,
1515
Const, Crate, DefWithBody, DeriveHelper, DocLinkDef, ExternAssocItem, ExternCrateDecl, Field,
16-
Function, GenericParam, GenericSubstitution, HasVisibility, HirDisplay, Impl, InlineAsmOperand,
17-
Label, Local, Macro, Module, ModuleDef, Name, PathResolution, Semantics, Static,
18-
StaticLifetime, Struct, ToolModule, Trait, TraitAlias, TupleField, TypeAlias, Variant,
19-
VariantDef, Visibility,
16+
Function, GenericDef, GenericParam, GenericSubstitution, HasContainer, HasVisibility,
17+
HirDisplay, Impl, InlineAsmOperand, ItemContainer, Label, Local, Macro, Module, ModuleDef,
18+
Name, PathResolution, Semantics, Static, StaticLifetime, Struct, ToolModule, Trait, TraitAlias,
19+
TupleField, TypeAlias, Variant, VariantDef, Visibility,
2020
};
2121
use span::Edition;
2222
use stdx::{format_to, impl_from};
@@ -98,8 +98,30 @@ impl Definition {
9898

9999
pub fn enclosing_definition(&self, db: &RootDatabase) -> Option<Definition> {
100100
match self {
101+
Definition::Macro(it) => Some(it.module(db).into()),
102+
Definition::Module(it) => it.parent(db).map(Definition::Module),
103+
Definition::Field(it) => Some(it.parent_def(db).into()),
104+
Definition::Function(it) => it.container(db).try_into().ok(),
105+
Definition::Adt(it) => Some(it.module(db).into()),
106+
Definition::Const(it) => it.container(db).try_into().ok(),
107+
Definition::Static(it) => it.container(db).try_into().ok(),
108+
Definition::Trait(it) => it.container(db).try_into().ok(),
109+
Definition::TraitAlias(it) => it.container(db).try_into().ok(),
110+
Definition::TypeAlias(it) => it.container(db).try_into().ok(),
111+
Definition::Variant(it) => Some(Adt::Enum(it.parent_enum(db)).into()),
112+
Definition::SelfType(it) => Some(it.module(db).into()),
101113
Definition::Local(it) => it.parent(db).try_into().ok(),
102-
_ => None,
114+
Definition::GenericParam(it) => Some(it.parent().into()),
115+
Definition::Label(it) => it.parent(db).try_into().ok(),
116+
Definition::ExternCrateDecl(it) => it.container(db).try_into().ok(),
117+
Definition::DeriveHelper(it) => Some(it.derive().module(db).into()),
118+
Definition::InlineAsmOperand(it) => it.parent(db).try_into().ok(),
119+
Definition::BuiltinAttr(_)
120+
| Definition::BuiltinType(_)
121+
| Definition::BuiltinLifetime(_)
122+
| Definition::TupleField(_)
123+
| Definition::ToolModule(_)
124+
| Definition::InlineAsmRegOrRegClass(_) => None,
103125
}
104126
}
105127

@@ -932,3 +954,29 @@ impl TryFrom<DefWithBody> for Definition {
932954
}
933955
}
934956
}
957+
958+
impl TryFrom<ItemContainer> for Definition {
959+
type Error = ();
960+
fn try_from(container: ItemContainer) -> Result<Self, Self::Error> {
961+
match container {
962+
ItemContainer::Trait(it) => Ok(it.into()),
963+
ItemContainer::Impl(it) => Ok(it.into()),
964+
ItemContainer::Module(it) => Ok(it.into()),
965+
ItemContainer::ExternBlock() | ItemContainer::Crate(_) => Err(()),
966+
}
967+
}
968+
}
969+
970+
impl From<GenericDef> for Definition {
971+
fn from(def: GenericDef) -> Self {
972+
match def {
973+
GenericDef::Function(it) => it.into(),
974+
GenericDef::Adt(it) => it.into(),
975+
GenericDef::Trait(it) => it.into(),
976+
GenericDef::TraitAlias(it) => it.into(),
977+
GenericDef::TypeAlias(it) => it.into(),
978+
GenericDef::Impl(it) => it.into(),
979+
GenericDef::Const(it) => it.into(),
980+
}
981+
}
982+
}

src/tools/rust-analyzer/crates/ide/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,8 @@ pub use crate::{
9696
join_lines::JoinLinesConfig,
9797
markup::Markup,
9898
moniker::{
99-
MonikerDescriptorKind, MonikerKind, MonikerResult, PackageInformation,
100-
SymbolInformationKind,
99+
Moniker, MonikerDescriptorKind, MonikerIdentifier, MonikerKind, MonikerResult,
100+
PackageInformation, SymbolInformationKind,
101101
},
102102
move_item::Direction,
103103
navigation_target::{NavigationTarget, TryToNav, UpmappingResult},

0 commit comments

Comments
 (0)