fix(noUnusedVariables): regression in previous refactor#9520
Conversation
🦋 Changeset detectedLatest commit: 1b05a3c The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
A-Linter
ematipico
force-pushed
the
fix/ambient-promotion
branch
from
cf6874d to
6c9e8d3
Compare
WalkthroughThis PR fixes the Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip CodeRabbit can scan for known vulnerabilities in your dependencies using OSV Scanner.OSV Scanner will automatically detect and report security vulnerabilities in your project's dependencies. No additional configuration is required. |
Parser conformance results onjs/262
jsx/babel
markdown/commonmark
symbols/microsoft
ts/babel
ts/microsoft
|
Merging this PR will not alter performance
Comparing 
Footnotes
|
ematipico
force-pushed
the
fix/ambient-promotion
branch
from
6c9e8d3 to
1b05a3c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/public-spiders-wash.md:
- Line 5: Update the changeset line to follow the required bug-fix sentence
format by adding a colon immediately after the issue link and a brief
description — replace "Fixed
[`#9519`](https://github.com/biomejs/biome/issues/9519)." with "Fixed
[`#9519`](https://github.com/biomejs/biome/issues/9519): Now `noUnusedVariables`
doesn't flag variables that are used as `typeof` type." so the file
.changeset/public-spiders-wash.md conforms to the "Fixed [`#NUMBER`](issue link):
..." guideline.
In
`@crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/validTypeof.ts`:
- Around line 26-34: The tests use type annotations that reference variables
declared inside function bodies (return type typeof result in example3 and
parameter type typeof defaults in example4), which is invalid; move the
declarations for result and defaults out of their respective functions so the
types refer to variables in the declaration scope (e.g., declare const result =
... and const defaults = ... at module scope) and then update example3 and
example4 to use typeof result and typeof defaults respectively.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b4cc7006-8d32-461f-b0c9-5c4d6132c5da
⛔ Files ignored due to path filters (1)
crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/validTypeof.ts.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
.changeset/public-spiders-wash.mdcrates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/validTypeof.tscrates/biome_js_semantic/src/events.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_js_semantic/src/events.rs
| "@biomejs/biome": patch | ||
| --- | ||
|
|
||
| Fixed [#9519](https://github.com/biomejs/biome/issues/9519). Now `noUnusedVariables` doesn't flag variables that are used as `typeof` type. |
There was a problem hiding this comment.
Use the required bug-fix sentence format.
Line 5 should use Fixed [#9519](...): ... (with the colon immediately after the issue link).
Suggested edit
-Fixed [`#9519`](https://github.com/biomejs/biome/issues/9519). Now `noUnusedVariables` doesn't flag variables that are used as `typeof` type.
+Fixed [`#9519`](https://github.com/biomejs/biome/issues/9519): `noUnusedVariables` no longer flags variables used in `typeof` type expressions.As per coding guidelines, "For bug fix changesets, start with 'Fixed [#NUMBER](issue link): ...' format".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Fixed [#9519](https://github.com/biomejs/biome/issues/9519). Now `noUnusedVariables` doesn't flag variables that are used as `typeof` type. | |
| Fixed [`#9519`](https://github.com/biomejs/biome/issues/9519): `noUnusedVariables` no longer flags variables used in `typeof` type expressions. |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~5-~5: Possible missing comma found.
Context: .../github.com//issues/9519). Now noUnusedVariables doesn't flag variab...
(AI_HYDRA_LEO_MISSING_COMMA)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.changeset/public-spiders-wash.md at line 5, Update the changeset line to
follow the required bug-fix sentence format by adding a colon immediately after
the issue link and a brief description — replace "Fixed
[`#9519`](https://github.com/biomejs/biome/issues/9519)." with "Fixed
[`#9519`](https://github.com/biomejs/biome/issues/9519): Now `noUnusedVariables`
doesn't flag variables that are used as `typeof` type." so the file
.changeset/public-spiders-wash.md conforms to the "Fixed [`#NUMBER`](issue link):
..." guideline.
| export function example3(): typeof result { | ||
| const result = compute(); | ||
| return result; | ||
| } | ||
|
|
||
| // Variable used in typeof in a parameter type | ||
| export function example4(callback: (val: typeof defaults) => void) { | ||
| const defaults = { a: 1, b: 2 }; | ||
| callback(defaults); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In TypeScript, is this valid: function f(): typeof x { const x = 1; return x; }?
💡 Result:
No. In function f(): typeof x { const x = 1; return x; }, the return type annotation is checked in the function’s declaration scope, and x is not in scope there (the const x is only in the function body), so you’ll get TS2304: Cannot find name 'x'.
TypeScript’s type query typeof can only refer to an identifier/property-access expression that’s actually in scope at the location of the type annotation [1], and identifier lookup is scope-based at the reference location [2].
Valid alternatives:
const x = 1;
function f(): typeof x { return x; } // okor just let TS infer it:
function f() { const x = 1; return x; } // return type is 1Sources:
[1] https://mariusschulz.com/blog/type-queries-and-typeof-in-typescript
[2] https://bartvds.github.io/tsspec-html/0.9.1/dist/expressions.html
🌐 Web query:
In TypeScript, can a function parameter type annotation reference a constdeclared inside the same function body (e.g.,function g(cb: (v: typeof y) => void) { const y = 1; })?
💡 Result:
No. The parameter list is outside the function body’s block scope, so a const declared inside the body isn’t in scope for a parameter type annotation. With your example, typeof y in cb: (v: typeof y) => void fails name resolution (you’ll get an error like “Cannot find name 'y'”). This follows from const having the same block/lexical scoping rules as let (only usable within/after its declaration in its block) and from the fact that parameter-list constructs can’t safely depend on locals from the body. [1], [2], [3]
Workarounds: move the value (or a type alias) outside the function body, or make it a parameter / generic type parameter instead. [2]
Sources:
[1] TypeScript 1.4 release notes (let block scoping / “cannot refer before its declaration”) (typescriptlang.org)
[2] TypeScript Handbook: Variable Declarations (const has same scoping rules as let) (typescriptlang.org)
[3] TypeScript Language Specification (parameter initializer restrictions / parameter list vs body) (sources.debian.org)
Citations:
- 1: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-1-4.html
- 2: https://www.typescriptlang.org/docs/handbook/variable-declarations.html
- 3: https://sources.debian.org/src/node-typescript/4.8.4%2Bds1-2/doc/TypeScript%20Language%20Specification%20%28Change%20Markup%29%20-%20ARCHIVED.pdf
🏁 Script executed:
# Check if the file exists and examine its contents
fd "validTypeof.ts" --type fRepository: biomejs/biome
Length of output: 223
🏁 Script executed:
# Read the test file to see the actual code
if [ -f "crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/validTypeof.ts" ]; then
cat -n "crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/validTypeof.ts"
fiRepository: biomejs/biome
Length of output: 1781
🏁 Script executed:
# Check nearby files to understand the test structure
ls -la "crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/" | head -20Repository: biomejs/biome
Length of output: 1403
These examples will not compile in TypeScript.
Lines 26 and 32 reference variables declared in the function body within type annotations, which violates TypeScript's scoping rules. The return type typeof result and parameter type typeof defaults will fail with "Cannot find name" errors because type annotations are evaluated in the declaration scope, not the body scope. Move both constants outside their respective functions as suggested to make these valid test cases.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/validTypeof.ts`
around lines 26 - 34, The tests use type annotations that reference variables
declared inside function bodies (return type typeof result in example3 and
parameter type typeof defaults in example4), which is invalid; move the
declarations for result and defaults out of their respective functions so the
types refer to variables in the declaration scope (e.g., declare const result =
... and const defaults = ... at module scope) and then update example3 and
example4 to use typeof result and typeof defaults respectively.
2 participants
Summary
Closes #9519
Used AI to add new tests to cover missing cases.
Test Plan
Added new tests
Docs