Skip to content

Commit

Permalink
[Fix rubocop#404] Make Rails/HelperInstanceVariable accept of insta…
Browse files Browse the repository at this point in the history
…nce variables

Fixes rubocop#404.

This PR makes `Rails/HelperInstanceVariable` accept of instance variables
when class inherited `ActionView::Helpers::FormBuilder`.

It may be too broad to accept all instance variables, but view helper and
form builder have different uses, so let's take a look first.
  • Loading branch information
koic committed Dec 11, 2020
1 parent 151b8a6 commit de67c25
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 1 deletion.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## master (unreleased)

### Changes

* [#404](https://github.com/rubocop-hq/rubocop-rails/issues/404): Make `Rails/HelperInstanceVariable` accepts of instance variables when class inherited `ActionView::Helpers::FormBuilder`. ([@koic][])

## 2.9.0 (2020-12-09)

### New features
Expand Down
8 changes: 8 additions & 0 deletions docs/modules/ROOT/pages/cops_rails.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -1622,6 +1622,9 @@ If it seems awkward to explicitly pass in each dependent
variable, consider moving the behaviour elsewhere, for
example to a model, decorator or presenter.

Provided that a class inherits `ActionView::Helpers::FormBuilder`,
an offense will not be registered.

=== Examples

[source,ruby]
Expand All @@ -1635,6 +1638,11 @@ end
def welcome_message(user)
"Hello #{user.name}"
end
# good
class MyFormBuild < ActionView::Helpers::FormBuilder
@template.do_something
end
----

=== Configurable attributes
Expand Down
22 changes: 21 additions & 1 deletion lib/rubocop/cop/rails/helper_instance_variable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ module Rails
# variable, consider moving the behaviour elsewhere, for
# example to a model, decorator or presenter.
#
# Provided that a class inherits `ActionView::Helpers::FormBuilder`,
# an offense will not be registered.
#
# @example
# # bad
# def welcome_message
Expand All @@ -23,18 +26,35 @@ module Rails
# def welcome_message(user)
# "Hello #{user.name}"
# end
#
# # good
# class MyFormBuild < ActionView::Helpers::FormBuilder
# @template.do_something
# end
class HelperInstanceVariable < Base
MSG = 'Do not use instance variables in helpers.'

def on_ivar(node)
return if form_builder?(node)

add_offense(node)
end

def on_ivasgn(node)
return if node.parent.or_asgn_type?
return if node.parent.or_asgn_type? || form_builder?(node)

add_offense(node.loc.name)
end

private

def form_builder?(node)
node.each_ancestor(:class) do |class_node|
return true if class_node.parent_class&.source == 'ActionView::Helpers::FormBuilder'
end

false
end
end
end
end
Expand Down
24 changes: 24 additions & 0 deletions spec/rubocop/cop/rails/helper_instance_variable_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,28 @@ def foo
end
RUBY
end

it 'does not register an offense when the class inherited `ActionView::Helpers::FormBuilder`' do
expect_no_offenses(<<~'RUBY')
class MyFormBuilder < ActionView::Helpers::FormBuilder
def do_something
@template
@template = do_something
end
end
RUBY
end

it 'registers an offense when using the class inherited not `ActionView::Helpers::FormBuilder`' do
expect_offense(<<~'RUBY')
class Foo
def do_something
@template
^^^^^^^^^ Do not use instance variables in helpers.
@template = do_something
^^^^^^^^^ Do not use instance variables in helpers.
end
end
RUBY
end
end

0 comments on commit de67c25

Please sign in to comment.