fix: interpret span correctly for github format

Reuse location logic from the lint package

The old location logic had two bugs
- keeping locations zero-indexed
- interpreting end column as end line in case of span size 3

Author: SoMuchForSubtlety

cmd/api-linter/github_actions.go

@@ -32,23 +32,13 @@ func formatGitHubActionOutput(responses []lint.Response) []byte {
 			// https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-an-error-message
 
 			fmt.Fprintf(&buf, "::error file=%s", response.FilePath)
+
 			if problem.Location != nil {
-				// Some findings are *line level* and only have start positions but no
-				// starting column. Construct a switch fallthrough to emit as many of
-				// the location indicators are included.
-				switch len(problem.Location.Span) {
-				case 4:
-					fmt.Fprintf(&buf, ",endColumn=%d", problem.Location.Span[3])
-					fallthrough
-				case 3:
-					fmt.Fprintf(&buf, ",endLine=%d", problem.Location.Span[2])
-					fallthrough
-				case 2:
-					fmt.Fprintf(&buf, ",col=%d", problem.Location.Span[1])
-					fallthrough
-				case 1:
-					fmt.Fprintf(&buf, ",line=%d", problem.Location.Span[0])
-				}
+				loc := lint.FileLocationFromPBLocation(problem.Location, nil)
+				fmt.Fprintf(&buf, ",line=%d", loc.Start.Line)
+				fmt.Fprintf(&buf, ",endLine=%d", loc.End.Line)
+				fmt.Fprintf(&buf, ",col=%d", loc.Start.Column)
+				fmt.Fprintf(&buf, ",endColumn=%d", loc.End.Column)
 			}
 
 			// GitHub uses :: as control characters (which are also used to delimit

cmd/api-linter/github_actions_test.go

@@ -53,27 +53,11 @@ func TestFormatGitHubActionOutput(t *testing.T) {
 								Span: []int32{5, 6, 7},
 							},
 						},
-						{
-							RuleID:  "line::col",
-							Message: "Line and column",
-							Location: &descriptorpb.SourceCodeInfo_Location{
-								Span: []int32{5, 6},
-							},
-						},
-						{
-							RuleID:  "line",
-							Message: "Line only",
-							Location: &descriptorpb.SourceCodeInfo_Location{
-								Span: []int32{5},
-							},
-						},
 					},
 				},
 			},
-			want: `::error file=example.proto,endColumn=8,endLine=7,col=6,line=5,title=line։։col։։endLine։։endColumn::line, column, endline, and endColumn
-::error file=example.proto,endLine=7,col=6,line=5,title=line։։col։։endLine::Line, column, and endline
-::error file=example.proto,col=6,line=5,title=line։։col::Line and column
-::error file=example.proto,line=5,title=line::Line only
+			want: `::error file=example.proto,line=6,endLine=8,col=7,endColumn=8,title=line։։col։։endLine։։endColumn::line, column, endline, and endColumn
+::error file=example.proto,line=6,endLine=6,col=7,endColumn=7,title=line։։col։։endLine::Line, column, and endline
 `,
 		},
 		{
@@ -98,8 +82,8 @@ func TestFormatGitHubActionOutput(t *testing.T) {
 					},
 				},
 			},
-			want: `::error file=example.proto,endColumn=4,endLine=3,col=2,line=1,title=core։։naming_formats։։field_names::\n\nhttps://linter.aip.dev/naming_formats/field_names
-::error file=example.proto,endColumn=8,endLine=7,col=6,line=5,title=core։։naming_formats։։field_names::multi\nline\ncomment\n\nhttps://linter.aip.dev/naming_formats/field_names
+			want: `::error file=example.proto,line=2,endLine=4,col=3,endColumn=4,title=core։։naming_formats։։field_names::\n\nhttps://linter.aip.dev/naming_formats/field_names
+::error file=example.proto,line=6,endLine=8,col=7,endColumn=8,title=core։։naming_formats։։field_names::multi\nline\ncomment\n\nhttps://linter.aip.dev/naming_formats/field_names
 `,
 		},
 		{

lint/problem.go

@@ -76,36 +76,36 @@ func (p Problem) MarshalYAML() (interface{}, error) {
 
 // Marshal defines how to represent a serialized Problem.
 func (p Problem) marshal() interface{} {
-	var fl fileLocation
+	var fl FileLocation
 	if p.Location != nil {
 		// If Location is set, use it.
-		fl = fileLocationFromPBLocation(p.Location, p.Descriptor)
+		fl = FileLocationFromPBLocation(p.Location, p.Descriptor)
 	} else if p.Descriptor != nil {
 		// Otherwise, use the descriptor's location.
 		// This is the protobuf-go idiomatic way to get the source location.
 		// Note: ParentFile() called on a FileDescriptor returns itself.
 		loc := p.Descriptor.ParentFile().SourceLocations().ByDescriptor(p.Descriptor)
-		fl = fileLocation{
+		fl = FileLocation{
 			Path: p.Descriptor.ParentFile().Path(),
-			Start: position{
+			Start: Position{
 				Line:   loc.StartLine + 1,
 				Column: loc.StartColumn + 1,
 			},
-			End: position{
+			End: Position{
 				Line:   loc.EndLine + 1,
 				Column: loc.EndColumn,
 			},
 		}
 	} else {
 		// Default location if no descriptor.
-		fl = fileLocationFromPBLocation(nil, nil)
+		fl = FileLocationFromPBLocation(nil, nil)
 	}
 
 	// Return a marshal-able structure.
 	return struct {
 		Message    string       `json:"message" yaml:"message"`
 		Suggestion string       `json:"suggestion,omitempty" yaml:"suggestion,omitempty"`
-		Location   fileLocation `json:"location" yaml:"location"`
+		Location   FileLocation `json:"location" yaml:"location"`
 		RuleID     RuleName     `json:"rule_id" yaml:"rule_id"`
 		RuleDocURI string       `json:"rule_doc_uri" yaml:"rule_doc_uri"`
 		Category   string       `json:"category,omitempty" yaml:"category,omitempty"`
@@ -124,35 +124,35 @@ func (p Problem) GetRuleURI() string {
 	return getRuleURL(string(p.RuleID), ruleURLMappings)
 }
 
-// position describes a one-based position in a source code file.
+// Position describes a one-based Position in a source code file.
 // They are one-indexed, as a human counts lines or columns.
-type position struct {
+type Position struct {
 	Line   int `json:"line_number" yaml:"line_number"`
 	Column int `json:"column_number" yaml:"column_number"`
 }
 
-// fileLocation describes a location in a source code file.
+// FileLocation describes a location in a source code file.
 //
 // Note: Positions are one-indexed, as a human counts lines or columns
 // in a file.
-type fileLocation struct {
-	Start position `json:"start_position" yaml:"start_position"`
-	End   position `json:"end_position" yaml:"end_position"`
+type FileLocation struct {
+	Start Position `json:"start_position" yaml:"start_position"`
+	End   Position `json:"end_position" yaml:"end_position"`
 	Path  string   `json:"path" yaml:"path"`
 }
 
-// fileLocationFromPBLocation returns a new fileLocation object based on a
+// FileLocationFromPBLocation returns a new fileLocation object based on a
 // protocol buffer SourceCodeInfo_Location
-func fileLocationFromPBLocation(l *dpb.SourceCodeInfo_Location, d protoreflect.Descriptor) fileLocation {
+func FileLocationFromPBLocation(l *dpb.SourceCodeInfo_Location, d protoreflect.Descriptor) FileLocation {
 	// Spans are guaranteed by protobuf to have either three or four ints.
 	span := []int32{0, 0, 1}
 	if l != nil {
 		span = l.Span
 	}
 
-	var fl fileLocation
+	var fl FileLocation
 	if d != nil {
-		fl = fileLocation{Path: d.ParentFile().Path()}
+		fl = FileLocation{Path: d.ParentFile().Path()}
 	}
 
 	// If `span` has four ints; they correspond to
@@ -161,11 +161,11 @@ func fileLocationFromPBLocation(l *dpb.SourceCodeInfo_Location, d protoreflect.D
 	// We add one because spans are zero-indexed, but not to the end column
 	// because we want the ending position to be inclusive and not exclusive.
 	if len(span) == 4 {
-		fl.Start = position{
+		fl.Start = Position{
 			Line:   int(span[0]) + 1,
 			Column: int(span[1]) + 1,
 		}
-		fl.End = position{
+		fl.End = Position{
 			Line:   int(span[2]) + 1,
 			Column: int(span[3]),
 		}
@@ -177,11 +177,11 @@ func fileLocationFromPBLocation(l *dpb.SourceCodeInfo_Location, d protoreflect.D
 	//
 	// We add one because spans are zero-indexed, but not to the end column
 	// because we want the ending position to be inclusive and not exclusive.
-	fl.Start = position{
+	fl.Start = Position{
 		Line:   int(span[0]) + 1,
 		Column: int(span[1]) + 1,
 	}
-	fl.End = position{
+	fl.End = Position{
 		Line:   int(span[0]) + 1,
 		Column: int(span[2]),
 	}