shell: expand $RANDOM, $UID, $PPID, $SECONDS, $IFS, $OSTYPE and friends - #34905
Open
robobun wants to merge 7 commits into
Open
shell: expand $RANDOM, $UID, $PPID, $SECONDS, $IFS, $OSTYPE and friends#34905robobun wants to merge 7 commits into
robobun wants to merge 7 commits into
Claude / Claude Code Review
completed
Jul 21, 2026 in 31m 39s
Code review found 2 potential issues
Found 3 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 2 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | test/js/bun/shell/bunshell.test.ts:1384-1386 |
"$SECONDS starts at 0" is a wall-clock-dependent assertion |
| 🟡 Nit | src/runtime/shell/interpreter.rs:2180 |
append_special_var's bool return value is dead |
Annotations
Check warning on line 1386 in test/js/bun/shell/bunshell.test.ts
claude / Claude Code Review
"$SECONDS starts at 0" is a wall-clock-dependent assertion
`.toBe("0")` asserts that strictly <1s of wall-clock time elapsed between `Instant::now()` at interpreter construction and the `$SECONDS` expansion — nothing structurally guarantees this on a loaded debug+ASAN CI runner. Consider `expect(Number(out)).toBeLessThan(5)` (or `.toMatch(/^\d+$/)`, matching the `$LINENO` treatment at line 1393); either still fails on the pre-PR empty string.
Check warning on line 2180 in src/runtime/shell/interpreter.rs
claude / Claude Code Review
append_special_var's bool return value is dead
The `-> bool` return value on `append_special_var` is dead: the sole call site at `src/runtime/shell/states/Expansion.rs:499` discards it (`shell.append_special_var(label, out);`). Either drop the return type (and the `true`/`return false` + the "returns `true` on a hit" doc line), or — if the hit/miss signal is meant for a future `${VAR-default}` consumer — mark it `#[must_use]` and `let _ = ...` at the call site so the intent is explicit.
Loading