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

[Ruby 3.0] Reserved for numparams #4323

Merged
merged 4 commits into from Jul 7, 2021

Conversation

vinistock
Copy link
Collaborator

This PR implements the changes for Ruby 3.0 to numbered parameters based on whitequark/parser#725.

The change is to check that the reserved names for numbered parameters are not used anywhere except for numbered parameters themselves.

I had to update some unrelated RBIs and tests because they used these reserved names and started throwing the new error. It might be easier to review per commit.

Motivation

In Ruby 2.7, assigning a numbered parameter would throw an error only inside blocks. That is

lambda {_1 = nil} # error

Since Ruby 3.0, the numbered parameter names are actually reserved everywhere. Trying to use _1 as a variable, method definition or any type of argument will throw an error and the message has changed.

def _1; end # error
_1 = nil # error
def foo(_1); end # error
def bar(*_1); end # error
def baz(&_1); end # error

Test plan

See included automated tests.

@vinistock vinistock requested a review from a team as a code owner June 29, 2021 20:54
@vinistock vinistock requested review from froydnj and removed request for a team June 29, 2021 20:54
@vinistock vinistock force-pushed the ruby30-reserved-for-numparams branch from ad3ed13 to 4a4d488 Compare June 30, 2021 20:22
Copy link
Contributor

@froydnj froydnj left a comment

Choose a reason for hiding this comment

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

It looks like there are some test failures with some test code that need to be resolved first.

@vinistock vinistock force-pushed the ruby30-reserved-for-numparams branch from 4a4d488 to 96473c9 Compare July 6, 2021 18:59
@vinistock
Copy link
Collaborator Author

@froydnj fixed the hidden definition generation in the latest commit.

Copy link
Contributor

@froydnj froydnj left a comment

Choose a reason for hiding this comment

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

Thanks for fixing up the CI failures!

@@ -57,7 +57,7 @@ tuple<string, string> MESSAGES[] = {
{"InvalidRegexp", "{}"},
{"InvalidReturn", "invalid return in class/module body"},
{"CSendInLHSOfMAsgn", "&. inside multiple assignment destination"},
{"CantAssignToNumparam", "cannot assign to numbered parameter {}"},
{"ReservedForNumparam", "{} is reserved for numbered parameter"},
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if this is the best error message, but I think we can fix this in a followup.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed. I used the same message Ruby uses.

@froydnj froydnj merged commit 2b3a6c0 into sorbet:master Jul 7, 2021
@vinistock vinistock deleted the ruby30-reserved-for-numparams branch July 7, 2021 20:45
elliottt added a commit that referenced this pull request Jul 8, 2021
elliottt added a commit that referenced this pull request Jul 8, 2021
vinistock added a commit to vinistock/sorbet that referenced this pull request Jul 12, 2021
vinistock added a commit to vinistock/sorbet that referenced this pull request Jul 14, 2021
Revert: (sorbet#4337)
Original: (sorbet#4323)

This reverts commit f2f50a0.

Co-authored-by: Alexandre Terrasa <583144+Morriar@users.noreply.github.com>
jez pushed a commit that referenced this pull request Aug 4, 2021
Revert: (#4337)
Original: (#4323)

This reverts commit f2f50a0.

Co-authored-by: Alexandre Terrasa <583144+Morriar@users.noreply.github.com>
jez pushed a commit that referenced this pull request Aug 6, 2021
Revert: (#4337)
Original: (#4323)

This reverts commit f2f50a0.

Co-authored-by: Alexandre Terrasa <583144+Morriar@users.noreply.github.com>
vinistock added a commit to vinistock/sorbet that referenced this pull request Aug 16, 2021
Revert: (sorbet#4337)
Original: (sorbet#4323)

This reverts commit f2f50a0.

Co-authored-by: Alexandre Terrasa <583144+Morriar@users.noreply.github.com>
elliottt pushed a commit that referenced this pull request Aug 17, 2021
Revert: (#4337)
Original: (#4323)

This reverts commit f2f50a0.

Co-authored-by: Alexandre Terrasa <583144+Morriar@users.noreply.github.com>
vinistock added a commit to vinistock/sorbet that referenced this pull request Aug 18, 2021
Revert: (sorbet#4337)
Original: (sorbet#4323)

This reverts commit f2f50a0.

Co-authored-by: Alexandre Terrasa <583144+Morriar@users.noreply.github.com>
elliottt pushed a commit that referenced this pull request Aug 18, 2021
* [Ruby 3.0] Add reserved for numparams errors to all usages

Revert: (#4337)
Original: (#4323)

This reverts commit f2f50a0.

Co-authored-by: Alexandre Terrasa <583144+Morriar@users.noreply.github.com>

* Autocorrect reserved numbered parameters from `_[0-9]` to `arg[0-9]`

Co-authored-by: Alexandre Terrasa <583144+Morriar@users.noreply.github.com>

* Use errors-with-dup for reserved numparams

Co-authored-by: Alexandre Terrasa <583144+Morriar@users.noreply.github.com>

* Add tests and builder changes related to endless methods combined with numbered parameters

Co-authored-by: Alexandre Terrasa <583144+Morriar@users.noreply.github.com>

* Use a new error class to be able to supress reserved numparam errors

* Fix numbered parameters test cases

Co-authored-by: Alexandre Terrasa <583144+Morriar@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants