From 5746d91862da460a2f6cc5f2710b811251432b94 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Mon, 19 Jul 2021 23:52:47 +0000 Subject: [PATCH 1/8] details-1 --- CONTRIBUTING.md | 22 ++---- checker/check_request.go | 3 + checker/check_result.go | 161 ++++++++++++++++++++++++++++++++++++++- checker/check_runner.go | 81 +++++++++++++++++++- checker/check_test.go | 2 +- checks/write.md | 33 ++++++++ cmd/root.go | 15 +++- cmd/serve.go | 2 +- cron/worker/main.go | 3 +- errors/errors.md | 28 +++++++ errors/internal.go | 30 ++++++++ errors/names.go | 12 +-- errors/public.go | 38 +++++++++ errors/types.go | 10 +-- pkg/scorecard.go | 8 +- pkg/scorecard_result.go | 126 ++++++++++++++++++++++++++++-- 16 files changed, 526 insertions(+), 48 deletions(-) create mode 100644 checks/write.md create mode 100644 errors/errors.md create mode 100644 errors/internal.go create mode 100644 errors/public.go diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f3fd55feedc..fbdc49ed10e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -12,6 +12,7 @@ project. This document describes the contribution guidelines for the project. * [Getting started](#getting-started) * [Environment Setup](#environment-setup) * [Contributing steps](#contributing-steps) +* [Error handling](#error-handling) * [How to build scorecard locally](#how-to-build-scorecard-locally) * [PR Process](#pr-process) * [What to do before submitting a pull request](#what-to-do-before-submitting-a-pull-request) @@ -52,6 +53,10 @@ You must install these tools: 1. Fork the desired repo, develop and test your code changes. 1. Submit a pull request. +## Error handling + +See [errors/errors.md]. + ## How to build scorecard locally Note that, by building the scorecard from the source code we are allowed to test @@ -140,22 +145,7 @@ Submit a PR for this file and scorecard would start scanning in subsequent runs. ## Adding New Checks -Each check is currently just a function of type `CheckFn`. The signature is: - -```golang -type CheckFn func(*c.Checker) CheckResult -``` - -Checks are registered in an init function: - -```golang -const codeReviewStr = "Code-Review" - -//nolint:gochecknoinits -func init() { - registerCheck(codeReviewStr, DoesCodeReview) -} -``` +See [checks/write.md](checks/write.md). ### Updating Docs diff --git a/checker/check_request.go b/checker/check_request.go index 004d0fd7ef9..661d3d8c7ba 100644 --- a/checker/check_request.go +++ b/checker/check_request.go @@ -30,6 +30,9 @@ type CheckRequest struct { GraphClient *githubv4.Client HTTPClient *http.Client RepoClient clients.RepoClient + // Note: Ultimately Log will be removed and replaced by + // CLogger. Logf func(s string, f ...interface{}) + Dlogger DetailLogger Owner, Repo string } diff --git a/checker/check_result.go b/checker/check_result.go index ccce9c9ae02..0c14e05955b 100644 --- a/checker/check_result.go +++ b/checker/check_result.go @@ -16,30 +16,180 @@ package checker import ( "errors" + "fmt" + "math" scorecarderrors "github.com/ossf/scorecard/errors" ) -const MaxResultConfidence = 10 +// UPGRADEv2: to remove. +const ( + MaxResultConfidence = 10 + HalfResultConfidence = 5 + MinResultConfidence = 0 +) // ErrorDemoninatorZero indicates the denominator for a proportional result is 0. +// UPGRADEv2: to remove. var ErrorDemoninatorZero = errors.New("internal error: denominator is 0") +//nolint +const ( + MaxResultScore = 10 + HalfResultScore = 5 + MinResultScore = 0 + InconclusiveResultScore = -1 +) + +//nolint type CheckResult struct { + // Old structure Error error `json:"-"` Name string Details []string Confidence int Pass bool ShouldRetry bool `json:"-"` + + // UPGRADEv2: New structure. Omitting unchanged Name field + // for simplicity. + Version int // 2. Default value of 0 indicates old structure + Error2 error `json:"-"` // Runtime error indicate a filure to run the check. + Details2 []CheckDetail `json:"-"` // Details of tests and sub-checks + Score2 int `json:"-"` // {[0...1], -1 = Inconclusive} + Reason2 string `json:"-"` // A sentence describing the check result (score, etc) +} + +// CreateResultWithScore is used when +// the check runs without runtime errors and want to assign a +// specific score. +func CreateResultWithScore(name, reason string, score int) CheckResult { + pass := true + //nolint + if score < 8 { + pass = false + } + return CheckResult{ + Name: name, + // Old structure. + Error: nil, + Confidence: MaxResultScore, + Pass: pass, + ShouldRetry: false, + // New structure. + //nolint + Version: 2, + Error2: nil, + Score2: score, + Reason2: reason, + } +} + +// CreateProportionalScoreResult is used when +// the check runs without runtime errors and we assign a +// proportional score. This may be used if a check contains +// multiple tests and we want to assign a score proportional +// the the number of tests that succeeded. +func CreateProportionalScoreResult(name, reason string, b, t int) CheckResult { + pass := true + //nolint + score := int(math.Min(float64(10*b/t), float64(10))) + //nolint + if score < 8 { + pass = false + } + return CheckResult{ + Name: name, + // Old structure. + Error: nil, + //nolint + Confidence: 10, + Pass: pass, + ShouldRetry: false, + // New structure. + //nolint + Version: 2, + Error2: nil, + Score2: score, + Reason2: fmt.Sprintf("%v -- score normalized to %d", reason, score), + } +} + +// CreateMaxScoreResult is used when +// the check runs without runtime errors and we can assign a +// maximum score to the result. +func CreateMaxScoreResult(name, reason string) CheckResult { + return CreateResultWithScore(name, reason, MaxResultScore) +} + +// CreateMinScoreResult is used when +// the check runs without runtime errors and we can assign a +// minimum score to the result. +func CreateMinScoreResult(name, reason string) CheckResult { + return CreateResultWithScore(name, reason, MinResultScore) } +// CreateInconclusiveResult is used when +// the check runs without runtime errors, but we don't +// have enough evidence to set a score. +func CreateInconclusiveResult(name, reason string) CheckResult { + return CheckResult{ + Name: name, + // Old structure. + Confidence: 0, + Pass: false, + ShouldRetry: false, + // New structure. + //nolint + Version: 2, + Score2: InconclusiveResultScore, + Reason2: reason, + } +} + +// CreateRuntimeErrorResult is used when the check fails to run because of a runtime error. +func CreateRuntimeErrorResult(name string, e error) CheckResult { + return CheckResult{ + Name: name, + // Old structure. + Error: e, + Confidence: 0, + Pass: false, + ShouldRetry: false, + // New structure. + //nolint + Version: 2, + Error2: e, + Score2: InconclusiveResultScore, + Reason2: e.Error(), // Note: message already accessible by caller thru `Error`. + } +} + +// UPGRADEv2: will be renamed. +func MakeAndResult2(checks ...CheckResult) CheckResult { + if len(checks) == 0 { + // That should never happen. + panic("MakeResult called with no checks") + } + + worseResult := checks[0] + // UPGRADEv2: will go away after old struct is removed. + //nolint + for _, result := range checks[1:] { + if result.Score2 < worseResult.Score2 { + worseResult = result + } + } + return worseResult +} + +// UPGRADEv2: will be removed. func MakeInconclusiveResult(name string, err error) CheckResult { return CheckResult{ Name: name, Pass: false, Confidence: 0, - Error: scorecarderrors.MakeZeroConfidenceError(err), + Error: scorecarderrors.MakeLowConfidenceError(err), } } @@ -48,6 +198,7 @@ func MakePassResult(name string) CheckResult { Name: name, Pass: true, Confidence: MaxResultConfidence, + Error: nil, } } @@ -69,6 +220,8 @@ func MakeRetryResult(name string, err error) CheckResult { } } +// TODO: update this function to return a ResultDontKnow +// if the confidence is low? func MakeProportionalResult(name string, numerator int, denominator int, threshold float32) CheckResult { if denominator == 0 { @@ -98,6 +251,7 @@ func MakeProportionalResult(name string, numerator int, denominator int, } // Given a min result, check if another result is worse. +//nolint func isMinResult(result, min CheckResult) bool { if Bool2int(result.Pass) < Bool2int(min.Pass) { return true @@ -117,7 +271,8 @@ func MakeAndResult(checks ...CheckResult) CheckResult { Pass: true, Confidence: MaxResultConfidence, } - + // UPGRADEv2: will go away after old struct is removed. + //nolint for _, result := range checks { if minResult.Name == "" { minResult.Name = result.Name diff --git a/checker/check_runner.go b/checker/check_runner.go index a9843dd728a..08855e9a10f 100644 --- a/checker/check_runner.go +++ b/checker/check_runner.go @@ -39,15 +39,56 @@ type CheckFn func(*CheckRequest) CheckResult type CheckNameToFnMap map[string]CheckFn +// Types of details. +type DetailType int + +const ( + DetailInfo DetailType = iota + DetailWarn + DetailDebug +) + +// CheckDetail contains information for each detail. +//nolint:govet +type CheckDetail struct { + Type DetailType // Any of DetailWarn, DetailInfo, DetailDebug. + Msg string // A short string explaining why the details was recorded/logged.. +} + +type DetailLogger interface { + Info(desc string, args ...interface{}) + Warn(desc string, args ...interface{}) + Debug(desc string, args ...interface{}) +} + +// UPGRADEv2: messages2 will ultimately +// be renamed to messages. type logger struct { - messages []string + messages []string + messages2 []CheckDetail +} + +func (l *logger) Info(desc string, args ...interface{}) { + cd := CheckDetail{Type: DetailInfo, Msg: fmt.Sprintf(desc, args...)} + l.messages2 = append(l.messages2, cd) } +func (l *logger) Warn(desc string, args ...interface{}) { + cd := CheckDetail{Type: DetailWarn, Msg: fmt.Sprintf(desc, args...)} + l.messages2 = append(l.messages2, cd) +} + +func (l *logger) Debug(desc string, args ...interface{}) { + cd := CheckDetail{Type: DetailDebug, Msg: fmt.Sprintf(desc, args...)} + l.messages2 = append(l.messages2, cd) +} + +// UPGRADEv2: to remove. func (l *logger) Logf(s string, f ...interface{}) { l.messages = append(l.messages, fmt.Sprintf(s, f...)) } -func logStats(ctx context.Context, startTime time.Time, result CheckResult) error { +func logStats(ctx context.Context, startTime time.Time, result *CheckResult) error { runTimeInSecs := time.Now().Unix() - startTime.Unix() opencensusstats.Record(ctx, stats.CheckRuntimeInSec.M(runTimeInSecs)) @@ -74,17 +115,22 @@ func (r *Runner) Run(ctx context.Context, f CheckFn) CheckResult { checkRequest := r.CheckRequest checkRequest.Ctx = ctx l = logger{} + // UPGRADEv2: to remove. checkRequest.Logf = l.Logf + checkRequest.Dlogger = &l res = f(&checkRequest) + // UPGRADEv2: to fix using proper error check. if res.ShouldRetry && !strings.Contains(res.Error.Error(), "invalid header field value") { checkRequest.Logf("error, retrying: %s", res.Error) continue } break } + // UPGRADEv2: to remove. res.Details = l.messages + res.Details2 = l.messages2 - if err := logStats(ctx, startTime, res); err != nil { + if err := logStats(ctx, startTime, &res); err != nil { panic(err) } return res @@ -97,6 +143,35 @@ func Bool2int(b bool) int { return 0 } +// UPGRADEv2: will be renamed. +func MultiCheckOr2(fns ...CheckFn) CheckFn { + return func(c *CheckRequest) CheckResult { + //nolint + maxResult := CheckResult{Version: 2} + + for _, fn := range fns { + result := fn(c) + + if result.Score2 > maxResult.Score2 { + maxResult = result + } + } + return maxResult + } +} + +func MultiCheckAnd2(fns ...CheckFn) CheckFn { + return func(c *CheckRequest) CheckResult { + var checks []CheckResult + for _, fn := range fns { + res := fn(c) + checks = append(checks, res) + } + return MakeAndResult2(checks...) + } +} + +// UPGRADEv2: will be removed. // MultiCheckOr returns the best check result out of several ones performed. func MultiCheckOr(fns ...CheckFn) CheckFn { return func(c *CheckRequest) CheckResult { diff --git a/checker/check_test.go b/checker/check_test.go index 99ad6f6fa80..b0e18517cbd 100644 --- a/checker/check_test.go +++ b/checker/check_test.go @@ -27,8 +27,8 @@ func TestMakeCheckAnd(t *testing.T) { t.Parallel() tests := []struct { name string - checks []CheckResult want CheckResult + checks []CheckResult }{ { name: "Multiple passing", diff --git a/checks/write.md b/checks/write.md new file mode 100644 index 00000000000..955fdea0dfd --- /dev/null +++ b/checks/write.md @@ -0,0 +1,33 @@ +# How to write a check +The steps to writting a check are as follow: + +1. Create a file under `checks/` folder, say `checks/mycheck.go` +2. Give the check a name and register the check: +``` +// Note: do not export the name: start its name with a lower-case letter. +const checkMyCheckName string = "My-Check" + +func init() { + registerCheck(checkMyCheckName, EntryPointMyCheck) +} +``` +3. Log information that is benfical to the user using `checker.DetailLogger`: + * Use `checker.DetailLogger.Warn()` to provide detail on low-score results. This is showed when the user supplies the `show-results` option. + * Use `checker.DetailLogger.Info()` to provide detail on high-score results. This is showed when the user supplies the `show-results` option. + * Use `checker.DetailLogger.Debug()` to provide detail in verbose mode: this is showed only when the user supplies the `--verbosity Debug` option. +4. If the checks fails in a way that is irrecoverable, return a result with `checker.CreateRuntimeErrorResult()` function: For example, +if an error is returned from an API you call, use the function. +5. Create the result of the check as follow: + * Always provide a high-level sentence explaining the result/score of the check. + * If the check runs properly but is unable to determine a score, use `checker.CreateInconclusiveResult()` function. + * For propertional results, use `checker.CreateProportionalScoreResult()`. + * For maximum score, use `checker.CreateMaxScoreResult()`; for min score use `checker.CreateMinScoreResult()`. + * If you need more flexibility and need to set a specific score, use `checker.CreateResultWithScore()` with one of the constants declared, such as `checker.HalfResultScore`. +6. Dealing with errors: see [../errors/errors.md](errors/errors/md). +7. Create unit tests for both low, high and inconclusive score. Put them in a file `checks/mycheck_test.go`. +8. Create e2e tests in `e2e/mycheck_test.go`. Use a dedicated repo that will not change over time, so that it's reliable for the tests. +9. Update the `checks/checks.yaml` with the description of your check. +10. Gerenate the `checks/check.md` using `go build && cd checks/main && ./main`. Verify `checks/check.md` was updated. +10. Update the [README.md](https://github.com/ossf/scorecard#scorecard-checks) with a short description of your check. + +For actual examples, look at [checks/binary_artifact.go](binary_artifact.go), [checks/code_review.go](code_review.go) and [checks/frozen_deps.go](frozen_deps.go). \ No newline at end of file diff --git a/cmd/root.go b/cmd/root.go index 98dea859b3a..7bf56267d67 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -51,6 +51,9 @@ var ( pypi string rubygems string showDetails bool + // UPGRADEv2: will be removed. + v2 bool + // TODO(laurent): add explain command. // ErrorInvalidFormatFlag indicates an invalid option was passed for the 'format' argument. ErrorInvalidFormatFlag = errors.New("invalid format flag") ) @@ -157,11 +160,15 @@ or ./scorecard --{npm,pypi,rubgems}= [--checks=check1,...] [--show switch format { case formatDefault: - err = repoResult.AsString(showDetails, os.Stdout) + if v2 { + err = repoResult.AsString2(showDetails, *logLevel, os.Stdout) + } else { + err = repoResult.AsString(showDetails, *logLevel, os.Stdout) + } case formatCSV: - err = repoResult.AsCSV(showDetails, os.Stdout) + err = repoResult.AsCSV(showDetails, *logLevel, os.Stdout) case formatJSON: - err = repoResult.AsJSON(showDetails, os.Stdout) + err = repoResult.AsJSON(showDetails, *logLevel, os.Stdout) default: err = fmt.Errorf("%w %s. allowed values are: [default, csv, json]", ErrorInvalidFormatFlag, format) } @@ -306,6 +313,8 @@ func init() { rootCmd.Flags().StringSliceVar( &metaData, "metadata", []string{}, "metadata for the project.It can be multiple separated by commas") rootCmd.Flags().BoolVar(&showDetails, "show-details", false, "show extra details about each check") + // UPGRADEv2: will be removed. + rootCmd.Flags().BoolVar(&v2, "v2", false, "temporary flag to display v2 changes") checkNames := []string{} for checkName := range checks.AllChecks { checkNames = append(checkNames, checkName) diff --git a/cmd/serve.go b/cmd/serve.go index 42917f1aaaf..8d5cb02feda 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -85,7 +85,7 @@ var serveCmd = &cobra.Command{ } if r.Header.Get("Content-Type") == "application/json" { - if err := repoResult.AsJSON(showDetails, rw); err != nil { + if err := repoResult.AsJSON(showDetails, *logLevel, rw); err != nil { sugar.Error(err) rw.WriteHeader(http.StatusInternalServerError) } diff --git a/cron/worker/main.go b/cron/worker/main.go index 0a3e5467ad2..d8aabfe0ea7 100644 --- a/cron/worker/main.go +++ b/cron/worker/main.go @@ -29,6 +29,7 @@ import ( "github.com/shurcooL/githubv4" "go.opencensus.io/stats/view" "go.uber.org/zap" + "go.uber.org/zap/zapcore" "github.com/ossf/scorecard/checker" "github.com/ossf/scorecard/checks" @@ -89,7 +90,7 @@ func processRequest(ctx context.Context, return fmt.Errorf("error during RunScorecards: %w", err) } result.Date = batchRequest.GetJobTime().AsTime().Format("2006-01-02") - if err := result.AsJSON(true /*showDetails*/, &buffer); err != nil { + if err := result.AsJSON(true /*showDetails*/, zapcore.InfoLevel, &buffer); err != nil { return fmt.Errorf("error during result.AsJSON: %w", err) } } diff --git a/errors/errors.md b/errors/errors.md new file mode 100644 index 00000000000..01ee98e99e8 --- /dev/null +++ b/errors/errors.md @@ -0,0 +1,28 @@ +# How to handle errors + +```golang +import sce "github.com/ossf/scorecard/errors" + +// Public errors are defined in errors/public.go and are exposed to callers. +// Internal errors are defined in errors/innternal.go. Their names start with ErrInternalXXX + +// Examples: + +// Return a standard check run failure, with an error message from an internal error. +// We only create internal errors for errors that may happen in several places in the code: this provides +// consistent erorr messages to the caller. +return sce.Create(sce.ErrRunFailure, ErrInternalInvalidYamlFile.Error()) + +// Return a standard check run failure, with an error message from an internal error and an API call error. +err := dependency.apiCall() +if err != nil { + return sce.Create(sce.ErrRunFailure, fmt.Sprintf("%v: %v", sce.ErrInternalSomething, err)) +} + +// Return a standard check run failure, only with API call error. Use this format when there is no internal error associated +// to the failure. In many cases, we don't need internal errors. +err := dependency.apiCall() +if err != nil { + return sce.Create(sce.ErrRunFailure, fmt.Sprintf("dependency.apiCall: %v", err)) +} +``` \ No newline at end of file diff --git a/errors/internal.go b/errors/internal.go new file mode 100644 index 00000000000..1b2844c1f28 --- /dev/null +++ b/errors/internal.go @@ -0,0 +1,30 @@ +// Copyright 2020 Security 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 errors + +import ( + "errors" +) + +//nolint +var ( + ErrInternalInvalidDockerFile = errors.New("invalid Dockerfile") + ErrInternalInvalidYamlFile = errors.New("invalid yaml file") + ErrInternalFilenameMatch = errors.New("filename match error") + ErrInternalEmptyFile = errors.New("empty file") + ErrInternalInvalidShellCode = errors.New("invalid shell code") + ErrCommitishNil = errors.New("commitish is nil") + ErrBranchNotFound = errors.New("branch not found") +) diff --git a/errors/names.go b/errors/names.go index b78162b5ccc..81df9e14c46 100644 --- a/errors/names.go +++ b/errors/names.go @@ -21,23 +21,23 @@ import ( const ( // RetryError occurs when checks fail after exhausting all retry attempts. RetryError = "RetryError" - // ZeroConfidenceError shows an inconclusive result. - ZeroConfidenceError = "ZeroConfidenceError" + // LowConfidenceError shows a low-confidence result. + LowConfidenceError = "LowConfidenceError" // UnknownError for all error types not handled. UnknownError = "UnknownError" ) var ( - errRetry *ErrRetry - errZeroConfidence *ErrZeroConfidence + errRetry *ErrRetry + errLowConfidence *ErrLowConfidence ) func GetErrorName(err error) string { switch { case errors.As(err, &errRetry): return RetryError - case errors.As(err, &errZeroConfidence): - return ZeroConfidenceError + case errors.As(err, &errLowConfidence): + return LowConfidenceError default: return UnknownError } diff --git a/errors/public.go b/errors/public.go new file mode 100644 index 00000000000..c0fc4ba5ef7 --- /dev/null +++ b/errors/public.go @@ -0,0 +1,38 @@ +// Copyright 2020 Security 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 errors + +import ( + "errors" + "fmt" +) + +// UPGRADEv2: delete other files in folder. +//nolint +var ( + ErrRunFailure = errors.New("cannot run check") + ErrRepoUnreachable = errors.New("repo unreachable") +) + +// Create a public error using any of the errors +// listed above. For examples, see errors/errors.md. +func Create(e error, msg string) error { + // Note: Errorf automatically wraps the error when used with `%w`. + if len(msg) > 0 { + return fmt.Errorf("%w: %v", e, msg) + } + // We still need to use %w to prevent callers from using e == ErrInvalidDockerFile. + return fmt.Errorf("%w", e) +} diff --git a/errors/types.go b/errors/types.go index 421324b3d15..414102cfbd2 100644 --- a/errors/types.go +++ b/errors/types.go @@ -19,8 +19,8 @@ import ( ) type ( - ErrRetry struct{ wrappedError } - ErrZeroConfidence struct{ wrappedError } + ErrRetry struct{ wrappedError } + ErrLowConfidence struct{ wrappedError } ) func MakeRetryError(err error) error { @@ -32,10 +32,10 @@ func MakeRetryError(err error) error { } } -func MakeZeroConfidenceError(err error) error { - return &ErrZeroConfidence{ +func MakeLowConfidenceError(err error) error { + return &ErrLowConfidence{ wrappedError{ - msg: "check result was unknown", + msg: "low confidence check result", innerError: err, }, } diff --git a/pkg/scorecard.go b/pkg/scorecard.go index d712532f1d1..926ceb25a76 100644 --- a/pkg/scorecard.go +++ b/pkg/scorecard.go @@ -14,6 +14,7 @@ package pkg +//nolint:gci import ( "context" "fmt" @@ -22,14 +23,13 @@ import ( "time" "github.com/google/go-github/v32/github" - "github.com/shurcooL/githubv4" - opencensusstats "go.opencensus.io/stats" - "go.opencensus.io/tag" - "github.com/ossf/scorecard/checker" "github.com/ossf/scorecard/clients" "github.com/ossf/scorecard/repos" "github.com/ossf/scorecard/stats" + "github.com/shurcooL/githubv4" + opencensusstats "go.opencensus.io/stats" + "go.opencensus.io/tag" ) func logStats(ctx context.Context, startTime time.Time) { diff --git a/pkg/scorecard_result.go b/pkg/scorecard_result.go index ef12a9a128b..75bba2af74e 100644 --- a/pkg/scorecard_result.go +++ b/pkg/scorecard_result.go @@ -14,6 +14,7 @@ package pkg +//nolint:gci import ( "encoding/csv" "encoding/json" @@ -25,8 +26,8 @@ import ( "strings" "github.com/olekukonko/tablewriter" - "github.com/ossf/scorecard/checker" + "go.uber.org/zap/zapcore" ) type ScorecardResult struct { @@ -38,7 +39,7 @@ type ScorecardResult struct { // AsJSON outputs the result in JSON format with a newline at the end. // If called on []ScorecardResult will create NDJson formatted output. -func (r *ScorecardResult) AsJSON(showDetails bool, writer io.Writer) error { +func (r *ScorecardResult) AsJSON(showDetails bool, logLevel zapcore.Level, writer io.Writer) error { encoder := json.NewEncoder(writer) if showDetails { if err := encoder.Encode(r); err != nil { @@ -51,6 +52,8 @@ func (r *ScorecardResult) AsJSON(showDetails bool, writer io.Writer) error { Date: r.Date, Metadata: r.Metadata, } + // UPGRADEv2: remove nolint after uggrade. + //nolint for _, checkResult := range r.Checks { tmpResult := checker.CheckResult{ Name: checkResult.Name, @@ -65,10 +68,12 @@ func (r *ScorecardResult) AsJSON(showDetails bool, writer io.Writer) error { return nil } -func (r *ScorecardResult) AsCSV(showDetails bool, writer io.Writer) error { +func (r *ScorecardResult) AsCSV(showDetails bool, logLevel zapcore.Level, writer io.Writer) error { w := csv.NewWriter(writer) record := []string{r.Repo} columns := []string{"Repository"} + // UPGRADEv2: remove nolint after uggrade. + //nolint for _, checkResult := range r.Checks { columns = append(columns, checkResult.Name+"_Pass", checkResult.Name+"_Confidence") record = append(record, strconv.FormatBool(checkResult.Pass), @@ -89,8 +94,10 @@ func (r *ScorecardResult) AsCSV(showDetails bool, writer io.Writer) error { return nil } -func (r *ScorecardResult) AsString(showDetails bool, writer io.Writer) error { +// UPGRADEv2: will be removed. +func (r *ScorecardResult) AsString(showDetails bool, logLevel zapcore.Level, writer io.Writer) error { sortedChecks := make([]checker.CheckResult, len(r.Checks)) + //nolint for i, checkResult := range r.Checks { sortedChecks[i] = checkResult } @@ -102,6 +109,7 @@ func (r *ScorecardResult) AsString(showDetails bool, writer io.Writer) error { }) data := make([][]string, len(sortedChecks)) + //nolint for i, row := range sortedChecks { const withdetails = 4 const withoutdetails = 3 @@ -117,7 +125,19 @@ func (r *ScorecardResult) AsString(showDetails bool, writer io.Writer) error { x[1] = strconv.Itoa(row.Confidence) x[2] = row.Name if showDetails { - x[3] = strings.Join(row.Details, "\n") + //nolint + if row.Version == 2 { + var sa []string + for _, v := range row.Details2 { + if v.Type == checker.DetailDebug && logLevel != zapcore.DebugLevel { + continue + } + sa = append(sa, fmt.Sprintf("%s: %s", typeToString(v.Type), v.Msg)) + } + x[3] = strings.Join(sa, "\n") + } else { + x[3] = strings.Join(row.Details, "\n") + } } data[i] = x } @@ -135,9 +155,105 @@ func (r *ScorecardResult) AsString(showDetails bool, writer io.Writer) error { table.SetCenterSeparator("|") table.AppendBulk(data) table.Render() + + return nil +} + +// UPGRADEv2: new code. +func (r *ScorecardResult) AsString2(showDetails bool, logLevel zapcore.Level, writer io.Writer) error { + sortedChecks := make([]checker.CheckResult, len(r.Checks)) + //nolint + // UPGRADEv2: not needed after upgrade. + for i, checkResult := range r.Checks { + sortedChecks[i] = checkResult + } + sort.Slice(sortedChecks, func(i, j int) bool { + if sortedChecks[i].Pass == sortedChecks[j].Pass { + return sortedChecks[i].Name < sortedChecks[j].Name + } + return sortedChecks[i].Pass + }) + + data := make([][]string, len(sortedChecks)) + //nolint + // UPGRADEv2: not needed after upgrade. + for i, row := range sortedChecks { + //nolint + if row.Version != 2 { + continue + } + const withdetails = 5 + const withoutdetails = 4 + var x []string + + if showDetails { + x = make([]string, withdetails) + } else { + x = make([]string, withoutdetails) + } + + // UPGRADEv2: rename variable. + if row.Score2 == checker.InconclusiveResultScore { + x[0] = "?" + } else { + x[0] = fmt.Sprintf("%d", row.Score2) + } + + doc := fmt.Sprintf("https://github.com/ossf/scorecard/blob/main/checks/checks.md#%s", strings.ToLower(row.Name)) + x[1] = row.Reason2 + x[2] = row.Name + if showDetails { + // UPGRADEv2: change to make([]string, len(row.Details)) + // followed by sa[i] = instead of append + var sa []string + for _, v := range row.Details2 { + if v.Type == checker.DetailDebug && logLevel != zapcore.DebugLevel { + continue + } + sa = append(sa, fmt.Sprintf("%s: %s", typeToString(v.Type), v.Msg)) + } + x[3] = strings.Join(sa, "\n") + x[4] = doc + } else { + x[3] = doc + } + + data[i] = x + } + + table := tablewriter.NewWriter(os.Stdout) + header := []string{"Score", "Reason", "Name"} + if showDetails { + header = append(header, "Details") + } + header = append(header, "Documentation/Remediation") + table.SetHeader(header) + table.SetBorders(tablewriter.Border{Left: true, Top: true, Right: true, Bottom: true}) + table.SetRowSeparator("-") + table.SetRowLine(true) + table.SetCenterSeparator("|") + table.AppendBulk(data) + table.SetAlignment(tablewriter.ALIGN_LEFT) + table.SetRowLine(true) + table.Render() + return nil } +func typeToString(cd checker.DetailType) string { + switch cd { + default: + panic("invalid detail") + case checker.DetailInfo: + return "Info" + case checker.DetailWarn: + return "Warn" + case checker.DetailDebug: + return "Debug" + } +} + +// UPGRADEv2: not needed after upgrade. func displayResult(result bool) string { if result { return "Pass" From 53d971739d9273834365ae5987faaf867e4905b9 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Mon, 19 Jul 2021 23:55:21 +0000 Subject: [PATCH 2/8] comment --- checker/check_request.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/checker/check_request.go b/checker/check_request.go index 661d3d8c7ba..aadc917cd05 100644 --- a/checker/check_request.go +++ b/checker/check_request.go @@ -30,8 +30,7 @@ type CheckRequest struct { GraphClient *githubv4.Client HTTPClient *http.Client RepoClient clients.RepoClient - // Note: Ultimately Log will be removed and replaced by - // CLogger. + // UPGRADEv2: Logf will be removed. Logf func(s string, f ...interface{}) Dlogger DetailLogger Owner, Repo string From fbfe601f049a6b80fe7a2cee50a2a2f519bb4ecf Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Mon, 19 Jul 2021 23:56:34 +0000 Subject: [PATCH 3/8] doc --- checks/write.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/checks/write.md b/checks/write.md index 955fdea0dfd..6e7cdeaf1f6 100644 --- a/checks/write.md +++ b/checks/write.md @@ -4,11 +4,11 @@ The steps to writting a check are as follow: 1. Create a file under `checks/` folder, say `checks/mycheck.go` 2. Give the check a name and register the check: ``` -// Note: do not export the name: start its name with a lower-case letter. -const checkMyCheckName string = "My-Check" +// Note: export the name: start its name with an upper-case letter. +const CheckMyCheckName string = "My-Check" func init() { - registerCheck(checkMyCheckName, EntryPointMyCheck) + registerCheck(CheckMyCheckName, EntryPointMyCheck) } ``` 3. Log information that is benfical to the user using `checker.DetailLogger`: From 34f5c0b25c41cf5004c2dcfc7a7c7435d7350813 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Tue, 20 Jul 2021 00:04:36 +0000 Subject: [PATCH 4/8] nits --- checker/check_result.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/checker/check_result.go b/checker/check_result.go index 0c14e05955b..b2fbfd58c26 100644 --- a/checker/check_result.go +++ b/checker/check_result.go @@ -53,7 +53,7 @@ type CheckResult struct { // UPGRADEv2: New structure. Omitting unchanged Name field // for simplicity. - Version int // 2. Default value of 0 indicates old structure + Version int `json:"-"` // Default value of 0 indicates old structure. Error2 error `json:"-"` // Runtime error indicate a filure to run the check. Details2 []CheckDetail `json:"-"` // Details of tests and sub-checks Score2 int `json:"-"` // {[0...1], -1 = Inconclusive} @@ -61,7 +61,7 @@ type CheckResult struct { } // CreateResultWithScore is used when -// the check runs without runtime errors and want to assign a +// the check runs without runtime errors and we want to assign a // specific score. func CreateResultWithScore(name, reason string, score int) CheckResult { pass := true @@ -92,8 +92,7 @@ func CreateResultWithScore(name, reason string, score int) CheckResult { // the the number of tests that succeeded. func CreateProportionalScoreResult(name, reason string, b, t int) CheckResult { pass := true - //nolint - score := int(math.Min(float64(10*b/t), float64(10))) + score := int(math.Min(float64(MaxResultScore*b/t), float64(MaxResultScore))) //nolint if score < 8 { pass = false @@ -165,7 +164,7 @@ func CreateRuntimeErrorResult(name string, e error) CheckResult { } } -// UPGRADEv2: will be renamed. +// UPGRADEv2: will be renaall functions belowed will be removed. func MakeAndResult2(checks ...CheckResult) CheckResult { if len(checks) == 0 { // That should never happen. @@ -183,7 +182,6 @@ func MakeAndResult2(checks ...CheckResult) CheckResult { return worseResult } -// UPGRADEv2: will be removed. func MakeInconclusiveResult(name string, err error) CheckResult { return CheckResult{ Name: name, @@ -220,8 +218,6 @@ func MakeRetryResult(name string, err error) CheckResult { } } -// TODO: update this function to return a ResultDontKnow -// if the confidence is low? func MakeProportionalResult(name string, numerator int, denominator int, threshold float32) CheckResult { if denominator == 0 { From 00ea0de8d8e09af12d0eda62316c1a77b6245944 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Tue, 20 Jul 2021 00:15:09 +0000 Subject: [PATCH 5/8] typo --- errors/internal.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/errors/internal.go b/errors/internal.go index 1b2844c1f28..0ee0f75fa02 100644 --- a/errors/internal.go +++ b/errors/internal.go @@ -25,6 +25,6 @@ var ( ErrInternalFilenameMatch = errors.New("filename match error") ErrInternalEmptyFile = errors.New("empty file") ErrInternalInvalidShellCode = errors.New("invalid shell code") - ErrCommitishNil = errors.New("commitish is nil") - ErrBranchNotFound = errors.New("branch not found") + ErrInternalCommitishNil = errors.New("commitish is nil") + ErrInternalBranchNotFound = errors.New("branch not found") ) From 493da6af5e9cb183f9b74b5e6a0b35a16bfe2a38 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Tue, 20 Jul 2021 16:33:26 +0000 Subject: [PATCH 6/8] commments --- checker/check_result.go | 58 +++++++++++++++++++++++---------- checker/check_runner.go | 28 +++------------- checks/branch_protected_test.go | 1 + checks/write.md | 2 +- clients/repo_client.go | 1 + cmd/root.go | 9 +++-- errors/errors.md | 8 ++--- errors/internal.go | 30 ----------------- errors/public.go | 4 +-- pkg/scorecard_result.go | 48 +++++++++++++++------------ 10 files changed, 85 insertions(+), 104 deletions(-) delete mode 100644 errors/internal.go diff --git a/checker/check_result.go b/checker/check_result.go index b2fbfd58c26..f492c8514ce 100644 --- a/checker/check_result.go +++ b/checker/check_result.go @@ -29,10 +29,35 @@ const ( MinResultConfidence = 0 ) +// UPGRADEv2: to remove. +const migrationThresholdPassValue = 8 + // ErrorDemoninatorZero indicates the denominator for a proportional result is 0. // UPGRADEv2: to remove. var ErrorDemoninatorZero = errors.New("internal error: denominator is 0") +// Types of details. +type DetailType int + +const ( + DetailInfo DetailType = iota + DetailWarn + DetailDebug +) + +// CheckDetail contains information for each detail. +//nolint:govet +type CheckDetail struct { + Type DetailType // Any of DetailWarn, DetailInfo, DetailDebug. + Msg string // A short string explaining why the details was recorded/logged.. +} + +type DetailLogger interface { + Info(desc string, args ...interface{}) + Warn(desc string, args ...interface{}) + Debug(desc string, args ...interface{}) +} + //nolint const ( MaxResultScore = 10 @@ -56,8 +81,8 @@ type CheckResult struct { Version int `json:"-"` // Default value of 0 indicates old structure. Error2 error `json:"-"` // Runtime error indicate a filure to run the check. Details2 []CheckDetail `json:"-"` // Details of tests and sub-checks - Score2 int `json:"-"` // {[0...1], -1 = Inconclusive} - Reason2 string `json:"-"` // A sentence describing the check result (score, etc) + Score int `json:"-"` // {[-1,0...10], -1 = Inconclusive} + Reason string `json:"-"` // A sentence describing the check result (score, etc) } // CreateResultWithScore is used when @@ -65,8 +90,7 @@ type CheckResult struct { // specific score. func CreateResultWithScore(name, reason string, score int) CheckResult { pass := true - //nolint - if score < 8 { + if score < migrationThresholdPassValue { pass = false } return CheckResult{ @@ -80,8 +104,8 @@ func CreateResultWithScore(name, reason string, score int) CheckResult { //nolint Version: 2, Error2: nil, - Score2: score, - Reason2: reason, + Score: score, + Reason: reason, } } @@ -93,24 +117,22 @@ func CreateResultWithScore(name, reason string, score int) CheckResult { func CreateProportionalScoreResult(name, reason string, b, t int) CheckResult { pass := true score := int(math.Min(float64(MaxResultScore*b/t), float64(MaxResultScore))) - //nolint - if score < 8 { + if score < migrationThresholdPassValue { pass = false } return CheckResult{ Name: name, // Old structure. - Error: nil, - //nolint - Confidence: 10, + Error: nil, + Confidence: MaxResultConfidence, Pass: pass, ShouldRetry: false, // New structure. //nolint Version: 2, Error2: nil, - Score2: score, - Reason2: fmt.Sprintf("%v -- score normalized to %d", reason, score), + Score: score, + Reason: fmt.Sprintf("%v -- score normalized to %d", reason, score), } } @@ -141,8 +163,8 @@ func CreateInconclusiveResult(name, reason string) CheckResult { // New structure. //nolint Version: 2, - Score2: InconclusiveResultScore, - Reason2: reason, + Score: InconclusiveResultScore, + Reason: reason, } } @@ -159,8 +181,8 @@ func CreateRuntimeErrorResult(name string, e error) CheckResult { //nolint Version: 2, Error2: e, - Score2: InconclusiveResultScore, - Reason2: e.Error(), // Note: message already accessible by caller thru `Error`. + Score: InconclusiveResultScore, + Reason: e.Error(), // Note: message already accessible by caller thru `Error`. } } @@ -175,7 +197,7 @@ func MakeAndResult2(checks ...CheckResult) CheckResult { // UPGRADEv2: will go away after old struct is removed. //nolint for _, result := range checks[1:] { - if result.Score2 < worseResult.Score2 { + if result.Score < worseResult.Score { worseResult = result } } diff --git a/checker/check_runner.go b/checker/check_runner.go index 08855e9a10f..d97581ac9c6 100644 --- a/checker/check_runner.go +++ b/checker/check_runner.go @@ -39,28 +39,6 @@ type CheckFn func(*CheckRequest) CheckResult type CheckNameToFnMap map[string]CheckFn -// Types of details. -type DetailType int - -const ( - DetailInfo DetailType = iota - DetailWarn - DetailDebug -) - -// CheckDetail contains information for each detail. -//nolint:govet -type CheckDetail struct { - Type DetailType // Any of DetailWarn, DetailInfo, DetailDebug. - Msg string // A short string explaining why the details was recorded/logged.. -} - -type DetailLogger interface { - Info(desc string, args ...interface{}) - Warn(desc string, args ...interface{}) - Debug(desc string, args ...interface{}) -} - // UPGRADEv2: messages2 will ultimately // be renamed to messages. type logger struct { @@ -152,9 +130,13 @@ func MultiCheckOr2(fns ...CheckFn) CheckFn { for _, fn := range fns { result := fn(c) - if result.Score2 > maxResult.Score2 { + if result.Score > maxResult.Score { maxResult = result } + + if maxResult.Score >= MaxResultScore { + break + } } return maxResult } diff --git a/checks/branch_protected_test.go b/checks/branch_protected_test.go index be5845c4b45..106fdc229e4 100644 --- a/checks/branch_protected_test.go +++ b/checks/branch_protected_test.go @@ -88,6 +88,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { //nolint:tparallel // mock rel1 := "release/v.1" sha := "8fb3cb86082b17144a80402f5367ae65f06083bd" main := "main" + //nolint tests := []struct { name string branches []*string diff --git a/checks/write.md b/checks/write.md index 6e7cdeaf1f6..0f08a7e95e4 100644 --- a/checks/write.md +++ b/checks/write.md @@ -20,7 +20,7 @@ if an error is returned from an API you call, use the function. 5. Create the result of the check as follow: * Always provide a high-level sentence explaining the result/score of the check. * If the check runs properly but is unable to determine a score, use `checker.CreateInconclusiveResult()` function. - * For propertional results, use `checker.CreateProportionalScoreResult()`. + * For proportional results, use `checker.CreateProportionalScoreResult()`. * For maximum score, use `checker.CreateMaxScoreResult()`; for min score use `checker.CreateMinScoreResult()`. * If you need more flexibility and need to set a specific score, use `checker.CreateResultWithScore()` with one of the constants declared, such as `checker.HalfResultScore`. 6. Dealing with errors: see [../errors/errors.md](errors/errors/md). diff --git a/clients/repo_client.go b/clients/repo_client.go index 1643c625250..66fd1ca6210 100644 --- a/clients/repo_client.go +++ b/clients/repo_client.go @@ -18,6 +18,7 @@ import ( "fmt" ) +// UPGRADEv2: use ErrRepoUnreachable instead. type ErrRepoUnavailable struct { innerError error } diff --git a/cmd/root.go b/cmd/root.go index 7bf56267d67..e5f2f4061b3 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -51,9 +51,6 @@ var ( pypi string rubygems string showDetails bool - // UPGRADEv2: will be removed. - v2 bool - // TODO(laurent): add explain command. // ErrorInvalidFormatFlag indicates an invalid option was passed for the 'format' argument. ErrorInvalidFormatFlag = errors.New("invalid format flag") ) @@ -70,6 +67,9 @@ or ./scorecard --{npm,pypi,rubgems}= [--checks=check1,...] [--show Short: "Security Scorecards", Long: "A program that shows security scorecard for an open source software.", Run: func(cmd *cobra.Command, args []string) { + // UPGRADEv2: to remove. + _, v2 := os.LookupEnv("SCORECARD_V2") + fmt.Printf("*** USING v2 MIGRATION CODE ***\n\n") cfg := zap.NewProductionConfig() cfg.Level.SetLevel(*logLevel) logger, err := cfg.Build() @@ -160,6 +160,7 @@ or ./scorecard --{npm,pypi,rubgems}= [--checks=check1,...] [--show switch format { case formatDefault: + // UPGRADEv2: to remove. if v2 { err = repoResult.AsString2(showDetails, *logLevel, os.Stdout) } else { @@ -313,8 +314,6 @@ func init() { rootCmd.Flags().StringSliceVar( &metaData, "metadata", []string{}, "metadata for the project.It can be multiple separated by commas") rootCmd.Flags().BoolVar(&showDetails, "show-details", false, "show extra details about each check") - // UPGRADEv2: will be removed. - rootCmd.Flags().BoolVar(&v2, "v2", false, "temporary flag to display v2 changes") checkNames := []string{} for checkName := range checks.AllChecks { checkNames = append(checkNames, checkName) diff --git a/errors/errors.md b/errors/errors.md index 01ee98e99e8..6060729757b 100644 --- a/errors/errors.md +++ b/errors/errors.md @@ -4,19 +4,19 @@ import sce "github.com/ossf/scorecard/errors" // Public errors are defined in errors/public.go and are exposed to callers. -// Internal errors are defined in errors/innternal.go. Their names start with ErrInternalXXX +// Internal errors are defined in checks/errors.go. Their names start with errInternalXXX // Examples: // Return a standard check run failure, with an error message from an internal error. // We only create internal errors for errors that may happen in several places in the code: this provides -// consistent erorr messages to the caller. -return sce.Create(sce.ErrRunFailure, ErrInternalInvalidYamlFile.Error()) +// consistent error messages to the caller. +return sce.Create(sce.ErrRunFailure, errInternalInvalidYamlFile.Error()) // Return a standard check run failure, with an error message from an internal error and an API call error. err := dependency.apiCall() if err != nil { - return sce.Create(sce.ErrRunFailure, fmt.Sprintf("%v: %v", sce.ErrInternalSomething, err)) + return sce.Create(sce.ErrRunFailure, fmt.Sprintf("%v: %v", errInternalSomething, err)) } // Return a standard check run failure, only with API call error. Use this format when there is no internal error associated diff --git a/errors/internal.go b/errors/internal.go deleted file mode 100644 index 0ee0f75fa02..00000000000 --- a/errors/internal.go +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright 2020 Security 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 errors - -import ( - "errors" -) - -//nolint -var ( - ErrInternalInvalidDockerFile = errors.New("invalid Dockerfile") - ErrInternalInvalidYamlFile = errors.New("invalid yaml file") - ErrInternalFilenameMatch = errors.New("filename match error") - ErrInternalEmptyFile = errors.New("empty file") - ErrInternalInvalidShellCode = errors.New("invalid shell code") - ErrInternalCommitishNil = errors.New("commitish is nil") - ErrInternalBranchNotFound = errors.New("branch not found") -) diff --git a/errors/public.go b/errors/public.go index c0fc4ba5ef7..7a1b23487a0 100644 --- a/errors/public.go +++ b/errors/public.go @@ -22,8 +22,8 @@ import ( // UPGRADEv2: delete other files in folder. //nolint var ( - ErrRunFailure = errors.New("cannot run check") - ErrRepoUnreachable = errors.New("repo unreachable") + ErrScorecardInternal = errors.New("internal error") + ErrRepoUnreachable = errors.New("repo unreachable") ) // Create a public error using any of the errors diff --git a/pkg/scorecard_result.go b/pkg/scorecard_result.go index 75bba2af74e..4bbbb8054ef 100644 --- a/pkg/scorecard_result.go +++ b/pkg/scorecard_result.go @@ -14,7 +14,6 @@ package pkg -//nolint:gci import ( "encoding/csv" "encoding/json" @@ -26,8 +25,9 @@ import ( "strings" "github.com/olekukonko/tablewriter" - "github.com/ossf/scorecard/checker" "go.uber.org/zap/zapcore" + + "github.com/ossf/scorecard/checker" ) type ScorecardResult struct { @@ -127,14 +127,11 @@ func (r *ScorecardResult) AsString(showDetails bool, logLevel zapcore.Level, wri if showDetails { //nolint if row.Version == 2 { - var sa []string - for _, v := range row.Details2 { - if v.Type == checker.DetailDebug && logLevel != zapcore.DebugLevel { - continue - } - sa = append(sa, fmt.Sprintf("%s: %s", typeToString(v.Type), v.Msg)) + details, show := detailsToString(row.Details2, logLevel) + if !show { + continue } - x[3] = strings.Join(sa, "\n") + x[3] = details } else { x[3] = strings.Join(row.Details, "\n") } @@ -193,26 +190,21 @@ func (r *ScorecardResult) AsString2(showDetails bool, logLevel zapcore.Level, wr } // UPGRADEv2: rename variable. - if row.Score2 == checker.InconclusiveResultScore { + if row.Score == checker.InconclusiveResultScore { x[0] = "?" } else { - x[0] = fmt.Sprintf("%d", row.Score2) + x[0] = fmt.Sprintf("%d", row.Score) } doc := fmt.Sprintf("https://github.com/ossf/scorecard/blob/main/checks/checks.md#%s", strings.ToLower(row.Name)) - x[1] = row.Reason2 + x[1] = row.Reason x[2] = row.Name if showDetails { - // UPGRADEv2: change to make([]string, len(row.Details)) - // followed by sa[i] = instead of append - var sa []string - for _, v := range row.Details2 { - if v.Type == checker.DetailDebug && logLevel != zapcore.DebugLevel { - continue - } - sa = append(sa, fmt.Sprintf("%s: %s", typeToString(v.Type), v.Msg)) + details, show := detailsToString(row.Details2, logLevel) + if !show { + continue } - x[3] = strings.Join(sa, "\n") + x[3] = details x[4] = doc } else { x[3] = doc @@ -240,6 +232,20 @@ func (r *ScorecardResult) AsString2(showDetails bool, logLevel zapcore.Level, wr return nil } +func detailsToString(details []checker.CheckDetail, logLevel zapcore.Level) (string, bool) { + // UPGRADEv2: change to make([]string, len(details)) + // followed by sa[i] = instead of append. + //nolint + var sa []string + for _, v := range details { + if v.Type == checker.DetailDebug && logLevel != zapcore.DebugLevel { + continue + } + sa = append(sa, fmt.Sprintf("%s: %s", typeToString(v.Type), v.Msg)) + } + return strings.Join(sa, "\n"), len(sa) > 0 +} + func typeToString(cd checker.DetailType) string { switch cd { default: From 4836854fa08c77edaa87d816db6d9ddcb1bed6e0 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Tue, 20 Jul 2021 16:34:55 +0000 Subject: [PATCH 7/8] nit --- checker/check_result.go | 1 - 1 file changed, 1 deletion(-) diff --git a/checker/check_result.go b/checker/check_result.go index f492c8514ce..79c4e68a425 100644 --- a/checker/check_result.go +++ b/checker/check_result.go @@ -61,7 +61,6 @@ type DetailLogger interface { //nolint const ( MaxResultScore = 10 - HalfResultScore = 5 MinResultScore = 0 InconclusiveResultScore = -1 ) From 34740a53480e84c2de7dbec86c0296caa37826a6 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Tue, 20 Jul 2021 17:09:07 +0000 Subject: [PATCH 8/8] linter --- pkg/scorecard.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/scorecard.go b/pkg/scorecard.go index 926ceb25a76..d712532f1d1 100644 --- a/pkg/scorecard.go +++ b/pkg/scorecard.go @@ -14,7 +14,6 @@ package pkg -//nolint:gci import ( "context" "fmt" @@ -23,13 +22,14 @@ import ( "time" "github.com/google/go-github/v32/github" + "github.com/shurcooL/githubv4" + opencensusstats "go.opencensus.io/stats" + "go.opencensus.io/tag" + "github.com/ossf/scorecard/checker" "github.com/ossf/scorecard/clients" "github.com/ossf/scorecard/repos" "github.com/ossf/scorecard/stats" - "github.com/shurcooL/githubv4" - opencensusstats "go.opencensus.io/stats" - "go.opencensus.io/tag" ) func logStats(ctx context.Context, startTime time.Time) {