Invalidate the cache when linter dependencies change

README.md

@@ -665,6 +665,10 @@ def autocorrect(_processed_source, offense)
 end
 ```
 
+If your linter's offenses depend on anything besides its configuration and the file being linted, such as another
+config file or a gem's version, define a `cache_key` method that returns a string that changes whenever those inputs
+do. Otherwise `--cache` keeps returning results computed with the old inputs.
+
 ## Output formats
 
 You can change the output format of ERB Lint by specifying formatters with the `-f/--format` option.
@@ -766,7 +770,8 @@ No errors were found in ERB files
 ```
 
 Cached lint results are stored in the `.erb_lint_cache` directory by default, though a custom directory can be provided
-via the `--cache-dir` option. Cache filenames are computed with a hash of information about the file and `erb_lint` settings.
+via the `--cache-dir` option. Cache filenames are computed with a hash of information about the file, `erb_lint` settings,
+and other inputs that linters depend on, such as the RuboCop configuration and version.
 
 These files store instance attributes of the `CachedOffense` object, which only contain the `Offense` attributes
 necessary to restore the results of running `erb_lint` for output. The cache also automatically prunes outdated files each time it's run.

lib/erb_lint/cache.rb

@@ -67,6 +67,8 @@ def clear
       FileUtils.rm_r(@cache_dir)
     end
 
+    attr_writer :runner_cache_key
+
     private
 
     attr_reader :config, :hits, :new_results
@@ -76,7 +78,7 @@ def checksum(filename, file_content)
       mode = File.stat(filename).mode
 
       digester.update(
-        "#{mode}#{config.to_hash}#{ERBLint::VERSION}#{file_content}",
+        "#{mode}#{config.to_hash}#{ERBLint::VERSION}#{@runner_cache_key}#{file_content}",
       )
       digester.hexdigest
     rescue Errno::ENOENT

lib/erb_lint/cli.rb

@@ -79,6 +79,9 @@ def run(args = ARGV)
       reporter.preview
 
       runner = ERBLint::Runner.new(file_loader, @config, @options[:disable_inline_configs])
+
+      @cache.runner_cache_key = runner.cache_key if cache?
+
       file_content = nil
 
       lint_files.each do |filename|

lib/erb_lint/linter.rb

@@ -70,6 +70,13 @@ def clear_offenses
       @offenses = []
     end
 
+    # Returns a string that changes whenever something besides the linter's
+    # config and the linted file affects its offenses, such as another config
+    # file or a gem version, so that `--cache` doesn't reuse stale results.
+    def cache_key
+      nil
+    end
+
     private
 
     def update_offense_status(processed_source)

lib/erb_lint/linters/erb_safety.rb

@@ -32,6 +32,10 @@ def run(processed_source)
         end
       end
 
+      def cache_key
+        @file_loader.yaml(@config_filename).to_s unless @config_filename.nil?
+      end
+
       private
 
       def tester_classes

lib/erb_lint/linters/rubocop.rb

@@ -46,8 +46,40 @@ def autocorrect(_processed_source, offense)
         end
       end
 
+      def cache_key
+        Digest::SHA1.hexdigest("#{rubocop_source_checksum}#{portable_rubocop_config}")
+      end
+
       private
 
+      # RuboCop's own result cache hashes the contents of every loaded file, so
+      # this covers plugins, gems, and project-local cops loaded through
+      # `require:`. It's memoized, which is fine because the runner only asks for
+      # cache keys once every linter has loaded its config. Before RuboCop 1.82.1
+      # it stays nil unless RuboCop's own cache computed it, so fall back to the
+      # versions of `rubocop*` gems.
+      def rubocop_source_checksum
+        ::RuboCop::ResultCache.source_checksum || rubocop_gem_versions.join(",")
+      end
+
+      def rubocop_gem_versions
+        Gem.loaded_specs.each_value.filter_map { |spec| spec.full_name if spec.name.start_with?("rubocop") }.sort
+      end
+
+      # RuboCop expands `Exclude` patterns to absolute paths, which would tie
+      # cached results to the directory the project is checked out in.
+      def portable_rubocop_config
+        base_path = File.expand_path(@file_loader.base_path)
+        @rubocop_config.to_hash.to_h do |name, config|
+          next [name, config] unless config.is_a?(Hash) && config["Exclude"].is_a?(Array)
+
+          excludes = config["Exclude"].map do |pattern|
+            pattern.is_a?(String) ? ::RuboCop::PathUtil.relative_path(pattern, base_path) : pattern
+          end
+          [name, config.merge("Exclude" => excludes)]
+        end
+      end
+
       def descendant_nodes(processed_source)
         processed_source.ast.descendants(:erb)
       end

lib/erb_lint/runner.rb

@@ -42,6 +42,15 @@ def restore_offenses(offenses)
       @offenses.concat(offenses)
     end
 
+    def cache_key
+      digester = Digest::SHA1.new
+      @linters.each do |linter|
+        linter_cache_key = linter.cache_key
+        digester.update(linter_cache_key) if linter_cache_key
+      end
+      digester.hexdigest
+    end
+
     private
 
     def enable_inline_configs?

spec/erb_lint/cache_spec.rb

@@ -70,6 +70,15 @@
     end
   end
 
+  describe "#runner_cache_key=" do
+    it "is part of the checksum of cached results" do
+      cache.runner_cache_key = "runner-cache-key"
+      cache.get(linted_file_path, linted_file_content)
+
+      expect(Digest::SHA1.new).to(have_received(:update).with(a_string_including("runner-cache-key")))
+    end
+  end
+
   describe "#cache_dir_exists?" do
     it "returns true if the cache dir exists" do
       expect(cache.cache_dir_exists?).to(be(true))

spec/erb_lint/cli_spec.rb

@@ -332,6 +332,16 @@ def run(processed_source)
 
             expect(Dir[ERBLint::Cache::CACHE_DIRECTORY].length).to(be(1))
           end
+
+          it "does not reuse cached results once a linter's cache key changes" do
+            allow_any_instance_of(ERBLint::Runner).to(receive(:cache_key).and_return("before"))
+            expect { described_class.new.run(args) }.to(output(/Cache mode is on/).to_stdout)
+
+            allow_any_instance_of(ERBLint::Runner).to(receive(:cache_key).and_return("after"))
+            expect { described_class.new.run(args) }.to(output(/Cache being created for the first time/).to_stdout)
+
+            expect(Dir.children(ERBLint::Cache::CACHE_DIRECTORY).size).to(eq(2))
+          end
         end
       end
     end

spec/erb_lint/linter_spec.rb

@@ -31,5 +31,11 @@ class Fake < ERBLint::Linter
         expect(linter.offenses).to(eq([]))
       end
     end
+
+    describe "#cache_key" do
+      it "is nil by default" do
+        expect(linter.cache_key).to(be_nil)
+      end
+    end
   end
 end

spec/erb_lint/linters/erb_safety_spec.rb

@@ -155,6 +155,24 @@ def yaml(_filename)
     end
   end
 
+  describe "#cache_key" do
+    let(:file) { "" }
+
+    it "is nil without a better-html config file" do
+      expect(linter.cache_key).to(be_nil)
+    end
+
+    context "with a better-html config file" do
+      let(:linter_config) { described_class.config_schema.new("better_html_config" => ".better-html.yml") }
+
+      it "changes when the config file changes" do
+        changed_file_loader = MockFileLoader.new(javascript_safe_methods: ["foobar"])
+
+        expect(described_class.new(changed_file_loader, linter_config).cache_key).not_to(eq(linter.cache_key))
+      end
+    end
+  end
+
   private
 
   def unsafe_interpolate(range)

spec/erb_lint/linters/rubocop_spec.rb

@@ -335,6 +335,112 @@
     it { expect(corrected_content).to(eq("<% dont_auto_correct_me(safe_method(dont_auto_correct_me)) %>\n")) }
   end
 
+  describe "#cache_key" do
+    let(:linter_config) do
+      described_class.config_schema.new(rubocop_config: { inherit_from: inherit_from_filename })
+    end
+    let(:nested_config) { { "Layout/LineLength" => { "Max" => 100 } } }
+    let(:file) { "" }
+
+    it "changes when an inherited RuboCop config changes" do
+      cache_key = linter.cache_key
+      allow(RuboCop::ConfigLoader).to(
+        receive(:load_file)
+          .with(a_string_ending_with(inherit_from_filename))
+          .and_return({ "Layout/LineLength" => { "Max" => 120 } }),
+      )
+
+      expect(described_class.new(file_loader, linter_config).cache_key).not_to(eq(cache_key))
+    end
+
+    it "changes with RuboCop's source checksum" do
+      allow(RuboCop::ResultCache).to(receive(:source_checksum).and_return("one"))
+      cache_key = linter.cache_key
+      allow(RuboCop::ResultCache).to(receive(:source_checksum).and_return("two"))
+
+      expect(linter.cache_key).not_to(eq(cache_key))
+    end
+
+    context "when RuboCop doesn't compute a source checksum" do
+      before { allow(RuboCop::ResultCache).to(receive(:source_checksum).and_return(nil)) }
+
+      def stub_loaded_gems(versions)
+        loaded_specs = versions.to_h { |name, version| [name, Gem::Specification.new(name, version)] }
+        allow(Gem).to(receive(:loaded_specs).and_return(loaded_specs))
+      end
+
+      it "changes when a RuboCop gem is upgraded" do
+        stub_loaded_gems("rubocop" => "1.0.0", "rubocop-rails" => "2.0.0")
+        cache_key = linter.cache_key
+        stub_loaded_gems("rubocop" => "1.0.0", "rubocop-rails" => "2.0.1")
+
+        expect(linter.cache_key).not_to(eq(cache_key))
+      end
+
+      it "does not change when other gems are upgraded" do
+        stub_loaded_gems("rubocop" => "1.0.0", "rake" => "13.0.0")
+        cache_key = linter.cache_key
+        stub_loaded_gems("rubocop" => "1.0.0", "rake" => "13.0.1")
+
+        expect(linter.cache_key).to(eq(cache_key))
+      end
+    end
+
+    # `ResultCache.source_checksum` is memoized for the life of the process, so
+    # these compute each key in a fresh one, the way a CLI run does.
+    context "in a new process" do
+      def cache_key_in_new_process(required_file)
+        script = <<~RUBY
+          require "erb_lint/all"
+          config = ERBLint::Linters::Rubocop.config_schema.new(rubocop_config: { "require" => [#{required_file.inspect}] })
+          linter = ERBLint::Linters::Rubocop.new(ERBLint::FileLoader.new(Dir.pwd), config)
+          puts "cache_key=\#{linter.cache_key}"
+        RUBY
+        lib_dir = File.expand_path("../../../lib", __dir__)
+        output = IO.popen([RbConfig.ruby, "-I", lib_dir, "-e", script], err: [:child, :out], &:read)
+
+        cache_key = output[/^cache_key=(\h+)$/, 1]
+        expect(cache_key).not_to(be_nil, output)
+        cache_key
+      end
+
+      def write_required_file(dir, content)
+        File.join(dir, "required_by_rubocop_config.rb").tap { |path| File.write(path, content) }
+      end
+
+      it "is the same for identical required files at different paths" do
+        cache_keys = Array.new(2) do
+          Dir.mktmpdir { |dir| cache_key_in_new_process(write_required_file(dir, "ErbLintCacheKeySpec = 1\n")) }
+        end
+
+        expect(cache_keys.first).to(eq(cache_keys.last))
+      end
+
+      it "changes when a file loaded through `require:` changes" do
+        if Gem::Version.new(RuboCop::Version::STRING) < Gem::Version.new("1.82.1")
+          skip("RuboCop #{RuboCop::Version::STRING} doesn't compute `ResultCache.source_checksum`")
+        end
+
+        Dir.mktmpdir do |dir|
+          cache_key = cache_key_in_new_process(write_required_file(dir, "ErbLintCacheKeySpec = 1\n"))
+
+          expect(cache_key_in_new_process(write_required_file(dir, "ErbLintCacheKeySpec = 2\n"))).not_to(eq(cache_key))
+        end
+      end
+    end
+
+    it "does not depend on where the project is checked out" do
+      linter_config = described_class.config_schema.new(rubocop_config: { AllCops: { Exclude: ["vendor/**/*"] } })
+      cache_keys = Array.new(2) do
+        Dir.mktmpdir do |dir|
+          Dir.chdir(dir) { described_class.new(ERBLint::FileLoader.new(Dir.pwd), linter_config).cache_key }
+        end
+      end
+
+      expect(cache_keys.first).to(eq(cache_keys.last))
+    end
+  end
+
   private
 
   def arbitrary_error_message(range)

spec/erb_lint/runner_spec.rb

@@ -219,4 +219,37 @@ class FakeLinter4 < FakeLinter3; end
       end
     end
   end
+
+  describe "#cache_key" do
+    module ERBLint
+      module Linters
+        class FakeLinterWithCacheKey < FakeLinter1
+          def cache_key
+            "fake cache key"
+          end
+        end
+      end
+    end
+
+    before do
+      allow(ERBLint::LinterRegistry).to(receive(:linters)
+        .and_return([
+          ERBLint::Linters::FakeLinter1,
+          ERBLint::Linters::FakeLinterWithCacheKey,
+        ]))
+    end
+
+    def cache_key_with(linters)
+      config = ERBLint::RunnerConfig.new(linters: linters.to_h { |name| [name, { "enabled" => true }] })
+      described_class.new(file_loader, config).cache_key
+    end
+
+    it "ignores linters without a cache key" do
+      expect(cache_key_with(["FakeLinter1"])).to(eq(cache_key_with([])))
+    end
+
+    it "includes the cache keys of enabled linters" do
+      expect(cache_key_with(["FakeLinter1", "FakeLinterWithCacheKey"])).not_to(eq(cache_key_with(["FakeLinter1"])))
+    end
+  end
 end