Skip to content

Commit 6172338

Browse files
authored
nolintlint: fix false positive. (#2013)
1 parent 4c27b33 commit 6172338

File tree

3 files changed

+21
-9
lines changed

3 files changed

+21
-9
lines changed

pkg/golinters/nolintlint/nolintlint.go

+17-5
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// nolintlint provides a linter to ensure that all //nolint directives are followed by explanations
1+
// Package nolintlint provides a linter to ensure that all //nolint directives are followed by explanations
22
package nolintlint
33

44
import (
@@ -19,10 +19,12 @@ type BaseIssue struct {
1919
replacement *result.Replacement
2020
}
2121

22+
//nolint:gocritic // TODO must be change in the future.
2223
func (b BaseIssue) Position() token.Position {
2324
return b.position
2425
}
2526

27+
//nolint:gocritic // TODO must be change in the future.
2628
func (b BaseIssue) Replacement() *result.Replacement {
2729
return b.replacement
2830
}
@@ -31,64 +33,75 @@ type ExtraLeadingSpace struct {
3133
BaseIssue
3234
}
3335

36+
//nolint:gocritic // TODO must be change in the future.
3437
func (i ExtraLeadingSpace) Details() string {
3538
return fmt.Sprintf("directive `%s` should not have more than one leading space", i.fullDirective)
3639
}
3740

41+
//nolint:gocritic // TODO must be change in the future.
3842
func (i ExtraLeadingSpace) String() string { return toString(i) }
3943

4044
type NotMachine struct {
4145
BaseIssue
4246
}
4347

48+
//nolint:gocritic // TODO must be change in the future.
4449
func (i NotMachine) Details() string {
4550
expected := i.fullDirective[:2] + strings.TrimLeftFunc(i.fullDirective[2:], unicode.IsSpace)
4651
return fmt.Sprintf("directive `%s` should be written without leading space as `%s`",
4752
i.fullDirective, expected)
4853
}
4954

55+
//nolint:gocritic // TODO must be change in the future.
5056
func (i NotMachine) String() string { return toString(i) }
5157

5258
type NotSpecific struct {
5359
BaseIssue
5460
}
5561

62+
//nolint:gocritic // TODO must be change in the future.
5663
func (i NotSpecific) Details() string {
5764
return fmt.Sprintf("directive `%s` should mention specific linter such as `%s:my-linter`",
5865
i.fullDirective, i.directiveWithOptionalLeadingSpace)
5966
}
6067

68+
//nolint:gocritic // TODO must be change in the future.
6169
func (i NotSpecific) String() string { return toString(i) }
6270

6371
type ParseError struct {
6472
BaseIssue
6573
}
6674

75+
//nolint:gocritic // TODO must be change in the future.
6776
func (i ParseError) Details() string {
6877
return fmt.Sprintf("directive `%s` should match `%s[:<comma-separated-linters>] [// <explanation>]`",
6978
i.fullDirective,
7079
i.directiveWithOptionalLeadingSpace)
7180
}
7281

82+
//nolint:gocritic // TODO must be change in the future.
7383
func (i ParseError) String() string { return toString(i) }
7484

7585
type NoExplanation struct {
7686
BaseIssue
7787
fullDirectiveWithoutExplanation string
7888
}
7989

90+
//nolint:gocritic // TODO must be change in the future.
8091
func (i NoExplanation) Details() string {
8192
return fmt.Sprintf("directive `%s` should provide explanation such as `%s // this is why`",
8293
i.fullDirective, i.fullDirectiveWithoutExplanation)
8394
}
8495

96+
//nolint:gocritic // TODO must be change in the future.
8597
func (i NoExplanation) String() string { return toString(i) }
8698

8799
type UnusedCandidate struct {
88100
BaseIssue
89101
ExpectedLinter string
90102
}
91103

104+
//nolint:gocritic // TODO must be change in the future.
92105
func (i UnusedCandidate) Details() string {
93106
details := fmt.Sprintf("directive `%s` is unused", i.fullDirective)
94107
if i.ExpectedLinter != "" {
@@ -97,6 +110,7 @@ func (i UnusedCandidate) Details() string {
97110
return details
98111
}
99112

113+
//nolint:gocritic // TODO must be change in the future.
100114
func (i UnusedCandidate) String() string { return toString(i) }
101115

102116
func toString(i Issue) string {
@@ -126,8 +140,7 @@ var commentPattern = regexp.MustCompile(`^//\s*(nolint)(:\s*[\w-]+\s*(?:,\s*[\w-
126140
var fullDirectivePattern = regexp.MustCompile(`^//\s*nolint(?::(\s*[\w-]+\s*(?:,\s*[\w-]+\s*)*))?\s*(//.*)?\s*\n?$`)
127141

128142
type Linter struct {
129-
excludes []string // lists individual linters that don't require explanations
130-
needs Needs // indicates which linter checks to perform
143+
needs Needs // indicates which linter checks to perform
131144
excludeByLinter map[string]bool
132145
}
133146

@@ -147,6 +160,7 @@ func NewLinter(needs Needs, excludes []string) (*Linter, error) {
147160
var leadingSpacePattern = regexp.MustCompile(`^//(\s*)`)
148161
var trailingBlankExplanation = regexp.MustCompile(`\s*(//\s*)?$`)
149162

163+
//nolint:funlen,gocyclo
150164
func (l Linter) Run(fset *token.FileSet, nodes ...ast.Node) ([]Issue, error) {
151165
var issues []Issue
152166

@@ -214,7 +228,6 @@ func (l Linter) Run(fset *token.FileSet, nodes ...ast.Node) ([]Issue, error) {
214228

215229
lintersText, explanation := fullMatches[1], fullMatches[2]
216230
var linters []string
217-
var linterRange []result.Range
218231
if len(lintersText) > 0 {
219232
lls := strings.Split(lintersText, ",")
220233
linters = make([]string, 0, len(lls))
@@ -227,7 +240,6 @@ func (l Linter) Run(fset *token.FileSet, nodes ...ast.Node) ([]Issue, error) {
227240
trimmedLinterName := strings.TrimSpace(ll)
228241
if trimmedLinterName != "" {
229242
linters = append(linters, trimmedLinterName)
230-
linterRange = append(linterRange, result.Range{From: rangeStart, To: rangeEnd})
231243
}
232244
rangeStart = rangeEnd
233245
}

pkg/result/processors/nolint.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"go/ast"
66
"go/parser"
77
"go/token"
8+
"regexp"
89
"sort"
910
"strings"
1011

@@ -46,7 +47,7 @@ func (i *ignoredRange) doesMatch(issue *result.Issue) bool {
4647

4748
// handle possible unused nolint directives
4849
// nolintlint generates potential issues for every nolint directive and they are filtered out here
49-
if issue.FromLinter == golinters.NolintlintName && issue.ExpectNoLint {
50+
if issue.FromLinter == golinters.NolintlintName && issue.ExpectNoLint {
5051
if issue.ExpectedNoLintLinter != "" {
5152
return i.matchedIssueFromLinter[issue.ExpectedNoLintLinter]
5253
}
@@ -146,7 +147,6 @@ func (p *Nolint) buildIgnoredRangesForFile(f *ast.File, fset *token.FileSet, fil
146147

147148
func (p *Nolint) shouldPassIssue(i *result.Issue) (bool, error) {
148149
nolintDebugf("got issue: %v", *i)
149-
150150
if i.FromLinter == golinters.NolintlintName && i.ExpectNoLint && i.ExpectedNoLintLinter != "" {
151151
// don't expect disabled linters to cover their nolint statements
152152
nolintDebugf("enabled linters: %v", p.enabledLinters)
@@ -234,7 +234,7 @@ func (p *Nolint) extractFileCommentsInlineRanges(fset *token.FileSet, comments .
234234

235235
func (p *Nolint) extractInlineRangeFromComment(text string, g ast.Node, fset *token.FileSet) *ignoredRange {
236236
text = strings.TrimLeft(text, "/ ")
237-
if !strings.HasPrefix(text, "nolint") {
237+
if ok, _ := regexp.MatchString(`^nolint( |:|$)`, text); !ok {
238238
return nil
239239
}
240240

test/testdata_etc/unused_exported/main.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,4 @@ import (
66

77
func main() {
88
lib.PublicFunc()
9-
}
9+
}

0 commit comments

Comments
 (0)