Skip to content

Fauty code: RBParser parse only faulty code #13080

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

privat
Copy link
Contributor

@privat privat commented Mar 20, 2023

RBParser is able (and tested) to produce AST with error nodes and precise error and warning information (notices)

The handling of error with error blocks or syntax error notifications is inferior because they are activated too early, without so much context: there is no AST yet, so such errors are only identified with a message string and a single position.
Moreover, they add complexity to the Parser code.

This PR tries to do what #12912 could not: remove errorBlock from RBParser.
What changed since then is a lot a cleanup and the RBNotice API, so I hope this will work now.

Features

  • no more beFaulty, errorBlock or even SyntaxErrorNotification: the parser only goal is to generate AST, not to care about your feeling or water down bad news. You ask an AST, you get an AST, no questions asked, not many flavors to choose.
  • new method RBProgramNode>>checkFaulty: that simulates the old parser handling of error.
    Therefore, clients that rely on beFaulty, errorBlock and/or SyntaxErrorNotification: can just use this method on the resulting AST and should get the same behavior.
    Note that the receiver is an AST node, not a parser, this is neat.
  • Method on the class side of RBParser parse(Faulty)?(Expression|Method):(onError:)? are still here, and use checkFaulty: to achieve the same results but in a smoother way.

@privat privat mentioned this pull request Mar 20, 2023
@privat
Copy link
Contributor Author

privat commented Mar 20, 2023

OMG! Bootstrap passed! 🎉

@jecisc
Copy link
Member

jecisc commented Mar 20, 2023

Congrats :P

@jecisc
Copy link
Member

jecisc commented Mar 20, 2023

The ci does not seems to like it

@privat
Copy link
Contributor Author

privat commented Mar 20, 2023

It's an acquired taste. I just have to insist enough.

@jecisc
Copy link
Member

jecisc commented Mar 20, 2023

I hope you got their consent :)

@privat
Copy link
Contributor Author

privat commented Mar 20, 2023

CI green except windows that like nothing anyway

@jecisc
Copy link
Member

jecisc commented Mar 20, 2023

This was a timeout on the windows slave :( I hope it’s not a new random failure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants