feat: refined the message display for some checks

salsadigitalauorg · Feb 14, 2024 · f6495cf · f6495cf
1 parent 649c4b8
commit f6495cf
Show file tree

Hide file tree

Showing 6 changed files with 33 additions and 28 deletions.
diff --git a/pkg/checks/file/filecheck.go b/pkg/checks/file/filecheck.go
@@ -2,7 +2,6 @@ package file
 
 import (
 	"path/filepath"
-	"strings"
 
 	"github.com/salsadigitalauorg/shipshape/pkg/config"
 	"github.com/salsadigitalauorg/shipshape/pkg/result"
@@ -64,8 +63,8 @@ func (c *FileCheck) RunCheck() {
 		c.AddPass("No illegal files")
 		return
 	}
-	c.AddBreach(&result.ValueBreach{
-		ValueLabel: "illegal files found",
-		Value:      strings.Join(files, ","),
+	c.AddBreach(&result.KeyValuesBreach{
+		Key:    "illegal files found",
+		Values: files,
 	})
 }
diff --git a/pkg/checks/file/filecheck_test.go b/pkg/checks/file/filecheck_test.go
@@ -81,13 +81,16 @@ func TestFileCheckRunCheck(t *testing.T) {
 	assert.Equal(0, len(c.Result.Passes))
 	assert.EqualValues(
 		[]result.Breach{
-			&result.ValueBreach{
-				BreachType: "value",
+			&result.KeyValuesBreach{
+				BreachType: "key-values",
 				CheckType:  "file",
 				CheckName:  "filecheck2",
 				Severity:   "normal",
-				ValueLabel: "illegal files found",
-				Value:      "testdata/adminer.php,testdata/sub/phpmyadmin.php",
+				Key:        "illegal files found",
+				Values: []string{
+					"testdata/adminer.php",
+					"testdata/sub/phpmyadmin.php",
+				},
 			},
 		},
 		c.Result.Breaches,

diff --git a/pkg/checks/phpstan/phpstan.go b/pkg/checks/phpstan/phpstan.go
@@ -190,18 +190,19 @@ func (c *PhpStanCheck) RunCheck() {
 	for file, errors := range c.phpstanResult.Files {
 		errLines := []string{}
 		for _, er := range errors.Messages {
-			errLines = append(errLines, fmt.Sprintf("line %d: %s", er.Line, er.Message))
-
+			errLines = append(errLines,
+				fmt.Sprintf("line %d: %s", er.Line,
+					strings.ReplaceAll(er.Message, "\n", "")))
 		}
-		c.AddBreach(&result.KeyValueBreach{
-			Key:   fmt.Sprintf("file contains banned functions: %s", file),
-			Value: strings.Join(errLines, "\n"),
+		c.AddBreach(&result.KeyValuesBreach{
+			Key:    fmt.Sprintf("file: %s", file),
+			Values: errLines,
 		})
 	}
 
 	if len(c.phpstanResult.Errors) > 0 {
-		c.AddBreach(&result.ValueBreach{
-			ValueLabel: "errors encountered when running phpstan",
-			Value:      strings.Join(c.phpstanResult.Errors, "\n")})
+		c.AddBreach(&result.KeyValuesBreach{
+			Key:    "errors encountered when running phpstan",
+			Values: c.phpstanResult.Errors})
 	}
 }
diff --git a/pkg/checks/phpstan/phpstan_test.go b/pkg/checks/phpstan/phpstan_test.go
@@ -268,10 +268,10 @@ func TestRunCheck(t *testing.T) {
 	c.Result.DetermineResultStatus(false)
 	assert.Equal(result.Fail, c.Result.Status)
 	assert.EqualValues(
-		[]result.Breach{&result.KeyValueBreach{
-			BreachType: "key-value",
-			Key:        "file contains banned functions: /app/web/themes/custom/custom/test-theme/info.php",
-			Value:      "line 3: Calling curl_exec() is forbidden, please change the code",
+		[]result.Breach{&result.KeyValuesBreach{
+			BreachType: "key-values",
+			Key:        "file: /app/web/themes/custom/custom/test-theme/info.php",
+			Values:     []string{"line 3: Calling curl_exec() is forbidden, please change the code"},
 		}},
 		c.Result.Breaches,
 	)
@@ -289,10 +289,10 @@ func TestRunCheck(t *testing.T) {
 	c.Result.DetermineResultStatus(false)
 	assert.Equal(result.Fail, c.Result.Status)
 	assert.EqualValues(
-		[]result.Breach{&result.ValueBreach{
-			BreachType: "value",
-			ValueLabel: "errors encountered when running phpstan",
-			Value:      "Error found in file foo",
+		[]result.Breach{&result.KeyValuesBreach{
+			BreachType: "key-values",
+			Key:        "errors encountered when running phpstan",
+			Values:     []string{"Error found in file foo"},
 		}},
 		c.Result.Breaches,
 	)

diff --git a/pkg/result/breach.go b/pkg/result/breach.go
@@ -102,10 +102,10 @@ type KeyValuesBreach struct {
 
 func (b KeyValuesBreach) String() string {
 	if b.KeyLabel != "" && b.ValueLabel != "" {
-		return fmt.Sprintf("[%s:%s] %s: %s", b.KeyLabel, b.Key, b.ValueLabel,
-			"["+strings.Join(b.Values, ", ")+"]")
+		return fmt.Sprintf("[%s:%s] %s:\n        - %s", b.KeyLabel, b.Key, b.ValueLabel,
+			strings.Join(b.Values, "\n        - "))
 	}
-	return fmt.Sprintf("%s: %s", b.Key, "["+strings.Join(b.Values, ", ")+"]")
+	return fmt.Sprintf("%s:\n        - %s", b.Key, strings.Join(b.Values, "\n        - "))
 }
 
 func BreachGetKeyLabel(bIfc Breach) string {

diff --git a/pkg/result/breach_test.go b/pkg/result/breach_test.go
@@ -86,7 +86,9 @@ func TestBreachKeyValuesBreachStringers(t *testing.T) {
 				ValueLabel: "disallowed permissions",
 				Values:     []string{"delete the site", "delete the world"},
 			},
-			expected: "[role:admin] disallowed permissions: [delete the site, delete the world]",
+			expected: `[role:admin] disallowed permissions:
+        - delete the site
+        - delete the world`,
 		},
 	}