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

This commit is contained in:
Ludovic Fernandez 2022-08-01 14:21:04 +02:00 committed by GitHub
parent 39fc81c19c
commit f8f8f9a6e7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
22 changed files with 68 additions and 76 deletions

View File

@ -61,7 +61,7 @@ linters-settings:
misspell: misspell:
locale: US locale: US
nolintlint: 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 allow-unused: false # report any unused nolint directives
require-explanation: false # don't require an explanation for 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 require-specific: false # don't require nolint directives to be specific about which linter is being skipped

View File

@ -95,11 +95,11 @@ run:
## Nolint Directive ## 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. For example, if it's used inline (not from the beginning of the line) it excludes issues only for this line.
```go ```go
var bad_name int //nolint var bad_name int //nolint:all
``` ```
To exclude issues from specific linters only: 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: To exclude issues for the block of code use this directive on the beginning of a line:
```go ```go
//nolint //nolint:all
func allIssuesInThisFunctionAreExcluded() *string { func allIssuesInThisFunctionAreExcluded() *string {
// ... // ...
} }

View File

@ -77,7 +77,6 @@ var defaultLintersSettings = LintersSettings{
}, },
NoLintLint: NoLintLintSettings{ NoLintLint: NoLintLintSettings{
RequireExplanation: false, RequireExplanation: false,
AllowLeadingSpace: true,
RequireSpecific: false, RequireSpecific: false,
AllowUnused: false, AllowUnused: false,
}, },
@ -488,7 +487,6 @@ type NlreturnSettings struct {
type NoLintLintSettings struct { type NoLintLintSettings struct {
RequireExplanation bool `mapstructure:"require-explanation"` RequireExplanation bool `mapstructure:"require-explanation"`
AllowLeadingSpace bool `mapstructure:"allow-leading-space"`
RequireSpecific bool `mapstructure:"require-specific"` RequireSpecific bool `mapstructure:"require-specific"`
AllowNoExplanation []string `mapstructure:"allow-no-explanation"` AllowNoExplanation []string `mapstructure:"allow-no-explanation"`
AllowUnused bool `mapstructure:"allow-unused"` AllowUnused bool `mapstructure:"allow-unused"`

View File

@ -30,7 +30,7 @@ func NewGodot(settings *config.GodotSettings) *goanalysis.Linter {
// Convert deprecated setting // Convert deprecated setting
// todo(butuzov): remove on v2 release // 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 dotSettings.Scope = godot.AllScope
} }
} }

View File

@ -4,7 +4,7 @@ import (
"fmt" "fmt"
"strconv" "strconv"
"github.com/julz/importas" // nolint: misspell "github.com/julz/importas" //nolint:misspell
"golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis"
"github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/config"
@ -25,7 +25,7 @@ func NewImportAs(settings *config.ImportAsSettings) *goanalysis.Linter {
return return
} }
if len(settings.Alias) == 0 { 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 { if err := analyzer.Flags.Set("no-unaliased", strconv.FormatBool(settings.NoUnaliased)); err != nil {

View File

@ -57,9 +57,6 @@ func runNoLintLint(pass *analysis.Pass, settings *config.NoLintLintSettings) ([]
if settings.RequireExplanation { if settings.RequireExplanation {
needs |= nolintlint.NeedsExplanation needs |= nolintlint.NeedsExplanation
} }
if !settings.AllowLeadingSpace {
needs |= nolintlint.NeedsMachineOnly
}
if settings.RequireSpecific { if settings.RequireSpecific {
needs |= nolintlint.NeedsSpecific needs |= nolintlint.NeedsSpecific
} }

View File

@ -1,6 +1,6 @@
# nolintlint # 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 ) (or any other linter, using th )
## Purpose ## 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: To ensure that lint exceptions have explanations. Consider the case below:
```Go ```Go
import "crypto/md5" //nolint import "crypto/md5" //nolint:all
func hash(data []byte) []byte { 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 `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`.

View File

@ -152,7 +152,7 @@ func NewLinter(needs Needs, excludes []string) (*Linter, error) {
} }
return &Linter{ return &Linter{
needs: needs, needs: needs | NeedsMachineOnly,
excludeByLinter: excludeByName, excludeByLinter: excludeByName,
}, nil }, nil
} }
@ -184,12 +184,14 @@ func (l Linter) Run(fset *token.FileSet, nodes ...ast.Node) ([]Issue, error) {
leadingSpace = leadingSpaceMatches[1] leadingSpace = leadingSpaceMatches[1]
} }
directiveWithOptionalLeadingSpace := comment.Text directiveWithOptionalLeadingSpace := "//"
if len(leadingSpace) > 0 { if len(leadingSpace) > 0 {
split := strings.Split(strings.SplitN(comment.Text, ":", 2)[0], "//") directiveWithOptionalLeadingSpace += " "
directiveWithOptionalLeadingSpace = "// " + strings.TrimSpace(split[1])
} }
split := strings.Split(strings.SplitN(comment.Text, ":", 2)[0], "//")
directiveWithOptionalLeadingSpace += strings.TrimSpace(split[1])
pos := fset.Position(comment.Pos()) pos := fset.Position(comment.Pos())
end := fset.Position(comment.End()) 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] lintersText, explanation := fullMatches[1], fullMatches[2]
var linters []string var linters []string
if len(lintersText) > 0 { if len(lintersText) > 0 && !strings.HasPrefix(lintersText, "all") {
lls := strings.Split(lintersText, ",") lls := strings.Split(lintersText, ",")
linters = make([]string, 0, len(lls)) linters = make([]string, 0, len(lls))
rangeStart := (pos.Column - 1) + len("//") + len(leadingSpace) + len("nolint:") rangeStart := (pos.Column - 1) + len("//") + len(leadingSpace) + len("nolint:")

View File

@ -12,7 +12,7 @@ import (
) )
//nolint:funlen //nolint:funlen
func TestNoLintLint(t *testing.T) { func TestLinter_Run(t *testing.T) {
type issueWithReplacement struct { type issueWithReplacement struct {
issue string issue string
replacement *result.Replacement replacement *result.Replacement
@ -80,20 +80,20 @@ package bar
func foo() { func foo() {
good() //nolint:my-linter good() //nolint:my-linter
bad() //nolint bad() //nolint
bad() // nolint // because bad() //nolint // because
}`, }`,
expected: []issueWithReplacement{ expected: []issueWithReplacement{
{issue: "directive `//nolint` should mention specific linter such as `//nolint:my-linter` at testing.go:6:9"}, {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", desc: "when machine-readable style isn't used",
needs: NeedsMachineOnly,
contents: ` contents: `
package bar package bar
func foo() { func foo() {
bad() // nolint
bad() // nolint bad() // nolint
good() //nolint good() //nolint
}`, }`,
@ -108,25 +108,13 @@ func foo() {
}, },
}, },
}, },
},
},
{ {
desc: "extra spaces in front of directive are reported", issue: "directive `// nolint` should be written without leading space as `//nolint` at testing.go:6:9",
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",
replacement: &result.Replacement{ replacement: &result.Replacement{
Inline: &result.InlineFix{ Inline: &result.InlineFix{
StartCol: 10, StartCol: 10,
Length: 2, Length: 3,
NewString: " ", NewString: "",
}, },
}, },
}, },
@ -138,13 +126,13 @@ func foo() {
package bar package bar
func foo() { func foo() {
good() // nolint:linter1,linter-two good() //nolint:linter1,linter-two
bad() // nolint:linter1 linter2 bad() //nolint:linter1 linter2
good() // nolint: linter1,linter2 good() //nolint: linter1,linter2
good() // nolint: linter1, linter2 good() //nolint: linter1, linter2
}`, }`,
expected: []issueWithReplacement{ 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
}, },
}, },
{ {

View File

@ -98,7 +98,7 @@ func staticCheckConfig(settings *config.StaticCheckSettings) *scconfig.Config {
} }
// https://github.com/dominikh/go-tools/blob/9bf17c0388a65710524ba04c2d821469e639fdc2/lintcmd/lint.go#L437-L477 // 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 { func filterAnalyzerNames(analyzers []string, checks []string) map[string]bool {
allowedChecks := map[string]bool{} allowedChecks := map[string]bool{}

View File

@ -58,7 +58,7 @@ func (r *baseRule) matchLinter(issue *result.Issue) bool {
return false 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()) sourceLine, errSourceLine := lineCache.GetLine(issue.FilePath(), issue.Line())
if errSourceLine != nil { if errSourceLine != nil {
log.Warnf("Failed to get line %s:%d from line cache: %s", issue.FilePath(), issue.Line(), errSourceLine) log.Warnf("Failed to get line %s:%d from line cache: %s", issue.FilePath(), issue.Line(), errSourceLine)

View File

@ -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 return buildRange(nil) // ignore all linters
} }
@ -260,8 +260,12 @@ func (p *Nolint) extractInlineRangeFromComment(text string, g ast.Node, fset *to
var linters []string var linters []string
text = strings.Split(text, "//")[0] // allow another comment after this comment text = strings.Split(text, "//")[0] // allow another comment after this comment
linterItems := strings.Split(strings.TrimPrefix(text, "nolint:"), ",") linterItems := strings.Split(strings.TrimPrefix(text, "nolint:"), ",")
for _, linter := range linterItems { for _, item := range linterItems {
linterName := strings.ToLower(strings.TrimSpace(linter)) linterName := strings.ToLower(strings.TrimSpace(item))
if linterName == "all" {
p.unknownLintersSet = map[string]bool{}
return buildRange(nil)
}
lcs := p.dbManager.GetLinterConfigs(linterName) lcs := p.dbManager.GetLinterConfigs(linterName)
if lcs == nil { if lcs == nil {

View File

@ -4,7 +4,7 @@ package testdata
import "fmt" import "fmt"
// nolint //nolint:all
func PrintString(s string) { func PrintString(s string) {
fmt.Printf("%s", s) fmt.Printf("%s", s)
} }

View File

@ -1,28 +1,28 @@
package testdata package testdata
var nolintSpecific int // nolint:gofmt var nolintSpecific int //nolint:gofmt
var nolintSpace int // nolint: gofmt var nolintSpace int // nolint: gofmt
var nolintSpaces int //nolint: gofmt, govet var nolintSpaces int //nolint:gofmt, govet
var nolintAll int // nolint var nolintAll int // nolint:all
var nolintAndAppendix int // nolint // another comment var nolintAndAppendix int // nolint:all // another comment
//nolint //nolint:all
var nolintVarByPrecedingComment int var nolintVarByPrecedingComment int
//nolint //nolint:all
var dontNolintVarByPrecedingCommentBecauseOfNewLine int var dontNolintVarByPrecedingCommentBecauseOfNewLine int
var nolintPrecedingVar string //nolint var nolintPrecedingVar string //nolint:all
var dontNolintVarByPrecedingCommentBecauseOfDifferentColumn int var dontNolintVarByPrecedingCommentBecauseOfDifferentColumn int
//nolint //nolint:all
func nolintFuncByPrecedingComment() *string { func nolintFuncByPrecedingComment() *string {
xv := "v" xv := "v"
return &xv return &xv
} }
//nolint //nolint:all
// second line // second line
func nolintFuncByPrecedingMultilineComment1() *string { func nolintFuncByPrecedingMultilineComment1() *string {
xv := "v" xv := "v"
@ -30,14 +30,14 @@ func nolintFuncByPrecedingMultilineComment1() *string {
} }
// first line // first line
//nolint //nolint:all
func nolintFuncByPrecedingMultilineComment2() *string { func nolintFuncByPrecedingMultilineComment2() *string {
xv := "v" xv := "v"
return &xv return &xv
} }
// first line // first line
//nolint //nolint:all
// third line // third line
func nolintFuncByPrecedingMultilineComment3() *string { func nolintFuncByPrecedingMultilineComment3() *string {
xv := "v" xv := "v"

View File

@ -1,6 +1,6 @@
package testdata package testdata
var nolintUnknownLinter1 bool // nolint:bad1,deadcode,varcheck,megacheck var nolintUnknownLinter1 bool //nolint:bad1,deadcode,varcheck,megacheck
//nolint:bad2,deadcode,megacheck //nolint:bad2,deadcode,megacheck
func nolintUnknownLinter2() { func nolintUnknownLinter2() {

View File

@ -1,5 +1,5 @@
package testdata package testdata
var nolintVarcheck int // nolint:varcheck var nolintVarcheck int //nolint:varcheck
var nolintVarcheckUnusedOK int // nolint:varcheck,nolintlint var nolintVarcheckUnusedOK int //nolint:varcheck,nolintlint

View File

@ -137,7 +137,7 @@ type latestRelease struct {
} }
func getLatestVersion() (string, error) { func getLatestVersion() (string, error) {
req, err := http.NewRequest( // nolint:noctx req, err := http.NewRequest( //nolint:noctx
http.MethodGet, http.MethodGet,
"https://api.github.com/repos/golangci/golangci-lint/releases/latest", "https://api.github.com/repos/golangci/golangci-lint/releases/latest",
http.NoBody, http.NoBody,

View File

@ -11,10 +11,11 @@ func ExportedFunc1() {
} }
// InvalidFuncComment // ERROR stylecheck `ST1020: comment on exported function ExportedFunc2 should be of the form "ExportedFunc2 ..."` // InvalidFuncComment // ERROR stylecheck `ST1020: comment on exported function ExportedFunc2 should be of the form "ExportedFunc2 ..."`
// nolint:golint //
//nolint:golint
func ExportedFunc2() { func ExportedFunc2() {
} }
// nolint:stylecheck //nolint:stylecheck
func IgnoreAll() { func IgnoreAll() {
} }

View File

@ -10,7 +10,7 @@ func nolintlint() {
fmt.Println() // nolint:bob // leading spaces should be dropped fmt.Println() // nolint:bob // leading spaces should be dropped
// note that the next lines will retain trailing whitespace when fixed // 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:lll // nolint should be dropped
fmt.Println() //nolint:alice,lll // we don't drop individual linters from lists fmt.Println() //nolint:alice,lll // we don't drop individual linters from lists

View File

@ -12,13 +12,13 @@ import (
// Something just some struct // Something just some struct
type Something struct{} type Something struct{}
func aAllowedImport() { // nolint: deadcode,unused func aAllowedImport() { //nolint:deadcode,unused
mfile, _ := modfile.Parse("go.mod", []byte{}, nil) mfile, _ := modfile.Parse("go.mod", []byte{}, nil)
log.Println(mfile) log.Println(mfile)
} }
func aBlockedImport() { // nolint: deadcode,unused func aBlockedImport() { //nolint:deadcode,unused
data := []byte{} data := []byte{}
something := Something{} something := Something{}
_ = yaml.Unmarshal(data, &something) _ = yaml.Unmarshal(data, &something)

View File

@ -6,7 +6,7 @@ package testdata
import "fmt" import "fmt"
func Foo() { func Foo() {
fmt.Println("unused") // nolint // ERROR "directive `//nolint .*` is unused" 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("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("not run") //nolint:unparam // unparam is not run so this is ok
} }

View File

@ -1,3 +1,4 @@
//go:build tools
// +build tools // +build tools
package 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/wiki/Modules#how-can-i-track-tool-dependencies-for-a-module
// https://github.com/golang/go/issues/25922 // https://github.com/golang/go/issues/25922
// //
// nolint //nolint:all
import ( import (
_ "github.com/goreleaser/goreleaser" _ "github.com/goreleaser/goreleaser"
) )