Skip to content

Add specs for the it variable in blocks #1224

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

herwinw
Copy link
Member

@herwinw herwinw commented Dec 3, 2024

For #1216

This is very much a work in progress, I had this file hanging around in Natalie. It's mostly just a PoC, and misses a lot of cases. The numbered_parameters_spec can probably be used as a reference.

@herwinw herwinw self-assigned this Dec 3, 2024
@andrykonchin
Copy link
Member

There are some relevant new specs in a TruffleRuby corresponding PR oracle/truffleruby#3734. So later we will need to combine them and this PR changes during synchronising TruffleRuby's spec changes with ruby/spec.

The progress of TruffleRuby support of Ruby 3.3 is tracked in oracle/truffleruby#3681. Usually contributors leave a comment when they are starting working on some item.

@herwinw herwinw removed their assignment May 29, 2025
@herwinw herwinw marked this pull request as ready for review May 29, 2025 06:53
@herwinw
Copy link
Member Author

herwinw commented May 29, 2025

I have updated this one with the Ruby 3.4 behaviour and rebased it upon the current state. With extra credits for @zverok, the examples on "The Ruby Changes" were very helpful.

This includes the behaviour of Ruby 3.3 and 3.4, so it checks a box in #1265 as well.

I put everything in language/block_spec.rb, since that one already had checks for the it parameter. Since there is also a language/numbered_parameters_spec.rb, I would think this would warrant its own file.

@herwinw
Copy link
Member Author

herwinw commented May 29, 2025

Fun fact: this is the first time in the specs that matched syntax errors in Prism and Parse.y did not match up (https://bugs.ruby-lang.org/issues/21381)

it "acts as the first argument if no local variables exist" do
eval("proc { it * 2 }").call(5).should == 10
end

Copy link
Member

Choose a reason for hiding this comment

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

As for me it might be useful to add tests cases similar to what we have in the 3.3...3.4 section:

  • it "does not emit a deprecation warning when a local variable inside the block named it exists" do
  • it "calls the method it if defined" do
  • and probably it "does not emit a deprecation warning when a block has parameters" do


it "can be used in nested calls" do
eval("proc { it.map { it * 2 } }").call([1, 2, 3]).should == [2, 4, 6]
end
Copy link
Member

Choose a reason for hiding this comment

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

In this test it isn't clear whether a block for the map call has its own it variable or it refers the the it variable of the outer scope (a block for the proc call)

@andrykonchin
Copy link
Member

Since there is also a language/numbered_parameters_spec.rb, I would think this would warrant its own file.

Yes, makes sense.

@andrykonchin
Copy link
Member

andrykonchin commented May 29, 2025

Fun fact: this is the first time in the specs that matched syntax errors in Prism and Parse.y did not match up (https://bugs.ruby-lang.org/issues/21381)

We reported such issues usually in the ruby/prism GitHub repo. It seems the old parser (I mean parse.y) doesn't catch this issue at all and the error is emitted at YARV byte code generation (so we have printed compile error). So Prism seems behaves better and catches the issue earlier.

it "cannot be mixed with numbered parameters" do
-> {
eval "proc { it + _1 }"
}.should raise_error(SyntaxError, /numbered parameters are not allowed when 'it' is already used|'it' is already used in/)
Copy link
Member

@eregon eregon May 29, 2025

Choose a reason for hiding this comment

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

How does this spec pass given https://bugs.ruby-lang.org/issues/21381 and the fact we have a CI job using parse.y?
In general it's probably best to just accept either message.

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

3 participants