Simplify DetailLogger interface (#1628)

Co-authored-by: Azeem Shaikh <azeems@google.com>

checker/check_runner.go

@@ -87,15 +87,18 @@ func (r *Runner) Run(ctx context.Context, c Check) CheckResult {
 		checkRequest.Dlogger = &l
 		res = c.Fn(&checkRequest)
 		if res.Error2 != nil && errors.Is(res.Error2, sce.ErrRepoUnreachable) {
-			checkRequest.Dlogger.Warn("%v", res.Error2)
+			checkRequest.Dlogger.Warn(&LogMessage{
+				Text: fmt.Sprintf("%v", res.Error2),
+			})
 			continue
 		}
 		break
 	}
 
 	// Set details.
-	res.Details2 = l.messages2
+	// TODO(#1393): Remove.
-	for _, d := range l.messages2 {
+	res.Details2 = l.Flush()
+	for _, d := range res.Details2 {
 		res.Details = append(res.Details, d.Msg.Text)
 	}
 

checker/detail_logger.go

@@ -16,13 +16,9 @@ package checker
 
 // DetailLogger logs a CheckDetail struct.
 type DetailLogger interface {
-	Info(desc string, args ...interface{})
+	Info(msg *LogMessage)
-	Warn(desc string, args ...interface{})
+	Warn(msg *LogMessage)
-	Debug(desc string, args ...interface{})
+	Debug(msg *LogMessage)
-
+	// Flush resets the logger state and returns collected logs.
-	// Functions to use for moving to SARIF format.
+	Flush() []CheckDetail
-	// UPGRADEv3: to rename.
-	Info3(msg *LogMessage)
-	Warn3(msg *LogMessage)
-	Debug3(msg *LogMessage)
 }

checker/detail_logger_impl.go

@@ -14,55 +14,36 @@
 
 package checker
 
-import (
-	"fmt"
-)
-
-// UPGRADEv2: messages2 will ultimately
-// be renamed to messages.
 type logger struct {
-	messages2 []CheckDetail
+	logs []CheckDetail
 }
 
-func (l *logger) Info(desc string, args ...interface{}) {
+func (l *logger) Info(msg *LogMessage) {
-	cd := CheckDetail{Type: DetailInfo, Msg: LogMessage{Text: fmt.Sprintf(desc, args...)}}
-	l.messages2 = append(l.messages2, cd)
-}
-
-func (l *logger) Warn(desc string, args ...interface{}) {
-	cd := CheckDetail{Type: DetailWarn, Msg: LogMessage{Text: fmt.Sprintf(desc, args...)}}
-	l.messages2 = append(l.messages2, cd)
-}
-
-func (l *logger) Debug(desc string, args ...interface{}) {
-	cd := CheckDetail{Type: DetailDebug, Msg: LogMessage{Text: fmt.Sprintf(desc, args...)}}
-	l.messages2 = append(l.messages2, cd)
-}
-
-// UPGRADEv3: to rename.
-func (l *logger) Info3(msg *LogMessage) {
 	cd := CheckDetail{
 		Type: DetailInfo,
 		Msg:  *msg,
 	}
-	cd.Msg.Version = 3
+	l.logs = append(l.logs, cd)
-	l.messages2 = append(l.messages2, cd)
 }
 
-func (l *logger) Warn3(msg *LogMessage) {
+func (l *logger) Warn(msg *LogMessage) {
 	cd := CheckDetail{
 		Type: DetailWarn,
 		Msg:  *msg,
 	}
-	cd.Msg.Version = 3
+	l.logs = append(l.logs, cd)
-	l.messages2 = append(l.messages2, cd)
 }
 
-func (l *logger) Debug3(msg *LogMessage) {
+func (l *logger) Debug(msg *LogMessage) {
 	cd := CheckDetail{
 		Type: DetailDebug,
 		Msg:  *msg,
 	}
-	cd.Msg.Version = 3
+	l.logs = append(l.logs, cd)
-	l.messages2 = append(l.messages2, cd)
+}
+
+func (l *logger) Flush() []CheckDetail {
+	ret := l.logs
+	l.logs = nil
+	return ret
 }

checks/ci_tests.go

@@ -83,8 +83,9 @@ func CITests(c *checker.CheckRequest) checker.CheckResult {
 		}
 
 		if !foundCI {
-			c.Dlogger.Debug3(&checker.LogMessage{
+			c.Dlogger.Debug(&checker.LogMessage{
-				Text: fmt.Sprintf("merged PR without CI test: %d", pr.Number),
+				Text:    fmt.Sprintf("merged PR without CI test: %d", pr.Number),
+				Version: 3,
 			})
 		}
 	}
@@ -109,11 +110,12 @@ func prHasSuccessStatus(pr *clients.PullRequest, c *checker.CheckRequest) (bool,
 			continue
 		}
 		if isTest(status.Context) || isTest(status.TargetURL) {
-			c.Dlogger.Debug3(&checker.LogMessage{
+			c.Dlogger.Debug(&checker.LogMessage{
 				Path: status.URL,
 				Type: checker.FileTypeURL,
 				Text: fmt.Sprintf("CI test found: pr: %d, context: %s", pr.Number,
 					status.Context),
+				Version: 3,
 			})
 			return true, nil
 		}
@@ -136,11 +138,12 @@ func prHasSuccessfulCheck(pr *clients.PullRequest, c *checker.CheckRequest) (boo
 			continue
 		}
 		if isTest(cr.App.Slug) {
-			c.Dlogger.Debug3(&checker.LogMessage{
+			c.Dlogger.Debug(&checker.LogMessage{
 				Path: cr.URL,
 				Type: checker.FileTypeURL,
 				Text: fmt.Sprintf("CI test found: pr: %d, context: %s", pr.Number,
 					cr.App.Slug),
+				Version: 3,
 			})
 			return true, nil
 		}

checks/contributors.go

@@ -73,8 +73,9 @@ func Contributors(c *checker.CheckRequest) checker.CheckResult {
 		names = append(names, c)
 	}
 
-	c.Dlogger.Info3(&checker.LogMessage{
+	c.Dlogger.Info(&checker.LogMessage{
-		Text: fmt.Sprintf("contributors work for: %v", strings.Join(names, ",")),
+		Text:    fmt.Sprintf("contributors work for: %v", strings.Join(names, ",")),
+		Version: 3,
 	})
 
 	reason := fmt.Sprintf("%d different companies found", len(companies))

checks/dangerous_workflow.go

@@ -298,11 +298,12 @@ func checkJobForUntrustedCodeCheckout(job *actionlint.Job, path string,
 		}
 		if strings.Contains(ref.Value.Value, "github.event.pull_request") {
 			line := fileparser.GetLineNumber(step.Pos)
-			dl.Warn3(&checker.LogMessage{
+			dl.Warn(&checker.LogMessage{
-				Path:   path,
+				Path:    path,
-				Type:   checker.FileTypeSource,
+				Type:    checker.FileTypeSource,
-				Offset: line,
+				Offset:  line,
-				Text:   fmt.Sprintf("untrusted code checkout '%v'", ref.Value.Value),
+				Text:    fmt.Sprintf("untrusted code checkout '%v'", ref.Value.Value),
+				Version: 3,
 				// TODO: set Snippet.
 			})
 			// Detected untrusted checkout.
@@ -411,11 +412,12 @@ func checkSecretInScript(script string, pos *actionlint.Pos, path string,
 		variable := strings.Trim(script[s:s+e+2], " ")
 		if strings.Contains(variable, "secrets.") {
 			line := fileparser.GetLineNumber(pos)
-			dl.Warn3(&checker.LogMessage{
+			dl.Warn(&checker.LogMessage{
-				Path:   path,
+				Path:    path,
-				Type:   checker.FileTypeSource,
+				Type:    checker.FileTypeSource,
-				Offset: line,
+				Offset:  line,
-				Text:   fmt.Sprintf("secret accessible to pull requests '%v'", variable),
+				Text:    fmt.Sprintf("secret accessible to pull requests '%v'", variable),
+				Version: 3,
 				// TODO: set Snippet.
 			})
 			pdata.workflowPattern[secretsViaPullRequests] = true
@@ -442,11 +444,12 @@ func checkVariablesInScript(script string, pos *actionlint.Pos, path string,
 		variable := script[s+3 : s+e]
 		if containsUntrustedContextPattern(variable) {
 			line := fileparser.GetLineNumber(pos)
-			dl.Warn3(&checker.LogMessage{
+			dl.Warn(&checker.LogMessage{
-				Path:   path,
+				Path:    path,
-				Type:   checker.FileTypeSource,
+				Type:    checker.FileTypeSource,
-				Offset: line,
+				Offset:  line,
-				Text:   fmt.Sprintf("script injection with untrusted input '%v'", variable),
+				Text:    fmt.Sprintf("script injection with untrusted input '%v'", variable),
+				Version: 3,
 				// TODO: set Snippet.
 			})
 			pdata.workflowPattern[scriptInjection] = true

checks/evaluation/binary_artifacts.go

@@ -34,10 +34,11 @@ func BinaryArtifacts(name string, dl checker.DetailLogger,
 
 	score := checker.MaxResultScore
 	for _, f := range r.Files {
-		dl.Warn3(&checker.LogMessage{
+		dl.Warn(&checker.LogMessage{
 			Path: f.Path, Type: checker.FileTypeBinary,
-			Offset: f.Offset,
+			Offset:  f.Offset,
-			Text:   "binary detected",
+			Text:    "binary detected",
+			Version: 3,
 		})
 		// We remove one point for each binary.
 		score--

checks/evaluation/branch_protection.go

@@ -15,6 +15,8 @@
 package evaluation
 
 import (
+	"fmt"
+
 	"github.com/ossf/scorecard/v4/checker"
 	sce "github.com/ossf/scorecard/v4/errors"
 )
@@ -60,7 +62,9 @@ func BranchProtection(name string, dl checker.DetailLogger,
 		// so it does not provide any guarantees.
 		protected := !(b.Protected != nil && !*b.Protected)
 		if !protected {
-			dl.Warn("branch protection not enabled for branch '%s'", b.Name)
+			dl.Warn(&checker.LogMessage{
+				Text: fmt.Sprintf("branch protection not enabled for branch '%s'", b.Name),
+			})
 		}
 		score.scores.basic, score.maxes.basic = basicNonAdminProtection(&b, dl)
 		score.scores.adminBasic, score.maxes.adminBasic = basicAdminProtection(&b, dl)
@@ -223,7 +227,9 @@ func info(dl checker.DetailLogger, doLogging bool, desc string, args ...interfac
 		return
 	}
 
-	dl.Info(desc, args...)
+	dl.Info(&checker.LogMessage{
+		Text: fmt.Sprintf(desc, args...),
+	})
 }
 
 func debug(dl checker.DetailLogger, doLogging bool, desc string, args ...interface{}) {
@@ -231,7 +237,9 @@ func debug(dl checker.DetailLogger, doLogging bool, desc string, args ...interfa
 		return
 	}
 
-	dl.Debug(desc, args...)
+	dl.Debug(&checker.LogMessage{
+		Text: fmt.Sprintf(desc, args...),
+	})
 }
 
 func warn(dl checker.DetailLogger, doLogging bool, desc string, args ...interface{}) {
@@ -239,7 +247,9 @@ func warn(dl checker.DetailLogger, doLogging bool, desc string, args ...interfac
 		return
 	}
 
-	dl.Warn(desc, args...)
+	dl.Warn(&checker.LogMessage{
+		Text: fmt.Sprintf(desc, args...),
+	})
 }
 
 func basicNonAdminProtection(branch *checker.BranchProtectionData, dl checker.DetailLogger) (int, int) {

checks/evaluation/code_review.go

@@ -69,8 +69,9 @@ func CodeReview(name string, dl checker.DetailLogger,
 		// Only show all warnings if all fail.
 		// We should not show warning if at least one succeeds, as this is confusing.
 		for k := range totalReviewed {
-			dl.Warn3(&checker.LogMessage{
+			dl.Warn(&checker.LogMessage{
-				Text: fmt.Sprintf("no %s reviews found", k),
+				Text:    fmt.Sprintf("no %s reviews found", k),
+				Version: 3,
 			})
 		}
 
@@ -132,9 +133,10 @@ func isReviewedOnGitHub(c *checker.DefaultBranchCommit, dl checker.DetailLogger)
 
 	for _, r := range mr.Reviews {
 		if r.State == "APPROVED" {
-			dl.Debug3(&checker.LogMessage{
+			dl.Debug(&checker.LogMessage{
 				Text: fmt.Sprintf("%s #%d merge request approved",
 					reviewPlatformGitHub, mr.Number),
+				Version: 3,
 			})
 			return true
 		}
@@ -145,9 +147,10 @@ func isReviewedOnGitHub(c *checker.DefaultBranchCommit, dl checker.DetailLogger)
 	// time on clicking the approve button.
 	if c.Committer.Login != "" &&
 		c.Committer.Login != mr.Author.Login {
-		dl.Debug3(&checker.LogMessage{
+		dl.Debug(&checker.LogMessage{
 			Text: fmt.Sprintf("%s #%d merge request approved",
 				reviewPlatformGitHub, mr.Number),
+			Version: 3,
 		})
 		return true
 	}
@@ -157,8 +160,9 @@ func isReviewedOnGitHub(c *checker.DefaultBranchCommit, dl checker.DetailLogger)
 
 func isReviewedOnProw(c *checker.DefaultBranchCommit, dl checker.DetailLogger) bool {
 	if isBot(c.Committer.Login) {
-		dl.Debug3(&checker.LogMessage{
+		dl.Debug(&checker.LogMessage{
-			Text: fmt.Sprintf("skip commit from bot account: %s", c.Committer.Login),
+			Text:    fmt.Sprintf("skip commit from bot account: %s", c.Committer.Login),
+			Version: 3,
 		})
 		return true
 	}
@@ -166,9 +170,10 @@ func isReviewedOnProw(c *checker.DefaultBranchCommit, dl checker.DetailLogger) b
 	if c.MergeRequest != nil && !c.MergeRequest.MergedAt.IsZero() {
 		for _, l := range c.MergeRequest.Labels {
 			if l == "lgtm" || l == "approved" {
-				dl.Debug3(&checker.LogMessage{
+				dl.Debug(&checker.LogMessage{
 					Text: fmt.Sprintf("%s #%d merge request approved",
 						reviewPlatformProw, c.MergeRequest.Number),
+					Version: 3,
 				})
 				return true
 			}
@@ -179,8 +184,9 @@ func isReviewedOnProw(c *checker.DefaultBranchCommit, dl checker.DetailLogger) b
 
 func isReviewedOnGerrit(c *checker.DefaultBranchCommit, dl checker.DetailLogger) bool {
 	if isBot(c.Committer.Login) {
-		dl.Debug3(&checker.LogMessage{
+		dl.Debug(&checker.LogMessage{
-			Text: fmt.Sprintf("skip commit from bot account: %s", c.Committer.Login),
+			Text:    fmt.Sprintf("skip commit from bot account: %s", c.Committer.Login),
+			Version: 3,
 		})
 		return true
 	}
@@ -188,8 +194,9 @@ func isReviewedOnGerrit(c *checker.DefaultBranchCommit, dl checker.DetailLogger)
 	m := c.CommitMessage
 	if strings.Contains(m, "\nReviewed-on: ") &&
 		strings.Contains(m, "\nReviewed-by: ") {
-		dl.Debug3(&checker.LogMessage{
+		dl.Debug(&checker.LogMessage{
-			Text: fmt.Sprintf("%s commit approved", reviewPlatformGerrit),
+			Text:    fmt.Sprintf("%s commit approved", reviewPlatformGerrit),
+			Version: 3,
 		})
 		return true
 	}

checks/evaluation/dependency_update_tool.go

@@ -31,13 +31,15 @@ func DependencyUpdateTool(name string, dl checker.DetailLogger,
 
 	// Apply the policy evaluation.
 	if r.Tools == nil || len(r.Tools) == 0 {
-		dl.Warn3(&checker.LogMessage{
+		dl.Warn(&checker.LogMessage{
 			Text: `dependabot config file not detected in source location.
 			We recommend setting this configuration in code so it can be easily verified by others.`,
+			Version: 3,
 		})
-		dl.Warn3(&checker.LogMessage{
+		dl.Warn(&checker.LogMessage{
 			Text: `renovatebot config file not detected in source location.
 			We recommend setting this configuration in code so it can be easily verified by others.`,
+			Version: 3,
 		})
 		return checker.CreateMinScoreResult(name, "no update tool detected")
 	}
@@ -56,11 +58,12 @@ func DependencyUpdateTool(name string, dl checker.DetailLogger,
 
 	// Note: only one file per tool is present,
 	// so we do not iterate thru all entries.
-	dl.Info3(&checker.LogMessage{
+	dl.Info(&checker.LogMessage{
-		Path:   r.Tools[0].ConfigFiles[0].Path,
+		Path:    r.Tools[0].ConfigFiles[0].Path,
-		Type:   r.Tools[0].ConfigFiles[0].Type,
+		Type:    r.Tools[0].ConfigFiles[0].Type,
-		Offset: r.Tools[0].ConfigFiles[0].Offset,
+		Offset:  r.Tools[0].ConfigFiles[0].Offset,
-		Text:   fmt.Sprintf("%s detected", r.Tools[0].Name),
+		Text:    fmt.Sprintf("%s detected", r.Tools[0].Name),
+		Version: 3,
 	})
 
 	// High score result.

checks/evaluation/security_policy.go

@@ -33,16 +33,17 @@ func SecurityPolicy(name string, dl checker.DetailLogger, r *checker.SecurityPol
 
 	for _, f := range r.Files {
 		msg := checker.LogMessage{
-			Path:   f.Path,
+			Path:    f.Path,
-			Type:   f.Type,
+			Type:    f.Type,
-			Offset: f.Offset,
+			Offset:  f.Offset,
+			Version: 3,
 		}
 		if msg.Type == checker.FileTypeURL {
 			msg.Text = "security policy detected in org repo"
 		} else {
 			msg.Text = "security policy detected"
 		}
-		dl.Info3(&msg)
+		dl.Info(&msg)
 	}
 
 	return checker.CreateMaxScoreResult(name, "security policy file detected")

checks/evaluation/vulnerabilities.go

@@ -42,8 +42,9 @@ func Vulnerabilities(name string, dl checker.DetailLogger,
 	}
 
 	if len(IDs) > 0 {
-		dl.Warn3(&checker.LogMessage{
+		dl.Warn(&checker.LogMessage{
-			Text: fmt.Sprintf("HEAD is vulnerable to %s", strings.Join(IDs, ", ")),
+			Text:    fmt.Sprintf("HEAD is vulnerable to %s", strings.Join(IDs, ", ")),
+			Version: 3,
 		})
 		return checker.CreateResultWithScore(name,
 			fmt.Sprintf("%v existing vulnerabilities detected", len(IDs)), score)

checks/license.go

@@ -109,10 +109,11 @@ func LicenseCheck(c *checker.CheckRequest) checker.CheckResult {
 		pdata := fileparser.FileGetCbDataAsBoolPointer(data)
 
 		if checkLicense(name) {
-			c.Dlogger.Info3(&checker.LogMessage{
+			c.Dlogger.Info(&checker.LogMessage{
-				Path:   name,
+				Path:    name,
-				Type:   checker.FileTypeSource,
+				Type:    checker.FileTypeSource,
-				Offset: 1,
+				Offset:  1,
+				Version: 3,
 			})
 			*pdata = true
 			return false, nil

checks/packaging.go

@@ -66,25 +66,28 @@ func Packaging(c *checker.CheckRequest) checker.CheckResult {
 			return checker.CreateRuntimeErrorResult(CheckPackaging, e)
 		}
 		if len(runs) > 0 {
-			c.Dlogger.Info3(&checker.LogMessage{
+			c.Dlogger.Info(&checker.LogMessage{
-				Path:   fp,
+				Path:    fp,
-				Type:   checker.FileTypeSource,
+				Type:    checker.FileTypeSource,
-				Offset: checker.OffsetDefault,
+				Offset:  checker.OffsetDefault,
-				Text:   fmt.Sprintf("GitHub publishing workflow used in run %s", runs[0].URL),
+				Text:    fmt.Sprintf("GitHub publishing workflow used in run %s", runs[0].URL),
+				Version: 3,
 			})
 			return checker.CreateMaxScoreResult(CheckPackaging,
 				"publishing workflow detected")
 		}
-		c.Dlogger.Debug3(&checker.LogMessage{
+		c.Dlogger.Debug(&checker.LogMessage{
-			Path:   fp,
+			Path:    fp,
-			Type:   checker.FileTypeSource,
+			Type:    checker.FileTypeSource,
-			Offset: checker.OffsetDefault,
+			Offset:  checker.OffsetDefault,
-			Text:   "GitHub publishing workflow not used in runs",
+			Text:    "GitHub publishing workflow not used in runs",
+			Version: 3,
 		})
 	}
 
-	c.Dlogger.Warn3(&checker.LogMessage{
+	c.Dlogger.Warn(&checker.LogMessage{
-		Text: "no GitHub publishing workflow detected",
+		Text:    "no GitHub publishing workflow detected",
+		Version: 3,
 	})
 
 	return checker.CreateInconclusiveResult(CheckPackaging,
@@ -207,21 +210,23 @@ func isPackagingWorkflow(workflow *actionlint.Workflow, fp string, dl checker.De
 				continue
 			}
 
-			dl.Info3(&checker.LogMessage{
+			dl.Info(&checker.LogMessage{
-				Path:   fp,
+				Path:    fp,
-				Type:   checker.FileTypeSource,
+				Type:    checker.FileTypeSource,
-				Offset: fileparser.GetLineNumber(job.Pos),
+				Offset:  fileparser.GetLineNumber(job.Pos),
-				Text:   matcher.LogText,
+				Text:    matcher.LogText,
+				Version: 3,
 			})
 			return true
 		}
 	}
 
-	dl.Debug3(&checker.LogMessage{
+	dl.Debug(&checker.LogMessage{
-		Path:   fp,
+		Path:    fp,
-		Type:   checker.FileTypeSource,
+		Type:    checker.FileTypeSource,
-		Offset: checker.OffsetDefault,
+		Offset:  checker.OffsetDefault,
-		Text:   "not a publishing workflow",
+		Text:    "not a publishing workflow",
+		Version: 3,
 	})
 	return false
 }

checks/permissions.go

@@ -97,33 +97,36 @@ func validatePermission(permissionKey permission, permissionValue *actionlint.Pe
 	lineNumber := fileparser.GetLineNumber(permissionValue.Value.Pos)
 	if strings.EqualFold(val, "write") {
 		if isPermissionOfInterest(permissionKey, ignoredPermissions) {
-			dl.Warn3(&checker.LogMessage{
+			dl.Warn(&checker.LogMessage{
-				Path:   path,
+				Path:    path,
-				Type:   checker.FileTypeSource,
+				Type:    checker.FileTypeSource,
-				Offset: lineNumber,
+				Offset:  lineNumber,
-				Text:   fmt.Sprintf("%s '%v' permission set to '%v'", permLevel, permissionKey, val),
+				Text:    fmt.Sprintf("%s '%v' permission set to '%v'", permLevel, permissionKey, val),
+				Version: 3,
 				// TODO: set Snippet.
 			})
 			recordPermissionWrite(pPermissions, permissionKey)
 		} else {
 			// Only log for debugging, otherwise
 			// it may confuse users.
-			dl.Debug3(&checker.LogMessage{
+			dl.Debug(&checker.LogMessage{
-				Path:   path,
+				Path:    path,
-				Type:   checker.FileTypeSource,
+				Type:    checker.FileTypeSource,
-				Offset: lineNumber,
+				Offset:  lineNumber,
-				Text:   fmt.Sprintf("%s '%v' permission set to '%v'", permLevel, permissionKey, val),
+				Text:    fmt.Sprintf("%s '%v' permission set to '%v'", permLevel, permissionKey, val),
+				Version: 3,
 				// TODO: set Snippet.
 			})
 		}
 		return nil
 	}
 
-	dl.Info3(&checker.LogMessage{
+	dl.Info(&checker.LogMessage{
-		Path:   path,
+		Path:    path,
-		Type:   checker.FileTypeSource,
+		Type:    checker.FileTypeSource,
-		Offset: lineNumber,
+		Offset:  lineNumber,
-		Text:   fmt.Sprintf("%s '%v' permission set to '%v'", permLevel, permissionKey, val),
+		Text:    fmt.Sprintf("%s '%v' permission set to '%v'", permLevel, permissionKey, val),
+		Version: 3,
 		// TODO: set Snippet.
 	})
 	return nil
@@ -170,33 +173,36 @@ func validatePermissions(permissions *actionlint.Permissions, permLevel, path st
 	allIsSet := permissions != nil && permissions.All != nil && permissions.All.Value != ""
 	scopeIsSet := permissions != nil && len(permissions.Scopes) > 0
 	if permissions == nil || (!allIsSet && !scopeIsSet) {
-		dl.Info3(&checker.LogMessage{
+		dl.Info(&checker.LogMessage{
-			Path:   path,
+			Path:    path,
-			Type:   checker.FileTypeSource,
+			Type:    checker.FileTypeSource,
-			Offset: checker.OffsetDefault,
+			Offset:  checker.OffsetDefault,
-			Text:   fmt.Sprintf("%s permissions set to 'none'", permLevel),
+			Text:    fmt.Sprintf("%s permissions set to 'none'", permLevel),
+			Version: 3,
 		})
 	}
 	if allIsSet {
 		val := permissions.All.Value
 		lineNumber := fileparser.GetLineNumber(permissions.All.Pos)
 		if !strings.EqualFold(val, "read-all") && val != "" {
-			dl.Warn3(&checker.LogMessage{
+			dl.Warn(&checker.LogMessage{
-				Path:   path,
+				Path:    path,
-				Type:   checker.FileTypeSource,
+				Type:    checker.FileTypeSource,
-				Offset: lineNumber,
+				Offset:  lineNumber,
-				Text:   fmt.Sprintf("%s permissions set to '%v'", permLevel, val),
+				Text:    fmt.Sprintf("%s permissions set to '%v'", permLevel, val),
+				Version: 3,
 				// TODO: set Snippet.
 			})
 			recordAllPermissionsWrite(pdata, permLevel, path)
 			return nil
 		}
 
-		dl.Info3(&checker.LogMessage{
+		dl.Info(&checker.LogMessage{
-			Path:   path,
+			Path:    path,
-			Type:   checker.FileTypeSource,
+			Type:    checker.FileTypeSource,
-			Offset: lineNumber,
+			Offset:  lineNumber,
-			Text:   fmt.Sprintf("%s permissions set to '%v'", permLevel, val),
+			Text:    fmt.Sprintf("%s permissions set to '%v'", permLevel, val),
+			Version: 3,
 			// TODO: set Snippet.
 		})
 	} else /* scopeIsSet == true */ if err := validateMapPermissions(permissions.Scopes,
@@ -210,11 +216,12 @@ func validateTopLevelPermissions(workflow *actionlint.Workflow, path string,
 	dl checker.DetailLogger, pdata *permissionCbData) error {
 	// Check if permissions are set explicitly.
 	if workflow.Permissions == nil {
-		dl.Warn3(&checker.LogMessage{
+		dl.Warn(&checker.LogMessage{
-			Path:   path,
+			Path:    path,
-			Type:   checker.FileTypeSource,
+			Type:    checker.FileTypeSource,
-			Offset: checker.OffsetDefault,
+			Offset:  checker.OffsetDefault,
-			Text:   fmt.Sprintf("no %s permission defined", topLevelPermission),
+			Text:    fmt.Sprintf("no %s permission defined", topLevelPermission),
+			Version: 3,
 		})
 		recordAllPermissionsWrite(pdata, topLevelPermission, path)
 		return nil
@@ -232,11 +239,12 @@ func validatejobLevelPermissions(workflow *actionlint.Workflow, path string,
 		// For most workflows, no write permissions are needed,
 		// so only top-level read-only permissions need to be declared.
 		if job.Permissions == nil {
-			dl.Debug3(&checker.LogMessage{
+			dl.Debug(&checker.LogMessage{
-				Path:   path,
+				Path:    path,
-				Type:   checker.FileTypeSource,
+				Type:    checker.FileTypeSource,
-				Offset: fileparser.GetLineNumber(job.Pos),
+				Offset:  fileparser.GetLineNumber(job.Pos),
-				Text:   fmt.Sprintf("no %s permission defined", jobLevelPermission),
+				Text:    fmt.Sprintf("no %s permission defined", jobLevelPermission),
+				Version: 3,
 			})
 			recordAllPermissionsWrite(pdata, jobLevelPermission, path)
 			continue
@@ -472,22 +480,24 @@ func isSARIFUploadAction(workflow *actionlint.Workflow, fp string, dl checker.De
 				continue
 			}
 			if strings.HasPrefix(uses.Value, "github/codeql-action/upload-sarif@") {
-				dl.Debug3(&checker.LogMessage{
+				dl.Debug(&checker.LogMessage{
-					Path:   fp,
+					Path:    fp,
-					Type:   checker.FileTypeSource,
+					Type:    checker.FileTypeSource,
-					Offset: fileparser.GetLineNumber(uses.Pos),
+					Offset:  fileparser.GetLineNumber(uses.Pos),
-					Text:   "codeql SARIF upload workflow detected",
+					Text:    "codeql SARIF upload workflow detected",
+					Version: 3,
 					// TODO: set Snippet.
 				})
 				return true
 			}
 		}
 	}
-	dl.Debug3(&checker.LogMessage{
+	dl.Debug(&checker.LogMessage{
-		Path:   fp,
+		Path:    fp,
-		Type:   checker.FileTypeSource,
+		Type:    checker.FileTypeSource,
-		Offset: checker.OffsetDefault,
+		Offset:  checker.OffsetDefault,
-		Text:   "not a codeql upload SARIF workflow",
+		Text:    "not a codeql upload SARIF workflow",
+		Version: 3,
 	})
 	return false
 }
@@ -504,22 +514,24 @@ func isCodeQlAnalysisWorkflow(workflow *actionlint.Workflow, fp string, dl check
 				continue
 			}
 			if strings.HasPrefix(uses.Value, "github/codeql-action/analyze@") {
-				dl.Debug3(&checker.LogMessage{
+				dl.Debug(&checker.LogMessage{
-					Path:   fp,
+					Path:    fp,
-					Type:   checker.FileTypeSource,
+					Type:    checker.FileTypeSource,
-					Offset: fileparser.GetLineNumber(uses.Pos),
+					Offset:  fileparser.GetLineNumber(uses.Pos),
-					Text:   "codeql workflow detected",
+					Text:    "codeql workflow detected",
+					Version: 3,
 					// TODO: set Snippet.
 				})
 				return true
 			}
 		}
 	}
-	dl.Debug3(&checker.LogMessage{
+	dl.Debug(&checker.LogMessage{
-		Path:   fp,
+		Path:    fp,
-		Type:   checker.FileTypeSource,
+		Type:    checker.FileTypeSource,
-		Offset: checker.OffsetDefault,
+		Offset:  checker.OffsetDefault,
-		Text:   "not a codeql workflow",
+		Text:    "not a codeql workflow",
+		Version: 3,
 	})
 	return false
 }

checks/pinned_dependencies.go

@@ -161,19 +161,21 @@ func createReturnValuesForGitHubActionsWorkflowPinned(r worklowPinningResult, in
 
 	if r.gitHubOwned != notPinned {
 		score += 2
-		dl.Info3(&checker.LogMessage{
+		dl.Info(&checker.LogMessage{
-			Type:   checker.FileTypeSource,
+			Type:    checker.FileTypeSource,
-			Offset: checker.OffsetDefault,
+			Offset:  checker.OffsetDefault,
-			Text:   fmt.Sprintf("%s %s", "GitHub-owned", infoMsg),
+			Text:    fmt.Sprintf("%s %s", "GitHub-owned", infoMsg),
+			Version: 3,
 		})
 	}
 
 	if r.thirdParties != notPinned {
 		score += 8
-		dl.Info3(&checker.LogMessage{
+		dl.Info(&checker.LogMessage{
-			Type:   checker.FileTypeSource,
+			Type:    checker.FileTypeSource,
-			Offset: checker.OffsetDefault,
+			Offset:  checker.OffsetDefault,
-			Text:   fmt.Sprintf("%s %s", "Third-party", infoMsg),
+			Text:    fmt.Sprintf("%s %s", "Third-party", infoMsg),
+			Version: 3,
 		})
 	}
 
@@ -198,7 +200,9 @@ func createReturnValues(r pinnedResult, infoMsg string, dl checker.DetailLogger,
 	default:
 		panic("invalid value")
 	case pinned, pinnedUndefined:
-		dl.Info(infoMsg)
+		dl.Info(&checker.LogMessage{
+			Text: infoMsg,
+		})
 		return checker.MaxResultScore, nil
 	case notPinned:
 		// No logging needed as it's done by the checks.
@@ -429,13 +433,14 @@ func validateDockerfileIsPinned(pathfn string, content []byte,
 
 			// Not pinned.
 			ret = false
-			dl.Warn3(&checker.LogMessage{
+			dl.Warn(&checker.LogMessage{
 				Path:      pathfn,
 				Type:      checker.FileTypeSource,
 				Offset:    uint(child.StartLine),
 				EndOffset: uint(child.EndLine),
 				Text:      "docker image not pinned by hash",
 				Snippet:   child.Original,
+				Version:   3,
 			})
 
 		// FROM name.
@@ -444,13 +449,14 @@ func validateDockerfileIsPinned(pathfn string, content []byte,
 			pinned := pinnedAsNames[name]
 			if !pinned && !regex.MatchString(name) {
 				ret = false
-				dl.Warn3(&checker.LogMessage{
+				dl.Warn(&checker.LogMessage{
 					Path:      pathfn,
 					Type:      checker.FileTypeSource,
 					Offset:    uint(child.StartLine),
 					EndOffset: uint(child.EndLine),
 					Text:      "docker image not pinned by hash",
 					Snippet:   child.Original,
+					Version:   3,
 				})
 			}
 
@@ -654,12 +660,13 @@ func validateGitHubActionWorkflow(pathfn string, content []byte,
 			// Example: action-name@hash
 			match := hashRegex.MatchString(execAction.Uses.Value)
 			if !match {
-				dl.Warn3(&checker.LogMessage{
+				dl.Warn(&checker.LogMessage{
 					Path: pathfn, Type: checker.FileTypeSource,
 					Offset:    uint(execAction.Uses.Pos.Line),
 					EndOffset: uint(execAction.Uses.Pos.Line), // `Uses` always span a single line.
 					Snippet:   execAction.Uses.Value,
 					Text:      fmt.Sprintf("%s action not pinned by hash", owner),
+					Version:   3,
 				})
 			}
 

checks/sast.go

@@ -141,10 +141,11 @@ func sastToolInCheckRuns(c *checker.CheckRequest) (int, error) {
 				continue
 			}
 			if sastTools[cr.App.Slug] {
-				c.Dlogger.Debug3(&checker.LogMessage{
+				c.Dlogger.Debug(&checker.LogMessage{
-					Path: cr.URL,
+					Path:    cr.URL,
-					Type: checker.FileTypeURL,
+					Type:    checker.FileTypeURL,
-					Text: "tool detected",
+					Text:    "tool detected",
+					Version: 3,
 				})
 				totalTested++
 				break
@@ -152,19 +153,22 @@ func sastToolInCheckRuns(c *checker.CheckRequest) (int, error) {
 		}
 	}
 	if totalMerged == 0 {
-		c.Dlogger.Warn3(&checker.LogMessage{
+		c.Dlogger.Warn(&checker.LogMessage{
-			Text: "no pull requests merged into dev branch",
+			Text:    "no pull requests merged into dev branch",
+			Version: 3,
 		})
 		return checker.InconclusiveResultScore, nil
 	}
 
 	if totalTested == totalMerged {
-		c.Dlogger.Info3(&checker.LogMessage{
+		c.Dlogger.Info(&checker.LogMessage{
-			Text: fmt.Sprintf("all commits (%v) are checked with a SAST tool", totalMerged),
+			Text:    fmt.Sprintf("all commits (%v) are checked with a SAST tool", totalMerged),
+			Version: 3,
 		})
 	} else {
-		c.Dlogger.Warn3(&checker.LogMessage{
+		c.Dlogger.Warn(&checker.LogMessage{
-			Text: fmt.Sprintf("%v commits out of %v are checked with a SAST tool", totalTested, totalMerged),
+			Text:    fmt.Sprintf("%v commits out of %v are checked with a SAST tool", totalTested, totalMerged),
+			Version: 3,
 		})
 	}
 
@@ -184,25 +188,28 @@ func codeQLInCheckDefinitions(c *checker.CheckRequest) (int, error) {
 	}
 
 	for _, result := range resp.Results {
-		c.Dlogger.Debug3(&checker.LogMessage{
+		c.Dlogger.Debug(&checker.LogMessage{
-			Path:   result.Path,
+			Path:    result.Path,
-			Type:   checker.FileTypeSource,
+			Type:    checker.FileTypeSource,
-			Offset: checker.OffsetDefault,
+			Offset:  checker.OffsetDefault,
-			Text:   "CodeQL detected",
+			Text:    "CodeQL detected",
+			Version: 3,
 		})
 	}
 
 	// TODO: check if it's enabled as cron or presubmit.
 	// TODO: check which branches it is enabled on. We should find main.
 	if resp.Hits > 0 {
-		c.Dlogger.Info3(&checker.LogMessage{
+		c.Dlogger.Info(&checker.LogMessage{
-			Text: "SAST tool detected: CodeQL",
+			Text:    "SAST tool detected: CodeQL",
+			Version: 3,
 		})
 		return checker.MaxResultScore, nil
 	}
 
-	c.Dlogger.Warn3(&checker.LogMessage{
+	c.Dlogger.Warn(&checker.LogMessage{
-		Text: "CodeQL tool not detected",
+		Text:    "CodeQL tool not detected",
+		Version: 3,
 	})
 	return checker.MinResultScore, nil
 }

checks/shell_download_validate.go

@@ -324,13 +324,14 @@ func isFetchPipeExecute(startLine, endLine uint, node syntax.Node, cmd, pathfn s
 	}
 
 	startLine, endLine = getLine(startLine, endLine, node)
-	dl.Warn3(&checker.LogMessage{
+	dl.Warn(&checker.LogMessage{
 		Path:      pathfn,
 		Type:      checker.FileTypeSource,
 		Offset:    startLine,
 		EndOffset: endLine,
 		Snippet:   cmd,
 		Text:      "insecure (not pinned by hash) download detected",
+		Version:   3,
 	})
 	return true
 }
@@ -372,13 +373,14 @@ func isExecuteFiles(startLine, endLine uint, node syntax.Node, cmd, pathfn strin
 	startLine, endLine = getLine(startLine, endLine, node)
 	for fn := range files {
 		if isInterpreterWithFile(c, fn) || isExecuteFile(c, fn) {
-			dl.Warn3(&checker.LogMessage{
+			dl.Warn(&checker.LogMessage{
 				Path:      pathfn,
 				Type:      checker.FileTypeSource,
 				Offset:    startLine,
 				EndOffset: endLine,
 				Snippet:   cmd,
 				Text:      "insecure (not pinned by hash) download-then-run",
+				Version:   3,
 			})
 			ok = true
 		}
@@ -589,39 +591,42 @@ func isUnpinnedPakageManagerDownload(startLine, endLine uint, node syntax.Node,
 
 	// Go get/install.
 	if isGoUnpinnedDownload(c) {
-		dl.Warn3(&checker.LogMessage{
+		dl.Warn(&checker.LogMessage{
 			Path:      pathfn,
 			Type:      checker.FileTypeSource,
 			Offset:    startLine,
 			EndOffset: endLine,
 			Snippet:   cmd,
 			Text:      "go installation not pinned by hash",
+			Version:   3,
 		})
 		return true
 	}
 
 	// Pip install.
 	if isPipUnpinnedDownload(c) {
-		dl.Warn3(&checker.LogMessage{
+		dl.Warn(&checker.LogMessage{
 			Path:      pathfn,
 			Type:      checker.FileTypeSource,
 			Offset:    startLine,
 			EndOffset: endLine,
 			Snippet:   cmd,
 			Text:      "pip installation not pinned by hash",
+			Version:   3,
 		})
 		return true
 	}
 
 	// Npm install.
 	if isNpmUnpinnedDownload(c) {
-		dl.Warn3(&checker.LogMessage{
+		dl.Warn(&checker.LogMessage{
 			Path:      pathfn,
 			Type:      checker.FileTypeSource,
 			Offset:    startLine,
 			EndOffset: endLine,
 			Snippet:   cmd,
 			Text:      "npm installation not pinned by hash",
+			Version:   3,
 		})
 		return true
 	}
@@ -707,13 +712,14 @@ func isFetchProcSubsExecute(startLine, endLine uint, node syntax.Node, cmd, path
 
 	startLine, endLine = getLine(startLine, endLine, node)
 
-	dl.Warn3(&checker.LogMessage{
+	dl.Warn(&checker.LogMessage{
 		Path:      pathfn,
 		Type:      checker.FileTypeSource,
 		Offset:    startLine,
 		EndOffset: endLine,
 		Snippet:   cmd,
 		Text:      "insecure (not pinned by hash) download-then-run",
+		Version:   3,
 	})
 	return true
 }
@@ -946,7 +952,9 @@ func validateShellFile(pathfn string, startLine, endLine uint,
 	r, err := validateShellFileAndRecord(pathfn, startLine, endLine, content, taintedFiles, dl)
 	if err != nil && errors.Is(err, sce.ErrorShellParsing) {
 		// Discard and print this particular error for now.
-		dl.Debug(err.Error())
+		dl.Debug(&checker.LogMessage{
+			Text: err.Error(),
+		})
 		err = nil
 	}
 	return r, err

checks/signed_releases.go

@@ -52,18 +52,20 @@ func SignedReleases(c *checker.CheckRequest) checker.CheckResult {
 		if len(r.Assets) == 0 {
 			continue
 		}
-		c.Dlogger.Debug3(&checker.LogMessage{
+		c.Dlogger.Debug(&checker.LogMessage{
-			Text: fmt.Sprintf("GitHub release found: %s", r.TagName),
+			Text:    fmt.Sprintf("GitHub release found: %s", r.TagName),
+			Version: 3,
 		})
 		totalReleases++
 		signed := false
 		for _, asset := range r.Assets {
 			for _, suffix := range artifactExtensions {
 				if strings.HasSuffix(asset.Name, suffix) {
-					c.Dlogger.Info3(&checker.LogMessage{
+					c.Dlogger.Info(&checker.LogMessage{
-						Path: asset.URL,
+						Path:    asset.URL,
-						Type: checker.FileTypeURL,
+						Type:    checker.FileTypeURL,
-						Text: fmt.Sprintf("signed release artifact: %s", asset.Name),
+						Text:    fmt.Sprintf("signed release artifact: %s", asset.Name),
+						Version: 3,
 					})
 					signed = true
 					break
@@ -75,10 +77,11 @@ func SignedReleases(c *checker.CheckRequest) checker.CheckResult {
 			}
 		}
 		if !signed {
-			c.Dlogger.Warn3(&checker.LogMessage{
+			c.Dlogger.Warn(&checker.LogMessage{
-				Path: r.URL,
+				Path:    r.URL,
-				Type: checker.FileTypeURL,
+				Type:    checker.FileTypeURL,
-				Text: fmt.Sprintf("release artifact %s not signed", r.TagName),
+				Text:    fmt.Sprintf("release artifact %s not signed", r.TagName),
+				Version: 3,
 			})
 		}
 		if totalReleases >= releaseLookBack {
@@ -87,8 +90,9 @@ func SignedReleases(c *checker.CheckRequest) checker.CheckResult {
 	}
 
 	if totalReleases == 0 {
-		c.Dlogger.Warn3(&checker.LogMessage{
+		c.Dlogger.Warn(&checker.LogMessage{
-			Text: "no GitHub releases found",
+			Text:    "no GitHub releases found",
+			Version: 3,
 		})
 		// Generic summary.
 		return checker.CreateInconclusiveResult(CheckSignedReleases, "no releases found")

pkg/common.go

@@ -28,7 +28,7 @@ func textToMarkdown(s string) string {
 
 // DetailToString turns a detail information into a string.
 func DetailToString(d *checker.CheckDetail, logLevel log.Level) string {
-	// UPGRADEv3: remove switch statement.
+	// TODO(#1393): remove switch statement.
 	switch d.Msg.Version {
 	case 3:
 		if d.Type == checker.DetailDebug && logLevel != log.DebugLevel {

utests/utlib.go

@@ -41,27 +41,7 @@ type TestDetailLogger struct {
 }
 
 // Info implements DetailLogger.Info.
-func (l *TestDetailLogger) Info(desc string, args ...interface{}) {
+func (l *TestDetailLogger) Info(msg *checker.LogMessage) {
-	cd := checker.CheckDetail{Type: checker.DetailInfo, Msg: checker.LogMessage{Text: fmt.Sprintf(desc, args...)}}
-	l.messages = append(l.messages, cd)
-}
-
-// Warn implements DetailLogger.Warn.
-func (l *TestDetailLogger) Warn(desc string, args ...interface{}) {
-	cd := checker.CheckDetail{Type: checker.DetailWarn, Msg: checker.LogMessage{Text: fmt.Sprintf(desc, args...)}}
-	l.messages = append(l.messages, cd)
-}
-
-// Debug implements DetailLogger.Debug.
-func (l *TestDetailLogger) Debug(desc string, args ...interface{}) {
-	cd := checker.CheckDetail{Type: checker.DetailDebug, Msg: checker.LogMessage{Text: fmt.Sprintf(desc, args...)}}
-	l.messages = append(l.messages, cd)
-}
-
-// UPGRADEv3: to rename.
-
-// Info3 implements DetailLogger.Info3.
-func (l *TestDetailLogger) Info3(msg *checker.LogMessage) {
 	cd := checker.CheckDetail{
 		Type: checker.DetailInfo,
 		Msg:  *msg,
@@ -70,8 +50,8 @@ func (l *TestDetailLogger) Info3(msg *checker.LogMessage) {
 	l.messages = append(l.messages, cd)
 }
 
-// Warn3 implements DetailLogger.Warn3.
+// Warn implements DetailLogger.Warn.
-func (l *TestDetailLogger) Warn3(msg *checker.LogMessage) {
+func (l *TestDetailLogger) Warn(msg *checker.LogMessage) {
 	cd := checker.CheckDetail{
 		Type: checker.DetailWarn,
 		Msg:  *msg,
@@ -80,8 +60,8 @@ func (l *TestDetailLogger) Warn3(msg *checker.LogMessage) {
 	l.messages = append(l.messages, cd)
 }
 
-// Debug3 implements DetailLogger.Debug3.
+// Debug implements DetailLogger.Debug.
-func (l *TestDetailLogger) Debug3(msg *checker.LogMessage) {
+func (l *TestDetailLogger) Debug(msg *checker.LogMessage) {
 	cd := checker.CheckDetail{
 		Type: checker.DetailDebug,
 		Msg:  *msg,
@@ -90,6 +70,13 @@ func (l *TestDetailLogger) Debug3(msg *checker.LogMessage) {
 	l.messages = append(l.messages, cd)
 }
 
+// Flush implements DetailLogger.Flush.
+func (l *TestDetailLogger) Flush() []checker.CheckDetail {
+	ret := l.messages
+	l.messages = nil
+	return ret
+}
+
 func getTestReturn(cr *checker.CheckResult, logger *TestDetailLogger) (*TestReturn, error) {
 	ret := new(TestReturn)
 	for _, v := range logger.messages {