feat(biome_js_analyze): implement useGlobalThis#8928
Conversation
🦋 Changeset detectedLatest commit: a249c24 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 |
b96ec3c to
5175c30
Compare
WalkthroughA new nursery lint rule 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)
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. Comment |
CodSpeed Performance ReportMerging this PR will not alter performanceComparing Summary
Footnotes
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@crates/biome_js_analyze/src/lint/nursery/use_global_this.rs`:
- Around line 219-288: In check_expression, the JS_COMPUTED_MEMBER_ASSIGNMENT
and JS_COMPUTED_MEMBER_EXPRESSION branches currently treat any computed member
text as a literal property name; change them to only call
check_is_window_specific_api / check_is_web_worker_specific_api when the
computed member is actually a string literal by using as_static_value() (e.g.,
call member.as_static_value() or the crate helper to obtain a StaticValue and
verify it’s a string) before trimming and passing it to
check_is_window_specific_api; keep the existing fallback behavior (no exemption)
when as_static_value() is None so variables like window[innerWidth] are not
exempted.
🧹 Nitpick comments (1)
crates/biome_rule_options/src/use_global_this.rs (1)
1-6: Add a brief rustdoc for the options struct. Line 6 declares a public options type without a doc string; a one‑liner keeps generated docs consistent.Suggested tweak
+/// Options for the `useGlobalThis` rule. pub struct UseGlobalThisOptions {}As per coding guidelines, Use inline rustdoc documentation for rules, assists, and their options.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@crates/biome_js_analyze/src/lint/nursery/use_global_this.rs`:
- Around line 307-315: The current check extracts the first argument's text via
first_arg.to_trimmed_text() without confirming it's a string literal, so
variables would be treated as string names; update the logic inside the
call_expr branch to first try to get a static string via
first_arg.as_static_value() (or equivalent API) and only if that returns
Some(string) then trim quotes and call is_window_specific_event(string); keep
the existing guards (EVENT_TARGET_METHODS, call_expr.arguments(), etc.) and
avoid calling to_trimmed_text() on non-literals so the event check only runs for
true string constants.
🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/nursery/use_global_this.rs (1)
35-109: Consider sorting these arrays alphabetically for easier maintenance.The API lists are comprehensive and well-documented with spec references. Sorting alphabetically would make it easier to check for duplicates and find specific entries during future updates.
dyc3
left a comment
There was a problem hiding this comment.
A good first draft, i like it!
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@crates/biome_js_analyze/src/lint/nursery/use_global_this.rs`:
- Around line 324-338: The current logic lets non-literal event names fall
through and be treated as window-specific; change the branch that checks
EVENT_TARGET_METHODS so that when member_name.text() matches an event-target
method you explicitly handle two cases: if you successfully parse a string
literal (the existing chain ending with event.token_text_trimmed()) return
is_window_specific_event(&event), otherwise return false (do not fall through).
Keep the final "true" return for the other non-event-target window-specific API
checks. Reference symbols: EVENT_TARGET_METHODS, member_name.text(),
call_expr.arguments(), first_arg.as_js_string_literal_expression(),
event.token_text_trimmed(), and is_window_specific_event.
|
@dyc3 Thank you for your review! I have addressed your feedback. I would appreciate it if you could review again. |
Summary
prefer-global-thisfromeslint-plugin-unicorn#8826This PR adds
useGlobalThis, ported fromeslint-plugin-unicorn. It enforces the use ofglobalThisoverwindow,selfandglobalbecauseglobalThisis a standard way to access the global object which contributes to the portability of your code.Test Plan