Establish complete source spans and diagnostics - #328
Conversation
git-hulk
changed the title
765a403 to
921c028
Compare
Make token-backed AST spans complete half-open byte ranges for strings, placeholders, stars, function calls, query parameters, and set-operation queries. Improve caret rendering for tabs and Unicode while preserving byte-based diagnostic columns. Before applying this PR: Several exported spans excluded delimiters or were empty, including string literals, function calls, stars, placeholders, and query roots with set operations. Caret indentation treated byte offsets as display columns. After this PR: These nodes cover their complete source syntax, query roots contain their set-operation descendants, and diagnostic carets use terminal display width. AST golden files record the intentional span migration. Verification: - make test passed. - make update_test passed. - make lint remains blocked because golangci-lint v1.53.3 rejects the repository configuration format. Migration note: consumers relying on historical string or function end positions must adopt complete half-open byte spans. Assistant-By GPT-5
921c028 to
1b3cc46
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b3cc468b0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| func (f *FunctionExpr) End() Pos { | ||
| return f.Params.RightParenPos | ||
| return f.Params.RightParenPos + 1 |
There was a problem hiding this comment.
For parametric aggregate calls, Params.ColumnArgList contains the second argument list, but this end position still stops after the first list. For example, parsing SELECT quantile(0.9)(x) reports the function and root query ending after quantile(0.9), excluding (x), so source-rewriting consumers receive an incomplete span. Use the trailing ColumnArgList end when it is present.
Useful? React with 👍 / 👎.
| if e.Got != nil && displayWidth(e.Got.String) > width { | ||
| width = displayWidth(e.Got.String) |
There was a problem hiding this comment.
After string token positions were changed to include their quotes, Got.String still contains only the unquoted literal contents. Consequently, an error on 'abc' now starts the caret at the opening quote but emits only three carets instead of covering the five-character token; escaped or doubled quotes produce similarly inaccurate diagnostics. Compute the underline width from the token's source span, including delimiters.
Useful? React with 👍 / 👎.
| func eastAsianWide(r rune) bool { | ||
| return r >= 0x1100 && (r <= 0x115f || r == 0x2329 || r == 0x232a || | ||
| (r >= 0x2e80 && r <= 0xa4cf) || (r >= 0xac00 && r <= 0xd7a3) || | ||
| (r >= 0xf900 && r <= 0xfaff) || (r >= 0xfe10 && r <= 0xfe19) || | ||
| (r >= 0xfe30 && r <= 0xfe6f) || (r >= 0xff00 && r <= 0xff60) || | ||
| (r >= 0xffe0 && r <= 0xffe6)) |
There was a problem hiding this comment.
The new width table stops at BMP ranges, so supplementary characters that occupy two terminal cells—such as CJK Extension B rune 𠀀 and common emoji—fall through to width one. When such a rune appears before a later syntax error, for example in SELECT '𠀀' 123, the rendered caret is shifted one column left. Use a complete terminal-width implementation or include the applicable supplementary wide ranges.
Useful? React with 👍 / 👎.
1 participant
Make token-backed AST spans complete half-open byte ranges for strings,
placeholders, stars, function calls, query parameters, and set-operation
queries. Improve caret rendering for tabs and Unicode while preserving
byte-based diagnostic columns.
Before applying this PR:
Several exported spans excluded delimiters or were empty, including string
literals, function calls, stars, placeholders, and query roots with set
operations. Caret indentation treated byte offsets as display columns.
After this PR:
These nodes cover their complete source syntax, query roots contain their
set-operation descendants, and diagnostic carets use terminal display width.
AST golden files record the intentional span migration.
Verification:
repository configuration format.
Migration note: consumers relying on historical string or function end
positions must adopt complete half-open byte spans.
Assistant-By GPT-5