fix: no longer generate cross-package references for mixin objects

gapic-generator/lib/gapic/formatting_utils.rb

@@ -116,7 +116,12 @@ def escape_line_braces line
       def format_line_xrefs api, line, disable_xrefs, transport
         while (m = @xref_detector.match line)
           entity = api.lookup m[:addr]
-          return line if entity.nil?
+          is_mixin_field_addr = Gapic::Model::Mixins.mixin_message_field_address?(
+            m[:addr],
+            gem_name: api.configuration.fetch(:gem, nil)&.fetch(:name, "")
+          )
+          return line if entity.nil? || is_mixin_field_addr
+
           text = m[:text]
           yard_link = disable_xrefs ? text : yard_link_for_entity(entity, text, transport)
           return line if yard_link.nil?

gapic-generator/lib/gapic/model/mixins.rb

@@ -162,6 +162,25 @@ def self.mixin_service_address? service_address, gem_name: nil
         MIXIN_GEM_NAMES.include?(service_address) && gem_name != MIXIN_GEM_NAMES[service_address]
       end
 
+      ##
+      # Returns true if the given service address is a mixin.
+      # This just checks the service against a (hard-coded) set of known mixins.
+      # If `gem_name` is provided, objects from the package of the service
+      # that corresponds to that gem_name are not considered mixins.
+      #
+      # @param message_field_address [String,Array<String>] The address (either array
+      #     or dot-delimited) of the message or field to check.
+      # @param gem_name [String] The name of the gem.
+      # @return [boolean]
+      #
+      def self.mixin_message_field_address? message_field_address, gem_name: nil
+        message_field_address = message_field_address.join "." unless message_field_address.is_a? String
+        # NB: messages are checked against package, not service
+        service_address = MIXIN_GEM_NAMES.keys.find { |sn| message_field_address.start_with? MIXIN_PACKAGE_NAMES[sn] }
+
+        !service_address.nil? && gem_name != MIXIN_GEM_NAMES[service_address]
+      end
+
       private
 
       # @return [Enumerable<String>] Names of all services that are specified
@@ -179,6 +198,13 @@ def services_in_config
       }.freeze
       private_constant :MIXIN_GEM_NAMES
 
+      MIXIN_PACKAGE_NAMES = {
+        LOCATIONS_SERVICE => "google.cloud.location",
+        IAM_SERVICE => "google.iam.v1",
+        LRO_SERVICE => "google.longrunning.operations"
+      }.freeze
+      private_constant :MIXIN_PACKAGE_NAMES
+
       # Since mixins are scope-limited to a couple of services, it is easier to
       # have these in lookup tables than to construct a ServicePresenter
 

gapic-generator/test/gapic/mixins/mixins_test.rb

@@ -45,6 +45,25 @@ def test_mixin_service_address_checker
     refute Gapic::Model::Mixins.mixin_service_address? "testing.mixins.ServiceWithLoc"
   end
 
+  def test_mixin_message_field_address_checker
+    # sic. no `s` at the end, this is a message named `Location`, not service named `Locations`
+    assert Gapic::Model::Mixins.mixin_message_field_address? "google.cloud.location.Location"
+    assert Gapic::Model::Mixins.mixin_message_field_address? "google.cloud.location.Location.metadata"
+
+    assert Gapic::Model::Mixins.mixin_message_field_address? "google.cloud.location.Location",
+                                                             gem_name: "google-cloud-something-else"
+
+    refute Gapic::Model::Mixins.mixin_message_field_address? "google.cloud.location.Location",
+                                                             gem_name: "google-cloud-location"
+    refute Gapic::Model::Mixins.mixin_message_field_address? "google.cloud.location.Location.metadata",
+                                                             gem_name: "google-cloud-location"
+
+    assert Gapic::Model::Mixins.mixin_message_field_address? ["google", "iam", "v1", "Policy"]
+    assert Gapic::Model::Mixins.mixin_message_field_address? ["google", "iam", "v1", "Policy", "bindings"]
+
+    refute Gapic::Model::Mixins.mixin_message_field_address? "testing.mixins.Request"
+  end
+
   # Test the `Garbage` library, which does NOT have mixins specified
   # in its service.yaml (or service.yaml at all)
   def test_garbage_mixins

gapic-generator/test/test_helper.rb

@@ -344,6 +344,10 @@ def fix_namespace name
   def fix_service_name name
     @service_mapping[name] || name
   end
+
+  def configuration
+    {}
+  end
 end
 
 # A fake request builder

shared/gem_defaults.rb

@@ -156,6 +156,10 @@ def gem_defaults
         "testing/mixins/mixins.proto",
         "testing/routing_headers/routing_headers.proto",
         "testing/nonstandard_lro_grpc/nonstandard_lro_grpc.proto",
+        # `locations.proto` is included here because it is often
+        # included in the real world libraries
+        # as part of `files_to_generate` due to protoc limitations
+        "google/cloud/location/locations.proto",
       ],
       grpc_service_config: [
         "../shared/protos/testing/grpc_service_config/grpc_service_config.json",

shared/output/cloud/secretmanager_v1beta1/proto_docs/google/iam/v1/policy.rb

@@ -313,7 +313,7 @@ class AuditConfig
       #     Specifies the identities that do not cause logging for this type of
       #     permission.
       #     Follows the same format of
-      #     {::Google::Iam::V1::Binding#members Binding.members}.
+      #     [Binding.members][google.iam.v1.Binding.members].
       class AuditLogConfig
         include ::Google::Protobuf::MessageExts
         extend ::Google::Protobuf::MessageExts::ClassMethods

shared/output/gapic/templates/garbage/proto_docs/google/iam/v1/policy.rb

@@ -321,7 +321,7 @@ class AuditConfig
       #     Specifies the identities that do not cause logging for this type of
       #     permission.
       #     Follows the same format of
-      #     {::Google::Iam::V1::Binding#members Binding.members}.
+      #     [Binding.members][google.iam.v1.Binding.members].
       class AuditLogConfig
         include ::Google::Protobuf::MessageExts
         extend ::Google::Protobuf::MessageExts::ClassMethods

shared/output/gapic/templates/testing/proto_docs/testing/mixins/mixins.rb

@@ -36,5 +36,12 @@ class Response
       include ::Google::Protobuf::MessageExts
       extend ::Google::Protobuf::MessageExts::ClassMethods
     end
+
+    # This comment tests a mixin documentation reference:
+    # [google.cloud.location.Location.metadata][google.cloud.location.Location.metadata].
+    class ReferencedInServiceYaml
+      include ::Google::Protobuf::MessageExts
+      extend ::Google::Protobuf::MessageExts::ClassMethods
+    end
   end
 end

shared/protos/testing/mixins/mixins.proto

@@ -43,3 +43,8 @@ message Request {
 
 message Response {
 }
+
+// This comment tests a mixin documentation reference:
+// [google.cloud.location.Location.metadata][google.cloud.location.Location.metadata].
+message ReferencedInServiceYaml {
+}

shared/protos/testing/mixins/testing_service.yaml

@@ -12,6 +12,9 @@ apis:
 - name: google.longrunning.Operations
 - name: google.cloud.location.Locations
 
+types:
+- name: testing.mixins.ReferencedInServiceYaml
+
 http:
   rules:
   - selector: google.longrunning.Operations.ListOperations