Skip to content

Commit

Permalink
Improve SCIP symbols
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mgsloan committed Dec 26, 2024
1 parent 022bece commit b1c091c
Show file tree
Hide file tree
Showing 9 changed files with 522 additions and 270 deletions.
42 changes: 31 additions & 11 deletions src/tools/rust-analyzer/crates/hir-ty/src/display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
">::{}",
Expand Down Expand Up @@ -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()))?;
Expand All @@ -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)
}
}

Expand All @@ -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());
Expand Down
17 changes: 16 additions & 1 deletion src/tools/rust-analyzer/crates/hir/src/display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down Expand Up @@ -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)?;
Expand Down
3 changes: 2 additions & 1 deletion src/tools/rust-analyzer/crates/hir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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},
Expand Down
58 changes: 53 additions & 5 deletions src/tools/rust-analyzer/crates/ide-db/src/defs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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(),
}
}
}
4 changes: 2 additions & 2 deletions src/tools/rust-analyzer/crates/ide/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
Loading

0 comments on commit b1c091c

Please sign in to comment.