diff --git a/pkg/golinters/nolintlint/nolintlint.go b/pkg/golinters/nolintlint/nolintlint.go index 1bce5ef5d2d8..82678106239b 100644 --- a/pkg/golinters/nolintlint/nolintlint.go +++ b/pkg/golinters/nolintlint/nolintlint.go @@ -7,24 +7,20 @@ import ( "go/token" "regexp" "strings" - "unicode" "github.com/golangci/golangci-lint/pkg/result" ) type BaseIssue struct { - fullDirective string - directiveWithOptionalLeadingSpace string - position token.Position - replacement *result.Replacement + fullDirective string + position token.Position + replacement *result.Replacement } -//nolint:gocritic // TODO(ldez) must be change in the future. func (b BaseIssue) Position() token.Position { return b.position } -//nolint:gocritic // TODO(ldez) must be change in the future. func (b BaseIssue) Replacement() *result.Replacement { return b.replacement } @@ -33,34 +29,19 @@ type ExtraLeadingSpace struct { BaseIssue } -//nolint:gocritic // TODO(ldez) 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) } func (i ExtraLeadingSpace) String() string { return toString(i) } -type NotMachine struct { - BaseIssue -} - -//nolint:gocritic // TODO(ldez) 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) -} - -func (i NotMachine) String() string { return toString(i) } - type NotSpecific struct { BaseIssue } -//nolint:gocritic // TODO(ldez) 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) + return fmt.Sprintf("directive `%s` should mention specific linter such as `//nolint:my-linter`", + i.fullDirective) } func (i NotSpecific) String() string { return toString(i) } @@ -69,11 +50,9 @@ type ParseError struct { BaseIssue } -//nolint:gocritic // TODO(ldez) must be change in the future. func (i ParseError) Details() string { - return fmt.Sprintf("directive `%s` should match `%s[:] [// ]`", - i.fullDirective, - i.directiveWithOptionalLeadingSpace) + return fmt.Sprintf("directive `%s` should match `//nolint[:] [// ]`", + i.fullDirective) } func (i ParseError) String() string { return toString(i) } @@ -85,8 +64,9 @@ type NoExplanation struct { //nolint:gocritic // TODO(ldez) 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) + baseMsg := "directive `%s` is missing an explanation; " + explanationMsg := "it should follow the format `//nolint[:] // `" + return fmt.Sprintf(baseMsg+explanationMsg, i.fullDirective) } func (i NoExplanation) String() string { return toString(i) } @@ -121,17 +101,20 @@ type Issue interface { type Needs uint const ( + // Deprecated: NeedsMachineOnly is deprecated as leading spaces are no longer allowed, + // making this condition always true. Consumers should adjust their code to assume + // this as the default behavior and no longer rely on NeedsMachineOnly. NeedsMachineOnly Needs = 1 << iota NeedsSpecific NeedsExplanation NeedsUnused - NeedsAll = NeedsMachineOnly | NeedsSpecific | NeedsExplanation + NeedsAll = NeedsSpecific | NeedsExplanation ) var commentPattern = 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?$`) +var fullDirectivePattern = regexp.MustCompile(`^//nolint(?::(\s*[\w-]+\s*(?:,\s*[\w-]+\s*)*))?\s*(//.*)?\s*\n?$`) type Linter struct { needs Needs // indicates which linter checks to perform @@ -146,7 +129,7 @@ func NewLinter(needs Needs, excludes []string) (*Linter, error) { } return &Linter{ - needs: needs | NeedsMachineOnly, + needs: needs, excludeByLinter: excludeByName, }, nil } @@ -180,42 +163,12 @@ func (l Linter) Run(fset *token.FileSet, nodes ...ast.Node) ([]Issue, error) { leadingSpace = leadingSpaceMatches[1] } - directiveWithOptionalLeadingSpace := "//" - if leadingSpace != "" { - 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()) base := BaseIssue{ - fullDirective: comment.Text, - directiveWithOptionalLeadingSpace: directiveWithOptionalLeadingSpace, - position: pos, - } - - // check for, report and eliminate leading spaces, so we can check for other issues - if leadingSpace != "" { - removeWhitespace := &result.Replacement{ - Inline: &result.InlineFix{ - StartCol: pos.Column + 1, - Length: len(leadingSpace), - NewString: "", - }, - } - if (l.needs & NeedsMachineOnly) != 0 { - issue := NotMachine{BaseIssue: base} - issue.BaseIssue.replacement = removeWhitespace - issues = append(issues, issue) - } else if len(leadingSpace) > 1 { - issue := ExtraLeadingSpace{BaseIssue: base} - issue.BaseIssue.replacement = removeWhitespace - issue.BaseIssue.replacement.Inline.NewString = " " // assume a single space was intended - issues = append(issues, issue) - } + fullDirective: comment.Text, + position: pos, } fullMatches := fullDirectivePattern.FindStringSubmatch(comment.Text) diff --git a/pkg/golinters/nolintlint/nolintlint_test.go b/pkg/golinters/nolintlint/nolintlint_test.go index a483c7ea66b1..19ebcda8a7c7 100644 --- a/pkg/golinters/nolintlint/nolintlint_test.go +++ b/pkg/golinters/nolintlint/nolintlint_test.go @@ -39,10 +39,10 @@ func foo() { other() //nolintother }`, expected: []issueWithReplacement{ - {issue: "directive `//nolint` should provide explanation such as `//nolint // this is why` at testing.go:5:1"}, - {issue: "directive `//nolint` should provide explanation such as `//nolint // this is why` at testing.go:7:9"}, - {issue: "directive `//nolint //` should provide explanation such as `//nolint // this is why` at testing.go:8:9"}, - {issue: "directive `//nolint // ` should provide explanation such as `//nolint // this is why` at testing.go:9:9"}, + {issue: "directive `//nolint` is missing an explanation; it should follow the format `//nolint[:] // ` at testing.go:5:1"}, + {issue: "directive `//nolint` is missing an explanation; it should follow the format `//nolint[:] // ` at testing.go:7:9"}, + {issue: "directive `//nolint //` is missing an explanation; it should follow the format `//nolint[:] // ` at testing.go:8:9"}, + {issue: "directive `//nolint // ` is missing an explanation; it should follow the format `//nolint[:] // ` at testing.go:9:9"}, }, }, { @@ -56,7 +56,7 @@ package bar //nolint:dupl func foo() {}`, expected: []issueWithReplacement{ - {issue: "directive `//nolint:dupl` should provide explanation such as `//nolint:dupl // this is why` at testing.go:6:1"}, + {issue: "directive `//nolint:dupl` is missing an explanation; it should follow the format `//nolint[:] // ` at testing.go:6:1"}, }, }, { @@ -97,26 +97,8 @@ func foo() { good() //nolint }`, expected: []issueWithReplacement{ - { - issue: "directive `// nolint` should be written without leading space as `//nolint` at testing.go:5:9", - replacement: &result.Replacement{ - Inline: &result.InlineFix{ - StartCol: 10, - Length: 1, - NewString: "", - }, - }, - }, - { - 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: 3, - NewString: "", - }, - }, - }, + {issue: "directive `// nolint` should match `//nolint[:] [// ]` at testing.go:5:9"}, + {issue: "directive `// nolint` should match `//nolint[:] [// ]` at testing.go:6:9"}, }, }, { @@ -129,9 +111,13 @@ func foo() { bad() //nolint:linter1 linter2 good() //nolint: linter1,linter2 good() //nolint: linter1, linter2 + bad() //nolint / description + bad() //nolint, // hi }`, expected: []issueWithReplacement{ {issue: "directive `//nolint:linter1 linter2` should match `//nolint[:] [// ]` at testing.go:6:9"}, + {issue: "directive `//nolint / description` should match `//nolint[:] [// ]` at testing.go:9:9"}, + {issue: "directive `//nolint, // hi` should match `//nolint[:] [// ]` at testing.go:10:9"}, }, }, {