Rubocop: Allow NumericLiterals to be less strict

[jfelchner]

Recently it appears that NumericLiterals was "fixed" such that 12_12_12 now throws an error. I understand why this was done, however the underscores are used for readability, not simply as "a thousands separator" as the commit states.

For example, a good use case of _ is for currency. We often have occasion to store money as cents and as such, in tests for example, we might want to list 40000 cents as 400_00 for easier readability.

I'd be willing to PR a change to add a style configuration which will allow this.

I could either do it as an EnforcedStyle or as a configuration which would allow users to decide the minimum and maximum number of digits that should be allowed before it's broken up by an underscore.

---

Expected behavior

  400_00  # Allowed

Actual behavior

  40_000

Steps to reproduce the problem

See above.

RuboCop version

$ rubocop -V
0.47.1 (using Parser 2.3.3.1, running on ruby 2.4.0 x86_64-darwin16)

[mdlima]

+1 for the currency use case.

One possible way to have this flexible enough is to create an optional config to replace the regex that checks for the violation.

[bbatsov]

Well, probably making the regexp configurable is the easiest solution, although I can't really come up with some other exception to the current rule except the one mentioned already. And there's also the case that it's not really clear how to differentiate between a _ used on purpose as cents separator and some that placed mistakenly digits before the end of a literal.

[d-natoli]

+1, this is screwing with our financial app so we've disabled this cop for the moment.

[iGEL]

Please see #4045 for a PR for this.

[jfelchner]

Awesome.

[brandonweiss]

How does everyone use this with regards to unix timestamps? If I set MinDigits to anything less than 11 I get tons of false-positives anywhere I'm using a unix timestamp. They're all in tests… does everyone just exclude their test folder?

[iGEL]

I think, it's very uncommon in the Ruby world to work with unix timestamps directly in the first place, as you can't understand them as a human. As an internal representation of a time class it's fine, but nothing you want to expose to a developer if you can avoid it.

If you have to expose & test timestamps directly, I'd still compare it against a time object and call to_i on it. A lot easier to understand for you and your fellow developers.

expect(my_class.unix_timestamp).to eq(Time.mktime(2017, 2, 21,19, 01, 23).to_i)

And finally, if you really want to have the timestamp in your test, nothing speaks against grouping them according to this cop. Just run rubocop -a to let rubocop fix all of them.

[brandonweiss]

@iGEL Doh, I missed your reply, sorry! That makes sense, you're totally right. Thanks!