GitHub Action output format generates incorrect line numbers

[jsocol]

When using the --output-format github switch, the resulting can place the Span values into the wrong places, e.g.

[710, 32, 40]

produces

::error file=users/users.proto,endLine=40,col=32,line=710,title=core...

I believe the issue comes from this switch in the formatter code:

				// 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])
				}

Which does not treat the two different span lengths the same way as the JSON/YAML marshaling code

	// If `span` has four ints; they correspond to
	// [start line, start column, end line, end column].
	//
	// 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 {
		return fileLocation{
			Start: position{
				Line:   int(span[0]) + 1,
				Column: int(span[1]) + 1,
			},
			End: position{
				Line:   int(span[2]) + 1,
				Column: int(span[3]),
			},
		}
	}

	// Okay, `span` has three ints; they correspond to
	// [start line, start column, end column].
	//
	// 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.
	return fileLocation{
		Start: position{
			Line:   int(span[0]) + 1,
			Column: int(span[1]) + 1,
		},
		End: position{
			Line:   int(span[0]) + 1,
			Column: int(span[2]),
		},
	}

I think a focused fix for the formatter might look like

				startLine := problem.Location.Span[0] + 1
				startColumn := problem.Location.Span[1] + 1
				if len(problem.Location.Span) == 4 {
					fmt.Fprintf(&buf, ",line=%d,col=%d,endColumn=%d,endLine=%d", startLine, startColumn, problem.Location.Span[3], problem.Location.Span[2] + 1)
				} else {
					fmt.Fprintf(&buf, "line=%d,col=%d,endColumn=%d", startLine, startColumn, problem.Location.Span[2] + 1)
				}

I think it would correctly parse the Span data.

Obviously, though, it's somewhat awkward, and requires repeating the logic for converting the zero-indexed numbers from the protobuf location. A bigger refactor might look something taking the private structs in lint/problem.go for file locations, like fileLocation and position and exporting them, and some method that can use fileLocationFromPBLocation to enable relying the same logic.

Environment details

• Programming language: go
• OS: macos and ubuntu
• Language runtime version: go 1.19.5
• Package version: 1.39.5

Steps to reproduce

Using this test file:

syntax = "proto3";

package test;
option go_package = "test.code/test";

service TestService {
  rpc GetTest(GetTestRequest) returns (Test);
}

message GetTestRequest {
  string url = 1;
}

message Test {
  string url = 1;
}

1. Run the linter api-linter test.proto, observe the final complaint in the output has the following location

    location:
      start_position:
        line_number: 3
        column_number: 1
      end_position:
        line_number: 3
        column_number: 13
   

2. Run the linter with api-linter --output-format github test.proto, and compare:

   ::error file=test.proto,endLine=13,col=0,line=2,title=core։։0215։։versioned-packages::API components should be in versioned packages.\n\nhttps://linter.aip.dev/215/versioned-package