diff --git a/cmd/api-linter/github_actions.go b/cmd/api-linter/github_actions.go index 712f1b109..34087769d 100644 --- a/cmd/api-linter/github_actions.go +++ b/cmd/api-linter/github_actions.go @@ -20,6 +20,7 @@ import ( "strings" "github.com/googleapis/api-linter/v2/lint" + dpb "google.golang.org/protobuf/types/descriptorpb" ) // formatGitHubActionOutput returns lint errors in GitHub actions format. @@ -32,23 +33,10 @@ 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]) - } + start, end := fileLocationFromPBLocation(problem.Location) + fmt.Fprintf(&buf, ",line=%d,endLine=%d,col=%d,endColumn=%d", start.Line, end.Line, start.Column, end.Column) } // GitHub uses :: as control characters (which are also used to delimit @@ -56,10 +44,10 @@ func formatGitHubActionOutput(responses []lint.Response) []byte { // with two Armenian full stops which are indistinguishable to my eye. runeThatLooksLikeTwoColonsButIsActuallyTwoArmenianFullStops := "։։" title := strings.ReplaceAll(string(problem.RuleID), "::", runeThatLooksLikeTwoColonsButIsActuallyTwoArmenianFullStops) - message := strings.ReplaceAll(problem.Message, "\n", "\\n") + message := strings.ReplaceAll(problem.Message, "\n", "%0A") uri := problem.GetRuleURI() if uri != "" { - message += "\\n\\n" + uri + message += "%0A%0A" + uri } fmt.Fprintf(&buf, ",title=%s::%s\n", title, message) } @@ -67,3 +55,29 @@ func formatGitHubActionOutput(responses []lint.Response) []byte { return buf.Bytes() } + +type position struct { + Line int + Column int +} + +// Implementation copied from lint/problem.go +func fileLocationFromPBLocation(l *dpb.SourceCodeInfo_Location) (start, end position) { + start = position{ + Line: int(l.Span[0]) + 1, + Column: int(l.Span[1]) + 1, + } + + if len(l.Span) == 4 { + end = position{ + Line: int(l.Span[2]) + 1, + Column: int(l.Span[3]), + } + } else { + end = position{ + Line: int(l.Span[0]) + 1, + Column: int(l.Span[2]), + } + } + return start, end +} diff --git a/cmd/api-linter/github_actions_test.go b/cmd/api-linter/github_actions_test.go index 48e43b9d7..5a46b2906 100644 --- a/cmd/api-linter/github_actions_test.go +++ b/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 `, }, { @@ -90,7 +74,7 @@ func TestFormatGitHubActionOutput(t *testing.T) { }, { RuleID: "core::naming_formats::field_names", - Message: "multi\nline\ncomment", + Message: "multi%0Aline%0Acomment", Location: &descriptorpb.SourceCodeInfo_Location{ Span: []int32{5, 6, 7, 8}, }, @@ -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::%0A%0Ahttps://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%0Aline%0Acomment%0A%0Ahttps://linter.aip.dev/naming_formats/field_names `, }, { @@ -133,13 +117,13 @@ func TestFormatGitHubActionOutput(t *testing.T) { }, }, }, - want: `::error file=example.proto,title=core։։naming_formats։։field_names::\n\nhttps://linter.aip.dev/naming_formats/field_names -::error file=example.proto,title=core։։naming_formats։։field_names::\n\nhttps://linter.aip.dev/naming_formats/field_names -::error file=example2.proto,title=core։։0131։։request_message։։name::\n\nhttps://linter.aip.dev/131/request_message/name -::error file=example2.proto,title=core։։0132։։response_message։։name::\n\nhttps://linter.aip.dev/132/response_message/name -::error file=example3.proto,title=core։։naming_formats։։field_names::\n\nhttps://linter.aip.dev/naming_formats/field_names -::error file=example4.proto,title=core։։naming_formats։։field_names::\n\nhttps://linter.aip.dev/naming_formats/field_names -::error file=example4.proto,title=core։։0132։։response_message։։name::\n\nhttps://linter.aip.dev/132/response_message/name + want: `::error file=example.proto,title=core։։naming_formats։։field_names::%0A%0Ahttps://linter.aip.dev/naming_formats/field_names +::error file=example.proto,title=core։։naming_formats։։field_names::%0A%0Ahttps://linter.aip.dev/naming_formats/field_names +::error file=example2.proto,title=core։։0131։։request_message։։name::%0A%0Ahttps://linter.aip.dev/131/request_message/name +::error file=example2.proto,title=core։։0132։։response_message։։name::%0A%0Ahttps://linter.aip.dev/132/response_message/name +::error file=example3.proto,title=core։։naming_formats։։field_names::%0A%0Ahttps://linter.aip.dev/naming_formats/field_names +::error file=example4.proto,title=core։։naming_formats։։field_names::%0A%0Ahttps://linter.aip.dev/naming_formats/field_names +::error file=example4.proto,title=core։։0132։։response_message։։name::%0A%0Ahttps://linter.aip.dev/132/response_message/name `, }, }