April 2024 - ruby-core - ml.ruby-lang.org

[ruby-core:117643] [Ruby master Feature#15554] warn/error passing a block to a method which never use a block
by byroot (Jean Boussier) 22 Apr '24

22 Apr '24

Issue #15554 has been updated by byroot (Jean Boussier). @ko1 here's the result (I ran it on the 5 most important sub components) ``` ### Active Record activemodel/lib/active_model/attribute.rb:63: warning: the block passed to 'ActiveModel::Type::Value#serializable?' defined at activemodel/lib/active_model/type/value.rb:28 may be ignored activerecord/lib/arel/collectors/composite.rb:34: warning: the block passed to 'Arel::Collectors::Bind#add_binds' defined at activerecord/lib/arel/collectors/bind.rb:21 may be ignored activerecord/lib/arel/visitors/to_sql.rb:352: warning: the block passed to 'Arel::Collectors::SubstituteBinds#add_binds' defined at activerecord/lib/arel/collectors/substitute_binds.rb:23 may be ignored activerecord/lib/active_record/associations/association_scope.rb:148: warning: the block passed to 'ActiveRecord::Associations::AssociationScope::ReflectionProxy#all_includes' defined at activerecord/lib/active_record/associations/association_scope.rb:109 may be ignored activemodel/lib/active_model/attribute_set/builder.rb:43: warning: the block passed to 'ActiveModel::Attribute#value' defined at activemodel/lib/active_model/attribute.rb:41 may be ignored activemodel/lib/active_model/attribute_set.rb:51: warning: the block passed to 'ActiveModel::Attribute#value' defined at activemodel/lib/active_model/attribute.rb:41 may be ignored activerecord/test/cases/transactions_test.rb:158: warning: the block passed to 'ActiveRecord::ConnectionAdapters::NullTransaction#after_rollback' defined at activerecord/lib/active_record/connection_adapters/abstract/transaction.rb:121 may be ignored activerecord/lib/arel.rb:70: warning: the block passed to 'Arel::Nodes::Node#fetch_attribute' defined at activerecord/lib/arel/nodes/node.rb:155 may be ignored activerecord/lib/arel/visitors/to_sql.rb:757: warning: the block passed to 'Arel::Collectors::SubstituteBinds#add_bind' defined at activerecord/lib/arel/collectors/substitute_binds.rb:18 may be ignored activerecord/lib/arel/visitors/to_sql.rb:757: warning: the block passed to 'ActiveRecord::StatementCache::PartialQueryCollector#add_bind' defined at activerecord/lib/active_record/statement_cache.rb:77 may be ignored activerecord/lib/arel/visitors/to_sql.rb:352: warning: the block passed to 'ActiveRecord::StatementCache::PartialQueryCollector#add_binds' defined at activerecord/lib/active_record/statement_cache.rb:83 may be ignored activerecord/lib/active_record/migration/default_strategy.rb:10: warning: the block passed to 'drop_table' defined at activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb:531 may be ignored activerecord/lib/active_record/querying.rb:23: warning: the block passed to 'with' defined at activerecord/lib/active_record/relation/query_methods.rb:475 may be ignored /opt/rubies/3.4-dev-04-19/lib/ruby/3.4.0+0/delegate.rb:84: warning: the block passed to 'WeakRef#__getobj__' defined at /opt/rubies/3.4-dev-04-19/lib/ruby/3.4.0+0/weakref.rb:44 may be ignored activerecord/lib/arel/visitors/to_sql.rb:782: warning: the block passed to 'Arel::Collectors::SubstituteBinds#add_binds' defined at activerecord/lib/arel/collectors/substitute_binds.rb:23 may be ignored /opt/rubies/3.4-dev-04-19/lib/ruby/3.4.0+0/delegate.rb:349: warning: the block passed to 'ActiveModel::Type::Value#serializable?' defined at activemodel/lib/active_model/type/value.rb:28 may be ignored activerecord/lib/arel/nodes/grouping.rb:7: warning: the block passed to 'Arel::Nodes::SqlLiteral#fetch_attribute' defined at activerecord/lib/arel/nodes/sql_literal.rb:22 may be ignored activerecord/lib/active_record/migration/default_strategy.rb:10: warning: the block passed to 'drop_join_table' defined at activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb:418 may be ignored ### Active Model activemodel/lib/active_model/attribute_set/builder.rb:55: warning: the block passed to 'ActiveModel::Attribute#value' defined at activemodel/lib/active_model/attribute.rb:41 may be ignored ### Action View actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::RawFile#render' defined at actionview/lib/action_view/template/raw_file.rb:20 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::Text#render' defined at actionview/lib/action_view/template/text.rb:23 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::Renderable#render' defined at actionview/lib/action_view/template/renderable.rb:15 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::Renderable#render' defined at actionview/lib/action_view/template/renderable.rb:15 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::Renderable#render' defined at actionview/lib/action_view/template/renderable.rb:15 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::Text#render' defined at actionview/lib/action_view/template/text.rb:23 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::Text#render' defined at actionview/lib/action_view/template/text.rb:23 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::Text#render' defined at actionview/lib/action_view/template/text.rb:23 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::Text#render' defined at actionview/lib/action_view/template/text.rb:23 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::Text#render' defined at actionview/lib/action_view/template/text.rb:23 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::Text#render' defined at actionview/lib/action_view/template/text.rb:23 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::Text#render' defined at actionview/lib/action_view/template/text.rb:23 may be ignored activerecord/lib/arel/collectors/composite.rb:28: warning: the block passed to 'Arel::Collectors::Bind#add_bind' defined at activerecord/lib/arel/collectors/bind.rb:16 may be ignored activerecord/lib/arel/collectors/composite.rb:28: warning: the block passed to 'Arel::Collectors::Bind#add_bind' defined at activerecord/lib/arel/collectors/bind.rb:16 may be ignored activerecord/lib/arel/collectors/composite.rb:28: warning: the block passed to 'Arel::Collectors::Bind#add_bind' defined at activerecord/lib/arel/collectors/bind.rb:16 may be ignored activerecord/lib/arel/collectors/composite.rb:28: warning: the block passed to 'Arel::Collectors::Bind#add_bind' defined at activerecord/lib/arel/collectors/bind.rb:16 may be ignored activerecord/lib/arel/collectors/composite.rb:28: warning: the block passed to 'Arel::Collectors::Bind#add_bind' defined at activerecord/lib/arel/collectors/bind.rb:16 may be ignored activerecord/lib/arel/collectors/composite.rb:28: warning: the block passed to 'Arel::Collectors::Bind#add_bind' defined at activerecord/lib/arel/collectors/bind.rb:16 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::RawFile#render' defined at actionview/lib/action_view/template/raw_file.rb:20 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::Renderable#render' defined at actionview/lib/action_view/template/renderable.rb:15 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::Renderable#render' defined at actionview/lib/action_view/template/renderable.rb:15 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::Renderable#render' defined at actionview/lib/action_view/template/renderable.rb:15 may be ignored actionview/test/template/template_test.rb:198: warning: the block passed to 'TestERBTemplate#render' defined at actionview/test/template/template_test.rb:65 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::RawFile#render' defined at actionview/lib/action_view/template/raw_file.rb:20 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::RawFile#render' defined at actionview/lib/action_view/template/raw_file.rb:20 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::RawFile#render' defined at actionview/lib/action_view/template/raw_file.rb:20 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::Renderable#render' defined at actionview/lib/action_view/template/renderable.rb:15 may be ignored activerecord/lib/arel/collectors/composite.rb:28: warning: the block passed to 'Arel::Collectors::Bind#add_bind' defined at activerecord/lib/arel/collectors/bind.rb:16 may be ignored activerecord/lib/arel/collectors/composite.rb:28: warning: the block passed to 'Arel::Collectors::Bind#add_bind' defined at activerecord/lib/arel/collectors/bind.rb:16 may be ignored activerecord/lib/arel/collectors/composite.rb:34: warning: the block passed to 'Arel::Collectors::Bind#add_binds' defined at activerecord/lib/arel/collectors/bind.rb:21 may be ignored activerecord/lib/arel/collectors/composite.rb:34: warning: the block passed to 'Arel::Collectors::Bind#add_binds' defined at activerecord/lib/arel/collectors/bind.rb:21 may be ignored activerecord/lib/arel/collectors/composite.rb:34: warning: the block passed to 'Arel::Collectors::Bind#add_binds' defined at activerecord/lib/arel/collectors/bind.rb:21 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::Renderable#render' defined at actionview/lib/action_view/template/renderable.rb:15 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::RawFile#render' defined at actionview/lib/action_view/template/raw_file.rb:20 may be ignored ### Action Pack actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::Text#render' defined at actionview/lib/action_view/template/text.rb:23 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::Text#render' defined at actionview/lib/action_view/template/text.rb:23 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::Text#render' defined at actionview/lib/action_view/template/text.rb:23 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::Text#render' defined at actionview/lib/action_view/template/text.rb:23 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::Renderable#render' defined at actionview/lib/action_view/template/renderable.rb:15 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::RawFile#render' defined at actionview/lib/action_view/template/raw_file.rb:20 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::RawFile#render' defined at actionview/lib/action_view/template/raw_file.rb:20 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::Renderable#render' defined at actionview/lib/action_view/template/renderable.rb:15 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::Renderable#render' defined at actionview/lib/action_view/template/renderable.rb:15 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::HTML#render' defined at actionview/lib/action_view/template/html.rb:24 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::RawFile#render' defined at actionview/lib/action_view/template/raw_file.rb:20 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::HTML#render' defined at actionview/lib/action_view/template/html.rb:24 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::HTML#render' defined at actionview/lib/action_view/template/html.rb:24 may be ignored actionview/lib/action_view/renderer/template_renderer.rb:66: warning: the block passed to 'ActionView::Template::HTML#render' defined at actionview/lib/action_view/template/html.rb:24 may be ignored ``` As expected, it's all false positive caused by "duck-typing". ---------------------------------------- Feature #15554: warn/error passing a block to a method which never use a block https://bugs.ruby-lang.org/issues/15554#change-108054 * Author: ko1 (Koichi Sasada) * Status: Closed * 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/… 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-pat… 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#… 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-pat… 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/

1 0

[ruby-core:117640] [Ruby master Bug#20442] Printing class variable inconsistent when using nested hashes
by k776 (Kieran Pilkington) 22 Apr '24

22 Apr '24

Issue #20442 has been reported by k776 (Kieran Pilkington). ---------------------------------------- Bug #20442: Printing class variable inconsistent when using nested hashes https://bugs.ruby-lang.org/issues/20442 * Author: k776 (Kieran Pilkington) * Status: Open * ruby -v: 3.3.0 * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- See attached test.rb. I have a class with a class variable. The idea is simple, increment the value and then print out the resulting class var. ``` Mock1 0 1 Mock2 {} {"a"=>1} Mock3 {} {"a"=>1} Mock4 {} {} ``` Mock1 through Mock3 are working as expected. But Mock4 does not, it outputs a blank hash, rather than the expected: `{"a"=>{"a"=>1}}` The value is being set. If I replace the print method with `@val['a']['a']` is outputs the number 1 as expected. ---Files-------------------------------- test.rb (820 Bytes) -- https://bugs.ruby-lang.org/

2 1

[ruby-core:116456] [Ruby master Feature#20215] Introduce `IO#readable?`
by ioquatix (Samuel Williams) 22 Apr '24

22 Apr '24

Issue #20215 has been reported by ioquatix (Samuel Williams). ---------------------------------------- Feature #20215: Introduce `IO#readable?` https://bugs.ruby-lang.org/issues/20215 * Author: ioquatix (Samuel Williams) * Status: Open * Priority: Normal ---------------------------------------- There are some cases where, as an optimisation, it's useful to know whether more data is potentially available. We already have `IO#eof?` but the problem with using `IO#eof?` is that it can block indefinitely for sockets. Therefore, code which uses `IO#eof?` to determine if there is potentially more data, may hang. ```ruby def make_request(path = "/") client = connect_remote_host # HTTP/1.0 request: client.write("GET #{path} HTTP/1.0\r\n\r\n") # Read response client.gets("\r\n") # => "HTTP/1.0 200 OK\r\n" # Assuming connection close, there are two things the server can do: # 1. peer.close # 2. peer.write(...); peer.close if client.eof? # <--- Can hang here! puts "Connection closed" # Avoid yielding as we know there definitely won't be any data. else puts "Connection open, data may be available..." # There might be data available, so yield. yield(client) end ensure client&.close end make_request do |client| puts client.read # <--- Prefer to wait here. end ``` The proposed `IO#readable?` is similar to `IO#eof?` but rather than blocking, would simply return false. The expectation is the user will subsequently call `read` which may then wait. The proposed implementation would look something like this: ```ruby class IO def readable? !!self.closed? end end class BasicSocket # Is it likely that the socket is still connected? # May return false positive, but won't return false negative. def readable? return false unless super # If we can wait for the socket to become readable, we know that the socket may still be open. result = self.recv_nonblock(1, MSG_PEEK, exception: false) # No data was available - newer Ruby can return nil instead of empty string: return false if result.nil? # Either there was some data available, or we can wait to see if there is data avaialble. return !result.empty? || result == :wait_readable rescue Errno::ECONNRESET # This might be thrown by recv_nonblock. return false end end ``` For `IO` itself, when there is buffered data, `readable?` would also return true immediately, similar to `eof?`. This is not shown in the above implementation as I'm not sure if there is any Ruby method which exposes "there is buffered data". -- https://bugs.ruby-lang.org/

7 16

[ruby-core:117631] [Ruby master Misc#20441] Should passing keyword args to method_name(*) be an error?
by ozydingo (Andrew Schwartz) 21 Apr '24

21 Apr '24

Issue #20441 has been reported by ozydingo (Andrew Schwartz). ---------------------------------------- Misc #20441: Should passing keyword args to method_name(*) be an error? https://bugs.ruby-lang.org/issues/20441 * Author: ozydingo (Andrew Schwartz) * Status: Open ---------------------------------------- In the following method: ```rb def foo(*) super end ``` it is apparently the intended ruby 3 behavior to pass keyword args as a positional Hash to `super`. I believe this is confusing and can lead to hidden and hard-to-discover bugs (e.g. #20440). Since `*` is meant to only represent positional args, should it be an ArgumentError to pass keyword args at all to this method? Similar to how it is an error to pass positions args to `bar(**)`. -- https://bugs.ruby-lang.org/

2 4

[ruby-core:117624] [Ruby master Bug#20440] `super` from child class passing keyword arg as Hash if in a method with passthrough args called from base class
by ozydingo (Andrew Schwartz) 21 Apr '24

21 Apr '24

Issue #20440 has been reported by ozydingo (Andrew Schwartz). ---------------------------------------- Bug #20440: `super` from child class passing keyword arg as Hash if in a method with passthrough args called from base class https://bugs.ruby-lang.org/issues/20440 * Author: ozydingo (Andrew Schwartz) * Status: Open * ruby -v: 3.3.0 * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- Apologies for the verbose title, but that's the specific set of conditions that AFAICT are required to reproduce the bug! Here's the simplest setup I can reproduce: ```rb class Base def foo(*args, x: 1) puts "Base: calling foo with args: #{args}, x: #{x}" end def foo!(x: 1) puts "Base: calling foo! with x: #{x}" foo(x: x) end end class Child < Base def foo(*) puts "Child: calling foo" super end end ``` When I call `Child.new.foo!`, I expect it to call the base class method `foo!`, which will use the default keyword arg `x: 1`; then the child method `foo` with `x: 1`, and finally the base method `foo` with `x: 1`. However, this is not what I observe: ```rb Child.new.foo! Base: calling foo! with x: 1 Child: calling foo Base: calling foo with args: [{:x=>1}], x: 1 ``` So when the child `foo` method called `super`, it passed not only `x: 1` as a keyword arg, but *also* `{x: 1}` as a Hash positional arg to the super method. This is breaking my upgrade to Ruby 3.0 as I have a similar setup but without the `*args` param, this I am getting the error "wrong number of arguments (given 1, expected 0)". -- https://bugs.ruby-lang.org/

4 6

[ruby-core:117623] [Ruby master Feature#15554] warn/error passing a block to a method which never use a block
by byroot (Jean Boussier) 20 Apr '24

20 Apr '24

Issue #15554 has been updated by byroot (Jean Boussier). > it's basically a variant of the super case. I never thought about it this way, but I think it does make sense yes. > could you compare the results on rails tests? @ko1 sorry didn't answer yet. I'll try to do that Monday. ---------------------------------------- Feature #15554: warn/error passing a block to a method which never use a block https://bugs.ruby-lang.org/issues/15554#change-108031 * Author: ko1 (Koichi Sasada) * Status: Closed * 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/… 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-pat… 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#… 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-pat… 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/

1 0

[ruby-core:117622] [Ruby master Feature#15554] warn/error passing a block to a method which never use a block
by matthewd (Matthew Draper) 20 Apr '24

20 Apr '24

Issue #15554 has been updated by matthewd (Matthew Draper). Eregon (Benoit Daloze) wrote in #note-40: > With the example from @mame, I think adding `(&)` or `(&on_missing)` for `Attribute::Attribute#value` would be good to clarify the value method can receive a block, and purposefully ignores it. > To me it sounds weird that there are method overrides for the same method with "different" signatures. > In fact if it was a positional or keyword argument instead of a block argument, one would already need to add e.g. `def value(arg = nil/*/kw: nil/**)` for `Attribute::Attribute#value`. > So I think it's good to treat a block argument closer to positional or keyword arguments in this aspect. FWIW, as an Active Record maintainer, I agree. The code is currently structured as it is, using the block, specifically because it's aesthetically convenient to "hide" the rarely-needed argument in a place that other overriding methods won't need to know it's present... but I think it's more "clever cheat" than "valuable design that should be preserved, warning-free". byroot (Jean Boussier) wrote in #note-43: > `job_or_instantiate` doesn't accept a block. But for simplicity we forward all the arguments. I think this one is interesting because it's basically a variant of the `super` case. I'd still hope that both `...` and zsuper could be friendly to existing code, _and_ warn on truly-questionable code, by having them note that the block they're implicitly forwarding has already been consumed [by the calling method also using `yield` etc] -- meaning that a non-block-expecting method should _not_ warn/fail on that block argument, even though it would if given an explicitly supplied block [or an implicit forward that has not been consumed by any earlier caller]. ---------------------------------------- Feature #15554: warn/error passing a block to a method which never use a block https://bugs.ruby-lang.org/issues/15554#change-108030 * Author: ko1 (Koichi Sasada) * Status: Closed * 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/… 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-pat… 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#… 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-pat… 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/

1 0

[ruby-core:117620] [Ruby master Bug#20439] Invalid string format with n$ flag is ignored in some case
by tompng (tomoya ishida) 19 Apr '24

19 Apr '24

Issue #20439 has been reported by tompng (tomoya ishida). ---------------------------------------- Bug #20439: Invalid string format with n$ flag is ignored in some case https://bugs.ruby-lang.org/issues/20439 * Author: tompng (tomoya ishida) * Status: Open * ruby -v: ruby 3.3.0 (2023-12-25 revision 5124f9ac75) +YJIT +MN [arm64-darwin22] * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- String format including n$ flag and no type specifier raises ArgumentError ~~~ruby "%1$ " % 1 # invalid format character - % (ArgumentError) "%1$," % 1 # malformed format string - %, (ArgumentError) ~~~ But when it ends with n$ flag, or the next char to n$ flag is `"\n"` or `"\0"`, there is no error. Returns string with n$ flag removed. ~~~ruby "%1$\n" % 1 #=> "%\n" "%1$\0" % 1 #=> "%\u0000" "%1$" % 1 #=> "%" ~~~ -- https://bugs.ruby-lang.org/

1 0

[ruby-core:117615] [Ruby master Feature#6648] Provide a standard API for retrieving all command-line flags passed to Ruby
by Eregon (Benoit Daloze) 19 Apr '24

19 Apr '24

Issue #6648 has been updated by Eregon (Benoit Daloze). I fully agree with the proposal of @Dan0042. This is also needed for MSpec, which currently works around the lack of it by requiring to pass any ruby option through `-T-option` (which is awkward and error-prone), it would be much nicer if we could have `RbConfig.ruby_args`. In fact MSpec is also forced to create an extra process due to the lack of this API (which is a noticeable overhead, even more so on Ruby implementations with a slower startup than CRuby), because that's currently the only way to ensure the main process specs and subprocesses created by specs have the same VM options. We cannot know if the initial process which from the `mspec` executable has the same ruby options as the options passed through `-T` (typically not), hence the extra process. ---------------------------------------- Feature #6648: Provide a standard API for retrieving all command-line flags passed to Ruby https://bugs.ruby-lang.org/issues/6648#change-108024 * Author: headius (Charles Nutter) * Status: Assigned * Assignee: matz (Yukihiro Matsumoto) ---------------------------------------- Currently there are no standard mechanisms to get the flags passed to the currently running Ruby implementation. The available mechanisms are not ideal: * Scanning globals and hoping they have not been tweaked to new settings * Using external wrappers to launch Ruby * ??? Inability to get the full set of command-line flags, including flags passed to the VM itself (and probably VM-specific) makes it impossible to launch subprocess Ruby instances with the same settings. A real world example of this is "((%bundle exec%))" when called with a command line that sets various flags, a la ((%jruby -Xsome.vm.setting --1.9 -S bundle exec%)). None of these flags can propagate to the subprocess, so odd behaviors result. The only option is to put the flags into an env var (((|JRUBY_OPTS|)) or ((|RUBYOPT|))) but this breaks the flow of calling a simple command line. JRuby provides mechanisms to get all its command line options, but they require calling Java APIs from Ruby's API set. Rubinius provides its own API for accessing comand-line options, but I do not know if it includes VM-level flags as well as standard Ruby flags. I know there is a (({RubyVM})) namespace in the 2.0 line. If that namespace is intended to be general-purpose for VM-level features, it would be a good host for this API. Something like... ``` class << RubyVM def vm_args; end # returns array of command line args *not* passed to the target script def script; end # returns the script being executed...though this overlaps with $0 def script_args; end # returns args passed to the script...though this overlaps with ARGV, but that is perhaps warranted since ARGV can be modified (i.e. you probably want the original args) end ``` -- https://bugs.ruby-lang.org/

1 0

[ruby-core:117614] [Ruby master Feature#15554] warn/error passing a block to a method which never use a block
by Eregon (Benoit Daloze) 19 Apr '24

19 Apr '24

Issue #15554 has been updated by Eregon (Benoit Daloze). ko1 (Koichi Sasada) wrote in #note-42: > I made strict mode option for trial (will remove soon) > https://github.com/ruby/ruby/pull/10578 I was thinking about that too, to make the duck typing cases opt-in via env var/CLI arg. It's not ideal because quite hidden but it seems better than not having at all. I think we should keep that strict mode option (or make it default). ---------------------------------------- Feature #15554: warn/error passing a block to a method which never use a block https://bugs.ruby-lang.org/issues/15554#change-108023 * Author: ko1 (Koichi Sasada) * Status: Closed * 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/… 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-pat… 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#… 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-pat… 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/

1 0