Add InstanceVariable linter

Adds the InstanceVariable linter, which can be limited to partials only
(thus deprecating PartialInstanceVariable), but considers an instance
variable in any template to be an offense by default.

Also changed range behavior in both linters to highlight the instance
variables themselves. The previous behavior highlighted the start of the
variable to the end of the file.

Finally, added support for detecting class instance variables.

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