Merge pull request #1010 from Shopify/emily/mixin-attribution

Properly attribute mixin locations

lib/tapioca/gem/listeners/foreign_constants.rb

@@ -24,9 +24,8 @@ def on_scope(event)
           # The way we identify these "foreign constants" is by asking the mixin tracker which
           # constants have mixed in the current module that we are handling. We add all the
           # constants that we discover to the pipeline to be processed.
-          Runtime::Trackers::Mixin.constants_with_mixin(mixin).each do |constant, locations|
-            next if defined_by_application?(constant)
-            next unless mixed_in_by_gem?(locations)
+          Runtime::Trackers::Mixin.constants_with_mixin(mixin).each do |constant, location|
+            next unless mixed_in_by_gem?(location)
 
             name = @pipeline.name_of(constant)
 
@@ -47,28 +46,11 @@ def on_scope(event)
 
         sig do
           params(
-            locations: T::Array[String]
+            location: String,
           ).returns(T::Boolean)
         end
-        def mixed_in_by_gem?(locations)
-          locations.compact.any? { |location| @pipeline.gem.contains_path?(location) }
-        end
-
-        sig do
-          params(
-            constant: Module
-          ).returns(T::Boolean)
-        end
-        def defined_by_application?(constant)
-          application_dir = (Bundler.default_gemfile / "..").to_s
-          Tapioca::Runtime::Trackers::ConstantDefinition.files_for(constant).any? do |location|
-            location.start_with?(application_dir) && !in_bundle_path?(location)
-          end
-        end
-
-        sig { params(path: String).returns(T::Boolean) }
-        def in_bundle_path?(path)
-          path.start_with?(Bundler.bundle_path.to_s, Bundler.app_cache.to_s)
+        def mixed_in_by_gem?(location)
+          @pipeline.gem.contains_path?(location)
         end
 
         sig { params(constant: Module).returns(T.nilable(String)) }

lib/tapioca/runtime/reflection.rb

@@ -20,6 +20,8 @@ module Reflection
       PRIVATE_INSTANCE_METHODS_METHOD = T.let(Module.instance_method(:private_instance_methods), UnboundMethod)
       METHOD_METHOD = T.let(Kernel.instance_method(:method), UnboundMethod)
 
+      REQUIRED_FROM_LABELS = T.let(["<top (required)>", "<main>"].freeze, T::Array[String])
+
       sig do
         params(
           symbol: String,
@@ -152,6 +154,20 @@ def descendants_of(klass)
 
         T.unsafe(result)
       end
+
+      # Examines the call stack to identify the closest location where a "require" is performed
+      # by searching for the label "<top (required)>". If none is found, it returns the location
+      # labeled "<main>", which is the original call site.
+      sig { returns(String) }
+      def required_from_location
+        locations = Kernel.caller_locations
+        return "" unless locations
+
+        required_location = locations.find { |loc| REQUIRED_FROM_LABELS.include?(loc.label) }
+        return "" unless required_location
+
+        required_location.absolute_path || ""
+      end
     end
   end
 end

lib/tapioca/runtime/trackers/mixin.rb

@@ -23,21 +23,19 @@ class Type < T::Enum
             constant: Module,
             mixin: Module,
             mixin_type: Type,
-            locations: T.nilable(T::Array[Thread::Backtrace::Location])
           ).void
         end
-        def self.register(constant, mixin, mixin_type, locations)
-          locations ||= []
-          locations.map!(&:absolute_path).uniq!
+        def self.register(constant, mixin, mixin_type)
+          location = Reflection.required_from_location
 
           locs = mixin_locations_for(constant)
-          locs.fetch(mixin_type).store(mixin, T.cast(locations, T::Array[String]))
+          locs.fetch(mixin_type).store(mixin, location)
 
           constants = constants_with_mixin(mixin)
-          constants[constant] = T.cast(locations, T::Array[String])
+          constants[constant] = location
         end
 
-        sig { params(constant: Module).returns(T::Hash[Type, T::Hash[Module, T::Array[String]]]) }
+        sig { params(constant: Module).returns(T::Hash[Type, T::Hash[Module, String]]) }
         def self.mixin_locations_for(constant)
           @constants_to_mixin_locations[constant] ||= {
             Type::Prepend => {}.compare_by_identity,
@@ -46,7 +44,7 @@ def self.mixin_locations_for(constant)
           }
         end
 
-        sig { params(mixin: Module).returns(T::Hash[Module, T::Array[String]]) }
+        sig { params(mixin: Module).returns(T::Hash[Module, String]) }
         def self.constants_with_mixin(mixin)
           @mixins_to_constants[mixin] ||= {}.compare_by_identity
         end
@@ -62,7 +60,6 @@ def prepend_features(constant)
         constant,
         self,
         Tapioca::Runtime::Trackers::Mixin::Type::Prepend,
-        caller_locations
       )
       super
     end
@@ -72,7 +69,6 @@ def append_features(constant)
         constant,
         self,
         Tapioca::Runtime::Trackers::Mixin::Type::Include,
-        caller_locations
       )
       super
     end
@@ -82,7 +78,6 @@ def extend_object(obj)
         obj,
         self,
         Tapioca::Runtime::Trackers::Mixin::Type::Extend,
-        caller_locations
       ) if Module === obj
       super
     end

spec/tapioca/cli/gem_spec.rb

@@ -1116,6 +1116,92 @@ module TypedParameters; end
           RBI
         end
 
+        it "must do mixin attribution properly when include occurs in other gem" do
+          some_engine = mock_gem("some_engine", "0.0.2") do
+            write("lib/some_engine.rb", <<~RUBY)
+              require "action_controller"
+
+              module SomeEngine
+                class SomeController < ActionController::Base
+                  # This method triggers a dynamic mixin which should be attributed to this gem
+                  # and not actionpack, even though the real `include` happens inside actionpack
+                  helper_method :foo
+                end
+              end
+            RUBY
+          end
+
+          @project.require_real_gem("actionpack", "6.1.4.4")
+          @project.require_mock_gem(some_engine)
+          @project.bundle_install
+
+          response = @project.tapioca("gem actionpack some_engine")
+
+          assert_includes(response.out, "Compiled actionpack")
+          assert_includes(response.out, "Compiled some_engine")
+
+          actionpack_rbi = @project.read("sorbet/rbi/gems/actionpack@6.1.4.4.rbi")
+          # actionpack RBI should have nothing in it about `SomeEngine`
+          refute_includes(actionpack_rbi, "SomeEngine")
+
+          expected = template(<<~RBI)
+            # typed: true
+
+            # DO NOT EDIT MANUALLY
+            # This is an autogenerated file for types exported from the `some_engine` gem.
+            # Please instead update this file by running `bin/tapioca gem some_engine`.
+
+            module ActionController::Base::HelperMethods
+            <% if ruby_version(">= 3.1") %>
+              def alert(*args, **_arg1, &block); end
+              def combined_fragment_cache_key(*args, **_arg1, &block); end
+              def content_security_policy?(*args, **_arg1, &block); end
+              def content_security_policy_nonce(*args, **_arg1, &block); end
+              def cookies(*args, **_arg1, &block); end
+              def form_authenticity_token(*args, **_arg1, &block); end
+              def notice(*args, **_arg1, &block); end
+              def protect_against_forgery?(*args, **_arg1, &block); end
+              def view_cache_dependencies(*args, **_arg1, &block); end
+            <% else %>
+              def alert(*args, &block); end
+              def combined_fragment_cache_key(*args, &block); end
+              def content_security_policy?(*args, &block); end
+              def content_security_policy_nonce(*args, &block); end
+              def cookies(*args, &block); end
+              def form_authenticity_token(*args, &block); end
+              def notice(*args, &block); end
+              def protect_against_forgery?(*args, &block); end
+              def view_cache_dependencies(*args, &block); end
+            <% end %>
+            end
+
+            module SomeEngine; end
+
+            class SomeEngine::SomeController < ::ActionController::Base
+              private
+
+              def _layout(lookup_context, formats); end
+
+              class << self
+                def _helper_methods; end
+                def middleware_stack; end
+              end
+            end
+
+            module SomeEngine::SomeController::HelperMethods
+              include ::ActionController::Base::HelperMethods
+
+            <% if ruby_version(">= 3.1") %>
+              def foo(*args, **_arg1, &block); end
+            <% else %>
+              def foo(*args, &block); end
+            <% end %>
+            end
+          RBI
+
+          assert_project_file_equal("sorbet/rbi/gems/some_engine@0.0.2.rbi", expected)
+        end
+
         it "must generate RBIs for constants defined in a different gem but with mixins in this gem" do
           foo = mock_gem("foo", "0.0.1") do
             write("lib/foo.rb", <<~RBI)