chore: enable go-critic (aquasecurity#5302)

* chore: enable gocritic Signed-off-by: knqyf263 <[email protected]> * refactor: fix lint issues Signed-off-by: knqyf263 <[email protected]> * test: return true for latest versions Signed-off-by: knqyf263 <[email protected]> * chore(lint): enforce map and slice styles Signed-off-by: knqyf263 <[email protected]> --------- Signed-off-by: knqyf263 <[email protected]>
lleon1435 · Oct 2, 2023 · 3be5e6b · 3be5e6b
1 parent f6cd21c
commit 3be5e6b
Show file tree

Hide file tree

Showing 103 changed files with 584 additions and 531 deletions.
diff --git a/.golangci.yaml b/.golangci.yaml
@@ -43,6 +43,28 @@ linters-settings:
             recommendations:
               - github.com/aquasecurity/go-version
             reason: "`aquasecurity/go-version` is designed for our use-cases"
+  gocritic:
+    disabled-checks:
+      - appendAssign
+      - unnamedResult
+      - whyNoLint
+      - indexAlloc
+      - octalLiteral
+      - hugeParam
+      - rangeValCopy
+      - regexpSimplify
+      - sloppyReassign
+      - commentedOutCode
+    enabled-tags:
+      - diagnostic
+      - style
+      - performance
+      - experimental
+      - opinionated
+    settings:
+      ruleguard:
+        failOn: all
+        rules: '${configDir}/misc/lint/rules.go'
 
 linters:
   disable-all: true
@@ -62,6 +84,7 @@ linters:
     - gci
     - gomodguard
     - tenv
+    - gocritic
 
 run:
   go: '1.20'

diff --git a/go.mod b/go.mod
@@ -75,6 +75,7 @@ require (
 	github.com/openvex/go-vex v0.2.5
 	github.com/owenrumney/go-sarif/v2 v2.2.2
 	github.com/package-url/packageurl-go v0.1.2-0.20230812223828-f8bb31c1f10b
+	github.com/quasilyte/go-ruleguard/dsl v0.3.22
 	github.com/samber/lo v1.38.1
 	github.com/saracen/walker v0.1.3
 	github.com/secure-systems-lab/go-securesystemslib v0.7.0

diff --git a/go.sum b/go.sum
@@ -1534,6 +1534,8 @@ github.com/prometheus/procfs v0.6.0/go.mod h1:cz+aTbrPOrUb4q7XlbU9ygM+/jj0fzG6c1
 github.com/prometheus/procfs v0.10.1 h1:kYK1Va/YMlutzCGazswoHKo//tZVlFpKYh+PymziUAg=
 github.com/prometheus/procfs v0.10.1/go.mod h1:nwNm2aOCAYw8uTR/9bWRREkZFxAUcWzPHWJq+XBB/FM=
 github.com/prometheus/tsdb v0.7.1/go.mod h1:qhTCs0VvXwvX/y3TZrWD7rabWM+ijKTux40TwIPHuXU=
+github.com/quasilyte/go-ruleguard/dsl v0.3.22 h1:wd8zkOhSNr+I+8Qeciml08ivDt1pSXe60+5DqOpCjPE=
+github.com/quasilyte/go-ruleguard/dsl v0.3.22/go.mod h1:KeCP03KrjuSO0H1kTuZQCWlQPulDV6YMIXmpQss17rU=
 github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475 h1:N/ElC8H3+5XpJzTSTfLsJV/mx9Q9g7kxmchpfZyxgzM=
 github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4=
 github.com/remyoudompheng/bigfft v0.0.0-20200410134404-eec4a21b6bb0/go.mod h1:qqbHyh8v60DhA7CoWK5oRCqLrMHRGoxYCSS9EjAz6Eo=

diff --git a/magefiles/magefile.go b/magefiles/magefile.go
@@ -393,11 +393,12 @@ func (Docs) Generate() error {
 func findProtoFiles() ([]string, error) {
 	var files []string
 	err := filepath.WalkDir("rpc", func(path string, d fs.DirEntry, err error) error {
-		if err != nil {
+		switch {
+		case err != nil:
 			return err
-		} else if d.IsDir() {
+		case d.IsDir():
 			return nil
-		} else if filepath.Ext(path) == ".proto" {
+		case filepath.Ext(path) == ".proto":
 			files = append(files, path)
 		}
 		return nil

diff --git a/misc/lint/rules.go b/misc/lint/rules.go
@@ -0,0 +1,22 @@
+//go:build ruleguard
+
+package gorules
+
+import "github.com/quasilyte/go-ruleguard/dsl"
+
+// cf. https://github.com/golang/go/wiki/CodeReviewComments#declaring-empty-slices
+func declareEmptySlices(m dsl.Matcher) {
+	m.Match(
+		`$name := []$t{}`,
+		`$name := make([]$t, 0)`,
+	).
+		Suggest(`var $name []$t`).
+		Report(`replace '$$' with 'var $name []$t'`)
+}
+
+// cf. https://github.com/uber-go/guide/blob/master/style.md#initializing-maps
+func initializeMaps(m dsl.Matcher) {
+	m.Match(`map[$key]$value{}`).
+		Suggest(`make(map[$key]$value)`).
+		Report(`replace '$$' with 'make(map[$key]$value)`)
+}
diff --git a/pkg/cloud/aws/cache/cache.go b/pkg/cloud/aws/cache/cache.go
@@ -37,7 +37,7 @@ var ErrCacheNotFound = fmt.Errorf("cache record not found")
 var ErrCacheIncompatible = fmt.Errorf("cache record used incomatible schema")
 var ErrCacheExpired = fmt.Errorf("cache record expired")
 
-func New(cacheDir string, maxCacheAge time.Duration, accountID string, region string) *Cache {
+func New(cacheDir string, maxCacheAge time.Duration, accountID, region string) *Cache {
 	return &Cache{
 		path:      path.Join(cacheDir, "cloud", "aws", accountID, strings.ToLower(region), "data.json"),
 		accountID: accountID,
@@ -70,7 +70,7 @@ func (c *Cache) load() (*CacheData, error) {
 	return &data, nil
 }
 
-func (c *Cache) ListServices(required []string) (included []string, missing []string) {
+func (c *Cache) ListServices(required []string) (included, missing []string) {
 
 	data, err := c.load()
 	if err != nil {
@@ -101,12 +101,11 @@ func (c *Cache) LoadState() (*state.State, error) {
 	return data.State, nil
 }
 
-func (c *Cache) AddServices(state *state.State, includedServices []string) error {
-
+func (c *Cache) AddServices(s *state.State, includedServices []string) error {
 	data := &CacheData{
 		SchemaVersion: SchemaVersion,
-		State:         state,
-		Services:      map[string]ServiceMetadata{},
+		State:         s,
+		Services:      make(map[string]ServiceMetadata),
 		Updated:       time.Now(),
 	}
 

diff --git a/pkg/cloud/aws/commands/run.go b/pkg/cloud/aws/commands/run.go
@@ -95,10 +95,11 @@ func processOptions(ctx context.Context, opt *flag.Options) error {
 }
 
 func filterServices(opt *flag.Options) error {
-	if len(opt.Services) == 0 && len(opt.SkipServices) == 0 {
+	switch {
+	case len(opt.Services) == 0 && len(opt.SkipServices) == 0:
 		log.Logger.Debug("No service(s) specified, scanning all services...")
 		opt.Services = allSupportedServicesFunc()
-	} else if len(opt.SkipServices) > 0 {
+	case len(opt.SkipServices) > 0:
 		log.Logger.Debug("excluding services: ", opt.SkipServices)
 		for _, s := range allSupportedServicesFunc() {
 			if slices.Contains(opt.SkipServices, s) {
@@ -108,7 +109,7 @@ func filterServices(opt *flag.Options) error {
 				opt.Services = append(opt.Services, s)
 			}
 		}
-	} else if len(opt.Services) > 0 {
+	case len(opt.Services) > 0:
 		log.Logger.Debugf("Specific services were requested: [%s]...", strings.Join(opt.Services, ", "))
 		for _, service := range opt.Services {
 			var found bool

diff --git a/pkg/cloud/aws/scanner/scanner.go b/pkg/cloud/aws/scanner/scanner.go
@@ -87,15 +87,19 @@ func (s *AWSScanner) Scan(ctx context.Context, option flag.Options) (scan.Result
 		return nil, false, xerrors.Errorf("unable to create policyfs: %w", err)
 	}
 
-	scannerOpts = append(scannerOpts, options.ScannerWithPolicyFilesystem(policyFS))
-	scannerOpts = append(scannerOpts, options.ScannerWithPolicyDirs(policyPaths...))
+	scannerOpts = append(scannerOpts,
+		options.ScannerWithPolicyFilesystem(policyFS),
+		options.ScannerWithPolicyDirs(policyPaths...),
+	)
 
 	dataFS, dataPaths, err := misconf.CreateDataFS(option.RegoOptions.DataPaths)
 	if err != nil {
 		log.Logger.Errorf("Could not load config data: %s", err)
 	}
-	scannerOpts = append(scannerOpts, options.ScannerWithDataDirs(dataPaths...))
-	scannerOpts = append(scannerOpts, options.ScannerWithDataFilesystem(dataFS))
+	scannerOpts = append(scannerOpts,
+		options.ScannerWithDataDirs(dataPaths...),
+		options.ScannerWithDataFilesystem(dataFS),
+	)
 
 	scannerOpts = addPolicyNamespaces(option.RegoOptions.PolicyNamespaces, scannerOpts)
 

diff --git a/pkg/commands/app.go b/pkg/commands/app.go
@@ -307,7 +307,7 @@ func NewImageCommand(globalFlags *flag.GlobalFlagGroup) *cobra.Command {
 func NewFilesystemCommand(globalFlags *flag.GlobalFlagGroup) *cobra.Command {
 	reportFlagGroup := flag.NewReportFlagGroup()
 	reportFormat := flag.ReportFormatFlag
-	reportFormat.Usage = "specify a compliance report format for the output" //@TODO: support --report summary for non compliance reports
+	reportFormat.Usage = "specify a compliance report format for the output" // @TODO: support --report summary for non compliance reports
 	reportFlagGroup.ReportFormat = &reportFormat
 	reportFlagGroup.ExitOnEOL = nil // disable '--exit-on-eol'
 
@@ -626,7 +626,7 @@ func NewConfigCommand(globalFlags *flag.GlobalFlagGroup) *cobra.Command {
 	reportFlagGroup.ListAllPkgs = nil    // disable '--list-all-pkgs'
 	reportFlagGroup.ExitOnEOL = nil      // disable '--exit-on-eol'
 	reportFormat := flag.ReportFormatFlag
-	reportFormat.Usage = "specify a compliance report format for the output" //@TODO: support --report summary for non compliance reports
+	reportFormat.Usage = "specify a compliance report format for the output" // @TODO: support --report summary for non compliance reports
 	reportFlagGroup.ReportFormat = &reportFormat
 
 	scanFlags := &flag.ScanFlagGroup{
@@ -1213,6 +1213,6 @@ func flagErrorFunc(command *cobra.Command, err error) error {
 	if err := command.Help(); err != nil {
 		return err
 	}
-	command.Println() //add empty line after list of flags
+	command.Println() // add empty line after list of flags
 	return err
 }
diff --git a/pkg/compliance/report/report.go b/pkg/compliance/report/report.go
@@ -67,7 +67,10 @@ type Writer interface {
 func Write(report *ComplianceReport, option Option) error {
 	switch option.Format {
 	case types.FormatJSON:
-		jwriter := JSONWriter{Output: option.Output, Report: option.Report}
+		jwriter := JSONWriter{
+			Output: option.Output,
+			Report: option.Report,
+		}
 		return jwriter.Write(report)
 	case types.FormatTable:
 		if !report.empty() {
@@ -93,7 +96,7 @@ func (r ComplianceReport) empty() bool {
 
 // buildControlCheckResults create compliance results data
 func buildControlCheckResults(checksMap map[string]types.Results, controls []defsecTypes.Control) []*ControlCheckResult {
-	complianceResults := make([]*ControlCheckResult, 0)
+	var complianceResults []*ControlCheckResult
 	for _, control := range controls {
 		var results types.Results
 		for _, c := range control.Checks {
@@ -112,14 +115,14 @@ func buildControlCheckResults(checksMap map[string]types.Results, controls []def
 }
 
 // buildComplianceReportResults create compliance results data
-func buildComplianceReportResults(checksMap map[string]types.Results, spec defsecTypes.Spec) *ComplianceReport {
-	controlCheckResult := buildControlCheckResults(checksMap, spec.Controls)
+func buildComplianceReportResults(checksMap map[string]types.Results, s defsecTypes.Spec) *ComplianceReport {
+	controlCheckResult := buildControlCheckResults(checksMap, s.Controls)
 	return &ComplianceReport{
-		ID:               spec.ID,
-		Title:            spec.Title,
-		Description:      spec.Description,
-		Version:          spec.Version,
-		RelatedResources: spec.RelatedResources,
+		ID:               s.ID,
+		Title:            s.Title,
+		Description:      s.Description,
+		Version:          s.Version,
+		RelatedResources: s.RelatedResources,
 		Results:          controlCheckResult,
 	}
 }

diff --git a/pkg/compliance/spec/compliance.go b/pkg/compliance/spec/compliance.go
@@ -29,7 +29,7 @@ const (
 
 // Scanners reads spec control and determines the scanners by check ID prefix
 func (cs *ComplianceSpec) Scanners() (types.Scanners, error) {
-	scannerTypes := map[types.Scanner]struct{}{}
+	scannerTypes := make(map[types.Scanner]struct{})
 	for _, control := range cs.Spec.Controls {
 		for _, check := range control.Checks {
 			scannerType := scannerByCheckID(check.ID)
@@ -44,7 +44,7 @@ func (cs *ComplianceSpec) Scanners() (types.Scanners, error) {
 
 // CheckIDs return list of compliance check IDs
 func (cs *ComplianceSpec) CheckIDs() map[types.Scanner][]string {
-	checkIDsMap := map[types.Scanner][]string{}
+	checkIDsMap := make(map[types.Scanner][]string)
 	for _, control := range cs.Spec.Controls {
 		for _, check := range control.Checks {
 			scannerType := scannerByCheckID(check.ID)

diff --git a/pkg/db/db.go b/pkg/db/db.go
@@ -52,9 +52,9 @@ func WithDBRepository(dbRepository string) Option {
 }
 
 // WithClock takes a clock
-func WithClock(clock clock.Clock) Option {
+func WithClock(c clock.Clock) Option {
 	return func(opts *options) {
-		opts.clock = clock
+		opts.clock = c
 	}
 }
 

diff --git a/pkg/detector/ospkg/alma/alma.go b/pkg/detector/ospkg/alma/alma.go
@@ -9,6 +9,7 @@ import (
 	"k8s.io/utils/clock"
 
 	"github.com/aquasecurity/trivy-db/pkg/vulnsrc/alma"
+	osver "github.com/aquasecurity/trivy/pkg/detector/ospkg/version"
 	ftypes "github.com/aquasecurity/trivy/pkg/fanal/types"
 	"github.com/aquasecurity/trivy/pkg/log"
 	"github.com/aquasecurity/trivy/pkg/scanner/utils"
@@ -30,9 +31,9 @@ type options struct {
 
 type option func(*options)
 
-func WithClock(clock clock.Clock) option {
+func WithClock(c clock.Clock) option {
 	return func(opts *options) {
-		opts.clock = clock
+		opts.clock = c
 	}
 }
 
@@ -60,9 +61,8 @@ func NewScanner(opts ...option) *Scanner {
 // Detect vulnerabilities in package using AlmaLinux scanner
 func (s *Scanner) Detect(osVer string, _ *ftypes.Repository, pkgs []ftypes.Package) ([]types.DetectedVulnerability, error) {
 	log.Logger.Info("Detecting AlmaLinux vulnerabilities...")
-	if strings.Count(osVer, ".") > 0 {
-		osVer = osVer[:strings.Index(osVer, ".")]
-	}
+
+	osVer = osver.Major(osVer)
 	log.Logger.Debugf("AlmaLinux: os version: %s", osVer)
 	log.Logger.Debugf("AlmaLinux: the number of packages: %d", len(pkgs))
 
@@ -107,19 +107,9 @@ func (s *Scanner) Detect(osVer string, _ *ftypes.Repository, pkgs []ftypes.Packa
 	return vulns, nil
 }
 
-// IsSupportedVersion checks the OSFamily can be scanned using AlmaLinux scanner
+// IsSupportedVersion checks if the version is supported.
 func (s *Scanner) IsSupportedVersion(osFamily ftypes.OSType, osVer string) bool {
-	if strings.Count(osVer, ".") > 0 {
-		osVer = osVer[:strings.Index(osVer, ".")]
-	}
-
-	eol, ok := eolDates[osVer]
-	if !ok {
-		log.Logger.Warnf("This OS version is not on the EOL list: %s %s", osFamily, osVer)
-		return false
-	}
-
-	return s.clock.Now().Before(eol)
+	return osver.Supported(s.clock, eolDates, osFamily, osver.Major(osVer))
 }
 
 func addModularNamespace(name, label string) string {

diff --git a/pkg/detector/ospkg/alma/alma_test.go b/pkg/detector/ospkg/alma/alma_test.go
@@ -204,13 +204,13 @@ func TestScanner_IsSupportedVersion(t *testing.T) {
 			want: false,
 		},
 		{
-			name: "unknown",
+			name: "latest",
 			now:  time.Date(2019, 5, 2, 23, 59, 59, 0, time.UTC),
 			args: args{
 				osFamily: "alma",
-				osVer:    "unknown",
+				osVer:    "999",
 			},
-			want: false,
+			want: true,
 		},
 	}
 	for _, tt := range tests {

diff --git a/pkg/detector/ospkg/alpine/alpine.go b/pkg/detector/ospkg/alpine/alpine.go
@@ -10,6 +10,7 @@ import (
 
 	dbTypes "github.com/aquasecurity/trivy-db/pkg/types"
 	"github.com/aquasecurity/trivy-db/pkg/vulnsrc/alpine"
+	osver "github.com/aquasecurity/trivy/pkg/detector/ospkg/version"
 	ftypes "github.com/aquasecurity/trivy/pkg/fanal/types"
 	"github.com/aquasecurity/trivy/pkg/log"
 	"github.com/aquasecurity/trivy/pkg/scanner/utils"
@@ -55,9 +56,9 @@ type options struct {
 
 type option func(*options)
 
-func WithClock(clock clock.Clock) option {
+func WithClock(c clock.Clock) option {
 	return func(opts *options) {
-		opts.clock = clock
+		opts.clock = c
 	}
 }
 
@@ -85,9 +86,7 @@ func NewScanner(opts ...option) *Scanner {
 // Detect vulnerabilities in package using Alpine scanner
 func (s *Scanner) Detect(osVer string, repo *ftypes.Repository, pkgs []ftypes.Package) ([]types.DetectedVulnerability, error) {
 	log.Logger.Info("Detecting Alpine vulnerabilities...")
-	if strings.Count(osVer, ".") > 1 {
-		osVer = osVer[:strings.LastIndex(osVer, ".")]
-	}
+	osVer = osver.Minor(osVer)
 	repoRelease := s.repoRelease(repo)
 
 	log.Logger.Debugf("alpine: os version: %s", osVer)
@@ -173,19 +172,9 @@ func (s *Scanner) isVulnerable(installedVersion version.Version, adv dbTypes.Adv
 	return installedVersion.LessThan(fixedVersion)
 }
 
-// IsSupportedVersion checks the OSFamily can be scanned using Alpine scanner
+// IsSupportedVersion checks if the version is supported.
 func (s *Scanner) IsSupportedVersion(osFamily ftypes.OSType, osVer string) bool {
-	if strings.Count(osVer, ".") > 1 {
-		osVer = osVer[:strings.LastIndex(osVer, ".")]
-	}
-
-	eol, ok := eolDates[osVer]
-	if !ok {
-		log.Logger.Infof("This OS version is not on the EOL list: %s %s", osFamily, osVer)
-		return true // may be the latest version
-	}
-
-	return s.clock.Now().Before(eol)
+	return osver.Supported(s.clock, eolDates, osFamily, osver.Minor(osVer))
 }
 
 func (s *Scanner) repoRelease(repo *ftypes.Repository) string {