Updates to checks and checks/evaluation

Signed-off-by: AdamKorcz <[email protected]>

checks/contributors.go

@@ -19,6 +19,7 @@ import (
 	"github.com/ossf/scorecard/v4/checks/evaluation"
 	"github.com/ossf/scorecard/v4/checks/raw"
 	sce "github.com/ossf/scorecard/v4/errors"
+	"github.com/ossf/scorecard/v4/probes"
 )
 
 // CheckContributors is the registered name for Contributors.
@@ -34,17 +35,23 @@ func init() {
 
 // Contributors run Contributors check.
 func Contributors(c *checker.CheckRequest) checker.CheckResult {
-	rawData, err := raw.Contributors(c.RepoClient)
+	rawData, err := raw.Contributors(c)
 	if err != nil {
 		e := sce.WithMessage(sce.ErrScorecardInternal, err.Error())
 		return checker.CreateRuntimeErrorResult(CheckContributors, e)
 	}
 
-	// Return raw results.
-	if c.RawResults != nil {
-		c.RawResults.ContributorsResults = rawData
+	// Set the raw results.
+	pRawResults := getRawResults(c)
+	pRawResults.ContributorsResults = rawData
+
+	// Evaluate the probes.
+	findings, err := evaluateProbes(c, pRawResults, probes.Contributors)
+	if err != nil {
+		e := sce.WithMessage(sce.ErrScorecardInternal, err.Error())
+		return checker.CreateRuntimeErrorResult(CheckContributors, e)
 	}
 
 	// Return the score evaluation.
-	return evaluation.Contributors(CheckContributors, c.Dlogger, &rawData)
+	return evaluation.Contributors(CheckContributors, findings)
 }

checks/contributors_test.go

@@ -53,7 +53,7 @@ func TestContributors(t *testing.T) {
 				},
 			},
 			expected: checker.CheckResult{
-				Score: 0,
+				Score: -1,
 			},
 		},
 		{
@@ -142,7 +142,7 @@ func TestContributors(t *testing.T) {
 			name:    "No contributors",
 			contrib: []clients.User{},
 			expected: checker.CheckResult{
-				Score: 0,
+				Score: -1,
 			},
 		},
 		{

checks/evaluation/contributors.go

@@ -16,60 +16,38 @@ package evaluation
 
 import (
 	"fmt"
-	"sort"
-	"strings"
 
 	"github.com/ossf/scorecard/v4/checker"
 	sce "github.com/ossf/scorecard/v4/errors"
+	"github.com/ossf/scorecard/v4/finding"
+	"github.com/ossf/scorecard/v4/probes/contributorsFromOrgOrCompany"
 )
 
 const (
-	minContributionsPerUser    = 5
 	numberCompaniesForTopScore = 3
 )
 
-// Contributors applies the score policy for the Contributors check.
-func Contributors(name string, dl checker.DetailLogger,
-	r *checker.ContributorsData,
+func Contributors(name string,
+	findings []finding.Finding,
 ) checker.CheckResult {
-	if r == nil {
-		e := sce.WithMessage(sce.ErrScorecardInternal, "empty raw data")
-		return checker.CreateRuntimeErrorResult(name, e)
+	expectedProbes := []string{
+		contributorsFromOrgOrCompany.Probe,
 	}
 
-	entities := make(map[string]bool)
-
-	for _, user := range r.Users {
-		if user.NumContributions < minContributionsPerUser {
-			continue
-		}
-
-		for _, org := range user.Organizations {
-			entities[org.Login] = true
-		}
-
-		for _, comp := range user.Companies {
-			entities[comp] = true
-		}
-	}
-
-	names := []string{}
-	for c := range entities {
-		names = append(names, c)
+	if !finding.UniqueProbesEqual(findings, expectedProbes) {
+		e := sce.WithMessage(sce.ErrScorecardInternal, "invalid probe results")
+		return checker.CreateRuntimeErrorResult(name, e)
 	}
 
-	sort.Strings(names)
+	reason := fmt.Sprintf("project has %d contributing companies or organizations", len(findings))
 
-	if len(names) > 0 {
-		dl.Info(&checker.LogMessage{
-			Text: fmt.Sprintf("contributors work for %v", strings.Join(names, ",")),
-		})
-	} else {
-		dl.Warn(&checker.LogMessage{
-			Text: "no contributors have an org or company",
-		})
+	if len(findings) >= numberCompaniesForTopScore {
+		// Return max score. This may need changing if other probes
+		// are added for other contributors metrics. Right now, it the
+		// scoring is designed for a single probe that returns true
+		// or false.
+		return checker.CreateMaxScoreResult(name, reason)
 	}
 
-	reason := fmt.Sprintf("%d different organizations found", len(entities))
-	return checker.CreateProportionalScoreResult(name, reason, len(entities), numberCompaniesForTopScore)
+	return checker.CreateMinScoreResult(name, reason)
 }

checks/evaluation/contributors_test.go

@@ -20,134 +20,77 @@ import (
 	"github.com/google/go-cmp/cmp/cmpopts"
 
 	"github.com/ossf/scorecard/v4/checker"
-	"github.com/ossf/scorecard/v4/clients"
-	"github.com/ossf/scorecard/v4/utests"
+	// "github.com/ossf/scorecard/v4/clients"
+	//"github.com/ossf/scorecard/v4/utests".
+	"github.com/ossf/scorecard/v4/finding"
 )
 
 func TestContributors(t *testing.T) {
 	t.Parallel()
-	testCases := []struct {
+	type args struct { //nolint
 		name     string
-		raw      *checker.ContributorsData
-		expected checker.CheckResult
+		findings []finding.Finding
+	}
+	tests := []struct {
+		name string
+		args args
+		want checker.CheckResult
 	}{
 		{
-			name: "No data",
-			raw:  nil,
-			expected: checker.CheckResult{
-				Version: 2,
-				Score:   -1,
-				Reason:  "internal error: empty raw data",
-			},
-		},
-		{
-			name: "No contributors",
-			raw: &checker.ContributorsData{
-				Users: []clients.User{},
-			},
-			expected: checker.CheckResult{
-				Version: 2,
-				Score:   0,
-				Reason:  "0 different organizations found -- score normalized to 0",
-			},
-		},
-		{
-			name: "Contributors with orgs and number of contributions is greater than 5 with companies",
-			raw: &checker.ContributorsData{
-				Users: []clients.User{
+			name: "Only has two positive outcomes",
+			args: args{
+				name: "Contributors",
+				findings: []finding.Finding{
 					{
-						NumContributions: 10,
-						Organizations: []clients.User{
-							{
-								Login: "org1",
-							},
-						},
-						Companies: []string{"company1"},
+						Probe:   "contributorsFromOrgOrCompany",
+						Outcome: finding.OutcomePositive,
 					},
 					{
-						NumContributions: 10,
-						Organizations: []clients.User{
-							{
-								Login: "org2",
-							},
-						},
-					},
-					{
-						NumContributions: 10,
-						Organizations: []clients.User{
-							{
-								Login: "org3",
-							},
-						},
-					},
-					{
-						NumContributions: 1,
-						Organizations: []clients.User{
-							{
-								Login: "org2",
-							},
-						},
+						Probe:   "contributorsFromOrgOrCompany",
+						Outcome: finding.OutcomePositive,
 					},
 				},
 			},
-			expected: checker.CheckResult{
+			want: checker.CheckResult{
+				Score:   0,
 				Version: 2,
-				Score:   10,
-				Reason:  "4 different organizations found -- score normalized to 10",
+				Name:    "contributorsFromOrgOrCompany",
+				Reason:  "project has 2 contributing companies or organizations",
 			},
-		},
-		{
-			name: "Contributors with orgs and number of contributions is greater than 5 without companies",
-			raw: &checker.ContributorsData{
-				Users: []clients.User{
-					{
-						NumContributions: 10,
-						Organizations: []clients.User{
-							{
-								Login: "org1",
-							},
-						},
-					},
+		}, {
+			name: "Has two positive outcomes",
+			args: args{
+				name: "Contributors",
+				findings: []finding.Finding{
 					{
-						NumContributions: 10,
-						Organizations: []clients.User{
-							{
-								Login: "org2",
-							},
-						},
+						Probe:   "contributorsFromOrgOrCompany",
+						Outcome: finding.OutcomePositive,
 					},
 					{
-						NumContributions: 10,
-						Organizations: []clients.User{
-							{
-								Login: "org3",
-							},
-						},
+						Probe:   "contributorsFromOrgOrCompany",
+						Outcome: finding.OutcomePositive,
 					},
 					{
-						NumContributions: 1,
-						Organizations: []clients.User{
-							{
-								Login: "org10",
-							},
-						},
+						Probe:   "contributorsFromOrgOrCompany",
+						Outcome: finding.OutcomePositive,
 					},
 				},
 			},
-			expected: checker.CheckResult{
-				Version: 2,
+			want: checker.CheckResult{
 				Score:   10,
-				Reason:  "3 different organizations found -- score normalized to 10",
+				Version: 2,
+				Name:    "contributorsFromOrgOrCompany",
+				Reason:  "project has 3 contributing companies or organizations",
 			},
 		},
 	}
-	for _, tc := range testCases {
-		tc := tc
-		t.Run(tc.name, func(t *testing.T) {
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
-			result := Contributors("", &utests.TestDetailLogger{}, tc.raw)
-			if !cmp.Equal(result, tc.expected, cmpopts.IgnoreFields(checker.CheckResult{}, "Error")) { //nolint:govet
-				t.Errorf("expected %v, got %v", tc.expected, cmp.Diff(tc.expected, result, cmpopts.IgnoreFields(checker.CheckResult{}, "Error"))) //nolint:lll
+			result := Contributors("contributorsFromOrgOrCompany", tt.args.findings)
+			if !cmp.Equal(result, tt.want, cmpopts.IgnoreFields(checker.CheckResult{}, "Error")) { //nolint:govet
+				t.Errorf("expected %v, got %v", tt.want, cmp.Diff(tt.want, result, cmpopts.IgnoreFields(checker.CheckResult{}, "Error"))) //nolint:lll
 			}
 		})
 	}

checks/raw/contributors.go

@@ -23,7 +23,8 @@ import (
 )
 
 // Contributors retrieves the raw data for the Contributors check.
-func Contributors(c clients.RepoClient) (checker.ContributorsData, error) {
+func Contributors(cr *checker.CheckRequest) (checker.ContributorsData, error) {
+	c := cr.RepoClient
 	var users []clients.User
 
 	contribs, err := c.ListContributors()

checks/raw/contributors_test.go

@@ -13,6 +13,7 @@
 // limitations under the License.
 package raw
 
+/*
 import (
 	"testing"
 
@@ -161,3 +162,4 @@ func TestContributors(t *testing.T) {
 		t.Errorf("unexpected contributors data (-want +got):\n%s", diff)
 	}
 }
+*/