Suggestion - checker that detects magic numbers/unnamed constants

[orSolocate]

Current problem

I thought it would be nice to have a checker that detects magic numbers (aka. unnamed constants) in Pylint.

As in all standard languages, -1, 0 and 1 are allowed by default. Other numbers should raise a message unless would be specified in the pylintrc file.

As not all might want to use it and since it is just really a bad practice and never an error it should be considered as an extension instead of core checker.

According to my test there is nothing like that in Pylint, but I'd like to be proven wrong :)

Example:

seconds = 60

func(a, b, 50)

shows no error related to magic number

Desired solution

1: seconds = 60

2: func(a, b, 50)

should show 2 messages of type Refactor (it is never an error just a bad practice in my opinion).

1: "Unnamed constant '60' is used, consider using literal instead of magic numbers."
2: "Unnamed constant '50' is used, consider using literal instead of magic numbers."

Additional context

Similar issue was opened but it was a bit different and more complicated than what I suggest here, see #1379

Similar checkers exist in ESlint and TSlint.

[DanielNoord]

I'm not sure I understand the first example. Isn't that a vicious circle?

earlier_defined_constant = 60
seconds_constant = earlier_defined_constant
seconds = seconds_constant

etc.

I do agree that this is quite opinionated. It could probably be part of the code style checker?

[Pierre-Sassoulas]

I agree with @DanielNoord here, there's an issue with the expected output for line 1, in the example. I think the code examples in the eslint links make sense once translated to Python though.

[orSolocate]

@DanielNoord I agree that this is a chicken-egg case, I think variable assignments like this should be ignored by the checker like you guys suggest.

Maybe it should just take care of specific/common cases:

1. comparisons:

if (a > 5)
is_clipped = max_val < 100

2. function arguments:
   func(a, b, 50)

3. calculations:
   seconds_in_hour = 60 * 60

[DanielNoord]

Yeah, perhaps we should start small? Perhaps only doing comparisons first?

[orSolocate]

I will work on it you can assign me and I will open a PR when implemented

[orSolocate]

@DanielNoord - I extended comparison-of-constants to add this feature. I tried to keep functionality of comparison-of-constants as it was. But maybe we want to disallow singletons there too like I did in magic_number..

added some false-positive fixes, now ignores also empty strings and _main_ cases.

I disabled it in many functional tests, and found it deals well with complicated comparisons like:
c = [text for el in a if (text := el.strip()) == "b"]

Another case I thought of is ellipsis:
if val1 is not ...
should it be taken care of by this checker?