Skip to content

Commit

Permalink
[Fix #404] Make Rails/HelperInstanceVariable accept of instance var…
Browse files Browse the repository at this point in the history
…iables

Fixes #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 12, 2020
1 parent 97c8578 commit 4741d6b
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 1 deletion.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
* [#408](https://github.com/rubocop-hq/rubocop-rails/pull/408): Fix bug in `Rails/FindEach` where config was ignored. ([@ghiculescu][])
* [#401](https://github.com/rubocop-hq/rubocop-rails/issues/401): Fix an error for `Rails/WhereEquals` using only named placeholder template without replacement argument. ([@koic][])

### Changes

* [#404](https://github.com/rubocop-hq/rubocop-rails/issues/404): Make `Rails/HelperInstanceVariable` accepts of instance variables when a class which inherits `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 MyFormBuilder < ActionView::Helpers::FormBuilder
@template.do_something
end
----

=== Configurable attributes
Expand Down
28 changes: 27 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,41 @@ module Rails
# def welcome_message(user)
# "Hello #{user.name}"
# end
#
# # good
# class MyFormBuilder < ActionView::Helpers::FormBuilder
# @template.do_something
# end
class HelperInstanceVariable < Base
MSG = 'Do not use instance variables in helpers.'

def_node_matcher :form_builder_class?, <<~PATTERN
(const
(const
(const nil? :ActionView) :Helpers) :FormBuilder)
PATTERN

def on_ivar(node)
return if inherit_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? || inherit_form_builder?(node)

add_offense(node.loc.name)
end

private

def inherit_form_builder?(node)
node.each_ancestor(:class) do |class_node|
return true if form_builder_class?(class_node.parent_class)
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 a class which inherits `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 a class which does not inherit `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 4741d6b

Please sign in to comment.