[Feature] Implement file cache

lib/erb_lint/all.rb

@@ -4,6 +4,7 @@
 
 require "erb_lint"
 require "erb_lint/cache"
+require "erb_lint/cached_offense"
 require "erb_lint/corrector"
 require "erb_lint/file_loader"
 require "erb_lint/linter_config"

lib/erb_lint/cache.rb

@@ -4,54 +4,58 @@ module ERBLint
   class Cache
     CACHE_DIRECTORY = ".erb-lint-cache"
 
-    def initialize(config, file_loader = nil)
+    def initialize(config, file_loader = nil, prune = false)
       @config = config
       @file_loader = file_loader
       @hits = []
       @new_results = []
+      @prune = prune
       puts "Cache mode is on"
     end
 
     def get(filename, file_content)
-      JSON.parse(File.read(File.join(CACHE_DIRECTORY, checksum(filename)))).map do |offense|
-        ERBLint::Offense.from_json(offense, config, @file_loader, file_content)
+      file_checksum = checksum(filename, file_content)
+      begin
+        cache_file_contents_as_offenses = JSON.parse(
+          File.read(File.join(CACHE_DIRECTORY, file_checksum))
+        ).map do |offense|
+          ERBLint::CachedOffense.from_json(offense)
+        end
+      rescue Errno::ENOENT
+        return false
       end
+      @hits.push(file_checksum) if prune?
+      cache_file_contents_as_offenses
     end
 
-    def include?(filename)
-      File.exist?(File.join(CACHE_DIRECTORY, checksum(filename)))
-    end
+    def set(filename, file_content, offenses_as_json)
+      file_checksum = checksum(filename, file_content)
+      @new_results.push(file_checksum) if prune?
 
-    def []=(filename, offenses_as_json)
       FileUtils.mkdir_p(CACHE_DIRECTORY)
 
-      File.open(File.join(CACHE_DIRECTORY, checksum(filename)), "wb") do |f|
+      File.open(File.join(CACHE_DIRECTORY, file_checksum), "wb") do |f|
         f.write(offenses_as_json)
       end
     end
 
-    def add_hit(hit)
-      @hits.push(hit)
-    end
-
-    def add_new_result(filename)
-      @new_results.push(filename)
+    def close
+      prune_cache if prune?
     end
 
-    def prune
+    def prune_cache
+      puts "Prune cache mode is on - pruned file names will be logged"
       if hits.empty?
         puts "Cache being created for the first time, skipping prune"
         return
       end
 
       cache_files = Dir.new(CACHE_DIRECTORY).children
-      hits_as_checksums = hits.map { |hit| checksum(hit) }
-      new_results_as_checksums = new_results.map { |new_result| checksum(new_result) }
       cache_files.each do |cache_file|
-        next if hits_as_checksums.include?(cache_file)
+        next if hits.include?(cache_file)
 
-        if new_results_as_checksums.include?(cache_file)
-          puts "Skipping deletion of new cache result #{cache_file} in prune"
+        if new_results.include?(cache_file)
+          puts "Skipping deletion of new cache result #{cache_file}"
           next
         end
 
@@ -77,19 +81,22 @@ def clear
 
     attr_reader :config, :hits, :new_results
 
-    def checksum(file)
+    def checksum(filename, file_content)
       digester = Digest::SHA1.new
-      mode = File.stat(file).mode
+      mode = File.stat(filename).mode
 
       digester.update(
-        "#{file}#{mode}#{config.to_hash}#{ERBLint::VERSION}"
+        "#{mode}#{config.to_hash}#{ERBLint::VERSION}#{file_content}"
       )
-      digester.file(file)
       digester.hexdigest
     rescue Errno::ENOENT
       # Spurious files that come and go should not cause a crash, at least not
       # here.
       "_"
     end
+
+    def prune?
+      @prune
+    end
   end
 end

lib/erb_lint/cached_offense.rb

@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+
+module ERBLint
+  # A Cached version of an Offense with only essential information represented as strings
+  class CachedOffense
+    attr_reader :line_number, :message, :severity
+
+    def initialize(message, line_number, severity)
+      @message = message
+      @line_number = line_number
+      @severity = severity
+    end
+
+    def self.new_from_offense(offense)
+      new(
+        offense.message,
+        offense.line_number.to_s,
+        offense.severity
+      )
+    end
+
+    def to_json_format
+      {
+        message: message,
+        line_number: line_number,
+        severity: severity,
+      }
+    end
+
+    def self.from_json(parsed_json)
+      parsed_json.transform_keys!(&:to_sym)
+      new(
+        parsed_json[:message],
+        parsed_json[:line_number],
+        parsed_json[:severity]&.to_sym
+      )
+    end
+  end
+end

lib/erb_lint/cli.rb

@@ -39,7 +39,7 @@ def run(args = ARGV)
 
       load_config
 
-      @cache = Cache.new(@config, file_loader) if with_cache? || clear_cache?
+      @cache = Cache.new(@config, file_loader, prune_cache?) if with_cache? || clear_cache?
 
       if clear_cache?
         if cache.cache_dir_exists?
@@ -93,7 +93,7 @@ def run(args = ARGV)
         end
       end
 
-      cache.prune if prune_cache?
+      cache.close if with_cache? || clear_cache?
 
       reporter.show
 
@@ -133,13 +133,11 @@ def run_on_file(runner, filename)
     end
 
     def run_using_cache(runner, filename, file_content)
-      if cache.include?(filename)
-        runner.restore_offenses(cache.get(filename, file_content))
-        cache.add_hit(filename) if prune_cache?
+      if (cache_result_offenses = cache.get(filename, file_content))
+        runner.restore_offenses(cache_result_offenses)
       else
         run_with_corrections(runner, filename, file_content)
-        cache[filename] = runner.offenses.map(&:to_json_format).to_json
-        cache.add_new_result(filename) if prune_cache?
+        cache.set(filename, file_content, runner.offenses.map(&:to_cached_offense_json_format).to_json)
       end
     end
 

lib/erb_lint/offense.rb

@@ -17,49 +17,8 @@ def initialize(linter, source_range, message, context = nil, severity = nil)
       @severity = severity
     end
 
-    def to_json_format
-      {
-        linter_config: linter.config.to_hash,
-        linter_config_class: linter.config.class.name,
-        linter_class: linter.class.name,
-        source_range: source_range_to_json_format,
-        message: message,
-        context: context,
-        severity: severity,
-      }
-    end
-
-    def source_range_to_json_format
-      {
-        begin_pos: source_range.begin_pos,
-        end_pos: source_range.end_pos,
-        source_buffer_name: source_range.source_buffer.name,
-      }
-    end
-
-    def self.from_json(parsed_json, config, file_loader, file_content)
-      parsed_json.transform_keys!(&:to_sym)
-      linter_config = const_get(parsed_json[:linter_config_class]).new(parsed_json[:linter_config])
-      linter = const_get(parsed_json[:linter_class]).new(file_loader, linter_config)
-      new(
-        linter,
-        source_range_from_json_format(parsed_json[:source_range], file_content),
-        parsed_json[:message].presence,
-        parsed_json[:context].presence,
-        parsed_json[:severity].presence&.to_sym
-      )
-    end
-
-    def self.source_range_from_json_format(parsed_json_source_range, file_content)
-      parsed_json_source_range.transform_keys!(&:to_sym)
-      Parser::Source::Range.new(
-        Parser::Source::Buffer.new(
-          parsed_json_source_range[:source_buffer_name],
-          source: file_content
-        ),
-        parsed_json_source_range[:begin_pos],
-        parsed_json_source_range[:end_pos]
-      )
+    def to_cached_offense_json_format
+      ERBLint::CachedOffense.new_from_offense(self).to_json_format
     end
 
     def inspect

spec/erb_lint/cache_spec.rb

@@ -51,14 +51,14 @@
 
   describe "#get" do
     it "returns a cached lint result" do
-      cache_result = cache.get(linted_file_path, cache_file_content)
+      cache_result = cache.get(linted_file_path, linted_file_content)
       expect(cache_result.count).to(eq(2))
     end
   end
 
   describe "#[]=" do
     it "caches a lint result" do
-      cache[linted_file_path] = cache_file_content
+      cache.set(linted_file_path, linted_file_content, cache_file_content)
       expect(File.exist?(
         File.join(
           cache_dir,
@@ -68,16 +68,6 @@
     end
   end
 
-  describe "#include?" do
-    it "returns true if the cache includes the filename" do
-      expect(cache.include?(linted_file_path)).to(be(true))
-    end
-
-    it "returns false if the cache does not include the filename" do
-      expect(cache.include?("gibberish")).to(be(false))
-    end
-  end
-
   describe "#cache_dir_exists?" do
     it "returns true if the cache dir exists" do
       expect(cache.cache_dir_exists?).to(be(true))
@@ -95,15 +85,15 @@
     end
   end
 
-  describe "#prune" do
+  describe "#prune_cache" do
     it "skips prune if no cache hits" do
       allow(cache).to(receive(:hits).and_return([]))
 
-      expect { cache.prune }.to(output(/Cache being created for the first time, skipping prune/).to_stdout)
+      expect { cache.prune_cache }.to(output(/Cache being created for the first time, skipping prune/).to_stdout)
     end
 
     it "does not prune actual cache hits" do
-      cache.prune
+      cache.prune_cache
 
       expect(File.exist?(
         File.join(
@@ -115,12 +105,12 @@
 
     it "does not prune new cache results" do
       allow(cache).to(receive(:hits).and_return(["fake-hit"]))
-      allow(cache).to(receive(:new_results).and_return([linted_file_path]))
+      allow(cache).to(receive(:new_results).and_return([checksum]))
       fakefs_dir = Struct.new(:fakefs_dir)
       allow(fakefs_dir).to(receive(:children).and_return([checksum]))
       allow(FakeFS::Dir).to(receive(:new).and_return(fakefs_dir))
 
-      expect { cache.prune }.to(output(/Skipping deletion of new cache result #{checksum} in prune/).to_stdout)
+      expect { cache.prune_cache }.to(output(/.*Skipping deletion of new cache result #{checksum}/).to_stdout)
 
       expect(File.exist?(
         File.join(
@@ -140,7 +130,7 @@
         f.write(cache_file_content)
       end
 
-      expect { cache.prune }.to(output(/Cleaning deleted cached file with checksum fake-checksum/).to_stdout)
+      expect { cache.prune_cache }.to(output(/Cleaning deleted cached file with checksum fake-checksum/).to_stdout)
 
       expect(File.exist?(
         File.join(
@@ -151,19 +141,29 @@
     end
   end
 
-  describe "#add_new_result" do
-    it "adds new result to cache object new_results attribute" do
-      cache.add_new_result(linted_file_path)
+  describe "prune cache mode on #get and #[] behavior" do
+    before do
+      allow(cache).to(receive(:prune?).and_return(true))
+    end
 
-      expect(cache.send(:new_results)).to(include(linted_file_path))
+    it "adds new result to cache object new_results list attribute" do
+      cache.set(linted_file_path, linted_file_content, cache_file_content)
+
+      expect(cache.send(:new_results)).to(include(checksum))
     end
-  end
 
-  describe "#add_hit" do
-    it "adds new cache hit to cache object hits attribute" do
-      cache.add_hit(linted_file_path)
+    it "adds new cache hit to cache object hits list attribute" do
+      cache.get(linted_file_path, linted_file_content)
+
+      expect(cache.send(:hits)).to(include(checksum))
+    end
+  end
 
-      expect(cache.send(:hits)).to(include(linted_file_path))
+  describe "#close" do
+    it "Calls prune_cache if prune_cache mode is on" do
+      allow(cache).to(receive(:prune?).and_return(true))
+      expect(cache).to(receive(:prune_cache))
+      cache.close
     end
   end
 end

spec/erb_lint/fixtures/cache_file_content

@@ -1 +1 @@
-[{"linter_config":{"enabled":true,"exclude":[]},"linter_config_class":"ERBLint::LinterConfig","linter_class":"ERBLint::Linters::SpaceAroundErbTag","source_range":{"begin_pos":2,"end_pos":2,"source_buffer_name":"/home/zach/dev/fake-app/app/components/elements/image_component/image_component.html.erb"},"message":"Use 1 space after `<%` instead of 0 space.","context":" ","severity":null},{"linter_config":{"enabled":true,"exclude":[]},"linter_config_class":"ERBLint::LinterConfig","linter_class":"ERBLint::Linters::SpaceAroundErbTag","source_range":{"begin_pos":172,"end_pos":172,"source_buffer_name":"/home/zach/dev/fake-app/app/components/elements/image_component/image_component.html.erb"},"message":"Use 1 space before `%>` instead of 0 space.","context":" ","severity":null}]
+[{"message":"Layout/InitialIndentation: Indentation of first line in file detected.","line_number":"1","severity":"convention"},{"message":"Layout/TrailingEmptyLines: Final newline missing.","line_number":"1","severity":"convention"}]