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.

Sign up for GitHub

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

Jump to bottom

1.0 does not add Layout/SpaceInsideBlockBraces and Layout/SpaceInsideHashLiteralBraces excluding files to .rubocop_todo.yml #8938

Closed
marutosi opened this issue Oct 25, 2020 · 2 comments · Fixed by #9280
Closed

1.0 does not add Layout/SpaceInsideBlockBraces and Layout/SpaceInsideHashLiteralBraces excluding files to .rubocop_todo.yml #8938

marutosi opened this issue Oct 25, 2020 · 2 comments · Fixed by #9280
Labels
bug

Comments

@marutosi
Copy link

marutosi commented Oct 25, 2020 •
edited
Loading

0.89

https://github.com/marutosi/redmine/blob/82b58d98dff30edd6884696b596b00aba2050369/Gemfile#L95

  gem 'rubocop', '~> 0.89.0'
  gem 'rubocop-performance', '~> 1.5.0'
  gem 'rubocop-rails', '~> 2.5.0'

https://github.com/marutosi/redmine/blob/82b58d98dff30edd6884696b596b00aba2050369/.rubocop_todo.yml#L762

Layout/SpaceInsideBlockBraces:
  Exclude:
    - 'app/controllers/activities_controller.rb'
    - 'app/controllers/application_controller.rb'
              .
              .
              .

Layout/SpaceInsideHashLiteralBraces:
  Exclude:
    - 'app/controllers/imports_controller.rb'
              .
              .
              .

1.0

https://github.com/marutosi/redmine/blob/f730543ddee2d4bc4e547908f711df3320a31d18/Gemfile#L95

  gem 'rubocop', '~> 1.0.0'
  gem 'rubocop-performance', '~> 1.8.0'
  gem 'rubocop-rails', '~> 2.8.0'

https://github.com/marutosi/redmine/blob/f730543ddee2d4bc4e547908f711df3320a31d18/.rubocop_todo.yml#L866

Layout/SpaceInsideBlockBraces:
  EnforcedStyle: no_space

Layout/SpaceInsideHashLiteralBraces:
  EnforcedStyle: no_space
@marutosi marutosi mentioned this issue Oct 25, 2020
Open
@marutosi marutosi changed the title 1.0 does not add Layout/SpaceInsideBlockBraces and Layout/SpaceInsideBlockBraces excluding files to .rubocop_todo.yml 1.0 does not add Layout/SpaceInsideBlockBraces and Layout/SpaceInsideHashLiteralBraces excluding files to .rubocop_todo.yml Oct 25, 2020
@bbatsov bbatsov added the bug label Nov 5, 2020
@jsmestad
Copy link

jsmestad commented Nov 10, 2020 •
edited
Loading

Hitting this same issue. This also applies to:

  • Layout/SpaceBeforeBlockBraces
  • Layout/SpaceAroundEqualsInParameterDefault
  • Style/MethodDefParentheses
  • Style/RaiseArgs
  • Style/For

@drunkel
Copy link

drunkel commented Nov 13, 2020

Experiencing this on 1.3 as well. Preventing us upgrading to use recent rubocop-rspec.

@h-lame h-lame mentioned this issue Dec 22, 2020
Merged
8 tasks
h-lame added a commit to h-lame/rubocop that referenced this issue Dec 23, 2020
@h-lame
[Fix rubocop#8938] Fix --auto-gen-config for `Layout/SpaceAroundEqu…
8c40dba 
…alsInParameterDefault`

The expected use of the API provided by `ConfigurableEnforcedStyle` is
for the cop to call `correct_style_detected` if the cop detects no
offense or to call one of the other `xxx_style_detected` api methods if
it does detect an offense, and it should also call `add_offense` to
register the offense. Doing this updates `config_to_allow_offenses` which
is used during `--auto-gen-config` to decide how to configure the cop for
the current code under review so that regardless of the styles used the
code passes review.

Prior to this commit, this cop _only_ called `correct_style_detected`
which means it would always output `EnforcedStyle: <some value>` during
`--auto-gen-config` runs. If the code under review actually has a mix of
styles rather than a single consistent style then the generated config
for this cop in `.rubocop_todo.yml` will not produce a passing review as
instead of an exclusion list, or turning the cop off, it will only have
the `EnforcedStyle: <some value>` config.

This commit calls `opposite_style_detected` when we call `add_offense` to
avoid this problem. Note: to use `opposite_style_detected` we need exactly
two supported styles to be configured, which they are in `default.yml`,
but one of the specs for `LeadingEmptyLines` runs this cop with slimmed
down config so we have to fix that config.
h-lame added a commit to h-lame/rubocop that referenced this issue Dec 23, 2020
@h-lame
[Fix rubocop#8938] Fix --auto-gen-config for `Layout/SpaceBeforeBlo…
15a8ef1 
…ckBraces`

The expected use of the API provided by `ConfigurableEnforcedStyle` is
for the cop to call `correct_style_detected` if the cop detects no
offense or to call one of the other `xxx_style_detected` api methods if
it does detect an offense, and it should also call `add_offense` to
register the offense. Doing this updates `config_to_allow_offenses` which
is used during `--auto-gen-config` to decide how to configure the cop for
the current code under review so that regardless of the styles used the
code passes review.

Prior to this commit, this cop _only_ called `correct_style_detected`
which means it would always output `EnforcedStyle: <some value>` during
`--auto-gen-config` runs. If the code under review actually has a mix of
styles rather than a single consistent style then the generated config
for this cop in `.rubocop_todo.yml` will not produce a passing review as
instead of an exclusion list, or turning the cop off, it will only have
the `EnforcedStyle: <some value>` config.

This commit calls `opposite_style_detected` when we call `add_offense` to
avoid this problem.
h-lame added a commit to h-lame/rubocop that referenced this issue Dec 23, 2020
@h-lame
[Fix rubocop#8938] Fix --auto-gen-config for Style/For
6d1ad55 
The expected use of the API provided by `ConfigurableEnforcedStyle` is
for the cop to call `correct_style_detected` if the cop detects no
offense or to call one of the other `xxx_style_detected` api methods if
it does detect an offense, and it should also call `add_offense` to
register the offense. Doing this updates `config_to_allow_offenses` which
is used during `--auto-gen-config` to decide how to configure the cop for
the current code under review so that regardless of the styles used the
code passes review.

Prior to this commit, this cop _only_ called `correct_style_detected`
which means it would always output `EnforcedStyle: <some value>` during
`--auto-gen-config` runs. If the code under review actually has a mix of
styles rather than a single consistent style then the generated config
for this cop in `.rubocop_todo.yml` will not produce a passing review as
instead of an exclusion list, or turning the cop off, it will only have
the `EnforcedStyle: <some value>` config.

This commit calls `opposite_style_detected` when we call `add_offense` to
avoid this problem.
h-lame added a commit to h-lame/rubocop that referenced this issue Dec 23, 2020
@h-lame
[Fix rubocop#8938] Fix --auto-gen-config for Style/RaiseArgs
58f620f 
The expected use of the API provided by `ConfigurableEnforcedStyle` is
for the cop to call `correct_style_detected` if the cop detects no
offense or to call one of the other `xxx_style_detected` api methods if
it does detect an offense, and it should also call `add_offense` to
register the offense. Doing this updates `config_to_allow_offenses` which
is used during `--auto-gen-config` to decide how to configure the cop for
the current code under review so that regardless of the styles used the
code passes review.

Prior to this commit, this cop _only_ called `correct_style_detected`
which means it would always output `EnforcedStyle: <some value>` during
`--auto-gen-config` runs. If the code under review actually has a mix of
styles rather than a single consistent style then the generated config
for this cop in `.rubocop_todo.yml` will not produce a passing review as
instead of an exclusion list, or turning the cop off, it will only have
the `EnforcedStyle: <some value>` config.

This commit calls `opposite_style_detected` when we call `add_offense` to
avoid this problem.
h-lame added a commit to h-lame/rubocop that referenced this issue Dec 23, 2020
@h-lame
[Fix rubocop#8938] Fix --auto-gen-config for `Style/AccessModifierD…
5a81135 
…eclarations`

The expected use of the API provided by `ConfigurableEnforcedStyle` is
for the cop to call `correct_style_detected` if the cop detects no
offense or to call one of the other `xxx_style_detected` api methods if
it does detect an offense, and it should also call `add_offense` to
register the offense. Doing this updates `config_to_allow_offenses` which
is used during `--auto-gen-config` to decide how to configure the cop for
the current code under review so that regardless of the styles used the
code passes review.

Prior to this commit, this cop called `correct_style_detected` and
`opposite_style_detected` correctly, but it did not call `add_offense` to
register the offenses correctly. This has two outcomes:

1. During a normal run the cop would only report 1 offense for the whole
   code base under review. When this is fixed it would then report the
   next offense.
2. During a `--auto-gen-config` run the cop would only output
   the first file it encountered an offense in an `Exclude` list which
   means the configuration generated for this cop in `.rubocop_todo.yml`
   will not produce a passing review.

This commit stops treating `opposite_style_detected` as a predicate to
decide if we should call `add_offense` and instead always calls both
methods.
h-lame added a commit to h-lame/rubocop that referenced this issue Dec 23, 2020
@h-lame
[Fix rubocop#8938] Fix --auto-gen-config for `Style/MethodDefParent…
3c63d08 
…heses`

The expected use of the API provided by `ConfigurableEnforcedStyle` is
for the cop to call `correct_style_detected` if the cop detects no
offense or to call one of the other `xxx_style_detected` api methods if
it does detect an offense, and it should also call `add_offense` to
register the offense. Doing this updates `config_to_allow_offenses` which
is used during `--auto-gen-config` to decide how to configure the cop for
the current code under review so that regardless of the styles used the
code passes review.

Prior to this commit, this cop _only_ called `correct_style_detected`
which means it would always output `EnforcedStyle: <some value>` during
`--auto-gen-config` runs. If the code under review actually has a mix of
styles rather than a single consistent style then the generated config
for this cop in `.rubocop_todo.yml` will not produce a passing review as
instead of an exclusion list, or turning the cop off, it will only have
the `EnforcedStyle: <some value>` config.

This commit calls `unexpected_style_detected`, passing in the relevant
detected incorrect style whenever we call `add_offense` to avoid this
problem. We can't use `opposite_style_detected` because this cop has more
than 2 supported styles.
h-lame added a commit to h-lame/rubocop that referenced this issue Dec 23, 2020
@h-lame
[Fix rubocop#8938] Fix --auto-gen-config for `Style/SpaceInsideHash…
cd325af 
…LiteralBraces`

The expected use of the API provided by `ConfigurableEnforcedStyle` is
for the cop to call `correct_style_detected` if the cop detects no
offense or to call one of the other `xxx_style_detected` api methods if
it does detect an offense, and it should also call `add_offense` to
register the offense. Doing this updates `config_to_allow_offenses` which
is used during `--auto-gen-config` to decide how to configure the cop for
the current code under review so that regardless of the styles used the
code passes review.

Prior to this commit, this cop called `correct_style_detected` correctly
on success, but made several mistakes with calling
`ambiguous_style_detected` or `unexpected_style_detected` on error. It
treated these methods as predicates and wouldn't call `add_offense` if
they returned `true` so the detected offenses would not be registered. It
also passed the wrong `style` parameter to these methods - it passed the
configured style from `EnforcedStyle` rather than the one detected by the
offense. This means:

1. During a normal run the cop would not report all offenses detected in
   the whole code base under review.
2. During a `--auto-gen-config` run the cop would only ever output
   `EnforcedStyle: <some value>`. If the code under review actually has a
   mix of styles rather than a single consistent style then the generated
   config for this cop in `.rubocop_todo.yml` will not produce a passing
   review as instead of an exclusion list, or turning the cop off, it will
   only have the `EnforcedStyle: <some value>` config.

This commit corrects both the errors: it passes the detected incorrect
style when calling `ambiguous_style_detected` or
`unexpected_style_detected` and always calls `add_offense` instead of
treating the return value of `ambiguous_style_detected` or
`unexpected_style_detected` as a predicate to decide if it should or not.
h-lame added a commit to h-lame/rubocop that referenced this issue Dec 23, 2020
@h-lame
[Fix rubocop#8938] Fix --auto-gen-config for `Style/SpaceInsideBloc…
31fdf9f 
…kBraces`

The expected use of the API provided by `ConfigurableEnforcedStyle` is
for the cop to call `correct_style_detected` if the cop detects no
offense or to call one of the other `xxx_style_detected` api methods if
it does detect an offense, and it should also call `add_offense` to
register the offense. Doing this updates `config_to_allow_offenses` which
is used during `--auto-gen-config` to decide how to configure the cop for
the current code under review so that regardless of the styles used the
code passes review. Importantly, this API should _only_ be used when the
offense is caused by the configuration handled by `EnforcedStyle`.

Prior to this commit, this cop was inconsistent about when it called
`correct_style_detected` on success and `opposite_style_detected` on
failure. This means it would always output `EnforcedStyle: <some value>`
during `--auto-gen-config` runs. If the code under review actually has a
mix of styles rather than a single consistent style then the generated
config for this cop in `.rubocop_todo.yml` will not produce a passing
review as instead of an exclusion list, or turning the cop off, it will
only have the `EnforcedStyle: <some value>` config.

This commit makes sure we always call `opposite_style_detected` when we
call `add_offense`, and always call `correct_style_detected` when we
don't detect an offense. Note, because this cop has separate style
choices for braces with contents, `EnforcedStyle`, and empty braces
without contents, `EnforcedStyleForEmptyBraces`, we also have to make
sure to only call the `xxx_style_detected` methods when the offenses /
passes are detected for the former as the API provided by
`ConfigurableEnforcedStyle` only applies to `EnforcedStyle` config.
bbatsov pushed a commit that referenced this issue Dec 24, 2020
@h-lame @bbatsov
[Fix #8938] Fix --auto-gen-config for `Layout/SpaceAroundEqualsInPa…
6328e74 
…rameterDefault`

The expected use of the API provided by `ConfigurableEnforcedStyle` is
for the cop to call `correct_style_detected` if the cop detects no
offense or to call one of the other `xxx_style_detected` api methods if
it does detect an offense, and it should also call `add_offense` to
register the offense. Doing this updates `config_to_allow_offenses` which
is used during `--auto-gen-config` to decide how to configure the cop for
the current code under review so that regardless of the styles used the
code passes review.

Prior to this commit, this cop _only_ called `correct_style_detected`
which means it would always output `EnforcedStyle: <some value>` during
`--auto-gen-config` runs. If the code under review actually has a mix of
styles rather than a single consistent style then the generated config
for this cop in `.rubocop_todo.yml` will not produce a passing review as
instead of an exclusion list, or turning the cop off, it will only have
the `EnforcedStyle: <some value>` config.

This commit calls `opposite_style_detected` when we call `add_offense` to
avoid this problem. Note: to use `opposite_style_detected` we need exactly
two supported styles to be configured, which they are in `default.yml`,
but one of the specs for `LeadingEmptyLines` runs this cop with slimmed
down config so we have to fix that config.
bbatsov pushed a commit that referenced this issue Dec 24, 2020
@h-lame @bbatsov
[Fix #8938] Fix --auto-gen-config for Layout/SpaceBeforeBlockBraces
a51c500 
The expected use of the API provided by `ConfigurableEnforcedStyle` is
for the cop to call `correct_style_detected` if the cop detects no
offense or to call one of the other `xxx_style_detected` api methods if
it does detect an offense, and it should also call `add_offense` to
register the offense. Doing this updates `config_to_allow_offenses` which
is used during `--auto-gen-config` to decide how to configure the cop for
the current code under review so that regardless of the styles used the
code passes review.

Prior to this commit, this cop _only_ called `correct_style_detected`
which means it would always output `EnforcedStyle: <some value>` during
`--auto-gen-config` runs. If the code under review actually has a mix of
styles rather than a single consistent style then the generated config
for this cop in `.rubocop_todo.yml` will not produce a passing review as
instead of an exclusion list, or turning the cop off, it will only have
the `EnforcedStyle: <some value>` config.

This commit calls `opposite_style_detected` when we call `add_offense` to
avoid this problem.
bbatsov pushed a commit that referenced this issue Dec 24, 2020
@h-lame @bbatsov
[Fix #8938] Fix --auto-gen-config for Style/For
a88d475 
The expected use of the API provided by `ConfigurableEnforcedStyle` is
for the cop to call `correct_style_detected` if the cop detects no
offense or to call one of the other `xxx_style_detected` api methods if
it does detect an offense, and it should also call `add_offense` to
register the offense. Doing this updates `config_to_allow_offenses` which
is used during `--auto-gen-config` to decide how to configure the cop for
the current code under review so that regardless of the styles used the
code passes review.

Prior to this commit, this cop _only_ called `correct_style_detected`
which means it would always output `EnforcedStyle: <some value>` during
`--auto-gen-config` runs. If the code under review actually has a mix of
styles rather than a single consistent style then the generated config
for this cop in `.rubocop_todo.yml` will not produce a passing review as
instead of an exclusion list, or turning the cop off, it will only have
the `EnforcedStyle: <some value>` config.

This commit calls `opposite_style_detected` when we call `add_offense` to
avoid this problem.
bbatsov pushed a commit that referenced this issue Dec 24, 2020
@h-lame @bbatsov
[Fix #8938] Fix --auto-gen-config for Style/RaiseArgs
9fe5c2b 
The expected use of the API provided by `ConfigurableEnforcedStyle` is
for the cop to call `correct_style_detected` if the cop detects no
offense or to call one of the other `xxx_style_detected` api methods if
it does detect an offense, and it should also call `add_offense` to
register the offense. Doing this updates `config_to_allow_offenses` which
is used during `--auto-gen-config` to decide how to configure the cop for
the current code under review so that regardless of the styles used the
code passes review.

Prior to this commit, this cop _only_ called `correct_style_detected`
which means it would always output `EnforcedStyle: <some value>` during
`--auto-gen-config` runs. If the code under review actually has a mix of
styles rather than a single consistent style then the generated config
for this cop in `.rubocop_todo.yml` will not produce a passing review as
instead of an exclusion list, or turning the cop off, it will only have
the `EnforcedStyle: <some value>` config.

This commit calls `opposite_style_detected` when we call `add_offense` to
avoid this problem.
bbatsov pushed a commit that referenced this issue Dec 24, 2020
@h-lame @bbatsov
[Fix #8938] Fix --auto-gen-config for `Style/AccessModifierDeclarat…
3c6c1dd 
…ions`

The expected use of the API provided by `ConfigurableEnforcedStyle` is
for the cop to call `correct_style_detected` if the cop detects no
offense or to call one of the other `xxx_style_detected` api methods if
it does detect an offense, and it should also call `add_offense` to
register the offense. Doing this updates `config_to_allow_offenses` which
is used during `--auto-gen-config` to decide how to configure the cop for
the current code under review so that regardless of the styles used the
code passes review.

Prior to this commit, this cop called `correct_style_detected` and
`opposite_style_detected` correctly, but it did not call `add_offense` to
register the offenses correctly. This has two outcomes:

1. During a normal run the cop would only report 1 offense for the whole
   code base under review. When this is fixed it would then report the
   next offense.
2. During a `--auto-gen-config` run the cop would only output
   the first file it encountered an offense in an `Exclude` list which
   means the configuration generated for this cop in `.rubocop_todo.yml`
   will not produce a passing review.

This commit stops treating `opposite_style_detected` as a predicate to
decide if we should call `add_offense` and instead always calls both
methods.
bbatsov pushed a commit that referenced this issue Dec 24, 2020
@h-lame @bbatsov
[Fix #8938] Fix --auto-gen-config for Style/MethodDefParentheses
10c698b 
The expected use of the API provided by `ConfigurableEnforcedStyle` is
for the cop to call `correct_style_detected` if the cop detects no
offense or to call one of the other `xxx_style_detected` api methods if
it does detect an offense, and it should also call `add_offense` to
register the offense. Doing this updates `config_to_allow_offenses` which
is used during `--auto-gen-config` to decide how to configure the cop for
the current code under review so that regardless of the styles used the
code passes review.

Prior to this commit, this cop _only_ called `correct_style_detected`
which means it would always output `EnforcedStyle: <some value>` during
`--auto-gen-config` runs. If the code under review actually has a mix of
styles rather than a single consistent style then the generated config
for this cop in `.rubocop_todo.yml` will not produce a passing review as
instead of an exclusion list, or turning the cop off, it will only have
the `EnforcedStyle: <some value>` config.

This commit calls `unexpected_style_detected`, passing in the relevant
detected incorrect style whenever we call `add_offense` to avoid this
problem. We can't use `opposite_style_detected` because this cop has more
than 2 supported styles.
bbatsov pushed a commit that referenced this issue Dec 24, 2020
@h-lame @bbatsov
[Fix #8938] Fix --auto-gen-config for `Style/SpaceInsideHashLiteral…
bd0131c 
…Braces`

The expected use of the API provided by `ConfigurableEnforcedStyle` is
for the cop to call `correct_style_detected` if the cop detects no
offense or to call one of the other `xxx_style_detected` api methods if
it does detect an offense, and it should also call `add_offense` to
register the offense. Doing this updates `config_to_allow_offenses` which
is used during `--auto-gen-config` to decide how to configure the cop for
the current code under review so that regardless of the styles used the
code passes review.

Prior to this commit, this cop called `correct_style_detected` correctly
on success, but made several mistakes with calling
`ambiguous_style_detected` or `unexpected_style_detected` on error. It
treated these methods as predicates and wouldn't call `add_offense` if
they returned `true` so the detected offenses would not be registered. It
also passed the wrong `style` parameter to these methods - it passed the
configured style from `EnforcedStyle` rather than the one detected by the
offense. This means:

1. During a normal run the cop would not report all offenses detected in
   the whole code base under review.
2. During a `--auto-gen-config` run the cop would only ever output
   `EnforcedStyle: <some value>`. If the code under review actually has a
   mix of styles rather than a single consistent style then the generated
   config for this cop in `.rubocop_todo.yml` will not produce a passing
   review as instead of an exclusion list, or turning the cop off, it will
   only have the `EnforcedStyle: <some value>` config.

This commit corrects both the errors: it passes the detected incorrect
style when calling `ambiguous_style_detected` or
`unexpected_style_detected` and always calls `add_offense` instead of
treating the return value of `ambiguous_style_detected` or
`unexpected_style_detected` as a predicate to decide if it should or not.
bbatsov pushed a commit that referenced this issue Dec 24, 2020
@h-lame @bbatsov
[Fix #8938] Fix --auto-gen-config for Style/SpaceInsideBlockBraces
21a97b6 
The expected use of the API provided by `ConfigurableEnforcedStyle` is
for the cop to call `correct_style_detected` if the cop detects no
offense or to call one of the other `xxx_style_detected` api methods if
it does detect an offense, and it should also call `add_offense` to
register the offense. Doing this updates `config_to_allow_offenses` which
is used during `--auto-gen-config` to decide how to configure the cop for
the current code under review so that regardless of the styles used the
code passes review. Importantly, this API should _only_ be used when the
offense is caused by the configuration handled by `EnforcedStyle`.

Prior to this commit, this cop was inconsistent about when it called
`correct_style_detected` on success and `opposite_style_detected` on
failure. This means it would always output `EnforcedStyle: <some value>`
during `--auto-gen-config` runs. If the code under review actually has a
mix of styles rather than a single consistent style then the generated
config for this cop in `.rubocop_todo.yml` will not produce a passing
review as instead of an exclusion list, or turning the cop off, it will
only have the `EnforcedStyle: <some value>` config.

This commit makes sure we always call `opposite_style_detected` when we
call `add_offense`, and always call `correct_style_detected` when we
don't detect an offense. Note, because this cop has separate style
choices for braces with contents, `EnforcedStyle`, and empty braces
without contents, `EnforcedStyleForEmptyBraces`, we also have to make
sure to only call the `xxx_style_detected` methods when the offenses /
passes are detected for the former as the API provided by
`ConfigurableEnforcedStyle` only applies to `EnforcedStyle` config.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
4 participants
@jsmestad @bbatsov @marutosi @drunkel