Skip to content

Ruby 3: Deprecations are not caught in irb and not by DeprecationToolkit if the method is stubbed

Copied from slack:

There are 2 loopholes due to which kwargs deprecations might go unnoticed:

  • The author has stubbed this method in tests. We run automated detection for this warning in tests via deprecation_toolkit, but it relies on the fact that Kernel#warn emits a warning. Stubbing out the implementation removes that warning, and we never pick it up, so the build is green.
  • Testing in irb/rails c silences the deprecation warning, since irb in Ruby 2.7.x has a bug that prevents deprecation warnings from showing: https://bugs.ruby-lang.org/issues/17377 For now we just need to be extra vigilant. All occurrences of f({k: v}) should raise your eyebrows. This is only allowed in Ruby 3 if f actually takes a Hash, not keyword arguments.

More context: https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/2461#note_967340801 [internal only]

https://docs.gitlab.com/ee/development/ruby3_gotchas.html


The (interesting parts of the) discussion from the internal MR:

Stubbed method in tests:

Yes I had to go back and understand how DeprecationToolkit even works; it patches Warning (which is the impl of Kernel#warn and used to issue deprecation warnings by the VM): https://github.com/Shopify/deprecation_toolkit/blob/main/lib/deprecation_toolkit/warning.rb#L46-L60

These deprecations warnings are issued by the VM when it executes the iseqs representing the method call:

            else if (args_pop_keyword_hash(args, &keyword_hash, 1)) {
                /* Warn the following:
                 * def foo(k:1) p [k]; end
                 * foo({k:42}) #=> 42
                 */
                rb_warn_last_hash_to_keyword(ec, calling, ci, iseq);
                given_argc--;
            }

https://github.com/ruby/ruby/blob/v2_7_5/vm_args.c#L934-L941

So stubbing out this call will effectively remove the call to warn, so DT never sees it.

Testing in irb/rails c:

This might be a bug in irb: https://bugs.ruby-lang.org/issues/17377; even with -W2 it does not print deprecation warnings.

On the CLI, I see it:

$ ruby -W2 -e 'def f(a:); pp a; end; f({a: 42})'
-e:1: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
-e:1: warning: The called method `f' is defined here
42

As indicated in the PR for that bug report, you can see it like so:

[12:49:37] work/gl-gck::master ✔ irb -W2                                   
irb(main):001:0> Warning[:deprecated] = true
=> true
irb(main):002:0> def f(a:); pp a; end
=> :f
irb(main):003:0> f({a: 42})
(irb):3: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
(irb):2: warning: The called method `f' is defined here
42
=> 42

The problem:

It might be difficult to detect, since even a Cop will likely not suffice (it can only reason about syntax, but this issue can take many forms that are harder to catch than passing a hash literal)

The problem is that QA runs do not provide sufficient code coverage, so we cannot even fully rely on the 2-hour Ruby 3 pipeline to catch these issues.

cc @mkaeppler @alipniagov

Edited by Roy Zwambag