Skip to content

Commit

Permalink
feat: drop allow-leading-space and add "all"
Browse files Browse the repository at this point in the history
  • Loading branch information
ldez committed Jul 22, 2022
1 parent a768760 commit d684d83
Show file tree
Hide file tree
Showing 22 changed files with 68 additions and 76 deletions.
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

0 comments on commit d684d83

Please sign in to comment.