Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 34 additions & 12 deletions src/ast/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,11 @@ pub struct Location {
pub length: usize,
// TODO: document or remove
pub offset: usize,
/// 0-based column (UTF-16 units, like `column`) at which `line_text`
/// starts in the source line. Non-zero when `line_text` is a window of a
/// longer line; `write_format` subtracts it from `column` to place the
/// caret under the excerpt.
Comment thread
robobun marked this conversation as resolved.
Resolved
pub line_text_start_column: usize,

/// 1-based line number.
/// Line <= 0 means there is no line and column information.
Expand Down Expand Up @@ -679,6 +684,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,
}
}
}
Expand All @@ -691,6 +697,7 @@ impl Default for Location {
line_text: None,
length: 0,
offset: 0,
line_text_start_column: 0,
line: 0,
column: 0,
}
Expand Down Expand Up @@ -738,6 +745,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,
}
}

Expand All @@ -759,6 +767,7 @@ impl Location {
length: length as usize,
line_text: line_text.map(Cow::Borrowed),
offset: length as usize,
line_text_start_column: 0,
}
}

Expand Down Expand Up @@ -792,6 +801,7 @@ impl Location {
length: 0,
line_text: Some(Cow::Borrowed(b"")),
offset: 0,
line_text_start_column: 0,
});
}
let data = match tracker {
Expand All @@ -800,11 +810,15 @@ impl Location {
};
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.
// BYTE offsets. `line_text_start_column` records how many columns
// the window drops on the left so `write_format`'s caret aligns.
// Consumers that only know `column` (the bake overlay,
// `BuildMessage.position`) rely on the gate not left-trimming an
// error in the last 80 bytes of a line.
Comment thread
robobun marked this conversation as resolved.
Resolved
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());
Expand All @@ -816,6 +830,13 @@ impl Location {
{
hi += 1;
}
if lo > 0 {
// Count the <= 40 kept bytes with the scanner that produced
// `column_count`, instead of rescanning the dropped prefix.
Comment thread
robobun marked this conversation as resolved.
Resolved
let mut kept = ErrorPositionState::default();
kept.advance(full_line, lo, offset_in_line);
line_text_start_column = data.column_count.saturating_sub(kept.column_number);
}
full_line = &full_line[lo..hi];
}

Expand All @@ -832,11 +853,11 @@ impl Location {
// `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())),
// instead of borrowing `source.contents`. Only materialized on
// diagnostic paths.
Comment thread
robobun marked this conversation as resolved.
Resolved
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
Expand Down Expand Up @@ -981,8 +1002,13 @@ impl Data {
let line_text_right_trimmed = bun_core::trim_right(line_text_, b" \r\n\t");
let line_text = bun_core::trim_left(line_text_right_trimmed, b"\n\r");
if location.column > 0 && !line_text.is_empty() {
// The caret sits under the printed excerpt, so its indent
// is bounded by the excerpt, never by the full column.
Comment thread
robobun marked this conversation as resolved.
Resolved
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)
.min(line_text.len());

if location.line > -1 {
let bold = matches!(kind, Kind::Err | Kind::Warn);
Expand Down Expand Up @@ -2454,11 +2480,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,
Expand Down
41 changes: 41 additions & 0 deletions test/cli/install/bun-lock.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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("\n");
Comment thread
robobun marked this conversation as resolved.
Resolved
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<string, unknown>) {
const { packageDir, packageJson } = await registry.createTestDir();
const lockfile = JSON.stringify(
Expand Down
115 changes: 102 additions & 13 deletions test/js/bun/transpiler/parse-error-column.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 <args>` 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<string, string>) {
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 }]);
},
);
Comment thread
robobun marked this conversation as resolved.
Resolved

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 }]);
},
);
Loading