bug: the lexer has no error channel, so unclosed strings, tab indentation and invalid characters parse clean #1

New issue

Closed

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

barrettruth commented

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

Owner

Original issue: source #3
Original author: barrettruth
Original date: 2026-08-26T12:01:00Z

Version

0.1.1

Dialect

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

Which guarantee broke

Neither of the two documented invariants: round-trip holds byte for byte and
nothing panics. This is the inverse of the template's "Parse error on a file
Bazel accepts" — no parse error on a file Bazel rejects. The bug report
template has no option for that class, and all three symptoms below are in it.

Observed

input	`parse(..).errors()`	Bazel	starlark-rust 0.14.2
`x = "hello\ny = 1\n`	0	`unclosed string literal`	`unfinished string literal`
`x = "hello` (EOF)	0	`unclosed string literal`	`unfinished string literal`
`x = """hello\n`	0	`unclosed string literal`	`unfinished string literal`
`def f():\n\tpass\n`	0	`Tab characters are not allowed for indentation. Use spaces instead.`	`tabs are not allowed`
`x = ?\n`	1 × `expected an expression`	`invalid character: '?'`	invalid input `?`

A consumer cannot distinguish these from clean files. For a language server
that means no squiggle on an unterminated string — the single most common
transient state while typing a label.

Root cause

One structural gap, three symptoms: the lexer has no error output.

// src/lexer.rs:29
pub fn tokenize(src: &str, dialect: Dialect) -> Vec<Lexeme>

There is nowhere for a lexical diagnostic to go, so none is produced.

fn string (src/lexer.rs:287) terminates the literal on None
(src/lexer.rs:297) or on an unescaped newline outside a triple quote
(src/lexer.rs:318), then unconditionally pushes STRING/BYTES
(src/lexer.rs:327). Whether the closing quote was ever found is not
recorded.
Unclassifiable bytes become a bare ERROR_TOKEN carrying no message
(src/lexer.rs:275, src/lexer.rs:497). The parser then reports whatever
generic syntactic expectation it happened to hold — expected an expression — and the lexical fact is lost.
The indentation scanner has no tab branch at all.

Precedent

Bazel's Lexer.java is the normative implementation, and it reports the error
while still emitting the token:

// Lexer.java:287
error("unclosed string literal", literalStartPos);
setToken(TokenKind.STRING, literalStartPos, pos);
setValue(literal.toString());
return;

Four further sites do the same: Lexer.java:294 (backslash at EOF),
Lexer.java:421 (triple-quote EOF), Lexer.java:452 (raw string newline),
Lexer.java:501 (raw string EOF). Tabs are Lexer.java:213; invalid
characters are Lexer.java:874.

That shape matters here: because Bazel keeps the token and only adds a
diagnostic, adopting it changes no byte range, so to_string() == src is
unaffected. Both invariants survive.

LexerTest.java pins the range convention — the caret sits on the opening
quote (literalStartPos), not at the truncation point, and the STRING
token still spans the truncated literal:

// LexerTest.java:682
checkErrors("+ 'unterminated", "PLUS STRING(unterminated) NEWLINE EOF",
            "  ^ unclosed string literal");
// LexerTest.java:424 — triple raw string, caret at offset 0
checkErrors("r'''\\'''", "STRING(\\''') NEWLINE EOF", "^ unclosed string literal");

starlark-rust spans the whole truncated literal rather than a single point,
which is the better rendering of the same anchor:

error: Parse error: unfinished string literal
 --> t:1:5
1 | x = "hello
  |     ^^^^^^

Proposed behaviour

Give the lexer an error channel and keep the token stream exactly as it is
today. Messages verbatim from Lexer.java, since those are the ones users
have already read from Bazel itself:

condition	message	range
closing quote never found	`unclosed string literal`	opening quote including any `r`/`b` prefix, through the end of the truncated literal
`\t` in leading indentation	`Tab characters are not allowed for indentation. Use spaces instead.`	the tab
byte classifiable as no token	`invalid character: '<c>'`	the whole UTF-8 character, matching the existing `ERROR_TOKEN` width

Anchoring on the opening quote rather than on the newline is the useful choice
and the one both reference implementations make: it points at the quote that
needs closing.

For x = ? the lexical message should replace the parser's expected an expression, not stack with it — one byte of bad input is one diagnostic.

Minimal reproduction

use starlark_cst::{Dialect, parse};

#[test]
fn reports_lexical_errors() {
    for src in [
        "x = \"hello\ny = 1\n",   // unclosed string, terminated by newline
        "x = \"hello",            // unclosed string, terminated by EOF
        "x = \"\"\"hello\n",      // unclosed triple-quoted string
        "def f():\n\tpass\n",     // tab indentation
    ] {
        let parsed = parse(src, Dialect::Bazel);
        assert_eq!(parsed.syntax().to_string(), src, "round-trip must still hold");
        assert!(!parsed.errors().is_empty(), "no error reported for {src:?}");
    }
}

All four currently report zero errors while round-tripping correctly.

> Original issue: source #3 > Original author: `barrettruth` > Original date: 2026-08-26T12:01:00Z ## Version 0.1.1 ## Dialect `Dialect::Bazel`, reproduced on `BUILD` and `.bzl`. Not dialect-specific. ## Which guarantee broke Neither of the two documented invariants: round-trip holds byte for byte and nothing panics. This is the inverse of the template's *"Parse error on a file Bazel accepts"* — **no parse error on a file Bazel rejects**. The bug report template has no option for that class, and all three symptoms below are in it. ## Observed | input | `parse(..).errors()` | Bazel | starlark-rust 0.14.2 | | --- | --- | --- | --- | | `x = "hello\ny = 1\n` | **0** | `unclosed string literal` | `unfinished string literal` | | `x = "hello` (EOF) | **0** | `unclosed string literal` | `unfinished string literal` | | `x = """hello\n` | **0** | `unclosed string literal` | `unfinished string literal` | | `def f():\n\tpass\n` | **0** | `Tab characters are not allowed for indentation. Use spaces instead.` | `tabs are not allowed` | | `x = ?\n` | 1 × `expected an expression` | `invalid character: '?'` | ``invalid input `?` `` | A consumer cannot distinguish these from clean files. For a language server that means no squiggle on an unterminated string — the single most common transient state while typing a label. ## Root cause One structural gap, three symptoms: the lexer has no error output. ```rust // src/lexer.rs:29 pub fn tokenize(src: &str, dialect: Dialect) -> Vec<Lexeme> ``` There is nowhere for a lexical diagnostic to go, so none is produced. - `fn string` (`src/lexer.rs:287`) terminates the literal on `None` (`src/lexer.rs:297`) or on an unescaped newline outside a triple quote (`src/lexer.rs:318`), then unconditionally pushes `STRING`/`BYTES` (`src/lexer.rs:327`). Whether the closing quote was ever found is not recorded. - Unclassifiable bytes become a bare `ERROR_TOKEN` carrying no message (`src/lexer.rs:275`, `src/lexer.rs:497`). The parser then reports whatever generic syntactic expectation it happened to hold — `expected an expression` — and the lexical fact is lost. - The indentation scanner has no tab branch at all. ## Precedent Bazel's `Lexer.java` is the normative implementation, and it reports the error **while still emitting the token**: ```java // Lexer.java:287 error("unclosed string literal", literalStartPos); setToken(TokenKind.STRING, literalStartPos, pos); setValue(literal.toString()); return; ``` Four further sites do the same: `Lexer.java:294` (backslash at EOF), `Lexer.java:421` (triple-quote EOF), `Lexer.java:452` (raw string newline), `Lexer.java:501` (raw string EOF). Tabs are `Lexer.java:213`; invalid characters are `Lexer.java:874`. That shape matters here: because Bazel keeps the token and only adds a diagnostic, adopting it changes no byte range, so `to_string() == src` is unaffected. Both invariants survive. `LexerTest.java` pins the range convention — the caret sits on the **opening quote** (`literalStartPos`), not at the truncation point, and the `STRING` token still spans the truncated literal: ```java // LexerTest.java:682 checkErrors("+ 'unterminated", "PLUS STRING(unterminated) NEWLINE EOF", " ^ unclosed string literal"); // LexerTest.java:424 — triple raw string, caret at offset 0 checkErrors("r'''\\'''", "STRING(\\''') NEWLINE EOF", "^ unclosed string literal"); ``` starlark-rust spans the whole truncated literal rather than a single point, which is the better rendering of the same anchor: ``` error: Parse error: unfinished string literal --> t:1:5 1 | x = "hello | ^^^^^^ ``` ## Proposed behaviour Give the lexer an error channel and keep the token stream exactly as it is today. Messages verbatim from `Lexer.java`, since those are the ones users have already read from Bazel itself: | condition | message | range | | --- | --- | --- | | closing quote never found | `unclosed string literal` | opening quote **including** any `r`/`b` prefix, through the end of the truncated literal | | `\t` in leading indentation | `Tab characters are not allowed for indentation. Use spaces instead.` | the tab | | byte classifiable as no token | `invalid character: '<c>'` | the whole UTF-8 character, matching the existing `ERROR_TOKEN` width | Anchoring on the opening quote rather than on the newline is the useful choice and the one both reference implementations make: it points at the quote that needs closing. For `x = ?` the lexical message should replace the parser's `expected an expression`, not stack with it — one byte of bad input is one diagnostic. ## Minimal reproduction ```rust use starlark_cst::{Dialect, parse}; #[test] fn reports_lexical_errors() { for src in [ "x = \"hello\ny = 1\n", // unclosed string, terminated by newline "x = \"hello", // unclosed string, terminated by EOF "x = \"\"\"hello\n", // unclosed triple-quoted string "def f():\n\tpass\n", // tab indentation ] { let parsed = parse(src, Dialect::Bazel); assert_eq!(parsed.syntax().to_string(), src, "round-trip must still hold"); assert!(!parsed.errors().is_empty(), "no error reported for {src:?}"); } } ``` All four currently report zero errors while round-tripping correctly.

barrettruth

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