feat(AIP-215): augment foreign type checking (#1467)

Author: shollyman

docs/rules/0215/foreign-type-reference.md

@@ -0,0 +1,75 @@
+---
+rule:
+  aip: 215
+  name: [core, '0215', foreign-type-reference]
+  summary: API should not reference foreign types outside of the API scope.
+permalink: /215/foreign-type-reference
+redirect_from:
+  - /0215/foreign-type-reference
+---
+
+# Versioned packages
+
+This rule enforces that none of the fields in an API reference message types in a different
+proto package namespace other than well-known common packages.
+
+## Details
+
+This rule examines all fields in an API's messages and complains if the package of the
+referenced message type is not the same or from one of the common packages such as
+`google.api`, `google.protobuf`, etc.
+
+## Examples
+
+**Incorrect** code for this rule:
+
+```proto
+// Incorrect.
+package foo.bar;
+import "some/other.proto";
+
+message SomeMessage {
+    some.OtherMessage other_message = 1;
+}
+```
+
+**Correct** code for this rule:
+
+```proto
+// Correct.
+package foo.bar;
+import "other.proto";
+
+message SomeMessage {
+    OtherMessage other_message = 1;
+}
+```
+
+## Known issues
+
+This check only allows subpackage usage within a versioned path, but generates warnings for unversioned subpackage usage.
+It also ignores if the referenced package is a "common" package like `google.api`, or if the package path ends in `.type` indicating
+the package is an AIP-213 component package.
+
+Examples of foreign type references and their expected results:
+
+| Calling Package | Referenced Package | Result       |
+| --------------- | ------------------ | ------------ |
+| foo.bar         | foo.xyz            | lint warning |
+| foo.v2.bar      | foo.v2.xyz         | ok           |
+| foo.bar         | foo.type           | ok           |
+| foo.bar         | google.api         | ok           |
+
+## Disabling
+
+If you need to violate this rule, place the comment above the package statement.
+Remember to also include an [aip.dev/not-precedent][] comment explaining why.
+
+```proto
+// (-- api-linter: core::0215::foreign-type-reference=disabled
+//     aip.dev/not-precedent: We need to do this because reasons. --)
+package foo.bar;
+```
+
+[aip-215]: https://aip.dev/215
+[aip.dev/not-precedent]: https://aip.dev/not-precedent

rules/aip0215/aip0215.go

@@ -24,5 +24,6 @@ func AddRules(r lint.RuleRegistry) error {
 	return r.Register(
 		215,
 		versionedPackages,
+		foreignTypeReference,
 	)
 }

rules/aip0215/foreign_type_reference.go

@@ -0,0 +1,82 @@
+// Copyright 2025 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//	https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package aip0215
+
+import (
+	"fmt"
+	"regexp"
+	"strings"
+
+	"github.com/googleapis/api-linter/lint"
+	"github.com/googleapis/api-linter/rules/internal/utils"
+	"github.com/jhump/protoreflect/desc"
+	"google.golang.org/protobuf/types/descriptorpb"
+)
+
+var foreignTypeReference = &lint.FieldRule{
+	Name: lint.NewRuleName(215, "foreign-type-reference"),
+	OnlyIf: func(fd *desc.FieldDescriptor) bool {
+		return fd.GetType() == descriptorpb.FieldDescriptorProto_TYPE_MESSAGE
+	},
+	LintField: func(fd *desc.FieldDescriptor) []lint.Problem {
+		curPkg := getNormalizedPackage(fd)
+		if curPkg == "" {
+			return nil // Empty or unavailable package.
+		}
+		msg := fd.GetMessageType()
+		if msg == nil {
+			return nil // Couldn't resolve type.
+		}
+		msgPkg := getNormalizedPackage(msg)
+		if msgPkg == "" {
+			return nil // Empty or unavailable package.
+		}
+
+		if utils.IsCommonProto(msg.GetFile()) {
+			return nil // reference to a well known proto package.
+		}
+
+		if strings.HasSuffix(msgPkg, ".type") {
+			return nil // AIP-213 component type.
+		}
+
+		if curPkg != msgPkg {
+			return []lint.Problem{{
+				Message:    fmt.Sprintf("foreign type referenced, current field in %q message in %q", curPkg, msgPkg),
+				Descriptor: fd,
+			}}
+		}
+
+		return nil
+	},
+}
+
+// Regexp to capture everything up to a versioned segment.
+var versionedPrefix = regexp.MustCompile(`^.*\.v[\d]+(p[\d]+)?(alpha|beta|eap|test)?[\d]*`)
+
+// getNormalizedPackage returns a normalized package path.
+// If package cannot be resolved it returns the empty string.
+// If the package path has a "versioned" segment, the path is truncated to that segment.
+func getNormalizedPackage(d desc.Descriptor) string {
+	f := d.GetFile()
+	if f == nil {
+		return ""
+	}
+	pkg := f.GetPackage()
+	if normPkg := versionedPrefix.FindString(pkg); normPkg != "" {
+		pkg = normPkg
+	}
+	return pkg
+}

rules/aip0215/foreign_type_reference_test.go

@@ -0,0 +1,152 @@
+// Copyright 2025 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//	https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package aip0215
+
+import (
+	"testing"
+
+	"github.com/googleapis/api-linter/rules/internal/testutils"
+)
+
+// Tests our regexp normalizes strings to the expected path.
+func TestVersionNormalization(t *testing.T) {
+	for _, tc := range []struct {
+		in   string
+		want string
+	}{
+		{
+			in:   "",
+			want: "",
+		},
+		{
+			in:   "foo.bar.baz",
+			want: "",
+		},
+		{
+			// This one's a bit iffy.  Should a version be allowed as the first segment?
+			in:   "v1beta",
+			want: "",
+		},
+		{
+			in:   "foo.v3",
+			want: "foo.v3",
+		},
+		{
+			in:   "foo.v99alpha.bar",
+			want: "foo.v99alpha",
+		},
+		{
+			in:   "foo.v2.bar.v2",
+			want: "foo.v2.bar.v2",
+		},
+	} {
+		got := versionedPrefix.FindString(tc.in)
+		if got != tc.want {
+			t.Errorf("mismatch: in %q, got %q, want %q", tc.in, got, tc.want)
+		}
+	}
+}
+
+func TestForeignTypeReference(t *testing.T) {
+
+	for _, tc := range []struct {
+		description   string
+		CallingPkg    string
+		ReferencePkg  string
+		ReferencedMsg string
+		problems      testutils.Problems
+	}{
+		{
+			description:   "same pkg",
+			CallingPkg:    "same",
+			ReferencePkg:  "same",
+			ReferencedMsg: "OtherMessage",
+			problems:      nil,
+		},
+		{
+			description:   "refers to google.api",
+			CallingPkg:    "same",
+			ReferencePkg:  "google.api",
+			ReferencedMsg: "google.api.OtherMessage",
+			problems:      nil,
+		},
+		{
+			description:   "refers to google.protobuf",
+			CallingPkg:    "same",
+			ReferencePkg:  "google.protobuf",
+			ReferencedMsg: "google.protobuf.OtherMessage",
+			problems:      nil,
+		},
+		{
+			description:   "refers to foreign pkg",
+			CallingPkg:    "same",
+			ReferencePkg:  "other",
+			ReferencedMsg: "other.OtherMessage",
+			problems:      testutils.Problems{{Message: "foreign type referenced"}},
+		},
+		{
+			description:   "unversioned subpackage",
+			CallingPkg:    "somepackage",
+			ReferencePkg:  "somepackage.sub",
+			ReferencedMsg: "somepackage.sub.OtherMessage",
+			problems:      testutils.Problems{{Message: "foreign type referenced"}},
+		},
+		{
+			description:   "versioned subpackage",
+			CallingPkg:    "somepackage.v6",
+			ReferencePkg:  "somepackage.v6.sub",
+			ReferencedMsg: "somepackage.v6.sub.OtherMessage",
+			problems:      nil,
+		},
+		{
+			description:   "versioned deep subpackaging",
+			CallingPkg:    "somepackage.v1.abc",
+			ReferencePkg:  "somepackage.v1.lol.xyz",
+			ReferencedMsg: "somepackage.v1.lol.xyz.OtherMessage",
+			problems:      nil,
+		},
+		{
+			description:   "refers to component package",
+			CallingPkg:    "somepackage",
+			ReferencePkg:  "otherpackage.type",
+			ReferencedMsg: "otherpackage.type.OtherMessage",
+			problems:      nil,
+		},
+	} {
+		t.Run(tc.description, func(t *testing.T) {
+			files := testutils.ParseProto3Tmpls(t, map[string]string{
+				"calling.proto": `
+					package {{.CallingPkg}};
+					import "ref.proto";
+					message Caller {
+						string foo = 1;
+						{{.ReferencedMsg}} bar = 2;
+					}
+				`,
+				"ref.proto": `
+					package {{.ReferencePkg}};
+					message OtherMessage {
+						int32 baz = 1;
+					}
+				`,
+			}, tc)
+			file := files["calling.proto"]
+			field := file.GetMessageTypes()[0].GetFields()[1]
+			if diff := tc.problems.SetDescriptor(field).Diff(foreignTypeReference.Lint(file)); diff != "" {
+				t.Error(diff)
+			}
+		})
+	}
+}