Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Proposal: Introduce RubyParser::ParseFailure exception #54

Closed
brynary opened this Issue · 5 comments

2 participants

Bryan Helmkamp Ryan Davis
Bryan Helmkamp

Right now in Code Climate I have to rescue all of the following exceptions when I parse:

Racc::ParseError
ArgumentError
TypeError
RegexpError
RuntimeError
SyntaxError

What do you think of creating a contract where RubyParser raises a RubyParser::ParseFailure if it cannot parse?

Ryan Davis
Owner

HAHAHA... what's wrong with that huge list? :P

RegexpError is probably real and I think I just added something to the 1.9 parser that'll handle it.

TypeError? ArgumentError? I'd be interested in seeing a repro on those. Prolly my fault.

I'm sure I'm inconsistent on RuntimeError vs SyntaxError. Racc::ParseError should be folded up with those two.

Ryan Davis
Owner

ping

Bryan Helmkamp

When you say RegexpError is real do you mean that it's a legit error that RP may raise, or that it's a bug that RP should catch it and deal with it? (I think you mean the latter.)

Going to take some time for me to get a repro for TypeError and ArgumentError, unfortunately. I did a quick rescue bandaid I need to revisit.

Rolling Racc::ParseError up sounds good. I always found SyntaxError to be an odd thing to throw because it's not a StandardError. So rescue won't catch it by default. As first cut, I'd advocate always raising a RubyParser::SyntaxError which inherits from StandardError rather than a ::SyntaxError or RuntimeError (which I generally prefer reserving for just raise "foo" cases)

Thoughts?

-Bryan

Ryan Davis
Owner

RegexpError being real means I probably fucked up and it should be reported. Same with TypeError and ArgumentError.

I'm cool with namespacing everything else. I'm gonna leave my "no" exceptions alone because frankly they shouldn't exist and are only there for bug hunting. SyntaxError should totally move over.

Ryan Davis
Owner

I believe I've done 90+% of this by switching to RubyParser::SyntaxError. If you see something get past that, it's prolly a bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.