diff --git a/src/ast/lib.rs b/src/ast/lib.rs index 8f450860cfc8..85b79db65e9f 100644 --- a/src/ast/lib.rs +++ b/src/ast/lib.rs @@ -648,9 +648,11 @@ pub struct Location { pub line_text: Option>, /// Number of bytes this location should highlight. /// 0 to just point at a single character - pub length: usize, + pub length: u32, // TODO: document or remove pub offset: usize, + /// 0-based column (UTF-16 units) at which a windowed `line_text` starts. + pub line_text_start_column: u32, /// 1-based line number. /// Line <= 0 means there is no line and column information. @@ -679,6 +681,7 @@ impl Clone for Location { length: self.length, line_text: self.line_text.as_deref().map(|t| Cow::Owned(t.to_vec())), offset: self.offset, + line_text_start_column: self.line_text_start_column, } } } @@ -691,6 +694,7 @@ impl Default for Location { line_text: None, length: 0, offset: 0, + line_text_start_column: 0, line: 0, column: 0, } @@ -738,6 +742,7 @@ impl Location { length: self.length, line_text: self.line_text.as_deref().map(|t| Cow::Owned(t.to_vec())), offset: self.offset, + line_text_start_column: self.line_text_start_column, } } @@ -756,9 +761,10 @@ impl Location { namespace: Cow::Borrowed(namespace), line, column, - length: length as usize, + length, line_text: line_text.map(Cow::Borrowed), offset: length as usize, + line_text_start_column: 0, } } @@ -792,6 +798,7 @@ impl Location { length: 0, line_text: Some(Cow::Borrowed(b"")), offset: 0, + line_text_start_column: 0, }); } let data = match tracker { @@ -799,12 +806,11 @@ impl Location { None => source.init_error_position(r.loc), }; let mut full_line = &source.contents[data.line_start..data.line_end]; - // Window a long line to ~120 bytes around the error. Bounds are - // BYTE offsets; the gate keeps the original shape (no left trim for - // an error in the last 80 bytes) so `write_format`'s caret aligns. + // An error in the last 80 bytes keeps the whole line: bake's overlay pads by `column`. let offset_in_line = clamp_error_offset(&source.contents, r.loc) .saturating_sub(data.line_start) .min(full_line.len()); + let mut line_text_start_column = 0; if full_line.len() > 80 + offset_in_line { let mut lo = offset_in_line.saturating_sub(40); let mut hi = (offset_in_line + 80).min(full_line.len()); @@ -816,6 +822,14 @@ impl Location { { hi += 1; } + if lo > 0 { + // Same counter as `column_count`, over the kept bytes only. + let mut kept = ErrorPositionState::default(); + kept.advance(full_line, lo, offset_in_line); + line_text_start_column = + u32::try_from(data.column_count.saturating_sub(kept.column_number)) + .expect("int cast"); + } full_line = &full_line[lo..hi]; } @@ -825,18 +839,14 @@ impl Location { line: usize2loc(data.line_count).start, column: usize2loc(data.column_count).start, length: if r.len > -1 { - u32::try_from(r.len).expect("int cast") as usize + u32::try_from(r.len).expect("int cast") } else { 1 }, - // `source_backing` in `Transpiler::parse_*` is RAII and - // drops on the parse-error path *before* `process_fetch_log` - // clones the `Msg` into a `BuildMessage`, so own the bytes here - // instead of borrowing `source.contents`. `full_line` is - // bounded (≤ ~120 bytes) and only materialized on diagnostic - // paths. - line_text: Some(Cow::Owned(bun_core::trim_left(full_line, b"\n\r").to_vec())), + // Owned: `source.contents` can be freed before this `Msg` is cloned. + line_text: Some(Cow::Owned(full_line.to_vec())), offset: usize::try_from(r.loc.start.max(0)).expect("int cast"), + line_text_start_column, }); } None @@ -982,7 +992,10 @@ impl Data { let line_text = bun_core::trim_left(line_text_right_trimmed, b"\n\r"); if location.column > 0 && !line_text.is_empty() { let mut line_offset_for_second_line: usize = - usize::try_from(location.column - 1).expect("int cast"); + usize::try_from(location.column - 1) + .expect("int cast") + .saturating_sub(location.line_text_start_column as usize) + .min(line_text.len()); if location.line > -1 { let bold = matches!(kind, Kind::Err | Kind::Warn); @@ -2454,11 +2467,7 @@ impl ErrorPositionState { fn to_error_position(self, line_end: usize) -> ErrorPosition { ErrorPosition { - line_start: if self.line_start > 0 { - self.line_start - 1 - } else { - self.line_start - }, + line_start: self.line_start, line_end, line_count: self.line_count, column_count: self.column_number, diff --git a/src/parsers/json.rs b/src/parsers/json.rs index 60b64891831a..a2961ab8922c 100644 --- a/src/parsers/json.rs +++ b/src/parsers/json.rs @@ -151,7 +151,7 @@ fn parse_impl_in( .iter() .filter(|m| m.kind == bun_ast::Kind::Err) .filter_map(|m| m.data.location.as_ref()) - .any(|l| l.offset + l.length.max(1) <= pos); + .any(|l| l.offset + l.length.max(1) as usize <= pos); if !earlier_stage2_err { drop_stage2_errors(log); return Err(report_index_error(e, source, log)); diff --git a/src/runtime/bake/dev_server/serialized_failure.rs b/src/runtime/bake/dev_server/serialized_failure.rs index 1e6df8283827..871effc05aea 100644 --- a/src/runtime/bake/dev_server/serialized_failure.rs +++ b/src/runtime/bake/dev_server/serialized_failure.rs @@ -229,7 +229,7 @@ fn write_log_data(data: &bun_ast::Data, w: &mut Writer) { _ = w.write_int_le::(loc.line); _ = w.write_int_le::(u32::try_from(loc.column).expect("int cast")); - _ = w.write_int_le::(u32::try_from(loc.length).expect("int cast")); + _ = w.write_int_le::(loc.length); // TODO: syntax highlighted line text + give more context lines write_string32(loc.line_text.as_deref().unwrap_or(b""), w); diff --git a/test/cli/install/bun-lock.test.ts b/test/cli/install/bun-lock.test.ts index c8ba27d17e5f..de102f02d2f5 100644 --- a/test/cli/install/bun-lock.test.ts +++ b/test/cli/install/bun-lock.test.ts @@ -1064,6 +1064,47 @@ it("prints an actionable error for a lockfile version newer than this build supp expect(await exited).toBe(0); }); +it("indents the caret under the printed excerpt when a one-line bun.lock has many warnings", async () => { + const count = 300; + const lock = + `{\n "lockfileVersion": 1,\n "workspaces": { "": { "name": "one-line-lockfile" } },\n "packages": { ` + + Array.from({ length: count }, (_, i) => `"p${i}": ["p${i}@1.0.0", "", {}, "sha512-x"]`).join(", ") + + ` }\n}\n`; + using dir = tempDir("one-line-lockfile", { + "package.json": JSON.stringify({ name: "one-line-lockfile", dependencies: {} }), + "bun.lock": lock, + }); + + await using proc = spawn({ + cmd: [bunExe(), "install"], + cwd: String(dir), + env, + stdout: "pipe", + stderr: "pipe", + }); + const [, err, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + const lines = err.split(/\r?\n/); + const carets = lines.filter(line => /^ *\^$/.test(line)); + expect(carets.length).toBe(count); + + // Each caret points at the `"` that opens the bad integrity string in the + // excerpt above it. Before the fix it was padded to the column in the full + // line, so the caret lines alone grew quadratically with the entry count. + for (let i = 0; i < lines.length; i++) { + if (!/^ *\^$/.test(lines[i])) continue; + const excerpt = lines[i - 1]; + expect(excerpt).toMatch(/^4 \| /); + const caretAt = lines[i].indexOf("^"); + expect(caretAt).toBeLessThan(excerpt.length); + expect(excerpt.slice(caretAt, caretAt + 8)).toBe('"sha512-'); + } + // Linear in the entry count: each warning prints a ~120-byte excerpt, except + // the ones in the last 80 bytes of the line, which print the whole line. + expect(err.length).toBeLessThan(count * 500 + 4 * lock.length); + expect(exitCode).toBe(0); +}); + async function installWithHandEditedOverrides(overrides: Record) { const { packageDir, packageJson } = await registry.createTestDir(); const lockfile = JSON.stringify( diff --git a/test/js/bun/transpiler/parse-error-column.test.ts b/test/js/bun/transpiler/parse-error-column.test.ts index 796f1b3cf6e8..2c044e3daca9 100644 --- a/test/js/bun/transpiler/parse-error-column.test.ts +++ b/test/js/bun/transpiler/parse-error-column.test.ts @@ -141,22 +141,111 @@ test.concurrent("long non-ASCII line's lineText window does not split a UTF-8 se }); }); -test.concurrent("CLI caret stays under the token for an error at the end of a long line", async () => { - // `write_format` offsets the caret by `column - 1` with no knowledge of any - // left-trim, so the window gate must not left-trim this case. - using dir = tempDir("parse-col-caret", { - "long.js": Buffer.alloc(150, "a").toString() + "]", - }); +const fill = (count: number, char: string) => Buffer.alloc(count, char).toString(); + +/** + * Runs `bun ` in a directory holding `files` and returns every source + * excerpt the logger printed (a `N | text` line followed by a caret line) along + * with the column the `^` landed on. The excerpt may be a window of a long line; + * the caret must land on the offending token inside the printed excerpt. + */ +async function printedExcerpts(args: string[], files: Record) { + using dir = tempDir("parse-col-caret", files); await using proc = Bun.spawn({ - cmd: [bunExe(), "build", "long.js"], - env: { ...bunEnv, NO_COLOR: "1" }, + cmd: [bunExe(), ...args], + env: bunEnv, cwd: String(dir), stdout: "pipe", stderr: "pipe", }); - const [, stderr] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - const lines = stderr.split("\n"); - const textLine = lines.find(l => l.includes("]"))!; - const caretLine = lines.find(l => l.trimEnd().endsWith("^"))!; - expect({ token: textLine.indexOf("]"), caret: caretLine.indexOf("^") }).toEqual({ token: 154, caret: 154 }); + const [stderr] = await Promise.all([proc.stderr.text(), proc.stdout.text(), proc.exited]); + const lines = stderr.split(/\r?\n/); + const excerpts: { excerpt: string; caret: number }[] = []; + for (let i = 0; i + 1 < lines.length; i++) { + if (/^\d+ \| /.test(lines[i]) && /^ *\^$/.test(lines[i + 1])) { + excerpts.push({ excerpt: lines[i], caret: lines[i + 1].indexOf("^") }); + } + } + return { stderr, excerpts }; +} + +test.concurrent("CLI caret stays under the token for an error at the end of a long line", async () => { + // An error in the last 80 bytes of a line is never left-trimmed. + const { excerpts } = await printedExcerpts(["build", "long.js"], { "long.js": fill(150, "a") + "]" }); + expect(excerpts).toEqual([{ excerpt: "1 | " + fill(150, "a") + "]", caret: 4 + 150 }]); +}); + +test.concurrent.each([["build"], ["run"]])( + "bun %s: CLI caret stays under the token when the excerpt is left-trimmed", + async subcommand => { + // `]` is at byte 100 of a 201-byte line, so the excerpt is the 120 bytes + // around it (40 before, 80 after) and the caret belongs 40 characters in, + // not at the token's column in the full line (which is past the end of + // the excerpt). + const { excerpts } = await printedExcerpts([subcommand, "long.js"], { + "long.js": "let ok = 1;\n" + fill(100, "a") + "]" + fill(100, "b") + "\n", + }); + expect(excerpts).toEqual([{ excerpt: "2 | " + fill(40, "a") + "]" + fill(79, "b"), caret: 4 + 40 }]); + }, +); + +test.concurrent("CLI caret counts the left-trimmed prefix in columns, not bytes", async () => { + // U+00E9 is 2 UTF-8 bytes but 1 column (Buffer.alloc fills by bytes, so + // fill(200) is 100 characters). `]` is at byte 200 / column 101 of a + // 401-byte line; the window keeps the 40 bytes (20 characters) before it, + // so the caret belongs 20 characters in. + const { excerpts } = await printedExcerpts(["build", "long.js"], { + "long.js": fill(200, "\u00E9") + "]" + fill(200, "\u00E9"), + }); + expect(excerpts).toEqual([{ excerpt: "1 | " + fill(40, "\u00E9") + "]" + fill(80, "\u00E9"), caret: 4 + 20 }]); +}); + +test.concurrent("CLI caret counts a left-trimmed astral prefix in UTF-16 units, like the column", async () => { + // U+10400 (a valid identifier character) is 4 UTF-8 bytes and 2 UTF-16 + // units, and columns count UTF-16 units. 30 of them put `]` at byte 120 / + // column 61; the window keeps the 10 characters (20 units) before it, so + // the caret belongs 20 units in (10 if the trimmed width were counted in + // characters, 60 if it were not subtracted at all). + const { excerpts } = await printedExcerpts(["build", "long.js"], { + "long.js": fill(120, "\u{10400}") + "]" + fill(100, "b"), + }); + expect(excerpts).toEqual([{ excerpt: "1 | " + fill(40, "\u{10400}") + "]" + fill(79, "b"), caret: 4 + 20 }]); +}); + +test.concurrent("CLI caret is aligned for both the error and its note on a long line", async () => { + // The redeclaration at byte 176 is left-trimmed; the note pointing at the + // original declaration (byte 6) is windowed without a left trim. + const source = "const x = 1; /* " + fill(150, "p") + " */ const x = 2; " + fill(100, "q") + "\n"; + const { excerpts } = await printedExcerpts(["build", "long.js"], { "long.js": source }); + expect(excerpts).toEqual([ + { excerpt: "1 | " + fill(30, "p") + " */ const x = 2; " + fill(73, "q"), caret: 4 + 30 + " */ const ".length }, + { excerpt: "1 | const x = 1; /* " + fill(70, "p"), caret: 4 + "const ".length }, + ]); +}); + +test.concurrent("bun install points the caret at the token inside a long package.json line", async () => { + // The `}` is at column 179 of a 379-byte line. The excerpt starts 40 bytes + // before it (the caret used to be indented by the full 178 columns), while + // the `at file:line:col` suffix still reports the real column. + const { stderr, excerpts } = await printedExcerpts(["install"], { + "package.json": '{"name":"x",' + fill(150, " ") + '"dependencies": }' + fill(200, " ") + "\n", + }); + expect(excerpts).toEqual([ + { excerpt: "1 | " + fill(24, " ") + '"dependencies": }', caret: 4 + 24 + '"dependencies": '.length }, + ]); + expect(stderr).toContain("package.json:1:179"); }); + +test.concurrent( + "excerpt of a line following a U+2028 line separator starts at the line's first character", + async () => { + // The logger used to slice the line from one byte before its start to pick + // up (and later trim) a preceding `\n`; after a three-byte U+2028 that byte + // is a stray UTF-8 continuation byte, which was printed before the text and + // pushed it one cell to the right of the caret. + const { excerpts } = await printedExcerpts(["build", "ls.js"], { + "ls.js": "let a = 1;\u2028let b = 2; ]", + }); + expect(excerpts).toEqual([{ excerpt: "2 | let b = 2; ]", caret: 4 + "let b = 2; ".length }]); + }, +);