Skip to content
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

Rule Request: No magic number #4024

Closed
2 tasks done
amadeu01 opened this issue Jul 14, 2022 · 5 comments
Closed
2 tasks done

Rule Request: No magic number #4024

amadeu01 opened this issue Jul 14, 2022 · 5 comments
Labels
rule-request Requests for a new rules.

Comments

@amadeu01
Copy link

New Issue Checklist

New rule request

Please describe the rule idea, format
this issue's title as Rule Request: [Rule Name] and describe:

  1. Why should this rule be added? Share links to existing discussion about what
    the community thinks about this.
    https://eslint.org/docs/latest/rules/no-magic-numbers

  2. Provide several examples of what would and wouldn't trigger violations.

Basically, when you use a number without giving it some context.

Circle()
            .fill(Color.primary.opacity(isAnimate ? 0.1 : 1.0 )) // magic numbers
            .frame(
              width: 10, // magic number
              height: 10
            )
  1. Should the rule be configurable, if so what parameters should be configurable?
    No

  2. Should the rule be opt-in or enabled by default? Why?

It should be optional

@douglasdrumond
Copy link

This is a good idea. One small comment, I think you should edit the title to Rule Request: No magic number to comply with the template

@amadeu01 amadeu01 changed the title No magic number Rule Request: No magic number Jul 14, 2022
@amadeu01
Copy link
Author

This is a good idea. One small comment, I think you should edit the title to Rule Request: No magic number to comply with the template

Done! Thanks

@douglasdrumond
Copy link

About item 4: what do you think of having a configuration to allow some numbers? 0 and 1 are so common and understandable that this rule without configuration may trigger false positives.
Examples:

  • It's common to do array[0] to access the first element. Without this option, one would need to define a let firstIndex = 0 just to do array[firstIndex]. Less readable than just using array[0].
  • In the case of 1, it's common to have some operation plus or minus 1 to avoid off-by-one errors. Having to define another constant may make the code more unreadable.

Using that link you provided as an example, eslint accepts an array of ignored numbers. That way, you can have a rule no-magic-numbers ignore:[0,1]

@SimplyDanny
Copy link
Collaborator

About item 4: what do you think of having a configuration to allow some numbers? 0 and 1 are so common and understandable that this rule without configuration may trigger false positives.

Alternatively, 0 and 1 could just be ignored by default since they are so widely used in all kinds of contexts. Same for 0.0 and 1.0 probably. A configuration to ignore arbitrary numbers in a whole project or a subset of files might not turn out to be very useful. Why would someone like to ignore "57", for example? That said, I would not make the rule configurable at first until someone comes up with a very good example showing that it's really necessary.

@SimplyDanny SimplyDanny added the rule-request Requests for a new rules. label Jul 17, 2022
@luoxiu
Copy link

luoxiu commented Nov 7, 2022

Should be closed since no_magic_numbers is already implemented in #4265 ~

@jpsim jpsim closed this as completed Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule-request Requests for a new rules.
Projects
None yet
Development

No branches or pull requests

5 participants