From d92aea5578795b698ad0b93c96922acfef0b9cfd Mon Sep 17 00:00:00 2001 From: Oleksandr Redko Date: Thu, 4 Jan 2024 20:51:07 +0200 Subject: [PATCH 1/2] dev: replace raw loops with funcs from slices and maps --- go.mod | 2 +- go.sum | 2 ++ pkg/golinters/goanalysis/runner.go | 11 +++----- pkg/golinters/gocritic.go | 19 +++----------- pkg/golinters/govet_test.go | 33 +++++++++++------------- pkg/golinters/thelper.go | 6 ++--- pkg/lint/lintersdb/enabled_set.go | 12 +++------ pkg/lint/lintersdb/enabled_set_test.go | 4 +-- pkg/printers/checkstyle.go | 6 ++--- pkg/printers/junitxml.go | 6 ++--- pkg/result/processors/nolint.go | 7 +++-- scripts/expand_website_templates/main.go | 7 ++--- 12 files changed, 43 insertions(+), 72 deletions(-) diff --git a/go.mod b/go.mod index 92a81fb903b4..8d40e3d95c8e 100644 --- a/go.mod +++ b/go.mod @@ -122,7 +122,7 @@ require ( gitlab.com/bosi/decorder v0.4.1 go-simpler.org/musttag v0.8.0 go-simpler.org/sloglint v0.4.0 - golang.org/x/exp v0.0.0-20230510235704-dd950f8aeaea + golang.org/x/exp v0.0.0-20240103183307-be819d1f06fc golang.org/x/tools v0.16.1 gopkg.in/yaml.v3 v3.0.1 honnef.co/go/tools v0.4.6 diff --git a/go.sum b/go.sum index 0b979a22d37e..9bd52a46d74f 100644 --- a/go.sum +++ b/go.sum @@ -627,6 +627,8 @@ golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EH golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU= golang.org/x/exp v0.0.0-20230510235704-dd950f8aeaea h1:vLCWI/yYrdEHyN2JzIzPO3aaQJHQdp89IZBA/+azVC4= golang.org/x/exp v0.0.0-20230510235704-dd950f8aeaea/go.mod h1:V1LtkGg67GoY2N1AnLN78QLrzxkLyJw7RJb1gzOOz9w= +golang.org/x/exp v0.0.0-20240103183307-be819d1f06fc h1:ao2WRsKSzW6KuUY9IWPwWahcHCgR0s52IfwutMfEbdM= +golang.org/x/exp v0.0.0-20240103183307-be819d1f06fc/go.mod h1:iRJReGqOEeBhDZGkGbynYwcHlctCvnjTYIamk7uXpHI= golang.org/x/exp/typeparams v0.0.0-20220428152302-39d4317da171/go.mod h1:AbB0pIl9nAr9wVwH+Z2ZpaocVmF5I4GyWCDIsVjR0bk= golang.org/x/exp/typeparams v0.0.0-20230203172020-98cc5a0785f9/go.mod h1:AbB0pIl9nAr9wVwH+Z2ZpaocVmF5I4GyWCDIsVjR0bk= golang.org/x/exp/typeparams v0.0.0-20231219180239-dc181d75b848 h1:UhRVJ0i7bF9n/Hd8YjW3eKjlPVBHzbQdxrBgjbSKl64= diff --git a/pkg/golinters/goanalysis/runner.go b/pkg/golinters/goanalysis/runner.go index 46871bc5b287..a8660b612199 100644 --- a/pkg/golinters/goanalysis/runner.go +++ b/pkg/golinters/goanalysis/runner.go @@ -17,6 +17,7 @@ import ( "sort" "sync" + "golang.org/x/exp/maps" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/packages" @@ -159,10 +160,7 @@ func (r *runner) buildActionFactDeps(act *action, a *analysis.Analyzer, pkg *pac act.objectFacts = make(map[objectFactKey]analysis.Fact) act.packageFacts = make(map[packageFactKey]analysis.Fact) - paths := make([]string, 0, len(pkg.Imports)) - for path := range pkg.Imports { - paths = append(paths, path) - } + paths := maps.Keys(pkg.Imports) sort.Strings(paths) // for determinism for _, path := range paths { dep := r.makeAction(a, pkg.Imports[path], initialPkgs, actions, actAlloc) @@ -212,10 +210,7 @@ func (r *runner) prepareAnalysis(pkgs []*packages.Package, } } - allActions := make([]*action, 0, len(actions)) - for _, act := range actions { - allActions = append(allActions, act) - } + allActions := maps.Values(actions) debugf("Built %d actions", len(actions)) diff --git a/pkg/golinters/gocritic.go b/pkg/golinters/gocritic.go index 1319c72d9d42..3cf43afc6225 100644 --- a/pkg/golinters/gocritic.go +++ b/pkg/golinters/gocritic.go @@ -14,6 +14,7 @@ import ( "github.com/go-critic/go-critic/checkers" gocriticlinter "github.com/go-critic/go-critic/linter" + "golang.org/x/exp/maps" "golang.org/x/tools/go/analysis" "github.com/golangci/golangci-lint/pkg/config" @@ -219,10 +220,7 @@ func (w *goCriticWrapper) configureCheckerInfo(info *gocriticlinter.CheckerInfo, info.Name, k) } - var supportedKeys []string - for sk := range info.Params { - supportedKeys = append(supportedKeys, sk) - } + supportedKeys := maps.Keys(info.Params) sort.Strings(supportedKeys) return fmt.Errorf("checker %s config param %s doesn't exist, all existing: %s", @@ -311,11 +309,7 @@ func (s *goCriticSettingsWrapper) checkerTagsDebugf() { tagToCheckers := s.buildTagToCheckersMap() - allTags := make([]string, 0, len(tagToCheckers)) - for tag := range tagToCheckers { - allTags = append(allTags, tag) - } - + allTags := maps.Keys(tagToCheckers) sort.Strings(allTags) goCriticDebugf("All gocritic existing tags and checks:") @@ -609,12 +603,7 @@ func intersectStringSlice(s1, s2 []string) []string { } func sprintAllowedCheckerNames(allowedNames map[string]bool) string { - namesSlice := make([]string, 0, len(allowedNames)) - - for name := range allowedNames { - namesSlice = append(namesSlice, name) - } - + namesSlice := maps.Keys(allowedNames) return sprintStrings(namesSlice) } diff --git a/pkg/golinters/govet_test.go b/pkg/golinters/govet_test.go index 020870506067..8e19016bda0b 100644 --- a/pkg/golinters/govet_test.go +++ b/pkg/golinters/govet_test.go @@ -1,9 +1,9 @@ package golinters import ( - "sort" "testing" + "golang.org/x/exp/slices" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis/passes/asmdecl" "golang.org/x/tools/go/analysis/passes/assign" @@ -18,39 +18,36 @@ import ( func TestGovet(t *testing.T) { // Checking that every default analyzer is in "all analyzers" list. - var checkList []*analysis.Analyzer - checkList = append(checkList, defaultAnalyzers...) + checkList := append([]*analysis.Analyzer{}, defaultAnalyzers...) checkList = append(checkList, shadow.Analyzer) // special case, used in analyzersFromConfig for _, defaultAnalyzer := range checkList { - found := false - for _, a := range allAnalyzers { - if a.Name == defaultAnalyzer.Name { - found = true - break - } - } + found := slices.ContainsFunc(allAnalyzers, func(a *analysis.Analyzer) bool { + return a.Name == defaultAnalyzer.Name + }) if !found { t.Errorf("%s is not in allAnalyzers", defaultAnalyzer.Name) } } } -type sortedAnalyzers []*analysis.Analyzer - -func (p sortedAnalyzers) Len() int { return len(p) } -func (p sortedAnalyzers) Less(i, j int) bool { return p[i].Name < p[j].Name } -func (p sortedAnalyzers) Swap(i, j int) { p[i].Name, p[j].Name = p[j].Name, p[i].Name } - func TestGovetSorted(t *testing.T) { // Keeping analyzers sorted so their order match the import order. + cmp := func(a, b *analysis.Analyzer) int { + if a.Name < b.Name { + return -1 + } else if a.Name > b.Name { + return 1 + } + return 0 + } t.Run("All", func(t *testing.T) { - if !sort.IsSorted(sortedAnalyzers(allAnalyzers)) { + if !slices.IsSortedFunc(allAnalyzers, cmp) { t.Error("please keep all analyzers list sorted by name") } }) t.Run("Default", func(t *testing.T) { - if !sort.IsSorted(sortedAnalyzers(defaultAnalyzers)) { + if !slices.IsSortedFunc(defaultAnalyzers, cmp) { t.Error("please keep default analyzers list sorted by name") } }) diff --git a/pkg/golinters/thelper.go b/pkg/golinters/thelper.go index 9b372994909d..1ae85ef42e1f 100644 --- a/pkg/golinters/thelper.go +++ b/pkg/golinters/thelper.go @@ -4,6 +4,7 @@ import ( "strings" "github.com/kulti/thelper/pkg/analyzer" + "golang.org/x/exp/maps" "golang.org/x/tools/go/analysis" "github.com/golangci/golangci-lint/pkg/config" @@ -42,10 +43,7 @@ func NewThelper(cfg *config.ThelperSettings) *goanalysis.Linter { linterLogger.Fatalf("thelper: at least one option must be enabled") } - var args []string - for k := range opts { - args = append(args, k) - } + args := maps.Keys(opts) cfgMap := map[string]map[string]any{ a.Name: { diff --git a/pkg/lint/lintersdb/enabled_set.go b/pkg/lint/lintersdb/enabled_set.go index c5c7874e4508..6f7b91b4d139 100644 --- a/pkg/lint/lintersdb/enabled_set.go +++ b/pkg/lint/lintersdb/enabled_set.go @@ -4,6 +4,8 @@ import ( "os" "sort" + "golang.org/x/exp/maps" + "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/golangci/golangci-lint/pkg/lint/linter" @@ -115,10 +117,7 @@ func (es EnabledSet) GetOptimizedLinters() ([]*linter.Config, error) { es.verbosePrintLintersStatus(resultLintersSet) es.combineGoAnalysisLinters(resultLintersSet) - var resultLinters []*linter.Config - for _, lc := range resultLintersSet { - resultLinters = append(resultLinters, lc) - } + resultLinters := maps.Values(resultLintersSet) // Make order of execution of linters (go/analysis metalinter and unused) stable. sort.Slice(resultLinters, func(i, j int) bool { @@ -185,10 +184,7 @@ func (es EnabledSet) combineGoAnalysisLinters(linters map[string]*linter.Config) ml := goanalysis.NewMetaLinter(goanalysisLinters) - var presets []string - for p := range goanalysisPresets { - presets = append(presets, p) - } + presets := maps.Keys(goanalysisPresets) mlConfig := &linter.Config{ Linter: ml, diff --git a/pkg/lint/lintersdb/enabled_set_test.go b/pkg/lint/lintersdb/enabled_set_test.go index 1ab75d4d15eb..550a6dc04a97 100644 --- a/pkg/lint/lintersdb/enabled_set_test.go +++ b/pkg/lint/lintersdb/enabled_set_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "golang.org/x/exp/maps" "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/lint/linter" @@ -107,11 +108,10 @@ func TestGetEnabledLintersSet(t *testing.T) { } els := es.build(&c.cfg, defaultLinters) - var enabledLinters []string for ln, lc := range els { assert.Equal(t, ln, lc.Name()) - enabledLinters = append(enabledLinters, ln) } + enabledLinters := maps.Keys(els) assert.ElementsMatch(t, c.exp, enabledLinters) }) diff --git a/pkg/printers/checkstyle.go b/pkg/printers/checkstyle.go index 3762ca0569f3..e32eef7f51f5 100644 --- a/pkg/printers/checkstyle.go +++ b/pkg/printers/checkstyle.go @@ -7,6 +7,7 @@ import ( "sort" "github.com/go-xmlfmt/xmlfmt" + "golang.org/x/exp/maps" "github.com/golangci/golangci-lint/pkg/result" ) @@ -74,10 +75,7 @@ func (p Checkstyle) Print(issues []result.Issue) error { file.Errors = append(file.Errors, newError) } - out.Files = make([]*checkstyleFile, 0, len(files)) - for _, file := range files { - out.Files = append(out.Files, file) - } + out.Files = maps.Values(files) sort.Slice(out.Files, func(i, j int) bool { return out.Files[i].Name < out.Files[j].Name diff --git a/pkg/printers/junitxml.go b/pkg/printers/junitxml.go index 86a3811e4782..3e3f82f5805c 100644 --- a/pkg/printers/junitxml.go +++ b/pkg/printers/junitxml.go @@ -7,6 +7,8 @@ import ( "sort" "strings" + "golang.org/x/exp/maps" + "github.com/golangci/golangci-lint/pkg/result" ) @@ -71,9 +73,7 @@ func (p JunitXML) Print(issues []result.Issue) error { } var res testSuitesXML - for _, val := range suites { - res.TestSuites = append(res.TestSuites, val) - } + res.TestSuites = maps.Values(suites) sort.Slice(res.TestSuites, func(i, j int) bool { return res.TestSuites[i].Suite < res.TestSuites[j].Suite diff --git a/pkg/result/processors/nolint.go b/pkg/result/processors/nolint.go index 181d3bf1fea1..a72dd1ef292c 100644 --- a/pkg/result/processors/nolint.go +++ b/pkg/result/processors/nolint.go @@ -9,6 +9,8 @@ import ( "sort" "strings" + "golang.org/x/exp/maps" + "github.com/golangci/golangci-lint/pkg/golinters" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/lint/lintersdb" @@ -289,10 +291,7 @@ func (p *Nolint) Finish() { return } - unknownLinters := make([]string, 0, len(p.unknownLintersSet)) - for name := range p.unknownLintersSet { - unknownLinters = append(unknownLinters, name) - } + unknownLinters := maps.Keys(p.unknownLintersSet) sort.Strings(unknownLinters) p.log.Warnf("Found unknown linters in //nolint directives: %s", strings.Join(unknownLinters, ", ")) diff --git a/scripts/expand_website_templates/main.go b/scripts/expand_website_templates/main.go index bb5e83015c43..e4c5a20b30df 100644 --- a/scripts/expand_website_templates/main.go +++ b/scripts/expand_website_templates/main.go @@ -16,6 +16,7 @@ import ( "unicode" "unicode/utf8" + "golang.org/x/exp/maps" "gopkg.in/yaml.v3" "github.com/golangci/golangci-lint/internal/renameio" @@ -360,11 +361,7 @@ func getThanksList() string { } } - var authors []string - for author := range addedAuthors { - authors = append(authors, author) - } - + authors := maps.Keys(addedAuthors) sort.Slice(authors, func(i, j int) bool { return strings.ToLower(authors[i]) < strings.ToLower(authors[j]) }) From bfda79045b93ffd76284be775c88a56c4986fda2 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 4 Jan 2024 22:26:10 +0100 Subject: [PATCH 2/2] review --- go.sum | 2 -- pkg/golinters/govet_test.go | 25 +++++++++++++++---------- pkg/lint/lintersdb/enabled_set_test.go | 4 ++-- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/go.sum b/go.sum index 9bd52a46d74f..e45df37fa61e 100644 --- a/go.sum +++ b/go.sum @@ -625,8 +625,6 @@ golang.org/x/exp v0.0.0-20191227195350-da58074b4299/go.mod h1:2RIsYlXP63K8oxa1u0 golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u096TMicItID8zy7Y6sNkU49FU4= golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM= golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU= -golang.org/x/exp v0.0.0-20230510235704-dd950f8aeaea h1:vLCWI/yYrdEHyN2JzIzPO3aaQJHQdp89IZBA/+azVC4= -golang.org/x/exp v0.0.0-20230510235704-dd950f8aeaea/go.mod h1:V1LtkGg67GoY2N1AnLN78QLrzxkLyJw7RJb1gzOOz9w= golang.org/x/exp v0.0.0-20240103183307-be819d1f06fc h1:ao2WRsKSzW6KuUY9IWPwWahcHCgR0s52IfwutMfEbdM= golang.org/x/exp v0.0.0-20240103183307-be819d1f06fc/go.mod h1:iRJReGqOEeBhDZGkGbynYwcHlctCvnjTYIamk7uXpHI= golang.org/x/exp/typeparams v0.0.0-20220428152302-39d4317da171/go.mod h1:AbB0pIl9nAr9wVwH+Z2ZpaocVmF5I4GyWCDIsVjR0bk= diff --git a/pkg/golinters/govet_test.go b/pkg/golinters/govet_test.go index 8e19016bda0b..99e87045d882 100644 --- a/pkg/golinters/govet_test.go +++ b/pkg/golinters/govet_test.go @@ -31,23 +31,28 @@ func TestGovet(t *testing.T) { } } +func sortAnalyzers(a, b *analysis.Analyzer) int { + if a.Name < b.Name { + return -1 + } + + if a.Name > b.Name { + return 1 + } + + return 0 +} + func TestGovetSorted(t *testing.T) { // Keeping analyzers sorted so their order match the import order. - cmp := func(a, b *analysis.Analyzer) int { - if a.Name < b.Name { - return -1 - } else if a.Name > b.Name { - return 1 - } - return 0 - } t.Run("All", func(t *testing.T) { - if !slices.IsSortedFunc(allAnalyzers, cmp) { + if !slices.IsSortedFunc(allAnalyzers, sortAnalyzers) { t.Error("please keep all analyzers list sorted by name") } }) + t.Run("Default", func(t *testing.T) { - if !slices.IsSortedFunc(defaultAnalyzers, cmp) { + if !slices.IsSortedFunc(defaultAnalyzers, sortAnalyzers) { t.Error("please keep default analyzers list sorted by name") } }) diff --git a/pkg/lint/lintersdb/enabled_set_test.go b/pkg/lint/lintersdb/enabled_set_test.go index 550a6dc04a97..1ab75d4d15eb 100644 --- a/pkg/lint/lintersdb/enabled_set_test.go +++ b/pkg/lint/lintersdb/enabled_set_test.go @@ -4,7 +4,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - "golang.org/x/exp/maps" "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/lint/linter" @@ -108,10 +107,11 @@ func TestGetEnabledLintersSet(t *testing.T) { } els := es.build(&c.cfg, defaultLinters) + var enabledLinters []string for ln, lc := range els { assert.Equal(t, ln, lc.Name()) + enabledLinters = append(enabledLinters, ln) } - enabledLinters := maps.Keys(els) assert.ElementsMatch(t, c.exp, enabledLinters) })