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

Inline disabling of Style/NumericLiterals is ignored when generating MaxLength in the todo file #10066

Closed
reedstonefood opened this issue Sep 6, 2021 · 1 comment · Fixed by #10077

Comments

@reedstonefood
Copy link

Using # rubocop:disable Style/NumericLiterals inline, works fine when running rubocop.

However, the length of the disabled literal is used by the logic that creates .rubocop_todo.yml file. This is a problem when the literal that has been excluded for the cop is longer than the existing value of MinDigits in the todo file. This means that the MinDigits parameter can get set to something larger than is desired, which eventually means it's impossible to use # rubocop:disable Style/NumericLiterals on numbers that are longer than MinDigits as I go on to describe.


Expected behavior

Running rubocop --regenerate-todo should not change the MaxLength value.

Actual behavior

Running rubocop --regenerate-todo increases the value of MinDigits to be one more than the length of the literal that we have instructed the Style/NumericLiterals to ignore.

This then leads to follow-up problems - the next run of rubocop will not be green due to an un-necessary disable comment being detected, and removing the disable comment is undesirable as we intended to mark that specific literal is being OK.

Steps to reproduce the problem

  1. Create a 10 digit constant somewhere FIRST_VIOLATION = 1234567890
  2. Run rubocop --regenerate-todo which should generate something like
# Offense count: 1
# Cop supports --auto-correct.
# Configuration parameters: Strict.
Style/NumericLiterals:
  MinDigits: 11
  1. Create a 20 digit constant somewhere, but add an inline disable.
SECOND_VIOLATION = 12345678901234567890`
  1. Run rubocop and everything is green. It's all good and as desired up to this point.
  2. Now run rubocop --regenerate-todo again. It will get updated to
# Offense count: 2
# Cop supports --auto-correct.
# Configuration parameters: Strict.
Style/NumericLiterals:
  MinDigits: 21
  1. Run rubocop and it doesn't run succesfully - it tells you the disable is no longer required.
  2. Remove the inline disable and rubocop will return green. However, now the special case which we want to allow, is no longer explicitly allowed and is now part of the todo file.

RuboCop version

1.18.4 (using Parser 3.0.2.0, rubocop-ast 1.8.0, running on ruby 2.7.1 x86_64-darwin19)
  - rubocop-rails 2.11.3
@reedstonefood
Copy link
Author

The workaround I'm using for this is for now to Exclude the whole file in rubocop.yml - this is respected by the todo generation code. So it looks like this problem is specific to when a cop is disabled inline.

dvandersluis added a commit to dvandersluis/rubocop that referenced this issue Sep 14, 2021
…ricLiterals` when generating a configuration file.
bbatsov pushed a commit that referenced this issue Sep 14, 2021
…rals` when generating a configuration file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant