Feature #19979
openAllow methods to declare that they don't accept a block via `&nil`
Description
Abstract¶
This feature proposes new syntax to allow methods to explicitly declare that they don't accept blocks, and makes passing of a block to such methods an error.
Background¶
In #15554, it was proposed to automatically detect methods that do not use the block passed to them, and to error if a block was passed to such methods. As far as I can tell, it was later on closed since #10499 solved a large part of the problem.
That proposal has, as part of a dev meeting discussion, a proposal from @matz (Yukihiro Matsumoto) to allow methods to use &nil
to explicitly declare that they don't accept a block. At the time, the proposal was trying to solve a bigger problem, so this sub-proposal was never considered seriously. However, notes in the proposal say:
It is explicit, but it is tough to add this
&nil
parameter declaration to all of methods (do you want to add it todef []=(i, e, &nil)
?). (I agree&nil
is valuable on some situations)
This proposal extracts that sub-proposal to make this a new language feature.
Proposal¶
In Ruby, it is always valid for the caller to pass a block to a method call, even if the callee is not expecting a block to be passed. This leads to subtle user errors, where the author of some code assumes a method call uses a block, but the block passed to the method call is silently ignored.
The proposal is to introduce &nil
at method declaration sites to mean "This method does not accept a block". This is symmetric to the ability to pass &nil
at call sites to mean "I am not passing a block to this method call", which is sometimes useful when making super
calls (since blocks are always implicitly passed).
Explicitly, the proposal is to make the following behaviour be a part of Ruby:
def find(item = nil, &nil)
# some implementation that doesn't call `yield` or `block_given?`
end
find { |i| i == 42 }
# => ArgumentError: passing block to the method `find' that does not accept a block.
Implementation¶
I assume the implementation would be a grammar change to make &nil
valid at method declaration sites, as well as raising an ArgumentError
for methods that are called with a block but are declared with &nil
.
Evaluation¶
Since I don't have an implementation, I can't make a proper evaluation of the feature proposal. However, I would expect the language changes to be minimal with no runtime costs for methods that don't use the &nil
syntax.
Discussion¶
This proposal has much smaller scope than #15554 so that the Ruby language can start giving library authors the ability to explicitly mark their methods as not accepting a block. This is fully backward compatible, since it is an opt-in behaviour and not an opt-out one.
Future directions after this feature proposal could be a way to signal to the VM that any method in a file that doesn't explicitly use yield
/block_given?
or explicitly declared a block parameter should be treated as not accepting a block. This can be done via some kind of pragma similar to frozen_string_literal
, or through other means. However, such future directions are beyond the scope of this proposal.
Summary¶
Adding the ability for methods to declare that they don't accept a block will make writing code against such methods safer and more resilient, and will prevent silently ignored behaviour that is often hard to catch or troubleshoot.
Updated by ufuk (Ufuk Kayserilioglu) about 1 year ago
- Description updated (diff)
Updated by Eregon (Benoit Daloze) about 1 year ago
This leads to subtle user errors, where the author of some code assumes a method call uses a block, but the block passed to the method call is silently ignored.
Is this a frequent error?
I think the only times I saw it is due to confusion of the priority of do/end
vs {/}
(which needs to be learned separately anyway), and for some poorly-designed open
methods which don't take a block.
Note that this can already be done in a way that runs on older Rubies too with raise ArgumentError, 'this method does not accept a block' if block_given?
.
I think even if it was accepted it would be very unlikely for gems to use &nil
, first they would need to require Ruby 3.3+ and second it's extra noise in the source code with almost no benefit for the gem.
So I think in practice this would not achieve much.
For methods which sound like they might take a block but don't, the raise ArgumentError, ...
above seems a good existing solution.
It's also a bit similar to **nil
, which is almost never used.
Updated by Eregon (Benoit Daloze) about 1 year ago
Regarding #15554 I think that has the significant advantage that it does not require any changes in gems to be useful. It just automatically knows if a method accepts a block or not.
Updated by zverok (Victor Shepelev) about 1 year ago
Is this a frequent error?
I saw some, but most of them were about mistaking one method for another, or mis-guessing what the method should do without reading docs.
The example of the former is using Time.freeze { some code }
in tests instead of Timecop.freeze { some code }
, and the tests are "green" (because the block is ignored). In this case, and in most of the others I can recall, it is unlikely that the author of the misused method would ask themselves, "ugh, shouldn't I explicitly prohibit blocks in case somebody passes that erroneously?" So I am also not sure where the currently proposed feature would help.
Also, as a negative consequence, I can imagine a linter-enforced rule to "always add this declaration when the method doesn't need/accept block," which will make a lot of code worse.
#15554, OTOH, would help a lot (and I am not sure why is it closed, no reasons are given at https://bugs.ruby-lang.org/issues/15554?tab=history#note-9
Updated by rubyFeedback (robert heiler) about 1 year ago
Is this a frequent error?
I think the only times I saw it is due to confusion of the priority
of do/end vs {/} (which needs to be learned separately anyway), and
for some poorly-designed open methods which don't take a block.
Personally I never saw such an error and can not recall having made
such an error either. I also never had a need to prevent passing of
blocks, but I also agree that there may be situations where one may
specifically want to disable that, even though I personally never had
such a use case.
I think the only times when I had issues with calling parent methods,
and it confused me, was when I used super() versus super without the
(). Since then I, oddly enough, transitioned towards using super()
as default (unless I need the behaviour of super without () specifically).
If we reason that ruby is to be a flexible language, then I think we
can reason that ruby should have a way to disallow passing a block,
just as you, as a ruby developer, can decide on how many arguments
a method should accept, including keyword arguments (which, by the
way, also confused me when I first saw them; I also dislike when a
third party API demands of me to use keyword arguments - I always
felt that an option hash is less restrictive and in some ways
equivalent.)
Note that personally I am not a huge fan of the &nil, but I also
don't want to be too discouraging. One concern I have is that
it may still confuse users, when they see &block and then &nil
and then &urmom ... not sure I am a huge fan of that. I kind
of default to &block - somehow to me this is the most sensible
name for a block variable to be had. If others use &nil, in their
code base - well, they can do what they want to in their code
base. I only focus on trying to maintain my own ruby code and
hope that there is some sanity in what I maintain. :)
But, the TL;DR, I agree that it does not seem to be too common,
at the least I can not recall having seen it before. I don't
know all the ruby code out there, though.
I think even if it was accepted it would be very unlikely
for gems to use &nil, first they would need to require
Ruby 3.3+ and second it's extra noise in the source code
with almost no benefit for the gem.
So I think in practice this would not achieve much.
I kind of agree with that, but I suppose this may be a long
term suggestion and change, so you could re-evaluate in,
say +3 years. It may not be too overly common though; I can
not think of any real use case I have where I specifically
need to prevent passing of a block. To me I always treated
block variables as a nice, additional argument to allow for
more flexibility, rather than as a basic-must-have that
is the main driving factor for designing APIs, libraries and
use cases.
I do not think that "only on ruby 3.3" is a necessary exclusive
criterium, though. For instance, I try to keep my code always
up-to-date with the latest ruby xmas release. Sometimes I may
try to maintain backwards compatibility, but it is never my
primary focus. I always default with MRI these days; it simply
is easier to do so. (I understand that some implementations lag
behind, e. g. jruby - I think they are still not at 3.2 or so
if I recall headius' summary-issue-tracker for it.)
For methods which sound like they might take a block but
don't, the raise ArgumentError, ... above seems a good
existing solution.
Understandable as well, but I would like to point out that
raising an error may not necessarily always be that useful
or necessary. For instance, I tend to not care too much about
block variables, so I rarely ever raise an error about it.
At best I may give a notice to the user, but that's about it
for block variables usually.
It's also a bit similar to **nil, which is almost never used.
Agreed. I actually also never used **nil so far.
zverok wrote:
So I am also not sure where the currently proposed feature would help.
Agreed, but matz also discussed this before, so there may be a use
case, even if it may be very small.
zverok wrote:
Also, as a negative consequence, I can imagine a linter-enforced
rule to "always add this declaration when the method doesn't
need/accept block," which will make a lot of code worse.
I think that can be tweaked, e. g. like rubocop styling rules.
Any linter that would tell me to sprinkle my sane code via &nil
I'd throw out the door though. ;)
zverok wrote:
OTOH, would help a lot (and I am not sure why is it closed, no reasons are
given at https://bugs.ruby-lang.org/issues/15554?tab=history#note-9
That's from 5 years ago or so. Perhaps it may be useful to have
it at an upcoming developer meeting, discussed, and then also
someone who feels motivated and has time, to explain the decision
making towards pro or con of it anyway, since that seems to be
missing slightly (or, at the least, the context has not been put
down in detail; ufuk here may have given the biggest summary so
far about it; also note that ko1 in #15554 also mentioned a warning
rather than the raise ArgumentError as pointed out by eregon. I
guess it comes down to how you want to treat block variables in
general, which is a design consideration of ruby.)
Updated by ufuk (Ufuk Kayserilioglu) about 1 year ago
Is this a frequent error?
I would not say that it is frequent but for the case where it happens the resulting behaviour is confusing and misleading. I have caught some instances in Shopify's code base, most of them in tests for mocking as @zverok (Victor Shepelev) says, where folks expected to add mock behaviour via a block, but the mocking method didn't expect one. That rendered the tests to end up testing the wrong thing, which is worse than not having tests at all.
An explicit error or warning for such cases would be for the benefit of everyone.
I think the only times I saw it is due to confusion of the priority of
do/end
vs{/}
(which needs to be learned separately anyway), and for some poorly-designedopen
methods which don't take a block.
Yes, this is another source of such errors.
Note that this can already be done in a way that runs on older Rubies too with
raise ArgumentError, 'this method does not accept a block' if block_given?
.
True, it can be, but making it explicit in the method signature will also allow tooling to take such behaviour into account as well. A static analyzer will be able to consume a &nil
annotation and act on it, but won't be able to do the same for implicit raise ArgumentError
code.
I think even if it was accepted it would be very unlikely for gems to use
&nil
, first they would need to require Ruby 3.3+ and second it's extra noise in the source code with almost no benefit for the gem.
So I think in practice this would not achieve much.
Of course, but that is true for any new syntax. In my opinion, if we had adopted the &nil
annotation back in 2019 when it was originally suggested, we would have had it in all supported versions today. I would like us to be there for Ruby 3.6, at least.
It's also a bit similar to
**nil
, which is almost never used.
The intention isn't for this to be frequently used, but useful when an API author wants to make sure that the API is defined strictly.
Regarding #15554 I think that has the significant advantage that it does not require any changes in gems to be useful. It just automatically knows if a method accepts a block or not.
Yes, doing it automatically would be my preference as well, but it seems that are many caveats to doing proper detection of calls to a block that makes automation not feasible. For example, we would have to forbid the ability to do eval("yield")
which is a big ask.
Also, as a negative consequence, I can imagine a linter-enforced rule to "always add this declaration when the method doesn't need/accept block," which will make a lot of code worse.
Again, my intention isn't to make this be the default state of affairs for general Ruby code. If I am writing a "mocking library" for example and my mocks don't accept any blocks for mocking behaviour, I would like to make sure my public mock methods are annotated with &nil
so that end users don't use it incorrectly. I can also see Ruby stdlib/core methods using the annotation as well. Doing so would make calls to Time.freeze { some code }
error out, for example.
Updated by Eregon (Benoit Daloze) about 1 year ago
ufuk (Ufuk Kayserilioglu) wrote in #note-6:
A static analyzer will be able to consume a
&nil
annotation and act on it, but won't be able to do the same for implicitraise ArgumentError
code.
A static analyzer could detect a raise ... if block_given?
if at the start of a method/not under any other control flow.
Yes, doing it automatically would be my preference as well, but it seems that are many caveats to doing proper detection of calls to a block that makes automation not feasible. For example, we would have to forbid the ability to do
eval("yield")
which is a big ask.
Regarding eval("yield")
probably we should have a way to mark methods using a block in a very hidden way like that, just adding &block
would be enough (or maybe even just &
).
I would suspect very few places use that (template engines do, though), so it'd be OK to warn until they explicitly declare a block might be used.
If I am writing a "mocking library" for example and my mocks don't accept any blocks for mocking behaviour, I would like to make sure my public mock methods are annotated with
&nil
so that end users don't use it incorrectly.
I think it'd be good to use raise ... if block_given?
for these methods which are unclear whether they accept a block.
We'd want to solve it for these particular mock library methods before Ruby 3.6, no?
Updated by ufuk (Ufuk Kayserilioglu) about 1 year ago
Eregon (Benoit Daloze) wrote in #note-7:
A static analyzer could detect a
raise ... if block_given?
if at the start of a method/not under any other control flow.
I understand your position and respect it, but I can't help but feel like this workaround is similar to if the language said "We don't have required keywords, all keywords are optional. In order to have required keywords, you need to check if the keyword is supplied or not, and raise". We have the concept of required keywords since they communicate intent and make it easier for the VM and developers to reason about the code as it is written. That's why I think the addition of &nil
would be a good thing, even if the benefits can only be unlocked 3 years into the future.
We'd want to solve it for these particular mock library methods before Ruby 3.6, no?
Sure, and those libraries can start using the raise ... if block_given?
workaround today, but move to using &nil
a few years later. On the other hand, we at Shopify move our baseline Ruby across the company aggressively to the brand new version of Ruby every year. If the &nil
syntax makes it to Ruby 3.3, we could start using it for a lot of our internal code by May 2024, for example. However, by delaying implementation, we would be making sure that no one gets to benefit from this useful language feature.
Updated by kddnewton (Kevin Newton) about 1 year ago
I'm +1 on this proposal. Anything that makes the implicit block more explicit.
On the static analysis point, a static analyzer can't detect that without knowing if raise
and block_given?
are not overridden. With &nil
it would be explicit, and we could build up common knowledge around it.
I'm not particularly worried about the proliferation of it, as we haven't seen that with **nil
either. But in the cases where it is necessary, it's nice to have that option.
I'm not sure I understand the point about Ruby 3.6 - that point could be made to try to refute any syntax addition. We're going to have a Ruby 3.6 anyway, we may as well have it with &nil
.
Updated by Dan0042 (Daniel DeLorme) about 1 year ago
It's also a bit similar to
**nil
, which is almost never used.The intention isn't for this to be frequently used, but useful when an API author wants to make sure that the API is defined strictly.
If it's not meant to be frequently used, I don't think it's worth adding new syntax. I mean, a new method is fine and can often be backported/polyfilled, but the burden of new syntax is much higher and I don't think this is worth it. IMHO #15554 is an order of magnitude better approach.
Updated by matz (Yukihiro Matsumoto) 9 months ago
Honestly, '&nil' isn't very appealing looking, but I'm willing to accept this suggestion because I was hoping for a way to explicitly indicate that a method won't accept a block. There is already an equally unattractive **nil
notation in use, so that's no reason to reject it.
Matz.
Updated by mame (Yusuke Endoh) 9 months ago
How about introducing a magic comment like # no_block_by_default: true
? Methods defined under this magic comment should raise an exception if a block is passed, unless they explicitly receive &blk
or literally have yield
keyword within their definition.
I feel that adding &nil
to individual methods seems like repeating the history of introducing .freeze
and then # frozen_string_literal: true
.
Updated by nobu (Nobuyoshi Nakada) 9 months ago
Updated by Eregon (Benoit Daloze) 9 months ago
Updated by Dan0042 (Daniel DeLorme) 9 months ago
Updated by matz (Yukihiro Matsumoto) 9 months ago
- Related to Feature #15554: warn/error passing a block to a method which never use a block added
Updated by matz (Yukihiro Matsumoto) 9 months ago
Actually, I prefer #15554, but my concern is that it causes compatibility issues with false positives.
Matz.
Updated by Eregon (Benoit Daloze) 7 months ago
From https://bugs.ruby-lang.org/issues/20436#note-12:
@ufuk (Ufuk Kayserilioglu) wrote:
Can we at least get runtime introspection for methods that should not be accepting a block? Something like:
method(:foo).parameters #=> [:noblock]
maybe?
#parameters
represents the parameters defined in the source code, so it does not seem the right place.
Also I would think alternative Ruby implementations might not want to implement the check of #15554 (or not immediately), as it is essentially a warning to help developers but likely requires a significant effort to implement.
So if a predicate is provided for this I think it should be clearly marked as implementation-specific.
What would you use this predicate for?
Updated by kddnewton (Kevin Newton) 7 months ago
This already exists for **nil
, as in:
irb(main):001> def foo(**nil); end
=> :foo
irb(main):002> method(:foo).parameters
=> [[:nokey]]
irb(main):003>
I think adding this would be really nice from any static perspective — it gets around having to attempt to interpret the body.
Updated by Eregon (Benoit Daloze) 7 months ago
True, OTOH the trade-offs made in #15554 like every method using super
considered to "accept a block" might not be suitable for static analysis.
And it's also fairly easy to find whether a method uses yield
and has a block parameter, e.g. using Prism.
Updated by ufuk (Ufuk Kayserilioglu) 5 months ago
The result of the discussion at the face to face dev meeting in Naha was that while #15554 is preferred, it has some drawbacks and an explicit method for declaring that a method accepts no blocks is still a good thing to have.
For that reason, I've gone ahead with implementing this feature (based on the original work done by @nobu (Nobuyoshi Nakada), thank you!) and exposed a &nil
declaration through {Method,UnboundMethod,Proc}#parameters
as [[:noblock]]
: https://github.com/ruby/ruby/pull/11065
Updated by matz (Yukihiro Matsumoto) 4 months ago
This proposal is suspended due to the Syntax Moratorium. Since this might cause "no block policing" in the community, let me think while (at least til 3.5).
Matz.