Skip to content

feat(biome_js_analyse): added new rule noMagicNumbers#6562

Merged
vladimir-ivanov merged 17 commits intobiomejs:mainfrom
vladimir-ivanov:feature/noMagicNumbers
Jun 27, 2025
Merged

feat(biome_js_analyse): added new rule noMagicNumbers#6562
vladimir-ivanov merged 17 commits intobiomejs:mainfrom
vladimir-ivanov:feature/noMagicNumbers

Conversation

@vladimir-ivanov
Copy link
Copy Markdown
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
Copy link
Copy Markdown

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 and super languages 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 and super languages A-Diagnostic Area: diagnostocis L-Grit Language: GritQL A-Resolver Area: resolver labels Jun 26, 2025
@codspeed-hq
Copy link
Copy Markdown

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
Copy Markdown
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

Comment thread .changeset/no_magic_numbers.md Outdated
Comment thread crates/biome_js_analyze/src/lint/nursery/no_magic_numbers.rs Outdated
@vladimir-ivanov
Copy link
Copy Markdown
Contributor Author

vladimir-ivanov commented Jun 26, 2025

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
Copy Markdown
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.

Comment thread .changeset/no_magic_numbers.md Outdated
Comment thread crates/biome_js_analyze/src/lint/nursery/no_magic_numbers.rs Outdated
@vladimir-ivanov
Copy link
Copy Markdown
Contributor Author

vladimir-ivanov commented Jun 27, 2025

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
Copy Markdown
Contributor Author

vladimir-ivanov commented Jun 27, 2025

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

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