diff --git a/README.md b/README.md index 0218b841..cd397cd3 100644 --- a/README.md +++ b/README.md @@ -612,6 +612,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). diff --git a/lib/erb_lint/all.rb b/lib/erb_lint/all.rb index ecd95cb0..4aee601c 100644 --- a/lib/erb_lint/all.rb +++ b/lib/erb_lint/all.rb @@ -3,6 +3,8 @@ require "rubocop" 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 new file mode 100644 index 00000000..ee537e17 --- /dev/null +++ b/lib/erb_lint/cache.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +module ERBLint + class Cache + CACHE_DIRECTORY = ".erb-lint-cache" + + def initialize(config, file_loader = nil) + @config = config + @file_loader = file_loader + @hits = [] + @new_results = [] + puts "Cache mode is on" + end + + def get(filename, 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_hash| + ERBLint::CachedOffense.new(offense_hash) + end + rescue Errno::ENOENT + return false + end + @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) + + FileUtils.mkdir_p(CACHE_DIRECTORY) + + File.open(File.join(CACHE_DIRECTORY, file_checksum), "wb") do |f| + f.write(offenses_as_json) + end + end + + def close + prune_cache + end + + def prune_cache + if hits.empty? + puts "Cache being created for the first time, skipping prune" + return + end + + cache_files = Dir.new(CACHE_DIRECTORY).children + cache_files.each do |cache_file| + next if hits.include?(cache_file) || new_results.include?(cache_file) + + File.delete(File.join(CACHE_DIRECTORY, cache_file)) + end + end + + def cache_dir_exists? + File.directory?(CACHE_DIRECTORY) + end + + def clear + return unless cache_dir_exists? + + puts "Clearing cache by deleting cache directory" + FileUtils.rm_r(CACHE_DIRECTORY) + end + + private + + attr_reader :config, :hits, :new_results + + def checksum(filename, file_content) + digester = Digest::SHA1.new + mode = File.stat(filename).mode + + digester.update( + "#{mode}#{config.to_hash}#{ERBLint::VERSION}#{file_content}" + ) + 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/cached_offense.rb b/lib/erb_lint/cached_offense.rb new file mode 100644 index 00000000..3c5fa74e --- /dev/null +++ b/lib/erb_lint/cached_offense.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +module ERBLint + # A Cached version of an Offense with only essential information represented as strings + class CachedOffense + attr_reader( + :message, + :line_number, + :severity, + :column, + :simple_name, + :last_line, + :last_column, + :length, + ) + + 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( + { + 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_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 + end +end diff --git a/lib/erb_lint/cli.rb b/lib/erb_lint/cli.rb index 7e5396eb..e21e4c4f 100644 --- a/lib/erb_lint/cli.rb +++ b/lib/erb_lint/cli.rb @@ -30,10 +30,26 @@ def initialize def run(args = ARGV) dupped_args = args.dup load_options(dupped_args) + + if cache? && autocorrect? + failure!("cannot run autocorrect mode with cache") + end + @files = @options[:stdin] || dupped_args load_config + @cache = Cache.new(@config, file_loader) if cache? || clear_cache? + + if clear_cache? + if cache.cache_dir_exists? + cache.clear + success!("cache directory cleared") + else + failure!("cache directory doesn't exist, skipping deletion.") + end + end + if !@files.empty? && lint_files.empty? if allow_no_files? success!("no files found...\n") @@ -65,7 +81,7 @@ def run(args = ARGV) lint_files.each do |filename| runner.clear_offenses begin - file_content = run_with_corrections(runner, filename) + file_content = run_on_file(runner, filename) rescue => e @stats.exceptions += 1 puts "Exception occurred when processing: #{relative_filename(filename)}" @@ -77,6 +93,8 @@ def run(args = ARGV) end end + cache&.close + reporter.show if stdin? && autocorrect? @@ -99,13 +117,43 @@ def run(args = ARGV) private + attr_reader :cache, :config + + def run_on_file(runner, filename) + file_content = read_content(filename) + + if 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_result_offenses = cache.get(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_hash).to_json) + end + end + def autocorrect? @options[:autocorrect] end - def run_with_corrections(runner, filename) - file_content = read_content(filename) + def cache? + @options[:cache] + end + + def clear_cache? + @options[:clear_cache] + end + def run_with_corrections(runner, filename, file_content) 7.times do processed_source = ERBLint::ProcessedSource.new(filename, file_content) runner.run(processed_source) @@ -127,6 +175,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 || [] @@ -138,8 +191,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) @@ -283,6 +334,14 @@ def option_parser @options[:enabled_linters] = known_linter_names end + opts.on("--cache", "Enable caching") do |config| + @options[:cache] = config + end + + opts.on("--clear-cache", "Clear cache") do |config| + @options[:clear_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| 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..5c8db6b7 100644 --- a/lib/erb_lint/offense.rb +++ b/lib/erb_lint/offense.rb @@ -17,6 +17,10 @@ def initialize(linter, source_range, message, context = nil, severity = nil) @severity = severity end + def to_cached_offense_hash + ERBLint::CachedOffense.new_from_offense(self).to_h + end + def inspect "#<#{self.class.name} linter=#{linter.class.name} "\ "source_range=#{source_range.begin_pos}...#{source_range.end_pos} "\ @@ -43,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/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 diff --git a/spec/erb_lint/cache_spec.rb b/spec/erb_lint/cache_spec.rb new file mode 100644 index 00000000..93731202 --- /dev/null +++ b/spec/erb_lint/cache_spec.rb @@ -0,0 +1,152 @@ +# frozen_string_literal: true + +require "spec_helper" +require "erb_lint/cache" +require "fakefs" +require "fakefs/spec_helpers" + +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(:checksum) { "2dc3e17183b87889cc783b0157723570d4bbb90a" } + 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 + + 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 + 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 + + describe "#[]=" do + it "caches a lint result" do + cache.set(linted_file_path, linted_file_content, cache_file_content) + expect(File.exist?( + File.join( + cache_dir, + checksum + ) + )).to(be(true)) + expect(cache.send(:new_results)).to(include(checksum)) + 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_cache" do + it "skips prune if no cache hits" do + allow(cache).to(receive(:hits).and_return([])) + + 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 + + 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([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)) + + cache.prune_cache + + expect(File.exist?( + File.join( + cache_dir, + checksum + ) + )).to(be(true)) + end + + 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)) + 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 + + cache.prune_cache + + expect(File.exist?( + File.join( + cache_dir, + "fake-checksum" + ) + )).to(be(false)) + end + end + + describe "#close" do + it "Calls prune_cache if prune_cache mode is on" do + expect(cache).to(receive(:prune_cache)) + cache.close + end + end +end diff --git a/spec/erb_lint/cli_spec.rb b/spec/erb_lint/cli_spec.rb index 77c61b5b..a1a23d38 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 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 + 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", "--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", "--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", "--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 diff --git a/spec/erb_lint/fixtures/cache_file_content b/spec/erb_lint/fixtures/cache_file_content new file mode 100644 index 00000000..20d5f8d6 --- /dev/null +++ b/spec/erb_lint/fixtures/cache_file_content @@ -0,0 +1 @@ +[{"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/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 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