From 6a6ee27847b5a82a92f2b94db90cb4c2bdaf84ca Mon Sep 17 00:00:00 2001 From: Gavin Morrow Date: Wed, 8 Jul 2026 21:20:24 -0400 Subject: [PATCH] Remove redundant positions in UnqualifiedImport Co-authored-by: Louis Pilfold --- compiler-core/src/analyse/imports.rs | 12 ++-- compiler-core/src/ast.rs | 26 +++++--- compiler-core/src/build.rs | 61 +++++++++++-------- compiler-core/src/parse.rs | 6 +- ...gleam_core__parse__tests__import_type.snap | 15 +---- language-server/src/engine.rs | 19 +++--- language-server/src/reference.rs | 9 ++- 7 files changed, 80 insertions(+), 68 deletions(-) diff --git a/compiler-core/src/analyse/imports.rs b/compiler-core/src/analyse/imports.rs index b6998217e..368fddccc 100644 --- a/compiler-core/src/analyse/imports.rs +++ b/compiler-core/src/analyse/imports.rs @@ -120,7 +120,7 @@ impl<'context, 'problems> Importer<'context, 'problems> { ); let alias_location = SrcSpan { - start: import.imported_name_location.end, + start: import.name_location().end, end: import.location.end, }; @@ -128,7 +128,7 @@ impl<'context, 'problems> Importer<'context, 'problems> { type_info.module.clone(), import.name.clone(), imported_name, - import.imported_name_location, + import.name_location(), ReferenceKind::Import(alias_location), ); @@ -203,7 +203,7 @@ impl<'context, 'problems> Importer<'context, 'problems> { ); let alias_location = SrcSpan { - start: import.imported_name_location.end, + start: import.name_location().end, end: import.location.end, }; @@ -211,7 +211,7 @@ impl<'context, 'problems> Importer<'context, 'problems> { module.clone(), import_name.clone(), used_name, - import.imported_name_location, + import.name_location(), ReferenceKind::Import(alias_location), ); } @@ -227,7 +227,7 @@ impl<'context, 'problems> Importer<'context, 'problems> { ); let alias_location = SrcSpan { - start: import.imported_name_location.end, + start: import.name_location().end, end: import.location.end, }; @@ -235,7 +235,7 @@ impl<'context, 'problems> Importer<'context, 'problems> { module.clone(), import_name.clone(), used_name, - import.imported_name_location, + import.name_location(), ReferenceKind::Import(alias_location), ); } diff --git a/compiler-core/src/ast.rs b/compiler-core/src/ast.rs index 07aeee5f6..f9212d5e9 100644 --- a/compiler-core/src/ast.rs +++ b/compiler-core/src/ast.rs @@ -1003,7 +1003,7 @@ impl TypedImport { if let Some(UnqualifiedImport { location, - imported_name_location, + name_position, name, as_name, }) = self @@ -1015,9 +1015,8 @@ impl TypedImport { crate::build::UnqualifiedImport { name, module: &self.module, - is_type: false, location, - imported_name_location, + name_position: *name_position, as_name: as_name.as_ref(), }, )); @@ -1025,7 +1024,7 @@ impl TypedImport { if let Some(UnqualifiedImport { location, - imported_name_location, + name_position, name, as_name, }) = self @@ -1037,9 +1036,8 @@ impl TypedImport { crate::build::UnqualifiedImport { name, module: &self.module, - is_type: true, location, - imported_name_location, + name_position: *name_position, as_name: as_name.as_ref(), }, )); @@ -1324,10 +1322,10 @@ impl Definition { #[derive(Debug, Clone, PartialEq, Eq)] pub struct UnqualifiedImport { pub location: SrcSpan, - /// The location excluding the potential `as ...` clause, or the `type` keyword. - /// For example, in `type Wibble as Wobble`, it covers `Wibble`. - pub imported_name_location: SrcSpan, pub name: EcoString, + /// The position of the original name. For example, in `type Wibble as + /// Wobble`, it points to the start of `Wibble`. + pub name_position: u32, pub as_name: Option, } @@ -1335,6 +1333,16 @@ impl UnqualifiedImport { pub fn used_name(&self) -> &EcoString { self.as_name.as_ref().unwrap_or(&self.name) } + + /// The location of the original name, excluding the potential `as ...` + /// clause or the `type` keyword. For example, in `type Wibble as Wobble`, + /// it covers `Wibble`. + pub fn name_location(&self) -> SrcSpan { + SrcSpan::new( + self.name_position, + self.name_position + self.name.len() as u32, + ) + } } #[derive(Debug, Clone, PartialEq, Eq, Copy, Default, serde::Serialize, serde::Deserialize)] diff --git a/compiler-core/src/build.rs b/compiler-core/src/build.rs index 4e4a90a92..c099c63a3 100644 --- a/compiler-core/src/build.rs +++ b/compiler-core/src/build.rs @@ -424,21 +424,30 @@ pub fn module_erlang_name(gleam_name: &EcoString) -> EcoString { pub struct UnqualifiedImport<'a> { pub name: &'a EcoString, pub module: &'a EcoString, - pub is_type: bool, pub location: &'a SrcSpan, - /// The location excluding the potential `as ...` clause, or the `type` keyword. - /// For example, in `type Wibble as Wobble`, it covers `Wibble`. - pub imported_name_location: &'a SrcSpan, + /// The position of the original name. For example, in `type Wibble as + /// Wobble`, it points to the start of `Wibble`. + pub name_position: u32, pub as_name: Option<&'a EcoString>, } impl<'a> UnqualifiedImport<'a> { + /// The location of the original name, excluding the potential `as ...` + /// clause or the `type` keyword. For example, in `type Wibble as Wobble`, + /// it covers `Wibble`. + pub fn name_location(&self) -> SrcSpan { + SrcSpan::new( + self.name_position, + self.name_position + self.name.len() as u32, + ) + } + /// If the import is aliased, it is the start of the alias. Otherwise, it is /// the start of the imported name. /// /// For example, in `type Wibble as Wobble`, the used name will start at /// `Wobble`. In `type Wibble`, it will start at `Wibble`. - pub fn used_name_start(&self) -> u32 { + pub fn used_name_position(&self) -> u32 { match self.as_name { // For aliases, the location span will cover the whole of `type // Wibble as Wobble`. @@ -447,7 +456,7 @@ impl<'a> UnqualifiedImport<'a> { Some(as_name) => self.location.end - as_name.len() as u32, // For non-aliased imports, use the start of the imported name // location. It covers `Wibble` in `type Wibble as Wobble`. - None => self.imported_name_location.start, + None => self.name_position, } } @@ -463,6 +472,13 @@ impl<'a> UnqualifiedImport<'a> { Named::Function } } + + pub fn is_type(&self) -> bool { + // If the position is not at the start location then that means + // that there is a `type` keyword before it, along with an amount of + // whitespace and optional comments + self.name_position != self.location.start + } } /// The position of a located expression. Used to determine extra context, @@ -661,24 +677,21 @@ impl<'a> Located<'a> { module: None, span: record.location, }), - Self::UnqualifiedImport(UnqualifiedImport { - module, - name, - is_type, - .. - }) => importable_modules.get(*module).and_then(|m| { - if *is_type { - m.types.get(*name).map(|t| DefinitionLocation { - module: Some((*module).clone()), - span: t.origin, - }) - } else { - m.values.get(*name).map(|v| DefinitionLocation { - module: Some((*module).clone()), - span: v.definition_location().span, - }) - } - }), + Self::UnqualifiedImport(import @ UnqualifiedImport { module, name, .. }) => { + importable_modules.get(*module).and_then(|m| { + if import.is_type() { + m.types.get(*name).map(|t| DefinitionLocation { + module: Some((*module).clone()), + span: t.origin, + }) + } else { + m.values.get(*name).map(|v| DefinitionLocation { + module: Some((*module).clone()), + span: v.definition_location().span, + }) + } + }) + } Self::Arg(_) => None, Self::Annotation { type_, .. } => self.type_location(importable_modules, type_.clone()), Self::Label { .. } => None, diff --git a/compiler-core/src/parse.rs b/compiler-core/src/parse.rs index 05b628a61..51541dc0a 100644 --- a/compiler-core/src/parse.rs +++ b/compiler-core/src/parse.rs @@ -3122,7 +3122,7 @@ where let mut import = UnqualifiedImport { name, location, - imported_name_location: location, + name_position: location.start, as_name: None, }; if self.maybe_one(&Token::As).is_some() { @@ -3140,7 +3140,7 @@ where let mut import = UnqualifiedImport { name, location, - imported_name_location: location, + name_position: location.start, as_name: None, }; if self.maybe_one(&Token::As).is_some() { @@ -3158,7 +3158,7 @@ where let mut import = UnqualifiedImport { name, location, - imported_name_location: SrcSpan::new(name_start, end), + name_position: name_start, as_name: None, }; if self.maybe_one(&Token::As).is_some() { diff --git a/compiler-core/src/parse/snapshots/gleam_core__parse__tests__import_type.snap b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__import_type.snap index 3e16be0c3..e0f269423 100644 --- a/compiler-core/src/parse/snapshots/gleam_core__parse__tests__import_type.snap +++ b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__import_type.snap @@ -28,11 +28,8 @@ Parsed { start: 28, end: 34, }, - imported_name_location: SrcSpan { - start: 28, - end: 34, - }, name: "Wobble", + name_position: 28, as_name: None, }, ], @@ -42,11 +39,8 @@ Parsed { start: 15, end: 26, }, - imported_name_location: SrcSpan { - start: 20, - end: 26, - }, name: "Wobble", + name_position: 20, as_name: None, }, UnqualifiedImport { @@ -54,11 +48,8 @@ Parsed { start: 36, end: 47, }, - imported_name_location: SrcSpan { - start: 41, - end: 47, - }, name: "Wabble", + name_position: 41, as_name: None, }, ], diff --git a/language-server/src/engine.rs b/language-server/src/engine.rs index a06a4322f..30f04ebfc 100644 --- a/language-server/src/engine.rs +++ b/language-server/src/engine.rs @@ -1318,18 +1318,19 @@ where Located::VariantConstructorDefinition(constructor) => { Some(hover_for_constructor(constructor, lines, module)) } - Located::UnqualifiedImport(UnqualifiedImport { - name, - module: module_name, - is_type, - location, - imported_name_location: _, - as_name: _, - }) => this + Located::UnqualifiedImport( + import @ UnqualifiedImport { + name, + module: module_name, + location, + name_position: _, + as_name: _, + }, + ) => this .compiler .get_module_interface(module_name.as_str()) .and_then(|module_interface| { - if is_type { + if import.is_type() { module_interface.types.get(name).map(|constructor| { hover_for_annotation( *location, diff --git a/language-server/src/reference.rs b/language-server/src/reference.rs index 623360a42..cd4a70021 100644 --- a/language-server/src/reference.rs +++ b/language-server/src/reference.rs @@ -407,18 +407,17 @@ pub fn reference_for_ast_node( import @ UnqualifiedImport { name, module, - is_type, location, - imported_name_location: _, + name_position: _, as_name: _, }, ) => { - if is_type { + if import.is_type() { Some(Referenced::ModuleType { module: module.clone(), name: name.clone(), location: *location, - name_start: import.used_name_start(), + name_start: import.used_name_position(), target_kind: RenameTarget::Unqualified, }) } else { @@ -426,7 +425,7 @@ pub fn reference_for_ast_node( module: module.clone(), name: name.clone(), location: *location, - name_start: import.used_name_start(), + name_start: import.used_name_position(), name_kind: import.name_kind(), target_kind: RenameTarget::Unqualified, }) -- 2.51.2