Readability.LargeNumbers false positives

[novaugust]

Precheck

Environment

• Credo version (mix credo -v): 0.5.2
• Erlang/Elixir version (elixir -v): 1.3.0
• Operating system: osx

What where you trying to do?

I have lists with lots of floats and large numbers. Sometimes credo is giving me false positives on needing underscores.

For this data, the first 7 lines are all fine, but it's compalning about the last 5.

test_data = [
      {[14_434, 0.000, 0.250, -0.500, 4_200_000], {14_193.7059228487, -461.81222427595}},
      {[7_275, -0.013, 0.300, -0.400, 3_242_800], {6_705.06114827335, -686.923024527031}},
      {[2_656, 0.000, 0.320, -0.283, 1_070_000], {1_875.04749892584, -540.482846075626}},
      {[6_928, 0.011, 0.280, -0.283, 2_580_000], {6_238.09826567172, -633.308400721897}},
      {[6_928, -0.011, 0.280, -0.283, 2_580_000], {6_238.09625455701, -633.328209922203}},
      {[6_928, 0.011, 0.280, 0.283, 2_580_000], {6_238.09625455701, 633.328209922203}},
      {[6_928, -0.011, 0.280, 0.283, 2_580_000], {6_238.09826567172, 633.308400721897}},
      {[14_434, 0.000, 0.287, -0.526, 8_740_000], {13_922.7016262733, -872.190348982156}}, #bad
      {[7_270, 0.000, 0.287, -0.526, 68_000], {7_262.51042806277, -13.6958182877388}}, #bad
      {[7_270, 0.000, 0.287, -0.526, 70_000], {7_262.28975554153, -14.0984231358245}}, #bad
      {[7_270, 0.000, 0.200, -0.500, 70_000], {7_263.34062452151, -16.5934712666501}}, #bad
      {[7_270, 0.000, 0.200, 0.500, 70_000], {7_263.34062452151, 16.5934712666501}} #bad
    ]

In each of the last 5 lines, the 5th item in the line is being reported as not having underscores

[rrrene]

The problem is that pre Elixir 1.3.2 we use an unreliable function to retrieve the fragment:

credo/lib/credo/check/readability/large_numbers.ex

Line 131 in 6e4d376

# There's a bug in the :elixir_tokenizer.tokenize/3 in versions prior to

The solution will be to deprecate this check for Elixir < 1.3.2. Thx for reporting! 👍

[novaugust]

Thanks for telling me what's wrong. How do you go about deprecating checks in Credo? I've been making enough noise over here I'd be happy to help

[rrrene]

We don't have a mode of operation for this yet. The Dogma project, which I contributed to before creating Credo has integrated something like this after we ran into the same issue.

The question is if we can adapt the approach for Credo without changing the architecture or if we want to "invent" a new module attribute for checks to show their Elixir compatibility (and then filter checks before-hand).

[devonestes]

This seems like a fun thing to tackle!

I'd imagine that we're going to have more of these in the future as well, and since I was just in the Config module, I think I have a simple enough way of filtering out checks based on the current version of Elixir that Credo is running in.

I'll put together a quick version of it on my plane ride home tomorrow and see what you all think 😉

[rrrene]

@devonestes That'd be great. It would help to have some quick&dirty implementation of this, so we can discuss the direction which we are going to take 👍

[devonestes]

I think we can close this issue now, right @rrrene?

[rrrene]

@devonestes Yes.
@novaugust This will be fixed in the next release. Thx again for reporting! 👍