feat: improve formatter messages (golangci#5243)

ldez · web-flow · commit f996a66dc94e · 2024-12-24T12:47:28.000+01:00
diff --git a/pkg/golinters/gci/internal/analyzer.go b/pkg/golinters/gci/internal/analyzer.go
@@ -108,7 +108,7 @@ func runAnalysis(pass *analysis.Pass) (any, error) {
 
 		pass.Report(analysis.Diagnostic{
 			Pos:            fix.TextEdits[0].Pos,
-			Message:        "Invalid import order",
+			Message:        "File is not properly formatted",
 			SuggestedFixes: []analysis.SuggestedFix{*fix},
 		})
 	}
diff --git a/pkg/golinters/gci/testdata/gci.go b/pkg/golinters/gci/testdata/gci.go
@@ -2,7 +2,7 @@
 //golangcitest:config_path testdata/gci.yml
 package testdata
 
-// want +1 "Invalid import order"
+// want +1 "File is not properly formatted"
 import (
 	"golang.org/x/tools/go/analysis"
 	"github.com/golangci/golangci-lint/pkg/config"
diff --git a/pkg/golinters/gci/testdata/gci_cgo.go b/pkg/golinters/gci/testdata/gci_cgo.go
@@ -16,7 +16,7 @@ package testdata
 */
 import "C"
 
-// want +1 "Invalid import order"
+// want +1 "File is not properly formatted"
 import (
 	"golang.org/x/tools/go/analysis"
 	"github.com/golangci/golangci-lint/pkg/config"
diff --git a/pkg/golinters/gofmt/gofmt.go b/pkg/golinters/gofmt/gofmt.go
@@ -56,23 +56,11 @@ func runGofmt(lintCtx *linter.Context, pass *analysis.Pass, settings *config.GoF
 			continue
 		}
 
-		err = internal.ExtractDiagnosticFromPatch(pass, file, string(diff), lintCtx, getIssuedTextGoFmt)
+		err = internal.ExtractDiagnosticFromPatch(pass, file, string(diff), lintCtx)
 		if err != nil {
 			return fmt.Errorf("can't extract issues from gofmt diff output %q: %w", string(diff), err)
 		}
 	}
 
 	return nil
 }
-
-func getIssuedTextGoFmt(settings *config.LintersSettings) string {
-	text := "File is not `gofmt`-ed"
-	if settings.Gofmt.Simplify {
-		text += " with `-s`"
-	}
-	for _, rule := range settings.Gofmt.RewriteRules {
-		text += fmt.Sprintf(" `-r '%s -> %s'`", rule.Pattern, rule.Replacement)
-	}
-
-	return text
-}
diff --git a/pkg/golinters/gofmt/testdata/gofmt.go b/pkg/golinters/gofmt/testdata/gofmt.go
@@ -5,5 +5,5 @@ import "fmt"
 
 func GofmtNotSimplified() {
 	var x []string
-	fmt.Print(x[1:len(x)]) // want "File is not `gofmt`-ed with `-s`"
+	fmt.Print(x[1:len(x)]) // want "File is not properly formatted"
 }
diff --git a/pkg/golinters/gofmt/testdata/gofmt_cgo.go b/pkg/golinters/gofmt/testdata/gofmt_cgo.go
@@ -24,5 +24,5 @@ func _() {
 
 func GofmtNotSimplified() {
 	var x []string
-	fmt.Print(x[1:len(x)]) // want "File is not `gofmt`-ed with `-s`"
+	fmt.Print(x[1:len(x)]) // want "File is not properly formatted"
 }
diff --git a/pkg/golinters/gofmt/testdata/gofmt_no_simplify.go b/pkg/golinters/gofmt/testdata/gofmt_no_simplify.go
@@ -9,5 +9,5 @@ func GofmtNotSimplifiedOk() {
 	fmt.Print(x[1:len(x)])
 }
 
-func GofmtBadFormat(){  // want "^File is not `gofmt`-ed"
+func GofmtBadFormat(){  // want "File is not properly formatted"
 }
diff --git a/pkg/golinters/gofmt/testdata/gofmt_rewrite_rules.go b/pkg/golinters/gofmt/testdata/gofmt_rewrite_rules.go
@@ -11,7 +11,7 @@ func GofmtRewriteRule() {
 	vals = append(vals, 2)
 	vals = append(vals, 3)
 
-	slice := vals[1:len(vals)] // want "^File is not `gofmt`-ed"
+	slice := vals[1:len(vals)] // want "File is not properly formatted"
 
 	fmt.Println(slice)
 }
diff --git a/pkg/golinters/gofumpt/gofumpt.go b/pkg/golinters/gofumpt/gofumpt.go
@@ -83,7 +83,7 @@ func runGofumpt(lintCtx *linter.Context, pass *analysis.Pass, diff differ, optio
 
 			diff := out.String()
 
-			err = internal.ExtractDiagnosticFromPatch(pass, file, diff, lintCtx, getIssuedTextGoFumpt)
+			err = internal.ExtractDiagnosticFromPatch(pass, file, diff, lintCtx)
 			if err != nil {
 				return fmt.Errorf("can't extract issues from gofumpt diff output %q: %w", diff, err)
 			}
@@ -101,13 +101,3 @@ func getLangVersion(settings *config.GofumptSettings) string {
 
 	return "go" + strings.TrimPrefix(settings.LangVersion, "go")
 }
-
-func getIssuedTextGoFumpt(settings *config.LintersSettings) string {
-	text := "File is not `gofumpt`-ed"
-
-	if settings.Gofumpt.ExtraRules {
-		text += " with `-extra`"
-	}
-
-	return text
-}
diff --git a/pkg/golinters/gofumpt/testdata/gofumpt.go b/pkg/golinters/gofumpt/testdata/gofumpt.go
@@ -4,5 +4,5 @@ package testdata
 import "fmt"
 
 func GofumptNewLine() {
-	fmt.Println( "foo" ) // want "File is not `gofumpt`-ed"
+	fmt.Println( "foo" ) // want "File is not properly formatted"
 }
diff --git a/pkg/golinters/gofumpt/testdata/gofumpt_cgo.go b/pkg/golinters/gofumpt/testdata/gofumpt_cgo.go
@@ -23,5 +23,5 @@ func _() {
 }
 
 func GofumptNewLine() {
-	fmt.Println( "foo" ) // want "File is not `gofumpt`-ed"
+	fmt.Println( "foo" ) // want "File is not properly formatted"
 }
diff --git a/pkg/golinters/gofumpt/testdata/gofumpt_with_extra.go b/pkg/golinters/gofumpt/testdata/gofumpt_with_extra.go
@@ -4,6 +4,6 @@ package testdata
 
 import "fmt"
 
-func GofmtNotExtra(bar string, baz string) { // want "File is not `gofumpt`-ed with `-extra`"
+func GofmtNotExtra(bar string, baz string) { // want "File is not properly formatted"
 	fmt.Print("foo")
 }
diff --git a/pkg/golinters/goimports/goimports.go b/pkg/golinters/goimports/goimports.go
@@ -54,21 +54,11 @@ func runGoImports(lintCtx *linter.Context, pass *analysis.Pass) error {
 			continue
 		}
 
-		err = internal.ExtractDiagnosticFromPatch(pass, file, string(diff), lintCtx, getIssuedTextGoImports)
+		err = internal.ExtractDiagnosticFromPatch(pass, file, string(diff), lintCtx)
 		if err != nil {
 			return fmt.Errorf("can't extract issues from gofmt diff output %q: %w", string(diff), err)
 		}
 	}
 
 	return nil
 }
-
-func getIssuedTextGoImports(settings *config.LintersSettings) string {
-	text := "File is not `goimports`-ed"
-
-	if settings.Goimports.LocalPrefixes != "" {
-		text += " with -local " + settings.Goimports.LocalPrefixes
-	}
-
-	return text
-}
diff --git a/pkg/golinters/goimports/testdata/goimports.go b/pkg/golinters/goimports/testdata/goimports.go
@@ -2,7 +2,7 @@
 package testdata
 
 import (
-	"fmt" // want "File is not `goimports`-ed"
+	"fmt" // want "File is not properly formatted"
 	"github.com/golangci/golangci-lint/pkg/config"
 )
 
diff --git a/pkg/golinters/goimports/testdata/goimports_cgo.go b/pkg/golinters/goimports/testdata/goimports_cgo.go
@@ -13,7 +13,7 @@ import "C"
 
 import (
 	"fmt"
-	"unsafe" // want "File is not `goimports`-ed"
+	"unsafe" // want "File is not properly formatted"
 	"github.com/golangci/golangci-lint/pkg/config"
 )
 
diff --git a/pkg/golinters/goimports/testdata/goimports_local.go b/pkg/golinters/goimports/testdata/goimports_local.go
@@ -5,7 +5,7 @@ package testdata
 import (
 	"fmt"
 
-	"github.com/golangci/golangci-lint/pkg/config" // want "File is not `goimports`-ed with -local github.com/golangci/golangci-lint"
+	"github.com/golangci/golangci-lint/pkg/config" // want "File is not properly formatted"
 	"golang.org/x/tools/go/analysis"
 )
 
diff --git a/pkg/golinters/internal/diff.go b/pkg/golinters/internal/diff.go
@@ -11,7 +11,6 @@ import (
 	diffpkg "github.com/sourcegraph/go-diff/diff"
 	"golang.org/x/tools/go/analysis"
 
-	"github.com/golangci/golangci-lint/pkg/config"
 	"github.com/golangci/golangci-lint/pkg/goanalysis"
 	"github.com/golangci/golangci-lint/pkg/lint/linter"
 	"github.com/golangci/golangci-lint/pkg/logutils"
@@ -30,8 +29,6 @@ const (
 	diffLineDeleted  diffLineType = "deleted"
 )
 
-type fmtTextFormatter func(settings *config.LintersSettings) string
-
 type diffLine struct {
 	originalNumber int // 1-based original line number
 	typ            diffLineType
@@ -219,7 +216,6 @@ func ExtractDiagnosticFromPatch(
 	file *ast.File,
 	patch string,
 	lintCtx *linter.Context,
-	formatter fmtTextFormatter,
 ) error {
 	diffs, err := diffpkg.ParseMultiFileDiff([]byte(patch))
 	if err != nil {
@@ -246,23 +242,23 @@ func ExtractDiagnosticFromPatch(
 			changes := p.parse(hunk)
 
 			for _, change := range changes {
-				pass.Report(toDiagnostic(ft, change, formatter(lintCtx.Settings()), adjLine))
+				pass.Report(toDiagnostic(ft, change, adjLine))
 			}
 		}
 	}
 
 	return nil
 }
 
-func toDiagnostic(ft *token.File, change Change, message string, adjLine int) analysis.Diagnostic {
+func toDiagnostic(ft *token.File, change Change, adjLine int) analysis.Diagnostic {
 	start := ft.LineStart(change.From + adjLine)
 
 	end := goanalysis.EndOfLinePos(ft, change.To+adjLine)
 
 	return analysis.Diagnostic{
 		Pos:     start,
 		End:     end,
-		Message: message, // TODO(ldez) change message formatter to have a better message.
+		Message: "File is not properly formatted",
 		SuggestedFixes: []analysis.SuggestedFix{{
 			TextEdits: []analysis.TextEdit{{
 				Pos:     start,
diff --git a/test/run_test.go b/test/run_test.go
@@ -133,7 +133,7 @@ func TestCgoWithIssues(t *testing.T) {
 			desc:     "gofmt",
 			args:     []string{"--no-config", "--disable-all", "-Egofmt"},
 			dir:      "cgo_with_issues",
-			expected: "File is not `gofmt`-ed with `-s` (gofmt)",
+			expected: "File is not properly formatted (gofmt)",
 		},
 		{
 			desc:     "revive",
@@ -186,7 +186,7 @@ func TestLineDirective(t *testing.T) {
 				"--disable-all",
 			},
 			targetPath: "linedirective",
-			expected:   "File is not `gofmt`-ed with `-s` (gofmt)",
+			expected:   "File is not properly formatted (gofmt)",
 		},
 		{
 			desc: "goimports",
@@ -195,7 +195,7 @@ func TestLineDirective(t *testing.T) {
 				"--disable-all",
 			},
 			targetPath: "linedirective",
-			expected:   "File is not `goimports`-ed (goimports)",
+			expected:   "File is not properly formatted (goimports)",
 		},
 		{
 			desc: "gomodguard",

Original file line number	Diff line number	Diff line change
`@@ -108,7 +108,7 @@ func runAnalysis(pass *analysis.Pass) (any, error) {`
`108`	`108`
`109`	`109`	`pass.Report(analysis.Diagnostic{`
`110`	`110`	`Pos: fix.TextEdits[0].Pos,`
`111`		`- Message: "Invalid import order",`
	`111`	`+ Message: "File is not properly formatted",`
`112`	`112`	`SuggestedFixes: []analysis.SuggestedFix{*fix},`
`113`	`113`	`})`
`114`	`114`	`}`
Original file line number	Diff line number	Diff line change
`@@ -56,23 +56,11 @@ func runGofmt(lintCtx linter.Context, pass analysis.Pass, settings *config.GoF`
`56`	`56`	`continue`
`57`	`57`	`}`
`58`	`58`
`59`		`- err = internal.ExtractDiagnosticFromPatch(pass, file, string(diff), lintCtx, getIssuedTextGoFmt)`
	`59`	`+ err = internal.ExtractDiagnosticFromPatch(pass, file, string(diff), lintCtx)`
`60`	`60`	`if err != nil {`
`61`	`61`	`return fmt.Errorf("can't extract issues from gofmt diff output %q: %w", string(diff), err)`
`62`	`62`	`}`
`63`	`63`	`}`
`64`	`64`
`65`	`65`	`return nil`
`66`	`66`	`}`
`67`		`-`
`68`		`-func getIssuedTextGoFmt(settings *config.LintersSettings) string {`
`69`		- text := "File is not `gofmt`-ed"
`70`		`- if settings.Gofmt.Simplify {`
`71`		- text += " with `-s`"
`72`		`- }`
`73`		`- for _, rule := range settings.Gofmt.RewriteRules {`
`74`		- text += fmt.Sprintf(" `-r '%s -> %s'`", rule.Pattern, rule.Replacement)
`75`		`- }`
`76`		`-`
`77`		`- return text`
`78`		`-}`
Original file line number	Diff line number	Diff line change
`@@ -5,5 +5,5 @@ import "fmt"`
`5`	`5`
`6`	`6`	`func GofmtNotSimplified() {`
`7`	`7`	`var x []string`
`8`		- fmt.Print(x[1:len(x)]) // want "File is not `gofmt`-ed with `-s`"
	`8`	`+ fmt.Print(x[1:len(x)]) // want "File is not properly formatted"`
`9`	`9`	`}`
Original file line number	Diff line number	Diff line change
`@@ -24,5 +24,5 @@ func _() {`
`24`	`24`
`25`	`25`	`func GofmtNotSimplified() {`
`26`	`26`	`var x []string`
`27`		- fmt.Print(x[1:len(x)]) // want "File is not `gofmt`-ed with `-s`"
	`27`	`+ fmt.Print(x[1:len(x)]) // want "File is not properly formatted"`
`28`	`28`	`}`
Original file line number	Diff line number	Diff line change
`@@ -9,5 +9,5 @@ func GofmtNotSimplifiedOk() {`
`9`	`9`	`fmt.Print(x[1:len(x)])`
`10`	`10`	`}`
`11`	`11`
`12`		-func GofmtBadFormat(){ // want "^File is not `gofmt`-ed"
	`12`	`+func GofmtBadFormat(){ // want "File is not properly formatted"`
`13`	`13`	`}`
Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,7 @@ func GofmtRewriteRule() {`
`11`	`11`	`vals = append(vals, 2)`
`12`	`12`	`vals = append(vals, 3)`
`13`	`13`
`14`		- slice := vals[1:len(vals)] // want "^File is not `gofmt`-ed"
	`14`	`+ slice := vals[1:len(vals)] // want "File is not properly formatted"`
`15`	`15`
`16`	`16`	`fmt.Println(slice)`
`17`	`17`	`}`
Original file line number	Diff line number	Diff line change
`@@ -4,5 +4,5 @@ package testdata`
`4`	`4`	`import "fmt"`
`5`	`5`
`6`	`6`	`func GofumptNewLine() {`
`7`		- fmt.Println( "foo" ) // want "File is not `gofumpt`-ed"
	`7`	`+ fmt.Println( "foo" ) // want "File is not properly formatted"`
`8`	`8`	`}`
Original file line number	Diff line number	Diff line change
`@@ -23,5 +23,5 @@ func _() {`
`23`	`23`	`}`
`24`	`24`
`25`	`25`	`func GofumptNewLine() {`
`26`		- fmt.Println( "foo" ) // want "File is not `gofumpt`-ed"
	`26`	`+ fmt.Println( "foo" ) // want "File is not properly formatted"`
`27`	`27`	`}`
Original file line number	Diff line number	Diff line change
`@@ -4,6 +4,6 @@ package testdata`
`4`	`4`
`5`	`5`	`import "fmt"`
`6`	`6`
`7`		-func GofmtNotExtra(bar string, baz string) { // want "File is not `gofumpt`-ed with `-extra`"
	`7`	`+func GofmtNotExtra(bar string, baz string) { // want "File is not properly formatted"`
`8`	`8`	`fmt.Print("foo")`
`9`	`9`	`}`
Original file line number	Diff line number	Diff line change
`@@ -54,21 +54,11 @@ func runGoImports(lintCtx linter.Context, pass analysis.Pass) error {`
`54`	`54`	`continue`
`55`	`55`	`}`
`56`	`56`
`57`		`- err = internal.ExtractDiagnosticFromPatch(pass, file, string(diff), lintCtx, getIssuedTextGoImports)`
	`57`	`+ err = internal.ExtractDiagnosticFromPatch(pass, file, string(diff), lintCtx)`
`58`	`58`	`if err != nil {`
`59`	`59`	`return fmt.Errorf("can't extract issues from gofmt diff output %q: %w", string(diff), err)`
`60`	`60`	`}`
`61`	`61`	`}`
`62`	`62`
`63`	`63`	`return nil`
`64`	`64`	`}`
`65`		`-`
`66`		`-func getIssuedTextGoImports(settings *config.LintersSettings) string {`
`67`		- text := "File is not `goimports`-ed"
`68`		`-`
`69`		`- if settings.Goimports.LocalPrefixes != "" {`
`70`		`- text += " with -local " + settings.Goimports.LocalPrefixes`
`71`		`- }`
`72`		`-`
`73`		`- return text`
`74`		`-}`