Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from Aug 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .golangci.yml
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions docs/src/docs/usage/false-positives.mdx
Expand Up @@ -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:
Expand All @@ -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 {
// ...
}
Expand Down
2 changes: 0 additions & 2 deletions pkg/config/linters_settings.go
Expand Up @@ -77,7 +77,6 @@ var defaultLintersSettings = LintersSettings{
},
NoLintLint: NoLintLintSettings{
RequireExplanation: false,
AllowLeadingSpace: true,
RequireSpecific: false,
AllowUnused: false,
},
Expand Down Expand Up @@ -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"`
Expand Down
2 changes: 1 addition & 1 deletion pkg/golinters/godot.go
Expand Up @@ -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
}
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/golinters/importas.go
Expand Up @@ -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"
Expand All @@ -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 {
Expand Down
3 changes: 0 additions & 3 deletions pkg/golinters/nolintlint.go
Expand Up @@ -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
}
Expand Down
8 changes: 4 additions & 4 deletions 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
}
```

Expand 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`.

13 changes: 8 additions & 5 deletions pkg/golinters/nolintlint/nolintlint.go
Expand Up @@ -152,7 +152,7 @@ func NewLinter(needs Needs, excludes []string) (*Linter, error) {
}

return &Linter{
needs: needs,
needs: needs | NeedsMachineOnly,
excludeByLinter: excludeByName,
}, nil
}
Expand Down Expand Up @@ -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())

Expand Down Expand Up @@ -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:")
Expand Down
38 changes: 13 additions & 25 deletions pkg/golinters/nolintlint/nolintlint_test.go
Expand Up @@ -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
Expand Down Expand Up @@ -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{
Expand All @@ -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: "",
},
},
},
Expand All @@ -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
},
},
{
Expand Down
2 changes: 1 addition & 1 deletion pkg/golinters/staticcheck_common.go
Expand Up @@ -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{}

Expand Down
2 changes: 1 addition & 1 deletion pkg/result/processors/base_rule.go
Expand Up @@ -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)
Expand Down
10 changes: 7 additions & 3 deletions pkg/result/processors/nolint.go
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion pkg/result/processors/testdata/autogen_exclude_doc.go
Expand Up @@ -4,7 +4,7 @@ package testdata

import "fmt"

// nolint
//nolint:all
func PrintString(s string) {
fmt.Printf("%s", s)
}
22 changes: 11 additions & 11 deletions 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"
Expand Down
2 changes: 1 addition & 1 deletion 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() {
Expand Down
4 changes: 2 additions & 2 deletions 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
2 changes: 1 addition & 1 deletion scripts/expand_website_templates/main.go
Expand Up @@ -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,
Expand Down
5 changes: 3 additions & 2 deletions test/testdata/default_exclude.go
Expand Up @@ -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() {
}
2 changes: 1 addition & 1 deletion test/testdata/fix/in/nolintlint.go
Expand Up @@ -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
Expand Down