diff --git a/.golangci.example.yml b/.golangci.example.yml
index 36f8ed40..d7f89f78 100644
--- a/.golangci.example.yml
+++ b/.golangci.example.yml
@@ -224,6 +224,17 @@ linters-settings:
simple: true
range-loops: true # Report preallocation suggestions on range loops, true by default
for-loops: false # Report preallocation suggestions on for loops, false by default
+ nolintlint:
+ # Enable to ensure that nolint directives are all used. Default is true.
+ allow-unused: false
+ # Disable to ensure that nolint directives don't have a leading space. Default is true.
+ allow-leading-space: true
+ # Exclude following linters from requiring an explanation. Default is [].
+ allow-no-explanation: []
+ # Enable to require an explanation after each nolint directive. Default is false.
+ require-explanation: true
+ # Enable to require an explanation after each nolint directive. Default is false.
+ require-specific: true
rowserrcheck:
packages:
- github.com/jmoiron/sqlx
diff --git a/.golangci.yml b/.golangci.yml
index e5843395..c59782da 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -54,6 +54,11 @@ linters-settings:
suggest-new: true
misspell:
locale: US
+ nolintlint:
+ allow-leading-space: true # don't require machine-readable nolint directives (i.e. with no leading space)
+ 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
linters:
# please, do not use `enable-all`: it's deprecated and will be removed soon.
@@ -84,6 +89,7 @@ linters:
- lll
- misspell
- nakedret
+ - nolintlint
- rowserrcheck
- scopelint
- staticcheck
diff --git a/README.md b/README.md
index 60f42533..2af4e3d4 100644
--- a/README.md
+++ b/README.md
@@ -232,6 +232,7 @@ maligned: Tool to detect Go structs that would take less memory if their fields
misspell: Finds commonly misspelled English words in comments [fast: true, auto-fix: true]
nakedret: Finds naked returns in functions greater than a specified function length [fast: true, auto-fix: false]
nestif: Reports deeply nested if statements [fast: true, auto-fix: false]
+nolintlint: Reports ill-formed or insufficient nolint directives [fast: true, auto-fix: false]
prealloc: Finds slice declarations that could potentially be preallocated [fast: true, auto-fix: false]
rowserrcheck: checks whether Err of rows is checked successfully [fast: true, auto-fix: false]
scopelint: Scopelint checks for unpinned variables in go programs [fast: true, auto-fix: false]
@@ -498,6 +499,7 @@ golangci-lint help linters
- [godot](https://github.com/tetafro/godot) - Check if comments end in a period
- [testpackage](https://github.com/maratori/testpackage) - linter that makes you use a separate _test package
- [nestif](https://github.com/nakabonne/nestif) - Reports deeply nested if statements
+- [nolintlint](https://github.com/golangci-lint/pkg/golinters/nolintlint) - Reports ill-formed or insufficient nolint directives
## Configuration
@@ -845,6 +847,17 @@ linters-settings:
simple: true
range-loops: true # Report preallocation suggestions on range loops, true by default
for-loops: false # Report preallocation suggestions on for loops, false by default
+ nolintlint:
+ # Enable to ensure that nolint directives are all used. Default is true.
+ allow-unused: false
+ # Disable to ensure that nolint directives don't have a leading space. Default is true.
+ allow-leading-space: true
+ # Exclude following linters from requiring an explanation. Default is [].
+ allow-no-explanation: []
+ # Enable to require an explanation after each nolint directive. Default is false.
+ require-explanation: true
+ # Enable to require an explanation after each nolint directive. Default is false.
+ require-specific: true
rowserrcheck:
packages:
- github.com/jmoiron/sqlx
@@ -1034,6 +1047,11 @@ linters-settings:
suggest-new: true
misspell:
locale: US
+ nolintlint:
+ allow-leading-space: true # don't require machine-readable nolint directives (i.e. with no leading space)
+ 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
linters:
# please, do not use `enable-all`: it's deprecated and will be removed soon.
@@ -1064,6 +1082,7 @@ linters:
- lll
- misspell
- nakedret
+ - nolintlint
- rowserrcheck
- scopelint
- staticcheck
@@ -1301,6 +1320,7 @@ Thanks to developers and authors of used linters:
- [tetafro](https://github.com/tetafro)
- [maratori](https://github.com/maratori)
- [nakabonne](https://github.com/nakabonne)
+- [golangci-lint](https://github.com/golangci-lint)
## Changelog
diff --git a/pkg/commands/run.go b/pkg/commands/run.go
index 434b85d5..4149a00c 100644
--- a/pkg/commands/run.go
+++ b/pkg/commands/run.go
@@ -441,7 +441,7 @@ func (e *Executor) executeRun(_ *cobra.Command, args []string) {
// to be removed when deadline is finally decommissioned
func (e *Executor) setTimeoutToDeadlineIfOnlyDeadlineIsSet() {
//lint:ignore SA1019 We want to promoted the deprecated config value when needed
- deadlineValue := e.cfg.Run.Deadline // nolint: staticcheck
+ deadlineValue := e.cfg.Run.Deadline // nolint:staticcheck
if deadlineValue != 0 && e.cfg.Run.Timeout == defaultTimeout {
e.cfg.Run.Timeout = deadlineValue
}
diff --git a/pkg/config/config.go b/pkg/config/config.go
index 1c5290b0..2315f308 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -235,6 +235,7 @@ type LintersSettings struct {
Godot GodotSettings
Testpackage TestpackageSettings
Nestif NestifSettings
+ NoLintLint NoLintLintSettings
Custom map[string]CustomLinterSettings
}
@@ -316,6 +317,14 @@ type GodotSettings struct {
CheckAll bool `mapstructure:"check-all"`
}
+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"`
+}
+
type TestpackageSettings struct {
SkipRegexp string `mapstructure:"skip-regexp"`
}
@@ -324,7 +333,6 @@ type NestifSettings struct {
MinComplexity int `mapstructure:"min-complexity"`
}
-//nolint:gomnd
var defaultLintersSettings = LintersSettings{
Lll: LllSettings{
LineLength: 120,
@@ -363,6 +371,12 @@ var defaultLintersSettings = LintersSettings{
ForceCuddleErrCheckAndAssign: false,
ForceCaseTrailingWhitespaceLimit: 0,
},
+ NoLintLint: NoLintLintSettings{
+ RequireExplanation: false,
+ AllowLeadingSpace: true,
+ RequireSpecific: false,
+ AllowUnused: false,
+ },
Testpackage: TestpackageSettings{
SkipRegexp: `(export|internal)_test\.go`,
},
diff --git a/pkg/golinters/deadcode.go b/pkg/golinters/deadcode.go
index 9889dad4..6ff38909 100644
--- a/pkg/golinters/deadcode.go
+++ b/pkg/golinters/deadcode.go
@@ -28,7 +28,7 @@ func NewDeadcode() *goanalysis.Linter {
}
res := make([]goanalysis.Issue, 0, len(issues))
for _, i := range issues {
- res = append(res, goanalysis.NewIssue(&result.Issue{ //nolint:scopelint
+ res = append(res, goanalysis.NewIssue(&result.Issue{
Pos: i.Pos,
Text: fmt.Sprintf("%s is unused", formatCode(i.UnusedIdentName, nil)),
FromLinter: linterName,
diff --git a/pkg/golinters/depguard.go b/pkg/golinters/depguard.go
index 611f6d49..3bd85481 100644
--- a/pkg/golinters/depguard.go
+++ b/pkg/golinters/depguard.go
@@ -94,7 +94,7 @@ func NewDepguard() *goanalysis.Linter {
if userSuppliedMsgSuffix != "" {
userSuppliedMsgSuffix = ": " + userSuppliedMsgSuffix
}
- res = append(res, goanalysis.NewIssue(&result.Issue{ //nolint:scopelint
+ res = append(res, goanalysis.NewIssue(&result.Issue{
Pos: i.Position,
Text: fmt.Sprintf("%s %s%s", formatCode(i.PackageName, lintCtx.Cfg), msgSuffix, userSuppliedMsgSuffix),
FromLinter: linterName,
diff --git a/pkg/golinters/dupl.go b/pkg/golinters/dupl.go
index d6dc67fb..eaac928e 100644
--- a/pkg/golinters/dupl.go
+++ b/pkg/golinters/dupl.go
@@ -57,7 +57,7 @@ func NewDupl() *goanalysis.Linter {
text := fmt.Sprintf("%d-%d lines are duplicate of %s",
i.From.LineStart(), i.From.LineEnd(),
formatCode(dupl, lintCtx.Cfg))
- res = append(res, goanalysis.NewIssue(&result.Issue{ //nolint:scopelint
+ res = append(res, goanalysis.NewIssue(&result.Issue{
Pos: token.Position{
Filename: i.From.Filename(),
Line: i.From.LineStart(),
diff --git a/pkg/golinters/errcheck.go b/pkg/golinters/errcheck.go
index bf6b9a45..7df11fc8 100644
--- a/pkg/golinters/errcheck.go
+++ b/pkg/golinters/errcheck.go
@@ -59,7 +59,7 @@ func NewErrcheck() *goanalysis.Linter {
} else {
text = "Error return value is not checked"
}
- issues = append(issues, goanalysis.NewIssue(&result.Issue{ //nolint:scopelint
+ issues = append(issues, goanalysis.NewIssue(&result.Issue{
FromLinter: linterName,
Text: text,
Pos: i.Pos,
diff --git a/pkg/golinters/funlen.go b/pkg/golinters/funlen.go
index 3031da48..29cb6b7e 100644
--- a/pkg/golinters/funlen.go
+++ b/pkg/golinters/funlen.go
@@ -42,7 +42,7 @@ func NewFunlen() *goanalysis.Linter {
res := make([]goanalysis.Issue, len(issues))
for k, i := range issues {
- res[k] = goanalysis.NewIssue(&result.Issue{ //nolint:scopelint
+ res[k] = goanalysis.NewIssue(&result.Issue{
Pos: token.Position{
Filename: i.Pos.Filename,
Line: i.Pos.Line,
diff --git a/pkg/golinters/goanalysis/issue.go b/pkg/golinters/goanalysis/issue.go
index b90a2912..f331a3ab 100644
--- a/pkg/golinters/goanalysis/issue.go
+++ b/pkg/golinters/goanalysis/issue.go
@@ -21,9 +21,11 @@ func NewIssue(i *result.Issue, pass *analysis.Pass) Issue {
}
type EncodingIssue struct {
- FromLinter string
- Text string
- Pos token.Position
- LineRange *result.Range
- Replacement *result.Replacement
+ FromLinter string
+ Text string
+ Pos token.Position
+ LineRange *result.Range
+ Replacement *result.Replacement
+ ExpectNoLint bool
+ ExpectedNoLintLinter string
}
diff --git a/pkg/golinters/goanalysis/linter.go b/pkg/golinters/goanalysis/linter.go
index 778a1ef6..f1cfcca8 100644
--- a/pkg/golinters/goanalysis/linter.go
+++ b/pkg/golinters/goanalysis/linter.go
@@ -323,11 +323,13 @@ func saveIssuesToCache(allPkgs []*packages.Package, pkgsFromCache map[*packages.
for ind := range pkgIssues {
i := &pkgIssues[ind]
encodedIssues = append(encodedIssues, EncodingIssue{
- FromLinter: i.FromLinter,
- Text: i.Text,
- Pos: i.Pos,
- LineRange: i.LineRange,
- Replacement: i.Replacement,
+ FromLinter: i.FromLinter,
+ Text: i.Text,
+ Pos: i.Pos,
+ LineRange: i.LineRange,
+ Replacement: i.Replacement,
+ ExpectNoLint: i.ExpectNoLint,
+ ExpectedNoLintLinter: i.ExpectedNoLintLinter,
})
}
@@ -392,12 +394,14 @@ func loadIssuesFromCache(pkgs []*packages.Package, lintCtx *linter.Context,
issues := make([]result.Issue, 0, len(pkgIssues))
for _, i := range pkgIssues {
issues = append(issues, result.Issue{
- FromLinter: i.FromLinter,
- Text: i.Text,
- Pos: i.Pos,
- LineRange: i.LineRange,
- Replacement: i.Replacement,
- Pkg: pkg,
+ FromLinter: i.FromLinter,
+ Text: i.Text,
+ Pos: i.Pos,
+ LineRange: i.LineRange,
+ Replacement: i.Replacement,
+ Pkg: pkg,
+ ExpectNoLint: i.ExpectNoLint,
+ ExpectedNoLintLinter: i.ExpectedNoLintLinter,
})
}
cacheRes.issues = issues
diff --git a/pkg/golinters/goanalysis/runner.go b/pkg/golinters/goanalysis/runner.go
index 2805ed2c..64087c28 100644
--- a/pkg/golinters/goanalysis/runner.go
+++ b/pkg/golinters/goanalysis/runner.go
@@ -312,7 +312,7 @@ func (r *runner) analyze(pkgs []*packages.Package, analyzers []*analysis.Analyze
debugf("There are %d initial and %d total packages", len(initialPkgs), len(loadingPackages))
for _, lp := range loadingPackages {
if lp.isInitial {
- wg.Add(1) //nolint:gomnd
+ wg.Add(1)
go func(lp *loadingPackage) {
lp.analyzeRecursive(r.loadMode, loadSem)
wg.Done()
diff --git a/pkg/golinters/gocognit.go b/pkg/golinters/gocognit.go
index 71301180..8044e3e5 100644
--- a/pkg/golinters/gocognit.go
+++ b/pkg/golinters/gocognit.go
@@ -49,7 +49,7 @@ func NewGocognit() *goanalysis.Linter {
break // Break as the stats is already sorted from greatest to least
}
- res = append(res, goanalysis.NewIssue(&result.Issue{ //nolint:scopelint
+ res = append(res, goanalysis.NewIssue(&result.Issue{
Pos: s.Pos,
Text: fmt.Sprintf("cognitive complexity %d of func %s is high (> %d)",
s.Complexity, formatCode(s.FuncName, lintCtx.Cfg), lintCtx.Settings().Gocognit.MinComplexity),
diff --git a/pkg/golinters/goconst.go b/pkg/golinters/goconst.go
index c77dc64e..8e91feef 100644
--- a/pkg/golinters/goconst.go
+++ b/pkg/golinters/goconst.go
@@ -70,7 +70,7 @@ func checkConstants(pass *analysis.Pass, lintCtx *linter.Context) ([]goanalysis.
} else {
textEnd = fmt.Sprintf(", but such constant %s already exists", formatCode(i.MatchingConst, lintCtx.Cfg))
}
- res = append(res, goanalysis.NewIssue(&result.Issue{ //nolint:scopelint
+ res = append(res, goanalysis.NewIssue(&result.Issue{
Pos: i.Pos,
Text: textBegin + textEnd,
FromLinter: goconstName,
diff --git a/pkg/golinters/gocyclo.go b/pkg/golinters/gocyclo.go
index 4fb28898..f9c0a799 100644
--- a/pkg/golinters/gocyclo.go
+++ b/pkg/golinters/gocyclo.go
@@ -49,7 +49,7 @@ func NewGocyclo() *goanalysis.Linter {
break // Break as the stats is already sorted from greatest to least
}
- res = append(res, goanalysis.NewIssue(&result.Issue{ //nolint:scopelint
+ res = append(res, goanalysis.NewIssue(&result.Issue{
Pos: s.Pos,
Text: fmt.Sprintf("cyclomatic complexity %d of func %s is high (> %d)",
s.Complexity, formatCode(s.FuncName, lintCtx.Cfg), lintCtx.Settings().Gocyclo.MinComplexity),
diff --git a/pkg/golinters/godox.go b/pkg/golinters/godox.go
index 78d13f3b..2a4dd9fa 100644
--- a/pkg/golinters/godox.go
+++ b/pkg/golinters/godox.go
@@ -41,7 +41,7 @@ func NewGodox() *goanalysis.Linter {
res := make([]goanalysis.Issue, len(issues))
for k, i := range issues {
- res[k] = goanalysis.NewIssue(&result.Issue{ //nolint:scopelint
+ res[k] = goanalysis.NewIssue(&result.Issue{
Pos: token.Position{
Filename: i.Pos.Filename,
Line: i.Pos.Line,
diff --git a/pkg/golinters/gofmt_common.go b/pkg/golinters/gofmt_common.go
index ae152ad7..fb1e3f66 100644
--- a/pkg/golinters/gofmt_common.go
+++ b/pkg/golinters/gofmt_common.go
@@ -59,7 +59,6 @@ func (p *hunkChangesParser) parseDiffLines(h *diffpkg.Hunk) {
lineStr := string(line)
- //nolint:gocritic
if strings.HasPrefix(lineStr, "-") {
dl.typ = diffLineDeleted
dl.data = strings.TrimPrefix(lineStr, "-")
diff --git a/pkg/golinters/gomodguard.go b/pkg/golinters/gomodguard.go
index aae78f81..29cc3c30 100644
--- a/pkg/golinters/gomodguard.go
+++ b/pkg/golinters/gomodguard.go
@@ -73,7 +73,7 @@ func NewGomodguard() *goanalysis.Linter {
defer mu.Unlock()
for _, err := range gomodguardErrors {
- issues = append(issues, goanalysis.NewIssue(&result.Issue{ //nolint:scopelint
+ issues = append(issues, goanalysis.NewIssue(&result.Issue{
FromLinter: gomodguardName,
Pos: err.Position,
Text: err.Reason,
diff --git a/pkg/golinters/gosec.go b/pkg/golinters/gosec.go
index 0b84824c..97442958 100644
--- a/pkg/golinters/gosec.go
+++ b/pkg/golinters/gosec.go
@@ -73,7 +73,7 @@ func NewGosec() *goanalysis.Linter {
continue
}
- res = append(res, goanalysis.NewIssue(&result.Issue{ //nolint:scopelint
+ res = append(res, goanalysis.NewIssue(&result.Issue{
Pos: token.Position{
Filename: i.File,
Line: line,
diff --git a/pkg/golinters/ineffassign.go b/pkg/golinters/ineffassign.go
index 7d755809..601252f7 100644
--- a/pkg/golinters/ineffassign.go
+++ b/pkg/golinters/ineffassign.go
@@ -42,7 +42,7 @@ func NewIneffassign() *goanalysis.Linter {
res := make([]goanalysis.Issue, 0, len(issues))
for _, i := range issues {
- res = append(res, goanalysis.NewIssue(&result.Issue{ //nolint:scopelint
+ res = append(res, goanalysis.NewIssue(&result.Issue{
Pos: i.Pos,
Text: fmt.Sprintf("ineffectual assignment to %s", formatCode(i.IdentName, lintCtx.Cfg)),
FromLinter: ineffassignName,
diff --git a/pkg/golinters/interfacer.go b/pkg/golinters/interfacer.go
index 96f6b5b9..1edbe894 100644
--- a/pkg/golinters/interfacer.go
+++ b/pkg/golinters/interfacer.go
@@ -48,7 +48,7 @@ func NewInterfacer() *goanalysis.Linter {
res := make([]goanalysis.Issue, 0, len(issues))
for _, i := range issues {
pos := pass.Fset.Position(i.Pos())
- res = append(res, goanalysis.NewIssue(&result.Issue{ //nolint:scopelint
+ res = append(res, goanalysis.NewIssue(&result.Issue{
Pos: pos,
Text: i.Message(),
FromLinter: interfacerName,
diff --git a/pkg/golinters/maligned.go b/pkg/golinters/maligned.go
index 4f34f0ea..22422b8c 100644
--- a/pkg/golinters/maligned.go
+++ b/pkg/golinters/maligned.go
@@ -40,7 +40,7 @@ func NewMaligned() *goanalysis.Linter {
if lintCtx.Settings().Maligned.SuggestNewOrder {
text += fmt.Sprintf(":\n%s", formatCodeBlock(i.NewStructDef, lintCtx.Cfg))
}
- issues = append(issues, goanalysis.NewIssue(&result.Issue{ //nolint:scopelint
+ issues = append(issues, goanalysis.NewIssue(&result.Issue{
Pos: i.Pos,
Text: text,
FromLinter: linterName,
diff --git a/pkg/golinters/nestif.go b/pkg/golinters/nestif.go
index 841a22d4..0998a8ce 100644
--- a/pkg/golinters/nestif.go
+++ b/pkg/golinters/nestif.go
@@ -46,7 +46,7 @@ func NewNestif() *goanalysis.Linter {
res := make([]goanalysis.Issue, 0, len(issues))
for _, i := range issues {
- res = append(res, goanalysis.NewIssue(&result.Issue{ //nolint:scopelint
+ res = append(res, goanalysis.NewIssue(&result.Issue{
Pos: i.Pos,
Text: i.Message,
FromLinter: nestifName,
diff --git a/pkg/golinters/nolintlint.go b/pkg/golinters/nolintlint.go
new file mode 100644
index 00000000..d98b50cf
--- /dev/null
+++ b/pkg/golinters/nolintlint.go
@@ -0,0 +1,92 @@
+package golinters
+
+import (
+ "fmt"
+ "go/ast"
+ "sync"
+
+ "golang.org/x/tools/go/analysis"
+
+ "github.com/golangci/golangci-lint/pkg/golinters/goanalysis"
+ "github.com/golangci/golangci-lint/pkg/golinters/nolintlint"
+ "github.com/golangci/golangci-lint/pkg/lint/linter"
+ "github.com/golangci/golangci-lint/pkg/result"
+)
+
+const NolintlintName = "nolintlint"
+
+func NewNoLintLint() *goanalysis.Linter {
+ var mu sync.Mutex
+ var resIssues []goanalysis.Issue
+
+ analyzer := &analysis.Analyzer{
+ Name: NolintlintName,
+ Doc: goanalysis.TheOnlyanalyzerDoc,
+ }
+ return goanalysis.NewLinter(
+ NolintlintName,
+ "Reports ill-formed or insufficient nolint directives",
+ []*analysis.Analyzer{analyzer},
+ nil,
+ ).WithContextSetter(func(lintCtx *linter.Context) {
+ analyzer.Run = func(pass *analysis.Pass) (interface{}, error) {
+ var needs nolintlint.Needs
+ settings := lintCtx.Settings().NoLintLint
+ if settings.RequireExplanation {
+ needs |= nolintlint.NeedsExplanation
+ }
+ if !settings.AllowLeadingSpace {
+ needs |= nolintlint.NeedsMachineOnly
+ }
+ if settings.RequireSpecific {
+ needs |= nolintlint.NeedsSpecific
+ }
+ if !settings.AllowUnused {
+ needs |= nolintlint.NeedsUnused
+ }
+
+ lnt, err := nolintlint.NewLinter(needs, settings.AllowNoExplanation)
+ if err != nil {
+ return nil, err
+ }
+
+ nodes := make([]ast.Node, 0, len(pass.Files))
+ for _, n := range pass.Files {
+ nodes = append(nodes, n)
+ }
+ issues, err := lnt.Run(pass.Fset, nodes...)
+ if err != nil {
+ return nil, fmt.Errorf("linter failed to run: %s", err)
+ }
+ var res []goanalysis.Issue
+ for _, i := range issues {
+ expectNoLint := false
+ var expectedNolintLinter string
+ if ii, ok := i.(nolintlint.UnusedCandidate); ok {
+ expectedNolintLinter = ii.ExpectedLinter
+ expectNoLint = true
+ }
+ issue := &result.Issue{
+ FromLinter: NolintlintName,
+ Text: i.Details(),
+ Pos: i.Position(),
+ ExpectNoLint: expectNoLint,
+ ExpectedNoLintLinter: expectedNolintLinter,
+ }
+ res = append(res, goanalysis.NewIssue(issue, pass))
+ }
+
+ if len(res) == 0 {
+ return nil, nil
+ }
+
+ mu.Lock()
+ resIssues = append(resIssues, res...)
+ mu.Unlock()
+
+ return nil, nil
+ }
+ }).WithIssuesReporter(func(*linter.Context) []goanalysis.Issue {
+ return resIssues
+ }).WithLoadMode(goanalysis.LoadModeSyntax)
+}
diff --git a/pkg/golinters/nolintlint/README.md b/pkg/golinters/nolintlint/README.md
new file mode 100644
index 00000000..3d440d5a
--- /dev/null
+++ b/pkg/golinters/nolintlint/README.md
@@ -0,0 +1,31 @@
+# nolintlint
+
+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
+
+func hash(data []byte) []byte {
+ return md5.New().Sum(data) //nolint
+}
+```
+
+In the above case, nolint directives are present but the user has no idea why this is being done or which linter
+is being suppressed (in this case, gosec recommends against use of md5). `nolintlint` can require that the code provide an explanation, which might look as follows:
+
+```Go
+import "crypto/md5" //nolint:gosec // this is not used in a secure application
+
+func hash(data []byte) []byte {
+ return md5.New().Sum(data) //nolint:gosec // this result is not used in a secure application
+}
+```
+
+`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`.
+
diff --git a/pkg/golinters/nolintlint/nolintlint.go b/pkg/golinters/nolintlint/nolintlint.go
new file mode 100644
index 00000000..6da31c60
--- /dev/null
+++ b/pkg/golinters/nolintlint/nolintlint.go
@@ -0,0 +1,239 @@
+// nolintlint provides a linter to ensure that all //nolint directives are followed by explanations
+package nolintlint
+
+import (
+ "fmt"
+ "go/ast"
+ "go/token"
+ "regexp"
+ "strings"
+ "unicode"
+)
+
+type BaseIssue struct {
+ fullDirective string
+ directiveWithOptionalLeadingSpace string
+ position token.Position
+}
+
+func (b BaseIssue) Position() token.Position {
+ return b.position
+}
+
+type ExtraLeadingSpace struct {
+ BaseIssue
+}
+
+func (i ExtraLeadingSpace) Details() string {
+ return fmt.Sprintf("directive `%s` should not have more than one leading space", i.fullDirective)
+}
+
+func (i ExtraLeadingSpace) String() string { return toString(i) }
+
+type NotMachine struct {
+ BaseIssue
+}
+
+func (i NotMachine) Details() string {
+ expected := i.fullDirective[:2] + strings.TrimLeftFunc(i.fullDirective[2:], unicode.IsSpace)
+ return fmt.Sprintf("directive `%s` should be written without leading space as `%s`",
+ i.fullDirective, expected)
+}
+
+func (i NotMachine) String() string { return toString(i) }
+
+type NotSpecific struct {
+ BaseIssue
+}
+
+func (i NotSpecific) Details() string {
+ return fmt.Sprintf("directive `%s` should mention specific linter such as `//%s:my-linter`",
+ i.fullDirective, i.directiveWithOptionalLeadingSpace)
+}
+
+func (i NotSpecific) String() string { return toString(i) }
+
+type ParseError struct {
+ BaseIssue
+}
+
+func (i ParseError) Details() string {
+ return fmt.Sprintf("directive `%s` should match `//%s[:<comma-separated-linters>] [// <explanation>]`",
+ i.fullDirective,
+ i.directiveWithOptionalLeadingSpace)
+}
+
+func (i ParseError) String() string { return toString(i) }
+
+type NoExplanation struct {
+ BaseIssue
+ fullDirectiveWithoutExplanation string
+}
+
+func (i NoExplanation) Details() string {
+ return fmt.Sprintf("directive `%s` should provide explanation such as `%s // this is why`",
+ i.fullDirective, i.fullDirectiveWithoutExplanation)
+}
+
+func (i NoExplanation) String() string { return toString(i) }
+
+type UnusedCandidate struct {
+ BaseIssue
+ ExpectedLinter string
+}
+
+func (i UnusedCandidate) Details() string {
+ details := fmt.Sprintf("directive `%s` is unused", i.fullDirective)
+ if i.ExpectedLinter != "" {
+ details += fmt.Sprintf(" for linter %s", i.ExpectedLinter)
+ }
+ return details
+}
+
+func (i UnusedCandidate) String() string { return toString(i) }
+
+func toString(i Issue) string {
+ return fmt.Sprintf("%s at %s", i.Details(), i.Position())
+}
+
+type Issue interface {
+ Details() string
+ Position() token.Position
+ String() string
+}
+
+type Needs uint
+
+const (
+ NeedsMachineOnly Needs = 1 << iota
+ NeedsSpecific
+ NeedsExplanation
+ NeedsUnused
+ NeedsAll = NeedsMachineOnly | NeedsSpecific | NeedsExplanation
+)
+
+// matches lines starting with the nolint directive
+var directiveOnlyPattern = regexp.MustCompile(`^\s*(nolint)(:\s*[\w-]+\s*(?:,\s*[\w-]+\s*)*)?\b`)
+
+// matches a complete nolint directive
+var fullDirectivePattern = regexp.MustCompile(`^//\s*nolint(:\s*[\w-]+\s*(?:,\s*[\w-]+\s*)*)?\s*(//.*)?\s*\n?$`)
+
+type Linter struct {
+ excludes []string // lists individual linters that don't require explanations
+ needs Needs // indicates which linter checks to perform
+ excludeByLinter map[string]bool
+}
+
+// NewLinter creates a linter that enforces that the provided directives fulfill the provided requirements
+func NewLinter(needs Needs, excludes []string) (*Linter, error) {
+ excludeByName := make(map[string]bool)
+ for _, e := range excludes {
+ excludeByName[e] = true
+ }
+
+ return &Linter{
+ needs: needs,
+ excludeByLinter: excludeByName,
+ }, nil
+}
+
+var leadingSpacePattern = regexp.MustCompile(`^//(\s*)`)
+var trailingBlankExplanation = regexp.MustCompile(`\s*(//\s*)?$`)
+
+func (l Linter) Run(fset *token.FileSet, nodes ...ast.Node) ([]Issue, error) {
+ var issues []Issue
+ for _, node := range nodes {
+ if file, ok := node.(*ast.File); ok {
+ for _, c := range file.Comments {
+ text := c.Text()
+ matches := directiveOnlyPattern.FindStringSubmatch(text)
+ if len(matches) == 0 {
+ continue
+ }
+ directive := matches[1]
+
+ // check for a space between the "//" and the directive
+ leadingSpaceMatches := leadingSpacePattern.FindStringSubmatch(c.List[0].Text) // c.Text() doesn't have all leading space
+ if len(leadingSpaceMatches) == 0 {
+ continue
+ }
+ leadingSpace := leadingSpaceMatches[1]
+
+ directiveWithOptionalLeadingSpace := directive
+ if len(leadingSpace) > 0 {
+ directiveWithOptionalLeadingSpace = " " + directive
+ }
+
+ base := BaseIssue{
+ fullDirective: c.List[0].Text,
+ directiveWithOptionalLeadingSpace: directiveWithOptionalLeadingSpace,
+ position: fset.Position(c.Pos()),
+ }
+
+ // check for, report and eliminate leading spaces so we can check for other issues
+ if leadingSpace != "" && leadingSpace != " " {
+ issues = append(issues, ExtraLeadingSpace{
+ BaseIssue: base,
+ })
+ }
+
+ if (l.needs&NeedsMachineOnly) != 0 && strings.HasPrefix(directiveWithOptionalLeadingSpace, " ") {
+ issues = append(issues, NotMachine{BaseIssue: base})
+ }
+
+ fullMatches := fullDirectivePattern.FindStringSubmatch(c.List[0].Text)
+ if len(fullMatches) == 0 {
+ issues = append(issues, ParseError{BaseIssue: base})
+ continue
+ }
+ lintersText, explanation := fullMatches[1], fullMatches[2]
+ var linters []string
+ if len(lintersText) > 0 {
+ lls := strings.Split(lintersText[1:], ",")
+ linters = make([]string, 0, len(lls))
+ for _, ll := range lls {
+ ll = strings.TrimSpace(ll)
+ if ll != "" {
+ linters = append(linters, ll)
+ }
+ }
+ }
+ if (l.needs & NeedsSpecific) != 0 {
+ if len(linters) == 0 {
+ issues = append(issues, NotSpecific{BaseIssue: base})
+ }
+ }
+
+ // when detecting unused directives, we send all the directives through and filter them out in the nolint processor
+ if l.needs&NeedsUnused != 0 {
+ if len(linters) == 0 {
+ issues = append(issues, UnusedCandidate{BaseIssue: base})
+ } else {
+ for _, linter := range linters {
+ issues = append(issues, UnusedCandidate{BaseIssue: base, ExpectedLinter: linter})
+ }
+ }
+ }
+
+ if (l.needs&NeedsExplanation) != 0 && (explanation == "" || strings.TrimSpace(explanation) == "//") {
+ needsExplanation := len(linters) == 0 // if no linters are mentioned, we must have explanation
+ // otherwise, check if we are excluding all of the mentioned linters
+ for _, ll := range linters {
+ if !l.excludeByLinter[ll] { // if a linter does require explanation
+ needsExplanation = true
+ break
+ }
+ }
+ if needsExplanation {
+ fullDirectiveWithoutExplanation := trailingBlankExplanation.ReplaceAllString(c.List[0].Text, "")
+ issues = append(issues, NoExplanation{
+ BaseIssue: base,
+ fullDirectiveWithoutExplanation: fullDirectiveWithoutExplanation,
+ })
+ }
+ }
+ }
+ }
+ }
+ return issues, nil
+}
diff --git a/pkg/golinters/nolintlint/nolintlint_test.go b/pkg/golinters/nolintlint/nolintlint_test.go
new file mode 100644
index 00000000..4eb72e8d
--- /dev/null
+++ b/pkg/golinters/nolintlint/nolintlint_test.go
@@ -0,0 +1,123 @@
+package nolintlint
+
+import (
+ "go/parser"
+ "go/token"
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+)
+
+func TestNoLintLint(t *testing.T) {
+ t.Run("when no explanation is provided", func(t *testing.T) {
+ linter, _ := NewLinter(NeedsExplanation, nil)
+ expectIssues(t, linter, `
+package bar
+
+func foo() {
+ bad() //nolint
+ bad() //nolint //
+ bad() //nolint //
+ good() //nolint // this is ok
+ other() //nolintother
+}`,
+ "directive `//nolint` should provide explanation such as `//nolint // this is why` at testing.go:5:9",
+ "directive `//nolint //` should provide explanation such as `//nolint // this is why` at testing.go:6:9",
+ "directive `//nolint // ` should provide explanation such as `//nolint // this is why` at testing.go:7:9",
+ )
+ })
+
+ t.Run("when no explanation is needed for a specific linter", func(t *testing.T) {
+ linter, _ := NewLinter(NeedsExplanation, []string{"lll"})
+ expectIssues(t, linter, `
+package bar
+
+func foo() {
+ thisIsAReallyLongLine() //nolint:lll
+}`)
+ })
+
+ t.Run("when no specific linter is mentioned", func(t *testing.T) {
+ linter, _ := NewLinter(NeedsSpecific, nil)
+ expectIssues(t, linter, `
+package bar
+
+func foo() {
+ good() //nolint:my-linter
+ bad() //nolint
+ bad() // nolint // because
+}`,
+ "directive `//nolint` should mention specific linter such as `//nolint:my-linter` at testing.go:6:9",
+ "directive `// nolint // because` should mention specific linter such as `// nolint:my-linter` at testing.go:7:9")
+ })
+
+ t.Run("when machine-readable style isn't used", func(t *testing.T) {
+ linter, _ := NewLinter(NeedsMachineOnly, nil)
+ expectIssues(t, linter, `
+package bar
+
+func foo() {
+ bad() // nolint
+ good() //nolint
+}`, "directive `// nolint` should be written without leading space as `//nolint` at testing.go:5:9")
+ })
+
+ t.Run("extra spaces in front of directive are reported", func(t *testing.T) {
+ linter, _ := NewLinter(0, nil)
+ expectIssues(t, linter, `
+package bar
+
+func foo() {
+ bad() // nolint
+ good() // nolint
+}`, "directive `// nolint` should not have more than one leading space at testing.go:5:9")
+ })
+
+ t.Run("spaces are allowed in comma-separated list of linters", func(t *testing.T) {
+ linter, _ := NewLinter(0, nil)
+ expectIssues(t, linter, `
+package bar
+
+func foo() {
+ good() // nolint:linter1,linter-two
+ bad() // nolint:linter1 linter2
+ good() // nolint: linter1,linter2
+ good() // nolint: linter1, linter2
+}`,
+ "directive `// nolint:linter1 linter2` should match `// nolint[:<comma-separated-linters>] [// <explanation>]` at testing.go:6:9", //nolint:lll // this is a string
+ )
+ })
+
+ t.Run("multi-line comments don't confuse parser", func(t *testing.T) {
+ linter, _ := NewLinter(0, nil)
+ expectIssues(t, linter, `
+package bar
+
+func foo() {
+ //nolint:test
+ // something else
+}`)
+ })
+}
+
+func expectIssues(t *testing.T, linter *Linter, contents string, issues ...string) {
+ actualIssues := parseFile(t, linter, contents)
+ actualIssueStrs := make([]string, 0, len(actualIssues))
+ for _, i := range actualIssues {
+ actualIssueStrs = append(actualIssueStrs, i.String())
+ }
+ assert.ElementsMatch(t, issues, actualIssueStrs, "expected %s but got %s", issues, actualIssues)
+}
+
+func parseFile(t *testing.T, linter *Linter, contents string) []Issue {
+ fset := token.NewFileSet()
+ expr, err := parser.ParseFile(fset, "testing.go", contents, parser.ParseComments)
+ if err != nil {
+ t.Fatalf("unable to parse file contents: %s", err)
+ }
+ issues, err := linter.Run(fset, expr)
+ if err != nil {
+ t.Fatalf("unable to parse file: %s", err)
+ }
+ return issues
+}
diff --git a/pkg/golinters/prealloc.go b/pkg/golinters/prealloc.go
index 168dc171..534a6925 100644
--- a/pkg/golinters/prealloc.go
+++ b/pkg/golinters/prealloc.go
@@ -34,7 +34,7 @@ func NewPrealloc() *goanalysis.Linter {
var res []goanalysis.Issue
hints := prealloc.Check(pass.Files, s.Simple, s.RangeLoops, s.ForLoops)
for _, hint := range hints {
- res = append(res, goanalysis.NewIssue(&result.Issue{ //nolint:scopelint
+ res = append(res, goanalysis.NewIssue(&result.Issue{
Pos: pass.Fset.Position(hint.Pos),
Text: fmt.Sprintf("Consider preallocating %s", formatCode(hint.DeclaredSliceName, lintCtx.Cfg)),
FromLinter: preallocName,
diff --git a/pkg/golinters/structcheck.go b/pkg/golinters/structcheck.go
index 8acfc403..7c16f8ec 100644
--- a/pkg/golinters/structcheck.go
+++ b/pkg/golinters/structcheck.go
@@ -37,7 +37,7 @@ func NewStructcheck() *goanalysis.Linter {
issues := make([]goanalysis.Issue, 0, len(structcheckIssues))
for _, i := range structcheckIssues {
- issues = append(issues, goanalysis.NewIssue(&result.Issue{ //nolint:scopelint
+ issues = append(issues, goanalysis.NewIssue(&result.Issue{
Pos: i.Pos,
Text: fmt.Sprintf("%s is unused", formatCode(i.FieldName, lintCtx.Cfg)),
FromLinter: linterName,
diff --git a/pkg/golinters/unconvert.go b/pkg/golinters/unconvert.go
index 14757017..456f6836 100644
--- a/pkg/golinters/unconvert.go
+++ b/pkg/golinters/unconvert.go
@@ -35,7 +35,7 @@ func NewUnconvert() *goanalysis.Linter {
issues := make([]goanalysis.Issue, 0, len(positions))
for _, pos := range positions {
- issues = append(issues, goanalysis.NewIssue(&result.Issue{ //nolint:scopelint
+ issues = append(issues, goanalysis.NewIssue(&result.Issue{
Pos: pos,
Text: "unnecessary conversion",
FromLinter: linterName,
diff --git a/pkg/golinters/unparam.go b/pkg/golinters/unparam.go
index 866d0663..fabb6b52 100644
--- a/pkg/golinters/unparam.go
+++ b/pkg/golinters/unparam.go
@@ -58,7 +58,7 @@ func NewUnparam() *goanalysis.Linter {
var res []goanalysis.Issue
for _, i := range unparamIssues {
- res = append(res, goanalysis.NewIssue(&result.Issue{ //nolint:scopelint
+ res = append(res, goanalysis.NewIssue(&result.Issue{
Pos: pass.Fset.Position(i.Pos()),
Text: i.Message(),
FromLinter: linterName,
diff --git a/pkg/golinters/varcheck.go b/pkg/golinters/varcheck.go
index 3c650d8c..dcf2e7de 100644
--- a/pkg/golinters/varcheck.go
+++ b/pkg/golinters/varcheck.go
@@ -37,7 +37,7 @@ func NewVarcheck() *goanalysis.Linter {
issues := make([]goanalysis.Issue, 0, len(varcheckIssues))
for _, i := range varcheckIssues {
- issues = append(issues, goanalysis.NewIssue(&result.Issue{ //nolint:scopelint
+ issues = append(issues, goanalysis.NewIssue(&result.Issue{
Pos: i.Pos,
Text: fmt.Sprintf("%s is unused", formatCode(i.VarName, lintCtx.Cfg)),
FromLinter: linterName,
diff --git a/pkg/golinters/whitespace.go b/pkg/golinters/whitespace.go
index 4a2ccce5..d475465a 100644
--- a/pkg/golinters/whitespace.go
+++ b/pkg/golinters/whitespace.go
@@ -70,7 +70,7 @@ func NewWhitespace() *goanalysis.Linter {
}
issue.Replacement.NewLines = []string{bracketLine}
- res[k] = goanalysis.NewIssue(&issue, pass) //nolint:scopelint
+ res[k] = goanalysis.NewIssue(&issue, pass)
}
mu.Lock()
diff --git a/pkg/golinters/wsl.go b/pkg/golinters/wsl.go
index 32fb00c4..f44d09f2 100644
--- a/pkg/golinters/wsl.go
+++ b/pkg/golinters/wsl.go
@@ -66,7 +66,7 @@ func NewWSL() *goanalysis.Linter {
defer mu.Unlock()
for _, err := range wslErrors {
- issues = append(issues, goanalysis.NewIssue(&result.Issue{ //nolint:scopelint
+ issues = append(issues, goanalysis.NewIssue(&result.Issue{
FromLinter: name,
Pos: err.Position,
Text: err.Reason,
diff --git a/pkg/lint/lintersdb/enabled_set.go b/pkg/lint/lintersdb/enabled_set.go
index a3b73e70..73e3bcfe 100644
--- a/pkg/lint/lintersdb/enabled_set.go
+++ b/pkg/lint/lintersdb/enabled_set.go
@@ -113,7 +113,7 @@ func (es EnabledSet) combineGoAnalysisLinters(linters map[string]*linter.Config)
}
}
- if len(goanalysisLinters) <= 1 { //nolint:gomnd
+ if len(goanalysisLinters) <= 1 {
es.debugf("Didn't combine go/analysis linters: got only %d linters", len(goanalysisLinters))
return
}
diff --git a/pkg/lint/lintersdb/manager.go b/pkg/lint/lintersdb/manager.go
index 030de788..492b9225 100644
--- a/pkg/lint/lintersdb/manager.go
+++ b/pkg/lint/lintersdb/manager.go
@@ -269,6 +269,10 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config {
linter.NewConfig(golinters.NewNestif()).
WithPresets(linter.PresetComplexity).
WithURL("https://github.com/nakabonne/nestif"),
+ // nolintlint must be last because it looks at the results of all the previous linters for unused nolint directives
+ linter.NewConfig(golinters.NewNoLintLint()).
+ WithPresets(linter.PresetStyle).
+ WithURL("https://github.com/golangci-lint/pkg/golinters/nolintlint"),
}
isLocalRun := os.Getenv("GOLANGCI_COM_RUN") == ""
diff --git a/pkg/lint/runner.go b/pkg/lint/runner.go
index 22859afc..e8f427e0 100644
--- a/pkg/lint/runner.go
+++ b/pkg/lint/runner.go
@@ -77,6 +77,13 @@ func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env,
excludeRulesProcessor = processors.NewExcludeRules(excludeRules, lineCache, log.Child("exclude_rules"))
}
+ enabledLintersSet := lintersdb.NewEnabledSet(dbManager,
+ lintersdb.NewValidator(dbManager), log.Child("enabledLinters"), cfg)
+ lcs, err := enabledLintersSet.Get(false)
+ if err != nil {
+ return nil, err
+ }
+
return &Runner{
Processors: []processors.Processor{
processors.NewCgo(goenv),
@@ -96,7 +103,7 @@ func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env,
excludeProcessor,
excludeRulesProcessor,
- processors.NewNolint(log.Child("nolint"), dbManager),
+ processors.NewNolint(log.Child("nolint"), dbManager, lcs),
processors.NewUniqByLine(cfg),
processors.NewDiff(icfg.Diff, icfg.DiffFromRevision, icfg.DiffPatchFilePath),
diff --git a/pkg/result/issue.go b/pkg/result/issue.go
index 83ba705e..16d9a8a8 100644
--- a/pkg/result/issue.go
+++ b/pkg/result/issue.go
@@ -41,6 +41,10 @@ type Issue struct {
// HunkPos is used only when golangci-lint is run over a diff
HunkPos int `json:",omitempty"`
+
+ // If we are expecting a nolint (because this is from nolintlint), record the expected linter
+ ExpectNoLint bool
+ ExpectedNoLintLinter string
}
func (i *Issue) FilePath() string {
diff --git a/pkg/result/processors/autogenerated_exclude.go b/pkg/result/processors/autogenerated_exclude.go
index 8bdaae66..249ba9d4 100644
--- a/pkg/result/processors/autogenerated_exclude.go
+++ b/pkg/result/processors/autogenerated_exclude.go
@@ -132,7 +132,7 @@ func getDoc(filePath string) (string, error) {
var docLines []string
for scanner.Scan() {
line := strings.TrimSpace(scanner.Text())
- if strings.HasPrefix(line, "//") { //nolint:gocritic
+ if strings.HasPrefix(line, "//") {
text := strings.TrimSpace(strings.TrimPrefix(line, "//"))
docLines = append(docLines, text)
} else if line == "" || strings.HasPrefix(line, "package") {
diff --git a/pkg/result/processors/fixer.go b/pkg/result/processors/fixer.go
index 29313d7f..75fdc93f 100644
--- a/pkg/result/processors/fixer.go
+++ b/pkg/result/processors/fixer.go
@@ -185,7 +185,7 @@ func (f Fixer) applyInlineFixes(lineIssues []result.Issue, origLine []byte, line
func (f Fixer) findNotIntersectingIssues(issues []result.Issue) []result.Issue {
sort.SliceStable(issues, func(i, j int) bool {
- a, b := issues[i], issues[j] //nolint:scopelint
+ a, b := issues[i], issues[j]
return a.Line() < b.Line()
})
diff --git a/pkg/result/processors/max_per_file_from_linter.go b/pkg/result/processors/max_per_file_from_linter.go
index e9842966..e83c569e 100644
--- a/pkg/result/processors/max_per_file_from_linter.go
+++ b/pkg/result/processors/max_per_file_from_linter.go
@@ -27,7 +27,7 @@ func NewMaxPerFileFromLinter(cfg *config.Config) *MaxPerFileFromLinter {
}
return &MaxPerFileFromLinter{
- flc: fileToLinterToCountMap{}, //nolint:goimports,gofmt
+ flc: fileToLinterToCountMap{},
maxPerFileFromLinterConfig: maxPerFileFromLinterConfig,
}
}
diff --git a/pkg/result/processors/nolint.go b/pkg/result/processors/nolint.go
index da7abacd..2f1e48db 100644
--- a/pkg/result/processors/nolint.go
+++ b/pkg/result/processors/nolint.go
@@ -8,6 +8,8 @@ import (
"sort"
"strings"
+ "github.com/golangci/golangci-lint/pkg/golinters"
+ "github.com/golangci/golangci-lint/pkg/lint/linter"
"github.com/golangci/golangci-lint/pkg/lint/lintersdb"
"github.com/golangci/golangci-lint/pkg/logutils"
"github.com/golangci/golangci-lint/pkg/result"
@@ -16,7 +18,8 @@ import (
var nolintDebugf = logutils.Debug("nolint")
type ignoredRange struct {
- linters []string
+ linters []string
+ matchedIssueFromLinter map[string]bool
result.Range
col int
}
@@ -26,6 +29,15 @@ func (i *ignoredRange) doesMatch(issue *result.Issue) bool {
return false
}
+ // handle possible unused nolint directives
+ // nolintlint generates potential issues for every nolint directive and they are filtered out here
+ if issue.ExpectNoLint {
+ if issue.ExpectedNoLintLinter != "" {
+ return i.matchedIssueFromLinter[issue.ExpectedNoLintLinter]
+ }
+ return len(i.matchedIssueFromLinter) > 0
+ }
+
if len(i.linters) == 0 {
return true
}
@@ -46,17 +58,24 @@ type fileData struct {
type filesCache map[string]*fileData
type Nolint struct {
- cache filesCache
- dbManager *lintersdb.Manager
- log logutils.Log
+ cache filesCache
+ dbManager *lintersdb.Manager
+ enabledLinters map[string]bool
+ log logutils.Log
unknownLintersSet map[string]bool
}
-func NewNolint(log logutils.Log, dbManager *lintersdb.Manager) *Nolint {
+func NewNolint(log logutils.Log, dbManager *lintersdb.Manager, enabledLCs []*linter.Config) *Nolint {
+ enabledLinters := make(map[string]bool, len(enabledLCs))
+ for _, lc := range enabledLCs {
+ enabledLinters[lc.Name()] = true
+ }
+
return &Nolint{
cache: filesCache{},
dbManager: dbManager,
+ enabledLinters: enabledLinters,
log: log,
unknownLintersSet: map[string]bool{},
}
@@ -69,6 +88,8 @@ func (p Nolint) Name() string {
}
func (p *Nolint) Process(issues []result.Issue) ([]result.Issue, error) {
+ // put nolintlint issues last because we process other issues first to determine which nolint directives are unused
+ sort.Stable(sortWithNolintlintLast(issues))
return filterIssuesErr(issues, p.shouldPassIssue)
}
@@ -124,6 +145,22 @@ func (p *Nolint) buildIgnoredRangesForFile(f *ast.File, fset *token.FileSet, fil
}
func (p *Nolint) shouldPassIssue(i *result.Issue) (bool, error) {
+ nolintDebugf("got issue: %v", *i)
+ if i.FromLinter == golinters.NolintlintName {
+ // always pass nolintlint issues except ones trying find unused nolint directives
+ if !i.ExpectNoLint {
+ return true, nil
+ }
+ if i.ExpectedNoLintLinter != "" {
+ // don't expect disabled linters to cover their nolint statements
+ nolintDebugf("enabled linters: %v", p.enabledLinters)
+ if !p.enabledLinters[i.ExpectedNoLintLinter] {
+ return false, nil
+ }
+ nolintDebugf("checking that lint issue was used for %s: %v", i.ExpectedNoLintLinter, i)
+ }
+ }
+
fd, err := p.getOrCreateFileData(i)
if err != nil {
return false, err
@@ -131,6 +168,7 @@ func (p *Nolint) shouldPassIssue(i *result.Issue) (bool, error) {
for _, ir := range fd.ignoredRanges {
if ir.doesMatch(i) {
+ ir.matchedIssueFromLinter[i.FromLinter] = true
return false, nil
}
}
@@ -203,8 +241,9 @@ func (p *Nolint) extractInlineRangeFromComment(text string, g ast.Node, fset *to
From: pos.Line,
To: fset.Position(g.End()).Line,
},
- col: pos.Column,
- linters: linters,
+ col: pos.Column,
+ linters: linters,
+ matchedIssueFromLinter: make(map[string]bool),
}
}
@@ -253,3 +292,18 @@ func (p Nolint) Finish() {
p.log.Warnf("Found unknown linters in //nolint directives: %s", strings.Join(unknownLinters, ", "))
}
+
+// put nolintlint last
+type sortWithNolintlintLast []result.Issue
+
+func (issues sortWithNolintlintLast) Len() int {
+ return len(issues)
+}
+
+func (issues sortWithNolintlintLast) Less(i, j int) bool {
+ return issues[i].FromLinter != golinters.NolintlintName && issues[j].FromLinter == golinters.NolintlintName
+}
+
+func (issues sortWithNolintlintLast) Swap(i, j int) {
+ issues[j], issues[i] = issues[i], issues[j]
+}
diff --git a/pkg/result/processors/nolint_test.go b/pkg/result/processors/nolint_test.go
index b547fbd0..051b1ebc 100644
--- a/pkg/result/processors/nolint_test.go
+++ b/pkg/result/processors/nolint_test.go
@@ -9,6 +9,8 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
+ "github.com/golangci/golangci-lint/pkg/config"
+ "github.com/golangci/golangci-lint/pkg/golinters"
"github.com/golangci/golangci-lint/pkg/lint/lintersdb"
"github.com/golangci/golangci-lint/pkg/logutils"
"github.com/golangci/golangci-lint/pkg/result"
@@ -31,7 +33,7 @@ func newNolint2FileIssue(line int) result.Issue {
}
func newTestNolintProcessor(log logutils.Log) *Nolint {
- return NewNolint(log, lintersdb.NewManager(nil, nil))
+ return NewNolint(log, lintersdb.NewManager(nil, nil), nil)
}
func getMockLog() *logutils.MockLog {
@@ -40,7 +42,6 @@ func getMockLog() *logutils.MockLog {
return log
}
-//nolint:funlen
func TestNolint(t *testing.T) {
p := newTestNolintProcessor(getMockLog())
defer p.Finish()
@@ -229,7 +230,7 @@ func TestNolintWholeFile(t *testing.T) {
Filename: fileName,
Line: 4,
},
- FromLinter: "unparam",
+ FromLinter: "varcheck",
})
processAssertSame(t, p, result.Issue{
Pos: token.Position{
@@ -239,3 +240,68 @@ func TestNolintWholeFile(t *testing.T) {
FromLinter: "deadcode",
})
}
+
+func TestNolintUnused(t *testing.T) {
+ fileName := filepath.Join("testdata", "nolint_unused.go")
+
+ log := getMockLog()
+ log.On("Warnf", "Found unknown linters in //nolint directives: %s", "blah")
+
+ createProcessor := func(t *testing.T, log *logutils.MockLog, enabledLinters []string) *Nolint {
+ enabledSetLog := logutils.NewMockLog()
+ enabledSetLog.On("Infof", "Active %d linters: %s", len(enabledLinters), enabledLinters)
+ cfg := &config.Config{Linters: config.Linters{DisableAll: true, Enable: enabledLinters}}
+ dbManager := lintersdb.NewManager(cfg, nil)
+ enabledLintersSet := lintersdb.NewEnabledSet(dbManager, lintersdb.NewValidator(dbManager), enabledSetLog, cfg)
+ lcs, err := enabledLintersSet.Get(false)
+ assert.NoError(t, err)
+ return NewNolint(log, dbManager, lcs)
+ }
+
+ // the issues below the nolintlint issues that would be generated for the test file
+ nolintlintIssueVarcheck := result.Issue{
+ Pos: token.Position{
+ Filename: fileName,
+ Line: 3,
+ },
+ FromLinter: golinters.NolintlintName,
+ ExpectNoLint: true,
+ ExpectedNoLintLinter: "varcheck",
+ }
+
+ t.Run("when an issue does not occur, it is not removed from the nolintlint issues", func(t *testing.T) {
+ p := createProcessor(t, log, []string{"nolintlint", "varcheck"})
+ defer p.Finish()
+
+ processAssertSame(t, p, nolintlintIssueVarcheck)
+ })
+
+ t.Run("when an issue occurs, it is removed from the nolintlint issues", func(t *testing.T) {
+ p := createProcessor(t, log, []string{"nolintlint", "varcheck"})
+ defer p.Finish()
+
+ processAssertEmpty(t, p, []result.Issue{{
+ Pos: token.Position{
+ Filename: fileName,
+ Line: 3,
+ },
+ FromLinter: "varcheck",
+ }, nolintlintIssueVarcheck}...)
+ })
+
+ t.Run("when a linter is not enabled, it is removed from the nolintlint unused issues", func(t *testing.T) {
+ enabledSetLog := logutils.NewMockLog()
+ enabledSetLog.On("Infof", "Active %d linters: %s", 1, []string{"nolintlint"})
+
+ cfg := &config.Config{Linters: config.Linters{DisableAll: true, Enable: []string{"nolintlint"}}}
+ dbManager := lintersdb.NewManager(cfg, nil)
+ enabledLintersSet := lintersdb.NewEnabledSet(dbManager, lintersdb.NewValidator(dbManager), enabledSetLog, cfg)
+
+ lcs, err := enabledLintersSet.Get(false)
+ assert.NoError(t, err)
+ p := NewNolint(log, dbManager, lcs)
+ defer p.Finish()
+
+ processAssertEmpty(t, p, nolintlintIssueVarcheck)
+ })
+}
diff --git a/pkg/result/processors/testdata/nolint_unused.go b/pkg/result/processors/testdata/nolint_unused.go
new file mode 100644
index 00000000..6ca35f3c
--- /dev/null
+++ b/pkg/result/processors/testdata/nolint_unused.go
@@ -0,0 +1,3 @@
+package testdata
+
+var nolintVarcheck int // nolint:varcheck
diff --git a/pkg/result/processors/testdata/nolint_whole_file.go b/pkg/result/processors/testdata/nolint_whole_file.go
index 649923b2..e0f93c83 100644
--- a/pkg/result/processors/testdata/nolint_whole_file.go
+++ b/pkg/result/processors/testdata/nolint_whole_file.go
@@ -1,4 +1,4 @@
-//nolint: unparam
+//nolint: varcheck
package testdata
-var nolintUnparam int
+var nolintVarcheck int
diff --git a/test/bench/bench_test.go b/test/bench/bench_test.go
index 42caffd7..77e4e5dc 100644
--- a/test/bench/bench_test.go
+++ b/test/bench/bench_test.go
@@ -211,7 +211,7 @@ type runResult struct {
duration time.Duration
}
-func compare(b *testing.B, gometalinterRun, golangciLintRun func(*testing.B), repoName, mode string, kLOC int) { // nolint
+func compare(b *testing.B, gometalinterRun, golangciLintRun func(*testing.B), repoName, mode string, kLOC int) {
gometalinterRes := runOne(b, gometalinterRun, "gometalinter")
golangciLintRes := runOne(b, golangciLintRun, "golangci-lint")
diff --git a/test/errchk.go b/test/errchk.go
index e1b90e6a..6cf12629 100644
--- a/test/errchk.go
+++ b/test/errchk.go
@@ -85,7 +85,7 @@ func errorCheck(outStr string, wantAuto bool, fullshort ...string) (err error) {
if len(errs) == 0 {
return nil
}
- if len(errs) == 1 { //nolint:gomnd
+ if len(errs) == 1 {
return errs[0]
}
var buf bytes.Buffer
@@ -103,7 +103,7 @@ func splitOutput(out string, wantAuto bool) []string {
var res []string
for _, line := range strings.Split(out, "\n") {
line = strings.TrimSuffix(line, "\r") // normalize Windows output
- if strings.HasPrefix(line, "\t") { //nolint:gocritic
+ if strings.HasPrefix(line, "\t") {
res[len(res)-1] += "\n" + line
} else if strings.HasPrefix(line, "go tool") || strings.HasPrefix(line, "#") || !wantAuto && strings.HasPrefix(line, "<autogenerated>") {
continue
diff --git a/test/testdata/nolintlint.go b/test/testdata/nolintlint.go
new file mode 100644
index 00000000..2294acc7
--- /dev/null
+++ b/test/testdata/nolintlint.go
@@ -0,0 +1,13 @@
+//args: -Enolintlint
+//config: linters-settings.nolintlint.require-explanation=true
+//config: linters-settings.nolintlint.require-specific=true
+//config: linters-settings.nolintlint.allowing-leading-space=false
+package testdata
+
+import "fmt"
+
+func Foo() {
+ fmt.Println("not specific") //nolint // ERROR "directive `.*` should mention specific linter such as `//nolint:my-linter`"
+ fmt.Println("not machine readable") // nolint // ERROR "directive `.*` should be written as `//nolint`"
+ fmt.Println("extra spaces") // nolint:deadcode // because // ERROR "directive `.*` should not have more than one leading space"
+}
diff --git a/test/testdata/nolintlint_unused.go b/test/testdata/nolintlint_unused.go
new file mode 100644
index 00000000..184a2116
--- /dev/null
+++ b/test/testdata/nolintlint_unused.go
@@ -0,0 +1,11 @@
+//args: -Enolintlint -Evarcheck
+//config: linters-settings.nolintlint.allow-unused=false
+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
+}