Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
11 changes: 10 additions & 1 deletion crates/perry-codegen/src/lower_call/property_get.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ mod static_dispatch;
// original unqualified names.
pub(crate) use helpers::{
class_chain_has_field_named, is_array_only_method_name, is_date_receiver,
is_inherited_object_prototype_method, resolve_static_dispatch_cls, string_only_method_arity_ok,
is_inherited_object_prototype_method, receiver_class_defines_method,
resolve_static_dispatch_cls, string_only_method_arity_ok,
};

/// Try to lower a `Call { callee: PropertyGet { .. } }` via the
Expand Down Expand Up @@ -203,6 +204,14 @@ pub fn try_lower_property_get_method_call(
if is_string_only_method
&& string_only_method_arity_ok(property, args.len())
&& !receiver_is_object_literal
// A receiver whose statically-known class defines its OWN method of
// this name is calling THAT method, never the String builtin — even
// when the arity matches. Critical for the char-access methods
// (`charAt`/`charCodeAt`/`codePointAt`), whose arity gate above is a
// no-op (any arg count is spec-valid), so a user `charAt(n)` helper
// (e.g. the `yaml` package's `Lexer`) would otherwise be coerced to
// `String.prototype.charAt` on a `"[object Object]"` receiver.
&& !receiver_class_defines_method(ctx, object, property)
&& !is_array_expr(ctx, object)
&& !is_buffer
&& !is_native_module_dynamic_index(object)
Expand Down
47 changes: 47 additions & 0 deletions crates/perry-codegen/src/lower_call/property_get/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,53 @@ pub(crate) fn string_only_method_arity_ok(name: &str, argc: usize) -> bool {
}
}

/// True when `object`'s statically-known class (or an ancestor) defines its
/// OWN instance method, getter, or field named `name`. Keeps the static
/// String-method fast path from hijacking a user class member that merely
/// shares a `String.prototype` name. This matters most for the char-access methods
/// (`charAt`/`charCodeAt`/`codePointAt`): their arity gate can never
/// disambiguate a user method from the builtin (any arg count is spec-valid,
/// so `string_only_method_arity_ok` always returns `true`), so without this a
/// `this.charAt(0)` on a class instance is lowered to `String.prototype.charAt`
/// with the receiver coerced to `"[object Object]"` (yielding `"["`, `"o"`, …).
/// The `yaml` package's `Lexer.charAt(n)` is exactly this shape — the tokenizer
/// then reads garbage, and its `*lex` state machine never advances `pos`,
/// spinning forever. A genuine string receiver is unaffected: it has no known
/// class, so this returns `false` and the string path (or the runtime
/// `jsval.is_string()` arm of `js_native_call_method`) still applies.
pub(crate) fn receiver_class_defines_method(ctx: &FnCtx<'_>, object: &Expr, name: &str) -> bool {
let Some(mut class_name) = receiver_class_name(ctx, object) else {
return false;
};
// Bounded walk up the inheritance chain (defensive against a cyclic
// `extends_name` in malformed input).
for _ in 0..64 {
let Some(class) = ctx.classes.get(&class_name) else {
return false;
};
if class.methods.iter().any(|m| m.name == name)
|| class.getters.iter().any(|(g, _)| g == name)
// An instance FIELD of that name shadows the builtin too: its
// init can be a function value (`charAt = (n) => …`), and even a
// non-function field makes `obj.charAt(0)` a runtime "not a
// function" TypeError — never the String builtin. A computed key
// (`key_expr`) could evaluate to `name`, so treat it as defining
// the member (same conservatism as `class_chain_has_field_named`).
|| class
.fields
.iter()
.any(|f| f.key_expr.is_some() || (!f.is_private && f.name == name))
{
return true;
}
match &class.extends_name {
Some(parent) => class_name = parent.clone(),
None => return false,
}
}
false
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

pub(crate) fn is_date_receiver(ctx: &FnCtx<'_>, object: &Expr) -> bool {
matches!(object, Expr::DateNew(_))
|| receiver_class_name(ctx, object).as_deref() == Some("Date")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,208 @@
//! Regression test for #6065: a user class method whose name shadows a
//! `String.prototype` char-access method (`charAt` / `charCodeAt` /
//! `codePointAt`) must call the USER method, not the String builtin.
//!
//! The static string-method fast path was guarded by an arity heuristic
//! (`string_only_method_arity_ok`) so that a user method sharing a String
//! builtin name (e.g. joi's `internals.trim(value, schema)`) falls through to
//! runtime dispatch when the arg count can't match the builtin. But the
//! char-access methods ignore surplus args per spec, so that gate returns
//! `true` for ANY arg count — a user `charAt(n)` on a class instance was
//! therefore lowered to `String.prototype.charAt` with the receiver coerced to
//! `"[object Object]"`, so `this.charAt(0)` returned `"["`, `this.charAt(1)`
//! returned `"o"`, and so on.
//!
//! The `yaml` package's `Lexer.charAt(n) { return this.buffer[this.pos + n]; }`
//! is exactly this shape: with it mis-dispatched the tokenizer reads garbage
//! and its `*lex` state machine (`while (next) next = yield* this.parseNext(next)`)
//! never advances `pos`, spinning forever — hanging YAML parsing (and any large
//! esbuild-bundled CLI app that parses YAML at module-init time) at 100% CPU.
//!
//! Fix: don't take the static String path when the receiver's statically-known
//! class defines its own method, getter, or instance field of that name.
//!
//! Expected outputs are byte-for-byte what `node --experimental-strip-types`
//! prints. The generator/lexer cases are timeout-bounded so a regression FAILS
//! (spins) instead of hanging the test process.

use std::path::PathBuf;
use std::process::Command;
use std::time::{Duration, Instant};

fn perry_bin() -> PathBuf {
PathBuf::from(env!("CARGO_BIN_EXE_perry"))
}

fn compile_and_run(dir: &std::path::Path, source: &str) -> String {
let entry = dir.join("main.ts");
let output = dir.join("main_bin");
std::fs::write(&entry, source).expect("write entry");

let compile = Command::new(perry_bin())
.current_dir(dir)
.arg("compile")
.arg(&entry)
.arg("-o")
.arg(&output)
.arg("--no-cache")
.output()
.expect("run perry compile");
assert!(
compile.status.success(),
"perry compile failed\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&compile.stdout),
String::from_utf8_lossy(&compile.stderr)
);

let mut child = Command::new(&output)
.current_dir(dir)
.stdout(std::process::Stdio::piped())
.stderr(std::process::Stdio::piped())
.spawn()
.expect("spawn compiled binary");

let timeout = Duration::from_secs(30);
let start = Instant::now();
loop {
match child.try_wait().expect("try_wait") {
Some(status) => {
let out = child.wait_with_output().expect("wait_with_output");
assert!(
status.success(),
"compiled binary failed (exit {:?})\nstdout:\n{}\nstderr:\n{}",
status.code(),
String::from_utf8_lossy(&out.stdout),
String::from_utf8_lossy(&out.stderr)
);
return String::from_utf8_lossy(&out.stdout).into_owned();
}
None => {
if start.elapsed() > timeout {
let _ = child.kill();
let _ = child.wait();
panic!(
"compiled binary did not finish within {:?} — a user \
`charAt`/`charCodeAt`/`codePointAt` method was mis-lowered \
to the String builtin (regression)",
timeout
);
}
std::thread::sleep(Duration::from_millis(50));
}
}
}
}

/// A plain class method named `charAt` must call the user's method — not
/// `String.prototype.charAt` on a `"[object Object]"`-coerced receiver.
#[test]
fn plain_method_named_char_at_is_user_method() {
let dir = tempfile::tempdir().expect("tempdir");
let stdout = compile_and_run(
dir.path(),
r#"
class Buf {
buffer = "hello";
pos = 0;
charAt(n: number) { return this.buffer[this.pos + n]; }
}
const b = new Buf();
console.log(b.charAt(0) + b.charAt(1) + b.charAt(4));
"#,
);
assert_eq!(stdout, "heo\n");
}

/// A field holding a function value (`charAt = (n) => …`) shadows the builtin
/// the same way a method does — the receiver-class guard must treat instance
/// fields as defining the name, not just methods/getters.
#[test]
fn arrow_function_field_named_char_at_is_user_function() {
let dir = tempfile::tempdir().expect("tempdir");
let stdout = compile_and_run(
dir.path(),
r#"
class Buf {
buffer = "hello";
pos = 0;
charAt = (n: number) => this.buffer[this.pos + n];
}
const b = new Buf();
console.log(b.charAt(0) + b.charAt(1) + b.charAt(4));
"#,
);
assert_eq!(stdout, "heo\n");
}

/// `charCodeAt` and `codePointAt` are affected by the same arity-gate no-op.
#[test]
fn plain_methods_named_char_code_at_and_code_point_at_are_user_methods() {
let dir = tempfile::tempdir().expect("tempdir");
let stdout = compile_and_run(
dir.path(),
r#"
class C {
data = [10, 20, 30];
charCodeAt(i: number) { return this.data[i] + 1; }
codePointAt(i: number) { return this.data[i] * 2; }
}
const c = new C();
console.log(c.charCodeAt(0) + "," + c.codePointAt(2));
"#,
);
assert_eq!(stdout, "11,60\n");
}

/// A genuine string receiver must still get `String.prototype.charAt` — the
/// fix must not break real string char access.
#[test]
fn real_string_char_at_still_works() {
let dir = tempfile::tempdir().expect("tempdir");
let stdout = compile_and_run(
dir.path(),
r#"
const s = "world";
console.log(s.charAt(0) + s.charAt(4) + "|" + s.charCodeAt(1));
"#,
);
assert_eq!(stdout, "wd|111\n");
}

/// The exact hang shape: a generator class method using its own `charAt` in a
/// yielding `switch` loop that only terminates when `charAt` returns the real
/// chars. Mirrors the `yaml` `Lexer` indicator scan.
#[test]
fn generator_using_own_char_at_in_switch_loop_terminates() {
let dir = tempfile::tempdir().expect("tempdir");
let stdout = compile_and_run(
dir.path(),
r#"
class Lexer {
buffer = "";
pos = 0;
charAt(n: number) { return this.buffer[this.pos + n]; }
*lex(): Generator<string, number, unknown> {
let count = 0;
loop: while (true) {
switch (this.charAt(0)) {
case 'a':
yield 'A'; this.pos++; count++; continue loop;
case ':':
yield 'C'; this.pos++; count++; continue loop;
}
break loop;
}
return count;
}
}
const lx = new Lexer();
lx.buffer = "aa:aZ";
const out: string[] = [];
const g = lx.lex();
let r = g.next();
while (!r.done) { out.push(r.value as string); r = g.next(); }
console.log(out.join(",") + "|count=" + r.value + "|pos=" + lx.pos);
"#,
);
assert_eq!(stdout, "A,A,C,A|count=4|pos=4\n");
}
Loading