From 3797ed90c38b8471c54f003ab9ac72492c1143ec Mon Sep 17 00:00:00 2001 From: "Eugene R." Date: Wed, 4 Sep 2024 15:13:37 +0300 Subject: [PATCH] nolintlint: remove empty line in unused directive replacement (#4973) Co-authored-by: Fernandez Ludovic --- .../nolintlint/internal/nolintlint.go | 15 +++++++++++---- .../nolintlint/internal/nolintlint_test.go | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/pkg/golinters/nolintlint/internal/nolintlint.go b/pkg/golinters/nolintlint/internal/nolintlint.go index 5fed41cfdfbb..08dd743783c3 100644 --- a/pkg/golinters/nolintlint/internal/nolintlint.go +++ b/pkg/golinters/nolintlint/internal/nolintlint.go @@ -252,12 +252,19 @@ func (l Linter) Run(fset *token.FileSet, nodes ...ast.Node) ([]Issue, error) { // when detecting unused directives, we send all the directives through and filter them out in the nolint processor if (l.needs & NeedsUnused) != 0 { - removeNolintCompletely := &result.Replacement{ - Inline: &result.InlineFix{ - StartCol: pos.Column - 1, + removeNolintCompletely := &result.Replacement{} + + startCol := pos.Column - 1 + + if startCol == 0 { + // if the directive starts from a new line, remove the line + removeNolintCompletely.NeedOnlyDelete = true + } else { + removeNolintCompletely.Inline = &result.InlineFix{ + StartCol: startCol, Length: end.Column - pos.Column, NewString: "", - }, + } } if len(linters) == 0 { diff --git a/pkg/golinters/nolintlint/internal/nolintlint_test.go b/pkg/golinters/nolintlint/internal/nolintlint_test.go index 0fff159285bd..ddc5bde7b1ea 100644 --- a/pkg/golinters/nolintlint/internal/nolintlint_test.go +++ b/pkg/golinters/nolintlint/internal/nolintlint_test.go @@ -188,6 +188,25 @@ func foo() { }, }, }, + { + desc: "needs unused with one specific linter in a new line generates replacement", + needs: NeedsUnused, + contents: ` +package bar + +//nolint:somelinter +func foo() { + bad() +}`, + expected: []issueWithReplacement{ + { + issue: "directive `//nolint:somelinter` is unused for linter \"somelinter\" at testing.go:4:1", + replacement: &result.Replacement{ + NeedOnlyDelete: true, + }, + }, + }, + }, { desc: "needs unused with multiple specific linters does not generate replacements", needs: NeedsUnused,