rubocop / rubocop-rails

A RuboCop extension focused on enforcing Rails best practices and coding conventions.
https://docs.rubocop.org/rubocop-rails
MIT License
821 stars 263 forks source link

False positive for `Rails/HelperInstanceVariable` when using block-level disabling of `Rails` department #1365

Closed pdobb closed 2 months ago

pdobb commented 2 months ago

The presence of a block-level disabling of the entire Rails department causes any instance variables in same file to report: Rails/HelperInstanceVariable: Do not use instance variables in helpers.

@ivar = 1  # <-- Invalidly reports: Rails/HelperInstanceVariable error

# rubocop:disable Rails
1
# rubocop:enable Rails

Expected behavior

I expect block-level disabling of the entire Rails department should be valid.

Actual behavior

Invalid reporting of an offense from Rails/HelperInstanceVariable cop.

rubocop -d rubocop.rb
For /Users/paul/dev/minesweeper_alliance: configuration from /Users/paul/dev/minesweeper_alliance/.rubocop.yml
configuration from /Users/paul/.gem/ruby/3.3.4/gems/rubocop-rails-2.26.1/config/default.yml
configuration from /Users/paul/.gem/ruby/3.3.4/gems/rubocop-rails-2.26.1/config/default.yml
Default configuration from /Users/paul/.gem/ruby/3.3.4/gems/rubocop-1.66.1/config/default.yml
Use parallel by default.
Skipping parallel inspection: only a single file needs inspection
Inspecting 1 file
Scanning /Users/paul/dev/minesweeper_alliance/rubocop.rb
W

Offenses:

rubocop.rb:3:1: C: Rails/HelperInstanceVariable: Do not use instance variables in helpers.
@ivar = 1
^^^^^
rubocop.rb:5:1: W: [Correctable] Lint/RedundantCopDisableDirective: Unnecessary disabling of Rails department.
# rubocop:disable Rails
^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 2 offenses detected, 1 offense autocorrectable
Finished in 0.2639049999997951 seconds

Steps to reproduce the problem

Simple example, though it is quite contrived. I put the following into a file and then disabled all but rubocop-rails and removed all of my config.

@ivar = 1  # <-- Invalidly reports: Rails/HelperInstanceVariable error

# rubocop:disable Rails
1
# rubocop:enable Rails

Here are a couple of ways that work to avoid the Rails/HelperInstanceVariable offense:

# Works fine if you pick any single cop to disable instead:
@ivar = 1

# rubocop:disable Rails/WhereRange
1
# rubocop:enable Rails/WhereRange
# Works fine if you disable just the one line, instead of a block
@ivar = 1

1 # rubocop:disable Rails

RuboCop version

$ rubocop -V
1.66.1 (using Parser 3.3.5.0, rubocop-ast 1.32.3, running on ruby 3.3.4) [arm64-darwin23]
  - rubocop-rails 2.26.1
pdobb commented 2 months ago

Similar to this old issue: https://github.com/rubocop/rubocop-rails/issues/1161

Earlopain commented 2 months ago
Old message, disregard

It seems like the department-level disable causes a cop to forget it's only supposed to look at a subset of files. This is reproducable with plain RuboCop and the following: ```yml # .rubocop.yml AllCops: DisabledByDefault: true Exclude: - test.rb Style/HashSyntax: Enabled: true ``` ```rb # test.rb { :a => :b } # rubocop:disable Style { :a => :b } # rubocop:enable Style ``` The same also applies to `rubocop:disable all` Where it then reports a offense for the first hash. I also notice the following warning which is unexpected: > An AutocorrectNotice must be defined in your RuboCop config (from file: /home/user/code/test/test.rb) From `Style/Copyright`.

Above I was running rubocop while passing in the filename which changes behaviour. I can still reproduce the original issue but the original message was nonsense.

Earlopain commented 2 months ago

I openend https://github.com/rubocop/rubocop/issues/13257 since this is more of a general problem and not specific to rubocop-rails. Can you close this issue?

pdobb commented 2 months ago

Thanks @Earlopain 👍. Closing in favor of your ticket.