fix(fs): restore fs/promises parity (#6785)

[proggeramlug]

Closes #6785.

What changed

• Detect the exact ExternFuncRef { name: "glob" } HIR shape produced by named node:fs/promises imports, so auto-optimized builds retain perry-runtime/regex-engine.
• Emit js_node_submod_install_fs_promises() before materializing the parent fs.promises property. This populates callable namespace fields for destructuring/indirect calls while preserving dead stripping for programs that only use synchronous node:fs.
• Add regressions for both the real module-collection path and the generated installer call.

Baseline evidence

The issue's abbreviated report hid the actual deltas:

• glob/async-iterator: the iterator shape was present, but every valid glob yielded no matches and null options were not validated because the regex-backed implementation was compiled out.
• imports/parent-promises-property: fs.promises was an object, but open, readFile, writeFile, mkdir, and rm all read as undefined; calling open then rejected with TypeError: value is not a function.

Validation

• cargo fmt --all -- --check
• cargo test -p perry-codegen --test native_proof_regressions fs_parent_promises_property_installs_submodule_dispatch -- --exact
• CARGO_PROFILE_DEV_DEBUG=0 CARGO_INCREMENTAL=0 cargo check -p perry --bin perry
• A forced old-compiler auto-build reached the expected perry-runtime feature set (dyn-eval, full, regex-engine, stdlib), confirming the runtime glob path selected by this detector. The shared volume filled while LLVM emitted the final static archives, so the complete executable comparison is left to CI.

Summary by CodeRabbit

• Bug Fixes

  • Fixed node:fs/promises.glob in auto-optimized builds.
  • Ensured fs.promises methods are available when accessed through the parent fs.promises namespace.
  • Improved detection of node:fs/promises.glob usage, including imports within class methods, so the required glob engine is enabled reliably.

• Tests

  • Added regression coverage for valid and unrelated glob imports.
  • Added coverage confirming correct fs.promises property resolution.

[coderabbitai]

📝 Walkthrough

Walkthrough

The compiler now installs the fs/promises submodule before resolving fs.promises and retains the regex engine for named node:fs/promises glob imports, including class usage. Regression tests and a changelog entry cover both fixes.

Changes

fs.promises parity

Layer / File(s)	Summary
fs.promises submodule dispatch crates/perry-codegen/src/expr/property_get.rs, crates/perry-codegen/src/expr/property_get/tests.rs, changelog.d/6785-fs-promises-parity.md	Property access installs the fs/promises submodule before namespace resolution, with LLVM IR ordering coverage and a changelog entry.
Named glob feature detection crates/perry/src/commands/compile/collect_modules/feature_detect.rs, crates/perry/src/commands/compile/collect_modules/tests.rs	Structured import provenance detects node:fs/promises named glob imports, includes class bodies in scanning, and tests positive and unrelated-import cases.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant EntryModule
  participant collect_modules
  participant CompilationContext
  EntryModule->>collect_modules: provide lowered imports and class bodies
  collect_modules->>CompilationContext: enable uses_regex for fs/promises glob

Loading

Possibly related PRs

• PerryTS/perry#6653: Complements the fs.promises property-get change through runtime builtin-module registry routing.
• PerryTS/perry#6818: Overlaps in regex-runtime feature detection logic.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately summarizes the fs/promises parity fix.
Description check	✅ Passed	The description covers the change summary, issue closure, and validation, though it doesn't mirror every template section exactly.
Linked Issues check	✅ Passed	[#6785] The PR addresses both listed parity failures by preserving regex-engine for fs/promises glob and installing fs.promises before resolution.
Out of Scope Changes check	✅ Passed	No clear out-of-scope changes stand out; the edits stay focused on fs/promises parity and its regression tests.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (1)

crates/perry-codegen/tests/native_proof_regressions.rs (1)

2076-2093: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Prefer a PR-visible test location for this regression.

This file is under crates/perry-codegen/tests, an integration suite that does not run on every PR. Move the assertion into cargo-test-visible unit coverage, or explicitly wire this target into the PR test matrix.

As per coding guidelines, integration suites under crates/*/tests/*.rs do not run on every PR; prefer acceptance coverage in cargo-test-visible unit tests.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-codegen/tests/native_proof_regressions.rs` around lines 2076 -
2093, Move the regression test
fs_parent_promises_property_installs_submodule_dispatch from the
crates/perry-codegen/tests integration suite into cargo-test-visible unit
coverage, preserving both IR assertions for fs.promises submodule installation
and native-module property resolution; alternatively, explicitly add this
integration target to the PR test matrix.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/perry-codegen/tests/native_proof_regressions.rs`:
- Around line 2085-2091: Update the assertions in the fs.promises regression
test to compare the positions of the `js_node_submod_install_fs_promises` and
`js_native_module_property_by_name` calls in the generated IR. Assert that the
installation call occurs before property resolution, while preserving the
existing presence checks and diagnostic messages.

In `@crates/perry/src/commands/compile/collect_modules/feature_detect.rs`:
- Around line 131-136: Update the feature-detection logic surrounding the HIR
debug check to identify glob references using structured HIR or module metadata
that confirms the source is node:fs/promises, rather than matching the bare
name: "glob" token. Ensure unrelated external imports such as ./util do not
enable the optional regex engine, while genuine node:fs/promises glob imports
continue to do so.
- Around line 131-136: Update the HIR serialization used by the regex scan in
the feature-detection logic to include hir_module.classes alongside init and
functions. Ensure class bodies, including instance and static methods, are
represented before checking for the exact `name: "glob"` marker so
ctx.uses_regex is set for node:fs/promises.glob usage.

---

Nitpick comments:
In `@crates/perry-codegen/tests/native_proof_regressions.rs`:
- Around line 2076-2093: Move the regression test
fs_parent_promises_property_installs_submodule_dispatch from the
crates/perry-codegen/tests integration suite into cargo-test-visible unit
coverage, preserving both IR assertions for fs.promises submodule installation
and native-module property resolution; alternatively, explicitly add this
integration target to the PR test matrix.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 22795d41-14be-4a82-a90c-c541aff873e4

📥 Commits

Reviewing files that changed from the base of the PR and between 20f1975 and 4dc0308.

📒 Files selected for processing (5)

• changelog.d/6785-fs-promises-parity.md
• crates/perry-codegen/src/expr/property_get.rs
• crates/perry-codegen/tests/native_proof_regressions.rs
• crates/perry/src/commands/compile/collect_modules/feature_detect.rs
• crates/perry/src/commands/compile/collect_modules/tests.rs

[coderabbitai]

🧹 Nitpick comments (1)

crates/perry/src/commands/compile/collect_modules/tests.rs (1)

140-182: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a regression that actually depends on scanning class bodies.

This assertion stays true if hir_module.classes is removed from the scan because imports_fs_promises_glob detects the import independently. Add a separate class-only HIR trigger—such as a namespace fs.promises.glob property read in Scanner.scan—so the test fails if class traversal regresses.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry/src/commands/compile/collect_modules/tests.rs` around lines 140
- 182, Add a class-only HIR trigger to
fs_promises_named_glob_in_class_enables_regex_engine by referencing a
namespace-style fs.promises.glob property inside Scanner.scan, ensuring the
assertion depends on class-body traversal rather than only
imports_fs_promises_glob. Keep the existing aliased import and regex assertion
intact, and ensure the added expression is the sole trigger that would fail if
hir_module.classes scanning is removed.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@crates/perry/src/commands/compile/collect_modules/tests.rs`:
- Around line 140-182: Add a class-only HIR trigger to
fs_promises_named_glob_in_class_enables_regex_engine by referencing a
namespace-style fs.promises.glob property inside Scanner.scan, ensuring the
assertion depends on class-body traversal rather than only
imports_fs_promises_glob. Keep the existing aliased import and regex assertion
intact, and ensure the added expression is the sole trigger that would fail if
hir_module.classes scanning is removed.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9faf39fd-3eca-43f8-b2e8-bc2307c8f9b9

📥 Commits

Reviewing files that changed from the base of the PR and between 4dc0308 and f653e2c.

📒 Files selected for processing (5)

• changelog.d/6785-fs-promises-parity.md
• crates/perry-codegen/src/expr/property_get.rs
• crates/perry-codegen/src/expr/property_get/tests.rs
• crates/perry/src/commands/compile/collect_modules/feature_detect.rs
• crates/perry/src/commands/compile/collect_modules/tests.rs

🚧 Files skipped from review as they are similar to previous changes (2)

• changelog.d/6785-fs-promises-parity.md
• crates/perry-codegen/src/expr/property_get.rs