From c73fc43a800e7a3df74c57e69e0857c16e6fe36e Mon Sep 17 00:00:00 2001 From: Aaron Pradhan <12a.pradhan@gmail.com> Date: Tue, 19 Mar 2024 16:48:29 -0400 Subject: [PATCH 01/10] Refactor nolint directive handling and update tests --- pkg/golinters/nolintlint/nolintlint.go | 77 ++++++++------------- pkg/golinters/nolintlint/nolintlint_test.go | 10 ++- 2 files changed, 38 insertions(+), 49 deletions(-) diff --git a/pkg/golinters/nolintlint/nolintlint.go b/pkg/golinters/nolintlint/nolintlint.go index 1bce5ef5d2d8..6e1fc2e4dbe8 100644 --- a/pkg/golinters/nolintlint/nolintlint.go +++ b/pkg/golinters/nolintlint/nolintlint.go @@ -12,28 +12,24 @@ import ( "github.com/golangci/golangci-lint/pkg/result" ) -type BaseIssue struct { - fullDirective string - directiveWithOptionalLeadingSpace string - position token.Position - replacement *result.Replacement +type baseIssue struct { + fullDirective string + position token.Position + replacement *result.Replacement } -//nolint:gocritic // TODO(ldez) must be change in the future. -func (b BaseIssue) Position() token.Position { +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 { +func (b baseIssue) Replacement() *result.Replacement { return b.replacement } type ExtraLeadingSpace struct { - BaseIssue + 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) } @@ -41,10 +37,9 @@ func (i ExtraLeadingSpace) Details() string { func (i ExtraLeadingSpace) String() string { return toString(i) } type NotMachine struct { - BaseIssue + 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`", @@ -54,32 +49,29 @@ func (i NotMachine) Details() string { func (i NotMachine) String() string { return toString(i) } type NotSpecific struct { - BaseIssue + 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) } type ParseError struct { - BaseIssue + 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) } type NoExplanation struct { - BaseIssue + baseIssue fullDirectiveWithoutExplanation string } @@ -92,7 +84,7 @@ func (i NoExplanation) Details() string { func (i NoExplanation) String() string { return toString(i) } type UnusedCandidate struct { - BaseIssue + baseIssue ExpectedLinter string } @@ -131,7 +123,7 @@ const ( 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(`^//\s*nolint(?::([\w-]+(?:,[\w-]+)*))?(?: (//.*))?\n?$`) type Linter struct { needs Needs // indicates which linter checks to perform @@ -180,21 +172,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, + base := baseIssue{ + fullDirective: comment.Text, + position: pos, } // check for, report and eliminate leading spaces, so we can check for other issues @@ -207,20 +190,20 @@ func (l Linter) Run(fset *token.FileSet, nodes ...ast.Node) ([]Issue, error) { }, } if (l.needs & NeedsMachineOnly) != 0 { - issue := NotMachine{BaseIssue: base} - issue.BaseIssue.replacement = removeWhitespace + 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 + issue := ExtraLeadingSpace{baseIssue: base} + issue.baseIssue.replacement = removeWhitespace + issue.baseIssue.replacement.Inline.NewString = " " // assume a single space was intended issues = append(issues, issue) } } fullMatches := fullDirectivePattern.FindStringSubmatch(comment.Text) if len(fullMatches) == 0 { - issues = append(issues, ParseError{BaseIssue: base}) + issues = append(issues, ParseError{baseIssue: base}) continue } @@ -246,7 +229,7 @@ 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}) + issues = append(issues, NotSpecific{baseIssue: base}) } } @@ -261,12 +244,12 @@ func (l Linter) Run(fset *token.FileSet, nodes ...ast.Node) ([]Issue, error) { } if len(linters) == 0 { - issue := UnusedCandidate{BaseIssue: base} + issue := UnusedCandidate{baseIssue: base} issue.replacement = removeNolintCompletely issues = append(issues, issue) } else { for _, linter := range linters { - issue := UnusedCandidate{BaseIssue: base, ExpectedLinter: linter} + issue := UnusedCandidate{baseIssue: base, ExpectedLinter: linter} // only offer replacement if there is a single linter // because of issues around commas and the possibility of all // linters being removed @@ -291,7 +274,7 @@ 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, + baseIssue: base, fullDirectiveWithoutExplanation: fullDirectiveWithoutExplanation, }) } diff --git a/pkg/golinters/nolintlint/nolintlint_test.go b/pkg/golinters/nolintlint/nolintlint_test.go index a483c7ea66b1..121b193182c9 100644 --- a/pkg/golinters/nolintlint/nolintlint_test.go +++ b/pkg/golinters/nolintlint/nolintlint_test.go @@ -127,11 +127,17 @@ package bar func foo() { good() //nolint:linter1,linter-two bad() //nolint:linter1 linter2 - good() //nolint: linter1,linter2 - good() //nolint: linter1, linter2 + bad() //nolint: linter1,linter2 + bad() //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: linter1,linter2` should match `//nolint[:] [// ]` at testing.go:7:9"}, + {issue: "directive `//nolint: linter1, linter2` should match `//nolint[:] [// ]` at testing.go:8: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"}, }, }, { From 852cc82b7fbe893560bae0de8e4eadf5727da377 Mon Sep 17 00:00:00 2001 From: Aaron Pradhan <12a.pradhan@gmail.com> Date: Tue, 19 Mar 2024 17:13:22 -0400 Subject: [PATCH 02/10] Deprecate NeedsMachineOnly and remove all references in code --- pkg/golinters/nolintlint/nolintlint.go | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/pkg/golinters/nolintlint/nolintlint.go b/pkg/golinters/nolintlint/nolintlint.go index 6e1fc2e4dbe8..e1c72b3a3f9f 100644 --- a/pkg/golinters/nolintlint/nolintlint.go +++ b/pkg/golinters/nolintlint/nolintlint.go @@ -113,11 +113,14 @@ 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`) @@ -138,7 +141,7 @@ func NewLinter(needs Needs, excludes []string) (*Linter, error) { } return &Linter{ - needs: needs | NeedsMachineOnly, + needs: needs, excludeByLinter: excludeByName, }, nil } @@ -189,16 +192,9 @@ func (l Linter) Run(fset *token.FileSet, nodes ...ast.Node) ([]Issue, error) { 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) - } + issue := NotMachine{baseIssue: base} + issue.baseIssue.replacement = removeWhitespace + issues = append(issues, issue) } fullMatches := fullDirectivePattern.FindStringSubmatch(comment.Text) From 8de2361e4beec1938b6b71d7be4c82235b1b426c Mon Sep 17 00:00:00 2001 From: Aaron Pradhan <12a.pradhan@gmail.com> Date: Tue, 19 Mar 2024 17:19:59 -0400 Subject: [PATCH 03/10] Remove NotMachine issue --- pkg/golinters/nolintlint/nolintlint.go | 27 -------------------------- 1 file changed, 27 deletions(-) diff --git a/pkg/golinters/nolintlint/nolintlint.go b/pkg/golinters/nolintlint/nolintlint.go index e1c72b3a3f9f..3daaebd4a735 100644 --- a/pkg/golinters/nolintlint/nolintlint.go +++ b/pkg/golinters/nolintlint/nolintlint.go @@ -7,7 +7,6 @@ import ( "go/token" "regexp" "strings" - "unicode" "github.com/golangci/golangci-lint/pkg/result" ) @@ -36,18 +35,6 @@ func (i ExtraLeadingSpace) Details() string { 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 } @@ -183,20 +170,6 @@ func (l Linter) Run(fset *token.FileSet, nodes ...ast.Node) ([]Issue, error) { 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: "", - }, - } - issue := NotMachine{baseIssue: base} - issue.baseIssue.replacement = removeWhitespace - issues = append(issues, issue) - } - fullMatches := fullDirectivePattern.FindStringSubmatch(comment.Text) if len(fullMatches) == 0 { issues = append(issues, ParseError{baseIssue: base}) From 5a0e9d7d2f4df039da541d1d8dc4a4a27af3bb1c Mon Sep 17 00:00:00 2001 From: Aaron Pradhan <12a.pradhan@gmail.com> Date: Tue, 19 Mar 2024 17:20:47 -0400 Subject: [PATCH 04/10] Disallow leading whitespace in fullDirectivePattern regular expression --- pkg/golinters/nolintlint/nolintlint.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/golinters/nolintlint/nolintlint.go b/pkg/golinters/nolintlint/nolintlint.go index 3daaebd4a735..410db5ab94e9 100644 --- a/pkg/golinters/nolintlint/nolintlint.go +++ b/pkg/golinters/nolintlint/nolintlint.go @@ -113,7 +113,7 @@ const ( var commentPattern = regexp.MustCompile(`^//\s*(nolint)(:\s*[\w-]+\s*(?:,\s*[\w-]+\s*)*)?\b`) // matches a complete nolint directive -var fullDirectivePattern = regexp.MustCompile(`^//\s*nolint(?::([\w-]+(?:,[\w-]+)*))?(?: (//.*))?\n?$`) +var fullDirectivePattern = regexp.MustCompile(`^//nolint(?::([\w-]+(?:,[\w-]+)*))?(?: (//.*))?\n?$`) type Linter struct { needs Needs // indicates which linter checks to perform From 0e91ea91446e78f54826edd5118a671e99f3a1b3 Mon Sep 17 00:00:00 2001 From: Aaron Pradhan <12a.pradhan@gmail.com> Date: Tue, 19 Mar 2024 17:23:26 -0400 Subject: [PATCH 05/10] Update test cases with leading space to use the ParseError issue --- pkg/golinters/nolintlint/nolintlint_test.go | 22 ++------------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/pkg/golinters/nolintlint/nolintlint_test.go b/pkg/golinters/nolintlint/nolintlint_test.go index 121b193182c9..f06dee612371 100644 --- a/pkg/golinters/nolintlint/nolintlint_test.go +++ b/pkg/golinters/nolintlint/nolintlint_test.go @@ -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"}, }, }, { From a6eb533da15a8588e985053b1716eb17224abcda Mon Sep 17 00:00:00 2001 From: Aaron Pradhan <12a.pradhan@gmail.com> Date: Tue, 19 Mar 2024 17:24:05 -0400 Subject: [PATCH 06/10] Fix unit test description for comma-separated linter list --- pkg/golinters/nolintlint/nolintlint_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/golinters/nolintlint/nolintlint_test.go b/pkg/golinters/nolintlint/nolintlint_test.go index f06dee612371..895127204a80 100644 --- a/pkg/golinters/nolintlint/nolintlint_test.go +++ b/pkg/golinters/nolintlint/nolintlint_test.go @@ -102,7 +102,7 @@ func foo() { }, }, { - desc: "spaces are allowed in comma-separated list of linters", + desc: "spaces are not allowed in comma-separated list of linters", contents: ` package bar From 29dda05413c1dced3f072a6ef9000a6f45adc210 Mon Sep 17 00:00:00 2001 From: Aaron Pradhan <12a.pradhan@gmail.com> Date: Tue, 19 Mar 2024 17:30:48 -0400 Subject: [PATCH 07/10] Update NoExplanation details to provide a generic directive template rather than a specific suggestion; update unit tests accordingly --- pkg/golinters/nolintlint/nolintlint.go | 4 ++-- pkg/golinters/nolintlint/nolintlint_test.go | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/golinters/nolintlint/nolintlint.go b/pkg/golinters/nolintlint/nolintlint.go index 410db5ab94e9..b746e6f3b2bc 100644 --- a/pkg/golinters/nolintlint/nolintlint.go +++ b/pkg/golinters/nolintlint/nolintlint.go @@ -64,8 +64,8 @@ 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) + return fmt.Sprintf("directive `%s` is missing an explanation; it should follow the format `//nolint[:] // `", + i.fullDirective) } func (i NoExplanation) String() string { return toString(i) } diff --git a/pkg/golinters/nolintlint/nolintlint_test.go b/pkg/golinters/nolintlint/nolintlint_test.go index 895127204a80..2daee30c81b6 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"}, }, }, { From 476902e107464c3b3eafb785939f2b0bea7d2ed3 Mon Sep 17 00:00:00 2001 From: Aaron Pradhan <12a.pradhan@gmail.com> Date: Thu, 21 Mar 2024 10:40:30 -0400 Subject: [PATCH 08/10] Make the ParseError issue more liberal to allow spaces in the linter list and allow an arbitrary number of spaces thereafter --- pkg/golinters/nolintlint/nolintlint.go | 2 +- pkg/golinters/nolintlint/nolintlint_test.go | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/pkg/golinters/nolintlint/nolintlint.go b/pkg/golinters/nolintlint/nolintlint.go index b746e6f3b2bc..8c0e250f8615 100644 --- a/pkg/golinters/nolintlint/nolintlint.go +++ b/pkg/golinters/nolintlint/nolintlint.go @@ -113,7 +113,7 @@ const ( var commentPattern = regexp.MustCompile(`^//\s*(nolint)(:\s*[\w-]+\s*(?:,\s*[\w-]+\s*)*)?\b`) // matches a complete nolint directive -var fullDirectivePattern = regexp.MustCompile(`^//nolint(?::([\w-]+(?:,[\w-]+)*))?(?: (//.*))?\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 diff --git a/pkg/golinters/nolintlint/nolintlint_test.go b/pkg/golinters/nolintlint/nolintlint_test.go index 2daee30c81b6..19ebcda8a7c7 100644 --- a/pkg/golinters/nolintlint/nolintlint_test.go +++ b/pkg/golinters/nolintlint/nolintlint_test.go @@ -102,22 +102,20 @@ func foo() { }, }, { - desc: "spaces are not allowed in comma-separated list of linters", + desc: "spaces are allowed in comma-separated list of linters", contents: ` package bar func foo() { good() //nolint:linter1,linter-two bad() //nolint:linter1 linter2 - bad() //nolint: linter1,linter2 - 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: linter1,linter2` should match `//nolint[:] [// ]` at testing.go:7:9"}, - {issue: "directive `//nolint: linter1, linter2` should match `//nolint[:] [// ]` at testing.go:8: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"}, }, From a17bca8d1954112e3d916be02a7428fba3442244 Mon Sep 17 00:00:00 2001 From: Aaron Pradhan <12a.pradhan@gmail.com> Date: Thu, 21 Mar 2024 10:41:27 -0400 Subject: [PATCH 09/10] Export BaseIssue struct --- pkg/golinters/nolintlint/nolintlint.go | 28 +++++++++++++------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/pkg/golinters/nolintlint/nolintlint.go b/pkg/golinters/nolintlint/nolintlint.go index 8c0e250f8615..b6d9f145fa03 100644 --- a/pkg/golinters/nolintlint/nolintlint.go +++ b/pkg/golinters/nolintlint/nolintlint.go @@ -11,22 +11,22 @@ import ( "github.com/golangci/golangci-lint/pkg/result" ) -type baseIssue struct { +type BaseIssue struct { fullDirective string position token.Position replacement *result.Replacement } -func (b baseIssue) Position() token.Position { +func (b BaseIssue) Position() token.Position { return b.position } -func (b baseIssue) Replacement() *result.Replacement { +func (b BaseIssue) Replacement() *result.Replacement { return b.replacement } type ExtraLeadingSpace struct { - baseIssue + BaseIssue } func (i ExtraLeadingSpace) Details() string { @@ -36,7 +36,7 @@ func (i ExtraLeadingSpace) Details() string { func (i ExtraLeadingSpace) String() string { return toString(i) } type NotSpecific struct { - baseIssue + BaseIssue } func (i NotSpecific) Details() string { @@ -47,7 +47,7 @@ func (i NotSpecific) Details() string { func (i NotSpecific) String() string { return toString(i) } type ParseError struct { - baseIssue + BaseIssue } func (i ParseError) Details() string { @@ -58,7 +58,7 @@ func (i ParseError) Details() string { func (i ParseError) String() string { return toString(i) } type NoExplanation struct { - baseIssue + BaseIssue fullDirectiveWithoutExplanation string } @@ -71,7 +71,7 @@ func (i NoExplanation) Details() string { func (i NoExplanation) String() string { return toString(i) } type UnusedCandidate struct { - baseIssue + BaseIssue ExpectedLinter string } @@ -165,14 +165,14 @@ func (l Linter) Run(fset *token.FileSet, nodes ...ast.Node) ([]Issue, error) { pos := fset.Position(comment.Pos()) end := fset.Position(comment.End()) - base := baseIssue{ + base := BaseIssue{ fullDirective: comment.Text, position: pos, } fullMatches := fullDirectivePattern.FindStringSubmatch(comment.Text) if len(fullMatches) == 0 { - issues = append(issues, ParseError{baseIssue: base}) + issues = append(issues, ParseError{BaseIssue: base}) continue } @@ -198,7 +198,7 @@ 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}) + issues = append(issues, NotSpecific{BaseIssue: base}) } } @@ -213,12 +213,12 @@ func (l Linter) Run(fset *token.FileSet, nodes ...ast.Node) ([]Issue, error) { } if len(linters) == 0 { - issue := UnusedCandidate{baseIssue: base} + issue := UnusedCandidate{BaseIssue: base} issue.replacement = removeNolintCompletely issues = append(issues, issue) } else { for _, linter := range linters { - issue := UnusedCandidate{baseIssue: base, ExpectedLinter: linter} + issue := UnusedCandidate{BaseIssue: base, ExpectedLinter: linter} // only offer replacement if there is a single linter // because of issues around commas and the possibility of all // linters being removed @@ -243,7 +243,7 @@ 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, + BaseIssue: base, fullDirectiveWithoutExplanation: fullDirectiveWithoutExplanation, }) } From acc782377034cd2c983c86acfef80006fcfd96ea Mon Sep 17 00:00:00 2001 From: Aaron Pradhan <12a.pradhan@gmail.com> Date: Thu, 21 Mar 2024 11:01:29 -0400 Subject: [PATCH 10/10] Split string literal in NoExplanation issue details --- pkg/golinters/nolintlint/nolintlint.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/golinters/nolintlint/nolintlint.go b/pkg/golinters/nolintlint/nolintlint.go index b6d9f145fa03..82678106239b 100644 --- a/pkg/golinters/nolintlint/nolintlint.go +++ b/pkg/golinters/nolintlint/nolintlint.go @@ -64,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` is missing an explanation; it should follow the format `//nolint[:] // `", - i.fullDirective) + 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) }