Skip to content

fix: lint diagnostics offset when statements contain comments - #162

Merged
mscolnick merged 1 commit into
mainfrom
ms/fix-lint-offset-comments
Jul 23, 2026
Merged

fix: lint diagnostics offset when statements contain comments#162
mscolnick merged 1 commit into
mainfrom
ms/fix-lint-offset-comments

Conversation

@mscolnick

@mscolnick mscolnick commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

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.


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.

Written for commit 6e3b4f5. Summary will update on new commits.

Review in cubic

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.
Copilot AI review requested due to automatic review settings July 23, 2026 14:56

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 93.68% 757 / 808
🔵 Statements 93.67% 785 / 838
🔵 Functions 98.97% 97 / 98
🔵 Branches 83.54% 579 / 693
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
src/sql/diagnostics.ts 100% 100% 100% 100%
src/sql/structure-analyzer.ts 93.12% 94.16% 100% 92.71% 56-58, 97-98, 204-206, 299-301
Generated in workflow #638 for commit 575c72e by the Vitest Coverage Report Action

@mscolnick
mscolnick merged commit 575c72e into main Jul 23, 2026
6 checks passed
@mscolnick
mscolnick deleted the ms/fix-lint-offset-comments branch July 23, 2026 15:16
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.

2 participants