From 41aa8433af52615edd5e4d5732ce1bbb6a2086c1 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 14 Jul 2022 22:30:21 +0200 Subject: [PATCH] feat: drop allow-leading-space and add "all" --- .golangci.yml | 2 +- docs/src/docs/usage/false-positives.mdx | 6 +-- pkg/config/linters_settings.go | 2 - pkg/golinters/godot.go | 2 +- pkg/golinters/importas.go | 4 +- pkg/golinters/nolintlint.go | 3 -- pkg/golinters/nolintlint/README.md | 8 ++-- pkg/golinters/nolintlint/nolintlint.go | 13 ++++--- pkg/golinters/nolintlint/nolintlint_test.go | 38 +++++++------------ pkg/golinters/staticcheck_common.go | 2 +- pkg/result/processors/base_rule.go | 2 +- pkg/result/processors/nolint.go | 9 +++-- .../testdata/autogen_exclude_doc.go | 2 +- pkg/result/processors/testdata/nolint.go | 22 +++++------ .../processors/testdata/nolint_bad_names.go | 2 +- .../processors/testdata/nolint_unused.go | 4 +- scripts/expand_website_templates/main.go | 2 +- test/testdata/default_exclude.go | 5 ++- test/testdata/fix/in/nolintlint.go | 2 +- test/testdata/gomodguard.go | 4 +- test/testdata/nolintlint_unused.go | 6 +-- tools/tools.go | 3 +- 22 files changed, 67 insertions(+), 76 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index fbbe30f93e77..9778170cb97a 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -61,7 +61,7 @@ linters-settings: misspell: locale: US nolintlint: - allow-leading-space: true # don't require machine-readable nolint directives (i.e. with no leading space) + allow-leading-space: false allow-unused: false # report any unused nolint directives require-explanation: false # don't require an explanation for nolint directives require-specific: false # don't require nolint directives to be specific about which linter is being skipped diff --git a/docs/src/docs/usage/false-positives.mdx b/docs/src/docs/usage/false-positives.mdx index 9b0f2fba174c..ade523fca4a4 100644 --- a/docs/src/docs/usage/false-positives.mdx +++ b/docs/src/docs/usage/false-positives.mdx @@ -95,11 +95,11 @@ run: ## Nolint Directive -To exclude issues from all linters use `//nolint`. +To exclude issues from all linters use `//nolint:all`. For example, if it's used inline (not from the beginning of the line) it excludes issues only for this line. ```go -var bad_name int //nolint +var bad_name int //nolint:all ``` To exclude issues from specific linters only: @@ -111,7 +111,7 @@ var bad_name int //nolint:golint,unused To exclude issues for the block of code use this directive on the beginning of a line: ```go -//nolint +//nolint:all func allIssuesInThisFunctionAreExcluded() *string { // ... } diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index b0bc1ac82ffd..14bf13cfd804 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -77,7 +77,6 @@ var defaultLintersSettings = LintersSettings{ }, NoLintLint: NoLintLintSettings{ RequireExplanation: false, - AllowLeadingSpace: true, RequireSpecific: false, AllowUnused: false, }, @@ -488,7 +487,6 @@ type NlreturnSettings struct { type NoLintLintSettings struct { RequireExplanation bool `mapstructure:"require-explanation"` - AllowLeadingSpace bool `mapstructure:"allow-leading-space"` RequireSpecific bool `mapstructure:"require-specific"` AllowNoExplanation []string `mapstructure:"allow-no-explanation"` AllowUnused bool `mapstructure:"allow-unused"` diff --git a/pkg/golinters/godot.go b/pkg/golinters/godot.go index 72322fa717a9..93ca7577ae89 100644 --- a/pkg/golinters/godot.go +++ b/pkg/golinters/godot.go @@ -30,7 +30,7 @@ func NewGodot(settings *config.GodotSettings) *goanalysis.Linter { // Convert deprecated setting // todo(butuzov): remove on v2 release - if settings.CheckAll { // nolint:staticcheck // Keep for retro-compatibility. + if settings.CheckAll { //nolint:staticcheck // Keep for retro-compatibility. dotSettings.Scope = godot.AllScope } } diff --git a/pkg/golinters/importas.go b/pkg/golinters/importas.go index d1f042829571..1917bbb0c68b 100644 --- a/pkg/golinters/importas.go +++ b/pkg/golinters/importas.go @@ -4,7 +4,7 @@ import ( "fmt" "strconv" - "github.com/julz/importas" // nolint: misspell + "github.com/julz/importas" //nolint:misspell "golang.org/x/tools/go/analysis" "github.com/golangci/golangci-lint/pkg/config" @@ -25,7 +25,7 @@ func NewImportAs(settings *config.ImportAsSettings) *goanalysis.Linter { return } if len(settings.Alias) == 0 { - lintCtx.Log.Infof("importas settings found, but no aliases listed. List aliases under alias: key.") // nolint: misspell + lintCtx.Log.Infof("importas settings found, but no aliases listed. List aliases under alias: key.") //nolint:misspell } if err := analyzer.Flags.Set("no-unaliased", strconv.FormatBool(settings.NoUnaliased)); err != nil { diff --git a/pkg/golinters/nolintlint.go b/pkg/golinters/nolintlint.go index 62b8064ba2ae..a809f44995ee 100644 --- a/pkg/golinters/nolintlint.go +++ b/pkg/golinters/nolintlint.go @@ -57,9 +57,6 @@ func runNoLintLint(pass *analysis.Pass, settings *config.NoLintLintSettings) ([] if settings.RequireExplanation { needs |= nolintlint.NeedsExplanation } - if !settings.AllowLeadingSpace { - needs |= nolintlint.NeedsMachineOnly - } if settings.RequireSpecific { needs |= nolintlint.NeedsSpecific } diff --git a/pkg/golinters/nolintlint/README.md b/pkg/golinters/nolintlint/README.md index 3d440d5a5b9c..9f4604d1202f 100644 --- a/pkg/golinters/nolintlint/README.md +++ b/pkg/golinters/nolintlint/README.md @@ -1,6 +1,6 @@ # nolintlint -nolintlint is a Go static analysis tool to find ill-formed or insufficiently explained `// nolint` directives for golangci +nolintlint is a Go static analysis tool to find ill-formed or insufficiently explained `//nolint` directives for golangci (or any other linter, using th ) ## Purpose @@ -8,10 +8,10 @@ nolintlint is a Go static analysis tool to find ill-formed or insufficiently exp To ensure that lint exceptions have explanations. Consider the case below: ```Go -import "crypto/md5" //nolint +import "crypto/md5" //nolint:all func hash(data []byte) []byte { - return md5.New().Sum(data) //nolint + return md5.New().Sum(data) //nolint:all } ``` @@ -27,5 +27,5 @@ func hash(data []byte) []byte { ``` `nolintlint` can also identify cases where you may have written `// nolint`. Finally `nolintlint`, can also enforce that you -use the machine-readable nolint directive format `//nolint` and that you mention what linter is being suppressed, as shown above when we write `//nolint:gosec`. +use the machine-readable nolint directive format `//nolint:all` and that you mention what linter is being suppressed, as shown above when we write `//nolint:gosec`. diff --git a/pkg/golinters/nolintlint/nolintlint.go b/pkg/golinters/nolintlint/nolintlint.go index 064fd61a6100..9c6b10f38c50 100644 --- a/pkg/golinters/nolintlint/nolintlint.go +++ b/pkg/golinters/nolintlint/nolintlint.go @@ -152,7 +152,7 @@ func NewLinter(needs Needs, excludes []string) (*Linter, error) { } return &Linter{ - needs: needs, + needs: needs | NeedsMachineOnly, excludeByLinter: excludeByName, }, nil } @@ -184,12 +184,14 @@ func (l Linter) Run(fset *token.FileSet, nodes ...ast.Node) ([]Issue, error) { leadingSpace = leadingSpaceMatches[1] } - directiveWithOptionalLeadingSpace := comment.Text + directiveWithOptionalLeadingSpace := "//" if len(leadingSpace) > 0 { - split := strings.Split(strings.SplitN(comment.Text, ":", 2)[0], "//") - directiveWithOptionalLeadingSpace = "// " + strings.TrimSpace(split[1]) + directiveWithOptionalLeadingSpace += " " } + split := strings.Split(strings.SplitN(comment.Text, ":", 2)[0], "//") + directiveWithOptionalLeadingSpace += strings.TrimSpace(split[1]) + pos := fset.Position(comment.Pos()) end := fset.Position(comment.End()) @@ -227,8 +229,9 @@ func (l Linter) Run(fset *token.FileSet, nodes ...ast.Node) ([]Issue, error) { } lintersText, explanation := fullMatches[1], fullMatches[2] + var linters []string - if len(lintersText) > 0 { + if len(lintersText) > 0 && !strings.HasPrefix(lintersText, "all") { lls := strings.Split(lintersText, ",") linters = make([]string, 0, len(lls)) rangeStart := (pos.Column - 1) + len("//") + len(leadingSpace) + len("nolint:") diff --git a/pkg/golinters/nolintlint/nolintlint_test.go b/pkg/golinters/nolintlint/nolintlint_test.go index dd6da110a41b..3dba4d6578b3 100644 --- a/pkg/golinters/nolintlint/nolintlint_test.go +++ b/pkg/golinters/nolintlint/nolintlint_test.go @@ -12,7 +12,7 @@ import ( ) //nolint:funlen -func TestNoLintLint(t *testing.T) { +func TestLinter_Run(t *testing.T) { type issueWithReplacement struct { issue string replacement *result.Replacement @@ -80,21 +80,21 @@ package bar func foo() { good() //nolint:my-linter bad() //nolint - bad() // nolint // because + bad() //nolint // because }`, expected: []issueWithReplacement{ {issue: "directive `//nolint` should mention specific linter such as `//nolint:my-linter` at testing.go:6:9"}, - {issue: "directive `// nolint // because` should mention specific linter such as `// nolint:my-linter` at testing.go:7:9"}, + {issue: "directive `//nolint // because` should mention specific linter such as `//nolint:my-linter` at testing.go:7:9"}, }, }, { - desc: "when machine-readable style isn't used", - needs: NeedsMachineOnly, + desc: "when machine-readable style isn't used", contents: ` package bar func foo() { bad() // nolint + bad() // nolint good() //nolint }`, expected: []issueWithReplacement{ @@ -108,25 +108,13 @@ func foo() { }, }, }, - }, - }, - { - desc: "extra spaces in front of directive are reported", - contents: ` -package bar - -func foo() { - bad() // nolint - good() // nolint -}`, - expected: []issueWithReplacement{ { - issue: "directive `// nolint` should not have more than one leading space at testing.go:5:9", + issue: "directive `// nolint` should be written without leading space as `//nolint` at testing.go:6:9", replacement: &result.Replacement{ Inline: &result.InlineFix{ StartCol: 10, - Length: 2, - NewString: " ", + Length: 3, + NewString: "", }, }, }, @@ -138,13 +126,13 @@ func foo() { package bar func foo() { - good() // nolint:linter1,linter-two - bad() // nolint:linter1 linter2 - good() // nolint: linter1,linter2 - good() // nolint: linter1, linter2 + good() //nolint:linter1,linter-two + bad() //nolint:linter1 linter2 + good() //nolint: linter1,linter2 + good() //nolint: linter1, linter2 }`, expected: []issueWithReplacement{ - {issue: "directive `// nolint:linter1 linter2` should match `// nolint[:] [// ]` at testing.go:6:9"}, //nolint:lll // this is a string + {issue: "directive `//nolint:linter1 linter2` should match `//nolint[:] [// ]` at testing.go:6:9"}, //nolint:lll // this is a string }, }, { diff --git a/pkg/golinters/staticcheck_common.go b/pkg/golinters/staticcheck_common.go index e54cf8e8baa5..35d473883940 100644 --- a/pkg/golinters/staticcheck_common.go +++ b/pkg/golinters/staticcheck_common.go @@ -98,7 +98,7 @@ func staticCheckConfig(settings *config.StaticCheckSettings) *scconfig.Config { } // https://github.com/dominikh/go-tools/blob/9bf17c0388a65710524ba04c2d821469e639fdc2/lintcmd/lint.go#L437-L477 -// nolint // Keep the original source code. +//nolint:gocritic // Keep the original source code. func filterAnalyzerNames(analyzers []string, checks []string) map[string]bool { allowedChecks := map[string]bool{} diff --git a/pkg/result/processors/base_rule.go b/pkg/result/processors/base_rule.go index b6ce4f2159e9..6958b9f2f37d 100644 --- a/pkg/result/processors/base_rule.go +++ b/pkg/result/processors/base_rule.go @@ -58,7 +58,7 @@ func (r *baseRule) matchLinter(issue *result.Issue) bool { return false } -func (r *baseRule) matchSource(issue *result.Issue, lineCache *fsutils.LineCache, log logutils.Log) bool { // nolint:interfacer +func (r *baseRule) matchSource(issue *result.Issue, lineCache *fsutils.LineCache, log logutils.Log) bool { //nolint:interfacer sourceLine, errSourceLine := lineCache.GetLine(issue.FilePath(), issue.Line()) if errSourceLine != nil { log.Warnf("Failed to get line %s:%d from line cache: %s", issue.FilePath(), issue.Line(), errSourceLine) diff --git a/pkg/result/processors/nolint.go b/pkg/result/processors/nolint.go index 01f597e94d5b..fd176c25e69e 100644 --- a/pkg/result/processors/nolint.go +++ b/pkg/result/processors/nolint.go @@ -252,7 +252,7 @@ func (p *Nolint) extractInlineRangeFromComment(text string, g ast.Node, fset *to } } - if !strings.HasPrefix(text, "nolint:") { + if strings.HasPrefix(text, "nolint:all") || !strings.HasPrefix(text, "nolint:") { return buildRange(nil) // ignore all linters } @@ -260,8 +260,11 @@ func (p *Nolint) extractInlineRangeFromComment(text string, g ast.Node, fset *to var linters []string text = strings.Split(text, "//")[0] // allow another comment after this comment linterItems := strings.Split(strings.TrimPrefix(text, "nolint:"), ",") - for _, linter := range linterItems { - linterName := strings.ToLower(strings.TrimSpace(linter)) + for _, item := range linterItems { + linterName := strings.ToLower(strings.TrimSpace(item)) + if linterName == "all" { + panic("OOPS") + } lcs := p.dbManager.GetLinterConfigs(linterName) if lcs == nil { diff --git a/pkg/result/processors/testdata/autogen_exclude_doc.go b/pkg/result/processors/testdata/autogen_exclude_doc.go index 7eadceffd666..7c01908cadb0 100644 --- a/pkg/result/processors/testdata/autogen_exclude_doc.go +++ b/pkg/result/processors/testdata/autogen_exclude_doc.go @@ -4,7 +4,7 @@ package testdata import "fmt" -// nolint +//nolint:all func PrintString(s string) { fmt.Printf("%s", s) } diff --git a/pkg/result/processors/testdata/nolint.go b/pkg/result/processors/testdata/nolint.go index f7b11701fbb9..5d63be199c54 100644 --- a/pkg/result/processors/testdata/nolint.go +++ b/pkg/result/processors/testdata/nolint.go @@ -1,28 +1,28 @@ package testdata -var nolintSpecific int // nolint:gofmt +var nolintSpecific int //nolint:gofmt var nolintSpace int // nolint: gofmt -var nolintSpaces int //nolint: gofmt, govet -var nolintAll int // nolint -var nolintAndAppendix int // nolint // another comment +var nolintSpaces int //nolint:gofmt, govet +var nolintAll int // nolint:all +var nolintAndAppendix int // nolint:all // another comment -//nolint +//nolint:all var nolintVarByPrecedingComment int -//nolint +//nolint:all var dontNolintVarByPrecedingCommentBecauseOfNewLine int -var nolintPrecedingVar string //nolint +var nolintPrecedingVar string //nolint:all var dontNolintVarByPrecedingCommentBecauseOfDifferentColumn int -//nolint +//nolint:all func nolintFuncByPrecedingComment() *string { xv := "v" return &xv } -//nolint +//nolint:all // second line func nolintFuncByPrecedingMultilineComment1() *string { xv := "v" @@ -30,14 +30,14 @@ func nolintFuncByPrecedingMultilineComment1() *string { } // first line -//nolint +//nolint:all func nolintFuncByPrecedingMultilineComment2() *string { xv := "v" return &xv } // first line -//nolint +//nolint:all // third line func nolintFuncByPrecedingMultilineComment3() *string { xv := "v" diff --git a/pkg/result/processors/testdata/nolint_bad_names.go b/pkg/result/processors/testdata/nolint_bad_names.go index 3b8b4caee8ae..c3b04dfb6ab2 100644 --- a/pkg/result/processors/testdata/nolint_bad_names.go +++ b/pkg/result/processors/testdata/nolint_bad_names.go @@ -1,6 +1,6 @@ package testdata -var nolintUnknownLinter1 bool // nolint:bad1,deadcode,varcheck,megacheck +var nolintUnknownLinter1 bool //nolint:bad1,deadcode,varcheck,megacheck //nolint:bad2,deadcode,megacheck func nolintUnknownLinter2() { diff --git a/pkg/result/processors/testdata/nolint_unused.go b/pkg/result/processors/testdata/nolint_unused.go index dc8eb49e6da1..b9960b807c47 100644 --- a/pkg/result/processors/testdata/nolint_unused.go +++ b/pkg/result/processors/testdata/nolint_unused.go @@ -1,5 +1,5 @@ package testdata -var nolintVarcheck int // nolint:varcheck +var nolintVarcheck int //nolint:varcheck -var nolintVarcheckUnusedOK int // nolint:varcheck,nolintlint +var nolintVarcheckUnusedOK int //nolint:varcheck,nolintlint diff --git a/scripts/expand_website_templates/main.go b/scripts/expand_website_templates/main.go index 4e8e2dcc3f21..9777271d25f4 100644 --- a/scripts/expand_website_templates/main.go +++ b/scripts/expand_website_templates/main.go @@ -137,7 +137,7 @@ type latestRelease struct { } func getLatestVersion() (string, error) { - req, err := http.NewRequest( // nolint:noctx + req, err := http.NewRequest( //nolint:noctx http.MethodGet, "https://api.github.com/repos/golangci/golangci-lint/releases/latest", http.NoBody, diff --git a/test/testdata/default_exclude.go b/test/testdata/default_exclude.go index 3c2a13a919c3..be4547f58ed8 100644 --- a/test/testdata/default_exclude.go +++ b/test/testdata/default_exclude.go @@ -11,10 +11,11 @@ func ExportedFunc1() { } // InvalidFuncComment // ERROR stylecheck `ST1020: comment on exported function ExportedFunc2 should be of the form "ExportedFunc2 ..."` -// nolint:golint +// +//nolint:golint func ExportedFunc2() { } -// nolint:stylecheck +//nolint:stylecheck func IgnoreAll() { } diff --git a/test/testdata/fix/in/nolintlint.go b/test/testdata/fix/in/nolintlint.go index 8b484ffbcf34..96468754da6b 100644 --- a/test/testdata/fix/in/nolintlint.go +++ b/test/testdata/fix/in/nolintlint.go @@ -10,7 +10,7 @@ func nolintlint() { fmt.Println() // nolint:bob // leading spaces should be dropped // note that the next lines will retain trailing whitespace when fixed - fmt.Println() //nolint // nolint should be dropped + fmt.Println() //nolint:all // nolint should be dropped fmt.Println() //nolint:lll // nolint should be dropped fmt.Println() //nolint:alice,lll // we don't drop individual linters from lists diff --git a/test/testdata/gomodguard.go b/test/testdata/gomodguard.go index 5b98163959df..64bfdc02bca2 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:deadcode,unused mfile, _ := modfile.Parse("go.mod", []byte{}, nil) log.Println(mfile) } -func aBlockedImport() { // nolint: deadcode,unused +func aBlockedImport() { //nolint:deadcode,unused data := []byte{} something := Something{} _ = yaml.Unmarshal(data, &something) diff --git a/test/testdata/nolintlint_unused.go b/test/testdata/nolintlint_unused.go index a343cc99960c..96bc2a70cbfa 100644 --- a/test/testdata/nolintlint_unused.go +++ b/test/testdata/nolintlint_unused.go @@ -6,7 +6,7 @@ package testdata import "fmt" func Foo() { - fmt.Println("unused") // nolint // ERROR "directive `//nolint .*` is unused" - fmt.Println("unused,specific") // nolint:varcheck // ERROR "directive `//nolint:varcheck .*` is unused for linter varcheck" - fmt.Println("not run") // nolint:unparam // unparam is not run so this is ok + fmt.Println("unused") //nolint:all // ERROR "directive `//nolint .*` is unused" + fmt.Println("unused,specific") //nolint:varcheck // ERROR "directive `//nolint:varcheck .*` is unused for linter varcheck" + fmt.Println("not run") //nolint:unparam // unparam is not run so this is ok } diff --git a/tools/tools.go b/tools/tools.go index 23898f66786a..b36a0271d599 100644 --- a/tools/tools.go +++ b/tools/tools.go @@ -1,3 +1,4 @@ +//go:build tools // +build tools package tools @@ -7,7 +8,7 @@ package tools // https://github.com/golang/go/wiki/Modules#how-can-i-track-tool-dependencies-for-a-module // https://github.com/golang/go/issues/25922 // -// nolint +//nolint:all import ( _ "github.com/goreleaser/goreleaser" )