Skip to content

Rescue ActiveSupport::DeprecationException to prevent crashes #2386

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Rescue ActiveSupport::DeprecationException to prevent crashes #2386

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions lib/tapioca/dsl/compilers/active_support_concern.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,14 @@ class << self
#: -> T::Enumerable[Module]
def gather_constants
all_modules.select do |mod|
name_of(mod) && # i.e. not anonymous
!mod.singleton_class? &&
ActiveSupport::Concern > mod.singleton_class &&
has_dependencies?(mod)
begin
name_of(mod) && # i.e. not anonymous
!mod.singleton_class? &&
ActiveSupport::Concern > mod.singleton_class &&
has_dependencies?(mod)
rescue ActiveSupport::DeprecationException
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is the right way to go 🙁 . Also, this could theoretically happen with any DSL compiler, and we'd need rescue calls in each one.

I took a look at alternatives, and unfortunately you can't use the --skip-constant option because those constants are still loaded during the gathering phase. I think the only other viable solutions are:

  1. Not loading the deprecated constant in the first place from your application. If it's not in ObjectSpace after app boot, it won't be accessed by Tapioca's DSL compilers.
  2. Don't raise deprecation warnings while Tapioca is running. You could detect this by looking at Kernel#caller or through an ENV variable set while running Tapioca.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, this isn't the right fix.

We already have handling for DeprecatedConstantProxy classes in the gem pipeline, we could implement something similar in the DSL pipeline as well: https://github.com/Shopify/tapioca/blob/main/lib/tapioca/gem/pipeline.rb#L512-L524

false
end
end
end

Expand Down
8 changes: 7 additions & 1 deletion lib/tapioca/dsl/compilers/mixed_in_class_attributes.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,13 @@ class << self
def gather_constants
# Select all non-anonymous modules that have overridden Module.included
all_modules.select do |mod|
!mod.is_a?(Class) && name_of(mod) && Runtime::Reflection.method_of(mod, :included).owner != Module
begin
!mod.is_a?(Class) &&
name_of(mod) &&
Runtime::Reflection.method_of(mod, :included).owner != Module
rescue ActiveSupport::DeprecationException
false
end
end
end
end
Expand Down
28 changes: 16 additions & 12 deletions lib/tapioca/dsl/compilers/url_helpers.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,18 +114,22 @@ def gather_constants
Object.const_set(:GeneratedPathHelpersModule, path_helpers_module)

constants = all_modules.select do |mod|
next unless name_of(mod)

# Fast-path to quickly disqualify most cases
next false unless url_helpers_module > mod || # rubocop:disable Style/InvertibleUnlessCondition
path_helpers_module > mod ||
url_helpers_module > mod.singleton_class ||
path_helpers_module > mod.singleton_class

includes_helper?(mod, url_helpers_module) ||
includes_helper?(mod, path_helpers_module) ||
includes_helper?(mod.singleton_class, url_helpers_module) ||
includes_helper?(mod.singleton_class, path_helpers_module)
begin
next unless name_of(mod)

# Fast-path to quickly disqualify most cases
next false unless url_helpers_module > mod || # rubocop:disable Style/InvertibleUnlessCondition
path_helpers_module > mod ||
url_helpers_module > mod.singleton_class ||
path_helpers_module > mod.singleton_class

includes_helper?(mod, url_helpers_module) ||
includes_helper?(mod, path_helpers_module) ||
includes_helper?(mod.singleton_class, url_helpers_module) ||
includes_helper?(mod.singleton_class, path_helpers_module)
rescue ActiveSupport::DeprecationException
next false
end
end

constants.concat(NON_DISCOVERABLE_INCLUDERS).push(GeneratedUrlHelpersModule, GeneratedPathHelpersModule)
Expand Down
39 changes: 39 additions & 0 deletions spec/tapioca/dsl/compilers/active_support_concern_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,45 @@ def before_setup
end

describe "gather_constants" do
it "ignores modules that raise ActiveSupport::DeprecationException" do
add_ruby_file("deprecated_case.rb", <<~RUBY)
module TestCase
module Foo
extend ActiveSupport::Concern
end

# This module will simulate a deprecated constant that raises when inspected
module DeprecatedMod
end

module Bar
extend ActiveSupport::Concern
include Foo
end
end
RUBY

# Stub name_of to raise for our specific deprecated module and verify no crash
compiler = Tapioca::Dsl::Compilers::ActiveSupportConcern
original = Tapioca::Dsl::Compiler.method(:name_of)
begin
def compiler.name_of(mod)
if mod == TestCase::DeprecatedMod
raise ActiveSupport::DeprecationException
else
Tapioca::Dsl::Compiler.method(:name_of).call(mod)
end
end

# Presence of DeprecatedMod should not cause crashes; Bar is the only gatherable constant
assert_equal(["TestCase::Bar"], gathered_constants_in_namespace(:TestCase))
ensure
# Restore original implementation
def compiler.name_of(mod)
original.call(mod)
end
end
end
it "does not gather anonymous constants" do
add_ruby_file("test_case.rb", <<~RUBY)
module TestCase
Expand Down
34 changes: 34 additions & 0 deletions spec/tapioca/dsl/compilers/mixed_in_class_attributes_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,40 @@ def before_setup
end

describe "gather_constants" do
it "ignores modules that raise ActiveSupport::DeprecationException" do
add_ruby_file("file.rb", <<~RUBY)
module ManualIncluded
def self.included(base); end
end

module Concern
extend ActiveSupport::Concern
end

module DeprecatedMod
end
RUBY

compiler = Tapioca::Dsl::Compilers::MixedInClassAttributes
original = Tapioca::Dsl::Compiler.method(:name_of)
begin
def compiler.name_of(mod)
if mod == DeprecatedMod
raise ActiveSupport::DeprecationException
else
Tapioca::Dsl::Compiler.method(:name_of).call(mod)
end
end

assert_includes(gathered_constants, "ManualIncluded")
assert_includes(gathered_constants, "Concern")
refute_includes(gathered_constants, "DeprecatedMod")
ensure
def compiler.name_of(mod)
original.call(mod)
end
end
end
it "gathers modules that respond to class_attribute" do
add_ruby_file("file.rb", <<~RUBY)
module ManualIncluded
Expand Down
33 changes: 33 additions & 0 deletions spec/tapioca/dsl/compilers/url_helpers_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,39 @@ def before_setup
end

describe "initialize" do
it "ignores modules that raise ActiveSupport::DeprecationException" do
add_ruby_file("content.rb", <<~RUBY)
class Application < Rails::Application
end

module DeprecatedMod
end
RUBY

compiler = Tapioca::Dsl::Compilers::UrlHelpers
original = Tapioca::Dsl::Compiler.method(:name_of)
begin
def compiler.name_of(mod)
if mod == DeprecatedMod
raise ActiveSupport::DeprecationException
else
Tapioca::Dsl::Compiler.method(:name_of).call(mod)
end
end

assert_equal(
[
"GeneratedPathHelpersModule",
"GeneratedUrlHelpersModule",
],
gathered_constants,
)
ensure
def compiler.name_of(mod)
original.call(mod)
end
end
end
it "does not gather constants when url_helpers is not included" do
add_ruby_file("content.rb", <<~RUBY)
class Application < Rails::Application
Expand Down