Repository: googleapis/api-linter
Pull request #444 (2019-12-11 19:28:49)

[fix,refactor] Allow imports for response/metadata types.
Also split the two into separate rules.


Patch:
diff --git a/docs/rules/0151/lro-metadata-reachable.md b/docs/rules/0151/lro-metadata-reachable.md
new file mode 100644
index 0000000..454dc72
--- /dev/null
+++ b/docs/rules/0151/lro-metadata-reachable.md
@@ -0,0 +1,154 @@
+---
+rule:
+  aip: 151
+  name: [core, '0151', lro-metadata-reachable]
+  summary: LRO metadata messages must be reachable.
+permalink: /151/lro-metadata-reachable
+redirect_from:
+  - /0151/lro-metadata-reachable
+---
+
+# LRO: Reachable metadata types
+
+This rule enforces that methods returning long-running operations define their
+metadata messages in the same file or a directly-imported file, as mandated in
+[AIP-151][].
+
+## Details
+
+This rule looks at any method with a return type of
+`google.longrunning.Operation`, and complains if the message designated by the
+`metadata_type` field are not defined in the same file, or a file directly
+imported by the file.
+
+Because these message names are strings, and a string reference does not
+require an `import` statement, defining the metadata types elsewhere can cause
+problems for tooling. To prevent this, and also to maintain consistency with
+the file layout in [AIP-191][], the linter complains if the message is not
+defined in the same file or a file directly imported by the file.
+
+## Examples
+
+**Incorrect** code for this rule:
+
+In `library_service.proto`:
+
+```proto
+// Incorrect.
+service Library {
+  rpc WriteBook(WriteBookRequest) returns (google.longrunning.Operation) {
+    option (google.api.http) = {
+      post: "/v1/{name=publishers/*/books}:write"
+      body: "*"
+    };
+    option (google.longrunning.operation_info) = {
+      response_type: "WriteBookResponse"
+      metadata_type: "WriteBookMetadata"
+    };
+  }
+}
+```
+
+In `operations.proto`:
+
+```proto
+// Incorrect.
+message WriteBookResponse {
+  // Should be in the same file or directly imported.
+}
+
+message WriteBookMetadata {
+  // Should be in the same file or directly imported.
+}
+```
+
+**Correct** code for this rule:
+
+Same file:
+
+```proto
+// Correct.
+service Library {
+  rpc WriteBook(WriteBookRequest) returns (google.longrunning.Operation) {
+    option (google.api.http) = {
+      post: "/v1/{name=publishers/*/books}:write"
+      body: "*"
+    };
+    option (google.longrunning.operation_info) = {
+      response_type: "WriteBookResponse"
+      metadata_type: "WriteBookMetadata"
+    };
+  }
+}
+
+// Later in the file...
+message WriteBookResponse {
+  // ...
+}
+
+message WriteBookMetadata {
+  // ...
+}
+```
+
+Separate files:
+
+In `library_service.proto`:
+
+```proto
+// Correct.
+import "operations.proto";
+
+service Library {
+  rpc WriteBook(WriteBookRequest) returns (google.longrunning.Operation) {
+    option (google.api.http) = {
+      post: "/v1/{name=publishers/*/books}:write"
+      body: "*"
+    };
+    option (google.longrunning.operation_info) = {
+      response_type: "WriteBookResponse"
+      metadata_type: "WriteBookMetadata"
+    };
+  }
+}
+```
+
+In `operations.proto`:
+
+```proto
+// Correct.
+message WriteBookResponse {
+  // ...
+}
+
+message WriteBookMetadata {
+  // ...
+}
+```
+
+## Disabling
+
+If you need to violate this rule, use a leading comment above the method.
+Remember to also include an [aip.dev/not-precedent][] comment explaining why.
+
+```proto
+// (-- api-linter: core::0151::lro-metadata-reachable=disabled
+//     aip.dev/not-precedent: We need to do this because reasons. --)
+rpc WriteBook(WriteBookRequest) returns (google.longrunning.Operation) {
+  option (google.api.http) = {
+    post: "/v1/{name=publishers/*/books}:write"
+    body: "*"
+  };
+  option (google.longrunning.operation_info) = {
+    response_type: "WriteBookResponse"
+    metadata_type: "UndefinedMetadataMessage"
+  };
+}
+```
+
+If you need to violate this rule for an entire file, place the comment at the
+top of the file.
+
+[aip-151]: https://aip.dev/151
+[aip-191]: https://aip.dev/191
+[aip.dev/not-precedent]: https://aip.dev/not-precedent
diff --git a/docs/rules/0151/lro-response-reachable.md b/docs/rules/0151/lro-response-reachable.md
new file mode 100644
index 0000000..8638827
--- /dev/null
+++ b/docs/rules/0151/lro-response-reachable.md
@@ -0,0 +1,154 @@
+---
+rule:
+  aip: 151
+  name: [core, '0151', lro-response-reachable]
+  summary: LRO response messages must be reachable.
+permalink: /151/lro-response-reachable
+redirect_from:
+  - /0151/lro-response-reachable
+---
+
+# LRO: Reachable response types
+
+This rule enforces that methods returning long-running operations define their
+response messages in the same file or a directly-imported file, as mandated in
+[AIP-151][].
+
+## Details
+
+This rule looks at any method with a return type of
+`google.longrunning.Operation`, and complains if the message designated by the
+`response_type` field are not defined in the same file, or a file directly
+imported by the file.
+
+Because these message names are strings, and a string reference does not
+require an `import` statement, defining the response types elsewhere can cause
+problems for tooling. To prevent this, and also to maintain consistency with
+the file layout in [AIP-191][], the linter complains if the message is not
+defined in the same file or a file directly imported by the file.
+
+## Examples
+
+**Incorrect** code for this rule:
+
+In `library_service.proto`:
+
+```proto
+// Incorrect.
+service Library {
+  rpc WriteBook(WriteBookRequest) returns (google.longrunning.Operation) {
+    option (google.api.http) = {
+      post: "/v1/{name=publishers/*/books}:write"
+      body: "*"
+    };
+    option (google.longrunning.operation_info) = {
+      response_type: "WriteBookResponse"
+      metadata_type: "WriteBookMetadata"
+    };
+  }
+}
+```
+
+In `operations.proto`:
+
+```proto
+// Incorrect.
+message WriteBookResponse {
+  // Should be in the same file or directly imported.
+}
+
+message WriteBookMetadata {
+  // Should be in the same file or directly imported.
+}
+```
+
+**Correct** code for this rule:
+
+Same file:
+
+```proto
+// Correct.
+service Library {
+  rpc WriteBook(WriteBookRequest) returns (google.longrunning.Operation) {
+    option (google.api.http) = {
+      post: "/v1/{name=publishers/*/books}:write"
+      body: "*"
+    };
+    option (google.longrunning.operation_info) = {
+      response_type: "WriteBookResponse"
+      metadata_type: "WriteBookMetadata"
+    };
+  }
+}
+
+// Later in the file...
+message WriteBookResponse {
+  // ...
+}
+
+message WriteBookMetadata {
+  // ...
+}
+```
+
+Separate files:
+
+In `library_service.proto`:
+
+```proto
+// Correct.
+import "operations.proto";
+
+service Library {
+  rpc WriteBook(WriteBookRequest) returns (google.longrunning.Operation) {
+    option (google.api.http) = {
+      post: "/v1/{name=publishers/*/books}:write"
+      body: "*"
+    };
+    option (google.longrunning.operation_info) = {
+      response_type: "WriteBookResponse"
+      metadata_type: "WriteBookMetadata"
+    };
+  }
+}
+```
+
+In `operations.proto`:
+
+```proto
+// Correct.
+message WriteBookResponse {
+  // ...
+}
+
+message WriteBookMetadata {
+  // ...
+}
+```
+
+## Disabling
+
+If you need to violate this rule, use a leading comment above the method.
+Remember to also include an [aip.dev/not-precedent][] comment explaining why.
+
+```proto
+// (-- api-linter: core::0151::lro-response-reachable=disabled
+//     aip.dev/not-precedent: We need to do this because reasons. --)
+rpc WriteBook(WriteBookRequest) returns (google.longrunning.Operation) {
+  option (google.api.http) = {
+    post: "/v1/{name=publishers/*/books}:write"
+    body: "*"
+  };
+  option (google.longrunning.operation_info) = {
+    response_type: "google.protobuf.Empty"
+    metadata_type: "WriteBookMetadata"
+  };
+}
+```
+
+If you need to violate this rule for an entire file, place the comment at the
+top of the file.
+
+[aip-151]: https://aip.dev/151
+[aip-191]: https://aip.dev/191
+[aip.dev/not-precedent]: https://aip.dev/not-precedent
diff --git a/docs/rules/0151/lro-types-defined-in-file.md b/docs/rules/0151/lro-types-defined-in-file.md
deleted file mode 100644
index b85dc9b..0000000
--- a/docs/rules/0151/lro-types-defined-in-file.md
+++ /dev/null
@@ -1,111 +0,0 @@
----
-rule:
-  aip: 151
-  name: [core, '0151', lro-types-defined-in-file]
-  summary: LRO response and metadata messages must be defined in the same file.
-permalink: /151/lro-types-defined-in-file
-redirect_from:
-  - /0151/lro-types-defined-in-file
----
-
-# LRO types defined in file
-
-This rule enforces that methods returning long-running operations define their
-response and metadata messages in the same file, as mandated in [AIP-151][].
-
-## Details
-
-This rule looks at any method with a return type of
-`google.longrunning.Operation`, and complains if the messages designated by the
-`response_type` and `metadata_type` fields are not defined in the same file.
-
-Because these message names are strings, and a string reference does not
-require an `import` statement, defining the response and metadata types
-elsewhere can cause problems for tooling. To prevent this, and also to maintain
-consistency with the file layout in [AIP-191][], the linter complains if the
-message is not defined in the same file.
-
-## Examples
-
-**Incorrect** code for this rule:
-
-In `library_service.proto`:
-
-```proto
-// Incorrect.
-rpc WriteBook(WriteBookRequest) returns (google.longrunning.Operation) {
-  option (google.api.http) = {
-    post: "/v1/{name=publishers/*/books}:write"
-    body: "*"
-  };
-  option (google.longrunning.operation_info) = {
-    response_type: "WriteBookResponse"
-    metadata_type: "WriteBookMetadata"
-  };
-}
-```
-
-In `operations.proto`:
-
-```proto
-// Incorrect.
-message WriteBookResponse {
-  // Should be in the same file.
-}
-
-message WriteBookMetadata {
-  // Should be in the same file.
-}
-```
-
-**Correct** code for this rule:
-
-```proto
-// Correct.
-rpc WriteBook(WriteBookRequest) returns (google.longrunning.Operation) {
-  option (google.api.http) = {
-    post: "/v1/{name=publishers/*/books}:write"
-    body: "*"
-  };
-  option (google.longrunning.operation_info) = {
-    response_type: "WriteBookResponse"
-    metadata_type: "WriteBookMetadata"
-  };
-}
-
-// Later in the file...
-message WriteBookResponse {
-  // ...
-}
-
-message WriteBookMetadata {
-  // ...
-}
-```
-
-## Disabling
-
-If you need to violate this rule, use a leading comment above the method.
-Remember to also include an [aip.dev/not-precedent][] comment explaining why.
-
-```proto
-// (-- api-linter: core::0151::lro-types-defined-in-file=disabled
-//     aip.dev/not-precedent: We need to do this because reasons. --)
-rpc WriteBook(WriteBookRequest) returns (google.longrunning.Operation) {
-  option (google.api.http) = {
-    post: "/v1/{name=publishers/*/books}:write"
-    body: "*"
-  };
-  option (google.longrunning.operation_info) = {
-    response_type: "google.protobuf.Empty"
-    metadata_type: "WriteBookMetadata"
-  };
-}
-```
-
-If you need to violate this rule for an entire file, place the comment at the
-top of the file.
-
-[aip-151]: https://aip.dev/151
-[aip-191]: https://aip.dev/191
-[aip.dev/not-precedent]: https://aip.dev/not-precedent
diff --git a/rules/aip0151/aip0151.go b/rules/aip0151/aip0151.go
index 3e2665d..5e004e6 100644
--- a/rules/aip0151/aip0151.go
+++ b/rules/aip0151/aip0151.go
@@ -26,9 +26,10 @@ func AddRules(r lint.RuleRegistry) error {
 	return r.Register(
 		151,
 		lroAnnotationExists,
-		lroDefinedInFile,
 		lroMetadata,
+		lroMetadataReachable,
 		lroResponse,
+		lroResponseReachable,
 	)
 }
 
diff --git a/rules/aip0151/lro_metadata_reachable.go b/rules/aip0151/lro_metadata_reachable.go
new file mode 100644
index 0000000..f932b2d
--- /dev/null
+++ b/rules/aip0151/lro_metadata_reachable.go
@@ -0,0 +1,30 @@
+// Copyright 2019 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 aip0151
+
+import (
+	"github.com/googleapis/api-linter/lint"
+	"github.com/googleapis/api-linter/rules/internal/utils"
+	"github.com/jhump/protoreflect/desc"
+)
+
+var lroMetadataReachable = &lint.MethodRule{
+	Name:   lint.NewRuleName(151, "lro-metadata-reachable"),
+	OnlyIf: isAnnotatedLRO,
+	LintMethod: func(m *desc.MethodDescriptor) (problems []lint.Problem) {
+		// See lro_response_reachable.go for `checkReachable` method.
+		return checkReachable(m, utils.GetOperationInfo(m).GetMetadataType())
+	},
+}
diff --git a/rules/aip0151/lro_response_reachable.go b/rules/aip0151/lro_response_reachable.go
new file mode 100644
index 0000000..500c952
--- /dev/null
+++ b/rules/aip0151/lro_response_reachable.go
@@ -0,0 +1,65 @@
+// Copyright 2019 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 aip0151
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/googleapis/api-linter/lint"
+	"github.com/googleapis/api-linter/locations"
+	"github.com/googleapis/api-linter/rules/internal/utils"
+	"github.com/jhump/protoreflect/desc"
+)
+
+var lroResponseReachable = &lint.MethodRule{
+	Name:   lint.NewRuleName(151, "lro-response-reachable"),
+	OnlyIf: isAnnotatedLRO,
+	LintMethod: func(m *desc.MethodDescriptor) (problems []lint.Problem) {
+		return checkReachable(m, utils.GetOperationInfo(m).GetResponseType())
+	},
+}
+
+func checkReachable(m *desc.MethodDescriptor, name string) []lint.Problem {
+	// Ignore types defined in other packages.
+	if name == "" || strings.Contains(name, ".") {
+		return nil
+	}
+
+	// Make this the fully qualified type name.
+	f := m.GetFile()
+	if pkg := f.GetPackage(); pkg != "" {
+		name = pkg + "." + name
+	}
+
+	// If the message is defined in the file, we are good to go.
+	files := []*desc.FileDescriptor{f}
+	files = append(files, f.GetDependencies()...)
+	for _, file := range files {
+		if file.FindMessage(name) != nil {
+			return nil
+		}
+	}
+
+	// We could not find the message; complain.
+	return []lint.Problem{{
+		Message: fmt.Sprintf(
+			"Message %q must be defined in the same file, or a file imported by this file.",
+			name,
+		),
+		Descriptor: m,
+		Location:   locations.MethodOperationInfo(m),
+	}}
+}
diff --git a/rules/aip0151/lro_types_defined_in_file.go b/rules/aip0151/lro_types_defined_in_file.go
deleted file mode 100644
index 9739e33..0000000
--- a/rules/aip0151/lro_types_defined_in_file.go
+++ /dev/null
@@ -1,57 +0,0 @@
-// Copyright 2019 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 aip0151
-
-import (
-	"fmt"
-	"strings"
-
-	"github.com/googleapis/api-linter/lint"
-	"github.com/googleapis/api-linter/locations"
-	"github.com/googleapis/api-linter/rules/internal/utils"
-	"github.com/jhump/protoreflect/desc"
-)
-
-var lroDefinedInFile = &lint.MethodRule{
-	Name:   lint.NewRuleName(151, "lro-types-defined-in-file"),
-	OnlyIf: isAnnotatedLRO,
-	LintMethod: func(m *desc.MethodDescriptor) (problems []lint.Problem) {
-		lro := utils.GetOperationInfo(m)
-		for k, t := range map[string]string{"Response": lro.GetResponseType(), "Metadata": lro.GetMetadataType()} {
-			// Ignore types defined in other packages.
-			if t == "" || strings.Contains(t, ".") {
-				continue
-			}
-
-			// Complain if the message is not defined in this file.
-			file := m.GetFile()
-			if pkg := file.GetPackage(); pkg != "" {
-				t = pkg + "." + t
-			}
-			if file.FindMessage(t) == nil {
-				problems = append(problems, lint.Problem{
-					Message: fmt.Sprintf(
-						"%s messages should be defined in the same file as the RPC.",
-						k,
-					),
-					Descriptor: m,
-					Location:   locations.MethodOperationInfo(m),
-				})
-			}
-		}
-
-		return
-	},
-}


Test patch:
diff --git a/rules/aip0151/lro_metadata_reachable_test.go b/rules/aip0151/lro_metadata_reachable_test.go
new file mode 100644
index 0000000..4bcad38
--- /dev/null
+++ b/rules/aip0151/lro_metadata_reachable_test.go
@@ -0,0 +1,104 @@
+// Copyright 2019 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 aip0151
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/googleapis/api-linter/rules/internal/testutils"
+)
+
+func TestLROMetadataReachable(t *testing.T) {
+	t.Run("SameFile", func(t *testing.T) {
+		tests := []struct {
+			testName     string
+			Package      string
+			MetadataType string
+			problems     testutils.Problems
+		}{
+			{"Valid", "", "WriteBookMetadata", testutils.Problems{}},
+			{"ValidPkg", "package test;", "WriteBookMetadata", testutils.Problems{}},
+			{"InvalidTypo", "", "WriteBookMeta", testutils.Problems{{Message: "WriteBookMeta"}}},
+			{"InvalidTypoPkg", "package test;", "WriteBookMeta", testutils.Problems{{Message: "test.WriteBookMeta"}}},
+			{"ValidExternal", "", "google.protobuf.Empty", testutils.Problems{}},
+			{"ValidExternalPkg", "package test;", "google.protobuf.Empty", testutils.Problems{}},
+		}
+		for _, test := range tests {
+			t.Run(test.testName, func(t *testing.T) {
+				f := testutils.ParseProto3Tmpl(t, `
+					{{.Package}}
+					import "google/longrunning/operations.proto";
+					service Library {
+						rpc WriteBook(WriteBookRequest) returns (google.longrunning.Operation) {
+							option (google.longrunning.operation_info) = {
+								response_type: "WriteBookResponse"
+								metadata_type: "{{.MetadataType}}"
+							};
+						}
+					}
+					message WriteBookRequest {}
+					message WriteBookResponse {}
+					message WriteBookMetadata {}
+				`, test)
+				problems := lroMetadataReachable.Lint(f)
+				m := f.GetServices()[0].GetMethods()[0]
+				if diff := test.problems.SetDescriptor(m).Diff(problems); diff != "" {
+					t.Errorf(diff)
+				}
+
+			})
+		}
+	})
+	t.Run("Imported", func(t *testing.T) {
+		for _, test := range []struct {
+			name     string
+			message  string
+			problems testutils.Problems
+		}{
+			{"Present", "message WriteBookMetadata {}", testutils.Problems{}},
+			{"Absent", "", testutils.Problems{{Message: "WriteBookMetadata"}}},
+		} {
+			t.Run(test.name, func(t *testing.T) {
+				files := testutils.ParseProtoStrings(t, map[string]string{
+					"imported.proto": strings.ReplaceAll(`
+						syntax = "proto3";
+						message WriteBookRequest {}
+						message WriteBookResponse {}
+						---
+					`, "---", test.message),
+					"test.proto": `
+						syntax = "proto3";
+						import "google/longrunning/operations.proto";
+						import "imported.proto";
+						service Library {
+							rpc WriteBook(WriteBookRequest) returns (google.longrunning.Operation) {
+								option (google.longrunning.operation_info) = {
+									response_type: "WriteBookResponse"
+									metadata_type: "WriteBookMetadata"
+								};
+							}
+						}
+					`,
+				})
+				problems := lroMetadataReachable.Lint(files["test.proto"])
+				method := files["test.proto"].GetServices()[0].GetMethods()[0]
+				if diff := test.problems.SetDescriptor(method).Diff(problems); diff != "" {
+					t.Errorf(diff)
+				}
+			})
+		}
+	})
+}
diff --git a/rules/aip0151/lro_response_reachable_test.go b/rules/aip0151/lro_response_reachable_test.go
new file mode 100644
index 0000000..207116c
--- /dev/null
+++ b/rules/aip0151/lro_response_reachable_test.go
@@ -0,0 +1,103 @@
+// Copyright 2019 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 aip0151
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/googleapis/api-linter/rules/internal/testutils"
+)
+
+func TestLROResponseReachable(t *testing.T) {
+	t.Run("SameFile", func(t *testing.T) {
+		tests := []struct {
+			testName     string
+			Package      string
+			ResponseType string
+			problems     testutils.Problems
+		}{
+			{"Valid", "", "WriteBookResponse", testutils.Problems{}},
+			{"ValidPkg", "package test;", "WriteBookResponse", testutils.Problems{}},
+			{"InvalidTypo", "", "WriteBookReponse", testutils.Problems{{Message: "WriteBookReponse"}}},
+			{"InvalidTypoPkg", "package test;", "WriteBookReponse", testutils.Problems{{Message: "test.WriteBookReponse"}}},
+			{"ValidExternal", "", "google.protobuf.Empty", testutils.Problems{}},
+			{"ValidExternalPkg", "package test;", "google.protobuf.Empty", testutils.Problems{}},
+		}
+		for _, test := range tests {
+			t.Run(test.testName, func(t *testing.T) {
+				f := testutils.ParseProto3Tmpl(t, `
+					{{.Package}}
+					import "google/longrunning/operations.proto";
+					service Library {
+						rpc WriteBook(WriteBookRequest) returns (google.longrunning.Operation) {
+							option (google.longrunning.operation_info) = {
+								response_type: "{{.ResponseType}}"
+								metadata_type: "OperationMetadata"
+							};
+						}
+					}
+					message WriteBookRequest {}
+					message WriteBookResponse {}
+					message OperationMetadata {}
+				`, test)
+				problems := lroResponseReachable.Lint(f)
+				m := f.GetServices()[0].GetMethods()[0]
+				if diff := test.problems.SetDescriptor(m).Diff(problems); diff != "" {
+					t.Errorf(diff)
+				}
+			})
+		}
+	})
+	t.Run("Imported", func(t *testing.T) {
+		for _, test := range []struct {
+			name     string
+			message  string
+			problems testutils.Problems
+		}{
+			{"Present", "message WriteBookResponse {}", testutils.Problems{}},
+			{"Absent", "", testutils.Problems{{Message: "WriteBookResponse"}}},
+		} {
+			t.Run(test.name, func(t *testing.T) {
+				files := testutils.ParseProtoStrings(t, map[string]string{
+					"imported.proto": strings.ReplaceAll(`
+						syntax = "proto3";
+						message WriteBookRequest {}
+						---
+						message OperationMetadata {}
+					`, "---", test.message),
+					"test.proto": `
+						syntax = "proto3";
+						import "google/longrunning/operations.proto";
+						import "imported.proto";
+						service Library {
+							rpc WriteBook(WriteBookRequest) returns (google.longrunning.Operation) {
+								option (google.longrunning.operation_info) = {
+									response_type: "WriteBookResponse"
+									metadata_type: "OperationMetadata"
+								};
+							}
+						}
+					`,
+				})
+				problems := lroResponseReachable.Lint(files["test.proto"])
+				method := files["test.proto"].GetServices()[0].GetMethods()[0]
+				if diff := test.problems.SetDescriptor(method).Diff(problems); diff != "" {
+					t.Errorf(diff)
+				}
+			})
+		}
+	})
+}
diff --git a/rules/aip0151/lro_types_defined_in_file_test.go b/rules/aip0151/lro_types_defined_in_file_test.go
deleted file mode 100644
index 4b5f9d3..0000000
--- a/rules/aip0151/lro_types_defined_in_file_test.go
+++ /dev/null
@@ -1,59 +0,0 @@
-// Copyright 2019 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 aip0151
-
-import (
-	"testing"
-
-	"github.com/googleapis/api-linter/rules/internal/testutils"
-)
-
-func TestDefinedInFile(t *testing.T) {
-	tests := []struct {
-		testName     string
-		Package      string
-		ResponseType string
-		problems     testutils.Problems
-	}{
-		{"Valid", "", "WriteBookResponse", testutils.Problems{}},
-		{"ValidPkg", "package test;", "WriteBookResponse", testutils.Problems{}},
-		{"InvalidTypo", "", "WriteBookRepsonse", testutils.Problems{{Message: "should be defined"}}},
-		{"InvalidTypoPkg", "package test;", "WriteBookRepsonse", testutils.Problems{{Message: "should be defined"}}},
-		{"ValidExternal", "", "google.protobuf.Empty", testutils.Problems{}},
-		{"ValidExternalPkg", "package test;", "google.protobuf.Empty", testutils.Problems{}},
-	}
-	for _, test := range tests {
-		f := testutils.ParseProto3Tmpl(t, `
-			{{.Package}}
-			import "google/longrunning/operations.proto";
-			service Library {
-				rpc WriteBook(WriteBookRequest) returns (google.longrunning.Operation) {
-					option (google.longrunning.operation_info) = {
-						response_type: "{{.ResponseType}}"
-						metadata_type: "OperationMetadata"
-					};
-				}
-			}
-			message WriteBookRequest {}
-			message WriteBookResponse {}
-			message OperationMetadata {}
-		`, test)
-		problems := lroDefinedInFile.Lint(f)
-		m := f.GetServices()[0].GetMethods()[0]
-		if diff := test.problems.SetDescriptor(m).Diff(problems); diff != "" {
-			t.Errorf(diff)
-		}
-	}
-}
diff --git a/rules/internal/testutils/parse.go b/rules/internal/testutils/parse.go
index bf1edd0..03bcca8 100644
--- a/rules/internal/testutils/parse.go
+++ b/rules/internal/testutils/parse.go
@@ -32,44 +32,45 @@ import (
 	_ "google.golang.org/genproto/googleapis/longrunning"
 )
 
-// ParseProtoString parses a string representing a proto file, and returns
-// a FileDescriptor.
+// ParseProtoStrings parses a map representing a proto files, and returns
+// a slice of FileDescriptors.
 //
 // It dedents the string before parsing.
-// It is unable to handle imports, and calls t.Fatalf if there is any error.
-func ParseProtoString(t *testing.T, src string, filename string) *desc.FileDescriptor {
-	// Make a map of filenames and file contents.
-	// We hard-code "test.proto"; we do not care what the filename is.
-	//
-	// Include the common protos here too, so our proto may safely import them
-	// if needed.
-	fileContents := map[string]string{
-		filename: strings.TrimSpace(dedent.Dedent(src)),
+func ParseProtoStrings(t *testing.T, src map[string]string) map[string]*desc.FileDescriptor {
+	filenames := []string{}
+	for k, v := range src {
+		filenames = append(filenames, k)
+		src[k] = strings.TrimSpace(dedent.Dedent(v))
 	}
 
 	// Parse the file.
 	parser := protoparse.Parser{
-		Accessor:              protoparse.FileContentsFromMap(fileContents),
+		Accessor:              protoparse.FileContentsFromMap(src),
 		IncludeSourceCodeInfo: true,
 		LookupImport:          desc.LoadFileDescriptor,
 	}
-	fds, err := parser.ParseFiles("test.proto")
+	fds, err := parser.ParseFiles(filenames...)
 	if err != nil {
 		t.Fatalf("%v", err)
 	}
-	return fds[0]
+	answer := map[string]*desc.FileDescriptor{}
+	for _, fd := range fds {
+		answer[fd.GetName()] = fd
+	}
+	return answer
 }
 
 // ParseProto3String parses a string representing a proto file, and returns
 // a FileDescriptor.
 //
-// It adds the `syntax = "proto3";` line to the beginning of the file
-// before calling ParseProtoString.
+// It adds the `syntax = "proto3";` line to the beginning of the file and
+// chooses a filename, and then calls ParseProtoStrings.
 func ParseProto3String(t *testing.T, src string) *desc.FileDescriptor {
-	return ParseProtoString(t, fmt.Sprintf(
-		"syntax = \"proto3\";\n\n%s",
-		strings.TrimSpace(dedent.Dedent(src)),
-	), "test.proto")
+	return ParseProtoStrings(t, map[string]string{
+		"test.proto": fmt.Sprintf(
+			"syntax = \"proto3\";\n\n%s",
+			strings.TrimSpace(dedent.Dedent(src)),
+		)})["test.proto"]
 }
 
 // ParseProto3Tmpl parses a template string representing a proto file, and
diff --git a/rules/internal/testutils/parse_test.go b/rules/internal/testutils/parse_test.go
index b5755d6..c5d43a7 100644
--- a/rules/internal/testutils/parse_test.go
+++ b/rules/internal/testutils/parse_test.go
@@ -21,8 +21,8 @@ import (
 	"github.com/jhump/protoreflect/desc"
 )
 
-func TestParseProtoString(t *testing.T) {
-	fd := ParseProtoString(t, `
+func TestParseProtoStrings(t *testing.T) {
+	fd := ParseProtoStrings(t, map[string]string{"test.proto": `
 		syntax = "proto3";
 
 		import "google/protobuf/timestamp.proto";
@@ -36,7 +36,7 @@ func TestParseProtoString(t *testing.T) {
 			string eggs = 2;
 			google.protobuf.Timestamp create_time = 3;
 		}
-	`, "test.proto")
+	`})["test.proto"]
 	if !fd.IsProto3() {
 		t.Errorf("Expected a proto3 file descriptor.")
 	}
@@ -69,11 +69,11 @@ func TestParseProtoStringError(t *testing.T) {
 	wg.Add(1)
 	go func() {
 		defer wg.Done()
-		ParseProtoString(canary, `
+		ParseProtoStrings(canary, map[string]string{"test.proto": `
 			syntax = "proto3";
 			message Foo {}
 			The quick brown fox jumped over the lazy dogs.
-		`, "test.proto")
+		`})
 	}()
 	wg.Wait()