Skip to content

feat(lint): no-alert rule #6355

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Jun 25, 2025
Merged

Conversation

anthonyshew
Copy link
Contributor

Summary

Adds the no-alert rule from ESLint.

Test Plan

Added valid/invalid tests for the rule.

@github-actions github-actions bot added A-Linter Area: linter L-JavaScript Language: JavaScript and super languages A-Diagnostic Area: diagnostocis labels Jun 17, 2025
Copy link

codspeed-hq bot commented Jun 17, 2025

CodSpeed Performance Report

Merging #6355 will not alter performance

Comparing anthonyshew:shew/no-alert-rule (1b48d29) with main (5705f1a)

Summary

✅ 115 untouched benchmarks

@github-actions github-actions bot added A-CLI Area: CLI A-Project Area: project labels Jun 17, 2025
let name_text = name.text();

if matches!(name_text, "alert" | "confirm" | "prompt") {
// Check if this is actually a global function call (not a local variable)
Copy link
Member

Choose a reason for hiding this comment

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

👍

window.prompt("enter name");

// Bracket notation calls (should trigger the rule)
window["alert"]("bracket notation");
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment says it should trigger, but I don't think it does.

Copy link
Contributor Author

@anthonyshew anthonyshew Jun 17, 2025

Choose a reason for hiding this comment

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

Ah, good catch, thank you. Should be fixed now.

}

fn is_global_object(name: &str) -> bool {
GLOBAL_OBJECTS.contains(&name)
Copy link
Contributor

@vladimir-ivanov vladimir-ivanov Jun 19, 2025

Choose a reason for hiding this comment

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

open question - should we care about cases like:
let anyAlias = window;

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd vote for no, otherwise we could keep chasing an endless amount of cases that are not yet caught 😅

Copy link
Contributor

@arendjr arendjr left a comment

Choose a reason for hiding this comment

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

Great work! Do you want to add a changeset?

}

fn is_global_object(name: &str) -> bool {
GLOBAL_OBJECTS.contains(&name)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd vote for no, otherwise we could keep chasing an endless amount of cases that are not yet caught 😅

Copy link

changeset-bot bot commented Jun 20, 2025

🦋 Changeset detected

Latest commit: 1b48d29

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-darwin-arm64 Patch
@biomejs/cli-darwin-x64 Patch
@biomejs/cli-linux-arm64-musl Patch
@biomejs/cli-linux-arm64 Patch
@biomejs/cli-linux-x64-musl Patch
@biomejs/cli-linux-x64 Patch
@biomejs/cli-win32-arm64 Patch
@biomejs/cli-win32-x64 Patch
@biomejs/wasm-bundler Patch
@biomejs/wasm-nodejs Patch
@biomejs/wasm-web 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

@anthonyshew
Copy link
Contributor Author

@arendjr Not the first time I've forgotten to add a changeset in this repo! Thanks for the reminder, added it on.

@arendjr arendjr added this to the Biome 2.1 milestone Jun 23, 2025
@ematipico ematipico removed this from the Biome 2.1 milestone Jun 25, 2025
@ematipico ematipico merged commit e128ea9 into biomejs:main Jun 25, 2025
28 checks passed
@github-actions github-actions bot mentioned this pull request Jun 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-CLI Area: CLI A-Diagnostic Area: diagnostocis A-Linter Area: linter A-Project Area: project L-JavaScript Language: JavaScript and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants