From fbf26a29c5f072bd10ac6ada6369ce254c5c206f Mon Sep 17 00:00:00 2001 From: Christian Bruckmayer Date: Wed, 4 Nov 2020 18:08:57 +0000 Subject: [PATCH 01/22] Implement file cache First approach to implement a file cache. https://github.com/Shopify/erb-lint/issues/158 Co-authored-by: Mike Dalessio --- lib/erb_lint.rb | 2 +- lib/erb_lint/all.rb | 1 + lib/erb_lint/cache.rb | 47 +++++++++++++++++++++++++++++++++++++++++++ lib/erb_lint/cli.rb | 33 +++++++++++++++++++++++++++++- 4 files changed, 81 insertions(+), 2 deletions(-) create mode 100644 lib/erb_lint/cache.rb diff --git a/lib/erb_lint.rb b/lib/erb_lint.rb index 66825167..e6afe535 100644 --- a/lib/erb_lint.rb +++ b/lib/erb_lint.rb @@ -1,3 +1,3 @@ # frozen_string_literal: true -require "erb_lint/version" +require "erb_lint/version" \ No newline at end of file diff --git a/lib/erb_lint/all.rb b/lib/erb_lint/all.rb index ecd95cb0..8280343a 100644 --- a/lib/erb_lint/all.rb +++ b/lib/erb_lint/all.rb @@ -3,6 +3,7 @@ require "rubocop" require "erb_lint" +require "erb_lint/cache" require "erb_lint/corrector" require "erb_lint/file_loader" require "erb_lint/linter_config" diff --git a/lib/erb_lint/cache.rb b/lib/erb_lint/cache.rb new file mode 100644 index 00000000..72d16f6f --- /dev/null +++ b/lib/erb_lint/cache.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +module ERBLint + class Cache + CACHE_DIRECTORY = '.erb-lint-cache' + private_constant :CACHE_DIRECTORY + + def initialize(config) + @config = config.to_hash + end + + def [](filename) + JSON.parse(File.read(File.join(CACHE_DIRECTORY, checksum(filename)))) + end + + def include?(filename) + File.exist?(File.join(CACHE_DIRECTORY, checksum(filename))) + end + + def []=(filename, messages) + FileUtils.mkdir_p(CACHE_DIRECTORY) + + File.open(File.join(CACHE_DIRECTORY, checksum(filename)), 'wb') do |f| + f.write messages.to_json + end + end + + private + + attr_reader :config + + def checksum(file) + digester = Digest::SHA1.new + mode = File.stat(file).mode + + digester.update( + "#{file}#{mode}#{config.to_s}" + ) + digester.file(file) + digester.hexdigest + rescue Errno::ENOENT + # Spurious files that come and go should not cause a crash, at least not + # here. + '_' + end + end +end diff --git a/lib/erb_lint/cli.rb b/lib/erb_lint/cli.rb index 7e5396eb..449de305 100644 --- a/lib/erb_lint/cli.rb +++ b/lib/erb_lint/cli.rb @@ -33,6 +33,7 @@ def run(args = ARGV) @files = @options[:stdin] || dupped_args load_config + @cache = Cache.new(config) if !@files.empty? && lint_files.empty? if allow_no_files? @@ -65,7 +66,7 @@ def run(args = ARGV) lint_files.each do |filename| runner.clear_offenses begin - file_content = run_with_corrections(runner, filename) + puts file_content = run_on_file(runner, filename) rescue => e @stats.exceptions += 1 puts "Exception occurred when processing: #{relative_filename(filename)}" @@ -99,10 +100,36 @@ def run(args = ARGV) private + attr_reader :cache, :config + + def run_on_file(runner, filename) + if with_cache? && !autocorrect? + run_using_cache(runner, filename) + else + run_with_corrections(runner, filename) + end + end + + def run_using_cache(runner, filename) + puts cache.include?(filename) + if cache.include?(filename) && !autocorrect? + result = cache[filename] + @stats.found += result.size + result + else + result = run_with_corrections(runner, filename) + cache[filename] = result + end + end + def autocorrect? @options[:autocorrect] end + def with_cache? + @options[:with_cache] + end + def run_with_corrections(runner, filename) file_content = read_content(filename) @@ -283,6 +310,10 @@ def option_parser @options[:enabled_linters] = known_linter_names end + opts.on("--with_cache", "Enable caching") do |config| + @options[:with_cache] = config + end + opts.on("--enable-linters LINTER[,LINTER,...]", Array, "Only use specified linter", "Known linters are: #{known_linter_names.join(", ")}") do |linters| linters.each do |linter| From 17ce88729da60df5fec50e1679ede38d0d9a6cf3 Mon Sep 17 00:00:00 2001 From: Zach Feldman Date: Fri, 12 Aug 2022 16:15:20 -0400 Subject: [PATCH 02/22] Initial cleaning implementation --- Gemfile | 1 + lib/erb_lint/cache.rb | 35 ++++++++++++++++++++++++++++++++++- lib/erb_lint/cli.rb | 18 +++++++++++++++--- 3 files changed, 50 insertions(+), 4 deletions(-) diff --git a/Gemfile b/Gemfile index 548e391e..efd1b411 100644 --- a/Gemfile +++ b/Gemfile @@ -3,6 +3,7 @@ source "https://rubygems.org" gem "rake" +gem 'pry' group "test" do gem "fakefs" diff --git a/lib/erb_lint/cache.rb b/lib/erb_lint/cache.rb index 72d16f6f..3416c8cd 100644 --- a/lib/erb_lint/cache.rb +++ b/lib/erb_lint/cache.rb @@ -7,6 +7,8 @@ class Cache def initialize(config) @config = config.to_hash + @hits = [] + @new_results = [] end def [](filename) @@ -25,9 +27,40 @@ def []=(filename, messages) end end + def add_hit(hit) + @hits.push(hit) + end + + def add_new_result(filename) + @new_results.push(filename) + end + + def clean + if hits.empty? + puts "Cache being created, skipping clean" + 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) + if new_results_as_checksums.include?(cache_file) + puts "Skipping deletion of new cache result #{cache_file} in clean" + return + end + + puts "Cleaning deleted cached file with checksum #{cache_file}}" + File.delete(File.join(CACHE_DIRECTORY, cache_file)) + end + + @hits = [] + end + private - attr_reader :config + attr_reader :config, :hits, :new_results def checksum(file) digester = Digest::SHA1.new diff --git a/lib/erb_lint/cli.rb b/lib/erb_lint/cli.rb index 449de305..7270d745 100644 --- a/lib/erb_lint/cli.rb +++ b/lib/erb_lint/cli.rb @@ -66,7 +66,7 @@ def run(args = ARGV) lint_files.each do |filename| runner.clear_offenses begin - puts file_content = run_on_file(runner, filename) + file_content = run_on_file(runner, filename) rescue => e @stats.exceptions += 1 puts "Exception occurred when processing: #{relative_filename(filename)}" @@ -78,6 +78,8 @@ def run(args = ARGV) end end + cache.clean if clean_cache? + reporter.show if stdin? && autocorrect? @@ -111,14 +113,16 @@ def run_on_file(runner, filename) end def run_using_cache(runner, filename) - puts cache.include?(filename) + # puts cache.include?(filename) if cache.include?(filename) && !autocorrect? result = cache[filename] @stats.found += result.size + cache.add_hit(filename) if clean_cache? result else result = run_with_corrections(runner, filename) cache[filename] = result + cache.add_new_result(filename) if clean_cache? end end @@ -130,6 +134,10 @@ def with_cache? @options[:with_cache] end + def clean_cache? + @options[:clean_cache] + end + def run_with_corrections(runner, filename) file_content = read_content(filename) @@ -310,10 +318,14 @@ def option_parser @options[:enabled_linters] = known_linter_names end - opts.on("--with_cache", "Enable caching") do |config| + opts.on("--with-cache", "Enable caching") do |config| @options[:with_cache] = config end + opts.on("--clean-cache", "Clean cache") do |config| + @options[:clean_cache] = config + end + opts.on("--enable-linters LINTER[,LINTER,...]", Array, "Only use specified linter", "Known linters are: #{known_linter_names.join(", ")}") do |linters| linters.each do |linter| From b5aeb01ec9f42e55ce061a9ebd8c57050144ef99 Mon Sep 17 00:00:00 2001 From: Zach Feldman Date: Sat, 13 Aug 2022 14:08:30 -0400 Subject: [PATCH 03/22] Rename clean to prune, implement clearing, add autocorrect error message --- lib/erb_lint/cache.rb | 16 +++++++++++++--- lib/erb_lint/cli.rb | 36 +++++++++++++++++++++++++++--------- 2 files changed, 40 insertions(+), 12 deletions(-) diff --git a/lib/erb_lint/cache.rb b/lib/erb_lint/cache.rb index 3416c8cd..1497948d 100644 --- a/lib/erb_lint/cache.rb +++ b/lib/erb_lint/cache.rb @@ -35,9 +35,9 @@ def add_new_result(filename) @new_results.push(filename) end - def clean + def prune if hits.empty? - puts "Cache being created, skipping clean" + puts "Cache being created for the first time, skipping prune" return end @@ -47,7 +47,7 @@ def clean cache_files.each do |cache_file| next if hits_as_checksums.include?(cache_file) if new_results_as_checksums.include?(cache_file) - puts "Skipping deletion of new cache result #{cache_file} in clean" + puts "Skipping deletion of new cache result #{cache_file} in prune" return end @@ -58,6 +58,16 @@ def clean @hits = [] end + def clear + puts "Clearing cache by deleting cache directory..." + begin + FileUtils.remove_dir(CACHE_DIRECTORY) + puts "...cache directory cleared" + rescue Errno::ENOENT => e + puts "...directory already doesn't exist, skipped deletion." + end + end + private attr_reader :config, :hits, :new_results diff --git a/lib/erb_lint/cli.rb b/lib/erb_lint/cli.rb index 7270d745..3e57f046 100644 --- a/lib/erb_lint/cli.rb +++ b/lib/erb_lint/cli.rb @@ -33,7 +33,18 @@ def run(args = ARGV) @files = @options[:stdin] || dupped_args load_config - @cache = Cache.new(config) + + if with_cache? && autocorrect? + failure!("cannot run autocorrect mode with cache") + end + + @cache = Cache.new(config) if with_cache? || clear_cache? + if clear_cache? + cache.clear + exit 0 + end + + if !@files.empty? && lint_files.empty? if allow_no_files? @@ -78,7 +89,7 @@ def run(args = ARGV) end end - cache.clean if clean_cache? + cache.prune if prune_cache? reporter.show @@ -116,13 +127,12 @@ def run_using_cache(runner, filename) # puts cache.include?(filename) if cache.include?(filename) && !autocorrect? result = cache[filename] - @stats.found += result.size - cache.add_hit(filename) if clean_cache? + cache.add_hit(filename) if prune_cache? result else result = run_with_corrections(runner, filename) cache[filename] = result - cache.add_new_result(filename) if clean_cache? + cache.add_new_result(filename) if prune_cache? end end @@ -134,8 +144,12 @@ def with_cache? @options[:with_cache] end - def clean_cache? - @options[:clean_cache] + def prune_cache? + @options[:prune_cache] + end + + def clear_cache? + @options[:clear_cache] end def run_with_corrections(runner, filename) @@ -322,8 +336,12 @@ def option_parser @options[:with_cache] = config end - opts.on("--clean-cache", "Clean cache") do |config| - @options[:clean_cache] = config + opts.on("--prune-cache", "Prune cache") do |config| + @options[:prune_cache] = config + end + + opts.on("--clear-cache", "Clear cache") do |config| + @options[:clear_cache] = config end opts.on("--enable-linters LINTER[,LINTER,...]", Array, From 95b5e224cf484ad7bcd772281c1c52a78ff3b969 Mon Sep 17 00:00:00 2001 From: Zach Feldman Date: Sat, 13 Aug 2022 14:10:24 -0400 Subject: [PATCH 04/22] Lints and cops --- Gemfile | 2 +- Gemfile.lock | 6 ++++++ lib/erb_lint.rb | 2 +- lib/erb_lint/cache.rb | 23 ++++++++++++----------- lib/erb_lint/cli.rb | 4 +--- 5 files changed, 21 insertions(+), 16 deletions(-) diff --git a/Gemfile b/Gemfile index efd1b411..3ef7f8e2 100644 --- a/Gemfile +++ b/Gemfile @@ -3,7 +3,7 @@ source "https://rubygems.org" gem "rake" -gem 'pry' +gem "pry" group "test" do gem "fakefs" diff --git a/Gemfile.lock b/Gemfile.lock index 72c45979..b5d2ec89 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -32,6 +32,7 @@ GEM parser (>= 2.4) smart_properties builder (3.2.4) + coderay (1.1.3) concurrent-ruby (1.1.10) crass (1.0.6) diff-lcs (1.5.0) @@ -42,6 +43,7 @@ GEM loofah (2.18.0) crass (~> 1.0.2) nokogiri (>= 1.5.9) + method_source (1.0.0) mini_portile2 (2.8.0) minitest (5.16.3) nokogiri (1.13.8) @@ -50,6 +52,9 @@ GEM parallel (1.22.1) parser (3.1.2.1) ast (~> 2.4.1) + pry (0.14.1) + coderay (~> 1.1) + method_source (~> 1.0) racc (1.6.0) rails-dom-testing (2.0.3) activesupport (>= 4.2.0) @@ -98,6 +103,7 @@ PLATFORMS DEPENDENCIES erb_lint! fakefs + pry rake rspec rubocop diff --git a/lib/erb_lint.rb b/lib/erb_lint.rb index e6afe535..66825167 100644 --- a/lib/erb_lint.rb +++ b/lib/erb_lint.rb @@ -1,3 +1,3 @@ # frozen_string_literal: true -require "erb_lint/version" \ No newline at end of file +require "erb_lint/version" diff --git a/lib/erb_lint/cache.rb b/lib/erb_lint/cache.rb index 1497948d..4bf13f06 100644 --- a/lib/erb_lint/cache.rb +++ b/lib/erb_lint/cache.rb @@ -2,7 +2,7 @@ module ERBLint class Cache - CACHE_DIRECTORY = '.erb-lint-cache' + CACHE_DIRECTORY = ".erb-lint-cache" private_constant :CACHE_DIRECTORY def initialize(config) @@ -14,16 +14,16 @@ def initialize(config) def [](filename) JSON.parse(File.read(File.join(CACHE_DIRECTORY, checksum(filename)))) end - + def include?(filename) File.exist?(File.join(CACHE_DIRECTORY, checksum(filename))) end - + def []=(filename, messages) FileUtils.mkdir_p(CACHE_DIRECTORY) - File.open(File.join(CACHE_DIRECTORY, checksum(filename)), 'wb') do |f| - f.write messages.to_json + File.open(File.join(CACHE_DIRECTORY, checksum(filename)), "wb") do |f| + f.write(messages.to_json) end end @@ -42,13 +42,14 @@ def prune 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) } + 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) + if new_results_as_checksums.include?(cache_file) puts "Skipping deletion of new cache result #{cache_file} in prune" - return + next end puts "Cleaning deleted cached file with checksum #{cache_file}}" @@ -63,7 +64,7 @@ def clear begin FileUtils.remove_dir(CACHE_DIRECTORY) puts "...cache directory cleared" - rescue Errno::ENOENT => e + rescue Errno::ENOENT puts "...directory already doesn't exist, skipped deletion." end end @@ -77,14 +78,14 @@ def checksum(file) mode = File.stat(file).mode digester.update( - "#{file}#{mode}#{config.to_s}" + "#{file}#{mode}#{config}" ) digester.file(file) digester.hexdigest rescue Errno::ENOENT # Spurious files that come and go should not cause a crash, at least not # here. - '_' + "_" end end end diff --git a/lib/erb_lint/cli.rb b/lib/erb_lint/cli.rb index 3e57f046..9f18aabb 100644 --- a/lib/erb_lint/cli.rb +++ b/lib/erb_lint/cli.rb @@ -41,11 +41,9 @@ def run(args = ARGV) @cache = Cache.new(config) if with_cache? || clear_cache? if clear_cache? cache.clear - exit 0 + exit(0) end - - if !@files.empty? && lint_files.empty? if allow_no_files? success!("no files found...\n") From 7032a73ee0ed805473a0d0b5c7456d618774b2df Mon Sep 17 00:00:00 2001 From: Zach Feldman Date: Sat, 13 Aug 2022 15:59:37 -0400 Subject: [PATCH 05/22] Add integration specs --- lib/erb_lint/cache.rb | 17 +++++----- lib/erb_lint/cli.rb | 15 ++++++--- spec/erb_lint/cli_spec.rb | 68 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 86 insertions(+), 14 deletions(-) diff --git a/lib/erb_lint/cache.rb b/lib/erb_lint/cache.rb index 4bf13f06..7e21c63e 100644 --- a/lib/erb_lint/cache.rb +++ b/lib/erb_lint/cache.rb @@ -3,12 +3,12 @@ module ERBLint class Cache CACHE_DIRECTORY = ".erb-lint-cache" - private_constant :CACHE_DIRECTORY def initialize(config) @config = config.to_hash @hits = [] @new_results = [] + puts "Cache mode is on" end def [](filename) @@ -59,14 +59,15 @@ def prune @hits = [] end + def cache_dir_exists? + File.directory?(CACHE_DIRECTORY) + end + def clear - puts "Clearing cache by deleting cache directory..." - begin - FileUtils.remove_dir(CACHE_DIRECTORY) - puts "...cache directory cleared" - rescue Errno::ENOENT - puts "...directory already doesn't exist, skipped deletion." - end + return unless cache_dir_exists? + + puts "Clearing cache by deleting cache directory" + FileUtils.rm_r(CACHE_DIRECTORY) end private diff --git a/lib/erb_lint/cli.rb b/lib/erb_lint/cli.rb index 9f18aabb..c4f6c7ab 100644 --- a/lib/erb_lint/cli.rb +++ b/lib/erb_lint/cli.rb @@ -30,18 +30,23 @@ def initialize def run(args = ARGV) dupped_args = args.dup load_options(dupped_args) - @files = @options[:stdin] || dupped_args - - load_config if with_cache? && autocorrect? failure!("cannot run autocorrect mode with cache") end + @files = @options[:stdin] || dupped_args + + load_config + @cache = Cache.new(config) if with_cache? || clear_cache? if clear_cache? - cache.clear - exit(0) + if cache.cache_dir_exists? + cache.clear + success!("cache directory cleared") + else + failure!("cache directory already doesn't exist, skipped deletion.") + end end if !@files.empty? && lint_files.empty? diff --git a/spec/erb_lint/cli_spec.rb b/spec/erb_lint/cli_spec.rb index 77c61b5b..d282b131 100644 --- a/spec/erb_lint/cli_spec.rb +++ b/spec/erb_lint/cli_spec.rb @@ -2,6 +2,7 @@ require "spec_helper" require "erb_lint/cli" +require "erb_lint/cache" require "pp" require "fakefs" require "fakefs/spec_helpers" @@ -97,6 +98,34 @@ def run(processed_source) end end + context "with --clear-cache" do + let(:args) { ["--clear-cache"] } + context "without a cache folder" do + it { expect { subject }.to(output(/cache directory already doesn't exist, skipped deletion/).to_stderr) } + it "shows cache not cleared message if cache is empty and fails" do + expect(subject).to(be(false)) + end + end + + context "with a cache folder" do + before do + FileUtils.mkdir_p(ERBLint::Cache::CACHE_DIRECTORY) + end + it { + expect do + subject + end.to(output(<<~EOF).to_stdout) + Cache mode is on + Clearing cache by deleting cache directory + cache directory cleared + EOF + } + it "is successful and empties the cache if there are cache file" do + expect(subject).to(be(true)) + end + end + end + context "with --config" do context "when file does not exist" do let(:args) { ["--config", ".somefile.yml"] } @@ -170,7 +199,7 @@ def run(processed_source) end end - context "when only errors with serverity info are found" do + context "when only errors with severity info are found" do let(:args) { ["--enable-linter", "linter_with_info_errors", linted_file] } it "shows all error messages and line numbers" do @@ -218,6 +247,19 @@ def run(processed_source) expect(subject).to(be(false)) end end + + context "with cache" do + let(:args) { ["--enable-linter", "linter_without_errors", "--with-cache", linted_file] } + + it "lints the file and adds it to the cache" do + expect(Dir[ERBLint::Cache::CACHE_DIRECTORY].length).to(be(0)) + + expect { subject }.to(output(/Cache mode is on/).to_stdout) + expect(subject).to(be(true)) + + expect(Dir[ERBLint::Cache::CACHE_DIRECTORY].length).to(be(1)) + end + end end end @@ -428,6 +470,19 @@ def run(processed_source) it "is successful" do expect(subject).to(be(true)) end + + context "with cache" do + let(:args) { ["--enable-linter", "linter_without_errors", "--with-cache", linted_dir] } + + it "lints the file and adds it to the cache" do + expect(Dir[ERBLint::Cache::CACHE_DIRECTORY].length).to(be(0)) + + expect { subject }.to(output(/Cache mode is on/).to_stdout) + expect(subject).to(be(true)) + + expect(Dir[ERBLint::Cache::CACHE_DIRECTORY].length).to(be(1)) + end + end end end end @@ -531,6 +586,17 @@ def run(processed_source) expect { subject }.to(output(/#{file_content}\n/).to_stdout) end end + + context "when autocorrecting and caching are turned on" do + # We assume that linter_with_errors is not autocorrectable... + let(:args) do + ["--enable-linter", "linter_without_errors", "--stdin", linted_file, "--autocorrect", "--with-cache"] + end + + it "throws an error saying the two modes cannot be used together" do + expect { subject }.to(output(/cannot run autocorrect mode with cache/).to_stderr) + end + end end context "when no errors are found" do From 6012ae87913ed0e0517ec1acf07766bbe35977fa Mon Sep 17 00:00:00 2001 From: Zach Feldman Date: Sun, 14 Aug 2022 13:41:52 -0400 Subject: [PATCH 06/22] Fix cache implementation, clean up code --- lib/erb_lint/cache.rb | 17 +++++++++------- lib/erb_lint/cli.rb | 29 ++++++++++++++------------- lib/erb_lint/linter.rb | 2 +- lib/erb_lint/offense.rb | 43 +++++++++++++++++++++++++++++++++++++++++ lib/erb_lint/runner.rb | 4 ++++ 5 files changed, 74 insertions(+), 21 deletions(-) diff --git a/lib/erb_lint/cache.rb b/lib/erb_lint/cache.rb index 7e21c63e..6fb0e0c0 100644 --- a/lib/erb_lint/cache.rb +++ b/lib/erb_lint/cache.rb @@ -4,26 +4,29 @@ module ERBLint class Cache CACHE_DIRECTORY = ".erb-lint-cache" - def initialize(config) - @config = config.to_hash + def initialize(config, file_loader = nil) + @config = config + @file_loader = file_loader @hits = [] @new_results = [] puts "Cache mode is on" end def [](filename) - JSON.parse(File.read(File.join(CACHE_DIRECTORY, checksum(filename)))) + JSON.parse(File.read(File.join(CACHE_DIRECTORY, checksum(filename)))).map do |offense| + ERBLint::Offense.from_json(offense, @file_loader, config) + end end def include?(filename) File.exist?(File.join(CACHE_DIRECTORY, checksum(filename))) end - def []=(filename, messages) + def []=(filename, offenses_as_json) FileUtils.mkdir_p(CACHE_DIRECTORY) File.open(File.join(CACHE_DIRECTORY, checksum(filename)), "wb") do |f| - f.write(messages.to_json) + f.write(offenses_as_json) end end @@ -52,7 +55,7 @@ def prune next end - puts "Cleaning deleted cached file with checksum #{cache_file}}" + puts "Cleaning deleted cached file with checksum #{cache_file}" File.delete(File.join(CACHE_DIRECTORY, cache_file)) end @@ -79,7 +82,7 @@ def checksum(file) mode = File.stat(file).mode digester.update( - "#{file}#{mode}#{config}" + "#{file}#{mode}#{config.to_hash}" ) digester.file(file) digester.hexdigest diff --git a/lib/erb_lint/cli.rb b/lib/erb_lint/cli.rb index c4f6c7ab..15aa2673 100644 --- a/lib/erb_lint/cli.rb +++ b/lib/erb_lint/cli.rb @@ -39,7 +39,7 @@ def run(args = ARGV) load_config - @cache = Cache.new(config) if with_cache? || clear_cache? + @cache = Cache.new(@config, file_loader) if with_cache? || clear_cache? if clear_cache? if cache.cache_dir_exists? cache.clear @@ -119,22 +119,24 @@ def run(args = ARGV) attr_reader :cache, :config def run_on_file(runner, filename) + file_content = read_content(filename) if with_cache? && !autocorrect? - run_using_cache(runner, filename) + run_using_cache(runner, filename, file_content) else - run_with_corrections(runner, filename) + file_content = run_with_corrections(runner, filename, file_content) end + log_offense_stats(runner, filename) + file_content end - def run_using_cache(runner, filename) + def run_using_cache(runner, filename, file_content) # puts cache.include?(filename) if cache.include?(filename) && !autocorrect? - result = cache[filename] + runner.restore_offenses(cache[filename]) cache.add_hit(filename) if prune_cache? - result else - result = run_with_corrections(runner, filename) - cache[filename] = result + 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? end end @@ -155,9 +157,7 @@ def clear_cache? @options[:clear_cache] end - def run_with_corrections(runner, filename) - file_content = read_content(filename) - + def run_with_corrections(runner, filename, file_content) 7.times do processed_source = ERBLint::ProcessedSource.new(filename, file_content) runner.run(processed_source) @@ -179,6 +179,11 @@ def run_with_corrections(runner, filename) file_content = corrector.corrected_content runner.clear_offenses end + + file_content + end + + def log_offense_stats(runner, filename) offenses_filename = relative_filename(filename) offenses = runner.offenses || [] @@ -190,8 +195,6 @@ def run_with_corrections(runner, filename) @stats.processed_files[offenses_filename] ||= [] @stats.processed_files[offenses_filename] |= offenses - - file_content end def read_content(filename) diff --git a/lib/erb_lint/linter.rb b/lib/erb_lint/linter.rb index 36c62511..3d452c90 100644 --- a/lib/erb_lint/linter.rb +++ b/lib/erb_lint/linter.rb @@ -30,7 +30,7 @@ def support_autocorrect? end end - attr_reader :offenses + attr_reader :offenses, :config # Must be implemented by the concrete inheriting class. def initialize(file_loader, config) diff --git a/lib/erb_lint/offense.rb b/lib/erb_lint/offense.rb index dc66014c..d730f9e4 100644 --- a/lib/erb_lint/offense.rb +++ b/lib/erb_lint/offense.rb @@ -17,6 +17,49 @@ def initialize(linter, source_range, message, context = nil, severity = nil) @severity = severity end + def to_json_format + { + linter_config: linter.config.to_hash, + 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_source: source_range.source_buffer.source, + source_buffer_name: source_range.source_buffer.name, + } + end + + def self.from_json(parsed_json, file_loader, config) + parsed_json.transform_keys!(&:to_sym) + linter_config = ERBLint::LinterConfig.new(parsed_json[:linter_config]) + new( + linter_config, + source_range_from_json_format(parsed_json[:source_range]), + parsed_json[:message].presence, + parsed_json[:context].presence, + parsed_json[:severity].presence + ) + end + + def self.source_range_from_json_format(parsed_json_source_range) + parsed_json_source_range.transform_keys!(&:to_sym) + Parser::Source::Range.new( + Parser::Source::Buffer.new( + parsed_json_source_range[:source_buffer_name], + source: parsed_json_source_range[:source_buffer_source] + ), + parsed_json_source_range[:begin_pos], + parsed_json_source_range[:end_pos] + ) + end + def inspect "#<#{self.class.name} linter=#{linter.class.name} "\ "source_range=#{source_range.begin_pos}...#{source_range.end_pos} "\ diff --git a/lib/erb_lint/runner.rb b/lib/erb_lint/runner.rb index 3965872c..f769f25a 100644 --- a/lib/erb_lint/runner.rb +++ b/lib/erb_lint/runner.rb @@ -30,5 +30,9 @@ def clear_offenses @offenses = [] @linters.each(&:clear_offenses) end + + def restore_offenses(offenses) + @offenses.concat(offenses) + end end end From ad47b9832df9a71da9f65ffd52100f4e453688b7 Mon Sep 17 00:00:00 2001 From: Zach Feldman Date: Sun, 14 Aug 2022 20:09:07 -0400 Subject: [PATCH 07/22] Specs for cache and associated changes --- lib/erb_lint/cache.rb | 4 +- lib/erb_lint/cli.rb | 2 +- lib/erb_lint/offense.rb | 18 +- spec/erb_lint/cache_spec.rb | 172 ++++++++++++++++++ spec/erb_lint/fixtures/cache_file_content | 1 + .../fixtures/image_component.html.erb | 11 ++ spec/erb_lint/fixtures/rubocop.yml | 2 + 7 files changed, 199 insertions(+), 11 deletions(-) create mode 100644 spec/erb_lint/cache_spec.rb create mode 100644 spec/erb_lint/fixtures/cache_file_content create mode 100644 spec/erb_lint/fixtures/image_component.html.erb create mode 100644 spec/erb_lint/fixtures/rubocop.yml diff --git a/lib/erb_lint/cache.rb b/lib/erb_lint/cache.rb index 6fb0e0c0..5547da1a 100644 --- a/lib/erb_lint/cache.rb +++ b/lib/erb_lint/cache.rb @@ -12,9 +12,9 @@ def initialize(config, file_loader = nil) puts "Cache mode is on" end - def [](filename) + def get(filename, file_content) JSON.parse(File.read(File.join(CACHE_DIRECTORY, checksum(filename)))).map do |offense| - ERBLint::Offense.from_json(offense, @file_loader, config) + ERBLint::Offense.from_json(offense, config, @file_loader, file_content) end end diff --git a/lib/erb_lint/cli.rb b/lib/erb_lint/cli.rb index 15aa2673..7d9a7a7c 100644 --- a/lib/erb_lint/cli.rb +++ b/lib/erb_lint/cli.rb @@ -132,7 +132,7 @@ def run_on_file(runner, filename) def run_using_cache(runner, filename, file_content) # puts cache.include?(filename) if cache.include?(filename) && !autocorrect? - runner.restore_offenses(cache[filename]) + runner.restore_offenses(cache.get(filename, file_content)) cache.add_hit(filename) if prune_cache? else run_with_corrections(runner, filename, file_content) diff --git a/lib/erb_lint/offense.rb b/lib/erb_lint/offense.rb index d730f9e4..8c306239 100644 --- a/lib/erb_lint/offense.rb +++ b/lib/erb_lint/offense.rb @@ -20,6 +20,8 @@ def initialize(linter, source_range, message, context = nil, severity = nil) 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, @@ -31,29 +33,29 @@ def source_range_to_json_format { begin_pos: source_range.begin_pos, end_pos: source_range.end_pos, - source_buffer_source: source_range.source_buffer.source, source_buffer_name: source_range.source_buffer.name, } end - def self.from_json(parsed_json, file_loader, config) + def self.from_json(parsed_json, config, file_loader, file_content) parsed_json.transform_keys!(&:to_sym) - linter_config = ERBLint::LinterConfig.new(parsed_json[:linter_config]) + 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_config, - source_range_from_json_format(parsed_json[:source_range]), + linter, + source_range_from_json_format(parsed_json[:source_range], file_content), parsed_json[:message].presence, parsed_json[:context].presence, - parsed_json[:severity].presence + parsed_json[:severity].presence&.to_sym ) end - def self.source_range_from_json_format(parsed_json_source_range) + 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: parsed_json_source_range[:source_buffer_source] + source: file_content ), parsed_json_source_range[:begin_pos], parsed_json_source_range[:end_pos] diff --git a/spec/erb_lint/cache_spec.rb b/spec/erb_lint/cache_spec.rb new file mode 100644 index 00000000..3f6e542f --- /dev/null +++ b/spec/erb_lint/cache_spec.rb @@ -0,0 +1,172 @@ +# frozen_string_literal: true + +require "spec_helper" + +require "fakefs" +require "fakefs/spec_helpers" +require "erb_lint/cache" + +require "pry" + +describe ERBLint::Cache do + include FakeFS::SpecHelpers + + let(:linter_config) { ERBLint::LinterConfig.new } + let(:cache) { described_class.new(linter_config) } + let(:linted_file_path) { "app/components/elements/image_component/image_component.html.erb" } + let(:file_content) do + %() + end + let(:checksum) { "835c15465bc22783257bdafb33acc2d78c29abaa" } + let(:cache_dir) { ERBLint::Cache::CACHE_DIRECTORY } + let(:rubocop_yml) { %(SpaceAroundErbTag:\n Enabled: true\n) } + let(:cache_file_content) do + FakeFS.deactivate! + content = File.read(File.expand_path("./spec/erb_lint/fixtures/cache_file_content")) + FakeFS.activate! + content + end + let(:linted_file_content) do + FakeFS.deactivate! + content = File.read(File.expand_path("./spec/erb_lint/fixtures/image_component.html.erb")) + FakeFS.activate! + content + end + + around do |example| + FakeFS do + example.run + end + end + + before do + FileUtils.mkdir_p(File.dirname(linted_file_path)) + File.write(linted_file_path, linted_file_content) + + FileUtils.mkdir_p(cache_dir) + File.open(File.join(cache_dir, checksum), "wb") do |f| + f.write(cache_file_content) + end + + allow(::RuboCop::ConfigLoader).to(receive(:load_file).and_call_original) + allow(::RuboCop::ConfigLoader).to(receive(:read_file).and_return(rubocop_yml)) + end + + describe "#get" do + it "returns a cached lint result" do + cache_result = cache.get(linted_file_path, cache_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 + expect(File.exist?( + File.join( + cache_dir, + checksum + ) + )).to(be(true)) + 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)) + end + it "returns false if the cache dir does not exist" do + FileUtils.rm_rf(cache_dir) + expect(cache.cache_dir_exists?).to(be(false)) + end + end + + describe "#clear" do + it "deletes the cache directory" do + cache.clear + expect(File.directory?(cache_dir)).to(be(false)) + end + end + + describe "#prune" 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) + end + + it "does not prune actual cache hits" do + cache.prune + + expect(File.exist?( + File.join( + cache_dir, + checksum + ) + )).to(be(true)) + end + + 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])) + 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(File.exist?( + File.join( + cache_dir, + checksum + ) + )).to(be(true)) + end + + it "prunes outdated cache results" do + fakefs_dir = Struct.new(:fakefs_dir) + allow(fakefs_dir).to(receive(:children).and_return([checksum, "fake-checksum"])) + allow(FakeFS::Dir).to(receive(:new).and_return(fakefs_dir)) + allow(cache).to(receive(:hits).and_return([linted_file_path])) + + File.open(File.join(cache_dir, "fake-checksum"), "wb") do |f| + f.write(cache_file_content) + end + + expect { cache.prune }.to(output(/Cleaning deleted cached file with checksum fake-checksum/).to_stdout) + + expect(File.exist?( + File.join( + cache_dir, + "fake-checksum" + ) + )).to(be(false)) + 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) + + expect(cache.send(:new_results)).to(include(linted_file_path)) + end + end + + describe "#add_hit" do + it "adds new cache hit to cache object hits attribute" do + cache.add_hit(linted_file_path) + + expect(cache.send(:hits)).to(include(linted_file_path)) + end + end +end diff --git a/spec/erb_lint/fixtures/cache_file_content b/spec/erb_lint/fixtures/cache_file_content new file mode 100644 index 00000000..e044f385 --- /dev/null +++ b/spec/erb_lint/fixtures/cache_file_content @@ -0,0 +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}] \ No newline at end of file diff --git a/spec/erb_lint/fixtures/image_component.html.erb b/spec/erb_lint/fixtures/image_component.html.erb new file mode 100644 index 00000000..a74c44ba --- /dev/null +++ b/spec/erb_lint/fixtures/image_component.html.erb @@ -0,0 +1,11 @@ +<%if variable %> +
+
+ <%= render_image %> +
+ <%= link_to "Hello World", root_path %> +
+
+<% else%> + <%= render_image %> +<% end %> \ No newline at end of file diff --git a/spec/erb_lint/fixtures/rubocop.yml b/spec/erb_lint/fixtures/rubocop.yml new file mode 100644 index 00000000..fc768c91 --- /dev/null +++ b/spec/erb_lint/fixtures/rubocop.yml @@ -0,0 +1,2 @@ +SpaceAroundErbTag: + Enabled: true From fbbcd2a218c831a0d7c33e645dcecb131ceba5df Mon Sep 17 00:00:00 2001 From: Zach Feldman Date: Sun, 14 Aug 2022 20:15:37 -0400 Subject: [PATCH 08/22] Clean up --- Gemfile | 1 - Gemfile.lock | 6 ------ lib/erb_lint/cli.rb | 1 - spec/erb_lint/cache_spec.rb | 8 +------- 4 files changed, 1 insertion(+), 15 deletions(-) diff --git a/Gemfile b/Gemfile index 3ef7f8e2..548e391e 100644 --- a/Gemfile +++ b/Gemfile @@ -3,7 +3,6 @@ source "https://rubygems.org" gem "rake" -gem "pry" group "test" do gem "fakefs" diff --git a/Gemfile.lock b/Gemfile.lock index b5d2ec89..72c45979 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -32,7 +32,6 @@ GEM parser (>= 2.4) smart_properties builder (3.2.4) - coderay (1.1.3) concurrent-ruby (1.1.10) crass (1.0.6) diff-lcs (1.5.0) @@ -43,7 +42,6 @@ GEM loofah (2.18.0) crass (~> 1.0.2) nokogiri (>= 1.5.9) - method_source (1.0.0) mini_portile2 (2.8.0) minitest (5.16.3) nokogiri (1.13.8) @@ -52,9 +50,6 @@ GEM parallel (1.22.1) parser (3.1.2.1) ast (~> 2.4.1) - pry (0.14.1) - coderay (~> 1.1) - method_source (~> 1.0) racc (1.6.0) rails-dom-testing (2.0.3) activesupport (>= 4.2.0) @@ -103,7 +98,6 @@ PLATFORMS DEPENDENCIES erb_lint! fakefs - pry rake rspec rubocop diff --git a/lib/erb_lint/cli.rb b/lib/erb_lint/cli.rb index 7d9a7a7c..d811f9a9 100644 --- a/lib/erb_lint/cli.rb +++ b/lib/erb_lint/cli.rb @@ -130,7 +130,6 @@ def run_on_file(runner, filename) end def run_using_cache(runner, filename, file_content) - # puts cache.include?(filename) if cache.include?(filename) && !autocorrect? runner.restore_offenses(cache.get(filename, file_content)) cache.add_hit(filename) if prune_cache? diff --git a/spec/erb_lint/cache_spec.rb b/spec/erb_lint/cache_spec.rb index 3f6e542f..a4098dc9 100644 --- a/spec/erb_lint/cache_spec.rb +++ b/spec/erb_lint/cache_spec.rb @@ -1,12 +1,9 @@ # frozen_string_literal: true require "spec_helper" - +require "erb_lint/cache" require "fakefs" require "fakefs/spec_helpers" -require "erb_lint/cache" - -require "pry" describe ERBLint::Cache do include FakeFS::SpecHelpers @@ -14,9 +11,6 @@ let(:linter_config) { ERBLint::LinterConfig.new } let(:cache) { described_class.new(linter_config) } let(:linted_file_path) { "app/components/elements/image_component/image_component.html.erb" } - let(:file_content) do - %() - end let(:checksum) { "835c15465bc22783257bdafb33acc2d78c29abaa" } let(:cache_dir) { ERBLint::Cache::CACHE_DIRECTORY } let(:rubocop_yml) { %(SpaceAroundErbTag:\n Enabled: true\n) } From a292ef095c4726ea437d6034bee9f6df2d579e6f Mon Sep 17 00:00:00 2001 From: Zach Feldman Date: Thu, 18 Aug 2022 20:13:26 -0400 Subject: [PATCH 09/22] Include ERBLint version in cache key --- lib/erb_lint/cache.rb | 2 +- spec/erb_lint/cache_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/erb_lint/cache.rb b/lib/erb_lint/cache.rb index 5547da1a..a2760e0d 100644 --- a/lib/erb_lint/cache.rb +++ b/lib/erb_lint/cache.rb @@ -82,7 +82,7 @@ def checksum(file) mode = File.stat(file).mode digester.update( - "#{file}#{mode}#{config.to_hash}" + "#{file}#{mode}#{config.to_hash}#{ERBLint::VERSION}" ) digester.file(file) digester.hexdigest diff --git a/spec/erb_lint/cache_spec.rb b/spec/erb_lint/cache_spec.rb index a4098dc9..ce7dde19 100644 --- a/spec/erb_lint/cache_spec.rb +++ b/spec/erb_lint/cache_spec.rb @@ -11,7 +11,7 @@ let(:linter_config) { ERBLint::LinterConfig.new } let(:cache) { described_class.new(linter_config) } let(:linted_file_path) { "app/components/elements/image_component/image_component.html.erb" } - let(:checksum) { "835c15465bc22783257bdafb33acc2d78c29abaa" } + let(:checksum) { "2dc3e17183b87889cc783b0157723570d4bbb90a" } let(:cache_dir) { ERBLint::Cache::CACHE_DIRECTORY } let(:rubocop_yml) { %(SpaceAroundErbTag:\n Enabled: true\n) } let(:cache_file_content) do From be113700428906b82974c02a50c42a342c87f397 Mon Sep 17 00:00:00 2001 From: Zach Feldman Date: Thu, 18 Aug 2022 20:19:23 -0400 Subject: [PATCH 10/22] Fix nonsensical error message --- lib/erb_lint/cli.rb | 2 +- spec/erb_lint/cli_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/erb_lint/cli.rb b/lib/erb_lint/cli.rb index d811f9a9..1644e712 100644 --- a/lib/erb_lint/cli.rb +++ b/lib/erb_lint/cli.rb @@ -45,7 +45,7 @@ def run(args = ARGV) cache.clear success!("cache directory cleared") else - failure!("cache directory already doesn't exist, skipped deletion.") + failure!("cache directory doesn't exist, skipping deletion.") end end diff --git a/spec/erb_lint/cli_spec.rb b/spec/erb_lint/cli_spec.rb index d282b131..25395dcc 100644 --- a/spec/erb_lint/cli_spec.rb +++ b/spec/erb_lint/cli_spec.rb @@ -101,7 +101,7 @@ def run(processed_source) context "with --clear-cache" do let(:args) { ["--clear-cache"] } context "without a cache folder" do - it { expect { subject }.to(output(/cache directory already doesn't exist, skipped deletion/).to_stderr) } + it { expect { subject }.to(output(/cache directory doesn't exist, skipping deletion/).to_stderr) } it "shows cache not cleared message if cache is empty and fails" do expect(subject).to(be(false)) end From deeb4f80efd8d7a33d33bbdadeee3c9376d77579 Mon Sep 17 00:00:00 2001 From: Zach Feldman Date: Thu, 18 Aug 2022 20:31:46 -0400 Subject: [PATCH 11/22] Spacing fixes and removing unneeded code --- lib/erb_lint/cli.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/erb_lint/cli.rb b/lib/erb_lint/cli.rb index 1644e712..016c370d 100644 --- a/lib/erb_lint/cli.rb +++ b/lib/erb_lint/cli.rb @@ -40,6 +40,7 @@ def run(args = ARGV) load_config @cache = Cache.new(@config, file_loader) if with_cache? || clear_cache? + if clear_cache? if cache.cache_dir_exists? cache.clear @@ -120,17 +121,19 @@ def run(args = ARGV) def run_on_file(runner, filename) file_content = read_content(filename) + if with_cache? && !autocorrect? run_using_cache(runner, filename, file_content) else file_content = run_with_corrections(runner, filename, file_content) end + log_offense_stats(runner, filename) file_content end def run_using_cache(runner, filename, file_content) - if cache.include?(filename) && !autocorrect? + if cache.include?(filename) runner.restore_offenses(cache.get(filename, file_content)) cache.add_hit(filename) if prune_cache? else From 1a5db058f820ec6c40bbcda0e423aff0d42a86c0 Mon Sep 17 00:00:00 2001 From: Zach Feldman Date: Fri, 19 Aug 2022 22:11:12 -0400 Subject: [PATCH 12/22] Fix tests by mocking returned checksum --- spec/erb_lint/cache_spec.rb | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/spec/erb_lint/cache_spec.rb b/spec/erb_lint/cache_spec.rb index ce7dde19..5651cf20 100644 --- a/spec/erb_lint/cache_spec.rb +++ b/spec/erb_lint/cache_spec.rb @@ -42,8 +42,11 @@ f.write(cache_file_content) end - allow(::RuboCop::ConfigLoader).to(receive(:load_file).and_call_original) - allow(::RuboCop::ConfigLoader).to(receive(:read_file).and_return(rubocop_yml)) + digest_sha1_double = instance_double(Digest::SHA1) + allow(Digest::SHA1).to(receive(:new).and_return(digest_sha1_double)) + allow(digest_sha1_double).to(receive(:hexdigest).and_return(checksum)) + allow(digest_sha1_double).to(receive(:update).and_return(true)) + allow(digest_sha1_double).to(receive(:file).and_return(true)) end describe "#get" do @@ -127,7 +130,7 @@ )).to(be(true)) end - it "prunes outdated cache results" do + it "prunes unused cache results" do fakefs_dir = Struct.new(:fakefs_dir) allow(fakefs_dir).to(receive(:children).and_return([checksum, "fake-checksum"])) allow(FakeFS::Dir).to(receive(:new).and_return(fakefs_dir)) From 8d4eaa86662ba9c34bf90259168cdda7a83660a1 Mon Sep 17 00:00:00 2001 From: Zach Feldman Date: Sat, 8 Oct 2022 14:07:56 -0400 Subject: [PATCH 13/22] Move cache pruning to the cache class --- lib/erb_lint/cache.rb | 24 +++++++++++++++--------- lib/erb_lint/cli.rb | 6 ++---- spec/erb_lint/cache_spec.rb | 34 ++++++++++++++++++++++------------ 3 files changed, 39 insertions(+), 25 deletions(-) diff --git a/lib/erb_lint/cache.rb b/lib/erb_lint/cache.rb index a2760e0d..425cc801 100644 --- a/lib/erb_lint/cache.rb +++ b/lib/erb_lint/cache.rb @@ -4,15 +4,18 @@ 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) + @hits.push(filename) if prune? + JSON.parse(File.read(File.join(CACHE_DIRECTORY, checksum(filename)))).map do |offense| ERBLint::Offense.from_json(offense, config, @file_loader, file_content) end @@ -23,6 +26,8 @@ def include?(filename) end def []=(filename, offenses_as_json) + @new_results.push(filename) if prune? + FileUtils.mkdir_p(CACHE_DIRECTORY) File.open(File.join(CACHE_DIRECTORY, checksum(filename)), "wb") do |f| @@ -30,15 +35,12 @@ def []=(filename, 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 @@ -51,7 +53,7 @@ def prune next if hits_as_checksums.include?(cache_file) if new_results_as_checksums.include?(cache_file) - puts "Skipping deletion of new cache result #{cache_file} in prune" + puts "Skipping deletion of new cache result #{cache_file}" next end @@ -91,5 +93,9 @@ def checksum(file) # here. "_" end + + def prune? + @prune + end end end diff --git a/lib/erb_lint/cli.rb b/lib/erb_lint/cli.rb index 016c370d..272bf2ad 100644 --- a/lib/erb_lint/cli.rb +++ b/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 @@ -135,11 +135,9 @@ def run_on_file(runner, filename) 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? 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? end end diff --git a/spec/erb_lint/cache_spec.rb b/spec/erb_lint/cache_spec.rb index 5651cf20..b30f238a 100644 --- a/spec/erb_lint/cache_spec.rb +++ b/spec/erb_lint/cache_spec.rb @@ -95,15 +95,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( @@ -120,7 +120,7 @@ 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 +140,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 +151,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 + + it "adds new result to cache object new_results list attribute" do + cache[linted_file_path] = cache_file_content expect(cache.send(:new_results)).to(include(linted_file_path)) 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(linted_file_path)) end end + + 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 From cdf089ccb2d92a8dbfc2cf179d3dcd9c7a70f5b9 Mon Sep 17 00:00:00 2001 From: Zach Feldman Date: Sat, 8 Oct 2022 16:01:57 -0400 Subject: [PATCH 14/22] Do less file reads and stats --- lib/erb_lint/cache.rb | 39 +++++++++++++++++++------------------ lib/erb_lint/cli.rb | 6 +++--- spec/erb_lint/cache_spec.rb | 24 +++++++---------------- 3 files changed, 30 insertions(+), 39 deletions(-) diff --git a/lib/erb_lint/cache.rb b/lib/erb_lint/cache.rb index 425cc801..ee8fb639 100644 --- a/lib/erb_lint/cache.rb +++ b/lib/erb_lint/cache.rb @@ -14,23 +14,27 @@ def initialize(config, file_loader = nil, prune = false) end def get(filename, file_content) - @hits.push(filename) if prune? - - 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::Offense.from_json(offense, config, @file_loader, file_content) + 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 []=(filename, offenses_as_json) - @new_results.push(filename) if prune? + def set(filename, file_content, offenses_as_json) + file_checksum = checksum(filename, file_content) + @new_results.push(file_checksum) if prune? 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 @@ -47,12 +51,10 @@ def prune_cache 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) + if new_results.include?(cache_file) puts "Skipping deletion of new cache result #{cache_file}" next end @@ -79,14 +81,13 @@ 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 diff --git a/lib/erb_lint/cli.rb b/lib/erb_lint/cli.rb index 272bf2ad..7cedf1df 100644 --- a/lib/erb_lint/cli.rb +++ b/lib/erb_lint/cli.rb @@ -133,11 +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)) + 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.set(filename, file_content, runner.offenses.map(&:to_json_format).to_json) end end diff --git a/spec/erb_lint/cache_spec.rb b/spec/erb_lint/cache_spec.rb index b30f238a..141ccb75 100644 --- a/spec/erb_lint/cache_spec.rb +++ b/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)) @@ -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_cache }.to(output(/Skipping deletion of new cache result #{checksum}/).to_stdout) + expect { cache.prune_cache }.to(output(/.*Skipping deletion of new cache result #{checksum}/).to_stdout) expect(File.exist?( File.join( @@ -157,15 +147,15 @@ end it "adds new result to cache object new_results list attribute" do - cache[linted_file_path] = cache_file_content + cache.set(linted_file_path, linted_file_content, cache_file_content) - expect(cache.send(:new_results)).to(include(linted_file_path)) + expect(cache.send(:new_results)).to(include(checksum)) end 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(linted_file_path)) + expect(cache.send(:hits)).to(include(checksum)) end end From b331799a1a0543d29cdbf33a94a7a7da871a90c3 Mon Sep 17 00:00:00 2001 From: Zach Feldman Date: Mon, 10 Oct 2022 15:48:52 -0400 Subject: [PATCH 15/22] Attempt a new implementation for storing offenses in a CachedOffense for speed --- lib/erb_lint/all.rb | 1 + lib/erb_lint/cache.rb | 2 +- lib/erb_lint/cached_offense.rb | 39 ++++++++++++++++++++ lib/erb_lint/cli.rb | 2 +- lib/erb_lint/offense.rb | 45 +---------------------- spec/erb_lint/fixtures/cache_file_content | 2 +- 6 files changed, 45 insertions(+), 46 deletions(-) create mode 100644 lib/erb_lint/cached_offense.rb diff --git a/lib/erb_lint/all.rb b/lib/erb_lint/all.rb index 8280343a..4aee601c 100644 --- a/lib/erb_lint/all.rb +++ b/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" diff --git a/lib/erb_lint/cache.rb b/lib/erb_lint/cache.rb index ee8fb639..73d1f42c 100644 --- a/lib/erb_lint/cache.rb +++ b/lib/erb_lint/cache.rb @@ -19,7 +19,7 @@ def get(filename, file_content) cache_file_contents_as_offenses = JSON.parse( File.read(File.join(CACHE_DIRECTORY, file_checksum)) ).map do |offense| - ERBLint::Offense.from_json(offense, config, @file_loader, file_content) + ERBLint::CachedOffense.from_json(offense) end rescue Errno::ENOENT return false diff --git a/lib/erb_lint/cached_offense.rb b/lib/erb_lint/cached_offense.rb new file mode 100644 index 00000000..7710d332 --- /dev/null +++ b/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 diff --git a/lib/erb_lint/cli.rb b/lib/erb_lint/cli.rb index 7cedf1df..d317c048 100644 --- a/lib/erb_lint/cli.rb +++ b/lib/erb_lint/cli.rb @@ -137,7 +137,7 @@ def run_using_cache(runner, filename, file_content) runner.restore_offenses(cache_result_offenses) else run_with_corrections(runner, filename, file_content) - cache.set(filename, file_content, runner.offenses.map(&:to_json_format).to_json) + cache.set(filename, file_content, runner.offenses.map(&:to_cached_offense_json_format).to_json) end end diff --git a/lib/erb_lint/offense.rb b/lib/erb_lint/offense.rb index 8c306239..9e76bcb5 100644 --- a/lib/erb_lint/offense.rb +++ b/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 diff --git a/spec/erb_lint/fixtures/cache_file_content b/spec/erb_lint/fixtures/cache_file_content index e044f385..62945f92 100644 --- a/spec/erb_lint/fixtures/cache_file_content +++ b/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}] \ No newline at end of file +[{"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"}] \ No newline at end of file From ce8d6da6d4d51cc460930bc905564485fd1f357a Mon Sep 17 00:00:00 2001 From: Zach Feldman Date: Tue, 11 Oct 2022 21:32:40 -0400 Subject: [PATCH 16/22] Update lib/erb_lint/cached_offense.rb to avoid errors on nil severity Co-authored-by: Eric Terry --- lib/erb_lint/cached_offense.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/erb_lint/cached_offense.rb b/lib/erb_lint/cached_offense.rb index 7710d332..4a726934 100644 --- a/lib/erb_lint/cached_offense.rb +++ b/lib/erb_lint/cached_offense.rb @@ -32,7 +32,7 @@ def self.from_json(parsed_json) new( parsed_json[:message], parsed_json[:line_number], - parsed_json[:severity].to_sym + parsed_json[:severity]&.to_sym ) end end From acba4a9011cf3e2e0479af0f77804030a14dd163 Mon Sep 17 00:00:00 2001 From: Eric Terry Date: Tue, 11 Oct 2022 23:56:52 -0500 Subject: [PATCH 17/22] Support caching with json and compact reporters --- lib/erb_lint/cache.rb | 4 +- lib/erb_lint/cached_offense.rb | 55 +++++++++++++------ lib/erb_lint/cli.rb | 2 +- lib/erb_lint/offense.rb | 20 ++++++- lib/erb_lint/reporters/json_reporter.rb | 8 +-- spec/erb_lint/fixtures/cache_file_content | 2 +- spec/erb_lint/reporters/json_reporter_spec.rb | 22 +++----- 7 files changed, 71 insertions(+), 42 deletions(-) diff --git a/lib/erb_lint/cache.rb b/lib/erb_lint/cache.rb index 73d1f42c..79b88a81 100644 --- a/lib/erb_lint/cache.rb +++ b/lib/erb_lint/cache.rb @@ -18,8 +18,8 @@ def get(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) + ).map do |offense_hash| + ERBLint::CachedOffense.new(offense_hash) end rescue Errno::ENOENT return false diff --git a/lib/erb_lint/cached_offense.rb b/lib/erb_lint/cached_offense.rb index 4a726934..3c5fa74e 100644 --- a/lib/erb_lint/cached_offense.rb +++ b/lib/erb_lint/cached_offense.rb @@ -3,37 +3,56 @@ module ERBLint # A Cached version of an Offense with only essential information represented as strings class CachedOffense - attr_reader :line_number, :message, :severity + attr_reader( + :message, + :line_number, + :severity, + :column, + :simple_name, + :last_line, + :last_column, + :length, + ) - def initialize(message, line_number, severity) - @message = message - @line_number = line_number - @severity = severity + def initialize(params) + params = params.transform_keys(&:to_sym) + + @message = params[:message] + @line_number = params[:line_number] + @severity = params[:severity]&.to_sym + @column = params[:column] + @simple_name = params[:simple_name] + @last_line = params[:last_line] + @last_column = params[:last_column] + @length = params[:length] end def self.new_from_offense(offense) new( - offense.message, - offense.line_number.to_s, - offense.severity + { + message: offense.message, + line_number: offense.line_number, + severity: offense.severity, + column: offense.column, + simple_name: offense.simple_name, + last_line: offense.last_line, + last_column: offense.last_column, + length: offense.length, + } ) end - def to_json_format + def to_h { message: message, line_number: line_number, severity: severity, + column: column, + simple_name: simple_name, + last_line: last_line, + last_column: last_column, + length: length, } 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 diff --git a/lib/erb_lint/cli.rb b/lib/erb_lint/cli.rb index d317c048..72b8c131 100644 --- a/lib/erb_lint/cli.rb +++ b/lib/erb_lint/cli.rb @@ -137,7 +137,7 @@ def run_using_cache(runner, filename, file_content) runner.restore_offenses(cache_result_offenses) else run_with_corrections(runner, filename, file_content) - cache.set(filename, file_content, runner.offenses.map(&:to_cached_offense_json_format).to_json) + cache.set(filename, file_content, runner.offenses.map(&:to_cached_offense_hash).to_json) end end diff --git a/lib/erb_lint/offense.rb b/lib/erb_lint/offense.rb index 9e76bcb5..5c8db6b7 100644 --- a/lib/erb_lint/offense.rb +++ b/lib/erb_lint/offense.rb @@ -17,8 +17,8 @@ def initialize(linter, source_range, message, context = nil, severity = nil) @severity = severity end - def to_cached_offense_json_format - ERBLint::CachedOffense.new_from_offense(self).to_json_format + def to_cached_offense_hash + ERBLint::CachedOffense.new_from_offense(self).to_h end def inspect @@ -47,5 +47,21 @@ def line_number def column source_range.column end + + def simple_name + linter.class.simple_name + end + + def last_line + source_range.last_line + end + + def last_column + source_range.last_column + end + + def length + source_range.length + end end end diff --git a/lib/erb_lint/reporters/json_reporter.rb b/lib/erb_lint/reporters/json_reporter.rb index c3f937f2..2abf35e7 100644 --- a/lib/erb_lint/reporters/json_reporter.rb +++ b/lib/erb_lint/reporters/json_reporter.rb @@ -56,14 +56,14 @@ def formatted_offenses(offenses) def format_offense(offense) { - linter: offense.linter.class.simple_name, + linter: offense.simple_name, message: offense.message.to_s, location: { start_line: offense.line_number, start_column: offense.column, - last_line: offense.source_range.last_line, - last_column: offense.source_range.last_column, - length: offense.source_range.length, + last_line: offense.last_line, + last_column: offense.last_column, + length: offense.length, }, } end diff --git a/spec/erb_lint/fixtures/cache_file_content b/spec/erb_lint/fixtures/cache_file_content index 62945f92..20d5f8d6 100644 --- a/spec/erb_lint/fixtures/cache_file_content +++ b/spec/erb_lint/fixtures/cache_file_content @@ -1 +1 @@ -[{"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"}] \ No newline at end of file +[{"message":"Layout/InitialIndentation: Indentation of first line in file detected.","line_number":"1","severity":"convention","line_number":"1","severity":"convention","column":"3","simple_name":"Rubocop","last_line":"1","last_column":"8","length":"5"},{"message":"Layout/TrailingEmptyLines: Final newline missing.","line_number":"1","severity":"convention","line_number":"1","severity":"convention","column":"3","simple_name":"Rubocop","last_line":"1","last_column":"8","length":"5"}] diff --git a/spec/erb_lint/reporters/json_reporter_spec.rb b/spec/erb_lint/reporters/json_reporter_spec.rb index 9119ab0f..d5b62d0b 100644 --- a/spec/erb_lint/reporters/json_reporter_spec.rb +++ b/spec/erb_lint/reporters/json_reporter_spec.rb @@ -23,26 +23,20 @@ message: "Extra space detected where there should be no space.", line_number: 1, column: 7, - source_range: instance_double( - BetterHtml::Tokenizer::Location, - last_line: 1, - last_column: 9, - length: 2, - ), - linter: ERBLint::Linters::SpaceInHtmlTag.new(nil, ERBLint::LinterConfig.new), + simple_name: "SpaceInHtmlTag", + last_line: 1, + last_column: 9, + length: 2, ), instance_double( ERBLint::Offense, message: "Remove newline before `%>` to match start of tag.", line_number: 52, column: 10, - source_range: instance_double( - BetterHtml::Tokenizer::Location, - last_line: 54, - last_column: 10, - length: 10, - ), - linter: ERBLint::Linters::ClosingErbTagIndent.new(nil, ERBLint::LinterConfig.new), + simple_name: "ClosingErbTagIndent", + last_line: 54, + last_column: 10, + length: 10, ), ] end From 9ec34e399cfd366d5fa6fec2214715a7e57fc97f Mon Sep 17 00:00:00 2001 From: Zach Feldman Date: Thu, 13 Oct 2022 07:41:30 -0400 Subject: [PATCH 18/22] Update lib/erb_lint/cli.rb to use safe navigation operator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Étienne Barrié --- lib/erb_lint/cli.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/erb_lint/cli.rb b/lib/erb_lint/cli.rb index 72b8c131..576b6df0 100644 --- a/lib/erb_lint/cli.rb +++ b/lib/erb_lint/cli.rb @@ -93,7 +93,7 @@ def run(args = ARGV) end end - cache.close if with_cache? || clear_cache? + cache&.close reporter.show From 877c28cfedb14d0cd51ac109aaa3ae6158ee9542 Mon Sep 17 00:00:00 2001 From: Zach Feldman Date: Thu, 13 Oct 2022 07:44:34 -0400 Subject: [PATCH 19/22] Rename with-cache to cache --- lib/erb_lint/cli.rb | 14 +++++++------- spec/erb_lint/cli_spec.rb | 6 +++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/erb_lint/cli.rb b/lib/erb_lint/cli.rb index 576b6df0..ae11f391 100644 --- a/lib/erb_lint/cli.rb +++ b/lib/erb_lint/cli.rb @@ -31,7 +31,7 @@ def run(args = ARGV) dupped_args = args.dup load_options(dupped_args) - if with_cache? && autocorrect? + if cache? && autocorrect? failure!("cannot run autocorrect mode with cache") end @@ -39,7 +39,7 @@ def run(args = ARGV) load_config - @cache = Cache.new(@config, file_loader, prune_cache?) if with_cache? || clear_cache? + @cache = Cache.new(@config, file_loader, prune_cache?) if cache? || clear_cache? if clear_cache? if cache.cache_dir_exists? @@ -122,7 +122,7 @@ def run(args = ARGV) def run_on_file(runner, filename) file_content = read_content(filename) - if with_cache? && !autocorrect? + if cache? && !autocorrect? run_using_cache(runner, filename, file_content) else file_content = run_with_corrections(runner, filename, file_content) @@ -145,8 +145,8 @@ def autocorrect? @options[:autocorrect] end - def with_cache? - @options[:with_cache] + def cache? + @options[:cache] end def prune_cache? @@ -338,8 +338,8 @@ def option_parser @options[:enabled_linters] = known_linter_names end - opts.on("--with-cache", "Enable caching") do |config| - @options[:with_cache] = config + opts.on("--cache", "Enable caching") do |config| + @options[:cache] = config end opts.on("--prune-cache", "Prune cache") do |config| diff --git a/spec/erb_lint/cli_spec.rb b/spec/erb_lint/cli_spec.rb index 25395dcc..a1a23d38 100644 --- a/spec/erb_lint/cli_spec.rb +++ b/spec/erb_lint/cli_spec.rb @@ -249,7 +249,7 @@ def run(processed_source) end context "with cache" do - let(:args) { ["--enable-linter", "linter_without_errors", "--with-cache", linted_file] } + let(:args) { ["--enable-linter", "linter_without_errors", "--cache", linted_file] } it "lints the file and adds it to the cache" do expect(Dir[ERBLint::Cache::CACHE_DIRECTORY].length).to(be(0)) @@ -472,7 +472,7 @@ def run(processed_source) end context "with cache" do - let(:args) { ["--enable-linter", "linter_without_errors", "--with-cache", linted_dir] } + let(:args) { ["--enable-linter", "linter_without_errors", "--cache", linted_dir] } it "lints the file and adds it to the cache" do expect(Dir[ERBLint::Cache::CACHE_DIRECTORY].length).to(be(0)) @@ -590,7 +590,7 @@ def run(processed_source) context "when autocorrecting and caching are turned on" do # We assume that linter_with_errors is not autocorrectable... let(:args) do - ["--enable-linter", "linter_without_errors", "--stdin", linted_file, "--autocorrect", "--with-cache"] + ["--enable-linter", "linter_without_errors", "--stdin", linted_file, "--autocorrect", "--cache"] end it "throws an error saying the two modes cannot be used together" do From 7ea048c5960d750d5a3b1bfe292436ecfe13436c Mon Sep 17 00:00:00 2001 From: Zach Feldman Date: Thu, 13 Oct 2022 07:55:51 -0400 Subject: [PATCH 20/22] Prune cache by default --- lib/erb_lint/cache.rb | 15 +++++---------- lib/erb_lint/cli.rb | 10 +--------- spec/erb_lint/cache_spec.rb | 21 ++------------------- 3 files changed, 8 insertions(+), 38 deletions(-) diff --git a/lib/erb_lint/cache.rb b/lib/erb_lint/cache.rb index 79b88a81..9efcd6a5 100644 --- a/lib/erb_lint/cache.rb +++ b/lib/erb_lint/cache.rb @@ -4,12 +4,11 @@ module ERBLint class Cache CACHE_DIRECTORY = ".erb-lint-cache" - def initialize(config, file_loader = nil, prune = false) + def initialize(config, file_loader = nil) @config = config @file_loader = file_loader @hits = [] @new_results = [] - @prune = prune puts "Cache mode is on" end @@ -24,13 +23,13 @@ def get(filename, file_content) rescue Errno::ENOENT return false end - @hits.push(file_checksum) if prune? + @hits.push(file_checksum) cache_file_contents_as_offenses end def set(filename, file_content, offenses_as_json) file_checksum = checksum(filename, file_content) - @new_results.push(file_checksum) if prune? + @new_results.push(file_checksum) FileUtils.mkdir_p(CACHE_DIRECTORY) @@ -40,11 +39,11 @@ def set(filename, file_content, offenses_as_json) end def close - prune_cache if prune? + prune_cache end def prune_cache - puts "Prune cache mode is on - pruned file names will be logged" + puts "File names pruned from the cache will be logged" if hits.empty? puts "Cache being created for the first time, skipping prune" return @@ -94,9 +93,5 @@ def checksum(filename, file_content) # here. "_" end - - def prune? - @prune - end end end diff --git a/lib/erb_lint/cli.rb b/lib/erb_lint/cli.rb index ae11f391..e21e4c4f 100644 --- a/lib/erb_lint/cli.rb +++ b/lib/erb_lint/cli.rb @@ -39,7 +39,7 @@ def run(args = ARGV) load_config - @cache = Cache.new(@config, file_loader, prune_cache?) if cache? || clear_cache? + @cache = Cache.new(@config, file_loader) if cache? || clear_cache? if clear_cache? if cache.cache_dir_exists? @@ -149,10 +149,6 @@ def cache? @options[:cache] end - def prune_cache? - @options[:prune_cache] - end - def clear_cache? @options[:clear_cache] end @@ -342,10 +338,6 @@ def option_parser @options[:cache] = config end - opts.on("--prune-cache", "Prune cache") do |config| - @options[:prune_cache] = config - end - opts.on("--clear-cache", "Clear cache") do |config| @options[:clear_cache] = config end diff --git a/spec/erb_lint/cache_spec.rb b/spec/erb_lint/cache_spec.rb index 141ccb75..d6d6689a 100644 --- a/spec/erb_lint/cache_spec.rb +++ b/spec/erb_lint/cache_spec.rb @@ -53,6 +53,7 @@ it "returns a cached lint result" do cache_result = cache.get(linted_file_path, linted_file_content) expect(cache_result.count).to(eq(2)) + expect(cache.send(:hits)).to(include(checksum)) end end @@ -65,6 +66,7 @@ checksum ) )).to(be(true)) + expect(cache.send(:new_results)).to(include(checksum)) end end @@ -141,27 +143,8 @@ end end - describe "prune cache mode on #get and #[] behavior" do - before do - allow(cache).to(receive(:prune?).and_return(true)) - end - - 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 - - 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 - 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 From a32816d29817bdd056b9fd3ca487d58db0467241 Mon Sep 17 00:00:00 2001 From: Zach Feldman Date: Thu, 13 Oct 2022 08:00:06 -0400 Subject: [PATCH 21/22] Add README for cache --- README.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/README.md b/README.md index 6c026dba..9d0acdb6 100644 --- a/README.md +++ b/README.md @@ -590,6 +590,23 @@ app/views/users/_graph.html.erb:27:37: Extra space detected where there should b 2 error(s) were found in ERB files ``` +## Caching + +The cache is currently opt-in - to turn it on, use the --cache option: + +```sh +erblint --cache ./app +Cache mode is on +Linting 413 files with 15 linters... +File names pruned from the cache will be logged + +No errors were found in ERB files +``` + +When the cache is on, lint results are stored in the `.erb-lint-cache` directory, in files with a filename computed with a hash of information about the file and `erb-lint` that should change when necessary. 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. + +You can also use the --clear-cache option to delete the cache file directory. + ## License This project is released under the [MIT license](LICENSE.txt). From e08285533ad14d9ffcfc17ae1475f7d416647452 Mon Sep 17 00:00:00 2001 From: Zach Feldman Date: Tue, 18 Oct 2022 08:14:20 -0400 Subject: [PATCH 22/22] Reduce output of pruning process --- lib/erb_lint/cache.rb | 11 +---------- spec/erb_lint/cache_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/lib/erb_lint/cache.rb b/lib/erb_lint/cache.rb index 9efcd6a5..ee537e17 100644 --- a/lib/erb_lint/cache.rb +++ b/lib/erb_lint/cache.rb @@ -43,7 +43,6 @@ def close end def prune_cache - puts "File names pruned from the cache will be logged" if hits.empty? puts "Cache being created for the first time, skipping prune" return @@ -51,18 +50,10 @@ def prune_cache cache_files = Dir.new(CACHE_DIRECTORY).children cache_files.each do |cache_file| - next if hits.include?(cache_file) + next if hits.include?(cache_file) || new_results.include?(cache_file) - if new_results.include?(cache_file) - puts "Skipping deletion of new cache result #{cache_file}" - next - end - - puts "Cleaning deleted cached file with checksum #{cache_file}" File.delete(File.join(CACHE_DIRECTORY, cache_file)) end - - @hits = [] end def cache_dir_exists? diff --git a/spec/erb_lint/cache_spec.rb b/spec/erb_lint/cache_spec.rb index d6d6689a..93731202 100644 --- a/spec/erb_lint/cache_spec.rb +++ b/spec/erb_lint/cache_spec.rb @@ -112,7 +112,7 @@ allow(fakefs_dir).to(receive(:children).and_return([checksum])) allow(FakeFS::Dir).to(receive(:new).and_return(fakefs_dir)) - expect { cache.prune_cache }.to(output(/.*Skipping deletion of new cache result #{checksum}/).to_stdout) + cache.prune_cache expect(File.exist?( File.join( @@ -132,7 +132,7 @@ f.write(cache_file_content) end - expect { cache.prune_cache }.to(output(/Cleaning deleted cached file with checksum fake-checksum/).to_stdout) + cache.prune_cache expect(File.exist?( File.join(