Repository: googleapis/api-linter
Pull request #370 (2019-11-21 01:11:48)

[refactor] Make locations.pathLocation better.
This refactors `locations.pathLocation` to take the baseline
descriptor and the path suffix. This significantly reduces
the amount of work being done in the public methods in the
package.


Patch:
diff --git a/locations/descriptor_locations.go b/locations/descriptor_locations.go
index 0112857..3ec2462 100644
--- a/locations/descriptor_locations.go
+++ b/locations/descriptor_locations.go
@@ -23,10 +23,6 @@ import (
 //
 // This works for any descriptor, regardless of type (message, field, etc.).
 func DescriptorName(d desc.Descriptor) *dpb.SourceCodeInfo_Location {
-	if sourceInfo := d.GetSourceInfo(); sourceInfo != nil {
-		// All descriptors seem to have `string name = 1`, so this conveniently works.
-		path := append(sourceInfo.Path, 1)
-		return pathLocation(d.GetFile(), path)
-	}
-	return nil
+	// All descriptors seem to have `string name = 1`, so this conveniently works.
+	return pathLocation(d, 1)
 }
diff --git a/locations/field_locations.go b/locations/field_locations.go
index 482d617..f6e4a16 100644
--- a/locations/field_locations.go
+++ b/locations/field_locations.go
@@ -21,14 +21,8 @@ import (
 
 // FieldType returns the precise location for a field's type.
 func FieldType(f *desc.FieldDescriptor) *dpb.SourceCodeInfo_Location {
-	if sourceInfo := f.GetSourceInfo(); sourceInfo != nil {
-		var path []int32
-		if f.GetMessageType() != nil || f.GetEnumType() != nil {
-			path = append(sourceInfo.Path, 6) // type_name
-		} else {
-			path = append(sourceInfo.Path, 5) // type
-		}
-		return pathLocation(f.GetFile(), path)
+	if f.GetMessageType() != nil || f.GetEnumType() != nil {
+		return pathLocation(f, 6) // FieldDescriptor.type_name == 6
 	}
-	return nil
+	return pathLocation(f, 5) // FieldDescriptor.type == 5
 }
diff --git a/locations/file_locations.go b/locations/file_locations.go
index 94f4bd7..6813005 100644
--- a/locations/file_locations.go
+++ b/locations/file_locations.go
@@ -24,7 +24,7 @@ import (
 // If the location can not be found (for example, because there is no syntax
 // statement), it returns nil.
 func FileSyntax(f *desc.FileDescriptor) *dpb.SourceCodeInfo_Location {
-	return pathLocation(f, []int32{12}) // syntax == 12
+	return pathLocation(f, 12) // FileDescriptor.syntax == 12
 }
 
 // FilePackage returns the location of the package definition in a file descriptor.
@@ -32,5 +32,5 @@ func FileSyntax(f *desc.FileDescriptor) *dpb.SourceCodeInfo_Location {
 // If the location can not be found (for example, because there is no package
 // statement), it returns nil.
 func FilePackage(f *desc.FileDescriptor) *dpb.SourceCodeInfo_Location {
-	return pathLocation(f, []int32{2}) // package == 2
+	return pathLocation(f, 2) // FileDescriptor.package == 2
 }
diff --git a/locations/locations.go b/locations/locations.go
index aa00fb5..a68d513 100644
--- a/locations/locations.go
+++ b/locations/locations.go
@@ -27,9 +27,14 @@ import (
 	"github.com/jhump/protoreflect/desc"
 )
 
-// pathLocation returns the precise location within a file for a given path.
-func pathLocation(f *desc.FileDescriptor, path []int32) *dpb.SourceCodeInfo_Location {
-	return sourceInfoRegistry.sourceInfo(f).findLocation(path)
+// pathLocation returns the precise location for a given descriptor and path.
+// It combines the path of the descriptor itself with any path provided appended.
+func pathLocation(d desc.Descriptor, path ...int) *dpb.SourceCodeInfo_Location {
+	fullPath := d.GetSourceInfo().GetPath()
+	for _, i := range path {
+		fullPath = append(fullPath, int32(i))
+	}
+	return sourceInfoRegistry.sourceInfo(d.GetFile()).findLocation(fullPath)
 }
 
 type sourceInfo map[string]*dpb.SourceCodeInfo_Location
diff --git a/locations/method_locations.go b/locations/method_locations.go
index fa07e70..6505734 100644
--- a/locations/method_locations.go
+++ b/locations/method_locations.go
@@ -22,28 +22,16 @@ import (
 
 // MethodRequestType returns the precise location of the method's input type.
 func MethodRequestType(m *desc.MethodDescriptor) *dpb.SourceCodeInfo_Location {
-	if sourceInfo := m.GetSourceInfo(); sourceInfo != nil {
-		path := append(sourceInfo.Path, 2) // input_type == 2
-		return pathLocation(m.GetFile(), path)
-	}
-	return nil
+	return pathLocation(m, 2) // MethodDecriptor.input_type == 2
 }
 
 // MethodResponseType returns the precise location of the method's output type.
 func MethodResponseType(m *desc.MethodDescriptor) *dpb.SourceCodeInfo_Location {
-	if sourceInfo := m.GetSourceInfo(); sourceInfo != nil {
-		path := append(m.GetSourceInfo().Path, 3) // output_type == 3
-		return pathLocation(m.GetFile(), path)
-	}
-	return nil
+	return pathLocation(m, 3) // MethodDescriptor.output_type == 3
 }
 
 // MethodHTTPRule returns the precise location of the method's `google.api.http`
 // rule, if any.
 func MethodHTTPRule(m *desc.MethodDescriptor) *dpb.SourceCodeInfo_Location {
-	if sourceInfo := m.GetSourceInfo(); sourceInfo != nil {
-		path := append(m.GetSourceInfo().Path, 4, apb.E_Http.Field) // options == 4
-		return pathLocation(m.GetFile(), path)
-	}
-	return nil
+	return pathLocation(m, 4, int(apb.E_Http.Field)) // MethodDescriptor.options == 4
 }


Test patch:
diff --git a/locations/file_locations_test.go b/locations/file_locations_test.go
index 2d7c768..f3f559b 100644
--- a/locations/file_locations_test.go
+++ b/locations/file_locations_test.go
@@ -58,13 +58,13 @@ func TestLocations(t *testing.T) {
 	t.Run("Bogus", func(t *testing.T) {
 		tests := []struct {
 			testName string
-			path     []int32
+			path     []int
 		}{
-			{"NotFound", []int32{6, 0}},
+			{"NotFound", []int{6, 0}},
 		}
 		for _, test := range tests {
 			t.Run(test.testName, func(t *testing.T) {
-				if loc := pathLocation(f, test.path); loc != nil {
+				if loc := pathLocation(f, test.path...); loc != nil {
 					t.Errorf("%v", loc)
 				}
 			})