From 1392d14a6b66f0bfe8792016abd1e644559ff6a9 Mon Sep 17 00:00:00 2001 From: Choko Date: Fri, 5 Aug 2022 14:24:04 +0900 Subject: [PATCH 01/11] feat: add reassign linter --- .golangci.reference.yml | 7 +++++++ go.mod | 1 + go.sum | 2 ++ pkg/config/linters_settings.go | 5 +++++ pkg/golinters/reassign.go | 29 +++++++++++++++++++++++++++++ pkg/lint/lintersdb/manager.go | 7 +++++++ test/testdata/reassign.go | 8 ++++++++ 7 files changed, 59 insertions(+) create mode 100644 pkg/golinters/reassign.go create mode 100644 test/testdata/reassign.go diff --git a/.golangci.reference.yml b/.golangci.reference.yml index e02897245170..20c8b274d848 100644 --- a/.golangci.reference.yml +++ b/.golangci.reference.yml @@ -1225,6 +1225,12 @@ linters-settings: - CamelCase - UnitAbbreviations + reassign: + # Pattern for variables that are checked for reassignment. + # See https://github.com/curioswitch/go-reassign#usage + # Defaults to "^(EOF|Err.*)$" + pattern: ".*" + revive: # Maximum number of open files at the same time. # See https://github.com/mgechev/revive#command-line-flags @@ -1951,6 +1957,7 @@ linters: - prealloc - predeclared - promlinter + - reassign - revive - rowserrcheck - scopelint diff --git a/go.mod b/go.mod index 4069dee67f7b..7f83ba5867f3 100644 --- a/go.mod +++ b/go.mod @@ -21,6 +21,7 @@ require ( github.com/breml/errchkjson v0.3.0 github.com/butuzov/ireturn v0.1.1 github.com/charithe/durationcheck v0.0.9 + github.com/curioswitch/go-reassign v0.1.0 github.com/daixiang0/gci v0.6.3 github.com/denis-tingaikin/go-header v0.4.3 github.com/esimonov/ifshort v1.0.4 diff --git a/go.sum b/go.sum index d615d7da055f..75ce9729b064 100644 --- a/go.sum +++ b/go.sum @@ -103,6 +103,8 @@ github.com/cncf/udpa/go v0.0.0-20201120205902-5459f2c99403/go.mod h1:WmhPx2Nbnht github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/cristalhq/acmd v0.7.0/go.mod h1:LG5oa43pE/BbxtfMoImHCQN++0Su7dzipdgBjMCBVDQ= +github.com/curioswitch/go-reassign v0.1.0 h1:QqQPBacVyFcBS/JNgHH2uGEeYMaYjNL4VEU8Nf6wX8k= +github.com/curioswitch/go-reassign v0.1.0/go.mod h1:e1Q0bnQMllWps7BLwuApSjKpJ+p7Xxa01rwgJBTRKF0= github.com/daixiang0/gci v0.6.3 h1:wUAqXChk8HbwXn8AfxD9DYSCp9Bpz1L3e6Q4Roe+q9E= github.com/daixiang0/gci v0.6.3/go.mod h1:EpVfrztufwVgQRXjnX4zuNinEpLj5OmMjtu/+MB0V0c= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index 242a1de1f3d8..7581c4dc714d 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -168,6 +168,7 @@ type LintersSettings struct { Prealloc PreallocSettings Predeclared PredeclaredSettings Promlinter PromlinterSettings + Reassign ReassignSettings Revive ReviveSettings RowsErrCheck RowsErrCheckSettings Staticcheck StaticCheckSettings @@ -532,6 +533,10 @@ type PromlinterSettings struct { DisabledLinters []string `mapstructure:"disabled-linters"` } +type ReassignSettings struct { + Pattern string `mapstructure:"pattern"` +} + type ReviveSettings struct { MaxOpenFiles int `mapstructure:"max-open-files"` IgnoreGeneratedHeader bool `mapstructure:"ignore-generated-header"` diff --git a/pkg/golinters/reassign.go b/pkg/golinters/reassign.go new file mode 100644 index 000000000000..b8a4c1c60cf8 --- /dev/null +++ b/pkg/golinters/reassign.go @@ -0,0 +1,29 @@ +package golinters + +import ( + "github.com/curioswitch/go-reassign" + "golang.org/x/tools/go/analysis" + + "github.com/golangci/golangci-lint/pkg/config" + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" +) + +func NewReassign(settings *config.ReassignSettings) *goanalysis.Linter { + a := reassign.NewAnalyzer() + + var cfg map[string]map[string]interface{} + if settings != nil { + cfg = map[string]map[string]interface{}{ + a.Name: { + reassign.FlagPattern: settings.Pattern, + }, + } + } + + return goanalysis.NewLinter( + a.Name, + a.Doc, + []*analysis.Analyzer{a}, + cfg, + ).WithLoadMode(goanalysis.LoadModeSyntax) +} diff --git a/pkg/lint/lintersdb/manager.go b/pkg/lint/lintersdb/manager.go index 9a1f1fa4d5f0..8eb119437866 100644 --- a/pkg/lint/lintersdb/manager.go +++ b/pkg/lint/lintersdb/manager.go @@ -154,6 +154,7 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { preallocCfg *config.PreallocSettings predeclaredCfg *config.PredeclaredSettings promlinterCfg *config.PromlinterSettings + reassignCfg *config.ReassignSettings reviveCfg *config.ReviveSettings rowserrcheckCfg *config.RowsErrCheckSettings staticcheckCfg *config.StaticCheckSettings @@ -227,6 +228,7 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { parallelTestCfg = &m.cfg.LintersSettings.ParallelTest predeclaredCfg = &m.cfg.LintersSettings.Predeclared promlinterCfg = &m.cfg.LintersSettings.Promlinter + reassignCfg = &m.cfg.LintersSettings.Reassign reviveCfg = &m.cfg.LintersSettings.Revive rowserrcheckCfg = &m.cfg.LintersSettings.RowsErrCheck staticcheckCfg = &m.cfg.LintersSettings.Staticcheck @@ -683,6 +685,11 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { WithPresets(linter.PresetStyle). WithURL("https://github.com/yeya24/promlinter"), + linter.NewConfig(golinters.NewReassign(reassignCfg)). + WithSince("1.49.0"). + WithPresets(linter.PresetBugs). + WithURL("https://github.com/curioswitch/go-reassign"), + linter.NewConfig(golinters.NewRevive(reviveCfg)). WithSince("v1.37.0"). WithPresets(linter.PresetStyle, linter.PresetMetaLinter). diff --git a/test/testdata/reassign.go b/test/testdata/reassign.go new file mode 100644 index 000000000000..cdb4550a93ac --- /dev/null +++ b/test/testdata/reassign.go @@ -0,0 +1,8 @@ +//golangcitest:args -Ereassign +package testdata + +import "io" + +func breakIO() { + io.EOF = nil // ERROR `reassigning variable EOF in other package io` +} From 306b1fccd4c7365ef83fc0315609b3d8583ad09e Mon Sep 17 00:00:00 2001 From: Choko Date: Mon, 22 Aug 2022 11:41:58 +0900 Subject: [PATCH 02/11] Slice for patterns --- .golangci.reference.yml | 7 ++++--- pkg/config/linters_settings.go | 2 +- pkg/golinters/reassign.go | 8 ++++++-- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/.golangci.reference.yml b/.golangci.reference.yml index 20c8b274d848..4aba356d542a 100644 --- a/.golangci.reference.yml +++ b/.golangci.reference.yml @@ -1226,10 +1226,11 @@ linters-settings: - UnitAbbreviations reassign: - # Pattern for variables that are checked for reassignment. + # Patterns for variables that are checked for reassignment. # See https://github.com/curioswitch/go-reassign#usage - # Defaults to "^(EOF|Err.*)$" - pattern: ".*" + # Defaults to [EOF, Err.*] + patterns: + - ".*" revive: # Maximum number of open files at the same time. diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index 7581c4dc714d..56501d6d1281 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -534,7 +534,7 @@ type PromlinterSettings struct { } type ReassignSettings struct { - Pattern string `mapstructure:"pattern"` + Patterns []string `mapstructure:"pattern"` } type ReviveSettings struct { diff --git a/pkg/golinters/reassign.go b/pkg/golinters/reassign.go index b8a4c1c60cf8..49c47858b8d8 100644 --- a/pkg/golinters/reassign.go +++ b/pkg/golinters/reassign.go @@ -1,6 +1,9 @@ package golinters import ( + "fmt" + "strings" + "github.com/curioswitch/go-reassign" "golang.org/x/tools/go/analysis" @@ -12,10 +15,11 @@ func NewReassign(settings *config.ReassignSettings) *goanalysis.Linter { a := reassign.NewAnalyzer() var cfg map[string]map[string]interface{} - if settings != nil { + if settings != nil && len(settings.Patterns) > 0 { + patternStr := fmt.Sprintf("^(%s)$", strings.Join(settings.Patterns, "|")) cfg = map[string]map[string]interface{}{ a.Name: { - reassign.FlagPattern: settings.Pattern, + reassign.FlagPattern: patternStr, }, } } From 59ed33532cf58d46c6722ef46e39394bcfc12f91 Mon Sep 17 00:00:00 2001 From: Choko Date: Tue, 23 Aug 2022 15:08:33 +0900 Subject: [PATCH 03/11] Fix yaml parse --- pkg/config/linters_settings.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index 56501d6d1281..f1c36c380d60 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -534,7 +534,7 @@ type PromlinterSettings struct { } type ReassignSettings struct { - Patterns []string `mapstructure:"pattern"` + Patterns []string `mapstructure:"patterns"` } type ReviveSettings struct { From 100c10254a7248425d5d8aba2c73cef74bed35f4 Mon Sep 17 00:00:00 2001 From: Choko Date: Tue, 23 Aug 2022 15:23:55 +0900 Subject: [PATCH 04/11] Add test with settings --- test/testdata/configs/reassign.yml | 5 +++++ test/testdata/reassign.go | 9 +++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) create mode 100644 test/testdata/configs/reassign.yml diff --git a/test/testdata/configs/reassign.yml b/test/testdata/configs/reassign.yml new file mode 100644 index 000000000000..7037defc92e5 --- /dev/null +++ b/test/testdata/configs/reassign.yml @@ -0,0 +1,5 @@ +linter-settings: + reassign: + patterns: + - EOF + - DefaultClient \ No newline at end of file diff --git a/test/testdata/reassign.go b/test/testdata/reassign.go index cdb4550a93ac..fdecde5feb0c 100644 --- a/test/testdata/reassign.go +++ b/test/testdata/reassign.go @@ -1,8 +1,13 @@ //golangcitest:args -Ereassign +//golangcitest:config_path testdata/configs/reassign.yml package testdata -import "io" +import ( + "io" + "net/http" +) func breakIO() { - io.EOF = nil // ERROR `reassigning variable EOF in other package io` + http.DefaultClient = nil // ERROR `reassigning variable DefaultClient in other package http` + io.EOF = nil // ERROR `reassigning variable EOF in other package io` } From 0a47421c16166ee02fb486be0825c02e044e7c57 Mon Sep 17 00:00:00 2001 From: Choko Date: Tue, 23 Aug 2022 15:24:50 +0900 Subject: [PATCH 05/11] go-reassign v0.1.2 --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 7f83ba5867f3..46d111d92b1d 100644 --- a/go.mod +++ b/go.mod @@ -21,7 +21,7 @@ require ( github.com/breml/errchkjson v0.3.0 github.com/butuzov/ireturn v0.1.1 github.com/charithe/durationcheck v0.0.9 - github.com/curioswitch/go-reassign v0.1.0 + github.com/curioswitch/go-reassign v0.1.2 github.com/daixiang0/gci v0.6.3 github.com/denis-tingaikin/go-header v0.4.3 github.com/esimonov/ifshort v1.0.4 diff --git a/go.sum b/go.sum index 75ce9729b064..e2e7ec2fa9ee 100644 --- a/go.sum +++ b/go.sum @@ -103,8 +103,8 @@ github.com/cncf/udpa/go v0.0.0-20201120205902-5459f2c99403/go.mod h1:WmhPx2Nbnht github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/cristalhq/acmd v0.7.0/go.mod h1:LG5oa43pE/BbxtfMoImHCQN++0Su7dzipdgBjMCBVDQ= -github.com/curioswitch/go-reassign v0.1.0 h1:QqQPBacVyFcBS/JNgHH2uGEeYMaYjNL4VEU8Nf6wX8k= -github.com/curioswitch/go-reassign v0.1.0/go.mod h1:e1Q0bnQMllWps7BLwuApSjKpJ+p7Xxa01rwgJBTRKF0= +github.com/curioswitch/go-reassign v0.1.2 h1:ekM07+z+VFT560Exz4mTv0/s1yU9gem6CJc/tlYpkmI= +github.com/curioswitch/go-reassign v0.1.2/go.mod h1:bFJIHgtTM3hRm2sKXSPkbwNjSFyGURQXyn4IXD2qwfQ= github.com/daixiang0/gci v0.6.3 h1:wUAqXChk8HbwXn8AfxD9DYSCp9Bpz1L3e6Q4Roe+q9E= github.com/daixiang0/gci v0.6.3/go.mod h1:EpVfrztufwVgQRXjnX4zuNinEpLj5OmMjtu/+MB0V0c= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= From 3963203af587b3bc70374d2f3416c050e2839618 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 23 Aug 2022 10:55:22 +0200 Subject: [PATCH 06/11] review: use new test framework --- test/testdata/reassign.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/testdata/reassign.go b/test/testdata/reassign.go index fdecde5feb0c..3a3ad0d3e1e9 100644 --- a/test/testdata/reassign.go +++ b/test/testdata/reassign.go @@ -8,6 +8,6 @@ import ( ) func breakIO() { - http.DefaultClient = nil // ERROR `reassigning variable DefaultClient in other package http` - io.EOF = nil // ERROR `reassigning variable EOF in other package io` + http.DefaultClient = nil // want `reassigning variable DefaultClient in other package http` + io.EOF = nil // want `reassigning variable EOF in other package io` } From b1e630d8fa9e80ccc955586b4ffb435ed412ad35 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 23 Aug 2022 11:27:48 +0200 Subject: [PATCH 07/11] review: doc --- .golangci.reference.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.golangci.reference.yml b/.golangci.reference.yml index 4aba356d542a..f1bf542c8967 100644 --- a/.golangci.reference.yml +++ b/.golangci.reference.yml @@ -1226,9 +1226,9 @@ linters-settings: - UnitAbbreviations reassign: - # Patterns for variables that are checked for reassignment. + # Patterns for global variable names that are checked for reassignment. # See https://github.com/curioswitch/go-reassign#usage - # Defaults to [EOF, Err.*] + # Default: ["EOF", "Err.*"] patterns: - ".*" From cc4fe21430656343caa29ab4971cdf632b1d477f Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 23 Aug 2022 12:42:57 +0200 Subject: [PATCH 08/11] review: fix typo --- test/testdata/configs/reassign.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/testdata/configs/reassign.yml b/test/testdata/configs/reassign.yml index 7037defc92e5..f10e2326e552 100644 --- a/test/testdata/configs/reassign.yml +++ b/test/testdata/configs/reassign.yml @@ -1,5 +1,5 @@ -linter-settings: +linters-settings: reassign: patterns: - EOF - - DefaultClient \ No newline at end of file + - DefaultClient From 2f332bc06a5c842c2c1b14ba5f82d669418a50a8 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 23 Aug 2022 12:44:12 +0200 Subject: [PATCH 09/11] review: missing doc --- .golangci.reference.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.golangci.reference.yml b/.golangci.reference.yml index f1bf542c8967..2baf9f7f67e7 100644 --- a/.golangci.reference.yml +++ b/.golangci.reference.yml @@ -2063,6 +2063,7 @@ linters: - prealloc - predeclared - promlinter + - reassign - revive - rowserrcheck - scopelint From cd9999a9258e448231882d8d566989319269c03c Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 23 Aug 2022 12:46:37 +0200 Subject: [PATCH 10/11] review: improve tests --- .../{reassign.yml => reassign_patterns.yml} | 2 +- test/testdata/reassign.go | 7 +++---- test/testdata/reassign_patterns.go | 14 ++++++++++++++ 3 files changed, 18 insertions(+), 5 deletions(-) rename test/testdata/configs/{reassign.yml => reassign_patterns.yml} (72%) create mode 100644 test/testdata/reassign_patterns.go diff --git a/test/testdata/configs/reassign.yml b/test/testdata/configs/reassign_patterns.yml similarity index 72% rename from test/testdata/configs/reassign.yml rename to test/testdata/configs/reassign_patterns.yml index f10e2326e552..5af124dbd4f1 100644 --- a/test/testdata/configs/reassign.yml +++ b/test/testdata/configs/reassign_patterns.yml @@ -1,5 +1,5 @@ linters-settings: reassign: patterns: - - EOF - DefaultClient + - DefaultTransport diff --git a/test/testdata/reassign.go b/test/testdata/reassign.go index 3a3ad0d3e1e9..0fc875c10d6f 100644 --- a/test/testdata/reassign.go +++ b/test/testdata/reassign.go @@ -1,5 +1,4 @@ //golangcitest:args -Ereassign -//golangcitest:config_path testdata/configs/reassign.yml package testdata import ( @@ -7,7 +6,7 @@ import ( "net/http" ) -func breakIO() { - http.DefaultClient = nil // want `reassigning variable DefaultClient in other package http` - io.EOF = nil // want `reassigning variable EOF in other package io` +func reassignTest() { + http.DefaultClient = nil + io.EOF = nil // want `reassigning variable EOF in other package io` } diff --git a/test/testdata/reassign_patterns.go b/test/testdata/reassign_patterns.go new file mode 100644 index 000000000000..dc83cd71c0ef --- /dev/null +++ b/test/testdata/reassign_patterns.go @@ -0,0 +1,14 @@ +//golangcitest:args -Ereassign +//golangcitest:config_path testdata/configs/reassign_patterns.yml +package testdata + +import ( + "io" + "net/http" +) + +func reassignTestPatterns() { + http.DefaultClient = nil // want `reassigning variable DefaultClient in other package http` + http.DefaultTransport = nil // want `reassigning variable DefaultTransport in other package http` + io.EOF = nil +} From e01da8c7a0487a7995c41d27eec64fc3dae27745 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 23 Aug 2022 12:47:36 +0200 Subject: [PATCH 11/11] review --- pkg/golinters/reassign.go | 3 +-- test/testdata/reassign.go | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/golinters/reassign.go b/pkg/golinters/reassign.go index 49c47858b8d8..a9ff67ee07aa 100644 --- a/pkg/golinters/reassign.go +++ b/pkg/golinters/reassign.go @@ -16,10 +16,9 @@ func NewReassign(settings *config.ReassignSettings) *goanalysis.Linter { var cfg map[string]map[string]interface{} if settings != nil && len(settings.Patterns) > 0 { - patternStr := fmt.Sprintf("^(%s)$", strings.Join(settings.Patterns, "|")) cfg = map[string]map[string]interface{}{ a.Name: { - reassign.FlagPattern: patternStr, + reassign.FlagPattern: fmt.Sprintf("^(%s)$", strings.Join(settings.Patterns, "|")), }, } } diff --git a/test/testdata/reassign.go b/test/testdata/reassign.go index 0fc875c10d6f..27a7baa5121e 100644 --- a/test/testdata/reassign.go +++ b/test/testdata/reassign.go @@ -8,5 +8,6 @@ import ( func reassignTest() { http.DefaultClient = nil + http.DefaultTransport = nil io.EOF = nil // want `reassigning variable EOF in other package io` }