Project

General

Profile

Actions

Feature #19979

open

Allow methods to declare that they don't accept a block via `&nil`

Added by ufuk (Ufuk Kayserilioglu) about 1 year ago. Updated 5 months ago.

Status:
Open
Assignee:
-
Target version:
-
[ruby-core:115196]

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 to def []=(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.


Related issues 1 (0 open1 closed)

Related to Ruby master - Feature #15554: warn/error passing a block to a method which never use a blockClosedmatz (Yukihiro Matsumoto)Actions
Actions #1

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-designed open 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 implicit raise 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) 10 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) 10 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 Eregon (Benoit Daloze) 10 months ago

Is there any reason not to do #15554 instead?
This proposal requires manual effort to annotate methods, which feels to me very un-Ruby-like. #15554 is automatic.

Regarding yield in eval that's already discussed in #15554 and requiring to use a named block seems reasonable, no?

Updated by Dan0042 (Daniel DeLorme) 10 months ago

Eregon (Benoit Daloze) wrote in #note-14:

Is there any reason not to do #15554 instead?
This proposal requires manual effort to annotate methods, which feels to me very un-Ruby-like. #15554 is automatic.

+1

Actions #16

Updated by matz (Yukihiro Matsumoto) 10 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) 10 months ago

Actually, I prefer #15554, but my concern is that it causes compatibility issues with false positives.

Matz.

Updated by Eregon (Benoit Daloze) 8 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) 8 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) 8 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) 6 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) 5 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.

Actions

Also available in: Atom PDF

Like2
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like1Like0Like0Like0Like0Like0Like0Like0Like0