feat: add isClosed() method to Deno.FsFile

[adarsh-priydarshi-5646]

Adds the isClosed() method to Deno.FsFile for querying close status.

Fixes #31341

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

Walkthrough

This PR exposes the closed state of FsFile by adding a public isClosed(): boolean to the FsFile interface and implementation. FsFile gains a private boolean field (#closed) initialized false; close() and the disposal path ([Symbol.dispose]) set this flag to true when the file is successfully closed. Tests were added to verify isClosed() across async, sync, using-block, and manual-close-in-using scenarios.

Sequence Diagram(s)

sequenceDiagram
    participant App as Application
    participant FsFile
    participant Core as Deno Core

    App->>FsFile: open()
    FsFile->>FsFile: `#closed` = false

    rect rgb(220,245,220)
    Note over App,FsFile: File is open
    App->>FsFile: isClosed()
    FsFile-->>App: false
    end

    App->>FsFile: close()
    FsFile->>Core: core.close(rid)
    Core-->>FsFile: success
    FsFile->>FsFile: `#closed` = true

    rect rgb(245,220,220)
    Note over App,FsFile: File is closed
    App->>FsFile: isClosed()
    FsFile-->>App: true
    end

    alt Using block disposal
        App->>FsFile: [Symbol.dispose]()
        FsFile->>Core: core.tryClose(rid)
        Core-->>FsFile: success
        FsFile->>FsFile: `#closed` = true
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

• Review alignment between lib.deno.ns.d.ts and the FsFile implementation (signature and visibility).
• Verify #closed is set only on successful close paths (close() and [Symbol.dispose] when core.tryClose succeeds).
• Check tests in tests/unit/files_test.ts cover all intended scenarios and that they correctly depend on the new behavior.

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: adding an isClosed() method to Deno.FsFile, which matches the changeset exactly.
Description check	✅ Passed	The description directly relates to the changeset by stating the isClosed() method is added to Deno.FsFile and references the linked issue #31341.
Linked Issues check	✅ Passed	The PR implements the exact requirement from issue #31341: adding an isClosed() method to Deno.FsFile for querying close status, with tests covering async, sync, and using block scenarios.
Out of Scope Changes check	✅ Passed	All changes are scoped to implementing the isClosed() method: type definitions, implementation with private state tracking, and comprehensive tests. No extraneous modifications detected.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

ext/fs/30_fs.js (1)

652-655: Consider making close() idempotent.

Currently, calling close() twice throws an error from core.close(). With isClosed() now available, consider checking the flag first to make close() idempotent and more user-friendly.

Apply this diff:

 close() {
+  if (this.#closed) return;
   core.close(this.#rid);
   this.#closed = true;
 }

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2de285c and 9344dc5.

📒 Files selected for processing (1)

• ext/fs/30_fs.js (3 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)

• GitHub Check: test debug windows-x86_64
• GitHub Check: test debug linux-aarch64
• GitHub Check: test debug linux-x86_64
• GitHub Check: test debug macos-aarch64
• GitHub Check: test release linux-x86_64
• GitHub Check: test debug macos-x86_64
• GitHub Check: lint debug windows-x86_64
• GitHub Check: build libs
• GitHub Check: lint debug linux-x86_64
• GitHub Check: lint debug macos-x86_64

🔇 Additional comments (3)

ext/fs/30_fs.js (3)

587-587: LGTM!

Field initialization follows the existing pattern and correctly defaults to false.

---

657-659: LGTM!

The implementation is correct and straightforward.

---

726-729: LGTM!

Setting #closed = true after tryClose is appropriate for the disposal pattern. Even if the resource was already closed, marking it as closed after disposal is the correct behavior.

[adarsh-priydarshi-5646]

Hi @fraidev, @marvinhagemeister, and @Tango992, please review.

[Tango992]

Thanks, looks good to me. Though I'd hand it to @bartlomieju on merging this one

[0f-0b]

Why should FsFile have a isClosed method? In what case would a user not know when a file is closed?

[adarsh-priydarshi-5646]

Why should FsFile have a isClosed method? In what case would a user not know when a file is closed?

It's useful when files are passed between functions or stored in data structures where the close state isn't immediately obvious. Similar to how streams have a closed property for checking state without triggering errors.

[0f-0b]

It's useful when files are passed between functions or stored in data structures where the close state isn't immediately obvious.

Even when you pass a file around like this, there should be one piece of code that owns the file and is responsible for closing it when it's no longer in use.

Similar to how streams have a closed property for checking state without triggering errors.

It makes sense that streams have a closed property, because a stream has two ends, and it's sometimes useful to know when the other end closes the stream. A file doesn't have two ends.

[adarsh-priydarshi-5646]

It's useful when files are passed between functions or stored in data structures where the close state isn't immediately obvious.

Even when you pass a file around like this, there should be one piece of code that owns the file and is responsible for closing it when it's no longer in use.

Similar to how streams have a closed property for checking state without triggering errors.

It makes sense that streams have a closed property, because a stream has two ends, and it's sometimes useful to know when the other end closes the stream. A file doesn't have two ends.

Fair point. The original issue #31341 requested this feature. If maintainers feel it's not needed, they can close both the issue and PR. I'm open to their decision.

[adarsh-priydarshi-5646]

@bartlomieju @Tango992 Please review.