tapioca icon indicating copy to clipboard operation
tapioca copied to clipboard

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

Open olivier-thatch opened this issue 1 year ago • 2 comments

(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/[email protected]):

# 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/[email protected] rather than sorbet/rbi/gems/[email protected].
  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.

olivier-thatch avatar Sep 09 '24 21:09 olivier-thatch

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.

egiurleo avatar Sep 26 '24 14:09 egiurleo

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 [email protected] and not regenerate the RBI for activerecord.

olivier-thatch avatar Sep 26 '24 15:09 olivier-thatch