Add InstanceVariable linter

README.md

@@ -98,7 +98,8 @@ linters:
 | [FinalNewline](#FinalNewline)                    | Yes      | warns about missing newline at the end of a ERB template |
 | [NoJavascriptTagHelper](#NoJavascriptTagHelper)  | Yes      | prevents the usage of Rails' `javascript_tag` |
 | ParserErrors                                     | Yes      |             |
-| PartialInstanceVariable                          | No       | detects instance variables in partials |
+| [InstanceVariable](#InstanceVariable)            | No       | detects instance variables |
+| PartialInstanceVariable                          | No       | detects instance variables in partials (deprecated in favor of [InstanceVariable](#InstanceVariable))|
 | [RequireInputAutocomplete](#RequireInputAutocomplete)        | Yes       | warns about missing autocomplete attributes in input tags |
 | [RightTrim](#RightTrim)                          | Yes      | enforces trimming at the right of an ERB tag |
 | [SelfClosingTag](#SelfClosingTag)                | Yes      | enforces self closing tag styles for void elements |
@@ -355,15 +356,15 @@ Linter-Specific Option | Description
 `correction_style`     | When configured with `cdata`, adds CDATA markers. When configured with `plain`, don't add makers. Defaults to `cdata`.
 
 ### RequireScriptNonce
-This linter prevents the usage of HTML `<script>`, Rails `javascript_tag`, `javascript_include_tag` and `javascript_pack_tag` without a `nonce` argument. The purpose of such a check is to ensure that when [content securty policy](https://edgeguides.rubyonrails.org/security.html#content-security-policy) is implemented in an application, there is a means of discovering tags that need to be updated with a `nonce` argument to enable script execution at application runtime. 
+This linter prevents the usage of HTML `<script>`, Rails `javascript_tag`, `javascript_include_tag` and `javascript_pack_tag` without a `nonce` argument. The purpose of such a check is to ensure that when [content securty policy](https://edgeguides.rubyonrails.org/security.html#content-security-policy) is implemented in an application, there is a means of discovering tags that need to be updated with a `nonce` argument to enable script execution at application runtime.
 
 ```
 Bad ❌
-<script> 
+<script>
     alert(1)
 </script>
 Good ✅
-<script nonce="<%= request.content_security_policy_nonce %>" > 
+<script nonce="<%= request.content_security_policy_nonce %>" >
     alert(1)
 </script>
 ```
@@ -486,6 +487,75 @@ Linter-Specific Option    | Description
 `allow_blank`             | True or false, depending on whether or not the `type` attribute may be omitted entirely from a `<script>` tag. Defaults to `true`.
 `disallow_inline_scripts` | Do not allow inline `<script>` tags anywhere in ERB templates. Defaults to `false`.
 
+### InstanceVariables
+
+This linter prevents instance variables from being referenced in templates.
+
+```erb
+Bad ❌
+<%= @instance_variable %>
+
+Good ✅
+<%= local_variable %>
+```
+
+Instance variables in templates, if not declared (for example because of
+changes or even misspellings), will evaluate to `nil`. Using locals offers better
+protection against inadvertently referencing an undefined instance variable.
+
+Locals also offer more explicit specification of the dependencies of a template,
+which you may prefer for your project.
+
+While locals may be used in any template (see below), they have often been
+limited to use in partials. To restrict this linter to checking partials,
+specify the option `partials_only` to `true`. This configuration can replace the
+usage of the deprecated `PartialInstanceVariable` linter.
+
+To use locals in non-partials in Rails controller actions, you can use an
+explicit `render` method call, e.g.
+
+```ruby
+class MyController < ApplicationController
+  def my_action
+    render locals: { local1: 5, local2: "local 2" }
+  end
+end
+```
+
+To use locals in non-partials in Rails mailers, you can use an explicit `render`
+method call in a `format` block, e.g.
+
+```ruby
+class MyMailer < ApplicationMailer
+  def send_my_mail
+    mail(to: "[email protected]", subject: "Partials in mailer") do |format|
+      format.html do
+        render locals: { local1: 5, local2: "local 2" }
+      end
+    end
+  end
+end
+```
+
+Locals need not be required in templates either. Either of the following techniques
+will allow use of optional locals, including setting defaults within the template:
+
+```erb
+<%
+  optional_variable ||= nil
+  optional_variable_with_default ||= "some default"
+%>
+
+<% if defined?(other_optional_variable) %>
+  <span><%= other_optional_variable %></span>
+<% end %>
+%>
+```
+
+Linter-Specific Option    | Description
+--------------------------|---------------------------------------------------------
+`partials_only`           | Boolean to limit linting to partial templates only.
+
 ## Custom Linters
 
 `erb-lint` allows you to create custom linters specific to your project. It will load linters from the `.erb-linters` directory in the root of your

lib/erb_lint/linters/instance_variable.rb

@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+module ERBLint
+  module Linters
+    # Checks for instance variables in partials.
+    class InstanceVariable < Linter
+      include LinterRegistry
+
+      class ConfigSchema < LinterConfig
+        property :partials_only, accepts: [true, false], default: false, reader: :partials_only?
+      end
+      self.config_schema = ConfigSchema
+
+      INSTANCE_VARIABLE_REGEX = /[^@]?(@?@[a-z_][a-zA-Z_0-9]*)/.freeze
+      PARTIAL_FILE_REGEX = %r{(\A|.*/)_[^/\s]*\.html\.erb\z}.freeze
+
+      def run(processed_source)
+        return unless process_file?(processed_source)
+
+        matches = processed_source
+          .file_content
+          .to_enum(:scan, INSTANCE_VARIABLE_REGEX)
+          .map { Regexp.last_match }
+        return if matches.empty?
+
+        matches.each do |match|
+          range = match.begin(1)...match.end(1)
+          add_offense(processed_source.to_source_range(range), offense_message)
+        end
+      end
+
+      private
+
+      def process_file?(processed_source)
+        return true unless @config.partials_only?
+
+        processed_source.filename.match?(PARTIAL_FILE_REGEX)
+      end
+
+      def offense_message
+        "Instance variable detected."
+      end
+    end
+  end
+end

lib/erb_lint/linters/partial_instance_variable.rb

@@ -2,21 +2,23 @@
 
 module ERBLint
   module Linters
-    # Checks for instance variables in partials.
-    class PartialInstanceVariable < Linter
-      include LinterRegistry
+    # Checks for instance variables in partials only
+    class PartialInstanceVariable < InstanceVariable
+      self.config_schema = ConfigSchema
 
-      def run(processed_source)
-        instance_variable_regex = /\s@\w+/
-        return unless processed_source.filename.match?(%r{(\A|.*/)_[^/\s]*\.html\.erb\z}) &&
-          processed_source.file_content.match?(instance_variable_regex)
-
-        add_offense(
-          processed_source.to_source_range(
-            processed_source.file_content =~ instance_variable_regex..processed_source.file_content.size
-          ),
-          "Instance variable detected in partial."
+      def initialize(file_loader, config)
+        warn(
+          "PartialInstanceVariable is deprecated. "\
+          "Please use InstanceVariable with partials_only=true."
         )
+        config[:partials_only] = true
+        super(file_loader, config)
+      end
+
+      private
+
+      def offense_message
+        "Instance variable detected in partial."
       end
     end
   end

spec/erb_lint/linters/instance_variable_spec.rb

@@ -0,0 +1,109 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+
+describe ERBLint::Linters::InstanceVariable do
+  let(:linter_config) { described_class.config_schema.new }
+  let(:file_loader) { ERBLint::FileLoader.new(".") }
+  let(:linter) { described_class.new(file_loader, linter_config) }
+  let(:processed_source_one) { ERBLint::ProcessedSource.new("_file.html.erb", file) }
+  let(:processed_source_two) { ERBLint::ProcessedSource.new("app/views/_a_model/a_view.html.erb", file) }
+  let(:offenses) { linter.offenses }
+  before do
+    linter_config[:partials_only] = partials_only
+    linter.run(processed_source_one)
+    linter.run(processed_source_two)
+  end
+
+  describe "offenses" do
+    subject { offenses }
+
+    context "with partials_only=true" do
+      let(:partials_only) { true }
+
+      context "when an instance variable is not present" do
+        let(:file) { "<%= user.first_name %>" }
+        it { expect(subject).to(eq([])) }
+      end
+
+      context "when an instance variable is present" do
+        let(:file) { "<h2><%= @user.first_name %></h2>" }
+        it do
+          expect(subject).to(eq([
+            build_offense(processed_source_one, 8...13, "Instance variable detected."),
+          ]))
+        end
+      end
+
+      context "when a class instance variable is present" do
+        let(:file) { "<h2><%= @@user.first_name %></h2>" }
+        it do
+          expect(subject).to(eq([
+            build_offense(processed_source_one, 8...14, "Instance variable detected."),
+          ]))
+        end
+      end
+
+      context "when multiple instance variables are present" do
+        let(:file) { "<h2><%= @user.first_name %> <%= @user.last_name %></h2>" }
+        it do
+          expect(subject).to(eq([
+            build_offense(processed_source_one, 8...13, "Instance variable detected."),
+            build_offense(processed_source_one, 32...37, "Instance variable detected."),
+          ]))
+        end
+      end
+    end
+
+    context "with partials_only=true" do
+      let(:partials_only) { false }
+
+      context "when an instance variable is not present" do
+        let(:file) { "<%= user.first_name %>" }
+        it { expect(subject).to(eq([])) }
+      end
+
+      context "when an instance variable is present" do
+        let(:file) { "<h2><%= @user.first_name %></h2>" }
+        it do
+          expect(subject).to(eq([
+            build_offense(processed_source_one, 8...13, "Instance variable detected."),
+            build_offense(processed_source_two, 8...13, "Instance variable detected."),
+          ]))
+        end
+      end
+
+      context "when a class instance variable is present" do
+        let(:file) { "<h2><%= @@user.first_name %></h2>" }
+        it do
+          expect(subject).to(eq([
+            build_offense(processed_source_one, 8...14, "Instance variable detected."),
+            build_offense(processed_source_two, 8...14, "Instance variable detected."),
+          ]))
+        end
+      end
+
+      context "when multiple instance variables are present" do
+        let(:file) { "<h2><%= @user.first_name %> <%= @user.last_name %></h2>" }
+        it do
+          expect(subject).to(eq([
+            build_offense(processed_source_one, 8...13, "Instance variable detected."),
+            build_offense(processed_source_one, 32...37, "Instance variable detected."),
+            build_offense(processed_source_two, 8...13, "Instance variable detected."),
+            build_offense(processed_source_two, 32...37, "Instance variable detected."),
+          ]))
+        end
+      end
+    end
+  end
+
+  private
+
+  def build_offense(processed_source, range, message)
+    ERBLint::Offense.new(
+      linter,
+      processed_source.to_source_range(range),
+      message
+    )
+  end
+end

spec/erb_lint/linters/partial_instance_variable_spec.rb

@@ -26,7 +26,26 @@
       let(:file) { "<h2><%= @user.first_name %></h2>" }
       it do
         expect(subject).to(eq([
-          build_offense(processed_source_one, 7..32, "Instance variable detected in partial."),
+          build_offense(processed_source_one, 8...13, "Instance variable detected in partial."),
+        ]))
+      end
+    end
+
+    context "when a class instance variable is present" do
+      let(:file) { "<h2><%= @@user.first_name %></h2>" }
+      it do
+        expect(subject).to(eq([
+          build_offense(processed_source_one, 8...14, "Instance variable detected in partial."),
+        ]))
+      end
+    end
+
+    context "when multiple instance variables are present" do
+      let(:file) { "<h2><%= @user.first_name %> <%= @user.last_name %></h2>" }
+      it do
+        expect(subject).to(eq([
+          build_offense(processed_source_one, 8...13, "Instance variable detected in partial."),
+          build_offense(processed_source_one, 32...37, "Instance variable detected in partial."),
         ]))
       end
     end