Skip to content

fix(lint/js): improve diagnostics for noGlobalObjectCalls, noMultiStr, noInvalidUseBeforeDeclaration#9891

Merged
ematipico merged 4 commits intomainfrom
dyc3/better-rule-diagnostics-3
Apr 11, 2026
Merged

fix(lint/js): improve diagnostics for noGlobalObjectCalls, noMultiStr, noInvalidUseBeforeDeclaration#9891
ematipico merged 4 commits intomainfrom
dyc3/better-rule-diagnostics-3

Conversation

@dyc3
Copy link
Copy Markdown
Contributor

@dyc3 dyc3 commented Apr 9, 2026

Summary

Improved the diagnostics for these rules. Split into separate PRs to hopefully make it a little more reviewable.

Generated by gpt 5.4, but I heavily guided the agent and reviewed all the new diagnostics myself.

Test Plan

snapshots

Docs

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 9, 2026

🦋 Changeset detected

Latest commit: f61619d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages
Name Type
@biomejs/biome Patch
@biomejs/cli-win32-x64 Patch
@biomejs/cli-win32-arm64 Patch
@biomejs/cli-darwin-x64 Patch
@biomejs/cli-darwin-arm64 Patch
@biomejs/cli-linux-x64 Patch
@biomejs/cli-linux-arm64 Patch
@biomejs/cli-linux-x64-musl Patch
@biomejs/cli-linux-arm64-musl Patch
@biomejs/wasm-web Patch
@biomejs/wasm-bundler Patch
@biomejs/wasm-nodejs Patch
@biomejs/backend-jsonrpc Patch

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

Copy link
Copy Markdown
Contributor Author

dyc3 commented Apr 9, 2026

@github-actions github-actions Bot added A-Linter Area: linter L-JavaScript Language: JavaScript and super languages labels Apr 9, 2026
@dyc3 dyc3 marked this pull request as ready for review April 9, 2026 18:59
@dyc3 dyc3 force-pushed the dyc3/better-rule-diagnostics-2 branch from c89f59e to f44d1d9 Compare April 10, 2026 18:02
@dyc3 dyc3 force-pushed the dyc3/better-rule-diagnostics-3 branch from bc5a66c to e4d5b11 Compare April 10, 2026 18:08
@dyc3 dyc3 force-pushed the dyc3/better-rule-diagnostics-2 branch from f44d1d9 to c7d48f7 Compare April 10, 2026 18:08
Base automatically changed from dyc3/better-rule-diagnostics-2 to main April 10, 2026 18:21
@dyc3 dyc3 force-pushed the dyc3/better-rule-diagnostics-3 branch from e4d5b11 to f61619d Compare April 10, 2026 18:24
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 10, 2026

Walkthrough

This pull request enhances diagnostic output for three JavaScript linting rules: noGlobalObjectCalls, noInvalidUseBeforeDeclaration, and noMultiStr. Each diagnostic now includes additional note messages that provide clearer explanations of the issues and suggest resolutions. The implementation of the NonCallableGlobals enum's Display trait was updated to use biome_console formatting instead of the standard library, and the JSON global object representation was corrected from "Json" to "JSON". Three corresponding changeset files document these improvements as patch-level updates.

Suggested labels

A-Diagnostic

Suggested reviewers

  • ematipico
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarises the main changes—improving diagnostics for three named JavaScript linting rules.
Description check ✅ Passed The pull request description clearly explains the motivation (improving diagnostics for three linting rules), discloses AI assistance, mentions the test plan (snapshots), and directly relates to all changeset files and code modifications in the diff.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dyc3/better-rule-diagnostics-3

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 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/fair-snails-applaud.md:
- Line 5: Edit the changeset entry to link the rule documentation for
noGlobalObjectCalls: update the sentence that names `noGlobalObjectCalls` to
include a markdown link to the rule docs (e.g.,
`[noGlobalObjectCalls](https://biomejs.dev/linter/rules/no-global-object-calls/)`)
so the changeset references the rule page directly; ensure the link text uses
the exact rule name `noGlobalObjectCalls` and the URL points to the linter/rules
page for that rule.

In @.changeset/famous-zebras-end.md:
- Line 5: Update the changeset entry to hyperlink the rule name "noMultiStr" to
its documentation URL and, if this change corresponds to a tracked GitHub issue,
prepend the entry with "Fixed [`#NUMBER`](issue link)" using the issue number and
URL; edit the text in the changeset entry that currently mentions `noMultiStr`
so the rule name is an actual markdown link to the rule docs and add the Fixed
prefix only when a matching issue exists.

In @.changeset/wise-frogs-pretend.md:
- Line 5: Update the changeset text to include a clickable link to the rule
documentation for noInvalidUseBeforeDeclaration; edit the sentence "Improved the
`noInvalidUseBeforeDeclaration` diagnostic..." to replace the plain inline code
with a markdown link pointing to the rule docs (e.g.,
[noInvalidUseBeforeDeclaration](https://biomejs.dev/linter/rules/no-invalid-use-before-declaration/))
so release notes directly navigate to the rule page while preserving the rest of
the description.

In `@crates/biome_js_analyze/src/lint/correctness/no_global_object_calls.rs`:
- Around line 120-126: The diagnostic message created in RuleDiagnostic::new
using the variable non_callable currently reads "... cannot be called as a
function." and misses the `new` (constructor) case; update the message to a
neutral phrasing that covers both function calls and constructor calls (for
example, use wording like "cannot be invoked as a function or constructor" or
"cannot be called or constructed") so the diagnostic produced by
no_global_object_calls.rs correctly reflects both call and new usages.

In
`@crates/biome_js_analyze/src/lint/correctness/no_invalid_use_before_declaration.rs`:
- Around line 206-208: The sentence in the markup uses the incorrect article "a"
before declaration_kind_text, producing awkward grammar for types like "enum";
update the message in no_invalid_use_before_declaration.rs where the note is
constructed (the markup! block that references declaration_kind_text) to use
"this" instead of "a" (e.g., "Using this "{declaration_kind_text}" before it is
declared...") so the wording reads correctly for "enum"/"enum member".
🪄 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: CHILL

Plan: Pro

Run ID: 98569580-e4a2-40ed-8ea1-073ad609699f

📥 Commits

Reviewing files that changed from the base of the PR and between 7ae83f2 and f61619d.

⛔ Files ignored due to path filters (5)
  • crates/biome_js_analyze/tests/specs/correctness/noGlobalObjectCalls/invalid.js.snap is excluded by !**/*.snap and included by **
  • crates/biome_js_analyze/tests/specs/correctness/noInvalidUseBeforeDeclaration/invalid.js.snap is excluded by !**/*.snap and included by **
  • crates/biome_js_analyze/tests/specs/correctness/noInvalidUseBeforeDeclaration/invalid.ts.snap is excluded by !**/*.snap and included by **
  • crates/biome_js_analyze/tests/specs/correctness/noInvalidUseBeforeDeclaration/invalidBindingPattern.js.snap is excluded by !**/*.snap and included by **
  • crates/biome_js_analyze/tests/specs/nursery/noMultiStr/invalid.js.snap is excluded by !**/*.snap and included by **
📒 Files selected for processing (6)
  • .changeset/fair-snails-applaud.md
  • .changeset/famous-zebras-end.md
  • .changeset/wise-frogs-pretend.md
  • crates/biome_js_analyze/src/lint/correctness/no_global_object_calls.rs
  • crates/biome_js_analyze/src/lint/correctness/no_invalid_use_before_declaration.rs
  • crates/biome_js_analyze/src/lint/nursery/no_multi_str.rs

"@biomejs/biome": patch
---

Improved the `noGlobalObjectCalls` diagnostic to better explain why calling global objects like `Math` or `JSON` is invalid and how to fix it.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Please link the rule docs in the changeset entry.

The text names noGlobalObjectCalls but does not link to the rule documentation, which makes release notes less actionable.

Suggested edit
-Improved the `noGlobalObjectCalls` diagnostic to better explain why calling global objects like `Math` or `JSON` is invalid and how to fix it.
+Improved the [`noGlobalObjectCalls`](https://biomejs.dev/linter/rules/no-global-object-calls/) diagnostic to better explain why calling global objects like `Math` or `JSON` is invalid and how to fix it.

As per coding guidelines, "For changeset descriptions of bug fixes, reference the issue with a link (e.g., 'Fixed #4444'); for rule references, include links to the rule documentation (e.g., 'useAwesomeThing')".

📝 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.

Suggested change
Improved the `noGlobalObjectCalls` diagnostic to better explain why calling global objects like `Math` or `JSON` is invalid and how to fix it.
Improved the [`noGlobalObjectCalls`](https://biomejs.dev/linter/rules/no-global-object-calls/) diagnostic to better explain why calling global objects like `Math` or `JSON` is invalid and how to fix it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.changeset/fair-snails-applaud.md at line 5, Edit the changeset entry to
link the rule documentation for noGlobalObjectCalls: update the sentence that
names `noGlobalObjectCalls` to include a markdown link to the rule docs (e.g.,
`[noGlobalObjectCalls](https://biomejs.dev/linter/rules/no-global-object-calls/)`)
so the changeset references the rule page directly; ensure the link text uses
the exact rule name `noGlobalObjectCalls` and the URL points to the linter/rules
page for that rule.

"@biomejs/biome": patch
---

Improved the `noMultiStr` diagnostic to explain why escaped multiline strings are discouraged and what to use instead.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Please hyperlink the rule name in the changeset entry.

This is user-facing release text, so noMultiStr should link to its rule docs; if there is a tracked issue, use the Fixed [#NUMBER](...) prefix too.

Suggested wording.
-Improved the `noMultiStr` diagnostic to explain why escaped multiline strings are discouraged and what to use instead.
+Improved the [`noMultiStr`](https://biomejs.dev/linter/rules/no-multi-str/) diagnostic to explain why escaped multiline strings are discouraged and what to use instead.

As per coding guidelines: “For changeset descriptions of bug fixes, reference the issue with a link …; for rule references, include links to the rule documentation …”. Based on learnings: use the Fixed [#NUMBER](issue link) prefix only when a corresponding GitHub issue exists.

📝 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.

Suggested change
Improved the `noMultiStr` diagnostic to explain why escaped multiline strings are discouraged and what to use instead.
Improved the [`noMultiStr`](https://biomejs.dev/linter/rules/no-multi-str/) diagnostic to explain why escaped multiline strings are discouraged and what to use instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.changeset/famous-zebras-end.md at line 5, Update the changeset entry to
hyperlink the rule name "noMultiStr" to its documentation URL and, if this
change corresponds to a tracked GitHub issue, prepend the entry with "Fixed
[`#NUMBER`](issue link)" using the issue number and URL; edit the text in the
changeset entry that currently mentions `noMultiStr` so the rule name is an
actual markdown link to the rule docs and add the Fixed prefix only when a
matching issue exists.

"@biomejs/biome": patch
---

Improved the `noInvalidUseBeforeDeclaration` diagnostic to better explain why using a declaration too early is problematic and how to fix it.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Link the rule name in the changeset text.

Nice summary, but this should link the rule docs so release notes are directly navigable.

✍️ Suggested tweak
-Improved the `noInvalidUseBeforeDeclaration` diagnostic to better explain why using a declaration too early is problematic and how to fix it.
+Improved the [`noInvalidUseBeforeDeclaration`](https://biomejs.dev/linter/rules/no-invalid-use-before-declaration/) diagnostic to better explain why using a declaration too early is problematic and how to fix it.

As per coding guidelines: "For changeset descriptions of bug fixes, reference the issue with a link (e.g., 'Fixed #4444'); for rule references, include links to the rule documentation (e.g., 'useAwesomeThing')".

📝 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.

Suggested change
Improved the `noInvalidUseBeforeDeclaration` diagnostic to better explain why using a declaration too early is problematic and how to fix it.
Improved the [`noInvalidUseBeforeDeclaration`](https://biomejs.dev/linter/rules/no-invalid-use-before-declaration/) diagnostic to better explain why using a declaration too early is problematic and how to fix it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.changeset/wise-frogs-pretend.md at line 5, Update the changeset text to
include a clickable link to the rule documentation for
noInvalidUseBeforeDeclaration; edit the sentence "Improved the
`noInvalidUseBeforeDeclaration` diagnostic..." to replace the plain inline code
with a markdown link pointing to the rule docs (e.g.,
[noInvalidUseBeforeDeclaration](https://biomejs.dev/linter/rules/no-invalid-use-before-declaration/))
so release notes directly navigate to the rule page while preserving the rest of
the description.

Comment on lines +120 to +126
Some(
RuleDiagnostic::new(
rule_category!(),
range,
markup! {
<Emphasis>{non_callable}</Emphasis>" cannot be called as a function."
},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Diagnostic headline misses the new case.

At Line [125], the message says “called as a function”, but this rule also reports new <global>(). A neutral phrasing avoids mismatch.

Suggested tweak
-                    <Emphasis>{non_callable}</Emphasis>" cannot be called as a function."
+                    <Emphasis>{non_callable}</Emphasis>" cannot be called or constructed."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/biome_js_analyze/src/lint/correctness/no_global_object_calls.rs`
around lines 120 - 126, The diagnostic message created in RuleDiagnostic::new
using the variable non_callable currently reads "... cannot be called as a
function." and misses the `new` (constructor) case; update the message to a
neutral phrasing that covers both function calls and constructor calls (for
example, use wording like "cannot be invoked as a function or constructor" or
"cannot be called or constructed") so the diagnostic produced by
no_global_object_calls.rs correctly reflects both call and new usages.

Comment on lines +206 to +208
.note(markup! {
"Using a "{declaration_kind_text}" before it is declared makes the code depend on declaration order and hoisting behavior."
})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Tidy the article to avoid a enum grammar.

This string reads awkwardly for enum/enum member. Using “this” avoids article agreement issues cleanly.

🛠️ Suggested fix
             .note(markup! {
-                "Using a "{declaration_kind_text}" before it is declared makes the code depend on declaration order and hoisting behavior."
+                "Using this "{declaration_kind_text}" before it is declared makes the code depend on declaration order and hoisting behavior."
             })
📝 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.

Suggested change
.note(markup! {
"Using a "{declaration_kind_text}" before it is declared makes the code depend on declaration order and hoisting behavior."
})
.note(markup! {
"Using this "{declaration_kind_text}" before it is declared makes the code depend on declaration order and hoisting behavior."
})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@crates/biome_js_analyze/src/lint/correctness/no_invalid_use_before_declaration.rs`
around lines 206 - 208, The sentence in the markup uses the incorrect article
"a" before declaration_kind_text, producing awkward grammar for types like
"enum"; update the message in no_invalid_use_before_declaration.rs where the
note is constructed (the markup! block that references declaration_kind_text) to
use "this" instead of "a" (e.g., "Using this "{declaration_kind_text}" before it
is declared...") so the wording reads correctly for "enum"/"enum member".

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 10, 2026

Merging this PR will not alter performance

✅ 58 untouched benchmarks
⏩ 196 skipped benchmarks1


Comparing dyc3/better-rule-diagnostics-3 (f61619d) with main (7ae83f2)

Open in CodSpeed

Footnotes

  1. 196 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@ematipico ematipico merged commit 4d9ac51 into main Apr 11, 2026
21 checks passed
@ematipico ematipico deleted the dyc3/better-rule-diagnostics-3 branch April 11, 2026 04:41
@github-actions github-actions Bot mentioned this pull request Apr 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Linter Area: linter L-JavaScript Language: JavaScript and super languages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants