From b11a84e81575b446103e2ecff58b59d77ada4e68 Mon Sep 17 00:00:00 2001 From: patrick brisbin Date: Mon, 7 Dec 2015 14:15:34 -0500 Subject: [PATCH 1/2] Add Rakefile --- Rakefile | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 Rakefile diff --git a/Rakefile b/Rakefile new file mode 100644 index 0000000..8281788 --- /dev/null +++ b/Rakefile @@ -0,0 +1,5 @@ +require "rspec/core/rake_task" + +RSpec::Core::RakeTask.new(:spec) + +task default: :spec From e7aff4f49f5d06bf64fe6f58ef64beb6ade968dc Mon Sep 17 00:00:00 2001 From: patrick brisbin Date: Mon, 7 Dec 2015 14:15:52 -0500 Subject: [PATCH 2/2] Don't report all errors on each file analyzed It's likely the original developer assumed that the pattern given to Node#xpath is treated relative to the node on which it's called. That doesn't seem to be the case. The XPath "//error" finds ALL error nodes across the entire document, even when invoked as file.xpath("//error"). This causes them all to be reported on every file analyzed. Besides being extremely non-sensical, this can also be a performance nightmare. To correct this, we instead use Node#children to get only the currently processing file's errors. This required replacing some hash accesses with methods and the chaining of #value on attributes. --- lib/cc/engine/csslint.rb | 18 +++++++++++------- spec/cc/engine/csslint_spec.rb | 6 ++++++ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/lib/cc/engine/csslint.rb b/lib/cc/engine/csslint.rb index d89da9d..0614c7a 100644 --- a/lib/cc/engine/csslint.rb +++ b/lib/cc/engine/csslint.rb @@ -14,23 +14,27 @@ def run Dir.chdir(@directory) do results.xpath('//file').each do |file| path = file['name'].sub(/\A#{@directory}\//, '') - file.xpath('//error').each do |lint| + file.children.each do |node| + next unless node.name == "error" + + lint = node.attributes + issue = { type: "issue", - check_name: lint["source"], - description: lint["message"], + check_name: lint["source"].value, + description: lint["message"].value, categories: ["Style"], remediation_points: 500, location: { path: path, positions: { begin: { - line: lint["line"].to_i, - column: lint["column"].to_i + line: lint["line"].value.to_i, + column: lint["column"].value.to_i }, end: { - line: lint["line"].to_i, - column: lint["column"].to_i + line: lint["line"].value.to_i, + column: lint["column"].value.to_i } } } diff --git a/spec/cc/engine/csslint_spec.rb b/spec/cc/engine/csslint_spec.rb index 7a12bff..e1fc37b 100644 --- a/spec/cc/engine/csslint_spec.rb +++ b/spec/cc/engine/csslint_spec.rb @@ -22,6 +22,12 @@ module Engine expect{ lint.run }.to_not output.to_stdout end + it "only reports issues in the file where they're present" do + create_source_file('bad.css', id_selector_content) + create_source_file('good.css', '.foo { margin: 0 }') + expect{ lint.run }.not_to output(/good\.css/).to_stdout + end + describe "with exclude_paths" do let(:engine_config) { {"exclude_paths" => %w(excluded.css)} }