add remediation info to scorecard output. switch to using strings.Bui…

…lder for more maintainable logic
ossf · Jul 19, 2022 · d2df929 · d2df929
1 parent e56e743
commit d2df929
Show file tree

Hide file tree

Showing 2 changed files with 161 additions and 9 deletions.
diff --git a/pkg/common.go b/pkg/common.go
@@ -32,16 +32,24 @@ func DetailToString(d *checker.CheckDetail, logLevel log.Level) string {
 		return ""
 	}
 
-	switch {
-	case d.Msg.Path != "" && d.Msg.Offset != 0 && d.Msg.EndOffset != 0 && d.Msg.Offset < d.Msg.EndOffset:
-		return fmt.Sprintf("%s: %s: %s:%d-%d", typeToString(d.Type), d.Msg.Text, d.Msg.Path, d.Msg.Offset, d.Msg.EndOffset)
-	case d.Msg.Path != "" && d.Msg.Offset != 0:
-		return fmt.Sprintf("%s: %s: %s:%d", typeToString(d.Type), d.Msg.Text, d.Msg.Path, d.Msg.Offset)
-	case d.Msg.Path != "" && d.Msg.Offset == 0:
-		return fmt.Sprintf("%s: %s: %s", typeToString(d.Type), d.Msg.Text, d.Msg.Path)
-	default:
-		return fmt.Sprintf("%s: %s", typeToString(d.Type), d.Msg.Text)
+	var sb strings.Builder
+	sb.WriteString(fmt.Sprintf("%s: %s", typeToString(d.Type), d.Msg.Text))
+
+	if d.Msg.Path != "" {
+		sb.WriteString(fmt.Sprintf(": %s", d.Msg.Path))
+		if d.Msg.Offset != 0 {
+			sb.WriteString(fmt.Sprintf(":%d", d.Msg.Offset))
+		}
+		if d.Msg.EndOffset != 0 && d.Msg.Offset < d.Msg.EndOffset {
+			sb.WriteString(fmt.Sprintf("-%d", d.Msg.EndOffset))
+		}
 	}
+
+	if d.Msg.Remediation != nil {
+		sb.WriteString(fmt.Sprintf(": %s", d.Msg.Remediation.HelpText))
+	}
+
+	return sb.String()
 }
 
 func detailsToString(details []checker.CheckDetail, logLevel log.Level) (string, bool) {

diff --git a/pkg/common_test.go b/pkg/common_test.go
@@ -0,0 +1,144 @@
+// Copyright 2022 Security Scorecard Authors
+//
+// 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
+//
+//      http://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 pkg
+
+import (
+	"testing"
+
+	"github.com/ossf/scorecard/v4/checker"
+	"github.com/ossf/scorecard/v4/log"
+)
+
+func TestDetailString(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name   string
+		detail checker.CheckDetail
+		log    log.Level
+		want   string
+	}{
+		{
+			name: "ignoreDebug",
+			detail: checker.CheckDetail{
+				Msg: checker.LogMessage{
+					Text: "should not appear",
+				},
+				Type: checker.DetailDebug,
+			},
+			log:  log.DefaultLevel,
+			want: "",
+		},
+		{
+			name: "includeDebug",
+			detail: checker.CheckDetail{
+				Msg: checker.LogMessage{
+					Text: "should appear",
+				},
+				Type: checker.DetailDebug,
+			},
+			log:  log.DebugLevel,
+			want: "Debug: should appear",
+		},
+		{
+			name: "onlyType",
+			detail: checker.CheckDetail{
+				Msg: checker.LogMessage{
+					Text: "some meaningful text",
+				},
+				Type: checker.DetailWarn,
+			},
+			log:  log.DefaultLevel,
+			want: "Warn: some meaningful text",
+		},
+		{
+			name: "displayPath",
+			detail: checker.CheckDetail{
+				Msg: checker.LogMessage{
+					Text: "some meaningful text",
+					Path: "Dockerfile",
+				},
+				Type: checker.DetailWarn,
+			},
+			log:  log.DefaultLevel,
+			want: "Warn: some meaningful text: Dockerfile",
+		},
+		{
+			name: "displayStartOffset",
+			detail: checker.CheckDetail{
+				Msg: checker.LogMessage{
+					Text:   "some meaningful text",
+					Path:   "Dockerfile",
+					Offset: 1,
+				},
+				Type: checker.DetailWarn,
+			},
+			log:  log.DefaultLevel,
+			want: "Warn: some meaningful text: Dockerfile:1",
+		},
+		{
+			name: "displayEndOffset",
+			detail: checker.CheckDetail{
+				Msg: checker.LogMessage{
+					Text:      "some meaningful text",
+					Path:      "Dockerfile",
+					Offset:    1,
+					EndOffset: 7,
+				},
+				Type: checker.DetailWarn,
+			},
+			log:  log.DefaultLevel,
+			want: "Warn: some meaningful text: Dockerfile:1-7",
+		},
+		{
+			name: "ignoreInvalidEndOffset",
+			detail: checker.CheckDetail{
+				Msg: checker.LogMessage{
+					Text:      "some meaningful text",
+					Path:      "Dockerfile",
+					Offset:    3,
+					EndOffset: 2,
+				},
+				Type: checker.DetailWarn,
+			},
+			log:  log.DefaultLevel,
+			want: "Warn: some meaningful text: Dockerfile:3",
+		},
+		{
+			name: "includeRemediation",
+			detail: checker.CheckDetail{
+				Msg: checker.LogMessage{
+					Text: "some meaningful text",
+					Path: "Dockerfile",
+					Remediation: &checker.Remediation{
+						HelpText: "fix x by doing y",
+					},
+				},
+				Type: checker.DetailWarn,
+			},
+			log:  log.DefaultLevel,
+			want: "Warn: some meaningful text: Dockerfile: fix x by doing y",
+		},
+	}
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			got := DetailToString(&tt.detail, tt.log)
+			if got != tt.want {
+				t.Errorf("got %v, want %v", got, tt.want)
+			}
+		})
+	}
+}