Conversation

wfleming

Instead of crashing out when encountering a parse error, this will report
the issue as a parse error.

cc @codeclimate/review a bit different from other engines because we're not outputting to stderr, but this seems useful.

Will Fleming added 3 commits August 16, 2016 17:03
Instead of crashing out when encountering a parse error, this will
report the issue as a parse error.
if node.attributes.key?("identifier")
create_issue(node, path)
else
create_error(node, path)
Copy link
Contributor

@pbrisbin pbrisbin Aug 16, 2016

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.

@wfleming

@pbrisbin cleaned up

require "shellwords"

module CC
module Engine
MissingAttributesError = Class.new(StandardError)

DEFAULT_IDENTIFIER = OpenStruct.new(value: "parse-error")
Copy link
Contributor

Choose a reason for hiding this comment

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

Nifty

@pbrisbin

LGTM. Nice fix.

@wflemingwfleming merged commit 0daec16 into master Aug 16, 2016
@wflemingwfleming deleted the will/parse-errors branch August 16, 2016 21:50
Sign up for free to join this conversation on . Already have an account? Sign in to comment
None yet
None yet

Successfully merging this pull request may close these issues.