From 09531cff8072e66f7c3baa37872797d6b879bbd6 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Mon, 24 May 2021 22:13:30 +0200 Subject: [PATCH 1/2] fix: nolint false positive. --- pkg/result/processors/nolint.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/result/processors/nolint.go b/pkg/result/processors/nolint.go index 06af04ea19cf..f7085ab93387 100644 --- a/pkg/result/processors/nolint.go +++ b/pkg/result/processors/nolint.go @@ -5,6 +5,7 @@ import ( "go/ast" "go/parser" "go/token" + "regexp" "sort" "strings" @@ -146,7 +147,6 @@ 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 && i.ExpectNoLint && i.ExpectedNoLintLinter != "" { // don't expect disabled linters to cover their nolint statements nolintDebugf("enabled linters: %v", p.enabledLinters) @@ -234,7 +234,7 @@ func (p *Nolint) extractFileCommentsInlineRanges(fset *token.FileSet, comments . func (p *Nolint) extractInlineRangeFromComment(text string, g ast.Node, fset *token.FileSet) *ignoredRange { text = strings.TrimLeft(text, "/ ") - if !strings.HasPrefix(text, "nolint") { + if ok, _ := regexp.MatchString(`^nolint( |:|$)`, text); !ok { return nil } From dcaf632bd1305b10cbf3d3c797ac9e66cb247b8e Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Mon, 24 May 2021 22:13:51 +0200 Subject: [PATCH 2/2] chore: apply fixes. --- pkg/golinters/nolintlint/nolintlint.go | 22 +++++++++++++++++----- pkg/result/processors/nolint.go | 2 +- test/testdata_etc/unused_exported/main.go | 2 +- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/pkg/golinters/nolintlint/nolintlint.go b/pkg/golinters/nolintlint/nolintlint.go index 26de1dcb98aa..4466cab41f36 100644 --- a/pkg/golinters/nolintlint/nolintlint.go +++ b/pkg/golinters/nolintlint/nolintlint.go @@ -1,4 +1,4 @@ -// nolintlint provides a linter to ensure that all //nolint directives are followed by explanations +// Package nolintlint provides a linter to ensure that all //nolint directives are followed by explanations package nolintlint import ( @@ -19,10 +19,12 @@ type BaseIssue struct { replacement *result.Replacement } +//nolint:gocritic // TODO must be change in the future. func (b BaseIssue) Position() token.Position { return b.position } +//nolint:gocritic // TODO must be change in the future. func (b BaseIssue) Replacement() *result.Replacement { return b.replacement } @@ -31,45 +33,53 @@ type ExtraLeadingSpace struct { BaseIssue } +//nolint:gocritic // TODO must be change in the future. func (i ExtraLeadingSpace) Details() string { return fmt.Sprintf("directive `%s` should not have more than one leading space", i.fullDirective) } +//nolint:gocritic // TODO must be change in the future. func (i ExtraLeadingSpace) String() string { return toString(i) } type NotMachine struct { BaseIssue } +//nolint:gocritic // TODO must be change in the future. 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) } +//nolint:gocritic // TODO must be change in the future. func (i NotMachine) String() string { return toString(i) } type NotSpecific struct { BaseIssue } +//nolint:gocritic // TODO must be change in the future. func (i NotSpecific) Details() string { return fmt.Sprintf("directive `%s` should mention specific linter such as `%s:my-linter`", i.fullDirective, i.directiveWithOptionalLeadingSpace) } +//nolint:gocritic // TODO must be change in the future. func (i NotSpecific) String() string { return toString(i) } type ParseError struct { BaseIssue } +//nolint:gocritic // TODO must be change in the future. func (i ParseError) Details() string { return fmt.Sprintf("directive `%s` should match `%s[:] [// ]`", i.fullDirective, i.directiveWithOptionalLeadingSpace) } +//nolint:gocritic // TODO must be change in the future. func (i ParseError) String() string { return toString(i) } type NoExplanation struct { @@ -77,11 +87,13 @@ type NoExplanation struct { fullDirectiveWithoutExplanation string } +//nolint:gocritic // TODO must be change in the future. func (i NoExplanation) Details() string { return fmt.Sprintf("directive `%s` should provide explanation such as `%s // this is why`", i.fullDirective, i.fullDirectiveWithoutExplanation) } +//nolint:gocritic // TODO must be change in the future. func (i NoExplanation) String() string { return toString(i) } type UnusedCandidate struct { @@ -89,6 +101,7 @@ type UnusedCandidate struct { ExpectedLinter string } +//nolint:gocritic // TODO must be change in the future. func (i UnusedCandidate) Details() string { details := fmt.Sprintf("directive `%s` is unused", i.fullDirective) if i.ExpectedLinter != "" { @@ -97,6 +110,7 @@ func (i UnusedCandidate) Details() string { return details } +//nolint:gocritic // TODO must be change in the future. func (i UnusedCandidate) String() string { return toString(i) } func toString(i Issue) string { @@ -126,8 +140,7 @@ var commentPattern = regexp.MustCompile(`^//\s*(nolint)(:\s*[\w-]+\s*(?:,\s*[\w- 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 + needs Needs // indicates which linter checks to perform excludeByLinter map[string]bool } @@ -147,6 +160,7 @@ func NewLinter(needs Needs, excludes []string) (*Linter, error) { var leadingSpacePattern = regexp.MustCompile(`^//(\s*)`) var trailingBlankExplanation = regexp.MustCompile(`\s*(//\s*)?$`) +//nolint:funlen,gocyclo func (l Linter) Run(fset *token.FileSet, nodes ...ast.Node) ([]Issue, error) { var issues []Issue @@ -214,7 +228,6 @@ func (l Linter) Run(fset *token.FileSet, nodes ...ast.Node) ([]Issue, error) { lintersText, explanation := fullMatches[1], fullMatches[2] var linters []string - var linterRange []result.Range if len(lintersText) > 0 { lls := strings.Split(lintersText, ",") linters = make([]string, 0, len(lls)) @@ -227,7 +240,6 @@ func (l Linter) Run(fset *token.FileSet, nodes ...ast.Node) ([]Issue, error) { trimmedLinterName := strings.TrimSpace(ll) if trimmedLinterName != "" { linters = append(linters, trimmedLinterName) - linterRange = append(linterRange, result.Range{From: rangeStart, To: rangeEnd}) } rangeStart = rangeEnd } diff --git a/pkg/result/processors/nolint.go b/pkg/result/processors/nolint.go index f7085ab93387..0788a7160ee9 100644 --- a/pkg/result/processors/nolint.go +++ b/pkg/result/processors/nolint.go @@ -47,7 +47,7 @@ func (i *ignoredRange) doesMatch(issue *result.Issue) bool { // handle possible unused nolint directives // nolintlint generates potential issues for every nolint directive and they are filtered out here - if issue.FromLinter == golinters.NolintlintName && issue.ExpectNoLint { + if issue.FromLinter == golinters.NolintlintName && issue.ExpectNoLint { if issue.ExpectedNoLintLinter != "" { return i.matchedIssueFromLinter[issue.ExpectedNoLintLinter] } diff --git a/test/testdata_etc/unused_exported/main.go b/test/testdata_etc/unused_exported/main.go index 575e34a146bd..11f3357a1734 100644 --- a/test/testdata_etc/unused_exported/main.go +++ b/test/testdata_etc/unused_exported/main.go @@ -6,4 +6,4 @@ import ( func main() { lib.PublicFunc() -} +}