From 77432addf39fb226cb4d87b7aac92036e72c71db Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 21 Mar 2024 22:30:31 +0100 Subject: [PATCH 1/9] feat: deprecate usage of linter alternative names --- pkg/lint/lintersdb/validator.go | 35 ++++++++++++++++++++++++++++ pkg/lint/lintersdb/validator_test.go | 23 ++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/pkg/lint/lintersdb/validator.go b/pkg/lint/lintersdb/validator.go index 364d5082a5e6..cd59223ff4c8 100644 --- a/pkg/lint/lintersdb/validator.go +++ b/pkg/lint/lintersdb/validator.go @@ -3,10 +3,12 @@ package lintersdb import ( "errors" "fmt" + "os" "slices" "strings" "github.com/golangci/golangci-lint/pkg/config" + "github.com/golangci/golangci-lint/pkg/logutils" ) type Validator struct { @@ -28,6 +30,7 @@ func (v Validator) Validate(cfg *config.Config) error { validators := []func(cfg *config.Linters) error{ v.validateLintersNames, v.validatePresets, + v.alternativeNamesDeprecation, } for _, v := range validators { @@ -75,3 +78,35 @@ func (v Validator) validatePresets(cfg *config.Linters) error { return nil } + +func (v Validator) alternativeNamesDeprecation(cfg *config.Linters) error { + if v.m.cfg.InternalTest || v.m.cfg.InternalCmdTest || os.Getenv(logutils.EnvTestRun) == "1" { + return nil + } + + altNames := map[string][]string{} + for _, lc := range v.m.GetAllSupportedLinterConfigs() { + for _, alt := range lc.AlternativeNames { + altNames[alt] = append(altNames[alt], lc.Name()) + } + } + + var names []string + names = append(names, cfg.Enable...) + names = append(names, cfg.Disable...) + + for _, name := range names { + lc, ok := altNames[name] + if !ok { + continue + } + + if len(lc) > 1 { + v.m.log.Warnf("The linter name %q is deprecated. It has been splited into: %s.", name, strings.Join(lc, ", ")) + } else { + v.m.log.Warnf("The linter name %q is deprecated. It has been renamed to: %s.", name, lc[0]) + } + } + + return nil +} diff --git a/pkg/lint/lintersdb/validator_test.go b/pkg/lint/lintersdb/validator_test.go index 1bfeb7078cb0..08eeff152d3f 100644 --- a/pkg/lint/lintersdb/validator_test.go +++ b/pkg/lint/lintersdb/validator_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/require" "github.com/golangci/golangci-lint/pkg/config" + "github.com/golangci/golangci-lint/pkg/logutils" ) type validateErrorTestCase struct { @@ -215,3 +216,25 @@ func TestValidator_validatePresets_error(t *testing.T) { }) } } + +func TestValidator_alternativeNamesDeprecation(t *testing.T) { + log := logutils.NewMockLog() + log.On("Warnf", "The linter name %q is deprecated. It has been renamed to: %s.", "vet", "govet") + log.On("Warnf", "The linter name %q is deprecated. It has been renamed to: %s.", "vetshadow", "govet") + log.On("Warnf", "The linter name %q is deprecated. It has been renamed to: %s.", "logrlint", "loggercheck") + log.On("Warnf", "The linter name %q is deprecated. It has been splited into: %s.", "megacheck", "gosimple, staticcheck, unused") + log.On("Warnf", "The linter name %q is deprecated. It has been renamed to: %s.", "gas", "gosec") + + m, err := NewManager(log, nil, NewLinterBuilder()) + require.NoError(t, err) + + v := NewValidator(m) + + cfg := &config.Linters{ + Enable: []string{"vet", "vetshadow", "logrlint"}, + Disable: []string{"megacheck", "gas"}, + } + + err = v.alternativeNamesDeprecation(cfg) + require.NoError(t, err) +} From acefe43e6f1e9ca0882f4a9415e14aef27bf3310 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 21 Mar 2024 22:48:42 +0100 Subject: [PATCH 2/9] chore: improve MockLog --- pkg/golinters/internal/diff_test.go | 4 +-- pkg/lint/lintersdb/validator_test.go | 12 ++++---- pkg/logutils/mock.go | 45 ++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 8 deletions(-) diff --git a/pkg/golinters/internal/diff_test.go b/pkg/golinters/internal/diff_test.go index 51946b4ff762..7aa55578258c 100644 --- a/pkg/golinters/internal/diff_test.go +++ b/pkg/golinters/internal/diff_test.go @@ -128,8 +128,8 @@ index 0000000..6399915 +// line ` - log := logutils.NewMockLog() - log.On("Infof", "The diff contains only additions: no original or deleted lines: %#v", mock.Anything) + log := logutils.NewMockLog(). + OnInfof("The diff contains only additions: no original or deleted lines: %#v", mock.Anything) var noChanges []Change testDiffProducesChanges(t, log, diff, noChanges...) diff --git a/pkg/lint/lintersdb/validator_test.go b/pkg/lint/lintersdb/validator_test.go index 08eeff152d3f..7ede69be083e 100644 --- a/pkg/lint/lintersdb/validator_test.go +++ b/pkg/lint/lintersdb/validator_test.go @@ -218,12 +218,12 @@ func TestValidator_validatePresets_error(t *testing.T) { } func TestValidator_alternativeNamesDeprecation(t *testing.T) { - log := logutils.NewMockLog() - log.On("Warnf", "The linter name %q is deprecated. It has been renamed to: %s.", "vet", "govet") - log.On("Warnf", "The linter name %q is deprecated. It has been renamed to: %s.", "vetshadow", "govet") - log.On("Warnf", "The linter name %q is deprecated. It has been renamed to: %s.", "logrlint", "loggercheck") - log.On("Warnf", "The linter name %q is deprecated. It has been splited into: %s.", "megacheck", "gosimple, staticcheck, unused") - log.On("Warnf", "The linter name %q is deprecated. It has been renamed to: %s.", "gas", "gosec") + log := logutils.NewMockLog(). + OnWarnf("The linter name %q is deprecated. It has been renamed to: %s.", "vet", "govet"). + OnWarnf("The linter name %q is deprecated. It has been renamed to: %s.", "vetshadow", "govet"). + OnWarnf("The linter name %q is deprecated. It has been renamed to: %s.", "logrlint", "loggercheck"). + OnWarnf("The linter name %q is deprecated. It has been splited into: %s.", "megacheck", "gosimple, staticcheck, unused"). + OnWarnf("The linter name %q is deprecated. It has been renamed to: %s.", "gas", "gosec") m, err := NewManager(log, nil, NewLinterBuilder()) require.NoError(t, err) diff --git a/pkg/logutils/mock.go b/pkg/logutils/mock.go index efda8cc20f6e..2e72e0ffeae9 100644 --- a/pkg/logutils/mock.go +++ b/pkg/logutils/mock.go @@ -45,3 +45,48 @@ func (m *MockLog) Child(name string) Log { func (m *MockLog) SetLevel(level LogLevel) { m.Called(level) } + +func (m *MockLog) OnFatalf(format string, args ...any) *MockLog { + arguments := []any{format} + arguments = append(arguments, args...) + + m.On("Fatalf", arguments...) + + return m +} + +func (m *MockLog) OnPanicf(format string, args ...any) *MockLog { + arguments := []any{format} + arguments = append(arguments, args...) + + m.On("Panicf", arguments...) + + return m +} + +func (m *MockLog) OnErrorf(format string, args ...any) *MockLog { + arguments := []any{format} + arguments = append(arguments, args...) + + m.On("Errorf", arguments...) + + return m +} + +func (m *MockLog) OnWarnf(format string, args ...any) *MockLog { + arguments := []any{format} + arguments = append(arguments, args...) + + m.On("Warnf", arguments...) + + return m +} + +func (m *MockLog) OnInfof(format string, args ...any) *MockLog { + arguments := []any{format} + arguments = append(arguments, args...) + + m.On("Infof", arguments...) + + return m +} From 1e1ed00cfbc46d5d1d07e4fb95e2c8b108d8b215 Mon Sep 17 00:00:00 2001 From: Ludovic Fernandez Date: Sun, 24 Mar 2024 16:58:18 +0100 Subject: [PATCH 3/9] review Co-authored-by: Simon Sawert --- pkg/lint/lintersdb/validator.go | 2 +- pkg/lint/lintersdb/validator_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/lint/lintersdb/validator.go b/pkg/lint/lintersdb/validator.go index cd59223ff4c8..91513931fc06 100644 --- a/pkg/lint/lintersdb/validator.go +++ b/pkg/lint/lintersdb/validator.go @@ -102,7 +102,7 @@ func (v Validator) alternativeNamesDeprecation(cfg *config.Linters) error { } if len(lc) > 1 { - v.m.log.Warnf("The linter name %q is deprecated. It has been splited into: %s.", name, strings.Join(lc, ", ")) + v.m.log.Warnf("The linter name %q is deprecated. It has been split into: %s.", name, strings.Join(lc, ", ")) } else { v.m.log.Warnf("The linter name %q is deprecated. It has been renamed to: %s.", name, lc[0]) } diff --git a/pkg/lint/lintersdb/validator_test.go b/pkg/lint/lintersdb/validator_test.go index 7ede69be083e..bfd236452d4a 100644 --- a/pkg/lint/lintersdb/validator_test.go +++ b/pkg/lint/lintersdb/validator_test.go @@ -222,7 +222,7 @@ func TestValidator_alternativeNamesDeprecation(t *testing.T) { OnWarnf("The linter name %q is deprecated. It has been renamed to: %s.", "vet", "govet"). OnWarnf("The linter name %q is deprecated. It has been renamed to: %s.", "vetshadow", "govet"). OnWarnf("The linter name %q is deprecated. It has been renamed to: %s.", "logrlint", "loggercheck"). - OnWarnf("The linter name %q is deprecated. It has been splited into: %s.", "megacheck", "gosimple, staticcheck, unused"). + OnWarnf("The linter name %q is deprecated. It has been split into: %s.", "megacheck", "gosimple, staticcheck, unused"). OnWarnf("The linter name %q is deprecated. It has been renamed to: %s.", "gas", "gosec") m, err := NewManager(log, nil, NewLinterBuilder()) From 06dc02918f931dbb2a2b535d1ee7caf86e44d34e Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sun, 24 Mar 2024 20:36:23 +0100 Subject: [PATCH 4/9] review: improve wording --- pkg/lint/lintersdb/validator.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/lint/lintersdb/validator.go b/pkg/lint/lintersdb/validator.go index 91513931fc06..2503624aebcd 100644 --- a/pkg/lint/lintersdb/validator.go +++ b/pkg/lint/lintersdb/validator.go @@ -102,9 +102,9 @@ func (v Validator) alternativeNamesDeprecation(cfg *config.Linters) error { } if len(lc) > 1 { - v.m.log.Warnf("The linter name %q is deprecated. It has been split into: %s.", name, strings.Join(lc, ", ")) + v.m.log.Warnf("The linter named %q is deprecated. It has been split into: %s.", name, strings.Join(lc, ", ")) } else { - v.m.log.Warnf("The linter name %q is deprecated. It has been renamed to: %s.", name, lc[0]) + v.m.log.Warnf("The name %q is deprecated. The linter has been renamed to: %s.", name, lc[0]) } } From da78a54cb56da1506d1fbb6156c82f3c3ee778de Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sun, 24 Mar 2024 21:57:29 +0100 Subject: [PATCH 5/9] chore: just to prove that the tests can fails on the CI --- pkg/lint/lintersdb/validator_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/lint/lintersdb/validator_test.go b/pkg/lint/lintersdb/validator_test.go index bfd236452d4a..6e02e18e81c6 100644 --- a/pkg/lint/lintersdb/validator_test.go +++ b/pkg/lint/lintersdb/validator_test.go @@ -217,7 +217,9 @@ func TestValidator_validatePresets_error(t *testing.T) { } } -func TestValidator_alternativeNamesDeprecation(t *testing.T) { +func TestValidator_alternativeNames(t *testing.T) { + t.Setenv(logutils.EnvTestRun, "0") + log := logutils.NewMockLog(). OnWarnf("The linter name %q is deprecated. It has been renamed to: %s.", "vet", "govet"). OnWarnf("The linter name %q is deprecated. It has been renamed to: %s.", "vetshadow", "govet"). From 961e81a42733399a50bd83073c99910eed0f6815 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sun, 24 Mar 2024 21:59:16 +0100 Subject: [PATCH 6/9] tests: update expectations --- pkg/lint/lintersdb/validator_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/lint/lintersdb/validator_test.go b/pkg/lint/lintersdb/validator_test.go index 6e02e18e81c6..d9e36760860e 100644 --- a/pkg/lint/lintersdb/validator_test.go +++ b/pkg/lint/lintersdb/validator_test.go @@ -217,15 +217,15 @@ func TestValidator_validatePresets_error(t *testing.T) { } } -func TestValidator_alternativeNames(t *testing.T) { +func TestValidator_alternativeNamesDeprecation(t *testing.T) { t.Setenv(logutils.EnvTestRun, "0") log := logutils.NewMockLog(). - OnWarnf("The linter name %q is deprecated. It has been renamed to: %s.", "vet", "govet"). - OnWarnf("The linter name %q is deprecated. It has been renamed to: %s.", "vetshadow", "govet"). - OnWarnf("The linter name %q is deprecated. It has been renamed to: %s.", "logrlint", "loggercheck"). - OnWarnf("The linter name %q is deprecated. It has been split into: %s.", "megacheck", "gosimple, staticcheck, unused"). - OnWarnf("The linter name %q is deprecated. It has been renamed to: %s.", "gas", "gosec") + OnWarnf("The name %q is deprecated. The linter has been renamed to: %s.", "vet", "govet"). + OnWarnf("The name %q is deprecated. The linter has been renamed to: %s.", "vetshadow", "govet"). + OnWarnf("The name %q is deprecated. The linter has been renamed to: %s.", "logrlint", "loggercheck"). + OnWarnf("The linter named %q is deprecated. It has been split into: %s.", "megacheck", "gosimple, staticcheck, unused"). + OnWarnf("The name %q is deprecated. The linter has been renamed to: %s.", "gas", "gosec") m, err := NewManager(log, nil, NewLinterBuilder()) require.NoError(t, err) From d724a610c6ed79884ef3ee0e5262d197c3e5cdec Mon Sep 17 00:00:00 2001 From: Ludovic Fernandez Date: Mon, 25 Mar 2024 13:55:33 +0100 Subject: [PATCH 7/9] review Co-authored-by: Oleksandr Redko --- pkg/lint/lintersdb/validator.go | 3 +-- pkg/logutils/mock.go | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/lint/lintersdb/validator.go b/pkg/lint/lintersdb/validator.go index 2503624aebcd..56d59c6332c8 100644 --- a/pkg/lint/lintersdb/validator.go +++ b/pkg/lint/lintersdb/validator.go @@ -91,8 +91,7 @@ func (v Validator) alternativeNamesDeprecation(cfg *config.Linters) error { } } - var names []string - names = append(names, cfg.Enable...) + names := append([]string{}, cfg.Enable...) names = append(names, cfg.Disable...) for _, name := range names { diff --git a/pkg/logutils/mock.go b/pkg/logutils/mock.go index 2e72e0ffeae9..e0a92346420f 100644 --- a/pkg/logutils/mock.go +++ b/pkg/logutils/mock.go @@ -47,8 +47,7 @@ func (m *MockLog) SetLevel(level LogLevel) { } func (m *MockLog) OnFatalf(format string, args ...any) *MockLog { - arguments := []any{format} - arguments = append(arguments, args...) + arguments := append([]any{format}, args...) m.On("Fatalf", arguments...) From 1b13606f3a95564816778a1f84f9c756651aecdc Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Mon, 25 Mar 2024 15:59:28 +0100 Subject: [PATCH 8/9] review --- pkg/lint/lintersdb/validator.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/lint/lintersdb/validator.go b/pkg/lint/lintersdb/validator.go index 56d59c6332c8..62100321ef13 100644 --- a/pkg/lint/lintersdb/validator.go +++ b/pkg/lint/lintersdb/validator.go @@ -43,7 +43,7 @@ func (v Validator) Validate(cfg *config.Config) error { } func (v Validator) validateLintersNames(cfg *config.Linters) error { - allNames := append([]string{}, cfg.Enable...) + allNames := cfg.Enable allNames = append(allNames, cfg.Disable...) var unknownNames []string @@ -91,7 +91,7 @@ func (v Validator) alternativeNamesDeprecation(cfg *config.Linters) error { } } - names := append([]string{}, cfg.Enable...) + names := cfg.Enable names = append(names, cfg.Disable...) for _, name := range names { From 44e002935aab5f71dd9a3ccedaac7429eadc5945 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 28 Mar 2024 20:38:43 +0100 Subject: [PATCH 9/9] review --- pkg/logutils/mock.go | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/pkg/logutils/mock.go b/pkg/logutils/mock.go index e0a92346420f..bddcf85523a7 100644 --- a/pkg/logutils/mock.go +++ b/pkg/logutils/mock.go @@ -13,28 +13,23 @@ func NewMockLog() *MockLog { } func (m *MockLog) Fatalf(format string, args ...any) { - mArgs := []any{format} - m.Called(append(mArgs, args...)...) + m.Called(append([]any{format}, args...)...) } func (m *MockLog) Panicf(format string, args ...any) { - mArgs := []any{format} - m.Called(append(mArgs, args...)...) + m.Called(append([]any{format}, args...)...) } func (m *MockLog) Errorf(format string, args ...any) { - mArgs := []any{format} - m.Called(append(mArgs, args...)...) + m.Called(append([]any{format}, args...)...) } func (m *MockLog) Warnf(format string, args ...any) { - mArgs := []any{format} - m.Called(append(mArgs, args...)...) + m.Called(append([]any{format}, args...)...) } func (m *MockLog) Infof(format string, args ...any) { - mArgs := []any{format} - m.Called(append(mArgs, args...)...) + m.Called(append([]any{format}, args...)...) } func (m *MockLog) Child(name string) Log { @@ -55,8 +50,7 @@ func (m *MockLog) OnFatalf(format string, args ...any) *MockLog { } func (m *MockLog) OnPanicf(format string, args ...any) *MockLog { - arguments := []any{format} - arguments = append(arguments, args...) + arguments := append([]any{format}, args...) m.On("Panicf", arguments...) @@ -64,8 +58,7 @@ func (m *MockLog) OnPanicf(format string, args ...any) *MockLog { } func (m *MockLog) OnErrorf(format string, args ...any) *MockLog { - arguments := []any{format} - arguments = append(arguments, args...) + arguments := append([]any{format}, args...) m.On("Errorf", arguments...) @@ -73,8 +66,7 @@ func (m *MockLog) OnErrorf(format string, args ...any) *MockLog { } func (m *MockLog) OnWarnf(format string, args ...any) *MockLog { - arguments := []any{format} - arguments = append(arguments, args...) + arguments := append([]any{format}, args...) m.On("Warnf", arguments...) @@ -82,8 +74,7 @@ func (m *MockLog) OnWarnf(format string, args ...any) *MockLog { } func (m *MockLog) OnInfof(format string, args ...any) *MockLog { - arguments := []any{format} - arguments = append(arguments, args...) + arguments := append([]any{format}, args...) m.On("Infof", arguments...)