Skip to content
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

- Parser#parse returns nil instead of false if error is thrown #722

Merged
merged 1 commit into from Jul 22, 2020

Conversation

@marcandre
Copy link
Contributor

@marcandre marcandre commented Jul 22, 2020

In some circumstances (when reaching the EOF too early), racc erroneously returns false instead of nil.

Even though it's an upstream issue, the fix is so simple that I thought it was worthwhile to circumvent it.

This PR also improves the documentation for non-fatal cases. There are no other tests with all_errors_are_fatal set to false.

@marcandre marcandre force-pushed the marcandre:non_fatal branch from d645e6b to 8963a21 Jul 22, 2020
…nts to return `false` instead of `nil`
@marcandre marcandre force-pushed the marcandre:non_fatal branch from 8963a21 to 79f541a Jul 22, 2020
@marcandre marcandre changed the title - Have Parser#parse and dependents return `nil` instead of `false` for non-fatal errors - Have Parser#parse and dependents return `nil` instead of `false` for all non-fatal errors Jul 22, 2020
marcandre referenced this pull request in rubocop-hq/rubocop-ast Jul 22, 2020
Fixes #6945.

By convention, a year after Ruby became an EOL, it seems that the old
Ruby version support has been dropped. I'm not sure if it's good to drop
Ruby 2.3 support, but it's good time to drop Ruby 2.2 support.
Copy link
Collaborator

@iliabylich iliabylich left a comment

LGTM 👍

@@ -210,8 +210,9 @@ def parse_with_comments(source_buffer)

##
# Parses a source buffer and returns the AST, the source code comments,
# and the tokens emitted by the lexer. If `recover` is true and a fatal
# {SyntaxError} is encountered, `nil` is returned instead of the AST, and
# and the tokens emitted by the lexer. In case of a fatal error, a {SyntaxError}

This comment has been minimized.

@iliabylich

iliabylich Jul 22, 2020
Collaborator

ha, it took me a while to understand that recover here is responsible for "swallowing the error", but not "error recovery". Not sure if it's worth renaming it, current name sounds good as a part of the public API (you "recover" from VM error, not from the parsing error). Thanks for changing this doc too 👍

@iliabylich iliabylich changed the title - Have Parser#parse and dependents return `nil` instead of `false` for all non-fatal errors - Parser#parse returns nil instead of false if error is thrown Jul 22, 2020
@iliabylich iliabylich merged commit e88d47b into whitequark:master Jul 22, 2020
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@iliabylich
Copy link
Collaborator

@iliabylich iliabylich commented Jul 22, 2020

@marcandre Thanks!

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

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.