From e64487013f5731873adf99a6f7d180ef01a90951 Mon Sep 17 00:00:00 2001 From: karamaru-alpha Date: Sun, 3 Mar 2024 20:01:14 +0900 Subject: [PATCH 1/2] upgrade: copyloopvar --- .golangci.reference.yml | 5 +++++ go.mod | 2 +- go.sum | 4 ++-- pkg/config/linters_settings.go | 5 +++++ pkg/golinters/copyloopvar.go | 21 +++++++++++++++------ pkg/lint/lintersdb/builder_linter.go | 2 +- test/testdata/configs/copyloopvar.yml | 3 +++ test/testdata/copyloopvar.go | 7 +++++-- 8 files changed, 37 insertions(+), 12 deletions(-) create mode 100644 test/testdata/configs/copyloopvar.yml diff --git a/.golangci.reference.yml b/.golangci.reference.yml index 808aea955f2d..7edfd7469245 100644 --- a/.golangci.reference.yml +++ b/.golangci.reference.yml @@ -149,6 +149,11 @@ linters-settings: first-strong-isolate: false pop-directional-isolate: false + copyloopvar: + # If true, ignore aliasing of loop variables. + # Default: false + ignore-alias: true + cyclop: # The maximal code complexity to report. # Default: 10 diff --git a/go.mod b/go.mod index 8026a52d0abb..f1521859f69b 100644 --- a/go.mod +++ b/go.mod @@ -61,7 +61,7 @@ require ( github.com/jirfag/go-printf-func-name v0.0.0-20200119135958-7558a9eaa5af github.com/jjti/go-spancheck v0.5.3 github.com/julz/importas v0.1.0 - github.com/karamaru-alpha/copyloopvar v1.0.4 + github.com/karamaru-alpha/copyloopvar v1.0.8 github.com/kisielk/errcheck v1.7.0 github.com/kkHAIKE/contextcheck v1.1.4 github.com/kulti/thelper v0.6.3 diff --git a/go.sum b/go.sum index a2ab0655c597..d07b59d76326 100644 --- a/go.sum +++ b/go.sum @@ -322,8 +322,8 @@ github.com/julienschmidt/httprouter v1.2.0/go.mod h1:SYymIcj16QtmaHHD7aYtjjsJG7V github.com/julienschmidt/httprouter v1.3.0/go.mod h1:JR6WtHb+2LUe8TCKY3cZOxFyyO8IZAc4RVcycCCAKdM= github.com/julz/importas v0.1.0 h1:F78HnrsjY3cR7j0etXy5+TU1Zuy7Xt08X/1aJnH5xXY= github.com/julz/importas v0.1.0/go.mod h1:oSFU2R4XK/P7kNBrnL/FEQlDGN1/6WoxXEjSSXO0DV0= -github.com/karamaru-alpha/copyloopvar v1.0.4 h1:JD6IPXo4+RawkSPe9uMKh9OtTzYKsCelAgPMUwaVxBw= -github.com/karamaru-alpha/copyloopvar v1.0.4/go.mod h1:u7CIfztblY0jZLOQZgH3oYsJzpC2A7S6u/lfgSXHy0k= +github.com/karamaru-alpha/copyloopvar v1.0.8 h1:gieLARwuByhEMxRwM3GRS/juJqFbLraftXIKDDNJ50Q= +github.com/karamaru-alpha/copyloopvar v1.0.8/go.mod h1:u7CIfztblY0jZLOQZgH3oYsJzpC2A7S6u/lfgSXHy0k= github.com/kisielk/errcheck v1.7.0 h1:+SbscKmWJ5mOK/bO1zS60F5I9WwZDWOfRsC4RwfwRV0= github.com/kisielk/errcheck v1.7.0/go.mod h1:1kLL+jV4e+CFfueBmI1dSK2ADDyQnlrnrY/FqKluHJQ= github.com/kisielk/gotool v1.0.0 h1:AV2c/EiW3KqPNT9ZKl07ehoAGi4C5/01Cfbblndcapg= diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index e55e5ad25a12..319f2101d9be 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -206,6 +206,7 @@ var defaultLintersSettings = LintersSettings{ type LintersSettings struct { Asasalint AsasalintSettings BiDiChk BiDiChkSettings + CopyLoopVar CopyLoopVarSettings Cyclop Cyclop Decorder DecorderSettings Depguard DepGuardSettings @@ -313,6 +314,10 @@ type BiDiChkSettings struct { PopDirectionalIsolate bool `mapstructure:"pop-directional-isolate"` } +type CopyLoopVarSettings struct { + IgnoreAlias bool `mapstructure:"ignore-alias"` +} + type Cyclop struct { MaxComplexity int `mapstructure:"max-complexity"` PackageAverage float64 `mapstructure:"package-average"` diff --git a/pkg/golinters/copyloopvar.go b/pkg/golinters/copyloopvar.go index 0389ddddb909..dee2bce4388e 100644 --- a/pkg/golinters/copyloopvar.go +++ b/pkg/golinters/copyloopvar.go @@ -4,16 +4,25 @@ import ( "github.com/karamaru-alpha/copyloopvar" "golang.org/x/tools/go/analysis" + "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" ) -func NewCopyLoopVar() *goanalysis.Linter { - a := copyloopvar.Analyzer +func NewCopyLoopVar(settings *config.CopyLoopVarSettings) *goanalysis.Linter { + analyzer := copyloopvar.NewAnalyzer() + var cfg map[string]map[string]any + if settings != nil { + cfg = map[string]map[string]any{ + analyzer.Name: { + "ignore-alias": settings.IgnoreAlias, + }, + } + } return goanalysis.NewLinter( - a.Name, - a.Doc, - []*analysis.Analyzer{a}, - nil, + analyzer.Name, + analyzer.Doc, + []*analysis.Analyzer{analyzer}, + cfg, ).WithLoadMode(goanalysis.LoadModeSyntax) } diff --git a/pkg/lint/lintersdb/builder_linter.go b/pkg/lint/lintersdb/builder_linter.go index 5fe538f6f3ca..67b19ce4a232 100644 --- a/pkg/lint/lintersdb/builder_linter.go +++ b/pkg/lint/lintersdb/builder_linter.go @@ -60,7 +60,7 @@ func (b LinterBuilder) Build(cfg *config.Config) []*linter.Config { WithLoadForGoAnalysis(). WithURL("https://github.com/kkHAIKE/contextcheck"), - linter.NewConfig(golinters.NewCopyLoopVar()). + linter.NewConfig(golinters.NewCopyLoopVar(&cfg.LintersSettings.CopyLoopVar)). WithSince("v1.57.0"). WithPresets(linter.PresetStyle). WithURL("https://github.com/karamaru-alpha/copyloopvar"). diff --git a/test/testdata/configs/copyloopvar.yml b/test/testdata/configs/copyloopvar.yml new file mode 100644 index 000000000000..2c3b5872edc9 --- /dev/null +++ b/test/testdata/configs/copyloopvar.yml @@ -0,0 +1,3 @@ +linters-settings: + copyloopvar: + ignore-alias: true diff --git a/test/testdata/copyloopvar.go b/test/testdata/copyloopvar.go index 7fdd2b04b579..9ee37c6c005e 100644 --- a/test/testdata/copyloopvar.go +++ b/test/testdata/copyloopvar.go @@ -1,6 +1,7 @@ //go:build go1.22 //golangcitest:args -Ecopyloopvar +//golangcitest:config_path testdata/configs/copyloopvar.yml package testdata import "fmt" @@ -13,10 +14,12 @@ func copyloopvarCase1() { fns = append(fns, func() { fmt.Println(i) }) - _v := v // want `The copy of the 'for' variable "v" can be deleted \(Go 1\.22\+\)` + v := v // want `The copy of the 'for' variable "v" can be deleted \(Go 1\.22\+\)` fns = append(fns, func() { - fmt.Println(_v) + fmt.Println(v) }) + _v := v + _ = _v } for _, fn := range fns { fn() From ca770c1d8d4132d1ac798d42719f98c3f0ec9a21 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sun, 3 Mar 2024 14:24:11 +0100 Subject: [PATCH 2/2] review --- pkg/golinters/copyloopvar.go | 11 ++++---- test/testdata/copyloopvar.go | 3 +-- test/testdata/copyloopvar_custom.go | 41 +++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 7 deletions(-) create mode 100644 test/testdata/copyloopvar_custom.go diff --git a/pkg/golinters/copyloopvar.go b/pkg/golinters/copyloopvar.go index dee2bce4388e..02ce5f37a6cd 100644 --- a/pkg/golinters/copyloopvar.go +++ b/pkg/golinters/copyloopvar.go @@ -9,20 +9,21 @@ import ( ) func NewCopyLoopVar(settings *config.CopyLoopVarSettings) *goanalysis.Linter { - analyzer := copyloopvar.NewAnalyzer() + a := copyloopvar.NewAnalyzer() + var cfg map[string]map[string]any if settings != nil { cfg = map[string]map[string]any{ - analyzer.Name: { + a.Name: { "ignore-alias": settings.IgnoreAlias, }, } } return goanalysis.NewLinter( - analyzer.Name, - analyzer.Doc, - []*analysis.Analyzer{analyzer}, + a.Name, + a.Doc, + []*analysis.Analyzer{a}, cfg, ).WithLoadMode(goanalysis.LoadModeSyntax) } diff --git a/test/testdata/copyloopvar.go b/test/testdata/copyloopvar.go index 9ee37c6c005e..b5f1a88423b9 100644 --- a/test/testdata/copyloopvar.go +++ b/test/testdata/copyloopvar.go @@ -1,7 +1,6 @@ //go:build go1.22 //golangcitest:args -Ecopyloopvar -//golangcitest:config_path testdata/configs/copyloopvar.yml package testdata import "fmt" @@ -18,7 +17,7 @@ func copyloopvarCase1() { fns = append(fns, func() { fmt.Println(v) }) - _v := v + _v := v // want `The copy of the 'for' variable "v" can be deleted \(Go 1\.22\+\)` _ = _v } for _, fn := range fns { diff --git a/test/testdata/copyloopvar_custom.go b/test/testdata/copyloopvar_custom.go new file mode 100644 index 000000000000..9ee37c6c005e --- /dev/null +++ b/test/testdata/copyloopvar_custom.go @@ -0,0 +1,41 @@ +//go:build go1.22 + +//golangcitest:args -Ecopyloopvar +//golangcitest:config_path testdata/configs/copyloopvar.yml +package testdata + +import "fmt" + +func copyloopvarCase1() { + slice := []int{1, 2, 3} + fns := make([]func(), 0, len(slice)*2) + for i, v := range slice { + i := i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)` + fns = append(fns, func() { + fmt.Println(i) + }) + v := v // want `The copy of the 'for' variable "v" can be deleted \(Go 1\.22\+\)` + fns = append(fns, func() { + fmt.Println(v) + }) + _v := v + _ = _v + } + for _, fn := range fns { + fn() + } +} + +func copyloopvarCase2() { + loopCount := 3 + fns := make([]func(), 0, loopCount) + for i := 1; i <= loopCount; i++ { + i := i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)` + fns = append(fns, func() { + fmt.Println(i) + }) + } + for _, fn := range fns { + fn() + } +}