From: "Dan0042 (Daniel DeLorme) via ruby-core" <ruby-core@ml.ruby-lang.org>
To: ruby-core@ml.ruby-lang.org
Cc: "Dan0042 (Daniel DeLorme)" <noreply@ruby-lang.org>
Subject: [ruby-core:115272] [Ruby master Feature#15554] warn/error passing a block to a method which never use a block
Date: Mon, 06 Nov 2023 14:01:24 +0000 (UTC) [thread overview]
Message-ID: <redmine.journal-105188.20231106140124.17@ruby-lang.org> (raw)
In-Reply-To: redmine.issue-15554.20190122044810.17@ruby-lang.org
Issue #15554 has been updated by Dan0042 (Daniel DeLorme).
I think this was closed by mistake and should be re-opened.
----------------------------------------
Feature #15554: warn/error passing a block to a method which never use a block
https://bugs.ruby-lang.org/issues/15554#change-105188
* Author: ko1 (Koichi Sasada)
* Status: Closed
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
----------------------------------------
# Abstract
Warn or raise an ArgumentError if block is passed to a method which does not use a block.
In other words, detect "block user methods" implicitly and only "block user methods" can accept a block.
# Background
Sometimes, we pass a block to a method which ignores the passed block accidentally.
```
def my_open(name)
open(name)
end
# user hopes it works as Kernel#open which invokes a block with opened file.
my_open(name){|f| important_work_with f }
# but simply ignored...
```
To solve this issue, this feature request propose showing warnings or raising an exception on such case.
Last developer's meeting, matz proposed `&nil` which declares this method never receive a block. 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)
# Spec
## Define "use a block" methods
We need to define which method accepts a block and which method does not.
* (1) method has a block parameter (`&b`)
* (2) method body has `yield'
* (3) method body has `super` (ZSUPER in internal terminology) or `super(...)`
* (4) method body has singleton method (optional)
(1) and (2) is very clear. I need to explain about (3) and (4).
(3). `super` (ZSUPER) passes all parameters as arguments. So there is no surprise that which can accept `block`.
However `super(...)` also passes a block if no explicit block passing (like `super(){}` or `super(&b)`) are written.
I'm not sure we need to continue this strange specification, but to keep compatibility depending this spec, I add this rule.
(4). surprisingly, the following code invoke a block:
```
def foo
class << Object.new
yield
end
end
foo{ p :ok } #=> :ok
```
I'm also not sure we need to keep this spec, but to allow this spec, I added (4) rule.
Strictly speaking, it is not required, but we don't keep the link from singleton class ISeq to lexical parent iseq now, so I added it.
## Exceptional cases
A method called by `super` doesn`t warn warning even if this method doesn't use a block.
The rule (3) can pass blocks easily and there are many methods don`t use a block.
So my patch ignores callings by `super`.
## corner cases
There are several cases to use block without (1)-(4) rules.
### `Proc.new/proc/lambda` without a block
Now it was deprecated in r66772 (commit:9f1fb0a17febc59356d58cef5e98db61a3c03550).
Related discussion: [Bug #15539]
### `block_given?`
`block_given?` expects block, but I believe we use it with `yield` or a block parameter.
If you know the usecase without them, please tell us.
### `yield` in `eval`
We can't know `yield` (or (3), (4) rule) in an `eval` evaluating string at calling time.
```
def foo
eval('yield`)
end
foo{} # at calling time,
# we can't know the method foo can accept a block or not.
```
So I added a warning to use `yield` in `eval` like that: `test.rb:4: warning: use yield in eval will not be supported in Ruby 3.`
Workaround is use a block parameter explicitly.
```
def foo &b
eval('b.call')
end
foo{ p :ok }
```
# Implementation
Strategy is:
* [compile time] introduce `iseq::has_yield` field and check it if the iseq (or child iseq) contains `yield` (or something)
* [calling time] if block is given, check `iseq::has_yield` flag and show warning (or raise an exception)
https://gist.github.com/ko1/c9148ad0224bf5befa3cc76ed2220c0b
On this patch, now it raises an error to make it easy to detect.
It is easy to switch to show the warning.
# Evaluation and discussion
I tried to avoid ruby's tests.
https://gist.github.com/ko1/37483e7940cdc4390bf8eb0001883786
Here is a patch.
There are several patterns to avoid warnings.
## tests for `block_given?`, `Proc.new` (and similar) without block
Add a dummy block parameter.
It is test-specific issue.
## empty `each`
Some tests add `each` methods do not `yield`, like: `def each; end`.
Maybe test-specific issue, and adding a dummy block parameter.
## Subtyping / duck typing
https://github.com/ruby/ruby/blob/c01a5ee85e2d6a7128cccafb143bfa694284ca87/lib/optparse.rb#L698
This `parse` method doesn't use `yield`, but other sub-type's `parse` methods use.
## `super` with `new` method
https://gist.github.com/ko1/37483e7940cdc4390bf8eb0001883786#file-tests-patch-L61
This method override `Class#new` method and introduce a hook with block (yield a block in this hook code).
https://github.com/ruby/ruby/blob/trunk/lib/rubygems/package/tar_writer.rb#L81
In this method, call `super` and it also passing a block. However, called `initialize` doesn't use a block.
## Change robustness
This change reduce robustness for API change.
`Delegator` requires to support `__getobj__` for client classes.
Now `__getobj__` should accept block but most of `__getobj__` clients do not call given block.
https://github.com/ruby/ruby/blob/trunk/lib/delegate.rb#L80
This is because of delegator.rb's API change.
https://gist.github.com/ko1/37483e7940cdc4390bf8eb0001883786#file-tests-patch-L86
Nobu says calling block is not required (ignoring a block is no problem) so it is not a bug for delegator client classes.
## Found issues.
```
[ 2945/20449] Rinda::TestRingServer#test_do_reply = 0.00 s
1) Error:
Rinda::TestRingServer#test_do_reply:
ArgumentError: passing block to the method "with_timeout" (defined at /home/ko1/src/ruby/trunk/test/rinda/test_rinda.rb:787) is never used.
/home/ko1/src/ruby/trunk/test/rinda/test_rinda.rb:635:in `test_do_reply'
[ 2946/20449] Rinda::TestRingServer#test_do_reply_local = 0.00 s
2) Error:
Rinda::TestRingServer#test_do_reply_local:
ArgumentError: passing block to the method "with_timeout" (defined at /home/ko1/src/ruby/trunk/test/rinda/test_rinda.rb:787) is never used.
/home/ko1/src/ruby/trunk/test/rinda/test_rinda.rb:657:in `test_do_reply_local'
[10024/20449] TestGemRequestSetGemDependencyAPI#test_platform_mswin = 0.01 s
3) Error:
TestGemRequestSetGemDependencyAPI#test_platform_mswin:
ArgumentError: passing block to the method "util_set_arch" (defined at /home/ko1/src/ruby/trunk/lib/rubygems/test_case.rb:1053) is never used.
/home/ko1/src/ruby/trunk/test/rubygems/test_gem_request_set_gem_dependency_api.rb:655:in `test_platform_mswin'
[10025/20449] TestGemRequestSetGemDependencyAPI#test_platforms = 0.01 s
4) Error:
TestGemRequestSetGemDependencyAPI#test_platforms:
ArgumentError: passing block to the method "util_set_arch" (defined at /home/ko1/src/ruby/trunk/lib/rubygems/test_case.rb:1053) is never used.
/home/ko1/src/ruby/trunk/test/rubygems/test_gem_request_set_gem_dependency_api.rb:711:in `test_platforms'
```
These 4 detection show the problem. `with_timeout` method (used in Rinda test) and `util_set_arch` method (used in Rubygems test) simply ignore the given block.
So these tests are simply ignored.
I reported them. (https://github.com/rubygems/rubygems/issues/2601)
## raise an error or show a warning?
At least, Ruby 2.7 should show warning for this kind of violation with `-w`.
How about for Ruby3?
--
https://bugs.ruby-lang.org/
______________________________________________
ruby-core mailing list -- ruby-core@ml.ruby-lang.org
To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/
next parent reply other threads:[~2023-11-06 14:01 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <redmine.issue-15554.20190122044810.17@ruby-lang.org>
2023-11-06 14:01 ` Dan0042 (Daniel DeLorme) via ruby-core [this message]
2024-02-29 8:05 ` [ruby-core:117005] [Ruby master Feature#15554] warn/error passing a block to a method which never use a block ko1 (Koichi Sasada) via ruby-core
2024-02-29 8:10 ` [ruby-core:117006] " ko1 (Koichi Sasada) via ruby-core
2024-02-29 8:12 ` [ruby-core:117007] " ko1 (Koichi Sasada) via ruby-core
2024-02-29 9:15 ` [ruby-core:117009] " Eregon (Benoit Daloze) via ruby-core
2024-02-29 10:59 ` [ruby-core:117011] " Edwing123 (Edwin Garcia) via ruby-core
2024-03-01 1:16 ` [ruby-core:117020] " Dan0042 (Daniel DeLorme) via ruby-core
2024-03-01 12:43 ` [ruby-core:117031] " Eregon (Benoit Daloze) via ruby-core
2024-03-10 14:23 ` [ruby-core:117096] " Dan0042 (Daniel DeLorme) via ruby-core
2024-03-14 6:11 ` [ruby-core:117139] " ko1 (Koichi Sasada) via ruby-core
2024-04-15 3:22 ` [ruby-core:117512] " ko1 (Koichi Sasada) via ruby-core
2024-04-15 21:33 ` [ruby-core:117515] " byroot (Jean Boussier) via ruby-core
2024-04-15 21:40 ` [ruby-core:117516] " byroot (Jean Boussier) via ruby-core
2024-04-17 10:17 ` [ruby-core:117558] " byroot (Jean Boussier) via ruby-core
2024-04-17 11:07 ` [ruby-core:117561] " ko1 (Koichi Sasada) via ruby-core
2024-04-17 11:12 ` [ruby-core:117562] " ko1 (Koichi Sasada) via ruby-core
2024-04-17 12:34 ` [ruby-core:117566] " byroot (Jean Boussier) via ruby-core
2024-04-17 12:52 ` [ruby-core:117567] " Eregon (Benoit Daloze) via ruby-core
2024-04-17 16:46 ` [ruby-core:117569] " byroot (Jean Boussier) via ruby-core
2024-04-17 19:28 ` [ruby-core:117576] " Eregon (Benoit Daloze) via ruby-core
2024-04-17 19:30 ` [ruby-core:117577] " byroot (Jean Boussier) via ruby-core
2024-04-18 1:34 ` [ruby-core:117583] " ko1 (Koichi Sasada) via ruby-core
2024-04-18 2:34 ` [ruby-core:117584] " akr (Akira Tanaka) via ruby-core
2024-04-18 3:14 ` [ruby-core:117586] " mame (Yusuke Endoh) via ruby-core
2024-04-18 6:07 ` [ruby-core:117590] " jeremyevans0 (Jeremy Evans) via ruby-core
2024-04-18 12:18 ` [ruby-core:117598] " Eregon (Benoit Daloze) via ruby-core
2024-04-19 0:53 ` [ruby-core:117602] " Dan0042 (Daniel DeLorme) via ruby-core
2024-04-19 4:31 ` [ruby-core:117606] " ko1 (Koichi Sasada) via ruby-core
2024-04-19 5:38 ` [ruby-core:117607] " byroot (Jean Boussier) via ruby-core
2024-04-19 8:07 ` [ruby-core:117608] " byroot (Jean Boussier) via ruby-core
2024-04-19 11:10 ` [ruby-core:117609] " Dan0042 (Daniel DeLorme) via ruby-core
2024-04-19 11:15 ` [ruby-core:117610] " byroot (Jean Boussier) via ruby-core
2024-04-19 12:04 ` [ruby-core:117611] " Dan0042 (Daniel DeLorme) via ruby-core
2024-04-19 12:11 ` [ruby-core:117612] " byroot (Jean Boussier) via ruby-core
2024-04-19 12:15 ` [ruby-core:117613] " Dan0042 (Daniel DeLorme) via ruby-core
2024-04-19 14:02 ` [ruby-core:117614] " Eregon (Benoit Daloze) via ruby-core
2024-04-20 6:58 ` [ruby-core:117622] " matthewd (Matthew Draper) via ruby-core
2024-04-20 7:51 ` [ruby-core:117623] " byroot (Jean Boussier) via ruby-core
2024-04-22 10:41 ` [ruby-core:117643] " byroot (Jean Boussier) via ruby-core
2024-04-23 2:17 ` [ruby-core:117648] " ko1 (Koichi Sasada) via ruby-core
2024-04-23 7:01 ` [ruby-core:117651] " byroot (Jean Boussier) via ruby-core
2024-04-23 21:38 ` [ruby-core:117663] " Dan0042 (Daniel DeLorme) via ruby-core
2024-04-24 6:19 ` [ruby-core:117669] " byroot (Jean Boussier) via ruby-core
2024-04-24 6:35 ` [ruby-core:117670] " ko1 (Koichi Sasada) via ruby-core
2024-05-07 4:01 ` [ruby-core:117785] " nobu (Nobuyoshi Nakada) via ruby-core
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.ruby-lang.org/en/community/mailing-lists/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=redmine.journal-105188.20231106140124.17@ruby-lang.org \
--to=ruby-core@ruby-lang.org \
--cc=noreply@ruby-lang.org \
--cc=ruby-core@ml.ruby-lang.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).