Skip to content

Feat/lsp fixes#12

Open
sdairs wants to merge 9 commits intoClickHouse:mainfrom
sdairs:feat/lsp-fixes
Open

Feat/lsp fixes#12
sdairs wants to merge 9 commits intoClickHouse:mainfrom
sdairs:feat/lsp-fixes

Conversation

@sdairs
Copy link
Copy Markdown
Contributor

@sdairs sdairs commented Apr 16, 2026

  • Formatter upper-cased identifiers that matched keywords. t.type, AS
    type, system.type were emitted as t.TYPE, AS TYPE, SYSTEM.TYPE — which
    changes the query's meaning or requires backticks that the user didn't
    write. Track an identifier_depth in FormatterContext; BareWords inside
    TableIdentifier / ColumnReference / QualifiedName / the name slot of
    TableAlias/ColumnAlias are never re-cased. Clause-keyword positions are
    unaffected.
  • Dot-access missed when cursor sat at a token boundary. Live-typing
    SELECT t. with FROM adjacent produced SELECT t.|FROM …;
    token_before_offset used token.start <= offset so it picked the next token
    as "prev" and completion fired as a plain select expression. Switched to
    token.end <= offset in token_before_impl and find_ident_before_dot_impl.
  • SELECT clause swallowed adjacent clause keywords. SELECT t.|FROM x was
    parsed as a qualified column t.FROM, destroying the FROM clause and
    blanking the scope. Added select::nth_is_clause_keyword(p, n); guarded
    both the identifier chain in expr_delimited and the postfix Dot branch in
    parse_expression_rec.
  • No columns inside subqueries, no projection through subquery aliases.
    scope_at now uses find_enclosing_statement, so a cursor inside a subquery
    gets the subquery's own scope. Added QueryScope::subquery_refs; when a
    TableAlias follows a SubqueryExpression, we record (alias, columns) with
    columns taken from the subquery's top-level SELECT list (AS-aliases
    preferred, trailing identifier otherwise). Completion surfaces those
    columns directly.
  • JSON(..., SKIP REGEXP 'pattern', ...) flagged a red squiggle. The SKIP
    handler accepted SKIP path and SKIP 'str' but not the three-token SKIP
    REGEXP 'pat' form, so REGEXP was consumed as the path and the string was
    left stray. Match the REGEXP keyword specifically and require a string
    literal after it.
  • CREATE TABLE … CLONE AS source flagged Unexpected token in CREATE
    statement. Added the Clone keyword and a CLONE AS branch in
    parse_create_table that wraps the source as an AsClause and falls through
    to standard ENGINE = … / ORDER BY … handling for optional overrides.
  • Connected server posted "Only SELECT is supported for EXPLAIN query" on every non-SELECT statement. publish_diagnostics validated anything whose source text started with SELECT/WITH/INSERT by running EXPLAIN PLAN, but ClickHouse
    only accepts EXPLAIN for SELECT (and UNION of SELECTs). Every INSERT/CREATE/ALTER/etc. in a connected workspace got a yellow squiggle with a server error the user couldn't act on. Filter on the CST statement kind (SelectStatement
    or UnionClause) instead of text prefixes.
  • Fix clippy test failing to run
  • Fix all clippy warning

sdairs and others added 9 commits April 16, 2026 23:25
emit_token was uppercasing any BareWord whose text matched a keyword,
so a column `t.type`, a qualified name `system.type`, or an alias
`AS type` would come out as `t.TYPE`, `SYSTEM.TYPE`, `AS TYPE` — which
changes semantics or breaks quoting expectations.

Track an identifier_depth in FormatterContext and skip keyword-casing
inside TableIdentifier, ColumnReference, QualifiedName, and the name
portion of TableAlias/ColumnAlias. Clause-level keyword positions are
unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
token_before_offset used token.start <= offset, which picked a token
that merely began at the cursor. For the common LSP autocomplete case
where the user hit `.` and the next character is adjacent, this meant
that token was returned as "prev" and dot-access detection missed.

Switch to token.end <= offset in both token_before_impl and
find_ident_before_dot_impl so only tokens that fully ended before the
cursor count as "before." Covers the alias and qualified-name column
completion paths with end-to-end tests.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`SELECT t.FROM x` (user hit `.` with FROM adjacent) was parsed as a
qualified column `t.FROM`, and the FromClause disappeared — so the
whole scope went empty and column completion fell back to the bare
qualifier. The same happened mid-expression for `.WHERE`, `.GROUP`, etc.

Introduce select::nth_is_clause_keyword and guard both the identifier
chain in expr_delimited and the postfix Dot branch in
parse_expression_rec. A clause keyword after a dot is never a valid
field access, so stop and let the clause parse normally.

Covers the in-flight edit case the user called out as the biggest
SELECT-clause resilience gap.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two related bugs: when the cursor was inside a subquery, scope was
built from the root so the subquery's own FROM clause was ignored
(the SubqueryExpression skip in collect_scope hid it). And when the
cursor was on the outside referencing a subquery alias, the alias
was dropped entirely because no TableIdentifier preceded it, so
completion fell back to the bare qualifier.

- Use find_enclosing_statement in handle_completion / scope_at so the
  innermost statement containing the cursor defines the scope.
- Track subquery projections in QueryScope::subquery_refs; when a
  TableAlias follows a SubqueryExpression, record (alias, columns)
  with columns extracted from the subquery's top-level SELECT list
  (AS-aliases preferred, trailing identifier otherwise).
- Completion surfaces those columns directly and skips the server
  metadata lookup for subquery aliases.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
JSON type parameters support `SKIP <path>` and `SKIP REGEXP '<pat>'`.
The handler only matched two-token forms (SKIP path, SKIP 'str'); the
REGEXP keyword was consumed as if it were the path and the following
string literal was left over, producing a red squiggle on real DDL
that uses `JSON(..., SKIP REGEXP 'utm_.*', ...)`.

Check for the REGEXP keyword specifically and require a string literal
after it. Leaves the bare-path branch unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ClickHouse supports `CREATE TABLE [IF NOT EXISTS] [db.]name [ON CLUSTER c]
CLONE AS [db.]source [ENGINE = ... / ORDER BY ... ]`, which copies the
source table's schema and data into a new table. The grammar didn't know
about CLONE, so the keyword was emitted as an Error and the trailing
table reference survived as a stray AsClause — producing the red
"Unexpected token in CREATE statement" squiggle.

Add a Clone keyword, wrap CLONE AS <source> as an AsClause, and fall
through to the standard table-clause handling so optional ENGINE /
ORDER BY / etc. overrides parse cleanly after.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
EXPLAIN PLAN accepts SELECT (and UNION of SELECTs); on INSERT/CREATE/
ALTER/etc. it returns "Only SELECT is supported for EXPLAIN query"
(code 80). The prior text-prefix check allowed INSERT through, so
every non-SELECT statement in a connected workspace produced a yellow
squiggle with a server error that wasn't the user's problem.

Filter on the CST statement kind instead of text prefixes and drop
INSERT from the accept list.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The CI clippy job failed with "'cargo-clippy' is not installed for the
toolchain '1.87.0'" because rust-toolchain.toml pins 1.87.0 with only
rustfmt listed as a component. The workflow's `components: clippy` on
dtolnay/rust-toolchain@stable was overridden by the toolchain file.

Add clippy to the pinned toolchain so the job actually installs it.
Fix the one deny-level lint that surfaces now that clippy runs:
`0 >= min_bp` is always true for a u8, rewrite as `min_bp == 0`
which matches the author's original intent (engage ternary parsing
only at the outermost precedence level).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Now that clippy actually runs in CI (previous commit installed it), the
warn-level lints were visible. Fix all 35:

- if_same_then_else (20): collapse chained `else if p.at_keyword(X)`
  dispatch blocks that all call `p.advance()` into single ||-joined
  conditions (drop/exists/attach/detach/backup/restore object-kind
  keywords, SHOW LIKE/ILIKE, DESCRIBE/DESC, GROUP BY WITH
  TOTALS/ROLLUP/CUBE, LIMIT OFFSET/comma, KILL QUERY/MUTATION, etc.)
- single_match (4): `match { Variant => ..., _ => {} }` -> `if let`
- while_let_loop (3): `loop { match { .. => break } }` -> `while let`
- collapsible_if (3): merge nested `if !first { if !p.eat(..) {..} }`
- doc_lazy_continuation (2): reword doc comment that started with `>0`
- collapsible_else_if (1): flatten `else { if .. }` in CREATE dispatch
- match_like_matches_macro (1): use `matches!` for bracket pair check
- let_and_return (1): return expression directly

No behavioural change. `cargo clippy --workspace --all-targets` is
clean; all tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant