Filter out stack trace from Gem::Deprecate deprecation messages

[davidstosik]

Context

Gem::Deprecate

Gem::Deprecate is a Ruby module that

Provides 3 methods for declaring when something is going away.

Similarly to ActiveSupport::Deprecation, it generates warning messages, but with a different format:

class Foo
  extend Gem::Deprecate
  def old_method; end
  deprecate :old_method, :new_method, 2019, 1
end

Foo.new.old_method

# Produces this warning:
# NOTE: Foo#old_method is deprecated; use new_method instead. It will be removed on or after 2019-01.
# Foo#old_method called from (irb):7.

Removing stack traces

deprecation_toolkit strips out stack traces from deprecation warnings when recording them:

https://github.com/Shopify/deprecation_toolkit/blob/ee55e609e8b8f943d206b780e1a45d1347c40974/lib/deprecation_toolkit/behaviors/record.rb#L11

https://github.com/Shopify/deprecation_toolkit/blob/ee55e609e8b8f943d206b780e1a45d1347c40974/lib/deprecation_toolkit/collector.rb#L36-L44

There are multiple reasons, but one of them is that we don't want the tool to detect a deprecation mismatch if the file path or line number changes.

"Generic" warnings get converted to ActiveSupport::Deprecation warnings

The gem patches the warn method to record generic warnings as deprecations:

https://github.com/Shopify/deprecation_toolkit/blob/ee55e609e8b8f943d206b780e1a45d1347c40974/lib/deprecation_toolkit/warning.rb#L53

Problem

As a consequence of the above, Gem::Deprecate deprecations trigger a warning that includes the stack trace twice:

DEPRECATION WARNING: NOTE: Stripe::Account#save is deprecated; use update instead. It will be removed on or after 2022-11.
Stripe::Account#save called from path/to/file.rb:521.
 (called from update_stripe_account at path/to/file.rb:521)

And because #deprecations_without_stacktrace's logic is based on ActiveSupport::Deprecation's format, the gem only strips one stack trace, and keeps the other when recording deprecations:

# test/deprecations/my_test.yml
---
test_do_something:
- |
  DEPRECATION WARNING: NOTE: Stripe::Account#save is deprecated; use update instead. It will be removed on or after 2022-11.
  Stripe::Account#save called from path/to/file.rb:521.

This goes against the idea that we'd like to filter out file paths and line numbers from recorded deprecations.

Solution

The solution I propose is just one more substitution based on a regular expression. Just as we're currently stripping out the stack trace added by ActiveSupport::Deprecation using String#sub and a regular expression, I propose we add another call to #sub with a different regular expression.

As Gem::Deprecate ships with Ruby, it would make sense to me that deprecation_toolkit be able to handle such deprecations out of the box.

Shortcomings

I'm not super happy with the idea of stacking yet another regex substitution on top of what we already have. I'd have prefered if the gem was able to recognize Gem::Deprecate warnings when they're generated, so that we're not wrapping them in ActiveSupport::Deprecation.

Now I think about it, I would even consider a refactor of the gem so that it does not have to depend on active_support: the gem could work the same based on Gem::Deprecate, and supporting ActiveSupport::Deprecation (or other types of deprecations) should be as simple as implementing a plug-in for them.

[shioyama]

A bit of a digression, but I wonder if Ruby could supply a specific method or hook to declare a deprecation so everything wouldn't have to patch Kernel#warn. Any fix like this is going to be fairly brittle.

[eileencodes]

A bit of a digression, but I wonder if Ruby could supply a specific method or hook to declare a deprecation so everything wouldn't have to patch Kernel#warn. Any fix like this is going to be fairly brittle.

Something like this got brought up when I wrote the warnings category feature. I'm not sure if opinions have changed but more structured deprecations have been rejected before.

Deprecations should be tagged as such though with the category, the point was to avoid regex-ing.