Skip to content

Commit 8aad5c5

Browse files
authored
feat: per-statement linting with token-wide error spans (#161)
- sqlLinter now lints each statement via SqlStructureAnalyzer, reporting errors for every broken statement instead of only the first parse failure - SqlStatement carries parse errors so the linter reuses the analyzer's per-statement parse (no double-parsing; cached by content) - Diagnostic spans widen to cover the offending token instead of one char - New SqlLinterConfig options: perStatement (default true) and structureAnalyzer (share an instance to reuse its cache)
1 parent 471028c commit 8aad5c5

6 files changed

Lines changed: 229 additions & 28 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ A CodeMirror extension for SQL linting and visual gutter indicators. Built by an
44

55
## Features
66

7-
-**Real-time validation** - SQL syntax checking as you type with detailed error messages
7+
-**Real-time validation** - Per-statement SQL syntax checking as you type, with detailed error messages for every broken statement
88
- 🎨 **Visual gutter** - Color-coded statement indicators and error highlighting
99
- 💡 **Hover tooltips** - Schema info, keywords, and column details on hover
1010
- 🔮 **CTE autocomplete** - Auto-complete support for CTEs

_typos.toml

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,8 @@
33
extend-ignore-re = ["(#|//)\\s*typos:ignore-next-line\\n.*"]
44

55
[files]
6-
extend-exclude = ["src/data/duckdb-keywords.json"]
6+
extend-exclude = [
7+
"src/data/duckdb-keywords.json",
8+
# Contains intentionally misspelled SQL (e.g. SELCT) to trigger parse errors
9+
"src/sql/__tests__/diagnostics.test.ts",
10+
]

src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
export { cteCompletionSource } from "./sql/cte-completion-source.js";
2-
export { sqlLinter } from "./sql/diagnostics.js";
2+
export { type SqlLinterConfig, sqlLinter } from "./sql/diagnostics.js";
33
export { sqlExtension } from "./sql/extension.js";
44
export type {
55
KeywordTooltipData,

src/sql/__tests__/diagnostics.test.ts

Lines changed: 131 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,22 @@
1-
import { Text } from "@codemirror/state";
1+
import { EditorState, Text } from "@codemirror/state";
22
import type { EditorView } from "@codemirror/view";
33
import { describe, expect, it, vi } from "vitest";
4-
import { sqlLinter } from "../diagnostics.js";
4+
import { exportedForTesting, type SqlLinterConfig, sqlLinter } from "../diagnostics.js";
5+
import { NodeSqlParser } from "../parser.js";
56
import type { SqlParser } from "../types.js";
67

7-
// Mock EditorView
8-
const _createMockView = (content: string) => {
9-
const doc = Text.of(content.split("\n"));
8+
const { createLintSource } = exportedForTesting;
9+
10+
const createMockView = (content: string) => {
1011
return {
11-
state: { doc },
12+
state: EditorState.create({ doc: content }),
1213
} as EditorView;
1314
};
1415

16+
const lint = (content: string, config: SqlLinterConfig = {}) => {
17+
return createLintSource(config)(createMockView(content));
18+
};
19+
1520
describe("sqlLinter", () => {
1621
it("should create a linter extension", () => {
1722
const linter = sqlLinter();
@@ -48,3 +53,123 @@ describe("sqlLinter", () => {
4853
expect(linter).toBeDefined();
4954
});
5055
});
56+
57+
describe("lint source", () => {
58+
it("should return no diagnostics for an empty document", async () => {
59+
expect(await lint("")).toEqual([]);
60+
});
61+
62+
it("should return no diagnostics for a whitespace-only document", async () => {
63+
expect(await lint(" \n\n \t ")).toEqual([]);
64+
});
65+
66+
it("should return no diagnostics for valid statements", async () => {
67+
const diagnostics = await lint(
68+
"SELECT * FROM users;\nINSERT INTO users (name) VALUES ('John');",
69+
);
70+
expect(diagnostics).toEqual([]);
71+
});
72+
73+
it("should report one diagnostic per broken statement", async () => {
74+
const doc = "SELCT * FROM users;\nSELECT * FORM users;\nDELETE FROMM users;";
75+
const diagnostics = await lint(doc);
76+
77+
expect(diagnostics).toHaveLength(3);
78+
79+
const text = Text.of(doc.split("\n"));
80+
expect(text.lineAt(diagnostics[0].from).number).toBe(1);
81+
expect(text.lineAt(diagnostics[1].from).number).toBe(2);
82+
expect(text.lineAt(diagnostics[2].from).number).toBe(3);
83+
for (const diagnostic of diagnostics) {
84+
expect(diagnostic.severity).toBe("error");
85+
expect(diagnostic.source).toBe("sql-parser");
86+
}
87+
});
88+
89+
it("should position diagnostics in a broken statement that follows a valid one", async () => {
90+
const doc = "SELECT * FROM users;\nSELCT * FROM orders;";
91+
const diagnostics = await lint(doc);
92+
93+
expect(diagnostics).toHaveLength(1);
94+
const secondStatementStart = doc.indexOf("SELCT");
95+
expect(diagnostics[0].from).toBeGreaterThanOrEqual(secondStatementStart);
96+
expect(diagnostics[0].to).toBeLessThanOrEqual(doc.length);
97+
});
98+
99+
it("should not split statements on semicolons inside string literals", async () => {
100+
const doc = "SELECT 'a; b' FROM users;\nSELCT * FROM orders;";
101+
const diagnostics = await lint(doc);
102+
103+
expect(diagnostics).toHaveLength(1);
104+
expect(diagnostics[0].from).toBeGreaterThanOrEqual(doc.indexOf("SELCT"));
105+
});
106+
107+
it("should not split statements on semicolons inside comments", async () => {
108+
const doc = "SELECT 1 -- trailing; comment\n;\nSELCT * FROM orders;";
109+
const diagnostics = await lint(doc);
110+
111+
expect(diagnostics).toHaveLength(1);
112+
expect(diagnostics[0].from).toBeGreaterThanOrEqual(doc.indexOf("SELCT"));
113+
});
114+
115+
it("should widen the diagnostic span to cover the offending token", async () => {
116+
// node-sql-parser reports this error at the "users" token
117+
const doc = "SELECT 1 FROMM users;";
118+
const diagnostics = await lint(doc);
119+
120+
expect(diagnostics).toHaveLength(1);
121+
const { from, to } = diagnostics[0];
122+
// The span should cover the whole token, not a single character
123+
expect(doc.slice(from, to)).toBe("users");
124+
});
125+
126+
it("should fall back to a one-character span when the error is not at a token", async () => {
127+
// node-sql-parser reports this error at the ";" character
128+
const doc = "SELECT * FROM users WHERE;";
129+
const diagnostics = await lint(doc);
130+
131+
expect(diagnostics).toHaveLength(1);
132+
expect(diagnostics[0].to).toBe(diagnostics[0].from + 1);
133+
});
134+
135+
it("should handle statements separated on the same line", async () => {
136+
const doc = "SELECT 1; SELCT 2;";
137+
const diagnostics = await lint(doc);
138+
139+
expect(diagnostics).toHaveLength(1);
140+
expect(diagnostics[0].from).toBeGreaterThanOrEqual(doc.indexOf("SELCT"));
141+
});
142+
143+
it("should report a single diagnostic when perStatement is disabled", async () => {
144+
const doc = "SELCT * FROM users;\nSELCT * FROM orders;";
145+
const diagnostics = await lint(doc, { perStatement: false });
146+
147+
expect(diagnostics).toHaveLength(1);
148+
const text = Text.of(doc.split("\n"));
149+
expect(text.lineAt(diagnostics[0].from).number).toBe(1);
150+
});
151+
152+
it("should keep diagnostics within statement bounds", async () => {
153+
const doc = "SELECT * FROM users;\nSELECT FROM WHERE;\nSELECT 2;";
154+
const diagnostics = await lint(doc);
155+
156+
expect(diagnostics.length).toBeGreaterThanOrEqual(1);
157+
const stmtFrom = doc.indexOf("SELECT FROM");
158+
const stmtTo = doc.indexOf("SELECT 2");
159+
for (const diagnostic of diagnostics) {
160+
expect(diagnostic.from).toBeGreaterThanOrEqual(stmtFrom);
161+
expect(diagnostic.to).toBeLessThanOrEqual(stmtTo);
162+
expect(diagnostic.to).toBeGreaterThanOrEqual(diagnostic.from);
163+
}
164+
});
165+
166+
it("should reuse errors from a provided structure analyzer without re-validating", async () => {
167+
const parser = new NodeSqlParser();
168+
const validateSpy = vi.spyOn(parser, "validateSql");
169+
170+
const diagnostics = await lint("SELCT 1;\nSELCT 2;", { parser });
171+
172+
expect(diagnostics).toHaveLength(2);
173+
expect(validateSpy).not.toHaveBeenCalled();
174+
});
175+
});

src/sql/diagnostics.ts

Lines changed: 87 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { type Diagnostic, linter } from "@codemirror/lint";
22
import type { Extension, Text } from "@codemirror/state";
33
import type { EditorView } from "@codemirror/view";
44
import { NodeSqlParser } from "./parser.js";
5+
import { type SqlStatement, SqlStructureAnalyzer } from "./structure-analyzer.js";
56
import type { SqlParseError, SqlParser } from "./types.js";
67

78
const DEFAULT_DELAY = 750;
@@ -14,19 +15,74 @@ export interface SqlLinterConfig {
1415
delay?: number;
1516
/** Custom SQL parser instance to use for validation */
1617
parser?: SqlParser;
18+
/**
19+
* Lint each statement in the document separately so every broken statement
20+
* gets its own diagnostic, instead of stopping at the first parse error (default: true)
21+
*/
22+
perStatement?: boolean;
23+
/**
24+
* Structure analyzer used to split the document into statements.
25+
* Pass a shared instance (e.g. the one used by the gutter) to reuse its cache.
26+
*/
27+
structureAnalyzer?: SqlStructureAnalyzer;
1728
}
1829

1930
/**
20-
* Converts a SQL parse error to a CodeMirror diagnostic
31+
* Extends an error span from `from` to cover the token at that position,
32+
* so the squiggle covers the offending word instead of a single character.
33+
*/
34+
function tokenEndAt(doc: Text, from: number): number {
35+
if (from >= doc.length) {
36+
return doc.length;
37+
}
38+
const line = doc.lineAt(from);
39+
const match = line.text.slice(from - line.from).match(/^[\w"'`.]+/);
40+
return match ? from + match[0].length : Math.min(from + 1, doc.length);
41+
}
42+
43+
/**
44+
* Converts a SQL parse error (relative to the whole document) to a CodeMirror diagnostic
2145
*/
2246
function convertToCodeMirrorDiagnostic(error: SqlParseError, doc: Text): Diagnostic {
23-
const lineStart = doc.line(error.line).from;
24-
const from = lineStart + Math.max(0, error.column - 1);
25-
const to = from + 1;
47+
const line = doc.line(Math.min(error.line, doc.lines));
48+
const from = Math.min(line.from + Math.max(0, error.column - 1), doc.length);
49+
50+
return {
51+
from,
52+
to: tokenEndAt(doc, from),
53+
severity: error.severity,
54+
message: error.message,
55+
source: "sql-parser",
56+
};
57+
}
58+
59+
/**
60+
* Converts a SQL parse error whose line/column are relative to a statement's
61+
* content into a CodeMirror diagnostic positioned in the document.
62+
*
63+
* Error line 1 corresponds to the statement's first line, with columns
64+
* relative to `stmt.from` (the statement content is trimmed, so its first
65+
* character is exactly at `stmt.from`).
66+
*/
67+
function convertStatementErrorToDiagnostic(
68+
error: SqlParseError,
69+
stmt: SqlStatement,
70+
doc: Text,
71+
): Diagnostic {
72+
let from: number;
73+
if (error.line <= 1) {
74+
from = stmt.from + Math.max(0, error.column - 1);
75+
} else {
76+
const line = doc.line(Math.min(stmt.lineFrom + error.line - 1, doc.lines));
77+
from = line.from + Math.max(0, error.column - 1);
78+
}
79+
// Clamp within the statement's range in case the parser's reported
80+
// position drifts (e.g. comments are stripped before parsing)
81+
from = Math.max(stmt.from, Math.min(from, stmt.to));
2682

2783
return {
2884
from,
29-
to,
85+
to: Math.max(from, Math.min(tokenEndAt(doc, from), stmt.to)),
3086
severity: error.severity,
3187
message: error.message,
3288
source: "sql-parser",
@@ -50,23 +106,36 @@ function convertToCodeMirrorDiagnostic(error: SqlParseError, doc: Text): Diagnos
50106
* ```
51107
*/
52108
export function sqlLinter(config: SqlLinterConfig = {}): Extension {
109+
return linter(createLintSource(config), {
110+
delay: config.delay || DEFAULT_DELAY,
111+
});
112+
}
113+
114+
function createLintSource(config: SqlLinterConfig = {}) {
53115
const parser = config.parser || new NodeSqlParser();
116+
const perStatement = config.perStatement !== false;
117+
const analyzer = config.structureAnalyzer || new SqlStructureAnalyzer(parser);
54118

55-
return linter(
56-
async (view: EditorView): Promise<Diagnostic[]> => {
57-
const doc = view.state.doc;
58-
const sql = doc.toString();
119+
return async (view: EditorView): Promise<Diagnostic[]> => {
120+
const doc = view.state.doc;
121+
const sql = doc.toString();
59122

60-
if (!sql.trim()) {
61-
return [];
62-
}
123+
if (!sql.trim()) {
124+
return [];
125+
}
63126

127+
if (!perStatement) {
64128
const errors = await parser.validateSql(sql, { state: view.state });
65-
66129
return errors.map((error) => convertToCodeMirrorDiagnostic(error, doc));
67-
},
68-
{
69-
delay: config.delay || DEFAULT_DELAY,
70-
},
71-
);
130+
}
131+
132+
// Lint each statement independently so errors are reported for all
133+
// broken statements, not just the first one in the document
134+
const statements = await analyzer.analyzeDocument(view.state);
135+
return statements.flatMap((stmt) =>
136+
stmt.errors.map((error) => convertStatementErrorToDiagnostic(error, stmt, doc)),
137+
);
138+
};
72139
}
140+
141+
export const exportedForTesting = { createLintSource };

src/sql/structure-analyzer.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { EditorState } from "@codemirror/state";
2-
import type { SqlParser } from "./types.js";
2+
import type { SqlParseError, SqlParser } from "./types.js";
33

44
/**
55
* Represents a SQL statement with position information
@@ -19,6 +19,8 @@ export interface SqlStatement {
1919
type: "select" | "insert" | "update" | "delete" | "create" | "drop" | "alter" | "use" | "other";
2020
/** Whether this statement is syntactically valid */
2121
isValid: boolean;
22+
/** Parse errors for this statement, with line/column relative to the statement content */
23+
errors: SqlParseError[];
2224
}
2325

2426
/**
@@ -128,6 +130,7 @@ export class SqlStructureAnalyzer {
128130
content: cleanContent,
129131
type,
130132
isValid: parseResult.success,
133+
errors: parseResult.errors ?? [],
131134
});
132135

133136
currentPosition += part.length;

0 commit comments

Comments
 (0)