Commit e690b8d
authored
feat: schema-aware semantic diagnostics (unknown tables/columns, ambiguous columns) (#163)
## Summary
Linting was syntax-only, even though the extension already accepts a
full `SQLNamespace` schema (used by hover). This adds
**`sqlSemanticLinter`**, a schema-aware linter that reports the
highest-value SQL diagnostics:
- **Unknown table** — `FROM`/`JOIN` references and DML targets
(`INSERT`/`UPDATE`/`DELETE`) resolved against the namespace.
Case-insensitive; qualified paths like `mydb.users` traverse nested
namespaces; CTE names and DDL targets (`CREATE TABLE x` defines `x`) are
skipped.
- **Unknown column** — qualified refs (`u.name`, `users.name`) resolved
through a per-scope alias map; unqualified refs checked only when the
statement reads exactly one known table.
- **Ambiguous column** — unqualified refs that exist in 2+ referenced
tables (suppressed for `USING`/`NATURAL` joins).
Severities are configurable per check (`"error" | "warning" | "off"`,
default `"warning"`), and semantic diagnostics carry `source:
"sql-schema"` vs. `"sql-parser"` for syntax errors.
## False positives are fatal to trust
The analyzer builds proper query scopes from the AST and skips anything
not confidently resolvable, preferring under-reporting:
- checks only run on statements that parse cleanly (no semantic noise on
top of syntax errors)
- CTE outputs, FROM-subquery results, and table functions are opaque —
their columns are never guessed
- unqualified columns are never checked inside correlated subqueries
(they may legally resolve to the outer scope)
- SELECT-list aliases referenced in `ORDER BY`/`GROUP BY` are skipped
- unresolved qualifiers (possible outer-scope aliases) are skipped
- under-qualified names match tables nested deeper in the schema, so
`FROM users` with schema `{mydb: {users}}` never flags
- without a schema (or with an empty placeholder while it lazily loads
upstream) the linter is fully inert — zero parser calls
## Shared schema facet
Also introduces **`sqlSchemaFacet`** so the schema is defined once for
hover + semantic linting (and future completion) instead of per
sub-extension. `sqlExtension` gains top-level `schema`,
`enableSemanticLinting` (default true; inert without a schema), and
`semanticLinterConfig`. Per-extension `schema` config still overrides
for back-compat.
## Implementation note on positions
The plan called for `parseOptions: { includeLocations: true }`, but
node-sql-parser does not attach `loc` to `column_ref`/table nodes even
with it enabled (verified empirically; only some expression wrappers get
it). Diagnostics are instead positioned by word-bounded,
case-insensitive text search within the statement's document range —
searching the original doc text keeps positions exact even around inline
comments (covered by a test).
## Testing
- 24 new tests in `src/sql/__tests__/semantic-diagnostics.test.ts`
covering all checks, CTE/alias/subquery/correlation scoping, nested
namespaces, `self/children` namespaces, Completion-object columns,
severity overrides, facet fallback/precedence, inertness gating, and
diagnostic positioning
- `pnpm test`: 334 passed | 1 expected fail (pre-existing)
- `pnpm run typecheck` and `pnpm exec oxlint`: clean (also fixed the one
pre-existing oxlint warning in `extension.ts`)
- demo updated to exercise semantic linting (`pnpm run demo` builds)
<!-- This is an auto-generated description by cubic. -->
---
## Summary by cubic
Add schema-aware SQL linting that flags unknown tables/columns and
ambiguous columns, shared with hover via a new schema facet. Introduces
`sqlSemanticLinter` and `sqlSchemaFacet`, and adds top-level `schema`
and `enableSemanticLinting` options to `sqlExtension`; also refines
parser/analyzer inheritance to avoid dialect mismatches.
- **New Features**
- `sqlSemanticLinter`: reports unknown tables, unknown columns, and
ambiguous unqualified columns; per-check severity (`error` | `warning` |
`off`, default `warning`); diagnostics use `source: "sql-schema"`; skips
unresolvable cases to avoid false positives.
- `sqlSchemaFacet`: shared schema for hover and semantic lint; supports
function sources; per-feature `schema` still overrides; exported
`resolveSqlSchema`.
- `sqlExtension`: new `schema`, `enableSemanticLinting` (default true;
inert without a schema), and `semanticLinterConfig`; hover falls back to
the shared schema; semantic linter reuses `linterConfig.parser` and
inherits `linterConfig.structureAnalyzer` only when the linter parser is
inherited, preventing analyzer/dialect mismatches.
- **Migration**
- If you already pass a schema to hover, move it to `sqlExtension({
schema })` to share across features.
- To enable semantic linting, supply a schema and optionally set
severities via `semanticLinterConfig.severity`.
- To disable, set `enableSemanticLinting: false`.
<sup>Written for commit fb7ca9e.
Summary will update on new commits.</sup>
<a
href="https://cubic.dev/pr/marimo-team/codemirror-sql/pull/163?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="Review in cubic"
src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a>
<!-- End of auto-generated description by cubic. -->1 parent 575c72e commit e690b8d
11 files changed
Lines changed: 1256 additions & 8 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
| 8 | + | |
8 | 9 | | |
9 | 10 | | |
10 | 11 | | |
| |||
47 | 48 | | |
48 | 49 | | |
49 | 50 | | |
| 51 | + | |
| 52 | + | |
50 | 53 | | |
51 | 54 | | |
52 | 55 | | |
| |||
57 | 60 | | |
58 | 61 | | |
59 | 62 | | |
60 | | - | |
61 | 63 | | |
62 | 64 | | |
63 | 65 | | |
| |||
69 | 71 | | |
70 | 72 | | |
71 | 73 | | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
72 | 108 | | |
73 | 109 | | |
74 | 110 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
| 10 | + | |
| 11 | + | |
10 | 12 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
134 | 134 | | |
135 | 135 | | |
136 | 136 | | |
| 137 | + | |
| 138 | + | |
137 | 139 | | |
138 | 140 | | |
139 | 141 | | |
140 | 142 | | |
141 | 143 | | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
142 | 149 | | |
143 | 150 | | |
144 | 151 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
| 17 | + | |
17 | 18 | | |
18 | 19 | | |
19 | 20 | | |
| 21 | + | |
| 22 | + | |
20 | 23 | | |
21 | 24 | | |
22 | 25 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | | - | |
| 3 | + | |
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
| |||
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
18 | 24 | | |
19 | 25 | | |
20 | 26 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
1 | 2 | | |
2 | 3 | | |
| 4 | + | |
3 | 5 | | |
4 | 6 | | |
5 | 7 | | |
| |||
35 | 37 | | |
36 | 38 | | |
37 | 39 | | |
| 40 | + | |
38 | 41 | | |
39 | 42 | | |
40 | 43 | | |
41 | 44 | | |
42 | 45 | | |
43 | 46 | | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
44 | 71 | | |
45 | 72 | | |
46 | 73 | | |
| |||
0 commit comments