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/l... 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-patc... 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#L... 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-patc... 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/