From a31810cd989c8681af497eb28fb92beaed885497 Mon Sep 17 00:00:00 2001 From: Will Fleming Date: Tue, 16 Aug 2016 17:03:37 -0400 Subject: [PATCH 1/5] Allow passing specific args to rspec in makefile --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index f11fb3d..cb4fb3e 100644 --- a/Makefile +++ b/Makefile @@ -4,4 +4,4 @@ image: docker build -t codeclimate/codeclimate-csslint . test: image - docker run --rm codeclimate/codeclimate-csslint rake + docker run --rm codeclimate/codeclimate-csslint rspec $(RSPEC_ARGS) From fd1773d3a91679586b9a1527175a3b2c74e92e91 Mon Sep 17 00:00:00 2001 From: Will Fleming Date: Tue, 16 Aug 2016 17:11:52 -0400 Subject: [PATCH 2/5] Don't crash on parse errors Instead of crashing out when encountering a parse error, this will report the issue as a parse error. --- lib/cc/engine/csslint.rb | 32 +++++++++++++++++++++++++++++++- spec/cc/engine/csslint_spec.rb | 2 +- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/lib/cc/engine/csslint.rb b/lib/cc/engine/csslint.rb index bb25c85..98a383e 100644 --- a/lib/cc/engine/csslint.rb +++ b/lib/cc/engine/csslint.rb @@ -21,7 +21,12 @@ def run path = file['name'].sub(/\A#{@directory}\//, '') file.children.each do |node| next unless node.name == "error" - issue = create_issue(node, path) + issue = + if node.attributes.has_key?("identifier") + create_issue(node, path) + else + create_error(node, path) + end puts("#{issue.to_json}\0") end end @@ -58,6 +63,31 @@ def create_issue(node, path) raise MissingAttributesError, "#{ex.message} on XML '#{node}' when analyzing file '#{path}'" end + def create_error(node, path) + { + type: "issue", + check_name: "parse_error", + description: node.attributes.fetch("message").value, + categories: ["Bug Risk"], + remediation_points: 5_000, + location: { + path: path, + positions: { + begin: { + line: node.attributes.fetch("line").value.to_i, + column: node.attributes.fetch("column").value.to_i + }, + end: { + line: node.attributes.fetch("line").value.to_i, + column: node.attributes.fetch("column").value.to_i + } + } + } + } + rescue KeyError => ex + raise MissingAttributesError, "#{ex.message} on XML error '#{node}' when analyzing file '#{path}'" + end + def results @results ||= Nokogiri::XML(csslint_xml) end diff --git a/spec/cc/engine/csslint_spec.rb b/spec/cc/engine/csslint_spec.rb index 356848c..9c4a40e 100644 --- a/spec/cc/engine/csslint_spec.rb +++ b/spec/cc/engine/csslint_spec.rb @@ -18,7 +18,7 @@ module Engine it 'fails on malformed file' do create_source_file('foo.css', '�6�') - expect{ lint.run }.to raise_error(MissingAttributesError) + expect{ lint.run }.to output(/Unexpected token/).to_stdout end it "doesn't analyze *.scss files" do From 32cfe66380ebe348031724ca987f69d2cc0b65fd Mon Sep 17 00:00:00 2001 From: Will Fleming Date: Tue, 16 Aug 2016 17:16:05 -0400 Subject: [PATCH 3/5] cc issues --- lib/cc/engine/csslint.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/cc/engine/csslint.rb b/lib/cc/engine/csslint.rb index 98a383e..7969edc 100644 --- a/lib/cc/engine/csslint.rb +++ b/lib/cc/engine/csslint.rb @@ -22,7 +22,7 @@ def run file.children.each do |node| next unless node.name == "error" issue = - if node.attributes.has_key?("identifier") + if node.attributes.key?("identifier") create_issue(node, path) else create_error(node, path) @@ -35,6 +35,7 @@ def run private + # rubocop:disable Metrics/MethodLength def create_issue(node, path) check_name = node.attributes.fetch("identifier").value check_details = CheckDetails.fetch(check_name) @@ -62,11 +63,13 @@ def create_issue(node, path) rescue KeyError => ex raise MissingAttributesError, "#{ex.message} on XML '#{node}' when analyzing file '#{path}'" end + # rubocop:enable Metrics/MethodLength + # rubocop:disable Metrics/MethodLength def create_error(node, path) { type: "issue", - check_name: "parse_error", + check_name: "parse-error", description: node.attributes.fetch("message").value, categories: ["Bug Risk"], remediation_points: 5_000, @@ -87,6 +90,7 @@ def create_error(node, path) rescue KeyError => ex raise MissingAttributesError, "#{ex.message} on XML error '#{node}' when analyzing file '#{path}'" end + # rubocop:enable Metrics/MethodLength def results @results ||= Nokogiri::XML(csslint_xml) From 4968a34f7fc5c34971e4c3f3bb5b2282d9642b62 Mon Sep 17 00:00:00 2001 From: Will Fleming Date: Tue, 16 Aug 2016 17:29:54 -0400 Subject: [PATCH 4/5] compact impl --- lib/cc/engine/csslint.rb | 39 ++++---------------------- lib/cc/engine/csslint/check_details.rb | 1 + 2 files changed, 6 insertions(+), 34 deletions(-) diff --git a/lib/cc/engine/csslint.rb b/lib/cc/engine/csslint.rb index 7969edc..00d15e1 100644 --- a/lib/cc/engine/csslint.rb +++ b/lib/cc/engine/csslint.rb @@ -1,11 +1,14 @@ require "json" require "nokogiri" +require "ostruct" require "shellwords" module CC module Engine MissingAttributesError = Class.new(StandardError) + DEFAULT_IDENTIFIER = OpenStruct.new(value: "parse-error") + class CSSlint autoload :CheckDetails, "cc/engine/csslint/check_details" @@ -21,12 +24,7 @@ def run path = file['name'].sub(/\A#{@directory}\//, '') file.children.each do |node| next unless node.name == "error" - issue = - if node.attributes.key?("identifier") - create_issue(node, path) - else - create_error(node, path) - end + issue = create_issue(node, path) puts("#{issue.to_json}\0") end end @@ -37,7 +35,7 @@ def run # rubocop:disable Metrics/MethodLength def create_issue(node, path) - check_name = node.attributes.fetch("identifier").value + check_name = node.attributes.fetch("identifier", DEFAULT_IDENTIFIER).value check_details = CheckDetails.fetch(check_name) { @@ -65,33 +63,6 @@ def create_issue(node, path) end # rubocop:enable Metrics/MethodLength - # rubocop:disable Metrics/MethodLength - def create_error(node, path) - { - type: "issue", - check_name: "parse-error", - description: node.attributes.fetch("message").value, - categories: ["Bug Risk"], - remediation_points: 5_000, - location: { - path: path, - positions: { - begin: { - line: node.attributes.fetch("line").value.to_i, - column: node.attributes.fetch("column").value.to_i - }, - end: { - line: node.attributes.fetch("line").value.to_i, - column: node.attributes.fetch("column").value.to_i - } - } - } - } - rescue KeyError => ex - raise MissingAttributesError, "#{ex.message} on XML error '#{node}' when analyzing file '#{path}'" - end - # rubocop:enable Metrics/MethodLength - def results @results ||= Nokogiri::XML(csslint_xml) end diff --git a/lib/cc/engine/csslint/check_details.rb b/lib/cc/engine/csslint/check_details.rb index 04133d5..5bdfa20 100644 --- a/lib/cc/engine/csslint/check_details.rb +++ b/lib/cc/engine/csslint/check_details.rb @@ -19,6 +19,7 @@ class CheckDetails "import" => { categories: "Bug Risk" }, "known-properties" => { categories: "Bug Risk" }, "overqualified-elements" => { categories: "Bug Risk" }, + "parse-error" => { categories: "Bug Risk" }, "regex-selectors" => { categories: "Bug Risk" }, "shorthand" => { categories: "Bug Risk" }, "star-property-hack" => { categories: "Compatibility" }, From b51b30f5700597dbb27b3a26d7cd26e8d75c7c0e Mon Sep 17 00:00:00 2001 From: Will Fleming Date: Tue, 16 Aug 2016 17:32:49 -0400 Subject: [PATCH 5/5] why would you use a tab? --- lib/cc/engine/csslint/check_details.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cc/engine/csslint/check_details.rb b/lib/cc/engine/csslint/check_details.rb index 5bdfa20..d268634 100644 --- a/lib/cc/engine/csslint/check_details.rb +++ b/lib/cc/engine/csslint/check_details.rb @@ -19,7 +19,7 @@ class CheckDetails "import" => { categories: "Bug Risk" }, "known-properties" => { categories: "Bug Risk" }, "overqualified-elements" => { categories: "Bug Risk" }, - "parse-error" => { categories: "Bug Risk" }, + "parse-error" => { categories: "Bug Risk" }, "regex-selectors" => { categories: "Bug Risk" }, "shorthand" => { categories: "Bug Risk" }, "star-property-hack" => { categories: "Compatibility" },