fix(hir,codegen): route class-extends userland stream-shim ctor through its real constructor (winston)

crates/perry-codegen/src/expr/this_super_call.rs

@@ -245,29 +245,47 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
                     // semantics (Error sets this.message + this.name; streams allocate
                     // a registry handle). Anything else with an extends_expr is a
                     // real runtime-value parent and routes through this dispatch.
-                    let is_builtin_parent_name =
-                        matches!(
-                            parent_name.as_str(),
-                            "Error"
-                                | "TypeError"
-                                | "RangeError"
-                                | "ReferenceError"
-                                | "SyntaxError"
-                                | "URIError"
-                                | "EvalError"
-                                | "AggregateError"
-                                | "Readable"
-                                | "Writable"
-                                | "Duplex"
-                                | "Transform"
-                                | "ReadableStream"
-                                | "WritableStream"
-                                | "TransformStream"
-                                | "Request"
-                                | "Response"
-                                | "Event"
-                                | "CustomEvent"
-                        ) || is_other_builtin_constructor_name(parent_name.as_str());
+                    // The classic node:stream / Web-Streams names are only the
+                    // genuine built-in parents when HIR did NOT capture an
+                    // `extends_expr`. When it did, the parent is a userland
+                    // stream-shim value (e.g. readable-stream's `Transform`,
+                    // winston's `class Logger extends Transform`) whose real
+                    // constructor — which sets `this._readableState`,
+                    // `this._writableState`, `this._transformState` — must run.
+                    // HIR's `is_genuine_node_stream_parent` gate only leaves
+                    // `extends_expr` set for the non-builtin case (the genuine
+                    // node:stream import keeps `native_extends` + no
+                    // `extends_expr`), so deferring to the dynamic dispatch here
+                    // whenever an `extends_expr` exists is safe.
+                    let has_extends_expr = current_class.extends_expr.is_some();
+                    let is_stream_family_name = matches!(
+                        parent_name.as_str(),
+                        "Readable"
+                            | "Writable"
+                            | "Duplex"
+                            | "Transform"
+                            | "ReadableStream"
+                            | "WritableStream"
+                            | "TransformStream"
+                    );
+                    let is_builtin_parent_name = (matches!(
+                        parent_name.as_str(),
+                        "Error"
+                            | "TypeError"
+                            | "RangeError"
+                            | "ReferenceError"
+                            | "SyntaxError"
+                            | "URIError"
+                            | "EvalError"
+                            | "AggregateError"
+                            | "Request"
+                            | "Response"
+                            | "Event"
+                            | "CustomEvent"
+                    ) || (is_stream_family_name
+                        && !has_extends_expr)
+                        || is_other_builtin_constructor_name(parent_name.as_str()))
+                        && !(is_stream_family_name && has_extends_expr);
                     if !is_builtin_parent_name {
                         if let Some(extends_expr) = current_class.extends_expr.as_deref() {
                             // Lower the super-call args first so they get fresh slots

crates/perry-hir/src/lower_decl/class_decl.rs

@@ -11,6 +11,39 @@ use crate::lower::{
 use crate::lower_patterns::*;
 use crate::lower_types::*;
 
+/// The four classic node:stream base-class names (`Readable`/`Writable`/
+/// `Duplex`/`Transform`). When a class extends a parent with one of these
+/// textual names, perry routes `super()` to the native `js_node_stream_*`
+/// shim (which installs the native stream surface but never sets the
+/// `_readableState`/`_writableState`/`_transformState` objects). That is only
+/// correct when the name actually resolves to the `node:stream` builtin —
+/// i.e. it was imported from `stream`/`node:stream`, in which case the import
+/// machinery registered it as the `stream` native module
+/// (`register_native_module(binding, "stream", Some(name))`, see
+/// `var_decl_sources::register_destructured_stream_ctors` and the import
+/// arms in `lower/module_decl.rs`).
+///
+/// The same textual name can instead be a userland binding from a
+/// stream-shim npm package — `const { Transform } =
+/// require('readable-stream')` in winston's `logger.js`, where
+/// `class Logger extends Transform` then reads `this._readableState.pipes`
+/// directly. Such a binding never registers as the `stream` native module
+/// (readable-stream is not a node builtin), so the package's real constructor
+/// must run instead. Returning `false` here lets the unknown-Ident arm
+/// capture the parent as `extends_expr` and route `super()` through the
+/// dynamic-parent path (`js_register_class_parent_dynamic` + the
+/// `js_fetch_or_value_super` dispatch), which runs the real `Transform`
+/// function body on the subclass instance.
+fn is_genuine_node_stream_parent(ctx: &LoweringContext, name: &str) -> bool {
+    if !matches!(name, "Readable" | "Writable" | "Duplex" | "Transform") {
+        return false;
+    }
+    // Only the genuine `node:stream` builtin import registers these names as
+    // the `stream` native module. Anything else (a userland require/import of
+    // a stream-shim package, or an unbound reference) is not the builtin.
+    matches!(ctx.lookup_native_module(name), Some(("stream", _)))
+}
+
 use super::*;
 
 fn generic_computed_member_key<'a>(
@@ -361,10 +394,16 @@ pub fn lower_class_decl(
                 // native stream methods directly onto `this`, were already
                 // built; this is the missing HIR wiring. Keep in lockstep
                 // with the parallel arm in `lower_class_from_ast` below.
-                "Readable" => Some(("node_stream".to_string(), "Readable".to_string())),
-                "Writable" => Some(("node_stream".to_string(), "Writable".to_string())),
-                "Duplex" => Some(("node_stream".to_string(), "Duplex".to_string())),
-                "Transform" => Some(("node_stream".to_string(), "Transform".to_string())),
+                //
+                // Gate the classic node:stream names on `is_genuine_node_stream_parent`
+                // so a userland stream-shim binding (readable-stream's
+                // `Transform`, winston) falls through to the dynamic
+                // `extends_expr` parent path and runs its real constructor.
+                "Readable" | "Writable" | "Duplex" | "Transform"
+                    if is_genuine_node_stream_parent(ctx, &parent_name) =>
+                {
+                    Some(("node_stream".to_string(), parent_name.clone()))
+                }
                 _ => None,
             };
             if native_parent.is_some() {
@@ -1329,11 +1368,15 @@ pub fn lower_class_from_ast(
                     "TransformStream".to_string(),
                 )),
                 // #1545: classic node:stream base classes — keep in lockstep
-                // with the parallel arm in `lower_class_decl` above.
-                "Readable" => Some(("node_stream".to_string(), "Readable".to_string())),
-                "Writable" => Some(("node_stream".to_string(), "Writable".to_string())),
-                "Duplex" => Some(("node_stream".to_string(), "Duplex".to_string())),
-                "Transform" => Some(("node_stream".to_string(), "Transform".to_string())),
+                // with the parallel arm in `lower_class_decl` above. Gated on
+                // `is_genuine_node_stream_parent` so a userland stream-shim
+                // binding (readable-stream's `Transform`) falls through to the
+                // dynamic `extends_expr` parent path.
+                "Readable" | "Writable" | "Duplex" | "Transform"
+                    if is_genuine_node_stream_parent(ctx, &parent_name) =>
+                {
+                    Some(("node_stream".to_string(), parent_name.clone()))
+                }
                 _ => None,
             };
             if native_parent.is_some() {

test-files/test_gap_class_extends_userland_stream_shim.ts

@@ -0,0 +1,79 @@
+// winston native-compile wall (branch fix/winston-stream-subclass-state):
+// `class Logger extends Transform` where `Transform` is NOT node:stream's
+// builtin but a userland stream-shim's ES5 function constructor
+// (`const { Transform } = require('readable-stream')`). readable-stream's
+// hierarchy is `Transform -> Duplex -> Readable` wired via ES5
+// `inherits()` (Object.create on the prototype), and the Readable ctor sets
+// `this._readableState = new ReadableState(...)`. winston's logger.js then
+// reads `const { pipes } = this._readableState`.
+//
+// Before the fix, perry routed `super()` for ANY parent textually named
+// `Transform`/`Readable`/`Writable`/`Duplex` to the native node:stream shim
+// (which never sets `_readableState`), so `this._readableState` was
+// `undefined` and the destructure threw
+// "Cannot convert undefined or null to object". The fix gates the native
+// routing on `is_genuine_node_stream_parent` (the name must resolve to the
+// `stream` native module) so a userland binding falls through to the
+// dynamic `extends_expr` parent path, which runs the real constructor chain.
+
+function inherits(ctor: any, superCtor: any) {
+  ctor.super_ = superCtor;
+  ctor.prototype = Object.create(superCtor.prototype, {
+    constructor: {
+      value: ctor,
+      enumerable: false,
+      writable: true,
+      configurable: true,
+    },
+  });
+}
+
+// Base of the chain: sets the state objects the userland code reads.
+function Readable(this: any, options: any) {
+  this._readableState = { pipes: [], objectMode: !!(options && options.objectMode) };
+  this.readable = true;
+}
+
+function Writable(this: any, options: any) {
+  this._writableState = { finished: false };
+  this.writable = true;
+}
+
+function Duplex(this: any, options: any) {
+  Readable.call(this, options);
+  Writable.call(this, options);
+  this.allowHalfOpen = true;
+}
+inherits(Duplex, Readable);
+
+function Transform(this: any, options: any) {
+  Duplex.call(this, options);
+  this._transformState = { transforming: false };
+  // Mirror readable-stream: read the state set by the Readable ctor.
+  this._readableState.needReadable = true;
+}
+inherits(Transform, Duplex);
+
+// User ES6 class extending the userland ES5 `Transform`.
+class Logger extends Transform {
+  configured: boolean;
+  constructor(options: any) {
+    super({ objectMode: true });
+    this.configured = true;
+  }
+  // Mirror winston logger.js line 695: read this._readableState directly.
+  pipeCount(): number {
+    const { pipes } = (this as any)._readableState;
+    return pipes.length;
+  }
+}
+
+const l: any = new Logger({});
+console.log("readableState defined:", l._readableState !== undefined);
+console.log("readableState.objectMode:", l._readableState.objectMode);
+console.log("readableState.needReadable:", l._readableState.needReadable);
+console.log("writableState defined:", l._writableState !== undefined);
+console.log("transformState defined:", l._transformState !== undefined);
+console.log("allowHalfOpen:", l.allowHalfOpen);
+console.log("configured:", l.configured);
+console.log("pipeCount:", l.pipeCount());