chore: refactor nolintlint

Author: ldez

pkg/golinters/nolintlint/internal/issues.go

@@ -0,0 +1,41 @@
+package internal
+
+import (
+	"fmt"
+	"strings"
+	"unicode"
+)
+
+func formatExtraLeadingSpace(fullDirective string) string {
+	return fmt.Sprintf("directive `%s` should not have more than one leading space", fullDirective)
+}
+
+func formatNotMachine(fullDirective string) string {
+	expected := fullDirective[:2] + strings.TrimLeftFunc(fullDirective[2:], unicode.IsSpace)
+	return fmt.Sprintf("directive `%s` should be written without leading space as `%s`",
+		fullDirective, expected)
+}
+
+func formatNotSpecific(fullDirective, directiveWithOptionalLeadingSpace string) string {
+	return fmt.Sprintf("directive `%s` should mention specific linter such as `%s:my-linter`",
+		fullDirective, directiveWithOptionalLeadingSpace)
+}
+
+func formatParseError(fullDirective, directiveWithOptionalLeadingSpace string) string {
+	return fmt.Sprintf("directive `%s` should match `%s[:<comma-separated-linters>] [// <explanation>]`",
+		fullDirective,
+		directiveWithOptionalLeadingSpace)
+}
+
+func formatNoExplanation(fullDirective, fullDirectiveWithoutExplanation string) string {
+	return fmt.Sprintf("directive `%s` should provide explanation such as `%s // this is why`",
+		fullDirective, fullDirectiveWithoutExplanation)
+}
+
+func formatUnusedCandidate(fullDirective, expectedLinter string) string {
+	details := fmt.Sprintf("directive `%s` is unused", fullDirective)
+	if expectedLinter != "" {
+		details += fmt.Sprintf(" for linter %q", expectedLinter)
+	}
+	return details
+}

pkg/golinters/nolintlint/internal/nolintlint.go

@@ -2,123 +2,16 @@
 package internal
 
 import (
-	"fmt"
-	"go/ast"
-	"go/token"
 	"regexp"
 	"strings"
-	"unicode"
 
+	"golang.org/x/tools/go/analysis"
+
+	"github.com/golangci/golangci-lint/pkg/goanalysis"
 	"github.com/golangci/golangci-lint/pkg/result"
 )
 
-type BaseIssue struct {
-	fullDirective                     string
-	directiveWithOptionalLeadingSpace 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
-}
-
-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)
-}
-
-func (i NotSpecific) String() string { return toString(i) }
-
-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[:<comma-separated-linters>] [// <explanation>]`",
-		i.fullDirective,
-		i.directiveWithOptionalLeadingSpace)
-}
-
-func (i ParseError) String() string { return toString(i) }
-
-type NoExplanation struct {
-	BaseIssue
-	fullDirectiveWithoutExplanation string
-}
-
-//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)
-}
-
-func (i NoExplanation) String() string { return toString(i) }
-
-type UnusedCandidate struct {
-	BaseIssue
-	ExpectedLinter string
-}
-
-//nolint:gocritic // TODO(ldez) must be change in the future.
-func (i UnusedCandidate) Details() string {
-	details := fmt.Sprintf("directive `%s` is unused", i.fullDirective)
-	if i.ExpectedLinter != "" {
-		details += fmt.Sprintf(" for linter %q", i.ExpectedLinter)
-	}
-	return details
-}
-
-func (i UnusedCandidate) String() string { return toString(i) }
-
-func toString(issue Issue) string {
-	return fmt.Sprintf("%s at %s", issue.Details(), issue.Position())
-}
-
-type Issue interface {
-	Details() string
-	Position() token.Position
-	String() string
-	Replacement() *result.Replacement
-}
-
-type Needs uint
+const LinterName = "nolintlint"
 
 const (
 	NeedsMachineOnly Needs = 1 << iota
@@ -128,6 +21,8 @@ const (
 	NeedsAll = NeedsMachineOnly | NeedsSpecific | NeedsExplanation
 )
 
+type Needs uint
+
 var commentPattern = regexp.MustCompile(`^//\s*(nolint)(:\s*[\w-]+\s*(?:,\s*[\w-]+\s*)*)?\b`)
 
 // matches a complete nolint directive
@@ -157,15 +52,10 @@ var (
 )
 
 //nolint:funlen,gocyclo // the function is going to be refactored in the future
-func (l Linter) Run(fset *token.FileSet, nodes ...ast.Node) ([]Issue, error) {
-	var issues []Issue
-
-	for _, node := range nodes {
-		file, ok := node.(*ast.File)
-		if !ok {
-			continue
-		}
+func (l Linter) Run(pass *analysis.Pass) ([]goanalysis.Issue, error) {
+	var issues []goanalysis.Issue
 
+	for _, file := range pass.Files {
 		for _, c := range file.Comments {
 			for _, comment := range c.List {
 				if !commentPattern.MatchString(comment.Text) {
@@ -188,39 +78,51 @@ func (l Linter) Run(fset *token.FileSet, nodes ...ast.Node) ([]Issue, error) {
 				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,
-				}
+				pos := pass.Fset.Position(comment.Pos())
+				end := pass.Fset.Position(comment.End())
 
 				// 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: "",
+							StartCol: pos.Column + 1,
+							Length:   len(leadingSpace),
 						},
 					}
+
 					if (l.needs & NeedsMachineOnly) != 0 {
-						issue := NotMachine{BaseIssue: base}
-						issue.BaseIssue.replacement = removeWhitespace
-						issues = append(issues, issue)
+						issue := &result.Issue{
+							FromLinter:  LinterName,
+							Text:        formatNotMachine(comment.Text),
+							Pos:         pos,
+							Replacement: removeWhitespace,
+						}
+
+						issues = append(issues, goanalysis.NewIssue(issue, pass))
 					} 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)
+						issue := &result.Issue{
+							FromLinter:  LinterName,
+							Text:        formatExtraLeadingSpace(comment.Text),
+							Pos:         pos,
+							Replacement: removeWhitespace,
+						}
+
+						issue.Replacement.Inline.NewString = " " // assume a single space was intended
+
+						issues = append(issues, goanalysis.NewIssue(issue, pass))
 					}
 				}
 
 				fullMatches := fullDirectivePattern.FindStringSubmatch(comment.Text)
 				if len(fullMatches) == 0 {
-					issues = append(issues, ParseError{BaseIssue: base})
+					issue := &result.Issue{
+						FromLinter: LinterName,
+						Text:       formatParseError(comment.Text, directiveWithOptionalLeadingSpace),
+						Pos:        pos,
+					}
+
+					issues = append(issues, goanalysis.NewIssue(issue, pass))
+
 					continue
 				}
 
@@ -246,7 +148,13 @@ func (l Linter) Run(fset *token.FileSet, nodes ...ast.Node) ([]Issue, error) {
 
 				if (l.needs & NeedsSpecific) != 0 {
 					if len(linters) == 0 {
-						issues = append(issues, NotSpecific{BaseIssue: base})
+						issue := &result.Issue{
+							FromLinter: LinterName,
+							Text:       formatNotSpecific(comment.Text, directiveWithOptionalLeadingSpace),
+							Pos:        pos,
+						}
+
+						issues = append(issues, goanalysis.NewIssue(issue, pass))
 					}
 				}
 
@@ -261,26 +169,39 @@ func (l Linter) Run(fset *token.FileSet, nodes ...ast.Node) ([]Issue, error) {
 						removeNolintCompletely.NeedOnlyDelete = true
 					} else {
 						removeNolintCompletely.Inline = &result.InlineFix{
-							StartCol:  startCol,
-							Length:    end.Column - pos.Column,
-							NewString: "",
+							StartCol: startCol,
+							Length:   end.Column - pos.Column,
 						}
 					}
 
 					if len(linters) == 0 {
-						issue := UnusedCandidate{BaseIssue: base}
-						issue.replacement = removeNolintCompletely
-						issues = append(issues, issue)
+						issue := &result.Issue{
+							FromLinter:   LinterName,
+							Text:         formatUnusedCandidate(comment.Text, ""),
+							Pos:          pos,
+							ExpectNoLint: true,
+							Replacement:  removeNolintCompletely,
+						}
+
+						issues = append(issues, goanalysis.NewIssue(issue, pass))
 					} else {
 						for _, linter := range linters {
-							issue := UnusedCandidate{BaseIssue: base, ExpectedLinter: linter}
+							issue := &result.Issue{
+								FromLinter:           LinterName,
+								Text:                 formatUnusedCandidate(comment.Text, linter),
+								Pos:                  pos,
+								ExpectNoLint:         true,
+								ExpectedNoLintLinter: linter,
+							}
+
 							// only offer replacement if there is a single linter
 							// because of issues around commas and the possibility of all
 							// linters being removed
 							if len(linters) == 1 {
-								issue.replacement = removeNolintCompletely
+								issue.Replacement = removeNolintCompletely
 							}
-							issues = append(issues, issue)
+
+							issues = append(issues, goanalysis.NewIssue(issue, pass))
 						}
 					}
 				}
@@ -297,10 +218,14 @@ func (l Linter) Run(fset *token.FileSet, nodes ...ast.Node) ([]Issue, error) {
 
 					if needsExplanation {
 						fullDirectiveWithoutExplanation := trailingBlankExplanation.ReplaceAllString(comment.Text, "")
-						issues = append(issues, NoExplanation{
-							BaseIssue:                       base,
-							fullDirectiveWithoutExplanation: fullDirectiveWithoutExplanation,
-						})
+
+						issue := &result.Issue{
+							FromLinter: LinterName,
+							Text:       formatNoExplanation(comment.Text, fullDirectiveWithoutExplanation),
+							Pos:        pos,
+						}
+
+						issues = append(issues, goanalysis.NewIssue(issue, pass))
 					}
 				}
 			}