From a2472d2728021760acbaa16a1bf0fc43bb3b76ab Mon Sep 17 00:00:00 2001 From: Tiago Lauer Date: Sun, 2 Aug 2026 00:52:22 -0300 Subject: [PATCH] fix(plugin): scope diagnostics to the first branch of a set operation select id from users union select id from posts ^^ ambiguous column: id findSources regex-scans every from/join in the whole statement into one scope, and the select-list check then counts the first SELECT's columns against that pooled list. UNION branches have to be column-compatible, so sharing a name is the normal case - this put a squiggle on nearly every UNION, including the shape type-locked in tests/union.test-d.ts, which the core types cleanly. Diagnostics now stop at the first top-level set operator. That matches what the core does (the row shape comes from the first branch) and the plugin's own policy: later branches are not reported on, instead of being reported wrongly. Truncating rather than rescoping keeps every offset before the cut aligned with the source, so no span arithmetic changes. The scan is paren-depth aware, so a union inside a subquery does not cut the statement short. Fixes #294 Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 4 +++ ts-plugin/src/diagnostics.cts | 38 ++++++++++++++++++++++++++++- ts-plugin/tests/diagnostics.test.ts | 33 +++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 684fed8..343217e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ Notable changes to this project, following [Keep a Changelog](https://keepachang ## [Unreleased] +### Fixed + +- The editor plugin stops reporting `ambiguous column` on the branches of a set operation. It pooled every FROM/JOIN in the statement into one scope, so the columns a `union` shares - which is all of them, since branches have to be column-compatible - looked like they came from two tables at once, putting a squiggle on nearly every UNION. Diagnostics now cover the first branch, which is also the branch the core takes the row shape from ([#294](https://github.com/tiagolauer/OwlSQL/issues/294)). + ## [0.2.0] - 2026-07-29 ### Changed diff --git a/ts-plugin/src/diagnostics.cts b/ts-plugin/src/diagnostics.cts index 02733b1..164772e 100644 --- a/ts-plugin/src/diagnostics.cts +++ b/ts-plugin/src/diagnostics.cts @@ -285,6 +285,40 @@ function columnTokenFromEntry(entry: ColumnEntry): { token: string; offset: numb return { token, offset: leadingWhitespace }; } +// Only the first branch of a set operation is diagnosed. The scanner pools +// every FROM/JOIN in the statement into one scope, so both branches of a +// `union` shared a source list and any column name they had in common - which +// is every column, since branches have to be compatible - came back as +// `ambiguous column` (issue #294). The core types a set operation by its first +// branch, and truncating here matches that: later branches are not reported +// on, instead of being reported wrongly. Truncation keeps every offset before +// the cut, so spans stay aligned with the source. +const SET_OPERATOR = /\b(?:union|intersect|except)\b/gi; + +function parenDepthAt(text: string, index: number): number { + let depth = 0; + for (let i = 0; i < index; i += 1) { + const char = text[i]; + if (char === '(') { + depth += 1; + } else if (char === ')') { + depth -= 1; + } + } + return depth; +} + +function firstBranchEnd(stripped: string): number { + SET_OPERATOR.lastIndex = 0; + let match: RegExpExecArray | null; + while ((match = SET_OPERATOR.exec(stripped)) !== null) { + if (parenDepthAt(stripped, match.index) === 0) { + return match.index; + } + } + return stripped.length; +} + function getQueryDiagnostics( typescript: typeof ts, checker: ts.TypeChecker, @@ -298,7 +332,9 @@ function getQueryDiagnostics( // computed against it drift out of alignment with literalStart (a raw // source position) by one character per preceding line break on a CRLF // file. Mirrors the same fix already applied to hover in index.cts. - const text = sourceFile.text.slice(literalStart, literal.getEnd() - 1); + const fullText = sourceFile.text.slice(literalStart, literal.getEnd() - 1); + const branchEnd = firstBranchEnd(stripStringLiterals(fullText).stripped); + const text = fullText.slice(0, branchEnd); const { stripped } = stripStringLiterals(text); // A CTE query's outer statement doesn't start at offset 0 - skip the // WITH-clause prefix (mirroring ParseWithClause in src/cte.ts) so the diff --git a/ts-plugin/tests/diagnostics.test.ts b/ts-plugin/tests/diagnostics.test.ts index 4b516c9..7ba49dd 100644 --- a/ts-plugin/tests/diagnostics.test.ts +++ b/ts-plugin/tests/diagnostics.test.ts @@ -284,4 +284,37 @@ describe('ts-plugin diagnostics: getQueryDiagnostics', () => { diagnosticsFor('select posts.titel from users join posts on users.id = posts.user_id'), ).toEqual([{ message: 'unknown column: titel', text: 'titel' }]); }); + + // Regression for #294: the scanner pooled both branches of a set operation + // into one scope, so a column name they share - which is every column, since + // branches have to be compatible - came back as ambiguous. + describe('set operations', () => { + it('does not report a shared column name across UNION branches', () => { + expect(diagnosticsFor('select id from users union select id from posts')).toEqual([]); + }); + + it('does not report across UNION ALL, INTERSECT or EXCEPT either', () => { + expect(diagnosticsFor('select id from users union all select id from posts')).toEqual([]); + expect(diagnosticsFor('select id from users intersect select id from posts')).toEqual([]); + expect(diagnosticsFor('select id from users except select id from posts')).toEqual([]); + }); + + it('still reports a typo in the first branch', () => { + expect(diagnosticsFor('select nope from users union select id from posts')).toEqual([ + { message: 'unknown column: nope', text: 'nope' }, + ]); + }); + + it('still reports an ambiguous column inside the first branch itself', () => { + expect( + diagnosticsFor('select id from users join posts on users.id = posts.user_id union select id from posts'), + ).toEqual([{ message: 'ambiguous column: id', text: 'id' }]); + }); + + it('does not cut the statement at a union inside a subquery', () => { + expect( + diagnosticsFor('select nope from users where id in (select id from posts union select id from posts)'), + ).toEqual([{ message: 'unknown column: nope', text: 'nope' }]); + }); + }); });