bug: errors raised at end of input carry zero-width ranges #3

New issue

Closed

opened 2026-09-21 15:26:38 +00:00 by barrettruth · 1 comment

barrettruth commented

2026-09-21 15:26:38 +00:00

Owner

Original issue: source #5
Original author: barrettruth
Original date: 2026-08-26T12:01:14Z

Version

0.1.1

Dialect

Dialect::Bazel, reproduced on BUILD and .bzl. Not dialect-specific.

Which guarantee broke

Neither invariant — round-trip holds and nothing panics. This is a diagnostic
quality bug: every error raised at end of input carries an empty range.

Observed

src="foo(\n" len=5
   5..5 "expected `)` to close arguments"          <-- zero width
src="x = [\n" len=6
   6..6 "expected an expression"                   <-- zero width
   6..6 "expected `]`"                             <-- zero width
src="x = {\n" len=6
   6..6 "expected an expression"                   <-- zero width
   6..6 "expected `:` in dict entry (set literals are not Starlark)"
   6..6 "expected `}`"                             <-- zero width
src="def f(:\n    pass\n" len=17
   12..16 "expected an expression"
   17..17 "expected `)` to close parameters"       <-- zero width
   17..17 "expected `:` after def signature"       <-- zero width
   17..17 "expected a statement"                   <-- zero width
src="foo(a\n" len=6
   6..6 "expected `)` to close arguments"          <-- zero width

Every zero-width range is at src.len(). Errors raised at a real token are
fine — 12..16 above is correct, and foo(a, ??? ) style errors are one
character wide.

Why it matters

An unclosed delimiter is not an edge case; it is the state a BUILD file is
in for most of the time anyone is editing it. cc_library( is what exists
between typing the callee and typing the first argument.

Under LSP, a Diagnostic whose range.start == range.end has no characters to
underline. Clients disagree on what to do with it: some render nothing at all,
some fall back to a caret on the following character, some to the whole line.
The most common diagnostic this crate produces is therefore the one most likely
to be invisible.

This is also a known trap that this project already committed to avoiding —
the same defect was catalogued in starpls before this crate was written:

Parser errors get zero-width ranges:
errors_sink(SyntaxError { message, range: TextRange::new(TextSize::new(token_pos), TextSize::new(token_pos)) });
Editors render a 0-character squiggle. (Lexer errors do get real ranges.)

and listed as a required change: "Emit non-zero-width error ranges (currently
TextRange::new(pos, pos))." It has been reproduced.

Precedent

Neither Bazel nor starlark-rust has a good answer for the EOF case
specifically — starlark-rust degenerates to an empty span at 1:1:

--- "foo(\n"
error: Parse error: unexpected new line, expected expression
--> t:1:1
 |
 |

But Bazel does have the right idea for unclosed constructs generally, and
applies it consistently to strings: the diagnostic is anchored on the token
that opened the thing that was never closed, not on the point where the parser
noticed.

// Lexer.java:287 — literalStartPos is the opening quote, not the truncation point
error("unclosed string literal", literalStartPos);

// LexerTest.java:682 — caret on the opening quote
checkErrors("+ 'unterminated", "PLUS STRING(unterminated) NEWLINE EOF",
            "  ^ unclosed string literal");

That anchor is strictly more useful than the truncation point: it points at the
( that needs closing rather than at the end of the buffer, and it is never
empty.

Proposed behaviour

Two rules, in order.

An "expected X to close Y" error anchors on the opening delimiter, and
spans it. The parser already knows the offset — it descended from there.

input	message	range	text
`foo(\n`	expected `)` to close arguments	`3..4`	`(`
`foo(a\n`	expected `)` to close arguments	`3..4`	`(`
`x = [\n`	``expected `]```	`4..5`	`[`
`x = {\n`	``expected `}```	`4..5`	`{`
`def f(:\n pass\n`	expected `)` to close parameters	`5..6`	`(`

Every other error at end of input falls back to a non-empty range: the
last non-trivia token, or if the file is empty, 0..0 is unavoidable and
acceptable. expected an expression on x = [\n should point at the [
or at the newline, not at 6..6.

A cheap invariant to hold the line, given errors already carry ranges:

debug_assert!(
    error.range.start() < error.range.end() || src.is_empty(),
    "empty diagnostic range: {error:?}"
);

Minimal reproduction

use starlark_cst::{Dialect, parse};

#[test]
fn errors_have_non_empty_ranges() {
    for src in ["foo(\n", "foo(a\n", "x = [\n", "x = {\n", "def f(:\n    pass\n"] {
        let parsed = parse(src, Dialect::Bazel);
        assert_eq!(parsed.syntax().to_string(), src, "round-trip must still hold");
        for error in parsed.errors() {
            assert!(
                error.range.start() < error.range.end(),
                "empty range for {src:?}: {error:?}"
            );
        }
    }
}

> Original issue: source #5 > Original author: `barrettruth` > Original date: 2026-08-26T12:01:14Z ## Version 0.1.1 ## Dialect `Dialect::Bazel`, reproduced on `BUILD` and `.bzl`. Not dialect-specific. ## Which guarantee broke Neither invariant — round-trip holds and nothing panics. This is a diagnostic quality bug: every error raised at end of input carries an empty range. ## Observed ``` src="foo(\n" len=5 5..5 "expected `)` to close arguments" <-- zero width src="x = [\n" len=6 6..6 "expected an expression" <-- zero width 6..6 "expected `]`" <-- zero width src="x = {\n" len=6 6..6 "expected an expression" <-- zero width 6..6 "expected `:` in dict entry (set literals are not Starlark)" 6..6 "expected `}`" <-- zero width src="def f(:\n pass\n" len=17 12..16 "expected an expression" 17..17 "expected `)` to close parameters" <-- zero width 17..17 "expected `:` after def signature" <-- zero width 17..17 "expected a statement" <-- zero width src="foo(a\n" len=6 6..6 "expected `)` to close arguments" <-- zero width ``` Every zero-width range is at `src.len()`. Errors raised at a real token are fine — `12..16` above is correct, and `foo(a, ??? )` style errors are one character wide. ## Why it matters An unclosed delimiter is not an edge case; it is the state a `BUILD` file is in for most of the time anyone is editing it. `cc_library(` is what exists between typing the callee and typing the first argument. Under LSP, a `Diagnostic` whose `range.start == range.end` has no characters to underline. Clients disagree on what to do with it: some render nothing at all, some fall back to a caret on the following character, some to the whole line. The most common diagnostic this crate produces is therefore the one most likely to be invisible. This is also a known trap that this project already committed to avoiding — the same defect was catalogued in starpls before this crate was written: > Parser errors get **zero-width ranges**: > ```rust > errors_sink(SyntaxError { message, range: TextRange::new(TextSize::new(token_pos), TextSize::new(token_pos)) }); > ``` > Editors render a 0-character squiggle. (Lexer errors *do* get real ranges.) and listed as a required change: *"Emit non-zero-width error ranges (currently `TextRange::new(pos, pos)`)."* It has been reproduced. ## Precedent Neither Bazel nor starlark-rust has a good answer for the EOF case specifically — starlark-rust degenerates to an empty span at `1:1`: ``` --- "foo(\n" error: Parse error: unexpected new line, expected expression --> t:1:1 | | ``` But Bazel does have the right idea for unclosed constructs generally, and applies it consistently to strings: the diagnostic is anchored on the token that opened the thing that was never closed, not on the point where the parser noticed. ```java // Lexer.java:287 — literalStartPos is the opening quote, not the truncation point error("unclosed string literal", literalStartPos); ``` ```java // LexerTest.java:682 — caret on the opening quote checkErrors("+ 'unterminated", "PLUS STRING(unterminated) NEWLINE EOF", " ^ unclosed string literal"); ``` That anchor is strictly more useful than the truncation point: it points at the `(` that needs closing rather than at the end of the buffer, and it is never empty. ## Proposed behaviour Two rules, in order. 1. **An "expected X to close Y" error anchors on the opening delimiter**, and spans it. The parser already knows the offset — it descended from there. | input | message | range | text | | --- | --- | --- | --- | | `foo(\n` | ``expected `)` to close arguments`` | `3..4` | `(` | | `foo(a\n` | ``expected `)` to close arguments`` | `3..4` | `(` | | `x = [\n` | ``expected `]``` | `4..5` | `[` | | `x = {\n` | ``expected `}``` | `4..5` | `{` | | `def f(:\n pass\n` | ``expected `)` to close parameters`` | `5..6` | `(` | 2. **Every other error at end of input falls back to a non-empty range**: the last non-trivia token, or if the file is empty, `0..0` is unavoidable and acceptable. `expected an expression` on `x = [\n` should point at the `[` or at the newline, not at `6..6`. A cheap invariant to hold the line, given errors already carry ranges: ```rust debug_assert!( error.range.start() < error.range.end() || src.is_empty(), "empty diagnostic range: {error:?}" ); ``` ## Minimal reproduction ```rust use starlark_cst::{Dialect, parse}; #[test] fn errors_have_non_empty_ranges() { for src in ["foo(\n", "foo(a\n", "x = [\n", "x = {\n", "def f(:\n pass\n"] { let parsed = parse(src, Dialect::Bazel); assert_eq!(parsed.syntax().to_string(), src, "round-trip must still hold"); for error in parsed.errors() { assert!( error.range.start() < error.range.end(), "empty range for {src:?}: {error:?}" ); } } } ```

barrettruth

2026-09-21 15:26:38 +00:00