From 37d3aa437a9192ae167c71e63719ddf1c858be7f Mon Sep 17 00:00:00 2001 From: Ludovic Fernandez Date: Sun, 21 Aug 2022 21:37:47 +0200 Subject: [PATCH] feat: deprecate varcheck, deadcode, and structcheck (#3125) --- .golangci.yml | 3 --- pkg/lint/lintersdb/manager.go | 9 ++++--- pkg/result/processors/nolint_test.go | 25 ++++++++++++------- pkg/result/processors/processor_test.go | 6 +++++ .../processors/testdata/nolint_bad_names.go | 23 ++++++++++++++--- .../processors/testdata/nolint_whole_file.go | 14 +++++++++-- test/bench/bench_test.go | 5 ---- test/run_test.go | 15 ++--------- test/testdata/deadcode.go | 2 +- test/testdata/deadcode_main_pkg/main_test.go | 15 ----------- test/testdata/gomodguard.go | 4 +-- test/testdata/nolintlint.go | 2 +- test/testdata/varcheck.go | 2 +- 13 files changed, 66 insertions(+), 59 deletions(-) delete mode 100644 test/testdata/deadcode_main_pkg/main_test.go diff --git a/.golangci.yml b/.golangci.yml index 90c71c9eb678..c12bc0d439cb 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -68,7 +68,6 @@ linters: disable-all: true enable: - bodyclose - - deadcode - depguard - dogsled - dupl @@ -93,13 +92,11 @@ linters: - noctx - nolintlint - staticcheck - - structcheck - stylecheck - typecheck - unconvert - unparam - unused - - varcheck - whitespace # don't enable: diff --git a/pkg/lint/lintersdb/manager.go b/pkg/lint/lintersdb/manager.go index 4d69aac6f6d2..668eae22ff0a 100644 --- a/pkg/lint/lintersdb/manager.go +++ b/pkg/lint/lintersdb/manager.go @@ -320,7 +320,8 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { WithSince("v1.0.0"). WithLoadForGoAnalysis(). WithPresets(linter.PresetUnused). - WithURL("https://github.com/remyoudompheng/go-misc/tree/master/deadcode"), + WithURL("https://github.com/remyoudompheng/go-misc/tree/master/deadcode"). + Deprecated("The owner seems to have abandoned the linter.", "v1.49.0", "unused"), linter.NewConfig(golinters.NewDepguard(depGuardCfg)). WithSince("v1.4.0"). @@ -714,6 +715,7 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { WithLoadForGoAnalysis(). WithPresets(linter.PresetUnused). WithURL("https://github.com/opennota/check"). + Deprecated("The owner seems to have abandoned the linter.", "v1.49.0", "unused"). WithNoopFallback(m.cfg), linter.NewConfig(golinters.NewStylecheck(stylecheckCfg)). @@ -786,7 +788,8 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { WithSince("v1.0.0"). WithLoadForGoAnalysis(). WithPresets(linter.PresetUnused). - WithURL("https://github.com/opennota/check"), + WithURL("https://github.com/opennota/check"). + Deprecated("The owner seems to have abandoned the linter.", "v1.49.0", "unused"), linter.NewConfig(golinters.NewVarnamelen(varnamelenCfg)). WithSince("v1.43.0"). @@ -831,9 +834,7 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { golinters.NewStaticcheck(staticcheckCfg).Name(): true, golinters.NewUnused(unusedCfg).Name(): true, golinters.NewGosimple(gosimpleCfg).Name(): true, - golinters.NewVarcheck(varcheckCfg).Name(): true, golinters.NewIneffassign().Name(): true, - golinters.NewDeadcode().Name(): true, golinters.NewTypecheck().Name(): true, } return enableLinterConfigs(lcs, func(lc *linter.Config) bool { diff --git a/pkg/result/processors/nolint_test.go b/pkg/result/processors/nolint_test.go index 97ab34e51379..0ebe8c9a78a0 100644 --- a/pkg/result/processors/nolint_test.go +++ b/pkg/result/processors/nolint_test.go @@ -128,16 +128,23 @@ func TestNolintInvalidLinterName(t *testing.T) { { Pos: token.Position{ Filename: fileName, - Line: 3, + Line: 10, }, - FromLinter: "varcheck", + FromLinter: "errcheck", + }, + { + Pos: token.Position{ + Filename: fileName, + Line: 13, + }, + FromLinter: "errcheck", }, { Pos: token.Position{ Filename: fileName, - Line: 6, + Line: 22, }, - FromLinter: "deadcode", + FromLinter: "ineffassign", }, } @@ -244,22 +251,22 @@ func TestIgnoredRangeMatches(t *testing.T) { func TestNolintWholeFile(t *testing.T) { fileName := filepath.Join("testdata", "nolint_whole_file.go") - p := newTestNolintProcessor(nil) + p := newTestNolintProcessor(getMockLog()) defer p.Finish() processAssertEmpty(t, p, result.Issue{ Pos: token.Position{ Filename: fileName, - Line: 4, + Line: 9, }, - FromLinter: "varcheck", + FromLinter: "errcheck", }) processAssertSame(t, p, result.Issue{ Pos: token.Position{ Filename: fileName, - Line: 4, + Line: 14, }, - FromLinter: "deadcode", + FromLinter: "govet", }) } diff --git a/pkg/result/processors/processor_test.go b/pkg/result/processors/processor_test.go index ef9117416255..3f396b437266 100644 --- a/pkg/result/processors/processor_test.go +++ b/pkg/result/processors/processor_test.go @@ -35,17 +35,23 @@ func newIssueFromTextTestCase(text string) result.Issue { } func process(t *testing.T, p Processor, issues ...result.Issue) []result.Issue { + t.Helper() + processedIssues, err := p.Process(issues) assert.NoError(t, err) return processedIssues } func processAssertSame(t *testing.T, p Processor, issues ...result.Issue) { + t.Helper() + processedIssues := process(t, p, issues...) assert.Equal(t, issues, processedIssues) } func processAssertEmpty(t *testing.T, p Processor, issues ...result.Issue) { + t.Helper() + processedIssues := process(t, p, issues...) assert.Empty(t, processedIssues) } diff --git a/pkg/result/processors/testdata/nolint_bad_names.go b/pkg/result/processors/testdata/nolint_bad_names.go index c3b04dfb6ab2..2335751a5bec 100644 --- a/pkg/result/processors/testdata/nolint_bad_names.go +++ b/pkg/result/processors/testdata/nolint_bad_names.go @@ -1,8 +1,25 @@ package testdata -var nolintUnknownLinter1 bool //nolint:bad1,deadcode,varcheck,megacheck +import "math" -//nolint:bad2,deadcode,megacheck -func nolintUnknownLinter2() { +func RetErr() error { + return nil +} + +func MissedErrorCheck() { + RetErr() //nolint:bad1,errcheck +} + +//nolint:bad2,errcheck +func MissedErrorCheck2() { + RetErr() +} +func _() { + x := math.MinInt8 + for { + _ = x + x = 0 //nolint:bad1,ineffassign + x = 0 + } } diff --git a/pkg/result/processors/testdata/nolint_whole_file.go b/pkg/result/processors/testdata/nolint_whole_file.go index e0f93c83b2f8..49826524d31c 100644 --- a/pkg/result/processors/testdata/nolint_whole_file.go +++ b/pkg/result/processors/testdata/nolint_whole_file.go @@ -1,4 +1,14 @@ -//nolint: varcheck +//nolint:errcheck package testdata -var nolintVarcheck int +func RetError() error { + return nil +} + +func MissedErrorCheck1() { + RetErr() +} + +func jo(v, w bool) bool { + return v == w || v == w +} diff --git a/test/bench/bench_test.go b/test/bench/bench_test.go index 3e565b9732c1..c96d14a37453 100644 --- a/test/bench/bench_test.go +++ b/test/bench/bench_test.go @@ -47,12 +47,7 @@ func prepareGithubProject(owner, name string) func(*testing.B) { func getBenchLintersArgsNoMegacheck() []string { return []string{ - "--enable=deadcode", "--enable=gocyclo", - "--enable=golint", - "--enable=varcheck", - "--enable=structcheck", - "--enable=maligned", "--enable=errcheck", "--enable=dupl", "--enable=ineffassign", diff --git a/test/run_test.go b/test/run_test.go index bde6f4347952..2128a68c5f9a 100644 --- a/test/run_test.go +++ b/test/run_test.go @@ -251,8 +251,8 @@ func TestSortedResults(t *testing.T) { { opt: "--sort-results=false", want: strings.Join([]string{ - "testdata/sort_results/main.go:12:5: `db` is unused (deadcode)", "testdata/sort_results/main.go:15:13: Error return value is not checked (errcheck)", + "testdata/sort_results/main.go:12:5: var `db` is unused (unused)", "testdata/sort_results/main.go:8:6: func `returnError` is unused (unused)", }, "\n"), }, @@ -260,7 +260,7 @@ func TestSortedResults(t *testing.T) { opt: "--sort-results=true", want: strings.Join([]string{ "testdata/sort_results/main.go:8:6: func `returnError` is unused (unused)", - "testdata/sort_results/main.go:12:5: `db` is unused (deadcode)", + "testdata/sort_results/main.go:12:5: var `db` is unused (unused)", "testdata/sort_results/main.go:15:13: Error return value is not checked (errcheck)", }, "\n"), }, @@ -435,17 +435,6 @@ func TestSkippedDirsTestdata(t *testing.T) { ExpectNoIssues() // all was skipped because in testdata } -func TestDeadcodeNoFalsePositivesInMainPkg(t *testing.T) { - testshared.NewRunnerBuilder(t). - WithNoConfig(). - WithArgs("--disable-all", "-Edeadcode"). - WithTargetPath(testdataDir, "deadcode_main_pkg"). - Runner(). - Install(). - Run(). - ExpectNoIssues() -} - func TestIdentifierUsedOnlyInTests(t *testing.T) { testshared.NewRunnerBuilder(t). WithNoConfig(). diff --git a/test/testdata/deadcode.go b/test/testdata/deadcode.go index e2c55580dd34..670047a548f0 100644 --- a/test/testdata/deadcode.go +++ b/test/testdata/deadcode.go @@ -1,4 +1,4 @@ -//golangcitest:args -Edeadcode +//golangcitest:args -Edeadcode --internal-cmd-test package testdata var y int diff --git a/test/testdata/deadcode_main_pkg/main_test.go b/test/testdata/deadcode_main_pkg/main_test.go deleted file mode 100644 index 6a30794fa9ff..000000000000 --- a/test/testdata/deadcode_main_pkg/main_test.go +++ /dev/null @@ -1,15 +0,0 @@ -package main - -import "testing" - -func TestF(t *testing.T) { - -} - -func BenchmarkF(t *testing.B) { - -} - -func ExampleF() { - -} diff --git a/test/testdata/gomodguard.go b/test/testdata/gomodguard.go index a4c18c4ce50e..9cb1cf68f67c 100644 --- a/test/testdata/gomodguard.go +++ b/test/testdata/gomodguard.go @@ -12,13 +12,13 @@ import ( // Something just some struct type Something struct{} -func aAllowedImport() { //nolint:deadcode,unused +func aAllowedImport() { //nolint:unused mfile, _ := modfile.Parse("go.mod", []byte{}, nil) log.Println(mfile) } -func aBlockedImport() { //nolint:deadcode,unused +func aBlockedImport() { //nolint:unused data := []byte{} something := Something{} _ = yaml.Unmarshal(data, &something) diff --git a/test/testdata/nolintlint.go b/test/testdata/nolintlint.go index 719576b77531..adb7bbd50a03 100644 --- a/test/testdata/nolintlint.go +++ b/test/testdata/nolintlint.go @@ -8,7 +8,7 @@ import "fmt" func Foo() { fmt.Println("not specific") //nolint // want "directive `.*` should mention specific linter such as `//nolint:my-linter`" fmt.Println("not machine readable") // nolint // want "directive `.*` should be written as `//nolint`" - fmt.Println("extra spaces") // nolint:deadcode // because // want "directive `.*` should not have more than one leading space" + fmt.Println("extra spaces") // nolint:unused // because // want "directive `.*` should not have more than one leading space" // test expanded range //nolint:misspell // deliberate misspelling to trigger nolintlint diff --git a/test/testdata/varcheck.go b/test/testdata/varcheck.go index d4bc8f7bc5a4..858c236d2010 100644 --- a/test/testdata/varcheck.go +++ b/test/testdata/varcheck.go @@ -1,4 +1,4 @@ -//golangcitest:args -Evarcheck +//golangcitest:args -Evarcheck --internal-cmd-test package testdata var v string // want "`v` is unused"