From 95c09bdfcd7cfd12ea3d632745bf10a96f37a492 Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Mon, 17 Aug 2026 11:07:58 -0400 Subject: [PATCH] ide: simplify some name comparisons --- crates/squawk_ide/src/binder.rs | 31 +++++---- crates/squawk_ide/src/classify.rs | 10 ++- .../code_actions/rewrite_select_as_values.rs | 2 +- crates/squawk_ide/src/name.rs | 64 +++++++++++++++++++ crates/squawk_ide/src/resolve.rs | 23 +++---- crates/squawk_ide/src/scope.rs | 5 +- 6 files changed, 99 insertions(+), 36 deletions(-) diff --git a/crates/squawk_ide/src/binder.rs b/crates/squawk_ide/src/binder.rs index fe489b7bd..868504bcf 100644 --- a/crates/squawk_ide/src/binder.rs +++ b/crates/squawk_ide/src/binder.rs @@ -6,7 +6,7 @@ use smallvec::SmallVec; use squawk_syntax::{SyntaxNodePtr, ast, ast::AstNode}; use crate::literals::literal_string_value; -use crate::name::schema_and_func_name; +use crate::name::{AsName, schema_and_func_name}; use crate::scope::Scope; use crate::symbols::{Name, Schema, Symbol, SymbolKind}; @@ -71,7 +71,11 @@ impl Binder { } } - pub(crate) fn lookup(&self, name: &Name, kind: SymbolKind) -> Option { + pub(crate) fn lookup( + &self, + name: &N, + kind: SymbolKind, + ) -> Option { let symbols = self.scope.get(name)?; let symbol_id = symbols.iter().copied().find(|id| { let symbol = &self.symbols[*id]; @@ -112,9 +116,9 @@ impl Binder { list } - pub(crate) fn lookup_with( + pub(crate) fn lookup_with( &self, - name: &Name, + name: &N, kind: SymbolKind, schemas: &ResolvedSchemas, ) -> Option { @@ -130,9 +134,9 @@ impl Binder { None } - pub(crate) fn lookup_with_params( + pub(crate) fn lookup_with_params( &self, - name: &Name, + name: &N, kind: SymbolKind, schemas: &ResolvedSchemas, params: Option<&[Name]>, @@ -155,9 +159,9 @@ impl Binder { None } - pub(crate) fn lookup_with_table( + pub(crate) fn lookup_with_table( &self, - name: &Name, + name: &N, kind: SymbolKind, schemas: &ResolvedSchemas, table: &Option, @@ -176,9 +180,9 @@ impl Binder { None } - pub(crate) fn lookup_info( + pub(crate) fn lookup_info( &self, - name: &Name, + name: &N, kind: SymbolKind, schemas: &ResolvedSchemas, ) -> Option<(Schema, String)> { @@ -189,7 +193,7 @@ impl Binder { symbol.kind == kind && symbol.schema.as_ref() == Some(search_schema) }) { let symbol = &self.symbols[symbol_id]; - return Some((symbol.schema.clone()?, name.to_string())); + return Some((symbol.schema.clone()?, name.as_name().to_string())); } } None @@ -844,12 +848,11 @@ fn multirange_type_from_range( fallback_ptr: SyntaxNodePtr, ) -> Option<(Name, SyntaxNodePtr, Schema)> { if let Some(attribute_list) = range_type.attribute_list() { - let multirange_key = Name::from_string("multirange_type_name"); for option in attribute_list.attribute_options() { let Some(name) = option.name() else { continue; }; - if Name::from_node(&name) != multirange_key { + if Name::from_node(&name) != "multirange_type_name" { continue; } if let Some(attribute_value) = option.attribute_value() { @@ -1905,7 +1908,7 @@ fn bind_select_set_config(b: &mut Binder, select: &ast::Select, position: TextSi let Some((schema, func_name)) = schema_and_func_name(&call_expr) else { return; }; - if func_name != Name::from_string("set_config") { + if func_name != "set_config" { return; } if let Some(schema) = &schema diff --git a/crates/squawk_ide/src/classify.rs b/crates/squawk_ide/src/classify.rs index 595143d76..1ea57f104 100644 --- a/crates/squawk_ide/src/classify.rs +++ b/crates/squawk_ide/src/classify.rs @@ -267,16 +267,14 @@ fn classify_ddl_function_option_value(ty_node: &SyntaxNode) -> Option) -> bool { return false; } path.segment() - .is_some_and(|name_ref| Name::from_node(&name_ref).0.as_str() == "search_path") + .is_some_and(|name_ref| Name::from_node(&name_ref) == "search_path") } fn is_rule_old_new_ref(name_ref: &impl ast::NameLike) -> bool { diff --git a/crates/squawk_ide/src/code_actions/rewrite_select_as_values.rs b/crates/squawk_ide/src/code_actions/rewrite_select_as_values.rs index fe3d6ddc6..f054580aa 100644 --- a/crates/squawk_ide/src/code_actions/rewrite_select_as_values.rs +++ b/crates/squawk_ide/src/code_actions/rewrite_select_as_values.rs @@ -84,7 +84,7 @@ fn is_values_row_column_name(target: &ast::Target, idx: usize) -> bool { return false; }; let expected = format!("column{}", idx + 1); - if Name::from_node(&column_name) != Name::from_string(expected) { + if Name::from_node(&column_name) != expected.as_str() { return false; } true diff --git a/crates/squawk_ide/src/name.rs b/crates/squawk_ide/src/name.rs index 3a4ba6f7e..e0e4ac000 100644 --- a/crates/squawk_ide/src/name.rs +++ b/crates/squawk_ide/src/name.rs @@ -1,5 +1,6 @@ use smol_str::SmolStr; use squawk_syntax::ast::{self, AstNode}; +use std::borrow::Borrow; use std::fmt; #[derive(Clone, Debug, PartialEq, Eq, Hash)] @@ -44,6 +45,62 @@ impl fmt::Display for Name { } } +fn debug_assert_normalized(text: &str) { + debug_assert!( + !text.bytes().any(|b| b.is_ascii_uppercase()), + "un-normalized str used as a Name: {text:?}" + ); +} + +// Allows us to use either Name or &str +pub(crate) trait AsName { + fn as_name(&self) -> &str; +} + +impl AsName for Name { + fn as_name(&self) -> &str { + &self.0 + } +} + +impl AsName for str { + fn as_name(&self) -> &str { + debug_assert_normalized(self); + self + } +} + +impl Borrow for Name { + fn borrow(&self) -> &str { + &self.0 + } +} + +impl PartialEq for Name { + fn eq(&self, other: &str) -> bool { + debug_assert_normalized(other); + self.0 == other + } +} + +impl PartialEq for str { + fn eq(&self, other: &Name) -> bool { + other == self + } +} + +impl PartialEq<&str> for Name { + fn eq(&self, other: &&str) -> bool { + self == *other + } +} + +impl PartialEq for &str { + fn eq(&self, other: &Name) -> bool { + other == *self + } +} + pub(crate) fn schema_and_name_path(path: &ast::PathRef) -> Option<(Option, Name)> { Some((schema_name(path), table_name(path)?)) } @@ -210,4 +267,11 @@ mod test { fn name_quote_comparing() { assert_eq!(Name::from_string(r#""foo""#), Name::from_string("foo")); } + + #[test] + fn name_str_comparing() { + assert_eq!(Name::from_string("FOO"), "foo"); + assert_eq!("foo", Name::from_string(r#""foo""#)); + assert_ne!(Name::from_string(r#""FOO""#), "foo"); + } } diff --git a/crates/squawk_ide/src/resolve.rs b/crates/squawk_ide/src/resolve.rs index 8c162b13b..d546cddec 100644 --- a/crates/squawk_ide/src/resolve.rs +++ b/crates/squawk_ide/src/resolve.rs @@ -10,7 +10,7 @@ use crate::binder::ResolvedSchemas; use crate::db::File; use crate::file::InFile; use crate::location::{Location, LocationKind}; -use crate::name::{self, Name, Schema}; +use crate::name::{self, AsName, Name, Schema}; use crate::symbols::SymbolKind; use crate::{ binder::extract_string_literal, @@ -1091,7 +1091,7 @@ fn fallback_type_alias(type_name: &Name) -> Option { } fn resolve_float_precision(name_ref: &impl ast::NameLike, type_name: Name) -> Name { - if type_name.0.as_str() == "float" + if type_name == "float" && let Some(ast::Expr::Literal(lit)) = name_ref .syntax() .ancestors() @@ -1218,9 +1218,9 @@ fn constraint_owner(name_ref: &impl ast::NameLike) -> Option<(Option, Op Some((fallback_schema, None)) } -fn resolve_for_kind_with_params( +fn resolve_for_kind_with_params( db: &dyn Db, - name: &Name, + name: &N, schemas: &ResolvedSchemas, params: Option<&[Name]>, file: File, @@ -1245,15 +1245,14 @@ fn resolve_special_keyword_as_function( SyntaxKind::SESSION_USER_KW => Some("session_user"), _ => None, })?; - let function_name = Name::from_string(function_name); let position = name_ref.value.syntax().text_range().start(); let schemas = bind(db, name_ref.file_id).resolved_schemas(position, None); - resolve_function(db, &function_name, &schemas, None, name_ref.file_id) + resolve_function(db, function_name, &schemas, None, name_ref.file_id) } -fn resolve_function( +fn resolve_function( db: &dyn Db, - function_name: &Name, + function_name: &N, schemas: &ResolvedSchemas, params: Option<&[Name]>, file: File, @@ -1982,9 +1981,7 @@ fn match_table_in_returning_clause( return Some(ReturningClauseMatch::TableAlias(alias.clone())); } - let old_name = Name::from_string("old"); - let new_name = Name::from_string("new"); - if *table_name == old_name || *table_name == new_name { + if *table_name == "old" || *table_name == "new" { return Some(ReturningClauseMatch::PseudoTable); } @@ -2061,7 +2058,7 @@ fn resolve_select_qualified_column_ptr( } if from_item.alias().and_then(|a| a.columns()).is_none() - && column_name == Name::from_string("ordinality") + && column_name == "ordinality" && let Some(ordinality_token) = from_item .with_ordinality() .and_then(|it| it.ordinality_token()) @@ -2543,7 +2540,7 @@ fn resolve_from_item_column_by_name_after_index( if original_skip == 0 && from_item.alias().and_then(|a| a.columns()).is_none() - && *column_name == Name::from_string("ordinality") + && *column_name == "ordinality" && let Some(ordinality_token) = from_item .with_ordinality() .and_then(|it| it.ordinality_token()) diff --git a/crates/squawk_ide/src/scope.rs b/crates/squawk_ide/src/scope.rs index 31d336a76..f60351770 100644 --- a/crates/squawk_ide/src/scope.rs +++ b/crates/squawk_ide/src/scope.rs @@ -1,5 +1,6 @@ use rustc_hash::FxHashMap; +use crate::name::AsName; use crate::symbols::{Name, SymbolId}; #[derive(Default, Debug, Clone, PartialEq)] @@ -12,7 +13,7 @@ impl Scope { self.entries.entry(name).or_default().push(id); } - pub(crate) fn get(&self, name: &Name) -> Option<&[SymbolId]> { - self.entries.get(name).map(|ids| ids.as_slice()) + pub(crate) fn get(&self, name: &N) -> Option<&[SymbolId]> { + self.entries.get(name.as_name()).map(|ids| ids.as_slice()) } }