fix(completion): treat a bare in-scope table alias as an alias, not an unknown column (#31)
A bare table alias typed where a column is expected — `… GROUP BY o`,
with `o` aliasing `FROM Orders o` — was a blind spot: completion offered
nothing for `o`, and the hint panel called the in-scope alias an unknown
column (`no such column o on table Orders, ...`).
Completion now offers each FROM source's qualifier (alias-if-present-else
table-name) at a bare sql_expr_ident slot, folded into the column
candidate list; on an exact-qualifier partial the alias source steps
aside so the diagnostic can surface. The bare-reference diagnostic arm
emits a targeted `alias_used_as_column` / `table_used_as_column` hint
("`o` is a table alias — write `o.<column>` ...") after the
projection-alias check, so ORDER-BY alias refs still win and a genuine
unknown column still reports `unknown_column`.
Two guards keep the qualified-form advice correct: SQL only (role
`sql_expr_ident`, so the DSL `expr_column` path keeps `unknown_column`
since the DSL has no `table.column` syntax) and effective-qualifier
match (alias-if-present-else-table, so an aliased source referenced by
its shadowed real name falls through rather than being advised as
`name.<column>`). The diagnostic is a drop-in replacement for
`unknown_column` at the same span/Error severity, so verdict/overlay/hint
paths are unchanged.
ADR-0032 Amendment 3; +10 tests.
This commit is contained in:
claude@clouddev1
@@ -753,6 +753,51 @@ pub fn candidates_at_cursor_with_in_mode(
|
||||
);
|
||||
}
|
||||
|
||||
// Source 1.95: in-scope table aliases (issue #31). At a bare
|
||||
// `sql_expr_ident` slot — one *not* already past a `qualifier .`
|
||||
// (handled by §10.5 column narrowing) — the partial may be a
|
||||
// FROM-source the learner is mid-typing as a qualifier
|
||||
// (`group by o` → `o.<column>`). Offer each binding's *qualifier*:
|
||||
// its alias if it has one, else the table name (an aliased source
|
||||
// must be referenced by its alias, not the raw table name). This
|
||||
// makes aliases Tab-discoverable and — since a non-empty candidate
|
||||
// set overlapping the partial suppresses the under-cursor error
|
||||
// (the `typing_over_diag` path) — keeps the alias from flashing as
|
||||
// a bogus "unknown column" while typing. Mixed into `identifiers`
|
||||
// so it sorts/dedups/colours uniformly with column candidates.
|
||||
let alias_candidates: Vec<String> =
|
||||
if has_sql_expr_slot && prefix_qualifier.is_none() {
|
||||
// Once the partial *exactly* matches an in-scope qualifier,
|
||||
// discoverability is served — the learner has a whole alias
|
||||
// in hand and now needs the "add `.column`" hint
|
||||
// (`diagnostic.alias_used_as_column`), not sibling aliases
|
||||
// that merely share the prefix. Offering them would also let
|
||||
// the `typing_over_diag` path suppress that very hint. So in
|
||||
// the exact-match case we emit no alias candidates and let
|
||||
// the targeted diagnostic surface.
|
||||
let partial_is_exact_alias = resolution_from_scope.iter().any(|b| {
|
||||
let q = b.alias.as_deref().unwrap_or(b.table.as_str());
|
||||
q.eq_ignore_ascii_case(&partial_prefix)
|
||||
});
|
||||
if partial_is_exact_alias {
|
||||
Vec::new()
|
||||
} else {
|
||||
let mut out: Vec<String> = Vec::new();
|
||||
for binding in resolution_from_scope {
|
||||
let qualifier =
|
||||
binding.alias.as_deref().unwrap_or(binding.table.as_str());
|
||||
if matches_prefix(qualifier)
|
||||
&& !out.iter().any(|q| q.eq_ignore_ascii_case(qualifier))
|
||||
{
|
||||
out.push(qualifier.to_string());
|
||||
}
|
||||
}
|
||||
out
|
||||
}
|
||||
} else {
|
||||
Vec::new()
|
||||
};
|
||||
|
||||
// Source 2: schema identifiers — accumulated across every
|
||||
// matching schema-listable `Ident { source }` expectation.
|
||||
// `NewName` / `Types` / `Free` sources don't query the
|
||||
@@ -788,6 +833,10 @@ pub fn candidates_at_cursor_with_in_mode(
|
||||
})
|
||||
.filter(|name| matches_prefix(name))
|
||||
.collect();
|
||||
// Fold in the in-scope alias qualifiers (Source 1.95). They are
|
||||
// already prefix-filtered; dedup against any column of the same
|
||||
// spelling happens via the shared sort/dedup below.
|
||||
identifiers.extend(alias_candidates);
|
||||
identifiers.sort();
|
||||
identifiers.dedup();
|
||||
// If an identifier shares its name with a keyword candidate
|
||||
@@ -1930,6 +1979,67 @@ mod tests {
|
||||
cache
|
||||
}
|
||||
|
||||
fn two_table_alias_cache() -> SchemaCache {
|
||||
use crate::dsl::types::Type;
|
||||
let mut cache = schema_with_table("a", &[("id", Type::Int), ("name", Type::Text)]);
|
||||
cache.tables.push("b".to_string());
|
||||
cache.columns.push("total".to_string());
|
||||
cache.table_columns.insert(
|
||||
"b".to_string(),
|
||||
vec![
|
||||
TableColumn::new("id", Type::Int),
|
||||
TableColumn::new("total", Type::Real),
|
||||
],
|
||||
);
|
||||
cache
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bare_expr_slot_offers_in_scope_aliases() {
|
||||
// Issue #31: at a bare SQL-expression slot (here GROUP BY) the
|
||||
// in-scope FROM aliases are Tab-discoverable, so a learner can
|
||||
// reach `o.<column>` without guessing the alias.
|
||||
let cache = two_table_alias_cache();
|
||||
let input = "select a.id from a o join b z on o.id = z.id group by ";
|
||||
let cs = cands_with(input, input.len(), &cache);
|
||||
assert!(cs.contains(&"o".to_string()), "alias `o` must be offered; got {cs:?}");
|
||||
assert!(cs.contains(&"z".to_string()), "alias `z` must be offered; got {cs:?}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bare_expr_slot_narrows_aliases_by_partial_prefix() {
|
||||
// A partial that prefix-matches several aliases offers each;
|
||||
// an exact match (`o`) is the learner's whole alias — no
|
||||
// sibling-alias noise, so the `alias_used_as_column` hint can
|
||||
// surface instead (issue #31).
|
||||
let cache = two_table_alias_cache();
|
||||
let input = "select a.id from a aa join b ab on aa.id = ab.id group by a";
|
||||
let cs = cands_with(input, input.len(), &cache);
|
||||
assert!(cs.contains(&"aa".to_string()), "alias `aa` must be offered; got {cs:?}");
|
||||
assert!(cs.contains(&"ab".to_string()), "alias `ab` must be offered; got {cs:?}");
|
||||
|
||||
// Exact-alias partial: the alias source steps aside.
|
||||
let exact = "select aa.id from a aa join b ab on aa.id = ab.id group by aa";
|
||||
let cs2 = cands_with(exact, exact.len(), &cache);
|
||||
assert!(
|
||||
!cs2.iter().any(|c| c == "ab"),
|
||||
"an exact-alias partial must not surface sibling aliases; got {cs2:?}",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn alias_not_offered_after_a_qualifier_dot() {
|
||||
// Past `o.` the §10.5 column-narrowing owns the slot; aliases
|
||||
// are not candidates there.
|
||||
let cache = two_table_alias_cache();
|
||||
let input = "select a.id from a o join b z on o.id = z.id group by o.";
|
||||
let cs = cands_with(input, input.len(), &cache);
|
||||
assert!(
|
||||
!cs.iter().any(|c| c == "o" || c == "z"),
|
||||
"aliases must not be offered after a qualifier dot; got {cs:?}",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn update_set_offers_only_current_table_columns() {
|
||||
use crate::dsl::types::Type;
|
||||
|
||||
@@ -1163,6 +1163,60 @@ fn schema_existence_diagnostics(
|
||||
// Allowed-clause alias ref — silent.
|
||||
continue;
|
||||
}
|
||||
// Issue #31: the bare ident is itself an
|
||||
// in-scope FROM source — a table alias
|
||||
// (`o` from `FROM Orders o`) or, when the
|
||||
// source is un-aliased, the table name. The
|
||||
// learner means a *column* of that source
|
||||
// (`o.<column>`); calling it an "unknown
|
||||
// column" misleads. Point at the qualified
|
||||
// form.
|
||||
//
|
||||
// Two guards keep the advice correct:
|
||||
// - SQL only (`role == "sql_expr_ident"`):
|
||||
// the DSL `Expr` (role `expr_column`)
|
||||
// has no `table.column` syntax, so the
|
||||
// qualified-form advice would be wrong;
|
||||
// it keeps the generic unknown_column.
|
||||
// - Match the *effective qualifier*
|
||||
// (alias if present, else table name),
|
||||
// not the table name independently. An
|
||||
// aliased source must be referenced by
|
||||
// its alias — `FROM Orders o … Orders`
|
||||
// is invalid SQL, so it must NOT be
|
||||
// advised as `Orders.<column>`. Mirrors
|
||||
// the completion side's qualifier rule.
|
||||
let qualifier_binding = (role
|
||||
== "sql_expr_ident")
|
||||
.then(|| {
|
||||
bindings.iter().find(|b| {
|
||||
let q = b
|
||||
.alias
|
||||
.as_deref()
|
||||
.unwrap_or(b.table.as_str());
|
||||
q.eq_ignore_ascii_case(&item.text)
|
||||
})
|
||||
})
|
||||
.flatten();
|
||||
if let Some(binding) = qualifier_binding {
|
||||
let key = if binding.alias.is_some() {
|
||||
"diagnostic.alias_used_as_column"
|
||||
} else {
|
||||
"diagnostic.table_used_as_column"
|
||||
};
|
||||
diagnostics.push(Diagnostic {
|
||||
severity: Severity::Error,
|
||||
span: item.span,
|
||||
message: crate::friendly::translate(
|
||||
key,
|
||||
&[(
|
||||
"name",
|
||||
&item.text as &dyn std::fmt::Display,
|
||||
)],
|
||||
),
|
||||
});
|
||||
continue;
|
||||
}
|
||||
let table_arg = if bindings.len() == 1 {
|
||||
bindings[0].table.clone()
|
||||
} else {
|
||||
@@ -6330,6 +6384,121 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
// ---- Issue #31 — bare table alias / table used as a column ----
|
||||
|
||||
#[test]
|
||||
fn bare_table_alias_in_group_by_is_alias_hint_not_unknown_column() {
|
||||
// Issue #31: `… GROUP BY o` where `o` aliases a FROM source.
|
||||
// The learner means `o.<column>`; the diagnostic must point
|
||||
// at the qualified form, NOT call `o` an unknown column.
|
||||
let schema = two_table_schema();
|
||||
let diags = diag_keys(
|
||||
"select a.id from a o join b on a.id = b.id group by o",
|
||||
&schema,
|
||||
);
|
||||
assert!(
|
||||
diags
|
||||
.iter()
|
||||
.any(|d| d.contains("`o` is a table alias") && d.contains("o.<column>")),
|
||||
"expected alias_used_as_column hint; got {diags:?}",
|
||||
);
|
||||
assert!(
|
||||
!diags.iter().any(|d| d.contains("no such column")),
|
||||
"unknown_column must not fire for an in-scope alias; got {diags:?}",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bare_table_alias_in_projection_is_alias_hint() {
|
||||
// The same applies outside GROUP BY — a bare alias in the
|
||||
// projection (`SELECT o …`) is equally not a column.
|
||||
let schema = two_table_schema();
|
||||
let diags =
|
||||
diag_keys("select o from a o join b on a.id = b.id", &schema);
|
||||
assert!(
|
||||
diags.iter().any(|d| d.contains("`o` is a table alias")),
|
||||
"expected alias_used_as_column hint in projection; got {diags:?}",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bare_unaliased_table_used_as_column_is_table_hint() {
|
||||
// An un-aliased FROM source referenced bare gets the
|
||||
// table-form hint (qualify with the table name).
|
||||
let schema = two_table_schema();
|
||||
let diags = diag_keys("select id from a group by a", &schema);
|
||||
assert!(
|
||||
diags
|
||||
.iter()
|
||||
.any(|d| d.contains("`a` is a table") && d.contains("a.<column>")),
|
||||
"expected table_used_as_column hint; got {diags:?}",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn genuine_unknown_column_still_reports_no_such_column() {
|
||||
// Regression guard: the alias branch must not swallow a
|
||||
// genuine typo. `nope` matches no alias, no table, no column.
|
||||
let schema = two_table_schema();
|
||||
let diags = diag_keys(
|
||||
"select a.id from a o join b on a.id = b.id group by nope",
|
||||
&schema,
|
||||
);
|
||||
assert!(
|
||||
diags.iter().any(|d| d.contains("no such column") && d.contains("nope")),
|
||||
"a genuine unknown column must still report no such column; got {diags:?}",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn aliased_table_referenced_by_real_name_is_not_table_hint() {
|
||||
// DA guard (issue #31): a source aliased as `x` must be
|
||||
// referenced by the alias — `FROM a x … GROUP BY a` is invalid
|
||||
// SQL, so we must NOT advise `a.<column>`. The branch matches
|
||||
// the *effective qualifier* (the alias when present), so `a`
|
||||
// (the now-shadowed table name) falls through to the generic
|
||||
// unknown_column rather than wrong qualified-form advice.
|
||||
let schema = two_table_schema();
|
||||
let diags = diag_keys(
|
||||
"select x.id from a x join b on x.id = b.id group by a",
|
||||
&schema,
|
||||
);
|
||||
assert!(
|
||||
diags.iter().any(|d| d.contains("no such column") && d.contains("`a`")),
|
||||
"an aliased table referenced by its real name must fall through to \
|
||||
unknown_column; got {diags:?}",
|
||||
);
|
||||
assert!(
|
||||
!diags.iter().any(|d| d.contains("is a table")),
|
||||
"must not advise `a.<column>` when `a` is aliased as `x`; got {diags:?}",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dsl_bare_table_name_in_where_keeps_unknown_column() {
|
||||
// DA guard (issue #31): the alias/table hint is SQL-only
|
||||
// (role `sql_expr_ident`). The DSL `Expr` (role `expr_column`)
|
||||
// has no `table.column` syntax, so advising the qualified form
|
||||
// would be wrong. A DSL bare table-name ref stays the generic
|
||||
// unknown_column it was before issue #31.
|
||||
let schema =
|
||||
schema_with("Customers", &[("id", Type::Int), ("Name", Type::Text)]);
|
||||
for input in [
|
||||
"show data Customers where Customers = 5",
|
||||
"update Customers set Name = 'x' where Customers = 5",
|
||||
] {
|
||||
let diags = diag_keys_simple(input, &schema);
|
||||
assert!(
|
||||
diags.iter().any(|d| d.contains("no such column")),
|
||||
"DSL bare table ref must stay unknown_column for {input:?}; got {diags:?}",
|
||||
);
|
||||
assert!(
|
||||
!diags.iter().any(|d| d.contains("is a table")),
|
||||
"DSL must not get SQL qualified-form advice for {input:?}; got {diags:?}",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// ---- ADR-0032 §11.2 — compound_arity_mismatch ----
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -38,6 +38,7 @@
|
||||
/// `(key, expected_placeholders)`. Sorted by key for grep-ability.
|
||||
pub const KEYS_AND_PLACEHOLDERS: &[(&str, &[&str])] = &[
|
||||
// ---- Pre-submit diagnostics (ADR-0027) ----
|
||||
("diagnostic.alias_used_as_column", &["name"]),
|
||||
("diagnostic.ambiguous_column", &["column", "qualifiers"]),
|
||||
("diagnostic.auto_column_overridden", &["column", "type"]),
|
||||
("diagnostic.compound_arity_mismatch", &["op", "left_n", "right_n"]),
|
||||
@@ -63,6 +64,7 @@ pub const KEYS_AND_PLACEHOLDERS: &[(&str, &[&str])] = &[
|
||||
("diagnostic.not_null_missing", &["column"]),
|
||||
("diagnostic.like_numeric", &["column", "type"]),
|
||||
("diagnostic.projection_alias_misplaced", &["alias", "clause"]),
|
||||
("diagnostic.table_used_as_column", &["name"]),
|
||||
("diagnostic.type_mismatch", &["column", "type"]),
|
||||
("diagnostic.unknown_column", &["name", "table"]),
|
||||
("diagnostic.unknown_qualifier", &["qualifier"]),
|
||||
|
||||
@@ -640,6 +640,12 @@ diagnostic:
|
||||
unknown_qualifier: "no such table or alias in scope: `{qualifier}`"
|
||||
ambiguous_column: "`{column}` is ambiguous — appears in {qualifiers}"
|
||||
projection_alias_misplaced: "alias `{alias}` cannot be used in {clause} — aliases are not bound until after `SELECT`'s projection list"
|
||||
# Issue #31: a bare table alias / table name used where the grammar
|
||||
# expects a column (e.g. `GROUP BY o`). The name *is* in scope — it
|
||||
# is the alias of a FROM source — so calling it an "unknown column"
|
||||
# misleads. Point the learner at the qualified `alias.column` form.
|
||||
alias_used_as_column: "`{name}` is a table alias — write `{name}.<column>` to reference one of its columns"
|
||||
table_used_as_column: "`{name}` is a table — write `{name}.<column>` to reference one of its columns"
|
||||
cte_arity_mismatch: "CTE `{cte}` declares {declared} columns but its body has {actual}"
|
||||
compound_arity_mismatch: "`{op}` requires both sides to have the same number of columns — left has {left_n}, right has {right_n}"
|
||||
duplicate_cte: "duplicate `WITH` table name: `{name}`"
|
||||
|
||||
@@ -1765,6 +1765,65 @@ mod tests {
|
||||
cache
|
||||
}
|
||||
|
||||
fn issue31_join_cache() -> crate::completion::SchemaCache {
|
||||
use crate::completion::{SchemaCache, TableColumn};
|
||||
use crate::dsl::types::Type;
|
||||
let mut cache = SchemaCache::default();
|
||||
let tables: &[(&str, &[(&str, Type)])] = &[
|
||||
("Customers", &[("id", Type::Serial), ("name", Type::Text)]),
|
||||
(
|
||||
"Products",
|
||||
&[("id", Type::Serial), ("name", Type::Text), ("price", Type::Decimal)],
|
||||
),
|
||||
(
|
||||
"OrderLines",
|
||||
&[
|
||||
("id", Type::Serial),
|
||||
("order_id", Type::Int),
|
||||
("product_id", Type::Int),
|
||||
("count", Type::Int),
|
||||
],
|
||||
),
|
||||
(
|
||||
"Orders",
|
||||
&[("id", Type::Serial), ("customer_id", Type::Int), ("date", Type::Date)],
|
||||
),
|
||||
];
|
||||
for (t, cols) in tables {
|
||||
cache.tables.push((*t).to_string());
|
||||
let tc: Vec<TableColumn> =
|
||||
cols.iter().map(|(n, ty)| TableColumn::new(*n, *ty)).collect();
|
||||
for c in &tc {
|
||||
cache.columns.push(c.name.clone());
|
||||
}
|
||||
cache.table_columns.insert((*t).to_string(), tc);
|
||||
}
|
||||
cache
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn issue31_group_by_partial_alias_shows_alias_hint() {
|
||||
// Issue #31 end-to-end: the manual-testing query ended in
|
||||
// `… group by o`, where `o` aliases `Orders`. The ambient
|
||||
// hint must guide the learner to `o.<column>`, not claim
|
||||
// `o` is an unknown column.
|
||||
let cache = issue31_join_cache();
|
||||
let input = "select c.name as customer_name, o.id as order_id, o.date, sum(ol.count*p.price) as total from Orders o join OrderLines ol on o.id=ol.order_id join Products p on p.id=ol.product_id join Customers c on c.id=o.customer_id group by o";
|
||||
match ambient_hint_in_mode(input, input.len(), None, &cache, Mode::Advanced) {
|
||||
Some(AmbientHint::Prose(p)) => {
|
||||
assert!(
|
||||
p.contains("`o` is a table alias") && p.contains("o.<column>"),
|
||||
"expected the alias hint; got: {p:?}",
|
||||
);
|
||||
assert!(
|
||||
!p.contains("no such column"),
|
||||
"must not show the misleading unknown-column message; got: {p:?}",
|
||||
);
|
||||
}
|
||||
other => panic!("expected a Prose alias hint; got: {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ambient_hint_at_insert_first_value_shows_int_prose() {
|
||||
use crate::dsl::types::Type;
|
||||
|
||||
Reference in New Issue
Block a user