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

[Fix #404] Make Rails/HelperInstanceVariable accept of instance variables #407

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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