From 324bc60c88c4cceba1f8a17ac7e990899456c2dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Lara?= Date: Tue, 19 Aug 2025 18:15:54 -0400 Subject: [PATCH] fix: warns suppressed even when allowed Using `warnings_treated_as_deprecation` caused `warn` messages to be suppressed even if the message was explicitly allowed. --- CHANGELOG.md | 2 ++ lib/deprecation_toolkit.rb | 1 + .../deprecation_allowed.rb | 26 +++++++++++++++++++ .../deprecation_subscriber.rb | 14 +--------- lib/deprecation_toolkit/warning.rb | 8 +++--- test/deprecation_toolkit/warning_test.rb | 16 ++++++++++++ 6 files changed, 51 insertions(+), 16 deletions(-) create mode 100644 lib/deprecation_toolkit/deprecation_allowed.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ccbd52..b56dec8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## main (unreleased) +* [#140](https://github.com/Shopify/deprecation_toolkit/pull/140) Fixed a bug where `warnings_treated_as_deprecation` would suppress warnings even if they were explicitly allowed. + ## 2.2.4 (2025-08-08) * [#136](https://github.com/Shopify/deprecation_toolkit/pull/136) Stable write logic. diff --git a/lib/deprecation_toolkit.rb b/lib/deprecation_toolkit.rb index 38ac910..410f355 100644 --- a/lib/deprecation_toolkit.rb +++ b/lib/deprecation_toolkit.rb @@ -10,6 +10,7 @@ module DeprecationToolkit autoload :Collector, "deprecation_toolkit/collector" autoload :ReadWriteHelper, "deprecation_toolkit/read_write_helper" autoload :TestTriggerer, "deprecation_toolkit/test_triggerer" + autoload :DeprecationAllowed, "deprecation_toolkit/deprecation_allowed" module Behaviors autoload :Disabled, "deprecation_toolkit/behaviors/disabled" diff --git a/lib/deprecation_toolkit/deprecation_allowed.rb b/lib/deprecation_toolkit/deprecation_allowed.rb new file mode 100644 index 0000000..c0da7e6 --- /dev/null +++ b/lib/deprecation_toolkit/deprecation_allowed.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module DeprecationToolkit + class DeprecationAllowed + class << self + # Checks if a deprecation is allowed by the configured rules. + # + # A rule can be a `Regexp` to match the deprecation message, or a callable object that will receive the + # message and the callstack to perform a more advanced check. + # + # @param payload [Hash] The payload from the deprecation event. + # @option payload [String] :message The deprecation message. + # @option payload [Array] :callstack The callstack for the deprecation. + # @return [Boolean] `true` if the deprecation is allowed, `false` otherwise. + def call(payload) + Configuration.allowed_deprecations.any? do |rule| + if rule.is_a?(Regexp) + rule.match?(payload[:message]) + else + rule.call(payload[:message], payload[:callstack]) + end + end + end + end + end +end diff --git a/lib/deprecation_toolkit/deprecation_subscriber.rb b/lib/deprecation_toolkit/deprecation_subscriber.rb index da50ba9..4454a0a 100644 --- a/lib/deprecation_toolkit/deprecation_subscriber.rb +++ b/lib/deprecation_toolkit/deprecation_subscriber.rb @@ -59,19 +59,7 @@ def attached_subscriber?(subscriber, gem_name) def deprecation(event) message = event.payload[:message] - Collector.collect(message) unless deprecation_allowed?(event.payload) - end - - private - - def deprecation_allowed?(payload) - Configuration.allowed_deprecations.any? do |rule| - if rule.is_a?(Regexp) - rule.match?(payload[:message]) - else - rule.call(payload[:message], payload[:callstack]) - end - end + Collector.collect(message) unless DeprecationToolkit::DeprecationAllowed.call(event.payload) end end end diff --git a/lib/deprecation_toolkit/warning.rb b/lib/deprecation_toolkit/warning.rb index ec98cc3..cc13883 100644 --- a/lib/deprecation_toolkit/warning.rb +++ b/lib/deprecation_toolkit/warning.rb @@ -37,8 +37,9 @@ def handle_multipart(str) str end - def deprecation_triggered?(str) - DeprecationToolkit::Configuration.warnings_treated_as_deprecation.any? { |warning| warning === str } + def deprecation_triggered?(str, callstack: []) + DeprecationToolkit::Configuration.warnings_treated_as_deprecation.any? { |warning| warning === str } && + !DeprecationToolkit::DeprecationAllowed.call({ message: str, callstack: callstack }) end def deprecator @@ -51,6 +52,7 @@ def deprecator module WarningPatch def warn(str, *) + callstack = caller[0,20] if Configuration.warnings_treated_as_deprecation.empty? return super end @@ -58,7 +60,7 @@ def warn(str, *) str = DeprecationToolkit::Warning.handle_multipart(str) return unless str - if DeprecationToolkit::Warning.deprecation_triggered?(str) + if DeprecationToolkit::Warning.deprecation_triggered?(str, callstack: callstack) DeprecationToolkit::Warning.deprecator.warn(str) else super diff --git a/test/deprecation_toolkit/warning_test.rb b/test/deprecation_toolkit/warning_test.rb index 8daed46..5e42345 100644 --- a/test/deprecation_toolkit/warning_test.rb +++ b/test/deprecation_toolkit/warning_test.rb @@ -6,10 +6,12 @@ module DeprecationToolkit class WarningTest < ActiveSupport::TestCase setup do @previous_warnings_treated_as_deprecation = Configuration.warnings_treated_as_deprecation + @previous_allowed_deprecations = Configuration.allowed_deprecations end teardown do Configuration.warnings_treated_as_deprecation = @previous_warnings_treated_as_deprecation + Configuration.allowed_deprecations = @previous_allowed_deprecations end test "treats warnings as deprecations" do @@ -80,5 +82,19 @@ class WarningTest < ActiveSupport::TestCase assert_match(/Using the last argument as keyword parameters/, error.message) assert_match(/The called method/, error.message) end + + test "using warnings_treated_as_deprecation do not supress warn messages if the deprecation is allowed" do + Configuration.allowed_deprecations = [/#example is deprecated/] + Configuration.warnings_treated_as_deprecation = [//] + + captured_io = capture_io do + assert_nothing_raised do + warn("#example is deprecated") + trigger_deprecation_toolkit_behavior + end + end + line_msg = captured_io.find { |line| line.include?("#example is deprecated") } + assert_equal("#example is deprecated\n", line_msg) + end end end