Merged

Don't crash on parse errors #34

Changes from 1 commit
Show all changes
5 commits
Select commit Hold shift + click to select a range
a31810c
Allow passing specific args to rspec in makefile
Aug 16, 2016
fd1773d
Don't crash on parse errors
Aug 16, 2016
32cfe66
cc issues
Aug 16, 2016
4968a34
compact impl
Aug 16, 2016
b51b30f
why would you use a tab?
Aug 16, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Failed to load files.
PrevPrevious commit
Next Next commit
cc issues
  • Loading branch information
Will Fleming committedAug 16, 2016
commit 32cfe66380ebe348031724ca987f69d2cc0b65fd
Original file line numberDiff line numberDiff line change
Expand Up@@ -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)
Copy link
Contributor

@pbrisbin pbrisbin Aug 16, 2016•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach introduces an unacceptable amount of syntactic duplication IMO. What do you think of instead doing the following:

  • check_name is (basically) node.identifier || "parse-error"
  • Have a check_details value that maps parse-error to [BugRisk]/5_000
  • Everything else about #create_issue stays as-is
  • No need for #create_error

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good points all around. Will do all of that.

Expand All@@ -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)
Expand DownExpand Up@@ -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,
Expand All@@ -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)
Expand Down