Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
8 changes: 8 additions & 0 deletions src/ast/e.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1911,7 +1911,15 @@

/// Return UTF-8 bytes, transcoding if UTF-16.
/// The transcode allocates via the global arena then copies into `bump`.
///
/// Reads `data` only: a string that went through the visit pass may be a
/// rope (`fold_string_addition`), of which this is just the first segment.
/// Use `slice` (or `resolve_rope_if_needed` first) on those.
Comment thread
robobun marked this conversation as resolved.
Outdated
pub fn string<'b>(&self, bump: &'b Bump) -> Result<&'b [u8], AllocError> {
debug_assert!(
self.next.is_none(),
"EString::string() called on an unresolved rope; use slice()"
);

Check warning on line 1922 in src/ast/e.rs

View check run for this annotation

Claude / Claude Code Review

Same-class rope readers via direct .data access are not covered by the .string() audit

The new `debug_assert` guards `.string()`, and the PR description says the three fixed sites "were the only ones that read a rope" — but the audit only covered `.string()` callers. Several post-visit readers reach the rope's first segment via a direct `.data` field access, which neither the audit nor the assert catches: `Import::import_record_loader` (this file, ~L2464 — `Loader::from_string(&str_.data)`; reachable with no minify flag because the `EImport` arm forces folding), `rewrite_import_me
Comment thread
claude[bot] marked this conversation as resolved.
Outdated
if self.is_utf8() {
// `self.data` is arena-owned with the same lifetime as `bump`;
// StoreStr re-borrows under that contract.
Expand Down
6 changes: 4 additions & 2 deletions src/ast/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -678,12 +678,14 @@ impl ArrayIterator {
// raw-ptr returns). Those drafts were dropped; only the methods without a live
// counterpart remain.
impl Expr {
/// Unlike `as_utf8_string_literal`, this accepts visited strings, which may
/// be ropes: the rope is flattened in place.
Comment thread
robobun marked this conversation as resolved.
Outdated
#[inline]
pub fn as_string_literal<'b>(&self, bump: &'b Bump) -> Option<&'b [u8]> {
let Data::EString(s) = &self.data else {
let Data::EString(mut s) = self.data else {
return None;
};
s.string(bump).ok()
Some(s.slice(bump))
}

/// `as_string_hash` for JSON-parsed trees (always UTF-8, no rope) where no
Expand Down
26 changes: 13 additions & 13 deletions src/js_parser/p.rs
Original file line number Diff line number Diff line change
Expand Up @@ -801,20 +801,24 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
arg: Expr,
buf: &'b mut BumpVec<'a, u8>,
) -> Result<&'b [u8], crate::Error> {
if let Some(tmpl) = arg.data.e_template() {
if let Some(mut tmpl) = arg.data.e_template() {
if tmpl.tag.is_some() {
return Ok(b""); // tagged template — opaque
}
match &tmpl.head {
// After folding "./fo" + `o/${x}` the head (or a tail) is a rope;
// `string` alone would give only its first segment.
Comment thread
robobun marked this conversation as resolved.
Outdated
match &mut tmpl.head {
js_ast::e::TemplateContents::Cooked(head) => {
head.resolve_rope_if_needed(self.arena);
buf.extend_from_slice(head.string(self.arena)?);
}
js_ast::e::TemplateContents::Raw(_) => return Ok(b""), // shouldn't happen post-visit but be safe
}
for part in tmpl.parts().iter() {
for part in tmpl.parts_mut().iter_mut() {
buf.push(0); // \x00 placeholder per interpolation
match &part.tail {
match &mut part.tail {
js_ast::e::TemplateContents::Cooked(tail) => {
tail.resolve_rope_if_needed(self.arena);
buf.extend_from_slice(tail.string(self.arena)?);
}
js_ast::e::TemplateContents::Raw(_) => return Ok(b""), // raw tail — treat as opaque
Expand Down Expand Up @@ -1080,15 +1084,11 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
return self.new_expr(E::Null {}, arg.loc);
}

let import_record_index = self.add_import_record(
ImportKind::RequireResolve,
arg.loc,
arg.data
.e_string()
.expect("infallible: variant checked")
.string(self.arena)
.expect("unreachable"),
);
// `slice` flattens the rope left behind by folding "./fo" + "o";
// `string` would record only its first segment.
Comment thread
robobun marked this conversation as resolved.
Outdated
let mut str_ = arg.data.e_string().expect("infallible: variant checked");
let import_record_index =
self.add_import_record(ImportKind::RequireResolve, arg.loc, str_.slice(self.arena));
self.import_records.items_mut()[import_record_index as usize]
.flags
.set(
Expand Down
51 changes: 51 additions & 0 deletions test/bundler/bundler_allow_unresolved.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,4 +226,55 @@ describe("bundler", () => {
backend: "cli",
allowUnresolved: ["./locales/*.json"],
});

// 17-20. Constant folding turns the template head/tail into a rope of string
// segments. The shape must include every segment ("./locales/*.json"), not
// just the first one ("./loc*.json"), or a matching pattern is rejected.
itBundled("allow-unresolved/RopeHeadFromStringAddition", {
files: {
"/entry.js": /* js */ `
export function load(x) {
return import("./loc" + \`ales/\${x}.json\`);
}
`,
},
outdir: "/out",
allowUnresolved: ["./locales/*.json"],
});

itBundled("allow-unresolved/RopeHeadFromTemplateFolding", {
files: {
"/entry.js": /* js */ `
export function load(x) {
return import(\`./loc\${"ales"}/\${x}.json\`);
}
`,
},
outdir: "/out",
allowUnresolved: ["./locales/*.json"],
});

itBundled("allow-unresolved/RopeTailFromStringAddition", {
files: {
"/entry.js": /* js */ `
export function load(x) {
return import(\`./locales/\${x}\` + ".json");
}
`,
},
outdir: "/out",
allowUnresolved: ["./locales/*.json"],
});

itBundled("allow-unresolved/RequireResolveRopeHead", {
files: {
"/entry.js": /* js */ `
export function load(x) {
return require.resolve("./loc" + \`ales/\${x}.json\`);
}
`,
},
outdir: "/out",
allowUnresolved: ["./locales/*.json"],
});
});
24 changes: 24 additions & 0 deletions test/bundler/transpiler/macro-test.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,30 @@ test("object destructuring of a macro result keeps every bound property regardle
expect(exitCode).toBe(0);
});

test("object destructuring of a macro result matches a computed key built from folded string literals", async () => {
// ["a" + "b"] is folded into a rope; matching it against the macro result must use the whole
// string, or the `ab` property is dropped from the inlined object.
using dir = tempDir("macro-destructure-folded-key", {
"m.ts": `export function m() {\n return { ab: 1, c: 2 };\n}\n`,
"index.ts": [
`import { m } from "./m.ts" with { type: "macro" };`,
`const { ["a" + "b"]: x, c } = m();`,
`console.log(JSON.stringify([x, c]));`,
``,
].join("\n"),
});
await using proc = Bun.spawn({
cmd: [bunExe(), "run", "index.ts"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect({ lastLine: stdout.trim().split("\n").pop(), stderr }).toEqual({ lastLine: "[1,2]", stderr: "" });
expect(exitCode).toBe(0);
});

describe("--no-macros", () => {
const files = {
"macro.ts": `
Expand Down
23 changes: 23 additions & 0 deletions test/bundler/transpiler/transpiler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3038,6 +3038,29 @@ console.log(resolve.length)
`export const foo = require.resolve("my-module")`,
);
});

it("require.resolve() keeps every segment of a folded string concatenation", () => {
// "./fo" + "o" is folded into a rope; the import record must hold the whole string, not its head.
expectPrinted_(
`export const foo = require.resolve('./fo' + 'o')`,
`export const foo = require.resolve("./foo")`,
);
expectPrinted_(
`export const foo = require.resolve('./a' + '/b' + '/c')`,
`export const foo = require.resolve("./a/b/c")`,
);
expectPrinted_(
`export const foo = require.resolve(x ? './fo' + 'o' : './ba' + 'r')`,
`export const foo = x ? require.resolve("./foo") : require.resolve("./bar")`,
);
// require() and import() of the same argument already behaved this way.
expectPrinted_(`export const foo = require('./fo' + 'o')`, `export const foo = require("./foo")`);
expectPrinted_(`export const foo = import('./fo' + 'o')`, `export const foo = import("./foo")`);
expect(transpiler.scan(`require.resolve("./fo" + "o"); import("./ba" + "r");`).imports).toEqual([
{ kind: "require-resolve", path: "./foo" },
{ kind: "dynamic-import", path: "./bar" },
]);
});
});

it("define", () => {
Expand Down
22 changes: 22 additions & 0 deletions test/js/bun/resolve/resolve.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,28 @@ it("file url in require.resolve resolves", async () => {
expect(stdout.toString("utf8")).toBe(`${dir}${sep}index.js\n`);
});

it("require.resolve of a folded string concatenation resolves the whole specifier", async () => {
// "./fo" + "o" is constant-folded by the transpiler; it used to record only "./fo".
await using dir = tempDir("require-resolve-folded", {
"foo.js": "module.exports = 'foo';",
"bar.js": "module.exports = 'bar';",
"test.js": `
console.log(require.resolve("./fo" + "o"));
console.log(require.resolve(process.argv.length > 100 ? "./fo" + "o" : "./ba" + "r"));
console.log(require("./fo" + "o"));
`,
});

const { exitCode, stdout, stderr } = Bun.spawnSync({
cmd: [bunExe(), `${dir}/test.js`],
env: bunEnv,
cwd: String(dir),
});
expect(stderr.toString("utf8")).toBe("");
expect(stdout.toString("utf8")).toBe(`${dir}${sep}foo.js\n${dir}${sep}bar.js\nfoo\n`);
expect(exitCode).toBe(0);
});

it("file url with special characters in require resolves", async () => {
const filename = "🅱️ndex.js";
await using dir = tempDir("file url", {
Expand Down
Loading