use helper to test for boolean values

Signed-off-by: Adam Korczynski <[email protected]>

probes/blocksDeleteOnBranches/impl.go

@@ -53,7 +53,6 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
 			text = fmt.Sprintf("'allow deletion' disabled on branch '%s'", *branch.Name)
 			outcome = finding.OutcomePositive
 		default:
-			//foo
 		}
 		f, err := finding.NewWith(fs, Probe, text, nil, outcome)
 		if err != nil {

probes/branchProtectionAppliesToAdmins/impl.go

@@ -21,6 +21,7 @@ import (
 
 	"github.com/ossf/scorecard/v4/checker"
 	"github.com/ossf/scorecard/v4/finding"
+	"github.com/ossf/scorecard/v4/probes/internal/utils/branchprotection"
 	"github.com/ossf/scorecard/v4/probes/internal/utils/uerror"
 )
 
@@ -39,43 +40,24 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
 
 	for i := range r.Branches {
 		branch := &r.Branches[i]
-		if branch.BranchProtectionRule.EnforceAdmins != nil {
-			switch *branch.BranchProtectionRule.EnforceAdmins {
-			case true:
-				f, err := finding.NewWith(fs, Probe,
-					fmt.Sprintf("settings apply to administrators on branch '%s'", *branch.Name),
-					nil, finding.OutcomePositive)
-				if err != nil {
-					return nil, Probe, fmt.Errorf("create finding: %w", err)
-				}
-				f = f.WithValues(map[string]int{
-					*branch.Name: 1,
-				})
-				findings = append(findings, *f)
-			case false:
-				f, err := finding.NewWith(fs, Probe,
-					fmt.Sprintf("settings do not apply to administrators on branch '%s'", *branch.Name),
-					nil, finding.OutcomeNegative)
-				if err != nil {
-					return nil, Probe, fmt.Errorf("create finding: %w", err)
-				}
-				f = f.WithValues(map[string]int{
-					*branch.Name: 1,
-				})
-				findings = append(findings, *f)
-			}
-		} else {
-			f, err := finding.NewWith(fs, Probe,
-				fmt.Sprintf("unable to retrieve whether or not settings apply to administrators on branch '%s'", *branch.Name),
-				nil, finding.OutcomeNotAvailable)
-			if err != nil {
-				return nil, Probe, fmt.Errorf("create finding: %w", err)
-			}
-			f = f.WithValues(map[string]int{
-				*branch.Name: 1,
-			})
-			findings = append(findings, *f)
+		nilMsg := fmt.Sprintf("unable to retrieve whether or not settings apply to administrators on branch '%s'",
+			*branch.Name)
+		trueMsg := fmt.Sprintf("settings apply to administrators on branch '%s'", *branch.Name)
+		falseMsg := fmt.Sprintf("'allow deletion' disabled on branch '%s'", *branch.Name)
+
+		p := branch.BranchProtectionRule.EnforceAdmins
+		text, outcome, err := branchprotection.GetTextOutcomeFromBool(p, nilMsg, trueMsg, falseMsg)
+		if err != nil {
+			return nil, Probe, fmt.Errorf("create finding: %w", err)
 		}
+		f, err := finding.NewWith(fs, Probe, text, nil, outcome)
+		if err != nil {
+			return nil, Probe, fmt.Errorf("create finding: %w", err)
+		}
+		f = f.WithValues(map[string]int{
+			*branch.Name: 1,
+		})
+		findings = append(findings, *f)
 	}
 	return findings, Probe, nil
 }

probes/dismissesStaleReviews/impl.go

@@ -21,6 +21,7 @@ import (
 
 	"github.com/ossf/scorecard/v4/checker"
 	"github.com/ossf/scorecard/v4/finding"
+	"github.com/ossf/scorecard/v4/probes/internal/utils/branchprotection"
 	"github.com/ossf/scorecard/v4/probes/internal/utils/uerror"
 )
 
@@ -39,43 +40,23 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
 
 	for i := range r.Branches {
 		branch := &r.Branches[i]
-		if branch.BranchProtectionRule.RequiredPullRequestReviews.DismissStaleReviews != nil {
-			switch *branch.BranchProtectionRule.RequiredPullRequestReviews.DismissStaleReviews {
-			case true:
-				f, err := finding.NewWith(fs, Probe,
-					fmt.Sprintf("stale review dismissal enabled on branch '%s'", *branch.Name),
-					nil, finding.OutcomePositive)
-				if err != nil {
-					return nil, Probe, fmt.Errorf("create finding: %w", err)
-				}
-				f = f.WithValues(map[string]int{
-					*branch.Name: 1,
-				})
-				findings = append(findings, *f)
-			case false:
-				f, err := finding.NewWith(fs, Probe,
-					fmt.Sprintf("stale review dismissal disabled on branch '%s'", *branch.Name),
-					nil, finding.OutcomeNegative)
-				if err != nil {
-					return nil, Probe, fmt.Errorf("create finding: %w", err)
-				}
-				f = f.WithValues(map[string]int{
-					*branch.Name: 1,
-				})
-				findings = append(findings, *f)
-			}
-		} else {
-			f, err := finding.NewWith(fs, Probe,
-				fmt.Sprintf("unable to retrieve review dismissal on branch '%s'", *branch.Name),
-				nil, finding.OutcomeNotAvailable)
-			if err != nil {
-				return nil, Probe, fmt.Errorf("create finding: %w", err)
-			}
-			f = f.WithValues(map[string]int{
-				*branch.Name: 1,
-			})
-			findings = append(findings, *f)
+		nilMsg := fmt.Sprintf("unable to retrieve review dismissal on branch '%s'", *branch.Name)
+		trueMsg := fmt.Sprintf("stale review dismissal enabled on branch '%s'", *branch.Name)
+		falseMsg := fmt.Sprintf("stale review dismissal disabled on branch '%s'", *branch.Name)
+
+		p := branch.BranchProtectionRule.RequiredPullRequestReviews.DismissStaleReviews
+		text, outcome, err := branchprotection.GetTextOutcomeFromBool(p, nilMsg, trueMsg, falseMsg)
+		if err != nil {
+			return nil, Probe, fmt.Errorf("create finding: %w", err)
 		}
+		f, err := finding.NewWith(fs, Probe, text, nil, outcome)
+		if err != nil {
+			return nil, Probe, fmt.Errorf("create finding: %w", err)
+		}
+		f = f.WithValues(map[string]int{
+			*branch.Name: 1,
+		})
+		findings = append(findings, *f)
 	}
 	return findings, Probe, nil
 }

probes/internal/utils/branchprotection/branchProtection.go

@@ -0,0 +1,47 @@
+// Copyright 2023 OpenSSF 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 branchprotection
+
+import (
+	"errors"
+
+	"github.com/ossf/scorecard/v4/finding"
+)
+
+var errWrongValue = errors.New("wrong value, should not happen")
+
+func GetTextOutcomeFromBool(b *bool, nilMsg, trueMsg, falseMsg string) (string, finding.Outcome, error) {
+	switch {
+	case b == nil:
+		return nilMsg, finding.OutcomeNotAvailable, nil
+	case *b:
+		return trueMsg, finding.OutcomePositive, nil
+	case !*b:
+		return falseMsg, finding.OutcomeNegative, nil
+	}
+	return "", finding.OutcomeNotApplicable, errWrongValue
+}
+
+func Uint32LargerThan0(u32 *int32, nilMsg, trueMsg, falseMsg string) (string, finding.Outcome, error) {
+	switch {
+	case u32 == nil:
+		return nilMsg, finding.OutcomeNotAvailable, nil
+	case *u32 > 0:
+		return trueMsg, finding.OutcomePositive, nil
+	case *u32 == 0:
+		return falseMsg, finding.OutcomeNegative, nil
+	}
+	return "", finding.OutcomeNotApplicable, errWrongValue
+}

probes/requiresApproversForPullRequests/impl.go

@@ -21,6 +21,7 @@ import (
 
 	"github.com/ossf/scorecard/v4/checker"
 	"github.com/ossf/scorecard/v4/finding"
+	"github.com/ossf/scorecard/v4/probes/internal/utils/branchprotection"
 	"github.com/ossf/scorecard/v4/probes/internal/utils/uerror"
 )
 
@@ -39,43 +40,23 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
 
 	for i := range r.Branches {
 		branch := &r.Branches[i]
-		//nolint:nestif
-		if branch.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount != nil {
-			if *branch.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount > 0 {
-				f, err := finding.NewWith(fs, Probe,
-					fmt.Sprintf("required approving review count on branch '%s'", *branch.Name), nil,
-					finding.OutcomePositive)
-				if err != nil {
-					return nil, Probe, fmt.Errorf("create finding: %w", err)
-				}
-				f = f.WithValues(map[string]int{
-					*branch.Name: 1,
-				})
-				findings = append(findings, *f)
-			} else {
-				f, err := finding.NewWith(fs, Probe,
-					fmt.Sprintf("required approving review count on branch '%s'", *branch.Name), nil,
-					finding.OutcomeNegative)
-				if err != nil {
-					return nil, Probe, fmt.Errorf("create finding: %w", err)
-				}
-				f = f.WithValues(map[string]int{
-					*branch.Name: 1,
-				})
-				findings = append(findings, *f)
-			}
-		} else {
-			f, err := finding.NewWith(fs, Probe,
-				fmt.Sprintf("could not determine whether branch '%s' has required approving review count", *branch.Name),
-				nil, finding.OutcomeNotAvailable)
-			if err != nil {
-				return nil, Probe, fmt.Errorf("create finding: %w", err)
-			}
-			f = f.WithValues(map[string]int{
-				*branch.Name: 1,
-			})
-			findings = append(findings, *f)
+		nilMsg := fmt.Sprintf("could not determine whether branch '%s' has required approving review count", *branch.Name)
+		trueMsg := fmt.Sprintf("required approving review count on branch '%s'", *branch.Name)
+		falseMsg := fmt.Sprintf("required approving review count on branch '%s'", *branch.Name)
+
+		p := branch.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount
+		text, outcome, err := branchprotection.Uint32LargerThan0(p, nilMsg, trueMsg, falseMsg)
+		if err != nil {
+			return nil, Probe, fmt.Errorf("create finding: %w", err)
 		}
+		f, err := finding.NewWith(fs, Probe, text, nil, outcome)
+		if err != nil {
+			return nil, Probe, fmt.Errorf("create finding: %w", err)
+		}
+		f = f.WithValues(map[string]int{
+			*branch.Name: 1,
+		})
+		findings = append(findings, *f)
 	}
 	return findings, Probe, nil
 }

probes/requiresLastPushApproval/impl.go

@@ -21,6 +21,7 @@ import (
 
 	"github.com/ossf/scorecard/v4/checker"
 	"github.com/ossf/scorecard/v4/finding"
+	"github.com/ossf/scorecard/v4/probes/internal/utils/branchprotection"
 	"github.com/ossf/scorecard/v4/probes/internal/utils/uerror"
 )
 
@@ -39,43 +40,24 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
 
 	for i := range r.Branches {
 		branch := &r.Branches[i]
-		//nolint:nestif
-		if branch.BranchProtectionRule.RequireLastPushApproval != nil {
-			if *branch.BranchProtectionRule.RequireLastPushApproval {
-				f, err := finding.NewWith(fs, Probe,
-					fmt.Sprintf("'last push approval' enabled on branch '%s'", *branch.Name),
-					nil, finding.OutcomePositive)
-				if err != nil {
-					return nil, Probe, fmt.Errorf("create finding: %w", err)
-				}
-				f = f.WithValues(map[string]int{
-					*branch.Name: 1,
-				})
-				findings = append(findings, *f)
-			} else {
-				f, err := finding.NewWith(fs, Probe,
-					fmt.Sprintf("'last push approval' disabled on branch '%s'", *branch.Name),
-					nil, finding.OutcomeNegative)
-				if err != nil {
-					return nil, Probe, fmt.Errorf("create finding: %w", err)
-				}
-				f = f.WithValues(map[string]int{
-					*branch.Name: 1,
-				})
-				findings = append(findings, *f)
-			}
-		} else {
-			f, err := finding.NewWith(fs, Probe,
-				fmt.Sprintf("unable to retrieve whether 'last push approval' is required to merge on branch '%s'", *branch.Name),
-				nil, finding.OutcomeNotAvailable)
-			if err != nil {
-				return nil, Probe, fmt.Errorf("create finding: %w", err)
-			}
-			f = f.WithValues(map[string]int{
-				*branch.Name: 1,
-			})
-			findings = append(findings, *f)
+		nilMsg := fmt.Sprintf("unable to retrieve whether 'last push approval' is required to merge on branch '%s'",
+			*branch.Name)
+		trueMsg := fmt.Sprintf("'last push approval' enabled on branch '%s'", *branch.Name)
+		falseMsg := fmt.Sprintf("'last push approval' disabled on branch '%s'", *branch.Name)
+
+		p := branch.BranchProtectionRule.RequireLastPushApproval
+		text, outcome, err := branchprotection.GetTextOutcomeFromBool(p, nilMsg, trueMsg, falseMsg)
+		if err != nil {
+			return nil, Probe, fmt.Errorf("create finding: %w", err)
 		}
+		f, err := finding.NewWith(fs, Probe, text, nil, outcome)
+		if err != nil {
+			return nil, Probe, fmt.Errorf("create finding: %w", err)
+		}
+		f = f.WithValues(map[string]int{
+			*branch.Name: 1,
+		})
+		findings = append(findings, *f)
 	}
 	return findings, Probe, nil
 }

probes/requiresUpToDateBranches/impl.go

@@ -21,6 +21,7 @@ import (
 
 	"github.com/ossf/scorecard/v4/checker"
 	"github.com/ossf/scorecard/v4/finding"
+	"github.com/ossf/scorecard/v4/probes/internal/utils/branchprotection"
 	"github.com/ossf/scorecard/v4/probes/internal/utils/uerror"
 )
 
@@ -39,44 +40,24 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
 
 	for i := range r.Branches {
 		branch := &r.Branches[i]
-		//nolint:nestif
-		if branch.BranchProtectionRule.CheckRules.UpToDateBeforeMerge == nil {
-			f, err := finding.NewWith(fs, Probe,
-				fmt.Sprintf("unable to retrieve whether up-to-date branches are needed to merge on branch '%s'", *branch.Name),
-				nil, finding.OutcomeNotAvailable)
-			if err != nil {
-				return nil, Probe, fmt.Errorf("create finding: %w", err)
-			}
-			f = f.WithValues(map[string]int{
-				*branch.Name: 1,
-			})
-			findings = append(findings, *f)
-		} else {
-			// Note: `This setting will not take effect unless at least one status check is enabled`.
-			if *branch.BranchProtectionRule.CheckRules.UpToDateBeforeMerge {
-				f, err := finding.NewWith(fs, Probe,
-					fmt.Sprintf("status checks require up-to-date branches for '%s'", *branch.Name),
-					nil, finding.OutcomePositive)
-				if err != nil {
-					return nil, Probe, fmt.Errorf("create finding: %w", err)
-				}
-				f = f.WithValues(map[string]int{
-					*branch.Name: 1,
-				})
-				findings = append(findings, *f)
-			} else {
-				f, err := finding.NewWith(fs, Probe,
-					fmt.Sprintf("status checks do not require up-to-date branches for '%s'", *branch.Name),
-					nil, finding.OutcomeNegative)
-				if err != nil {
-					return nil, Probe, fmt.Errorf("create finding: %w", err)
-				}
-				f = f.WithValues(map[string]int{
-					*branch.Name: 1,
-				})
-				findings = append(findings, *f)
-			}
+		nilMsg := fmt.Sprintf("unable to retrieve whether up-to-date branches are needed to merge on branch '%s'",
+			*branch.Name)
+		trueMsg := fmt.Sprintf("status checks require up-to-date branches for '%s'", *branch.Name)
+		falseMsg := fmt.Sprintf("status checks do not require up-to-date branches for '%s'", *branch.Name)
+
+		p := branch.BranchProtectionRule.CheckRules.UpToDateBeforeMerge
+		text, outcome, err := branchprotection.GetTextOutcomeFromBool(p, nilMsg, trueMsg, falseMsg)
+		if err != nil {
+			return nil, Probe, fmt.Errorf("create finding: %w", err)
 		}
+		f, err := finding.NewWith(fs, Probe, text, nil, outcome)
+		if err != nil {
+			return nil, Probe, fmt.Errorf("create finding: %w", err)
+		}
+		f = f.WithValues(map[string]int{
+			*branch.Name: 1,
+		})
+		findings = append(findings, *f)
 	}
 	return findings, Probe, nil
 }