nolint: drop allow-leading-space option and add "nolint:all" (#3002)

ldez · web-flow · commit f8f8f9a6e730 · 2022-08-01T14:21:04.000+02:00
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -1,17 +1,17 @@
 # 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
 
 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
@@ -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
@@ -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[:<comma-separated-linters>] [// <explanation>]` at testing.go:6:9"}, //nolint:lll // this is a string
+				{issue: "directive `//nolint:linter1 linter2` should match `//nolint[:<comma-separated-linters>] [// <explanation>]` 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
@@ -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
@@ -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
@@ -252,16 +252,20 @@ 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
 	}
 
 	// ignore specific linters
 	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" {
+			p.unknownLintersSet = map[string]bool{}
+			return buildRange(nil)
+		}
 
 		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
@@ -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
@@ -1,43 +1,43 @@
 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"
 	return &xv
 }
 
 // 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
diff --git a/test/testdata/nolintlint_unused.go b/test/testdata/nolintlint_unused.go
diff --git a/tools/tools.go b/tools/tools.go

Original file line number	Diff line number	Diff line change
`@@ -30,7 +30,7 @@ func NewGodot(settings config.GodotSettings) goanalysis.Linter {`
`30`	`30`
`31`	`31`	`// Convert deprecated setting`
`32`	`32`	`// todo(butuzov): remove on v2 release`
`33`		`- if settings.CheckAll { // nolint:staticcheck // Keep for retro-compatibility.`
	`33`	`+ if settings.CheckAll { //nolint:staticcheck // Keep for retro-compatibility.`
`34`	`34`	`dotSettings.Scope = godot.AllScope`
`35`	`35`	`}`
`36`	`36`	`}`
Original file line number	Diff line number	Diff line change
`@@ -4,7 +4,7 @@ import (`
`4`	`4`	`"fmt"`
`5`	`5`	`"strconv"`
`6`	`6`
`7`		`- "github.com/julz/importas" // nolint: misspell`
	`7`	`+ "github.com/julz/importas" //nolint:misspell`
`8`	`8`	`"golang.org/x/tools/go/analysis"`
`9`	`9`
`10`	`10`	`"github.com/golangci/golangci-lint/pkg/config"`
`@@ -25,7 +25,7 @@ func NewImportAs(settings config.ImportAsSettings) goanalysis.Linter {`
`25`	`25`	`return`
`26`	`26`	`}`
`27`	`27`	`if len(settings.Alias) == 0 {`
`28`		`- lintCtx.Log.Infof("importas settings found, but no aliases listed. List aliases under alias: key.") // nolint: misspell`
	`28`	`+ lintCtx.Log.Infof("importas settings found, but no aliases listed. List aliases under alias: key.") //nolint:misspell`
`29`	`29`	`}`
`30`	`30`
`31`	`31`	`if err := analyzer.Flags.Set("no-unaliased", strconv.FormatBool(settings.NoUnaliased)); err != nil {`
Original file line number	Diff line number	Diff line change
`@@ -57,9 +57,6 @@ func runNoLintLint(pass analysis.Pass, settings config.NoLintLintSettings) ([]`
`57`	`57`	`if settings.RequireExplanation {`
`58`	`58`	`needs \|= nolintlint.NeedsExplanation`
`59`	`59`	`}`
`60`		`- if !settings.AllowLeadingSpace {`
`61`		`- needs \|= nolintlint.NeedsMachineOnly`
`62`		`- }`
`63`	`60`	`if settings.RequireSpecific {`
`64`	`61`	`needs \|= nolintlint.NeedsSpecific`
`65`	`62`	`}`
Original file line number	Diff line number	Diff line change
`@@ -98,7 +98,7 @@ func staticCheckConfig(settings config.StaticCheckSettings) scconfig.Config {`
`98`	`98`	`}`
`99`	`99`
`100`	`100`	`// https://github.com/dominikh/go-tools/blob/9bf17c0388a65710524ba04c2d821469e639fdc2/lintcmd/lint.go#L437-L477`
`101`		`-// nolint // Keep the original source code.`
	`101`	`+//nolint:gocritic // Keep the original source code.`
`102`	`102`	`func filterAnalyzerNames(analyzers []string, checks []string) map[string]bool {`
`103`	`103`	`allowedChecks := map[string]bool{}`
`104`	`104`