diff --git a/src/completion.rs b/src/completion.rs index 112337f..3624ee0 100644 --- a/src/completion.rs +++ b/src/completion.rs @@ -263,8 +263,98 @@ pub fn candidates_at_cursor_with_in_mode( // with the active mode so simple-mode users don't see SQL // commands offered. let probe = crate::dsl::walker::completion_probe_in_mode(leading, cache, mode); + + // ADR-0032 §10.6 follow-up — the "edit an existing query" + // workflow: the user goes back inside a projection list to + // type a column, but the FROM clause is already typed + // *after* the cursor. The leading-only walk sees an empty + // from_scope and the column source falls back to the global + // column list — noisy. A second walk on the FULL input + // populates from_scope_stack[0] with the trailing FROM's + // bindings, which we can narrow against. The cost is one + // extra walk on each Tab press; gated on the leading walk + // having produced no from_scope (so the common case where + // the cursor is past FROM pays nothing). + let lookahead_probe = if probe.from_scope.is_empty() + && probe.cte_bindings.is_empty() + && input.len() > leading.len() + { + Some(crate::dsl::walker::completion_probe_in_mode(input, cache, mode)) + } else { + None + }; + + // Resolution scope = leading probe's scope first; fall back + // to the look-ahead probe when the leading walk produced + // nothing. The leading walk has precedence so a cursor + // inside a subquery's projection (where the inner FROM is + // local to that subquery's frame) doesn't get mis-narrowed + // by the outer SELECT's bindings. + let resolution_from_scope: &[crate::dsl::walker::context::TableBinding] = + if !probe.from_scope.is_empty() { + &probe.from_scope + } else { + lookahead_probe + .as_ref() + .map_or(&[][..], |la| &la.from_scope[..]) + }; + let resolution_cte_bindings: &[crate::dsl::walker::context::CteBinding] = + if !probe.cte_bindings.is_empty() { + &probe.cte_bindings + } else { + lookahead_probe + .as_ref() + .map_or(&[][..], |la| &la.cte_bindings[..]) + }; + + // For unqualified column completion, prefer the leading + // walk's `current_table_columns`; fall back to "the union of + // the look-ahead from_scope's bindings' columns" when leading + // produced no in-scope columns. Phase-1 DSL paths unaffected. + let lookahead_union_columns: Vec = + if probe.current_table_columns.is_none() { + let mut out: Vec = Vec::new(); + for binding in resolution_from_scope { + for col in &binding.columns { + if !out.iter().any(|c| { + c.name.eq_ignore_ascii_case(&col.name) + }) { + out.push(col.clone()); + } + } + } + out + } else { + Vec::new() + }; + let lookahead_slice: Option<&[TableColumn]> = if lookahead_union_columns.is_empty() { + None + } else { + Some(lookahead_union_columns.as_slice()) + }; let current_table_columns: Option<&[TableColumn]> = - probe.current_table_columns.as_deref(); + probe.current_table_columns.as_deref().or(lookahead_slice); + + // ADR-0032 §10.5 — qualified-prefix completion. When the + // cursor sits immediately after ` .` (ignoring + // whitespace), narrow `IdentSource::Columns` candidates to + // that qualifier's binding columns alone. The qualifier + // resolves against the active `from_scope` (alias matches + // first, then table names), falling back to `cte_bindings` + // for `cte_alias.|` shapes. Unresolved qualifier → empty + // column list (the structural error path surfaces the + // unresolved-prefix message). + let prefix_qualifier = peek_back_qualifier(input, start); + let qualified_columns: Option> = prefix_qualifier + .as_ref() + .map(|q| { + resolve_qualifier_columns_in( + q, + resolution_from_scope, + resolution_cte_bindings, + cache, + ) + }); let expected = if probe.expected.is_empty() { expected_at(leading, mode) @@ -445,9 +535,14 @@ pub fn candidates_at_cursor_with_in_mode( }) .flat_map(|source| { if source == IdentSource::Columns { - current_table_columns.map_or_else( - || cache.for_source(source).to_vec(), - |cols| cols.iter().map(|c| c.name.clone()).collect(), + qualified_columns.as_deref().map_or_else( + || { + current_table_columns.map_or_else( + || cache.for_source(source).to_vec(), + |cols| cols.iter().map(|c| c.name.clone()).collect(), + ) + }, + <[_]>::to_vec, ) } else { cache.for_source(source).to_vec() @@ -529,6 +624,116 @@ pub fn candidates_at_cursor_with_in_mode( }) } +/// Peek backward from `start` over whitespace and look for a +/// ` .` shape — the qualifier of a qualified reference +/// the cursor is mid-typing past. Returns the qualifier text in +/// case-preserved form, or `None` when no qualifier is present. +/// +/// Identifier characters are ASCII alphanumeric + underscore, +/// matching the partial-prefix recogniser used in +/// `candidates_at_cursor_with_in_mode`. +fn peek_back_qualifier(input: &str, start: usize) -> Option { + let bytes = input.as_bytes(); + // Skip whitespace immediately before the partial. + let mut i = start; + while i > 0 && bytes[i - 1].is_ascii_whitespace() { + i -= 1; + } + // Expect a `.` next. + if i == 0 || bytes[i - 1] != b'.' { + return None; + } + i -= 1; + // Skip whitespace between the `.` and the qualifier ident. + while i > 0 && bytes[i - 1].is_ascii_whitespace() { + i -= 1; + } + // Walk back over identifier characters. + let ident_end = i; + while i > 0 { + let c = bytes[i - 1]; + if c.is_ascii_alphanumeric() || c == b'_' { + i -= 1; + } else { + break; + } + } + if i == ident_end { + return None; + } + Some(input[i..ident_end].to_string()) +} + +/// Resolve a qualifier to a column-name list against the +/// supplied from_scope (alias-then-table match) and cte_bindings +/// (for `cte_alias.|`). An unresolved qualifier returns an empty +/// list per ADR-0032 §10.5. +fn resolve_qualifier_columns_in( + qualifier: &str, + from_scope: &[crate::dsl::walker::context::TableBinding], + cte_bindings: &[crate::dsl::walker::context::CteBinding], + cache: &SchemaCache, +) -> Vec { + // First: alias match in the active from_scope. + if let Some(binding) = from_scope.iter().find(|b| { + b.alias + .as_deref() + .is_some_and(|a| a.eq_ignore_ascii_case(qualifier)) + }) { + if !binding.columns.is_empty() { + return binding.columns.iter().map(|c| c.name.clone()).collect(); + } + if let Some(cte) = cte_bindings + .iter() + .find(|c| c.name.eq_ignore_ascii_case(&binding.table)) + { + return cte + .columns + .iter() + .filter_map(|c| c.name.clone()) + .collect(); + } + } + // Second: table-name match in the active from_scope. + if let Some(binding) = from_scope + .iter() + .find(|b| b.table.eq_ignore_ascii_case(qualifier)) + { + if !binding.columns.is_empty() { + return binding.columns.iter().map(|c| c.name.clone()).collect(); + } + if let Some(cte) = cte_bindings + .iter() + .find(|c| c.name.eq_ignore_ascii_case(&binding.table)) + { + return cte + .columns + .iter() + .filter_map(|c| c.name.clone()) + .collect(); + } + } + // Third: direct cte_bindings match (cte_alias.|). + if let Some(cte) = cte_bindings + .iter() + .find(|c| c.name.eq_ignore_ascii_case(qualifier)) + { + return cte + .columns + .iter() + .filter_map(|c| c.name.clone()) + .collect(); + } + // Fourth: a bare table name from the schema cache — DSL + // paths reach this for `from .` shapes where + // the probe didn't push a from_scope binding (no SQL FROM + // matched). Preserves Phase-1 behaviour for the DSL. + if let Some(cols) = cache.columns_for_table(qualifier) { + return cols.iter().map(|c| c.name.clone()).collect(); + } + Vec::new() +} + /// Detect a value-literal expected-set signature. A value-literal /// slot is the only position where the walker's expected-set /// simultaneously contains all five forms `null` / `true` / @@ -1736,6 +1941,214 @@ mod tests { ); } + // ---- ADR-0032 §10.5 qualified-prefix completion ---- + + fn two_table_schema() -> SchemaCache { + use crate::dsl::types::Type; + let mut s = SchemaCache::default(); + s.tables.push("a".to_string()); + s.tables.push("b".to_string()); + s.columns.push("id".to_string()); + s.columns.push("name".to_string()); + s.columns.push("total".to_string()); + s.table_columns.insert( + "a".to_string(), + vec![ + TableColumn { name: "id".to_string(), user_type: Type::Int }, + TableColumn { name: "name".to_string(), user_type: Type::Text }, + ], + ); + s.table_columns.insert( + "b".to_string(), + vec![ + TableColumn { name: "id".to_string(), user_type: Type::Int }, + TableColumn { name: "total".to_string(), user_type: Type::Real }, + ], + ); + s + } + + #[test] + fn qualified_prefix_narrows_to_table_columns() { + // `select a.|` — candidates should be a's columns + // alone, not the union of a's and b's. + let cache = two_table_schema(); + let input = "select a."; + let cs = cands_with(input, input.len(), &cache); + assert!( + cs.contains(&"id".to_string()) && cs.contains(&"name".to_string()), + "expected a's columns; got {cs:?}", + ); + assert!( + !cs.contains(&"total".to_string()), + "b's `total` must NOT appear under `a.|`; got {cs:?}", + ); + } + + #[test] + fn qualified_prefix_with_partial_filters_prefix() { + // `select a.na` — only `name` (starts with `na`). + let cache = two_table_schema(); + let input = "select a.na"; + let cs = cands_with(input, input.len(), &cache); + assert_eq!(cs, vec!["name".to_string()]); + } + + #[test] + fn qualified_prefix_alias_narrows_through_alias() { + // `select * from a x where x.|` — at the cursor, the + // walker has seen the FROM clause, so `x` is bound to + // `a`. The qualified-prefix resolves through the alias + // and narrows to a's columns. (The mid-projection + // case `select x.| from a x` falls under §10.6's + // projection-before-FROM problem and is handled by + // the post-walk fixup pass, not by this engine.) + let cache = two_table_schema(); + let input = "select * from a x where x."; + let cs = cands_with(input, input.len(), &cache); + assert!( + cs.contains(&"id".to_string()) && cs.contains(&"name".to_string()), + "expected a's columns via alias `x`; got {cs:?}", + ); + assert!( + !cs.contains(&"total".to_string()), + "b's columns must NOT appear under alias `x` of `a`; got {cs:?}", + ); + } + + #[test] + fn qualified_prefix_unresolved_qualifier_no_columns() { + // `select z.|` where `z` resolves to nothing — empty + // column candidate list (the structural error path + // surfaces the unresolved-prefix message). + let cache = two_table_schema(); + let input = "select z."; + let cs = cands_with(input, input.len(), &cache); + // The candidate list should NOT include b's columns + // (the bug we're fixing — global fallback). + assert!( + !cs.contains(&"total".to_string()), + "unknown qualifier must not fall back to global columns; got {cs:?}", + ); + } + + #[test] + fn qualified_prefix_cte_columns_narrow() { + // `with x as (select id, name from a) select x.|` + // should offer `id`, `name` only (x's harvested + // columns). + let cache = two_table_schema(); + let input = "with x as (select id, name from a) select x."; + let cs = cands_with(input, input.len(), &cache); + assert!( + cs.contains(&"id".to_string()) && cs.contains(&"name".to_string()), + "expected x's harvested columns; got {cs:?}", + ); + assert!( + !cs.contains(&"total".to_string()), + "b's `total` must not appear under `x.|`; got {cs:?}", + ); + } + + // ---- Look-ahead probe for the edit scenario ---- + + #[test] + fn lookahead_narrows_unqualified_completion_when_from_follows_cursor() { + // User edits an existing `select X from a` query, cursor + // sits at the start of the partial column name. Leading + // walk sees only `select ` → from_scope is empty. Full + // input `select X from a` parses; look-ahead picks up + // `a`, and the column candidates narrow to a's columns. + let cache = two_table_schema(); + let input = "select X from a"; + // Cursor on the `X` so the partial prefix is `X` and + // look-ahead has a parseable full input. + let cursor = "select ".len(); + let cs = cands_with(input, cursor, &cache); + // The partial `X` is shadowed by a partial-prefix filter + // so the candidates that come back must START with `X` + // (case-insensitive). Neither `id`/`name`/`total` start + // with `X`, so the candidate set is empty rather than + // global. Move the cursor to before the partial to test + // narrowing instead. + assert!( + !cs.contains(&"total".to_string()), + "global fallback must not fire; got {cs:?}", + ); + // Empty-prefix narrowing test: + let input2 = "select n from a"; + let cursor2 = "select ".len(); + let cs2 = cands_with(input2, cursor2, &cache); + assert!( + cs2.contains(&"name".to_string()), + "expected a's `name` via look-ahead; got {cs2:?}", + ); + assert!( + !cs2.contains(&"total".to_string()), + "b's `total` must NOT appear when FROM is `a` only; got {cs2:?}", + ); + } + + #[test] + fn lookahead_qualified_resolves_alias_with_from_after_cursor() { + // `select x.id from a x` — leading walks `select x.` + // (no FROM bound yet). Look-ahead walks the full input; + // x is bound to `a`. The qualified prefix narrows + // candidates to a's columns alone. + let cache = two_table_schema(); + let input = "select x.id from a x"; + let cursor = "select x.".len(); + let cs = cands_with(input, cursor, &cache); + // Partial prefix is `id`, so candidates that match must + // start with `id`. Only `id` itself does — and it does + // appear, meaning the qualifier resolved correctly. + assert!( + cs.contains(&"id".to_string()), + "expected `id` via alias `x` look-ahead; got {cs:?}", + ); + // Empty-prefix variant: cursor right after `x.` (move + // back from `.id` to `.`). + let input2 = "select x. from a x"; + // Doesn't parse cleanly mid-projection — leading walk + // alone produces the right scope ONLY if the full input + // happens to parse. Use a where-clause anchor instead: + let input3 = "select * from a x where x. = 1"; + let cursor3 = "select * from a x where x.".len(); + let cs3 = cands_with(input3, cursor3, &cache); + assert!( + cs3.contains(&"id".to_string()) && cs3.contains(&"name".to_string()), + "expected a's columns under alias `x` in WHERE; got {cs3:?}", + ); + assert!( + !cs3.contains(&"total".to_string()), + "b's columns must NOT appear under alias `x` of `a`; got {cs3:?}", + ); + let _ = input2; + } + + #[test] + fn lookahead_multi_table_from_unions_columns() { + // Two bindings in scope via look-ahead — candidates are + // the union of both, deduplicated. + let cache = two_table_schema(); + let input = "select n from a join b on a.id = b.id"; + let cursor = "select ".len(); + let cs = cands_with(input, cursor, &cache); + assert!(cs.contains(&"name".to_string())); + assert!(cs.contains(&"total".to_string())); + } + + #[test] + fn lookahead_with_partial_prefix_filters_correctly() { + // `select na| from a` — narrowing via look-ahead + + // partial-prefix filter yields just `name`. + let cache = two_table_schema(); + let input = "select na from a"; + let cursor = "select na".len(); + let cs = cands_with(input, cursor, &cache); + assert_eq!(cs, vec!["name".to_string()]); + } + #[test] fn ranker_can_filter_to_empty() { // A ranker that returns an empty list collapses the diff --git a/src/dsl/walker/mod.rs b/src/dsl/walker/mod.rs index 5fe46e9..3b87d83 100644 --- a/src/dsl/walker/mod.rs +++ b/src/dsl/walker/mod.rs @@ -241,6 +241,17 @@ pub struct CompletionProbe { /// WHERE-expression operand, which also accepts a column /// reference — ADR-0026 §8). pub pending_hint_mode: Option, + /// The active `from_scope` at the cursor (top frame on + /// the walker's scope stack). Empty when no FROM has been + /// reached or the walker is schemaless. Used by the + /// completion engine to narrow `cte.|` / `t.|` qualified- + /// prefix candidates to a single binding's columns + /// (ADR-0032 §10.5). + pub from_scope: Vec, + /// CTE bindings visible at the cursor across all in-scope + /// frames (innermost to outermost). The same source the + /// qualified-prefix completion consults for `cte.|` shapes. + pub cte_bindings: Vec, } /// Run a schema-aware walk and report the completion-engine's @@ -282,6 +293,8 @@ pub fn completion_probe_in_mode( expected: mode_filtered_entries(), current_table_columns: None, pending_hint_mode: None, + from_scope: Vec::new(), + cte_bindings: Vec::new(), }; } let mut ctx = context::WalkContext::with_schema(schema); @@ -292,6 +305,8 @@ pub fn completion_probe_in_mode( expected: mode_filtered_entries(), current_table_columns: None, pending_hint_mode: None, + from_scope: Vec::new(), + cte_bindings: Vec::new(), }; }; let expected = match result.outcome { @@ -318,10 +333,35 @@ pub fn completion_probe_in_mode( // position. outcome::WalkOutcome::ValidationFailed { .. } => result.tail_expected, }; + // Snapshot the cursor's lexical scope: top frame's + // from_scope and the union of every frame's cte_bindings + // (innermost first so a shadowing inner CTE wins on name + // collision per ADR-0032 §10.3). + let (from_scope, cte_bindings) = { + let top_from = ctx + .from_scope_stack + .last() + .map(|f| f.from_scope.clone()) + .unwrap_or_default(); + let mut ctes: Vec = Vec::new(); + for frame in ctx.from_scope_stack.iter().rev() { + for binding in &frame.cte_bindings { + if !ctes + .iter() + .any(|c| c.name.eq_ignore_ascii_case(&binding.name)) + { + ctes.push(binding.clone()); + } + } + } + (top_from, ctes) + }; CompletionProbe { expected, current_table_columns: ctx.current_table_columns, pending_hint_mode: ctx.pending_hint_mode, + from_scope, + cte_bindings, } }