+
Skip to content

feat(biome_js_analyse): added new rule noMagicNumbers #6562

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 17 commits into from
Jun 27, 2025
Merged

feat(biome_js_analyse): added new rule noMagicNumbers #6562

merged 17 commits into from
Jun 27, 2025

Conversation

vladimir-ivanov
Copy link
Contributor

@vladimir-ivanov vladimir-ivanov commented Jun 26, 2025 •
edited
Loading

Summary

Added noMagicNumbers rule.
The rule detects and reports the use of "magic numbers" — numeric literals that are used directly in code without being assigned to a named constant.

The no magic numbers rule ignores:

  • non-magic values (like 0, 1, 2, 10, 24, 60, and their negative or bigint forms) found anywhere, including arithmetic expressions, fn calls etc.
  • Array indices
  • Enum values
  • Initial values in variable or class property declarations
  • Default values in function parameters or destructuring patterns
  • Arguments to JSON.stringify and parseInt (e.g., JSON.stringify(22), parseInt("123", 25))
  • Operands in bitwise operations (e.g., a & 7, a | 7)
  • Values in JSX expressions (e.g., <div>{1}</div>)
  • Object property values (e.g., { tax: 0.25 })

Example

let total = price * 1.23; // Magic number for tax rate will highlight 1.23 as magic number

closes #4333

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Jun 26, 2025 •
edited
Loading

🦋 Changeset detected

Latest commit: 86af259

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

@github-actions github-actions bot added A-CLI Area: CLI A-Project Area: project A-Linter Area: linter A-Parser Area: parser A-Formatter Area: formatter A-LSP Area: language server protocol L-JavaScript Language: JavaScript and super languages L-CSS Language: CSS A-Diagnostic Area: diagnostocis L-Grit Language: GritQL A-Resolver Area: resolver labels Jun 26, 2025
@vladimir-ivanov vladimir-ivanov force-pushed the feature/noMagicNumbers branch from 7f0b76c to 8a306d7 Compare June 26, 2025 16:51
@github-actions github-actions bot removed A-Project Area: project A-Parser Area: parser A-Formatter Area: formatter A-LSP Area: language server protocol L-CSS Language: CSS A-Diagnostic Area: diagnostocis L-Grit Language: GritQL A-Resolver Area: resolver labels Jun 26, 2025
@codspeed-hq CodSpeed HQ
Copy link

codspeed-hq bot commented Jun 26, 2025 •
edited
Loading

CodSpeed Performance Report

Merging #6562 will not alter performance

Comparing vladimir-ivanov:feature/noMagicNumbers (86af259) with main (a74828c)

Summary

✅ 115 untouched benchmarks

@github-actions github-actions bot added the A-Diagnostic Area: diagnostocis label Jun 26, 2025
@github-actions github-actions bot added the A-Project Area: project label Jun 26, 2025
ematipico
ematipico reviewed Jun 26, 2025
View reviewed changes
Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Haven't checked the code yet. I left a couple of things to address

@vladimir-ivanov
Copy link
Contributor Author

Haven't checked the code yet. I left a couple of things to address

addressed the ones you mentioned, thanks for the suggestions.

@dyc3 dyc3 changed the title feat(biome-js-analyse): added new rule noMagicNumbers feat(biome_js_analyse): added new rule noMagicNumbers Jun 27, 2025
arendjr
arendjr approved these changes Jun 27, 2025
View reviewed changes
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.

Nice work!

Regarding "Operands in bitwise operations (e.g., a & 1, a | 2)", I personally think these shouldn't be an exception. 1 and 2 are already exceptions on their own, and more complex numbers are best assigned to a _MASK or _BITS constant IMO. Otherwise, your code is still not giving any explanation as to why these specific bits are relevant.

vladimir-ivanov and others added 2 commits June 27, 2025 09:03
@vladimir-ivanov @arendjr
Update .changeset/no_magic_numbers.md
74324e8 
Co-authored-by: Arend van Beelen jr. <arend@arendjr.nl>
@vladimir-ivanov @arendjr
Update crates/biome_js_analyze/src/lint/nursery/no_magic_numbers.rs
cd30b0d 
Co-authored-by: Arend van Beelen jr. <arend@arendjr.nl>
@vladimir-ivanov
Copy link
Contributor Author

Nice work!

Regarding "Operands in bitwise operations (e.g., a & 1, a | 2)", I personally think these shouldn't be an exception. 1 and 2 are already exceptions on their own, and more complex numbers are best assigned to a _MASK or _BITS constant IMO. Otherwise, your code is still not giving any explanation as to why these specific bits are relevant.

RE: 1 and 2 are already exceptions on their own - you are right about 1 and 2 being non magical :-), let me update with other examples.
RE: and more complex numbers are best assigned to a _MASK or _BITS constant IMO - suggests to me we need an option for these :-)? we can introduce as part of another task?

@vladimir-ivanov
Copy link
Contributor Author

I can now merge if it goes green right? @arendjr

arendjr
arendjr approved these changes Jun 27, 2025
View reviewed changes
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.

Yep, go for it!

@vladimir-ivanov vladimir-ivanov merged commit 153eda7 into biomejs:main Jun 27, 2025
29 checks passed
@vladimir-ivanov vladimir-ivanov deleted the feature/noMagicNumbers branch June 27, 2025 08:53
@github-actions github-actions bot mentioned this pull request Jun 27, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arendjr arendjr arendjr approved these changes

@siketyan siketyan Awaiting requested review from siketyan

@Conaclos Conaclos Awaiting requested review from Conaclos

@ematipico ematipico Awaiting requested review from ematipico

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

📎 Implement noMagicNumbers - eslint/no-magic-numbers, typescript-eslint/no-magic-numbers
3 participants
@vladimir-ivanov @arendjr @ematipico
点击 这是indexloc提供的php浏览器服务，不要输入任何密码和下载