fix: no longer generate cross-package references for mixin objects (#1211)

* fix: no longer generate cross-package references for mixin elements

Author: viacheslav-rostovtsev

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/input/testing_desc.bin

@@ -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/cloud/secretmanager_v1beta1/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