Skip to content

Commit 11e859f

Browse files
spencerschrockspencerschrock
authored
🌱 Combine hasLicenseFile and hasLicenseFileAtTopDir probes (#3955)
* delete hasLicenseFileAtTopDir probe Signed-off-by: Spencer Schrock <[email protected]> * increase value of having a license the old split was 6 for having a license and 3 for having it in the expected location but 1.5 years later, and there is still no other way we detect it. So it was effectively worth 9 points. This change makes it actually worth 9 points. Signed-off-by: Spencer Schrock <[email protected]> * simplify logging and scoring Signed-off-by: Spencer Schrock <[email protected]> * ensure license findings have locations Signed-off-by: Spencer Schrock <[email protected]> * update tests to reflect new logging Signed-off-by: Spencer Schrock <[email protected]> * match existing detail better Signed-off-by: Spencer Schrock <[email protected]> --------- Signed-off-by: Spencer Schrock <[email protected]>
1 parent be15709 commit 11e859f

File tree

9 files changed

+26
-397
lines changed

9 files changed

+26
-397
lines changed

‎checks/evaluation/license.go

+4-44
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"github.com/ossf/scorecard/v4/finding"
2121
"github.com/ossf/scorecard/v4/probes/hasFSFOrOSIApprovedLicense"
2222
"github.com/ossf/scorecard/v4/probes/hasLicenseFile"
23-
"github.com/ossf/scorecard/v4/probes/hasLicenseFileAtTopDir"
2423
)
2524

2625
// License applies the score policy for the License check.
@@ -32,7 +31,6 @@ func License(name string,
3231
expectedProbes := []string{
3332
hasLicenseFile.Probe,
3433
hasFSFOrOSIApprovedLicense.Probe,
35-
hasLicenseFileAtTopDir.Probe,
3634
}
3735

3836
if !finding.UniqueProbesEqual(findings, expectedProbes) {
@@ -45,58 +43,20 @@ func License(name string,
4543
m := make(map[string]bool)
4644
for i := range findings {
4745
f := &findings[i]
48-
switch f.Outcome {
49-
case finding.OutcomeNotApplicable:
50-
dl.Info(&checker.LogMessage{
51-
Type: finding.FileTypeSource,
52-
Offset: 1,
53-
Text: f.Message,
54-
})
55-
case finding.OutcomePositive:
46+
if f.Outcome == finding.OutcomePositive {
5647
switch f.Probe {
5748
case hasFSFOrOSIApprovedLicense.Probe:
58-
dl.Info(&checker.LogMessage{
59-
Type: finding.FileTypeSource,
60-
Offset: 1,
61-
Path: f.Message,
62-
Text: "FSF or OSI recognized license",
63-
})
6449
score += scoreProbeOnce(f.Probe, m, 1)
65-
case hasLicenseFileAtTopDir.Probe:
66-
dl.Info(&checker.LogMessage{
67-
Type: finding.FileTypeSource,
68-
Offset: 1,
69-
Path: f.Message,
70-
Text: "License file found in expected location",
71-
})
72-
score += scoreProbeOnce(f.Probe, m, 3)
7350
case hasLicenseFile.Probe:
74-
score += scoreProbeOnce(f.Probe, m, 6)
51+
score += scoreProbeOnce(f.Probe, m, 9)
7552
default:
7653
e := sce.WithMessage(sce.ErrScorecardInternal, "unknown probe results")
7754
return checker.CreateRuntimeErrorResult(name, e)
7855
}
79-
case finding.OutcomeNegative:
80-
switch f.Probe {
81-
case hasLicenseFileAtTopDir.Probe:
82-
dl.Warn(&checker.LogMessage{
83-
Type: finding.FileTypeSource,
84-
Offset: 1,
85-
Path: f.Message,
86-
Text: "License file found in unexpected location",
87-
})
88-
case hasFSFOrOSIApprovedLicense.Probe:
89-
dl.Warn(&checker.LogMessage{
90-
Type: finding.FileTypeSource,
91-
Offset: 1,
92-
Path: "",
93-
Text: f.Message,
94-
})
95-
}
96-
default:
97-
continue // for linting
9856
}
9957
}
58+
checker.LogFindings(findings, dl)
59+
10060
_, defined := m[hasLicenseFile.Probe]
10161
if !defined {
10262
if score > 0 {

‎checks/evaluation/license_test.go

+4-44
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,6 @@ func TestLicense(t *testing.T) {
4040
Probe: "hasFSFOrOSIApprovedLicense",
4141
Outcome: finding.OutcomePositive,
4242
},
43-
{
44-
Probe: "hasLicenseFileAtTopDir",
45-
Outcome: finding.OutcomePositive,
46-
},
4743
},
4844
result: scut.TestReturn{
4945
Score: checker.MaxResultScore,
@@ -60,17 +56,13 @@ func TestLicense(t *testing.T) {
6056
Probe: "hasFSFOrOSIApprovedLicense",
6157
Outcome: finding.OutcomeNegative,
6258
},
63-
{
64-
Probe: "hasLicenseFileAtTopDir",
65-
Outcome: finding.OutcomeNegative,
66-
},
6759
},
6860
result: scut.TestReturn{
6961
Score: checker.MinResultScore,
7062
NumberOfWarn: 2,
7163
},
7264
}, {
73-
name: "Has license file but not a top level or in OSI/FSF format",
65+
name: "Has license file but not OSI/FSF approved",
7466
findings: []finding.Finding{
7567
{
7668
Probe: "hasLicenseFile",
@@ -80,14 +72,11 @@ func TestLicense(t *testing.T) {
8072
Probe: "hasFSFOrOSIApprovedLicense",
8173
Outcome: finding.OutcomeNegative,
8274
},
83-
{
84-
Probe: "hasLicenseFileAtTopDir",
85-
Outcome: finding.OutcomeNegative,
86-
},
8775
},
8876
result: scut.TestReturn{
89-
Score: 6,
90-
NumberOfWarn: 2,
77+
Score: 9,
78+
NumberOfWarn: 1,
79+
NumberOfInfo: 1,
9180
},
9281
}, {
9382
name: "Findings missing a probe = Error",
@@ -96,10 +85,6 @@ func TestLicense(t *testing.T) {
9685
Probe: "hasLicenseFile",
9786
Outcome: finding.OutcomePositive,
9887
},
99-
{
100-
Probe: "hasFSFOrOSIApprovedLicense",
101-
Outcome: finding.OutcomeNegative,
102-
},
10388
},
10489
result: scut.TestReturn{
10590
Score: -1,
@@ -116,37 +101,12 @@ func TestLicense(t *testing.T) {
116101
Probe: "hasFSFOrOSIApprovedLicense",
117102
Outcome: finding.OutcomeNegative,
118103
},
119-
{
120-
Probe: "hasLicenseFileAtTopDir",
121-
Outcome: finding.OutcomePositive,
122-
},
123104
},
124105
result: scut.TestReturn{
125106
Score: 9,
126107
NumberOfInfo: 1,
127108
NumberOfWarn: 1,
128109
},
129-
}, {
130-
name: "Has an OSI/FSF approved license but not at top level dir",
131-
findings: []finding.Finding{
132-
{
133-
Probe: "hasLicenseFile",
134-
Outcome: finding.OutcomePositive,
135-
},
136-
{
137-
Probe: "hasFSFOrOSIApprovedLicense",
138-
Outcome: finding.OutcomePositive,
139-
},
140-
{
141-
Probe: "hasLicenseFileAtTopDir",
142-
Outcome: finding.OutcomeNegative,
143-
},
144-
},
145-
result: scut.TestReturn{
146-
Score: 7,
147-
NumberOfInfo: 1,
148-
NumberOfWarn: 1,
149-
},
150110
},
151111
}
152112
for _, tt := range tests {

‎checks/license_test.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,11 @@ func TestLicenseFileSubdirectory(t *testing.T) {
5252
name: "Without LICENSE",
5353
inputFolder: "testdata/licensedir/withoutlicense",
5454
expected: scut.TestReturn{
55-
Error: nil,
56-
Score: checker.MinResultScore,
57-
NumberOfWarn: 0,
58-
NumberOfInfo: 2,
55+
Error: nil,
56+
Score: checker.MinResultScore,
57+
NumberOfWarn: 1,
58+
NumberOfInfo: 0,
59+
NumberOfDebug: 1,
5960
},
6061
err: nil,
6162
},

‎probes/entries.go

-2
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ import (
3232
"github.com/ossf/scorecard/v4/probes/hasDangerousWorkflowUntrustedCheckout"
3333
"github.com/ossf/scorecard/v4/probes/hasFSFOrOSIApprovedLicense"
3434
"github.com/ossf/scorecard/v4/probes/hasLicenseFile"
35-
"github.com/ossf/scorecard/v4/probes/hasLicenseFileAtTopDir"
3635
"github.com/ossf/scorecard/v4/probes/hasNoGitHubWorkflowPermissionUnknown"
3736
"github.com/ossf/scorecard/v4/probes/hasOSVVulnerabilities"
3837
"github.com/ossf/scorecard/v4/probes/hasOpenSSFBadge"
@@ -95,7 +94,6 @@ var (
9594
License = []ProbeImpl{
9695
hasLicenseFile.Run,
9796
hasFSFOrOSIApprovedLicense.Run,
98-
hasLicenseFileAtTopDir.Run,
9997
}
10098
Contributors = []ProbeImpl{
10199
contributorsFromOrgOrCompany.Run,

‎probes/hasFSFOrOSIApprovedLicense/impl.go

+10-12
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
3939
return nil, "", fmt.Errorf("%w: raw", uerror.ErrNil)
4040
}
4141

42-
if raw.LicenseResults.LicenseFiles == nil || len(raw.LicenseResults.LicenseFiles) == 0 {
42+
if len(raw.LicenseResults.LicenseFiles) == 0 {
4343
f, err := finding.NewWith(fs, Probe,
4444
"project does not have a license file", nil,
4545
finding.OutcomeNotApplicable)
@@ -50,18 +50,16 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
5050
}
5151

5252
for _, licenseFile := range raw.LicenseResults.LicenseFiles {
53-
if licenseFile.LicenseInformation.Approved {
54-
// Store the file path in the msg
55-
msg := licenseFile.File.Path
56-
57-
f, err := finding.NewWith(fs, Probe,
58-
msg, nil,
59-
finding.OutcomePositive)
60-
if err != nil {
61-
return nil, Probe, fmt.Errorf("create finding: %w", err)
62-
}
63-
return []finding.Finding{*f}, Probe, nil
53+
if !licenseFile.LicenseInformation.Approved {
54+
continue
6455
}
56+
licenseFile := licenseFile
57+
loc := licenseFile.File.Location()
58+
f, err := finding.NewPositive(fs, Probe, "FSF or OSI recognized license: "+licenseFile.LicenseInformation.Name, loc)
59+
if err != nil {
60+
return nil, Probe, fmt.Errorf("create finding: %w", err)
61+
}
62+
return []finding.Finding{*f}, Probe, nil
6563
}
6664

6765
f, err := finding.NewWith(fs, Probe,

‎probes/hasLicenseFile/impl.go

+3-19
Original file line numberDiff line numberDiff line change
@@ -40,17 +40,11 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
4040
}
4141

4242
var findings []finding.Finding
43-
var outcome finding.Outcome
44-
var msg string
4543

4644
licenseFiles := raw.LicenseResults.LicenseFiles
4745

4846
if len(licenseFiles) == 0 {
49-
outcome = finding.OutcomeNegative
50-
msg = "project does not have a license file"
51-
f, err := finding.NewWith(fs, Probe,
52-
msg, nil,
53-
outcome)
47+
f, err := finding.NewNegative(fs, Probe, "project does not have a license file", nil)
5448
if err != nil {
5549
return nil, Probe, fmt.Errorf("create finding: %w", err)
5650
}
@@ -59,18 +53,8 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
5953
} else {
6054
for _, licenseFile := range licenseFiles {
6155
licenseFile := licenseFile
62-
loc := &finding.Location{
63-
Type: licenseFile.File.Type,
64-
Path: licenseFile.File.Path,
65-
LineStart: &licenseFile.File.Offset,
66-
LineEnd: &licenseFile.File.EndOffset,
67-
Snippet: &licenseFile.File.Snippet,
68-
}
69-
msg = "project has a license file"
70-
outcome = finding.OutcomePositive
71-
f, err := finding.NewWith(fs, Probe,
72-
msg, loc,
73-
outcome)
56+
loc := licenseFile.File.Location()
57+
f, err := finding.NewPositive(fs, Probe, "project has a license file", loc)
7458
if err != nil {
7559
return nil, Probe, fmt.Errorf("create finding: %w", err)
7660
}

‎probes/hasLicenseFileAtTopDir/def.yml

-34
This file was deleted.

0 commit comments

Comments
 (0)