From f653e2cb3d4b0c8894fb88b892b7f1271463fbfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 24 Jul 2026 04:29:51 +0200 Subject: [PATCH] fix(fs): restore promises namespace parity --- changelog.d/6785-fs-promises-parity.md | 1 + crates/perry-codegen/src/expr/property_get.rs | 19 +++- ...ullish_read_location_tests.rs => tests.rs} | 36 +++++++- .../compile/collect_modules/feature_detect.rs | 61 +++++++++++- .../commands/compile/collect_modules/tests.rs | 92 +++++++++++++++++++ 5 files changed, 200 insertions(+), 9 deletions(-) create mode 100644 changelog.d/6785-fs-promises-parity.md rename crates/perry-codegen/src/expr/property_get/{nullish_read_location_tests.rs => tests.rs} (79%) diff --git a/changelog.d/6785-fs-promises-parity.md b/changelog.d/6785-fs-promises-parity.md new file mode 100644 index 0000000000..7b5b5d5179 --- /dev/null +++ b/changelog.d/6785-fs-promises-parity.md @@ -0,0 +1 @@ +Fix `node:fs/promises.glob` in auto-optimized builds and populate callable methods on the parent `fs.promises` namespace. diff --git a/crates/perry-codegen/src/expr/property_get.rs b/crates/perry-codegen/src/expr/property_get.rs index 4fe02a0864..0d16756817 100644 --- a/crates/perry-codegen/src/expr/property_get.rs +++ b/crates/perry-codegen/src/expr/property_get.rs @@ -28,7 +28,7 @@ mod generic_dispatch; mod globalget; mod helpers; #[cfg(test)] -mod nullish_read_location_tests; +mod tests; pub(crate) use generic_dispatch::lower_generic_property_get; pub(crate) use globalget::lower_globalget_property; @@ -540,6 +540,23 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { if let Some(install_sym) = crate::nm_install::nm_install_symbol(module_name) { ctx.block().call_void(install_sym, &[]); } + // `fs.promises` is backed by the `fs_promises` submodule + // registry, not the parent fs dispatch bucket. Direct + // `node:fs/promises` imports emit their submodule installer at + // the import site, but a parent-property read has no such + // site. Install the precise submodule here before + // `js_native_module_property_by_name` asks the runtime for its + // namespace; otherwise it receives the unresolved empty-object + // stub and destructuring yields `undefined` for every method. + // + // Keeping this in codegen (rather than making the runtime + // parent module unconditionally retain fs/promises) preserves + // auto-optimize dead stripping for programs that only use + // synchronous `node:fs`. + if module_name == "fs" && property == "promises" { + ctx.block() + .call_void("js_node_submod_install_fs_promises", &[]); + } if module_name == "process" && property == "version" { let blk = ctx.block(); let handle = blk.call(I64, "js_process_version", &[]); diff --git a/crates/perry-codegen/src/expr/property_get/nullish_read_location_tests.rs b/crates/perry-codegen/src/expr/property_get/tests.rs similarity index 79% rename from crates/perry-codegen/src/expr/property_get/nullish_read_location_tests.rs rename to crates/perry-codegen/src/expr/property_get/tests.rs index a0b0e552a5..7f81613c62 100644 --- a/crates/perry-codegen/src/expr/property_get/nullish_read_location_tests.rs +++ b/crates/perry-codegen/src/expr/property_get/tests.rs @@ -1,9 +1,10 @@ -//! #5247 — cargo-test-visible coverage for the property-read-on-nullish source -//! location. The integration twin (`crates/perry/tests/ +//! Cargo-test-visible property-get codegen regressions. +//! +//! #5247's integration twin (`crates/perry/tests/ //! issue_5247_property_read_source_location.rs`) compiles + runs a real program -//! and only executes on nightly/tag workflows; this unit test asserts the -//! codegen contract directly on the emitted LLVM IR so it runs on every PR -//! (#5960 guideline). +//! and only executes on nightly/tag workflows; the tests here assert codegen +//! contracts directly on emitted LLVM IR so they run on every PR (#5960 +//! guideline). //! //! Contract: a general `Expr::PropertyGet` carrying a non-zero `byte_offset` //! emits a `js_set_call_location` call in `lower_generic_property_get` under a @@ -120,3 +121,28 @@ fn no_call_location_without_debug_symbols() { "no js_set_call_location CALL should be emitted without --debug-symbols:\n{ir}" ); } + +#[test] +fn fs_parent_promises_property_installs_before_resolution() { + let mut module = Module::new("fs_parent_promises_property.ts"); + module.init = vec![Stmt::Return(Some(Expr::PropertyGet { + object: Box::new(Expr::NativeModuleRef("fs".to_string())), + property: "promises".to_string(), + byte_offset: 0, + }))]; + + let ir = String::from_utf8(compile_module(&module, ir_opts(false, None)).unwrap()) + .expect("LLVM IR should be UTF-8"); + let install = ir + .find("call void @js_node_submod_install_fs_promises()") + .unwrap_or_else(|| panic!("fs.promises must emit its submodule installer:\n{ir}")); + let resolve = ir + .find("call double @js_native_module_property_by_name") + .unwrap_or_else(|| { + panic!("fs.promises must use the native-module property resolver:\n{ir}") + }); + assert!( + install < resolve, + "fs.promises submodule installation must precede property resolution:\n{ir}" + ); +} diff --git a/crates/perry/src/commands/compile/collect_modules/feature_detect.rs b/crates/perry/src/commands/compile/collect_modules/feature_detect.rs index 8bf5e01a65..2acd0ea704 100644 --- a/crates/perry/src/commands/compile/collect_modules/feature_detect.rs +++ b/crates/perry/src/commands/compile/collect_modules/feature_detect.rs @@ -29,6 +29,23 @@ fn debug_hir_uses_regex(hir_debug: &str) -> bool { || hir_debug.contains("property: \"globSync\"") } +fn imports_fs_promises_glob(hir_module: &perry_hir::Module) -> bool { + hir_module.imports.iter().any(|import| { + !import.type_only + && import + .source + .strip_prefix("node:") + .unwrap_or(&import.source) + == "fs/promises" + && import.specifiers.iter().any(|specifier| { + matches!( + specifier, + perry_hir::ImportSpecifier::Named { imported, .. } if imported == "glob" + ) + }) + }) +} + /// Inspect a lowered module and set the optional-feature gates it needs. pub(super) fn detect_optional_feature_usage( ctx: &mut CompilationContext, @@ -140,8 +157,18 @@ pub(super) fn detect_optional_feature_usage( // correctness, cost); the goal is zero false negatives. `eval` is // non-functional in Perry so it can't create a regex at runtime. { - let hir_debug: String = format!("{:?}{:?}", &hir_module.init, &hir_module.functions); - if debug_hir_uses_regex(&hir_debug) { + // Class methods and static initializers live under `classes`, not in + // `functions`; include them so a regex/glob use there cannot be + // stripped from an auto-optimized build. + let hir_debug = format!( + "{:?}{:?}{:?}", + &hir_module.init, &hir_module.functions, &hir_module.classes + ); + // A named import lowers to an `ExternFuncRef` that carries only its + // local binding name. Use the structured import record for provenance + // instead of treating every unrelated external named `glob` as + // `node:fs/promises.glob`. + if debug_hir_uses_regex(&hir_debug) || imports_fs_promises_glob(hir_module) { ctx.uses_regex = true; } } @@ -346,7 +373,8 @@ pub(super) fn detect_optional_feature_usage( #[cfg(test)] mod tests { - use super::debug_hir_uses_regex; + use super::{debug_hir_uses_regex, imports_fs_promises_glob}; + use perry_hir::{Import, ImportSpecifier, Module, ModuleKind}; #[test] fn regex_gate_detects_static_and_dynamic_path_matches_glob() { @@ -357,4 +385,31 @@ mod tests { r#"NativeMethodCall { module: String("path.win32"), method: String("matchesGlob"), args: [] }"# )); } + + #[test] + fn fs_promises_glob_gate_uses_import_provenance() { + let mut module = Module::new("entry.ts"); + module.imports.push(Import { + source: "node:fs/promises".to_string(), + specifiers: vec![ImportSpecifier::Named { + imported: "glob".to_string(), + local: "findFiles".to_string(), + }], + is_native: true, + module_kind: ModuleKind::NativeCompiled, + resolved_path: None, + type_only: false, + is_dynamic: false, + is_dynamic_target: false, + is_deferred_require: false, + is_adopted_require: false, + }); + assert!(imports_fs_promises_glob(&module)); + + module.imports[0].source = "./util".to_string(); + assert!( + !imports_fs_promises_glob(&module), + "an unrelated named glob import must not retain the regex engine" + ); + } } diff --git a/crates/perry/src/commands/compile/collect_modules/tests.rs b/crates/perry/src/commands/compile/collect_modules/tests.rs index 6c5da96cfe..762026f674 100644 --- a/crates/perry/src/commands/compile/collect_modules/tests.rs +++ b/crates/perry/src/commands/compile/collect_modules/tests.rs @@ -137,6 +137,98 @@ console.log(got); ); } +#[test] +fn fs_promises_named_glob_in_class_enables_regex_engine() { + let dir = tempfile::tempdir().expect("tempdir"); + let entry = dir.path().join("entry.ts"); + std::fs::write( + &entry, + r#" +import { glob as findFiles } from "node:fs/promises"; +class Scanner { + static scan() { + return findFiles("**/*.ts"); + } +} +const iterator = Scanner.scan(); +void iterator; +"#, + ) + .expect("write entry"); + + let mut ctx = CompilationContext::new(dir.path().to_path_buf()); + ctx.entry_canonical = Some(entry.canonicalize().unwrap()); + let mut visited = HashSet::new(); + let mut next_class_id: perry_hir::ClassId = 1; + let progress = VerboseProgress::new(OutputFormat::Text, 0); + + collect_modules( + &entry, + &mut ctx, + &mut visited, + OutputFormat::Text, + None, + &mut next_class_id, + false, + &progress, + None, + ) + .expect("collect modules"); + + assert!( + ctx.uses_regex, + "aliased fs/promises.glob in a class body must retain the regex-backed glob engine" + ); +} + +#[test] +fn unrelated_named_glob_does_not_enable_regex_engine() { + let dir = tempfile::tempdir().expect("tempdir"); + let entry = dir.path().join("entry.ts"); + std::fs::write( + dir.path().join("util.ts"), + r#" +export function glob(pattern: string): string { + return pattern; +} +"#, + ) + .expect("write dependency"); + std::fs::write( + &entry, + r#" +import { glob } from "./util"; +const value = glob("not-a-runtime-glob"); +void value; +"#, + ) + .expect("write entry"); + + let mut ctx = CompilationContext::new(dir.path().to_path_buf()); + ctx.entry_canonical = Some(entry.canonicalize().unwrap()); + let mut visited = HashSet::new(); + let mut next_class_id: perry_hir::ClassId = 1; + let progress = VerboseProgress::new(OutputFormat::Text, 0); + + collect_modules( + &entry, + &mut ctx, + &mut visited, + OutputFormat::Text, + None, + &mut next_class_id, + false, + &progress, + None, + ) + .expect("collect modules"); + + assert!( + !ctx.uses_regex, + "an unrelated external named glob binding must not retain the regex engine" + ); +} + #[cfg(unix)] #[test] fn symlinked_entry_resolves_relative_imports_from_lexical_path() {