From 4741d6b5ccc0c8b09fa7bc7922ad0a5f19ed1c77 Mon Sep 17 00:00:00 2001
From: Koichi ITO <koic.ito@gmail.com>
Date: Sat, 12 Dec 2020 00:23:58 +0900
Subject: [PATCH] [Fix #404] Make `Rails/HelperInstanceVariable` accept of
 instance variables

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.
---
 CHANGELOG.md                                  |  4 +++
 docs/modules/ROOT/pages/cops_rails.adoc       |  8 ++++++
 .../cop/rails/helper_instance_variable.rb     | 28 ++++++++++++++++++-
 .../rails/helper_instance_variable_spec.rb    | 24 ++++++++++++++++
 4 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 93f5b8be9a..2f4662321a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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
diff --git a/docs/modules/ROOT/pages/cops_rails.adoc b/docs/modules/ROOT/pages/cops_rails.adoc
index ff08803f61..6e23eddce1 100644
--- a/docs/modules/ROOT/pages/cops_rails.adoc
+++ b/docs/modules/ROOT/pages/cops_rails.adoc
@@ -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]
@@ -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
diff --git a/lib/rubocop/cop/rails/helper_instance_variable.rb b/lib/rubocop/cop/rails/helper_instance_variable.rb
index aab061aeaf..60d632fd44 100644
--- a/lib/rubocop/cop/rails/helper_instance_variable.rb
+++ b/lib/rubocop/cop/rails/helper_instance_variable.rb
@@ -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
@@ -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
diff --git a/spec/rubocop/cop/rails/helper_instance_variable_spec.rb b/spec/rubocop/cop/rails/helper_instance_variable_spec.rb
index f8fe47a406..8a407a024e 100644
--- a/spec/rubocop/cop/rails/helper_instance_variable_spec.rb
+++ b/spec/rubocop/cop/rails/helper_instance_variable_spec.rb
@@ -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