Skip to content

feat(eslint-plugin): squiggle the offending identifier in invalid-query errors#504

Merged
Newbie012 merged 1 commit into
mainfrom
feat/precise-invalid-query-position
Jun 29, 2026
Merged

feat(eslint-plugin): squiggle the offending identifier in invalid-query errors#504
Newbie012 merged 1 commit into
mainfrom
feat/precise-invalid-query-position

Conversation

@Newbie012

@Newbie012 Newbie012 commented Jun 28, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • Bug Fixes
    • Improved SQL invalid-query squiggles to underline only the specific offending identifier token reported by Postgres, rather than the entire embedded SQL fragment, with clearer behavior for Kysely builder chains.
    • Enhanced Postgres error-position parsing to pinpoint the exact token when available, falling back to the existing sourcemap-based location logic when needed.
  • Tests
    • Updated and expanded embedded SQL invalid-case coverage and span assertions to reflect the new, more precise invalidQuery locations.

@changeset-bot

changeset-bot Bot commented Jun 28, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 1136c7b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 17 packages
Name Type
@ts-safeql/eslint-plugin Patch
@ts-safeql-demos/basic-flat-config Patch
@ts-safeql-demos/basic-migrations-raw Patch
@ts-safeql-demos/basic-transform-type Patch
@ts-safeql-demos/basic Patch
@ts-safeql-demos/big-project Patch
@ts-safeql-demos/config-file-flat-config Patch
@ts-safeql-demos/from-config-file Patch
@ts-safeql-demos/multi-connections Patch
@ts-safeql-demos/playground Patch
@ts-safeql-demos/plugin-aws-iam Patch
@ts-safeql-demos/plugin-kysely Patch
@ts-safeql-demos/plugin-slonik Patch
@ts-safeql-demos/postgresjs-custom-types Patch
@ts-safeql-demos/postgresjs-demo Patch
@ts-safeql-demos/vercel-postgres Patch
@ts-safeql/generate Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel

vercel Bot commented Jun 28, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
safeql Ignored Ignored Preview Jun 29, 2026 7:47am

@coderabbitai

coderabbitai Bot commented Jun 28, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

getQueryErrorPosition in check-sql.utils.ts now extracts offending tokens from Postgres error messages with new regex patterns and returns token-sized source ranges instead of underlining the full SQL fragment when a match is found. The plugin test and Kysely integration tests were updated to assert the narrower positions, including additional invalid embedded-SQL cases and a new pet schema in the test setup. A patch changeset was added.

Poem

🐇 I hopped through SQL with a careful nose,
Found the tiny token where the red squiggle goes.
No more giant underlines stretching wide and far,
Just precise little wiggles where the bad bits are.
Hop hop! 🎯

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: narrowing invalid-query squiggles to the offending identifier.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/precise-invalid-query-position

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

⚪️ generate() performance

metric base PR Δ
msPerQueryDistinct (gate) 0.422 0.4212 -0.2%
msPerQueryRepeated 0.0051 0.0053 +3.9%

msPerQueryDistinct -0.2% vs base — within noise.

⚪️ check-sql lint-rule overhead

metric base PR Δ
msTotal 283 303 +7.1%
msRuleOverhead (gate) 158 179 +13.3%

msRuleOverhead +13.3% vs base — within noise.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/eslint-plugin/src/rules/check-sql.utils.ts`:
- Around line 758-768: The nearest-match lookup in check-sql.utils.ts is using
the mapped fragment start instead of the translated Postgres error offset, which
can highlight the wrong repeated token. Update the anchor passed to
findNearestMatchIndex in the errorToken handling path to use the translated
error.position value from the builder-sourcemap flow, and keep the existing
getSourceLocation logic intact. Add a regression test around the relevant
check-sql rule helper to cover repeated identifiers in the same fragment and
verify the squiggle lands on the actual error occurrence.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 77f2e864-7664-47c7-895b-b367b4253d78

📥 Commits

Reviewing files that changed from the base of the PR and between 6b79f06 and 616a009.

📒 Files selected for processing (4)
  • .changeset/precise-invalid-query-position.md
  • packages/eslint-plugin/src/rules/check-sql.utils.ts
  • packages/eslint-plugin/src/rules/check-sql/check-sql.plugins.test.ts
  • packages/plugins/kysely/src/plugin.integration.test.ts

Comment thread packages/eslint-plugin/src/rules/check-sql.utils.ts Outdated
@greptile-apps

greptile-apps Bot commented Jun 28, 2026

Copy link
Copy Markdown

Greptile Summary

This PR improves the diagnostic precision of SafeQL's invalid-query errors by extracting the specific offending identifier from Postgres error messages and narrowing the squiggle to that token, rather than underlining the whole query or embedded sql fragment.

  • Adds extractErrorToken with ERROR_TOKEN_PATTERNS that covers common Postgres error families (column, relation, type, missing FROM-clause entry, function, and existing syntax errors), falling back to the full-range highlight whenever no token is matched.
  • Restricts findNearestMatchIndex's search to the source-map fragment slice ([fragmentStart, fragmentEnd]) so recurring tokens (e.g. pet in builder chains) are found in the right place, not in an earlier .innerJoin(...) argument.
  • Adjusts three test files to assert the new, narrower column ranges and adds four new Kysely invalid cases covering column, function, table, and repeated-identifier scenarios.

Confidence Score: 5/5

Safe to merge — the change is purely additive, all new patterns fall back to the existing full-range highlight when no token is matched or found.

Every new code path has an explicit fallback to the prior behavior, so a bad regex match or a missing token simply leaves the squiggle where it was before. The new extractErrorToken patterns cover the most common Postgres identifier-error families. The fragment-restriction logic in getQueryErrorPosition is arithmetically verified by the test assertions, which pin exact column ranges against a live database. No existing behavior is removed.

No files require special attention.

Important Files Changed

Filename Overview
packages/eslint-plugin/src/rules/check-sql.utils.ts Core logic change: adds extractErrorToken/ERROR_TOKEN_PATTERNS and restricts findNearestMatchIndex search to the matched source-map fragment. Logic is correct; fallback path is preserved.
packages/eslint-plugin/src/rules/check-sql/check-sql.plugins.test.ts Tightens one column assertion for the plugin-sourcemap relation error; the new range [28,42] correctly reflects the 14-char missing_person token inside the string literal.
packages/plugins/kysely/src/plugin.integration.test.ts Extends Kysely builder invalid cases with four new assertions (column, function, table, recurring identifier) and updates two existing ranges to match the precise token squiggle behavior.
.changeset/precise-invalid-query-position.md Correctly categorised as a patch bump with an accurate user-facing description.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[getQueryErrorPosition called\nwith error + tag + sourceCode] --> B{Find matchingSourceMap\nfor error.position}
    B -- found --> C[sourceRange = original.start + offset]
    B -- not found --> D[sourceRange = getSourceRange\nposition, sourceMaps]
    C --> E[extractErrorToken\nerror.message]
    D --> E
    E --> F{Token pattern\nmatched?}
    F -- yes --> G{matchingSourceMap\nexists?}
    F -- no --> K[Return full sourceRange fallback]
    G -- yes --> H[fragmentStart = original.start\nfragmentEnd = start + text.length]
    G -- no --> I[fragmentStart = 0\nfragmentEnd = templateText.length]
    H --> J[findNearestMatchIndex\nsliced fragment, token, offset]
    I --> J
    J -- found --> L[start = fragmentStart + tokenIndex\nReturn precise token location]
    J -- not found --> K
    K --> M[getSourceLocation full sourceRange]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[getQueryErrorPosition called\nwith error + tag + sourceCode] --> B{Find matchingSourceMap\nfor error.position}
    B -- found --> C[sourceRange = original.start + offset]
    B -- not found --> D[sourceRange = getSourceRange\nposition, sourceMaps]
    C --> E[extractErrorToken\nerror.message]
    D --> E
    E --> F{Token pattern\nmatched?}
    F -- yes --> G{matchingSourceMap\nexists?}
    F -- no --> K[Return full sourceRange fallback]
    G -- yes --> H[fragmentStart = original.start\nfragmentEnd = start + text.length]
    G -- no --> I[fragmentStart = 0\nfragmentEnd = templateText.length]
    H --> J[findNearestMatchIndex\nsliced fragment, token, offset]
    I --> J
    J -- found --> L[start = fragmentStart + tokenIndex\nReturn precise token location]
    J -- not found --> K
    K --> M[getSourceLocation full sourceRange]
Loading

Reviews (3): Last reviewed commit: "feat(eslint-plugin): squiggle the offend..." | Re-trigger Greptile

Comment thread packages/eslint-plugin/src/rules/check-sql.utils.ts Outdated
@Newbie012 Newbie012 force-pushed the feat/precise-invalid-query-position branch 3 times, most recently from e49efce to 35d607e Compare June 28, 2026 20:07
@greptile-apps

greptile-apps Bot commented Jun 28, 2026

Copy link
Copy Markdown

Want your agent to iterate on Greptile's feedback? Try greploops.

@Newbie012 Newbie012 force-pushed the feat/precise-invalid-query-position branch from 35d607e to 3116165 Compare June 28, 2026 20:18
…ry errors

Postgres names the bad identifier in many error messages (unknown column —
quoted or qualified-unquoted — table, relation, type, function). Extract it
and locate it in the source so the report lands on that token instead of the
whole query. The search is confined to the failing fragment, so a token that
also appears earlier in a builder chain (e.g. a column named in a `.innerJoin`)
isn't matched ahead of the occurrence that actually failed.

This is most visible for Kysely builder chains, whose sourcemap spans the
entire embedded `sql` fragment, so the error previously underlined the whole
fragment.
@Newbie012 Newbie012 force-pushed the feat/precise-invalid-query-position branch from 3116165 to 1136c7b Compare June 29, 2026 07:47
@Newbie012 Newbie012 merged commit 21e97b3 into main Jun 29, 2026
6 checks passed
@Newbie012 Newbie012 deleted the feat/precise-invalid-query-position branch June 29, 2026 07:57
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