From 05f9f6514077984d28a846842d320c236c64c1c7 Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Tue, 5 May 2026 21:49:47 -0400 Subject: [PATCH] ide: fix infer column name w/ column aliases and goto def Also make the grammar more robust to the `having` clause being in the wrong place. --- crates/squawk_ide/src/goto_definition.rs | 46 ++++++++ crates/squawk_ide/src/resolve.rs | 7 +- crates/squawk_parser/src/grammar.rs | 10 ++ .../squawk_parser/tests/data/err/select.sql | 7 ++ .../tests/snapshots/tests__select_err.snap | 108 ++++++++++++++++-- 5 files changed, 163 insertions(+), 15 deletions(-) diff --git a/crates/squawk_ide/src/goto_definition.rs b/crates/squawk_ide/src/goto_definition.rs index d4b72c5d..45053ad1 100644 --- a/crates/squawk_ide/src/goto_definition.rs +++ b/crates/squawk_ide/src/goto_definition.rs @@ -4771,6 +4771,52 @@ order by a$0; "); } + #[test] + fn goto_select_alias_not_picked_window_order_by() { + assert_snapshot!(goto(" +with t as (select 4 a union select 2 a) +-- should go to the column def, not the alias +select 2 a, a, row_number() over (order by a$0) from t; +"), @" + ╭▸ + 2 │ with t as (select 4 a union select 2 a) + │ ─ 2. destination + 3 │ -- should go to the column def, not the alias + 4 │ select 2 a, a, row_number() over (order by a) from t; + ╰╴ ─ 1. source + "); + } + + #[test] + fn goto_select_alias_group_by_alias_func() { + assert_snapshot!(goto(" +with t as (select 'x'::text as name) +select lower(name) from t +group by lower$0; +"), @" + ╭▸ + 3 │ select lower(name) from t + │ ───── 2. destination + 4 │ group by lower; + ╰╴ ─ 1. source + "); + } + + #[test] + fn goto_select_alias_order_by_alias_func() { + assert_snapshot!(goto(" +with t as (select 'x'::text as name) +select lower(name) from t +order by lower$0; +"), @" + ╭▸ + 3 │ select lower(name) from t + │ ───── 2. destination + 4 │ order by lower; + ╰╴ ─ 1. source + "); + } + #[test] fn goto_select_alias_group_by_column_name_conflict() { // If a GROUP BY expression is a simple name that matches both output diff --git a/crates/squawk_ide/src/resolve.rs b/crates/squawk_ide/src/resolve.rs index 1e08da94..46841b54 100644 --- a/crates/squawk_ide/src/resolve.rs +++ b/crates/squawk_ide/src/resolve.rs @@ -1767,12 +1767,13 @@ fn resolve_select_target_alias_ptr( let column_name = Name::from_node(column_name_ref); let target_list = select.select_clause()?.target_list()?; for target in target_list.targets() { - if let Some(alias_name) = target.as_name().and_then(|a| a.name()) - && Name::from_node(&alias_name) == column_name + if let Some((target_name, node)) = ColumnName::from_target(target) + && let Some(target_name) = target_name.to_string() + && Name::from_string(target_name) == column_name { return Some(smallvec![Location::new( file, - alias_name.syntax().text_range(), + node.text_range(), LocationKind::Column, )]); } diff --git a/crates/squawk_parser/src/grammar.rs b/crates/squawk_parser/src/grammar.rs index 2abdf1e8..50b5a8c5 100644 --- a/crates/squawk_parser/src/grammar.rs +++ b/crates/squawk_parser/src/grammar.rs @@ -2775,6 +2775,7 @@ fn opt_select_trailing_clauses(p: &mut Parser<'_>) -> bool { } opt_order_by_clause(p); + opt_misplaced_having_clause(p); let mut has_locking_clause = false; while p.at(FOR_KW) { if opt_locking_clause(p).is_some() { @@ -3476,6 +3477,15 @@ fn opt_misplaced_joins(p: &mut Parser<'_>) { } } +fn opt_misplaced_having_clause(p: &mut Parser<'_>) { + if p.at(HAVING_KW) { + let m = p.start(); + p.error("HAVING must appear before ORDER BY"); + opt_having_clause(p); + m.complete(p, ERROR); + } +} + fn on_clause(p: &mut Parser<'_>) { assert!(p.at(ON_KW)); let m = p.start(); diff --git a/crates/squawk_parser/tests/data/err/select.sql b/crates/squawk_parser/tests/data/err/select.sql index 9ff57219..30c6c13b 100644 --- a/crates/squawk_parser/tests/data/err/select.sql +++ b/crates/squawk_parser/tests/data/err/select.sql @@ -95,6 +95,13 @@ select * from t where x > 1 join k on true; +-- having after order by instead of before +with t as (select 2 b) +select 1 a from t +group by a +order by a +having a > 10; + -- select end select end; diff --git a/crates/squawk_parser/tests/snapshots/tests__select_err.snap b/crates/squawk_parser/tests/snapshots/tests__select_err.snap index 36405595..d8e3c54c 100644 --- a/crates/squawk_parser/tests/snapshots/tests__select_err.snap +++ b/crates/squawk_parser/tests/snapshots/tests__select_err.snap @@ -983,6 +983,86 @@ SOURCE_FILE TRUE_KW "true" SEMICOLON ";" WHITESPACE "\n\n" + COMMENT "-- having after order by instead of before" + WHITESPACE "\n" + SELECT + WITH_CLAUSE + WITH_KW "with" + WHITESPACE " " + WITH_TABLE + NAME + IDENT "t" + WHITESPACE " " + AS_KW "as" + WHITESPACE " " + L_PAREN "(" + SELECT + SELECT_CLAUSE + SELECT_KW "select" + WHITESPACE " " + TARGET_LIST + TARGET + LITERAL + INT_NUMBER "2" + WHITESPACE " " + AS_NAME + NAME + IDENT "b" + R_PAREN ")" + WHITESPACE "\n" + SELECT_CLAUSE + SELECT_KW "select" + WHITESPACE " " + TARGET_LIST + TARGET + LITERAL + INT_NUMBER "1" + WHITESPACE " " + AS_NAME + NAME + IDENT "a" + WHITESPACE " " + FROM_CLAUSE + FROM_KW "from" + WHITESPACE " " + FROM_ITEM + NAME_REF + IDENT "t" + WHITESPACE "\n" + GROUP_BY_CLAUSE + GROUP_KW "group" + WHITESPACE " " + BY_KW "by" + WHITESPACE " " + GROUP_BY_LIST + GROUPING_EXPR + NAME_REF + IDENT "a" + WHITESPACE "\n" + ORDER_BY_CLAUSE + ORDER_KW "order" + WHITESPACE " " + BY_KW "by" + WHITESPACE " " + SORT_BY_LIST + SORT_BY + NAME_REF + IDENT "a" + WHITESPACE "\n" + ERROR + HAVING_CLAUSE + HAVING_KW "having" + WHITESPACE " " + BIN_EXPR + NAME_REF + IDENT "a" + WHITESPACE " " + R_ANGLE ">" + WHITESPACE " " + LITERAL + INT_NUMBER "10" + SEMICOLON ";" + WHITESPACE "\n\n" COMMENT "-- select end" WHITESPACE "\n" SELECT @@ -1225,31 +1305,35 @@ error[syntax-error]: JOINs must appear before WHERE ╭▸ 96 │ join k on true; ╰╴━ +error[syntax-error]: HAVING must appear before ORDER BY + ╭▸ +103 │ having a > 10; + ╰╴━ error[syntax-error]: expected SEMICOLON - ╭▸ -99 │ select - │ ┏━━━━━━━┛ -100│ ┃ end; - ╰╴┗━┛ + ╭▸ +106 │ select + │ ┏━━━━━━━┛ +107 │ ┃ end; + ╰╴┗━┛ error[syntax-error]: expected SEMICOLON ╭▸ -103 │ select +110 │ select │ ┏━━━━━━━┛ -104 │ ┃ analyze; +111 │ ┃ analyze; ╰╴┗━┛ error[syntax-error]: expected SEMICOLON ╭▸ -106 │ select +113 │ select │ ┏━━━━━━━┛ -107 │ ┃ analyse; +114 │ ┃ analyse; ╰╴┗━┛ error[syntax-error]: expected SEMICOLON ╭▸ -111 │ select +118 │ select │ ┏━━━━━━━┛ -112 │ ┃ with t as (select 1) +119 │ ┃ with t as (select 1) ╰╴┗━┛ error[syntax-error]: unexpected trailing comma ╭▸ -116 │ select 1, +123 │ select 1, ╰╴ ━