fix: lint diagnostics offset when statements contain comments (#162)

Parse the original statement text (comments included) instead of the
comment-stripped version. node-sql-parser handles comments natively and
reports positions relative to the original text, so stripping comments
before parsing shifted error lines/columns in the document.


<!-- This is an auto-generated description by cubic. -->
---
## Summary by cubic
Fixes incorrect lint diagnostic positions when SQL statements include
comments by parsing the original text (comments included). This aligns
error lines/columns with the document, leveraging `node-sql-parser`’s
comment-aware positions.

- **Bug Fixes**
- Parse the original statement content in `SqlStructureAnalyzer` and use
stripped content only to detect emptiness and determine statement type.
- Added tests for line comments, multi-line comments, and inline
comments to ensure correct offsets and no false positives for valid
statements.

<sup>Written for commit 6e3b4f5.
Summary will update on new commits.</sup>

<a
href="https://cubic.dev/pr/marimo-team/codemirror-sql/pull/162?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. -->

Author: mscolnick

src/sql/__tests__/diagnostics.test.ts

@@ -163,6 +163,66 @@ describe("lint source", () => {
     }
   });
 
+  it("should position diagnostics correctly when a line comment precedes the statement", async () => {
+    const doc = [
+      "-- Valid queries (no errors):",
+      "SELECT id, name, email",
+      "FROM users",
+      "WHERE active == true",
+      "ORDER BY created_at DESC;",
+    ].join("\n");
+    const diagnostics = await lint(doc);
+
+    expect(diagnostics).toHaveLength(1);
+    const text = Text.of(doc.split("\n"));
+    // The error is at the "==" operator on line 4, not shifted up by the comment
+    expect(text.lineAt(diagnostics[0].from).number).toBe(4);
+    expect(diagnostics[0].from).toBe(doc.indexOf("==") + 1);
+  });
+
+  it("should position diagnostics correctly when a multi-line comment precedes the statement", async () => {
+    const doc = [
+      "/* header",
+      "comment */",
+      "SELECT id",
+      "FROM users",
+      "WHERE active == true;",
+    ].join("\n");
+    const diagnostics = await lint(doc);
+
+    expect(diagnostics).toHaveLength(1);
+    const text = Text.of(doc.split("\n"));
+    expect(text.lineAt(diagnostics[0].from).number).toBe(5);
+    expect(diagnostics[0].from).toBe(doc.indexOf("==") + 1);
+  });
+
+  it("should position diagnostics correctly when a comment appears inside the statement", async () => {
+    const doc = [
+      "SELECT id",
+      "-- pick the table",
+      "FROM users",
+      "WHERE active == true;",
+    ].join("\n");
+    const diagnostics = await lint(doc);
+
+    expect(diagnostics).toHaveLength(1);
+    const text = Text.of(doc.split("\n"));
+    expect(text.lineAt(diagnostics[0].from).number).toBe(4);
+    expect(diagnostics[0].from).toBe(doc.indexOf("==") + 1);
+  });
+
+  it("should not report diagnostics for valid statements with comments", async () => {
+    const doc = [
+      "-- fetch users",
+      "SELECT id /* inline */, name",
+      "FROM users;",
+      "/* another",
+      "   comment */",
+      "SELECT 2;",
+    ].join("\n");
+    expect(await lint(doc)).toEqual([]);
+  });
+
   it("should reuse errors from a provided structure analyzer without re-validating", async () => {
     const parser = new NodeSqlParser();
     const validateSpy = vi.spyOn(parser, "validateSql");

src/sql/diagnostics.ts

@@ -77,7 +77,7 @@ function convertStatementErrorToDiagnostic(
     from = line.from + Math.max(0, error.column - 1);
   }
   // Clamp within the statement's range in case the parser's reported
-  // position drifts (e.g. comments are stripped before parsing)
+  // position drifts
   from = Math.max(stmt.from, Math.min(from, stmt.to));
 
   return {

src/sql/structure-analyzer.ts

@@ -104,7 +104,7 @@ export class SqlStructureAnalyzer {
       const fromLine = state.doc.lineAt(from);
       const toLine = state.doc.lineAt(to);
 
-      // Strip comments from the statement content
+      // Strip comments to detect comment-only parts and determine the type
       const strippedContent = this.stripComments(trimmedPart);
 
       // Skip if the statement is empty after stripping comments
@@ -113,8 +113,9 @@ export class SqlStructureAnalyzer {
         continue;
       }
 
-      // Parse the statement to determine validity and type (use stripped content)
-      const parseResult = await this.parser.parse(strippedContent, { state });
+      // Parse the original content (comments included) so error positions
+      // reported by the parser line up with the document
+      const parseResult = await this.parser.parse(trimmedPart, { state });
       const type = this.determineStatementType(strippedContent);
 
       // Remove trailing semicolon from content for cleaner display