lint(src/js): flag duplicate property reads between an if condition and its body

[robobun]

Adds a custom oxlint rule, bun/no-duplicate-conditional-property-access, enabled for src/js/** only.

What it flags

Any simple static property access that appears in an if condition and is read again in the body:

if (options.fragment != null) {
  fragment = Boolean(options.fragment);
}
if (options.cert) throwIfInvalid("cert", options.cert);
if (parser.maxHeaderPairs > 0) n = Math.min(n, parser.maxHeaderPairs);
if (typeof options.enc === "string") use(options.enc);

In src/js the object is often user-supplied, so each access may hit a getter or Proxy trap. The rule suggests reading the property into a local first:

const { fragment: fragmentOption } = options;
if (fragmentOption != null) {
  fragment = Boolean(fragmentOption);
}

How

Implemented as a jsPlugins entry in oxlint.json pointing at scripts/oxlint-plugins/bun.js. oxlint 1.70.0 does not ship no-restricted-syntax, and an ESQuery selector cannot express "the same member expression appears in the test and the body" anyway.

Match conditions:

• a simple static member chain (identifiers and this only; no computed access, no optional chaining) appears anywhere in the if test, other than as a callee or assignment target
• the same chain is read again in the consequent, ignoring occurrences inside nested functions and classes

The rule does not fire when the consequent:

• writes to the same member (assignment target, compound assignment, ++/--, or delete): caching it in a local would change semantics for reads after the write
• only calls the member as a method (obj.fn()): caching loses the receiver

Existing violations

All 363 existing instances across 62 files in src/js are fixed in this PR by reading the property into a local before the if (via destructuring or a const). http2.ts is already excluded from oxlint and is unchanged.

Tests

test/internal/oxlint-plugin-bun.test.ts runs oxlint against fixtures covering each match/skip condition and asserts bun run lint is clean on src/js.

bun bd test test/internal/oxlint-plugin-bun.test.ts
 6 pass
 0 fail

[robobun]

^{Updated 5:01 AM PT - Jun 20th, 2026}

✅ @robobun, your commit 07ca12bb24d320b9b7813b9024a2ef25c8d14963 passed in Build #63603! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 32522

That installs a local version of the PR into your bun-32522 executable, so you can run:

bun-32522 --bun

[github-actions]

Found 4 issues this PR may fix:

1. Intl.DateTimeFormat options object with getter keys causes TypeError in Bun but works in Node and Deno #20083 - Intl.DateTimeFormat options with getter keys causes TypeError — the exact class of bug (getter side effects on user-supplied options) this lint rule prevents
2. node:http http.request not accepting plain options object #26329 - node:http http.request not accepting plain options object — PR adds suppression comments to _http_client.ts for double-read patterns on options properties
3. node:http Agent ignores maxSockets #30270 - node:http Agent ignores maxSockets — PR adds suppression comments to _http_agent.ts where options properties are read twice
4. child_process.execFileSync uses stale PATH for command resolution when options.env is omitted #29237 - child_process.execFileSync uses stale PATH when options.env is omitted — PR adds suppression comments to child_process.ts for the double-read pattern on options

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #20083
Fixes #26329
Fixes #30270
Fixes #29237

🤖 Generated with Claude Code

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Introduces a custom oxlint plugin (scripts/oxlint-plugins/bun.js) that implements a bun/no-duplicate-conditional-property-access rule. This rule detects when the same static member-expression property (e.g., options.foo or this.bar.baz) is read in an if condition and again in that if body, reporting a diagnostic that suggests destructuring/caching to avoid redundant getter calls. The plugin is registered in oxlint.json with an override for src/js/** at error severity. To comply with the new rule, hundreds of sites across src/js have been refactored to cache property accesses into local variables before using them in conditional or assignment contexts. A comprehensive test suite validates the rule's detection, suppression, and end-to-end behavior.

Changes

Property caching refactor and oxlint rule

Layer / File(s)	Summary
Oxlint plugin and configuration scripts/oxlint-plugins/bun.js, oxlint.json	Implements noDuplicateConditionalPropertyAccess with helper functions for member-expression keying, nullish/conditional-check detection, write/call-target classification, and consequent traversal. Exports the rule under the bun plugin namespace. Registers the plugin in oxlint.json and configures a src/js/** override to enable the rule at error severity with explanatory comments.
Rule test fixtures and validation test/internal/oxlint-plugin-bun.test.ts	Establishes a test harness that creates temporary fixture files, generates oxlint.json, spawns bun x oxlint with GitHub formatting, and parses diagnostic output. Covers positive detection across conditional/nullish patterns, negative (allowed) cases, write-suppression semantics, inline disable comments, diagnostic message content including destructuring hints, and end-to-end validation against the actual src/js tree.
Built-in constructors and stream internals src/js/builtins/ReadableByteStreamInternals.ts, src/js/builtins/JSBufferConstructor.ts, src/js/builtins/JSBufferPrototype.ts, src/js/builtins/ReadableStreamInternals.ts, src/js/builtins/BundlerPlugin.ts	Refactors Buffer integer-reader methods, ReadableStream source/queue handling, and Bundler promise chain management to cache property accesses into locals before conditional checks and assignments.
SQL error constructors and database utilities src/js/internal/sql/errors.ts, src/js/internal/sql/shared.ts, src/js/internal/sql/sqlite.ts	Refactors SQL error constructors (PostgresError, SQLiteError, MySQLError) to destructure option fields into locals before assignments. Consolidates connection state reads and URL/adapter resolution logic via cached locals.
Stream constructors and consumer validation src/js/internal/streams/iter/consumers.ts, src/js/internal/streams/readable.ts, src/js/internal/streams/writable.ts, src/js/internal/streams/duplex.ts, src/js/internal/streams/transform.ts	Refactors stream constructors to destructure option hooks and cache limit/encoding/signal values into locals before validation and assignment. Improves readability while preserving existing control flow.
Stream flow control and data routing src/js/internal/streams/compose.ts, src/js/internal/streams/destroy.ts, src/js/internal/streams/end-of-stream.ts, src/js/internal/streams/iter/broadcast.ts, src/js/internal/streams/iter/pull.ts, src/js/internal/streams/iter/push.ts, src/js/internal/streams/iter/transform.ts, src/js/internal/streams/iter/utils.ts, src/js/internal/streams/legacy.ts, src/js/internal/streams/native-readable.ts	Refactors stream composition, destruction, end-of-stream detection, chunk batching, and consumer dispatching to cache batch/array lengths, signal references, and stream state before branching and write operations.
Inspection, URL utilities, and stream adapters src/js/internal/util/inspect.js, src/js/internal/url.ts, src/js/internal/webstreams_adapters.ts, src/js/internal/streams/utils.ts	Refactors inspect message construction, URL component caching, and web-stream adapter options by caching frequently-accessed object properties and descriptor values before conditional logic.
File system operations and globbing src/js/internal/fs/cp-sync.ts, src/js/internal/fs/glob.ts, src/js/internal/fs/streams.ts, src/js/internal/fs/watch.ts	Refactors error decoration, glob pattern matching, encoding initialization, and file watch handles by caching option values and pattern lengths into locals.
HTTP agent configuration src/js/node/_http_agent.ts	Refactors agent constructor validation, socket naming, request queueing, socket creation, and request removal by caching maxTotalSockets, socket pool references, and request metadata.
HTTP request/response lifecycle src/js/node/_http_client.ts	Refactors ClientRequest construction, proxy rewriting, header initialization, agent selection, connection initiation, parser state management, keep-alive release, and timeout handling by caching agent references, socket state, parser transitions, and request metadata throughout the request/response flow.
HTTP parser and server utilities src/js/node/_http_common.ts, src/js/node/_http_server.ts, src/js/node/_http_outgoing.ts	Refactors header completion, request dispatch, keep-alive header emission, content-length validation, and information headers by caching parser limits, header lengths, encoding values, and socket references.
HTTP/2 upgrade and TLS handshake src/js/node/_http2_upgrade.ts	Refactors mutual-TLS verification logic by caching request certificate flags and rejection settings before branching on handshake errors.
HTTPS session and tunnel management src/js/node/https.ts	Refactors proxy tunnel session caching, session reuse, and agent naming by caching agent keys, callback references, and TLS option values.
TLS socket and server security src/js/node/tls.ts	Refactors TLS handshake validation, secure-context creation, certificate identity checking, and server context management by caching ticket key lengths, certificate fields, and security parameters.
Network socket and connection management src/js/node/net.ts	Refactors socket error handling, authorization logic, SNI dispatch, connection setup, address resolution, and blocklist enforcement by caching error codes, socket handles, resolved addresses, and security flags.
Node.js assert and error utilities src/js/node/assert.ts, src/js/internal/assert/assertion_error.ts	Refactors error message construction and exception handling by caching error messages and reason strings before appending to formatted output.
Child process management src/js/node/child_process.ts	Refactors spawn/exec/fork operations, error handling, and environment setup by caching process options, error codes, stdio handles, and environment pairs into locals before validation and callback invocation.
DNS lookup and error handling src/js/node/dns.ts	Refactors DNS option validation and error handler by caching resolved flags, booleans, and error syscall values before conditional checks.
File system operations src/js/node/fs.ts, src/js/node/fs.promises.ts	Refactors read/write overload dispatch, pathname handling, and async copy by caching argument counts, pathname strings, and byte lengths into locals.
Query string parsing and encoding src/js/node/querystring.ts	Refactors encode/decode option handling by caching function overrides and maxKeys values before validation and substitution.
Readline terminal input and history src/js/node/readline.ts	Refactors terminal close, key press handling, history navigation, and cursor positioning by caching output references, history entries, line strings, and cursor/length values.
URL parsing and resolution src/js/node/url.ts	Refactors URL path construction, component extraction, and resolution logic by caching pathname/search/host/protocol components into locals before concatenation and branching.
Node.js utilities and VM src/js/node/util.ts, src/js/node/vm.ts	Refactors DEBUG environment initialization and constructor inspection by caching environment values and descriptor properties before validation.
WASI system interface src/js/node/wasi.ts	Refactors stdin initialization, flag handling, sleep invocation, and environment variable lookup by caching method references and flag constants.
Worker thread event handling src/js/node/worker_threads.ts	Refactors error event handling by caching message strings before condition checks and error construction.
Compression and performance utilities src/js/node/zlib.ts, src/js/node/perf_hooks.ts	Refactors buffer limit checking, chunk allocation, and performance histogram initialization by caching buffer lengths and histogram option parameters.
Third-party integrations src/js/thirdparty/node-fetch.ts, src/js/thirdparty/ws.js	Refactors fetch body conversion and WebSocket agent/server initialization by caching option values, agent properties, and validated configuration before wiring.
Internal utilities src/js/internal/cluster/Worker.ts, src/js/internal/cluster/child.ts, src/js/internal/debugger.ts, src/js/internal/errors.ts, src/js/internal/fifo.ts, src/js/internal/linkedlist.ts, src/js/internal/primordials.js, src/js/eval/feedback.ts, src/js/node/_tls_common.ts	Refactors worker process wiring, error aggregation, debugger socket handling, circular-queue manipulation, and certificate metadata parsing by caching frequently-accessed object references and computed values.

Possibly related PRs

• oven-sh/bun#31155: Overlaps in src/js/node/perf_hooks.ts where both PRs refactor PerformanceObserverForNodeTypes.observe() option destructuring and createHistogram() parameter caching.
• oven-sh/bun#31587: Overlaps in src/js/node/_http_client.ts HTTP client and proxy refactoring patterns including socket/parser routing and timeout handling.
• oven-sh/bun#31830: Overlaps in src/js/internal/fs/cp-sync.ts where both refactor error decoration and filter validation internals.

Suggested reviewers

• Jarred-Sumner
• alii

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: introducing a lint rule to flag duplicate property reads between an if condition and its body.
Description check	✅ Passed	The description comprehensively documents what the rule does, how it works, the violations fixed, and test coverage.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@oxlint.json`:
- Around line 90-93: The oxlint.json file contains JavaScript-style // comments
which are invalid in strict JSON syntax. Remove all // comments from the file
(including the multi-line comment block above
"bun/no-duplicate-nullish-property-access" and others at lines 20, 21, 23-24) or
convert them to multi-line /* */ comment blocks if the contextual information
needs to be preserved. Ensure the resulting file is valid JSON that can be
parsed by tooling and editor integrations.

🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: af9a931e-0b00-4530-aebd-b2bf31afe0c8

📥 Commits

Reviewing files that changed from the base of the PR and between 46c54cd and c865659.

📒 Files selected for processing (20)

• oxlint.json
• scripts/oxlint-plugin-bun.js
• src/js/builtins/ReadableByteStreamInternals.ts
• src/js/internal/fs/cp-sync.ts
• src/js/internal/sql/errors.ts
• src/js/internal/streams/iter/consumers.ts
• src/js/internal/util/inspect.js
• src/js/internal/webstreams_adapters.ts
• src/js/node/_http_agent.ts
• src/js/node/_http_client.ts
• src/js/node/_http_server.ts
• src/js/node/child_process.ts
• src/js/node/dns.ts
• src/js/node/https.ts
• src/js/node/net.ts
• src/js/node/perf_hooks.ts
• src/js/node/tls.ts
• src/js/node/wasi.ts
• src/js/thirdparty/ws.js
• test/internal/oxlint-plugin-bun.test.ts

[Jarred-Sumner]

Put the plugins in a better folder.

[Jarred-Sumner]

Remove all the disable liens and fix the underlying cause.

[robobun]

Addressed in f0d2eef:

• Plugin moved to scripts/oxlint-plugins/bun.js.
• All 61 oxlint-disable comments removed and the underlying double reads fixed: each property is read into a local (destructure or const) before the if, and the local is used for both the check and the body. Where the cached value was previously invoked as a method on this (getStdin/sleep in wasi.ts) the call now goes through .$call(this, ...) to preserve the receiver.
• delete obj.prop is now treated as a write so it no longer triggers a false positive.

bun run lint is clean and bun bd test test/internal/oxlint-plugin-bun.test.ts passes.

[claude]

All my earlier feedback has been addressed and I found no further issues, but the 17-file refactor across hot src/js paths (http server Expect handling, tls, net, child_process, the wasi .$call receiver change) is broad enough that a human should give it a once-over before merge.

Extended reasoning...

Overview

This PR introduces a custom oxlint plugin (scripts/oxlint-plugins/bun.js) implementing bun/no-duplicate-nullish-property-access, wires it into oxlint.json for src/js/**, and refactors 61 sites across 17 files in src/js (http, https, tls, net, child_process, dns, wasi, perf_hooks, streams, inspect, ws, ReadableByteStream builtins, etc.) to read each nullish-checked property into a local before the if. A new test file exercises the rule against fixtures and asserts src/js lints clean.

Security risks

None identified. The lint rule is build-time tooling. The runtime refactors are read-once caching of option/property values; they reduce, not increase, exposure to user-controlled getter side effects.

Level of scrutiny

Medium-high. Each individual edit is mechanical, but in aggregate the PR touches production-critical Node.js compatibility code: the HTTP server Expect: header dispatch was re-nested in _http_server.ts, wasi.ts switched this.getStdin() / this.sleep(ms) to cached fn.$call(this, ...) (a deliberate receiver-preserving change, but still a semantic shift), and perf_hooks.ts now eagerly destructures both entryTypes and type where previously type was only read on the fallback path. None of these look wrong, but they are the kind of subtle equivalence a human should confirm.

Other factors

• I previously flagged the missing delete handling in isWriteTarget, a stale test comment, and an exact-empty stderr assertion; all three were fixed in f0d2eef and 32e1b5c and the threads are resolved.
• The bug-hunting pass on the latest revision found nothing.
• Test coverage for the rule itself is solid (positive/negative/write/delete/suppression/message + an end-to-end clean-lint check).
• Given the breadth across src/js runtime modules, autonomous approval isn't appropriate even though I have no specific concerns left.

[robobun]

Broadened the rule per discussion: it now matches any static property access that appears in an if condition and is read again in the body, not just != null checks. Renamed to bun/no-duplicate-conditional-property-access.

It skips when the body writes to the property (assignment, compound assignment, ++/--, delete) or calls it as a method, since caching in a local would change semantics or lose the receiver.

All 363 existing instances across 62 files in src/js are fixed by reading the property into a local before the check. bun run lint is clean and bun bd test test/internal/oxlint-plugin-bun.test.ts passes; spot-checked url.resolve, readline, util.inspect, https.Agent.getName, tls, querystring, zlib, fs.cp, node:stream, and the HTTP Expect: path against existing suites with no regressions over main.

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@src/js/internal/streams/destroy.ts`:
- Around line 311-318: The code unconditionally reads stream.req at the start of
the destroyer function, which can trigger userland getters or Proxy traps that
may have side effects or throw errors, even on code paths that never use the req
variable. Move the const req = stream.req assignment to only execute immediately
before the else if (isRequest(req)) branch where req is actually needed, rather
than at the beginning of the function. This ensures req is only accessed when
necessary for the specific cleanup path being executed.

🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3ebff335-c841-4722-972d-912d60f6ac20

📥 Commits

Reviewing files that changed from the base of the PR and between 32e1b5c and fe3aa1c.

📒 Files selected for processing (65)

• oxlint.json
• scripts/oxlint-plugins/bun.js
• src/js/builtins/BundlerPlugin.ts
• src/js/builtins/JSBufferConstructor.ts
• src/js/builtins/JSBufferPrototype.ts
• src/js/builtins/ReadableStreamInternals.ts
• src/js/eval/feedback.ts
• src/js/internal/assert/assertion_error.ts
• src/js/internal/cluster/Worker.ts
• src/js/internal/cluster/child.ts
• src/js/internal/debugger.ts
• src/js/internal/errors.ts
• src/js/internal/fifo.ts
• src/js/internal/fs/glob.ts
• src/js/internal/fs/streams.ts
• src/js/internal/fs/watch.ts
• src/js/internal/linkedlist.ts
• src/js/internal/primordials.js
• src/js/internal/sql/shared.ts
• src/js/internal/sql/sqlite.ts
• src/js/internal/streams/compose.ts
• src/js/internal/streams/destroy.ts
• src/js/internal/streams/duplex.ts
• src/js/internal/streams/end-of-stream.ts
• src/js/internal/streams/iter/broadcast.ts
• src/js/internal/streams/iter/consumers.ts
• src/js/internal/streams/iter/pull.ts
• src/js/internal/streams/iter/push.ts
• src/js/internal/streams/iter/transform.ts
• src/js/internal/streams/iter/utils.ts
• src/js/internal/streams/legacy.ts
• src/js/internal/streams/native-readable.ts
• src/js/internal/streams/readable.ts
• src/js/internal/streams/transform.ts
• src/js/internal/streams/utils.ts
• src/js/internal/streams/writable.ts
• src/js/internal/url.ts
• src/js/internal/util/inspect.js
• src/js/internal/webstreams_adapters.ts
• src/js/node/_http2_upgrade.ts
• src/js/node/_http_agent.ts
• src/js/node/_http_client.ts
• src/js/node/_http_common.ts
• src/js/node/_http_outgoing.ts
• src/js/node/_http_server.ts
• src/js/node/_tls_common.ts
• src/js/node/assert.ts
• src/js/node/child_process.ts
• src/js/node/dns.ts
• src/js/node/fs.promises.ts
• src/js/node/fs.ts
• src/js/node/https.ts
• src/js/node/net.ts
• src/js/node/querystring.ts
• src/js/node/readline.ts
• src/js/node/tls.ts
• src/js/node/url.ts
• src/js/node/util.ts
• src/js/node/vm.ts
• src/js/node/wasi.ts
• src/js/node/worker_threads.ts
• src/js/node/zlib.ts
• src/js/thirdparty/node-fetch.ts
• src/js/thirdparty/ws.js
• test/internal/oxlint-plugin-bun.test.ts

[robobun]

Audited every refactored site for reads that I'd moved above a guard that previously gated them. Applied the declare-then-inline-assign pattern so the first read stays at its original position:

let x;
if (guard && (x = obj.prop)) {
  use(x);
}

For || cases where the body is reachable through the arm that skips the assignment, the body falls back with x ?? obj.prop, which matches the original's read timing (property only touched once either way).

Changed (read moved back to original position): BundlerPlugin, feedback.ts, glob.ts, sqlite.ts, duplex.ts, consumers.ts, perf_hooks.ts, tls.ts InternalSecureContext, net.ts (12 sites), child_process.ts (3), _http_outgoing.ts (3), _http2_upgrade.ts, webstreams_adapters.ts, internal/url.ts, assert.ts, readline.ts, node/url.ts (4).

Kept as eager reads with justification:

• Spread-copied/validated option bags (optsWithoutSignal, validateCpOptions() output, WebSocketServer merged options): plain own-property objects we just built.
• Internal arrays/state (batch.length in pull.ts, state.length in readable.ts, keys.length/output.length in inspect.js, self._chunkSize in zlib.ts): no getters, every path already read them.
• Destructures with no throw/return between the original sequential reads (writable/readable/duplex/transform constructors, sql/errors.ts, ws.js connectOpts, _http_agent getName, tls renegotiate): same set of properties accessed on every path.
• Sites that were already inside the else block the audit flagged (end-of-stream signal, _http_server expectHeader, destroy.ts req, url relativeSearch, vm.ts, querystring encodeURIComponent): false positives from the audit heuristic.

The rule now recognises (local = obj.prop) in the test as the pattern it recommends and skips the fallback read.

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@src/js/internal/url.ts`:
- Around line 17-21: The conditional assignment pattern in line 19 only assigns
url.password to the password variable when username is falsy, leaving password
undefined when username is truthy. Then line 20 uses the fallback expression
password ?? url.password which re-reads the url.password property unnecessarily.
Fix this by eagerly assigning both username and password from the url object
before the conditional check (extract them as const variables at the start),
then use the conditional only to decide whether to set options.auth, so the auth
assignment on line 20 only references the cached password variable without a
fallback property access.

🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8e1bfef1-2510-4b69-bc99-5be6f4197e8c

📥 Commits

Reviewing files that changed from the base of the PR and between fe3aa1c and 5ef85f0.

📒 Files selected for processing (21)

• scripts/oxlint-plugins/bun.js
• src/js/builtins/BundlerPlugin.ts
• src/js/eval/feedback.ts
• src/js/internal/fs/glob.ts
• src/js/internal/sql/sqlite.ts
• src/js/internal/streams/destroy.ts
• src/js/internal/streams/duplex.ts
• src/js/internal/streams/iter/consumers.ts
• src/js/internal/url.ts
• src/js/internal/webstreams_adapters.ts
• src/js/node/_http2_upgrade.ts
• src/js/node/_http_client.ts
• src/js/node/_http_outgoing.ts
• src/js/node/assert.ts
• src/js/node/child_process.ts
• src/js/node/net.ts
• src/js/node/perf_hooks.ts
• src/js/node/readline.ts
• src/js/node/tls.ts
• src/js/node/url.ts
• test/internal/oxlint-plugin-bun.test.ts

[robobun]

Pushed f9ef3de: converted the remaining hoists to declare-then-inline-assign so the first read stays at the original position. Specifically the ones called out (legacy.ts existing, readable.ts stateLength, inspect.js descGet, the four JSBufferPrototype this.length sites) plus _http_outgoing.ts _header, _http_client.ts socketPath, zlib.ts _chunkSize, fs.promises chunk.byteLength, iter/pull.ts batch.length, and inspect.js output.length.

The cases I'd described as "already inside the else block" actually are (glob GLOBSTAR, destroy.ts req, end-of-stream signal, inspect keysLength, _http_server expectHeader, url relativeSearch, vm descriptor.value, querystring encodeURIComponent): each one's const sits immediately inside the else { / if (guard) { that was the original gate, so the read position is unchanged from the original.

The remaining const x = obj.prop hoists are the ones where obj.prop was already the first-evaluated term of the immediately-following if (no code between the declaration and the original read site). Happy to convert those too if you want the inline form everywhere for uniformity.

[claude]

I didn't find any bugs, but this PR refactors ~363 property-access sites across 62 files in src/js — including net, tls, the HTTP client/server, streams, and child_process — and several of the transforms (eager destructuring, inline-assign-with-fallback) subtly change property read ordering/timing relative to the original; that's too much production-critical surface area for me to sign off on without a human spot-checking the trickier sites.

Extended reasoning...

Overview

This PR introduces a custom oxlint rule (bun/no-duplicate-conditional-property-access) and applies it to all of src/js. The rule flags reading the same static property in an if condition and again in its body. To make src/js clean under the rule, 363 sites across 62 files were refactored to cache the property in a local. The plugin itself (~220 lines) and its test suite are straightforward; the bulk of the diff is the refactors.

Security risks

None introduced directly. The refactored code touches TLS handshake/verification (tls.ts, net.ts, _http2_upgrade.ts), certificate parsing (_tls_common.ts), and HTTP auth header construction (_http_client.ts), but each change is a read-once → cache transform rather than a logic change. The risk is regression, not new attack surface.

Level of scrutiny

High. While each individual edit is mechanically simple, the PR uses three distinct transform patterns with different semantics:

1. Eager const x = obj.prop before the if — moves the read earlier; benign when nothing between the original and new read site can throw or short-circuit, but the author had to walk this back at several sites already (see the destroy.ts / stream.req thread).
2. Eager destructuring of many properties at once (e.g. https.Agent.getName now destructures 22 keys up front; Readable/Writable/Duplex constructors destructure all hooks; sql/errors.ts destructures 17 fields). This changes the order in which getters fire and reads properties that the original sometimes never touched.
3. let x; if (a || (x = obj.prop)) { use(x ?? obj.prop) } — the fallback re-read is intentional access-timing preservation, but it's non-obvious enough that CodeRabbit misread it once.

The author audited and fixed hoists-past-guards in two follow-up commits, but with 363 sites in core Node-compat code (net/tls/http/streams/child_process/readline/url), a human should at least skim the higher-risk files. The sql/shared.ts change also rewrites "filename" in options && options.filename into a hoisted ternary that reads options.filename/options.url before the adapter branch that previously gated which one was touched.

Other factors

• CI on the latest commit shows 3 musl build failures and 2 test failures — unclear if related.
• All my prior inline comments and CodeRabbit's were addressed and resolved.
• The bug-hunting system found nothing on this revision.
• No CODEOWNERS hits, but src/js/node/** and src/js/internal/streams/** are de-facto owned areas.

[robobun]

Pushed 49d0e22: moved options.filename / options.url reads in sql/shared.ts back into the adapter branches that originally gated them (review caught that one).

Build 63589 failures on the previous commit were unrelated:

• darwin x64 build-rust: rustup couldn't reach static.rust-lang.org ("tcp connect error: Operation timed out")
• test-tls-client-destroy-soon.js on darwin-aarch64: also fails on builds 63580/63578/63576/63569/63568/63563/63561/63558/63556/63545 across other branches; passed 10/10 locally with this diff
• next-pages/dev-server.test.ts on darwin-26-aarch64: puppeteer chrome-headless-shell download failure

270 lanes passed including debian-13-x64-asan and all linux/windows test lanes.

[claude]

No bugs found on this revision, but with 363 refactored sites across 62 core runtime files (net/tls/http/streams/readline/child_process) — several of which needed access-timing fixes during review — this one's worth a human pass.

Extended reasoning...

Overview

This PR adds a custom oxlint rule (bun/no-duplicate-conditional-property-access) via a new jsPlugins entry, pins oxlint@1.70.0 as a devDependency, and refactors 363 call sites across 62 files in src/js/ to read properties into locals before re-using them in if bodies. Touched modules include node:net, node:tls, node:http (client/server/agent/outgoing), node:https, node:child_process, node:readline, node:url, streams internals, util.inspect, wasi, and several builtins (JSBufferPrototype, ReadableStreamInternals).

Security risks

Several refactors sit in security-sensitive paths: TLS handshake/verification (tls.ts, net.ts _rejectUnauthorized handling, _http2_upgrade.ts mutual-TLS), https.Agent.getName (session-cache keying), and auth header construction (internal/url.ts, _http_client.ts). The transformations are mechanical and I didn't spot a case where a cached value could diverge from a guard in a way that bypasses a check, but the _rejectUnauthorized sites now use the (x = obj.prop) / x ?? obj.prop short-circuit pattern which is subtle enough to merit a second pair of eyes.

Level of scrutiny

High. The pattern being applied is simple, but it's applied at scale to hot/critical runtime code, and the PR's own history shows the refactor is error-prone: earlier revisions hoisted reads past guards (destroy.ts stream.req, sql/shared.ts adapter branches) and needed a dedicated audit pass plus the inline-assign-with-fallback pattern to preserve original access timing. A few sites also change call semantics (wasi.ts getStdin/sleep now go through .(this, ...)).

Other factors

All four of my prior inline comments were addressed (delete-as-write, stale test comment, exact-empty stderr assertion, unpinned bun x oxlint). The bug-hunting system found nothing on the latest revision. CI on eaa3fa8 shows two x64-musl build failures and an ASAN test failure for the new test (now skipped under ASAN), which the author should confirm are infra-only before merge.