Shopify/tapioca

Bad RBI for activerecord when the bundle includes flipper-active_record 1.3.1

Closed this issue · 2 comments

olivier-thatch commented

(Environment: Ruby 3.3.4, Rails 7.1, Sorbet 0.5.11558, Tapioca 0.16.2.)

Our gem bundle includes the flipper and flipper-active_record gems. After upgrading both gems from 1.3.0 to 1.3.1 and regenerating gem RBIs, we ended up with the following at the end of the RBI for activerecord (sorbet/rbi/gems/activerecord@7.1.3.4.rbi):

# source://activerecord//lib/active_record/base.rb#0
class Flipper::Adapters::ActiveRecord::Feature::ActiveRecord_AssociationRelation < ::ActiveRecord::AssociationRelation
  include ::ActiveRecord::Delegation::ClassSpecificRelation
  include ::Flipper::Adapters::ActiveRecord::Model::GeneratedRelationMethods
  include ::Flipper::Adapters::ActiveRecord::Feature::GeneratedRelationMethods
  extend ::ActiveRecord::Delegation::ClassSpecificRelation::ClassMethods
end

# source://activerecord//lib/active_record/base.rb#0
class Flipper::Adapters::ActiveRecord::Feature::ActiveRecord_Associations_CollectionProxy < ::ActiveRecord::Associations::CollectionProxy
  include ::ActiveRecord::Delegation::ClassSpecificRelation
  include ::Flipper::Adapters::ActiveRecord::Model::GeneratedRelationMethods
  include ::Flipper::Adapters::ActiveRecord::Feature::GeneratedRelationMethods
  extend ::ActiveRecord::Delegation::ClassSpecificRelation::ClassMethods
end

# source://activerecord//lib/active_record/base.rb#0
class Flipper::Adapters::ActiveRecord::Feature::ActiveRecord_DisableJoinsAssociationRelation < ::ActiveRecord::DisableJoinsAssociationRelation
  include ::ActiveRecord::Delegation::ClassSpecificRelation
  include ::Flipper::Adapters::ActiveRecord::Model::GeneratedRelationMethods
  include ::Flipper::Adapters::ActiveRecord::Feature::GeneratedRelationMethods
  extend ::ActiveRecord::Delegation::ClassSpecificRelation::ClassMethods
end

# source://activerecord//lib/active_record/base.rb#0
class Flipper::Adapters::ActiveRecord::Feature::ActiveRecord_Relation < ::ActiveRecord::Relation
  include ::ActiveRecord::Delegation::ClassSpecificRelation
  include ::Flipper::Adapters::ActiveRecord::Model::GeneratedRelationMethods
  include ::Flipper::Adapters::ActiveRecord::Feature::GeneratedRelationMethods
  extend ::ActiveRecord::Delegation::ClassSpecificRelation::ClassMethods
end

# source://activerecord//lib/active_record/base.rb#0
module Flipper::Adapters::ActiveRecord::Feature::GeneratedRelationMethods; end

# source://activerecord//lib/active_record/base.rb#0
class Flipper::Adapters::ActiveRecord::Gate::ActiveRecord_AssociationRelation < ::ActiveRecord::AssociationRelation
  include ::ActiveRecord::Delegation::ClassSpecificRelation
  include ::Flipper::Adapters::ActiveRecord::Model::GeneratedRelationMethods
  include ::Flipper::Adapters::ActiveRecord::Gate::GeneratedRelationMethods
  extend ::ActiveRecord::Delegation::ClassSpecificRelation::ClassMethods
end

# source://activerecord//lib/active_record/base.rb#0
class Flipper::Adapters::ActiveRecord::Gate::ActiveRecord_Associations_CollectionProxy < ::ActiveRecord::Associations::CollectionProxy
  include ::ActiveRecord::Delegation::ClassSpecificRelation
  include ::Flipper::Adapters::ActiveRecord::Model::GeneratedRelationMethods
  include ::Flipper::Adapters::ActiveRecord::Gate::GeneratedRelationMethods
  extend ::ActiveRecord::Delegation::ClassSpecificRelation::ClassMethods
end

# source://activerecord//lib/active_record/base.rb#0
class Flipper::Adapters::ActiveRecord::Gate::ActiveRecord_DisableJoinsAssociationRelation < ::ActiveRecord::DisableJoinsAssociationRelation
  include ::ActiveRecord::Delegation::ClassSpecificRelation
  include ::Flipper::Adapters::ActiveRecord::Model::GeneratedRelationMethods
  include ::Flipper::Adapters::ActiveRecord::Gate::GeneratedRelationMethods
  extend ::ActiveRecord::Delegation::ClassSpecificRelation::ClassMethods
end

# source://activerecord//lib/active_record/base.rb#0
class Flipper::Adapters::ActiveRecord::Gate::ActiveRecord_Relation < ::ActiveRecord::Relation
  include ::ActiveRecord::Delegation::ClassSpecificRelation
  include ::Flipper::Adapters::ActiveRecord::Model::GeneratedRelationMethods
  include ::Flipper::Adapters::ActiveRecord::Gate::GeneratedRelationMethods
  extend ::ActiveRecord::Delegation::ClassSpecificRelation::ClassMethods
end

# source://activerecord//lib/active_record/base.rb#0
module Flipper::Adapters::ActiveRecord::Gate::GeneratedRelationMethods; end

# source://activerecord//lib/active_record/base.rb#0
class Flipper::Adapters::ActiveRecord::Model::ActiveRecord_AssociationRelation < ::ActiveRecord::AssociationRelation
  include ::ActiveRecord::Delegation::ClassSpecificRelation
  include ::ActiveRecord::Base::GeneratedRelationMethods
  include ::Flipper::Adapters::ActiveRecord::Model::GeneratedRelationMethods
  extend ::ActiveRecord::Delegation::ClassSpecificRelation::ClassMethods
end

# source://activerecord//lib/active_record/base.rb#0
class Flipper::Adapters::ActiveRecord::Model::ActiveRecord_Associations_CollectionProxy < ::ActiveRecord::Associations::CollectionProxy
  include ::ActiveRecord::Delegation::ClassSpecificRelation
  include ::ActiveRecord::Base::GeneratedRelationMethods
  include ::Flipper::Adapters::ActiveRecord::Model::GeneratedRelationMethods
  extend ::ActiveRecord::Delegation::ClassSpecificRelation::ClassMethods
end

# source://activerecord//lib/active_record/base.rb#0
class Flipper::Adapters::ActiveRecord::Model::ActiveRecord_DisableJoinsAssociationRelation < ::ActiveRecord::DisableJoinsAssociationRelation
  include ::ActiveRecord::Delegation::ClassSpecificRelation
  include ::ActiveRecord::Base::GeneratedRelationMethods
  include ::Flipper::Adapters::ActiveRecord::Model::GeneratedRelationMethods
  extend ::ActiveRecord::Delegation::ClassSpecificRelation::ClassMethods
end

# source://activerecord//lib/active_record/base.rb#0
class Flipper::Adapters::ActiveRecord::Model::ActiveRecord_Relation < ::ActiveRecord::Relation
  include ::ActiveRecord::Delegation::ClassSpecificRelation
  include ::ActiveRecord::Base::GeneratedRelationMethods
  include ::Flipper::Adapters::ActiveRecord::Model::GeneratedRelationMethods
  extend ::ActiveRecord::Delegation::ClassSpecificRelation::ClassMethods
end

# source://activerecord//lib/active_record/base.rb#0
module Flipper::Adapters::ActiveRecord::Model::GeneratedRelationMethods; end

There are two issues here:

  1. These classes are defined by the flipper-active_record gem, so they should be in sorbet/rbi/gems/flipper-active_record@1.3.1.rbi rather than sorbet/rbi/gems/activerecord@7.1.3.4.rbi.
  2. More importantly, the generated RBI is invalid because ::ActiveRecord::Base::GeneratedRelationMethods doesn't exist.

I suspect this issue was caused by this change in Flipper's codebase, but hard to know for sure.

Happy to provide more information if needed, just let me know.

egiurleo commented

I've reproduced this issue. It is a bug/edge case in the mixin tracker logic.

Basically, tapioca's mixin tracker attempts to attribute every mixin (include, prepend, extend) to a specific location. It does this hooking into the mixin method, and then looking at the backtrace and finding the first line that says <top (required)>. This line indicates where the mixin was initiated, and then we can attribute the mixin to whichever gem that line belongs to.

The flipper-active_record gem uses a ActiveSupport.on_load(:active_record) hook to perform some mixins on ActiveRecord, and tapioca's mixin tracker logic is mistakenly attributing those mixins to activerecord instead of flipper-active_record.

Here's the backtrace we get from those mixins: 
["/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/debug-1.9.2/lib/debug/thread_client.rb:415:in `eval'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/debug-1.9.2/lib/debug/thread_client.rb:415:in `block in frame_eval_core'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/debug-1.9.2/lib/debug/thread_client.rb:388:in `block in tp_allow_reentry'",
"<internal:trace_point>:199:in `allow_reentry'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/debug-1.9.2/lib/debug/thread_client.rb:387:in `tp_allow_reentry'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/debug-1.9.2/lib/debug/thread_client.rb:411:in `frame_eval_core'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/debug-1.9.2/lib/debug/thread_client.rb:444:in `frame_eval'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/debug-1.9.2/lib/debug/thread_client.rb:1052:in `wait_next_action_'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/debug-1.9.2/lib/debug/thread_client.rb:875:in `block in wait_next_action'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/debug-1.9.2/lib/debug/thread_client.rb:866:in `block in fiber_blocking'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/debug-1.9.2/lib/debug/thread_client.rb:866:in `blocking'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/debug-1.9.2/lib/debug/thread_client.rb:866:in `fiber_blocking'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/debug-1.9.2/lib/debug/thread_client.rb:875:in `wait_next_action'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/debug-1.9.2/lib/debug/thread_client.rb:320:in `suspend'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/debug-1.9.2/lib/debug/thread_client.rb:251:in `on_breakpoint'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/debug-1.9.2/lib/debug/breakpoint.rb:69:in `suspend'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/debug-1.9.2/lib/debug/breakpoint.rb:170:in `block in setup'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/debug-1.9.2/lib/debug/session.rb:2641:in `debugger'",
"/Users/emilysamp/src/github.com/Shopify/tapioca/lib/tapioca/runtime/trackers/mixin.rb:40:in `register'",
"/Users/emilysamp/src/github.com/Shopify/tapioca/lib/tapioca/runtime/trackers/mixin.rb:106:in `append_features'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/activerecord-7.1.0/lib/active_record/relation/delegation.rb:57:in `include'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/activerecord-7.1.0/lib/active_record/relation/delegation.rb:57:in `include_relation_methods'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/activerecord-7.1.0/lib/active_record/relation/delegation.rb:36:in `block in initialize_relation_delegate_cache'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/activerecord-7.1.0/lib/active_record/relation/delegation.rb:32:in `each'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/activerecord-7.1.0/lib/active_record/relation/delegation.rb:32:in `initialize_relation_delegate_cache'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/activerecord-7.1.0/lib/active_record/relation/delegation.rb:46:in `inherited'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/activerecord-7.1.0/lib/active_record/core.rb:367:in `inherited'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/activerecord-7.1.0/lib/active_record/persistence.rb:629:in `inherited'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/activerecord-7.1.0/lib/active_record/model_schema.rb:599:in `inherited'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/activerecord-7.1.0/lib/active_record/inheritance.rb:286:in `inherited'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/activemodel-7.1.0/lib/active_model/validations.rb:292:in `inherited'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/activerecord-7.1.0/lib/active_record/locking/optimistic.rb:194:in `inherited'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/activemodel-7.1.0/lib/active_model/attribute_methods.rb:381:in `inherited'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/activerecord-7.1.0/lib/active_record/attribute_methods.rb:261:in `inherited'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/activerecord-7.1.0/lib/active_record/attribute_methods/primary_key.rb:181:in `inherited'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/activerecord-7.1.0/lib/active_record/reflection.rb:136:in `inherited'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/flipper-active_record-1.3.1/lib/flipper/adapters/active_record.rb:13:in `block in <class:ActiveRecord>'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/activesupport-7.1.0/lib/active_support/lazy_load_hooks.rb:97:in `class_eval'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/activesupport-7.1.0/lib/active_support/lazy_load_hooks.rb:97:in `block in execute_hook'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/activesupport-7.1.0/lib/active_support/lazy_load_hooks.rb:87:in `with_execution_control'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/activesupport-7.1.0/lib/active_support/lazy_load_hooks.rb:92:in `execute_hook'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/activesupport-7.1.0/lib/active_support/lazy_load_hooks.rb:78:in `block in run_load_hooks'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/activesupport-7.1.0/lib/active_support/lazy_load_hooks.rb:77:in `each'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/activesupport-7.1.0/lib/active_support/lazy_load_hooks.rb:77:in `run_load_hooks'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/activerecord-7.1.0/lib/active_record/base.rb:338:in `<module:ActiveRecord>'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/activerecord-7.1.0/lib/active_record/base.rb:15:in `<top (required)>'",
"/Users/emilysamp/.rubies/ruby-3.3.5/lib/ruby/3.3.0/bundled_gems.rb:75:in `require'",
"/Users/emilysamp/.rubies/ruby-3.3.5/lib/ruby/3.3.0/bundled_gems.rb:75:in `block (2 levels) in replace_require'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/zeitwerk-2.6.18/lib/zeitwerk/kernel.rb:34:in `require'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/railties-7.1.0/lib/rails/info.rb:98:in `block in <module:Info>'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/railties-7.1.0/lib/rails/info.rb:26:in `property'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/railties-7.1.0/lib/rails/info.rb:97:in `<module:Info>'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/railties-7.1.0/lib/rails/info.rb:9:in `<module:Rails>'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/railties-7.1.0/lib/rails/info.rb:5:in `<top (required)>'",
"/Users/emilysamp/.rubies/ruby-3.3.5/lib/ruby/3.3.0/bundled_gems.rb:75:in `require'",
"/Users/emilysamp/.rubies/ruby-3.3.5/lib/ruby/3.3.0/bundled_gems.rb:75:in `block (2 levels) in replace_require'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/zeitwerk-2.6.18/lib/zeitwerk/kernel.rb:34:in `require'",
"/Users/emilysamp/src/github.com/Shopify/tapioca/lib/tapioca/runtime/reflection.rb:50:in `const_get'",
"/Users/emilysamp/src/github.com/Shopify/tapioca/lib/tapioca/runtime/reflection.rb:50:in `constantize'",
"/Users/emilysamp/src/github.com/Shopify/tapioca/lib/tapioca/runtime/trackers/autoload.rb:25:in `block in eager_load_all!'",
"/Users/emilysamp/src/github.com/Shopify/tapioca/lib/tapioca/runtime/trackers/autoload.rb:50:in `with_disabled_exits'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/sorbet-runtime-0.5.11558/lib/types/private/methods/_methods.rb:279:in `bind_call'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/sorbet-runtime-0.5.11558/lib/types/private/methods/_methods.rb:279:in `block in _on_method_added'",
"/Users/emilysamp/src/github.com/Shopify/tapioca/lib/tapioca/runtime/trackers/autoload.rb:20:in `eager_load_all!'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/sorbet-runtime-0.5.11558/lib/types/private/methods/_methods.rb:279:in `bind_call'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/sorbet-runtime-0.5.11558/lib/types/private/methods/_methods.rb:279:in `block in _on_method_added'",
"/Users/emilysamp/src/github.com/Shopify/tapioca/lib/tapioca/loaders/gem.rb:69:in `require_gem_file'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/sorbet-runtime-0.5.11558/lib/types/private/methods/_methods.rb:279:in `bind_call'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/sorbet-runtime-0.5.11558/lib/types/private/methods/_methods.rb:279:in `block in _on_method_added'",
"/Users/emilysamp/src/github.com/Shopify/tapioca/lib/tapioca/loaders/gem.rb:35:in `load'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/sorbet-runtime-0.5.11558/lib/types/private/methods/_methods.rb:279:in `bind_call'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/sorbet-runtime-0.5.11558/lib/types/private/methods/_methods.rb:279:in `block in _on_method_added'",
"/Users/emilysamp/src/github.com/Shopify/tapioca/lib/tapioca/loaders/gem.rb:29:in `load_application'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/sorbet-runtime-0.5.11558/lib/types/private/methods/_methods.rb:279:in `bind_call'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/sorbet-runtime-0.5.11558/lib/types/private/methods/_methods.rb:279:in `block in _on_method_added'",
"/Users/emilysamp/src/github.com/Shopify/tapioca/lib/tapioca/commands/gem_generate.rb:11:in `execute'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/sorbet-runtime-0.5.11558/lib/types/private/methods/_methods.rb:279:in `bind_call'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/sorbet-runtime-0.5.11558/lib/types/private/methods/_methods.rb:279:in `block in _on_method_added'",
"/Users/emilysamp/src/github.com/Shopify/tapioca/lib/tapioca/commands/command.rb:27:in `block in run'",
"/Users/emilysamp/src/github.com/Shopify/tapioca/lib/tapioca.rb:24:in `block in silence_warnings'",
"/Users/emilysamp/.rubies/ruby-3.3.5/lib/ruby/3.3.0/rubygems/user_interaction.rb:46:in `use_ui'",
"/Users/emilysamp/src/github.com/Shopify/tapioca/lib/tapioca.rb:23:in `silence_warnings'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/sorbet-runtime-0.5.11558/lib/types/private/methods/_methods.rb:279:in `bind_call'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/sorbet-runtime-0.5.11558/lib/types/private/methods/_methods.rb:279:in `block in _on_method_added'",
"/Users/emilysamp/src/github.com/Shopify/tapioca/lib/tapioca/commands/command.rb:26:in `run'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/sorbet-runtime-0.5.11558/lib/types/private/methods/_methods.rb:279:in `bind_call'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/sorbet-runtime-0.5.11558/lib/types/private/methods/_methods.rb:279:in `block in _on_method_added'",
"/Users/emilysamp/src/github.com/Shopify/tapioca/lib/tapioca/cli.rb:308:in `gem'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/thor-1.3.2/lib/thor/command.rb:28:in `run'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/thor-1.3.2/lib/thor/invocation.rb:127:in `invoke_command'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/thor-1.3.2/lib/thor.rb:538:in `dispatch'",
"/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/thor-1.3.2/lib/thor/base.rb:584:in `start'",
"/Users/emilysamp/src/github.com/Shopify/tapioca/exe/tapioca:25:in `<top (required)>'",
"bin/tapioca:27:in `load'",
"bin/tapioca:27:in `<main>'"]

The line that says <top (required)> is from activerecord, hence why tapioca is attributing this RBI to activerecord. The only line from the flipper gem looks like this:

 "/Users/emilysamp/.gem/ruby/ruby-3.3.5/gems/flipper-active_record-1.3.1/lib/flipper/adapters/active_record.rb:13:in `block in <class:ActiveRecord>'"

I've also reproduced this in a test.

I don't think we have the bandwidth to work on this immediately. @olivier-thatch if you want to work on it, please go ahead, otherwise we will address it once some higher-priority work on our team has wrapped up.

olivier-thatch commented

Thanks for looking into this @egiurleo. I probably won't be able to work on this, but might take a stab at it if I can find the bandwidth. (Also a good opportunity to learn more about Tapioca's internals.)

As a rough workaround, we can just delete the incorrect lines from activerecord@7.1.3.4.rbi and not regenerate the RBI for activerecord.

  • 👍1