diff --git a/Cargo.toml b/Cargo.toml index 30a96322..b0a5ecdb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,5 @@ [workspace] members = ["crates/*"] -default-members = ["crates/squawk"] resolver = "2" [workspace.package] diff --git a/crates/squawk_ide/src/code_actions/test_utils.rs b/crates/squawk_ide/src/code_actions/test_utils.rs index 82846ba0..ca492f38 100644 --- a/crates/squawk_ide/src/code_actions/test_utils.rs +++ b/crates/squawk_ide/src/code_actions/test_utils.rs @@ -4,7 +4,7 @@ use squawk_linter::Edit; use squawk_syntax::ast; use crate::db::{Database, File}; -use crate::test_utils::fixture; +use crate::test_utils::Fixture; use super::{ActionKind, CodeAction}; @@ -12,13 +12,13 @@ pub(super) fn apply_code_action( f: impl Fn(&dyn Db, File, &mut Vec, TextSize) -> Option<()>, sql: &str, ) -> String { - let (mut offset, sql) = fixture(sql); + let fixture = Fixture::new(sql); + let offset = fixture.marker().offset_before(); + let sql = fixture.sql(); let db = Database::default(); - let file = File::new(&db, sql.clone().into()); + let file = File::new(&db, sql.into()); let parse_result = crate::db::parse(&db, file); - offset = offset.checked_sub(1.into()).unwrap_or_default(); - let mut actions = vec![]; f(&db, file, &mut actions, offset); @@ -38,7 +38,7 @@ pub(super) fn apply_code_action( } } - let mut result = sql.clone(); + let mut result = sql.to_string(); let mut edits = action.edits.clone(); edits.sort_by_key(|e| e.text_range.start()); @@ -93,9 +93,11 @@ fn code_action_not_applicable_( sql: &str, allow_errors: bool, ) -> bool { - let (offset, sql) = fixture(sql); + let fixture = Fixture::new(sql); + let offset = fixture.marker().offset(); + let sql = fixture.sql(); let db = Database::default(); - let file = File::new(&db, sql.clone().into()); + let file = File::new(&db, sql.into()); let parse_result = crate::db::parse(&db, file); if !allow_errors { assert_eq!(parse_result.errors(), vec![]); diff --git a/crates/squawk_ide/src/completion.rs b/crates/squawk_ide/src/completion.rs index ca8a307d..4ac81ec1 100644 --- a/crates/squawk_ide/src/completion.rs +++ b/crates/squawk_ide/src/completion.rs @@ -932,13 +932,15 @@ pub struct CompletionItem { mod tests { use super::completion; use crate::db::{Database, File}; - use crate::test_utils::fixture; + use crate::test_utils::Fixture; use insta::assert_snapshot; use tabled::builder::Builder; use tabled::settings::Style; fn completions(sql: &str) -> String { - let (offset, sql) = fixture(sql); + let fixture = Fixture::new(sql); + let offset = fixture.marker().offset(); + let sql = fixture.sql(); let db = Database::default(); let file = File::new(&db, sql.into()); let items = completion(&db, file, offset); @@ -950,7 +952,9 @@ mod tests { } fn completions_not_found(sql: &str) { - let (offset, sql) = fixture(sql); + let fixture = Fixture::new(sql); + let offset = fixture.marker().offset(); + let sql = fixture.sql(); let db = Database::default(); let file = File::new(&db, sql.into()); let items = completion(&db, file, offset); diff --git a/crates/squawk_ide/src/expand_selection.rs b/crates/squawk_ide/src/expand_selection.rs index a1569678..78609e9b 100644 --- a/crates/squawk_ide/src/expand_selection.rs +++ b/crates/squawk_ide/src/expand_selection.rs @@ -290,13 +290,15 @@ fn extend_list_item(node: &SyntaxNode) -> Option { #[cfg(test)] mod tests { use super::*; - use crate::test_utils::fixture; + use crate::test_utils::Fixture; use insta::assert_debug_snapshot; use squawk_syntax::{SourceFile, ast::AstNode}; fn expand(sql: &str) -> Vec { - let (offset, sql) = fixture(sql); - let parse = SourceFile::parse(&sql); + let fixture = Fixture::new(sql); + let offset = fixture.marker().offset(); + let sql = fixture.sql(); + let parse = SourceFile::parse(sql); let file = parse.tree(); let root = file.syntax(); diff --git a/crates/squawk_ide/src/find_references.rs b/crates/squawk_ide/src/find_references.rs index aa37e311..3c4fe307 100644 --- a/crates/squawk_ide/src/find_references.rs +++ b/crates/squawk_ide/src/find_references.rs @@ -41,7 +41,7 @@ mod test { use crate::builtins::builtins_file; use crate::db::{Database, File}; use crate::find_references::find_references; - use crate::test_utils::fixture; + use crate::test_utils::Fixture; use annotate_snippets::{AnnotationKind, Level, Renderer, Snippet, renderer::DecorStyle}; use insta::assert_snapshot; use rowan::TextRange; @@ -49,14 +49,16 @@ mod test { #[track_caller] fn find_refs(sql: &str) -> String { - let (mut offset, sql) = fixture(sql); - offset = offset.checked_sub(1.into()).unwrap_or_default(); + let fixture = Fixture::new(sql); + let marker = fixture.marker(); + let offset = marker.offset_before(); + let query_span = marker.range(); + let sql = fixture.sql(); let db = Database::default(); - let current_file = File::new(&db, sql.clone().into()); + let current_file = File::new(&db, sql.into()); assert_eq!(crate::db::parse(&db, current_file).errors(), vec![]); let references = find_references(&db, current_file, offset); - let offset_usize: usize = offset.into(); let mut file_paths = FxHashMap::default(); file_paths.insert(current_file, "current.sql"); @@ -72,15 +74,11 @@ mod test { let multi_file = refs_by_file.len() > 1 || !refs_by_file.contains_key(¤t_file); - let mut snippet = Snippet::source(&sql).fold(true); + let mut snippet = Snippet::source(sql).fold(true); if multi_file { snippet = snippet.path(*file_paths.get(¤t_file).unwrap()); } - snippet = snippet.annotation( - AnnotationKind::Context - .span(offset_usize..offset_usize + 1) - .label("0. query"), - ); + snippet = snippet.annotation(AnnotationKind::Context.span(query_span).label("0. query")); if let Some(current_refs) = refs_by_file.remove(¤t_file) { snippet = annotate_refs(snippet, current_refs); } diff --git a/crates/squawk_ide/src/goto_definition.rs b/crates/squawk_ide/src/goto_definition.rs index ef225cda..ff3de63c 100644 --- a/crates/squawk_ide/src/goto_definition.rs +++ b/crates/squawk_ide/src/goto_definition.rs @@ -135,7 +135,7 @@ mod test { use crate::builtins::builtins_file; use crate::db::{Database, File}; use crate::goto_definition::goto_definition; - use crate::test_utils::fixture; + use crate::test_utils::Fixture; use annotate_snippets::{AnnotationKind, Level, Renderer, Snippet, renderer::DecorStyle}; use insta::assert_snapshot; use log::info; @@ -150,10 +150,13 @@ mod test { #[track_caller] fn goto_(sql: &str) -> Option { info!("starting"); - let (mut offset, sql) = fixture(sql); + let fixture = Fixture::new(sql); // For go to def we want the previous character since we usually put the // marker after the item we're trying to go to def on. - offset = offset.checked_sub(1.into()).unwrap_or_default(); + let marker = fixture.marker(); + let offset = marker.offset_before(); + let source_span = marker.range(); + let sql = fixture.sql(); let db = Database::default(); let current_file = File::new(&db, sql.into()); assert_eq!(crate::db::parse(&db, current_file).errors(), vec![]); @@ -166,7 +169,6 @@ mod test { file_paths.insert(current_file, "current.sql"); file_paths.insert(builtins_file(&db), "builtins.sql"); - let offset: usize = offset.into(); let mut dests_by_file: FxHashMap> = FxHashMap::default(); for (i, location) in results.iter().enumerate() { dests_by_file @@ -184,11 +186,7 @@ mod test { if let Some(current_dests) = dests_by_file.remove(¤t_file) { snippet = annotate_destinations(snippet, current_dests); } - snippet = snippet.annotation( - AnnotationKind::Context - .span(offset..offset + 1) - .label("1. source"), - ); + snippet = snippet.annotation(AnnotationKind::Context.span(source_span).label("1. source")); let mut groups = vec![Level::INFO.primary_title("definition").element(snippet)]; @@ -983,6 +981,32 @@ select current_timestamp$0 from t; "); } + #[test] + fn goto_cte_casing() { + // postgres only folds ascii characters so Ä doesn't become ä + goto_not_found( + " + with t as (select 1 Äpfel) + select äpfel$0 from t; + ", + ); + } + + #[test] + fn goto_cte_emoji() { + assert_snapshot!(goto( + " + with t as (select 1 🦀) + select 🦀$0 from t; + "), @" + ╭▸ + 2 │ with t as (select 1 🦀) + │ ── 2. destination + 3 │ select 🦀 from t; + ╰╴ ── 1. source + "); + } + #[test] fn goto_current_timestamp_in_where() { assert_snapshot!(goto(" diff --git a/crates/squawk_ide/src/hover.rs b/crates/squawk_ide/src/hover.rs index a03faa80..666e4aab 100644 --- a/crates/squawk_ide/src/hover.rs +++ b/crates/squawk_ide/src/hover.rs @@ -1519,7 +1519,7 @@ fn unqualified_star_in_arg_list_ptrs( mod test { use crate::db::{Database, File}; use crate::hover::hover; - use crate::test_utils::fixture; + use crate::test_utils::Fixture; use annotate_snippets::{AnnotationKind, Level, Renderer, Snippet, renderer::DecorStyle}; use insta::assert_snapshot; @@ -1531,20 +1531,20 @@ mod test { #[track_caller] fn check_hover_(sql: &str) -> Option { let db = Database::default(); - let (mut offset, sql) = fixture(sql); - offset = offset.checked_sub(1.into()).unwrap_or_default(); - let file = File::new(&db, sql.clone().into()); + let fixture = Fixture::new(sql); + let marker = fixture.marker(); + let offset = marker.offset_before(); + let hover_span = marker.range(); + let sql = fixture.sql(); + let file = File::new(&db, sql.into()); assert_eq!(crate::db::parse(&db, file).errors(), vec![]); if let Some(type_info) = hover(&db, file, offset) { - let offset_usize: usize = offset.into(); let title = format!("hover: {}", type_info.snippet); let group = Level::INFO.primary_title(&title).element( - Snippet::source(&sql).fold(true).annotation( - AnnotationKind::Context - .span(offset_usize..offset_usize + 1) - .label("hover"), - ), + Snippet::source(sql) + .fold(true) + .annotation(AnnotationKind::Context.span(hover_span).label("hover")), ); let renderer = Renderer::plain().decor_style(DecorStyle::Unicode); return Some( @@ -1561,8 +1561,9 @@ mod test { #[track_caller] fn check_hover_info(sql: &str) -> super::Hover { let db = Database::default(); - let (mut offset, sql) = fixture(sql); - offset = offset.checked_sub(1.into()).unwrap_or_default(); + let fixture = Fixture::new(sql); + let offset = fixture.marker().offset_before(); + let sql = fixture.sql(); let file = File::new(&db, sql.into()); assert_eq!(crate::db::parse(&db, file).errors(), vec![]); diff --git a/crates/squawk_ide/src/quote.rs b/crates/squawk_ide/src/quote.rs index 95f2c60d..e337d050 100644 --- a/crates/squawk_ide/src/quote.rs +++ b/crates/squawk_ide/src/quote.rs @@ -78,11 +78,11 @@ pub(crate) fn is_reserved_word(text: &str) -> bool { } pub(crate) fn normalize_identifier(text: &str) -> String { - if text.starts_with('"') && text.ends_with('"') && text.len() >= 2 { - text[1..text.len() - 1].to_string() - } else { - text.to_lowercase() - } + // TODO: Cow/SmolStr/Salsa Interned? + text.strip_prefix('"') + .and_then(|t| t.strip_suffix('"')) + .map(|x| x.to_string()) + .unwrap_or_else(|| text.to_ascii_lowercase()) } #[cfg(test)] diff --git a/crates/squawk_ide/src/test_utils.rs b/crates/squawk_ide/src/test_utils.rs index 31feebbb..78430f59 100644 --- a/crates/squawk_ide/src/test_utils.rs +++ b/crates/squawk_ide/src/test_utils.rs @@ -1,12 +1,92 @@ +use std::ops::Range; + use rowan::TextSize; // TODO: we should probably use something else since `$0` is valid syntax, maybe `%0`? const MARKER: &str = "$0"; -#[track_caller] -pub(crate) fn fixture(sql: &str) -> (TextSize, String) { - if let Some(pos) = sql.find(MARKER) { - return (TextSize::new(pos as u32), sql.replace(MARKER, "")); +pub(crate) struct Fixture { + marker_offset: TextSize, + sql: String, +} + +pub(crate) struct Marker { + offset: TextSize, + offset_before: TextSize, + range: Range, +} + +impl Fixture { + #[track_caller] + pub(crate) fn new(sql: &str) -> Self { + if let Some(pos) = sql.find(MARKER) { + return Self { + marker_offset: TextSize::new(pos as u32), + sql: sql.replace(MARKER, ""), + }; + } + panic!("No marker found in SQL. Did you forget to add a marker `$0`?"); + } + + pub(crate) fn marker(&self) -> Marker { + let offset_before = self.offset_before_marker(); + Marker { + offset: self.marker_offset, + offset_before, + range: self.char_span_at(offset_before), + } + } + + fn offset_before_marker(&self) -> TextSize { + let marker_offset: usize = self.marker_offset.into(); + if marker_offset == 0 { + return 0.into(); + } + + TextSize::new( + self.sql[..marker_offset] + .char_indices() + .last() + .map_or(0, |(offset, _)| offset) as u32, + ) + } + + fn char_span_at(&self, offset: TextSize) -> Range { + let start: usize = offset.into(); + let len = self.sql[start..].chars().next().map_or(0, char::len_utf8); + start..start + len + } + + pub(crate) fn sql(&self) -> &str { + &self.sql + } +} + +impl Marker { + pub(crate) fn offset(&self) -> TextSize { + self.offset + } + + pub(crate) fn offset_before(&self) -> TextSize { + self.offset_before + } + + pub(crate) fn range(&self) -> Range { + self.range.clone() + } +} + +#[cfg(test)] +mod tests { + use super::Fixture; + + #[test] + fn marker_tracks_offset_and_offset_before_for_multibyte_chars() { + let fixture = Fixture::new("select 🦀$0"); + let marker = fixture.marker(); + + assert_eq!(usize::from(marker.offset()), "select 🦀".len()); + assert_eq!(marker.offset_before(), ("select ".len() as u32).into()); + assert_eq!(marker.range(), "select ".len().."select 🦀".len()); } - panic!("No marker found in SQL. Did you forget to add a marker `$0`?"); } diff --git a/crates/squawk_lexer/src/lib.rs b/crates/squawk_lexer/src/lib.rs index 24522d8d..7adcd0a3 100644 --- a/crates/squawk_lexer/src/lib.rs +++ b/crates/squawk_lexer/src/lib.rs @@ -6,12 +6,12 @@ pub use token::{Base, LiteralKind, Token, TokenKind}; // via: https://github.com/postgres/postgres/blob/db0c96cc18aec417101e37e59fcc53d4bf647915/src/backend/parser/scan.l#L346 // ident_start [A-Za-z\200-\377_] const fn is_ident_start(c: char) -> bool { - matches!(c, 'a'..='z' | 'A'..='Z' | '_' | '\u{80}'..='\u{FF}') + matches!(c, 'a'..='z' | 'A'..='Z' | '_' | '\u{80}'..) } // ident_cont [A-Za-z\200-\377_0-9\$] const fn is_ident_cont(c: char) -> bool { - matches!(c, 'a'..='z' | 'A'..='Z' | '_' | '0'..='9' | '$' | '\u{80}'..='\u{FF}') + matches!(c, 'a'..='z' | 'A'..='Z' | '_' | '0'..='9' | '$' | '\u{80}'..) } // see: @@ -765,4 +765,19 @@ U&"d!0061t!+000061" UESCAPE '!' ] "#); } + + #[test] + fn ident_non_ascii_above_latin1() { + assert_debug_snapshot!(lex("ẞ Ā 漢字 𐐷"), @r#" + [ + "ẞ" @ Ident, + " " @ Whitespace, + "Ā" @ Ident, + " " @ Whitespace, + "漢字" @ Ident, + " " @ Whitespace, + "𐐷" @ Ident, + ] + "#); + } } diff --git a/crates/squawk_linter/src/lib.rs b/crates/squawk_linter/src/lib.rs index 2fda9d04..e47bb38f 100644 --- a/crates/squawk_linter/src/lib.rs +++ b/crates/squawk_linter/src/lib.rs @@ -310,8 +310,8 @@ impl Violation { } } - fn fix(mut self, fix: Option) -> Violation { - self.fix = fix; + fn fix>>(mut self, fix: F) -> Violation { + self.fix = fix.into(); self } fn help(mut self, help: impl Into) -> Violation { diff --git a/crates/squawk_linter/src/rules/ban_char_field.rs b/crates/squawk_linter/src/rules/ban_char_field.rs index 25425a7f..d8435959 100644 --- a/crates/squawk_linter/src/rules/ban_char_field.rs +++ b/crates/squawk_linter/src/rules/ban_char_field.rs @@ -50,7 +50,7 @@ fn check_path_type(ctx: &mut Linter, path_type: ast::PathType) { Rule::BanCharField, "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.".into(), path_type.syntax(), - ).fix(Some(fix))); + ).fix(fix)); } } @@ -61,7 +61,7 @@ fn check_char_type(ctx: &mut Linter, char_type: ast::CharType) { Rule::BanCharField, "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.".into(), char_type.syntax(), - ).fix(Some(fix))); + ).fix(fix)); } } diff --git a/crates/squawk_linter/src/rules/ban_uncommitted_transaction.rs b/crates/squawk_linter/src/rules/ban_uncommitted_transaction.rs index a717f244..73562d8b 100644 --- a/crates/squawk_linter/src/rules/ban_uncommitted_transaction.rs +++ b/crates/squawk_linter/src/rules/ban_uncommitted_transaction.rs @@ -32,7 +32,7 @@ pub(crate) fn ban_uncommitted_transaction(ctx: &mut Linter, parse: &Parse