Simplify reports

The current report may be a bit confusing. if there are more than one fix suggestion, there will be multiple reports, where the first one contain only partial fix, and only the last one contains the final fix. To solve it, for each expression, there will be only one report, containing all the errors in the text, and only one fix suggestion, containing all the fixes.
nunnatsa · Feb 13, 2024 · fe8cba1 · fe8cba1
1 parent 1cb550c
commit fe8cba1
Show file tree

Hide file tree

Showing 55 changed files with 1,918 additions and 1,766 deletions.
diff --git a/linter/ginkgo_linter.go b/linter/ginkgo_linter.go
diff --git a/reports/report-builder.go b/reports/report-builder.go
@@ -0,0 +1,97 @@
+package reports
+
+import (
+	"bytes"
+	"fmt"
+	"go/ast"
+	"go/printer"
+	"go/token"
+	"golang.org/x/tools/go/analysis"
+	"strings"
+)
+
+type Builder struct {
+	pos        token.Pos
+	end        token.Pos
+	oldExpr    string
+	issues     []string
+	fixOffer   string
+	suggestFix bool
+}
+
+func NewBuilder(fset *token.FileSet, oldExpr ast.Expr) *Builder {
+	b := &Builder{
+		pos:        oldExpr.Pos(),
+		end:        oldExpr.End(),
+		oldExpr:    goFmt(fset, oldExpr),
+		suggestFix: false,
+	}
+
+	return b
+}
+
+func (b *Builder) AddIssue(suggestFix bool, issue string, args ...any) {
+	if len(args) > 0 {
+		issue = fmt.Sprintf(issue, args...)
+	}
+	b.issues = append(b.issues, issue)
+
+	if suggestFix {
+		b.suggestFix = true
+	}
+}
+
+func (b *Builder) SetFixOffer(fset *token.FileSet, fixOffer ast.Expr) {
+	if offer := goFmt(fset, fixOffer); offer != b.oldExpr {
+		b.fixOffer = offer
+	}
+}
+
+func (b *Builder) HasReport() bool {
+	return len(b.issues) > 0
+}
+
+func (b *Builder) Build() analysis.Diagnostic {
+	diagnostic := analysis.Diagnostic{
+		Pos:     b.pos,
+		Message: b.getMessage(),
+	}
+
+	if b.suggestFix && len(b.fixOffer) > 0 {
+		diagnostic.SuggestedFixes = []analysis.SuggestedFix{
+			{
+				Message: fmt.Sprintf("should replace %s with %s", b.oldExpr, b.fixOffer),
+				TextEdits: []analysis.TextEdit{
+					{
+						Pos:     b.pos,
+						End:     b.end,
+						NewText: []byte(b.fixOffer),
+					},
+				},
+			},
+		}
+	}
+
+	return diagnostic
+}
+
+func goFmt(fset *token.FileSet, x ast.Expr) string {
+	var b bytes.Buffer
+	_ = printer.Fprint(&b, fset, x)
+	return b.String()
+}
+
+func (b *Builder) getMessage() string {
+	sb := strings.Builder{}
+	sb.WriteString("ginkgo-linter: ")
+	if len(b.issues) > 1 {
+		sb.WriteString("multiple issues: ")
+	}
+	sb.WriteString(strings.Join(b.issues, "; "))
+
+	if b.suggestFix && len(b.fixOffer) != 0 {
+		sb.WriteString(fmt.Sprintf(". Consider using `%s` instead", b.fixOffer))
+	}
+
+	return sb.String()
+}
diff --git a/testdata/src/a/asyncconfig/asyncConfig.go b/testdata/src/a/asyncconfig/asyncConfig.go
@@ -24,6 +24,6 @@ var _ = Describe("test async suppress configuration", func() {
 		Eventually(slowInt()).WithPolling(time.Millisecond * 100).WithTimeout(time.Second * 2).Should(Equal(42))
 	})
 	It("should trigger comparison waring", func() {
-		Eventually(slowBool()).WithPolling(time.Millisecond * 100).WithTimeout(time.Second * 2).Should(Equal(true)) // want `ginkgo-linter: wrong boolean assertion; consider using .Eventually\(slowBool\(\)\)\.WithPolling\(time\.Millisecond \* 100\)\.WithTimeout\(time\.Second \* 2\)\.Should\(BeTrue\(\)\). instead`
+		Eventually(slowBool()).WithPolling(time.Millisecond * 100).WithTimeout(time.Second * 2).Should(Equal(true)) // want `ginkgo-linter: wrong boolean assertion\. Consider using .Eventually\(slowBool\(\)\)\.WithPolling\(time\.Millisecond \* 100\)\.WithTimeout\(time\.Second \* 2\)\.Should\(BeTrue\(\)\). instead`
 	})
 })
diff --git a/testdata/src/a/asyncconfig/asyncConfig.gomega.go b/testdata/src/a/asyncconfig/asyncConfig.gomega.go
@@ -14,6 +14,6 @@ var _ = Describe("test async suppress configuration", func() {
 		g.Eventually(slowInt()).WithPolling(time.Millisecond * 100).WithTimeout(time.Second * 2).Should(g.Equal(42))
 	})
 	It("should trigger comparison waring", func() {
-		g.Eventually(slowBool()).WithPolling(time.Millisecond * 100).WithTimeout(time.Second * 2).Should(g.Equal(true)) // want `ginkgo-linter: wrong boolean assertion; consider using .g\.Eventually\(slowBool\(\)\)\.WithPolling\(time\.Millisecond \* 100\)\.WithTimeout\(time\.Second \* 2\)\.Should\(g\.BeTrue\(\)\). instead`
+		g.Eventually(slowBool()).WithPolling(time.Millisecond * 100).WithTimeout(time.Second * 2).Should(g.Equal(true)) // want `ginkgo-linter: wrong boolean assertion\. Consider using .g\.Eventually\(slowBool\(\)\)\.WithPolling\(time\.Millisecond \* 100\)\.WithTimeout\(time\.Second \* 2\)\.Should\(g\.BeTrue\(\)\). instead`
 	})
 })
diff --git a/testdata/src/a/boolean/bool.go b/testdata/src/a/boolean/bool.go
@@ -10,18 +10,18 @@ var _ = Describe("boolean warnings", func() {
 	f := false
 
 	It("check Equal(true/false)", func() {
-		Expect(t).To(Equal(true))                        // want `ginkgo-linter: wrong boolean assertion; consider using .Expect\(t\)\.To\(BeTrue\(\)\). instead`
-		Expect(f).To(Equal(false))                       // want `ginkgo-linter: wrong boolean assertion; consider using .Expect\(f\)\.To\(BeFalse\(\)\). instead`
-		ExpectWithOffset(2, t).Should(Not(Equal(false))) // want `ginkgo-linter: wrong boolean assertion; consider using .ExpectWithOffset\(2, t\)\.Should\(BeTrue\(\)\). instead`
-		ExpectWithOffset(2, t).ShouldNot(Equal(false))   // want `ginkgo-linter: wrong boolean assertion; consider using .ExpectWithOffset\(2, t\)\.Should\(BeTrue\(\)\). instead`
-		Expect(t).WithOffset(2).ToNot(Equal(false))      // want `ginkgo-linter: wrong boolean assertion; consider using .Expect\(t\)\.WithOffset\(2\)\.To\(BeTrue\(\)\). instead`
-		Expect(t).NotTo(Equal(false))                    // want `ginkgo-linter: wrong boolean assertion; consider using .Expect\(t\)\.To\(BeTrue\(\)\). instead`
-		Expect(f).WithOffset(1).ToNot(Equal(true))       // want `ginkgo-linter: wrong boolean assertion; consider using .Expect\(f\)\.WithOffset\(1\)\.ToNot\(BeTrue\(\)\). instead`
+		Expect(t).To(Equal(true))                        // want `ginkgo-linter: wrong boolean assertion\. Consider using .Expect\(t\)\.To\(BeTrue\(\)\). instead`
+		Expect(f).To(Equal(false))                       // want `ginkgo-linter: wrong boolean assertion\. Consider using .Expect\(f\)\.To\(BeFalse\(\)\). instead`
+		ExpectWithOffset(2, t).Should(Not(Equal(false))) // want `ginkgo-linter: wrong boolean assertion\. Consider using .ExpectWithOffset\(2, t\)\.Should\(BeTrue\(\)\). instead`
+		ExpectWithOffset(2, t).ShouldNot(Equal(false))   // want `ginkgo-linter: wrong boolean assertion\. Consider using .ExpectWithOffset\(2, t\)\.Should\(BeTrue\(\)\). instead`
+		Expect(t).WithOffset(2).ToNot(Equal(false))      // want `ginkgo-linter: wrong boolean assertion\. Consider using .Expect\(t\)\.WithOffset\(2\)\.To\(BeTrue\(\)\). instead`
+		Expect(t).NotTo(Equal(false))                    // want `ginkgo-linter: wrong boolean assertion\. Consider using .Expect\(t\)\.To\(BeTrue\(\)\). instead`
+		Expect(f).WithOffset(1).ToNot(Equal(true))       // want `ginkgo-linter: wrong boolean assertion\. Consider using .Expect\(f\)\.WithOffset\(1\)\.ToNot\(BeTrue\(\)\). instead`
 	})
 
 	It("check double negative", func() {
-		Expect(t).ToNot(BeFalse())                   // want `ginkgo-linter: avoid double negative assertion; consider using .Expect\(t\)\.To\(BeTrue\(\)\). instead`
-		Expect(t).NotTo(BeFalse())                   // want `ginkgo-linter: avoid double negative assertion; consider using .Expect\(t\)\.To\(BeTrue\(\)\). instead`
-		Expect(t).WithOffset(2).ShouldNot(BeFalse()) // want `ginkgo-linter: avoid double negative assertion; consider using .Expect\(t\)\.WithOffset\(2\)\.Should\(BeTrue\(\)\). instead`
+		Expect(t).ToNot(BeFalse())                   // want `ginkgo-linter: avoid double negative assertion\. Consider using .Expect\(t\)\.To\(BeTrue\(\)\). instead`
+		Expect(t).NotTo(BeFalse())                   // want `ginkgo-linter: avoid double negative assertion\. Consider using .Expect\(t\)\.To\(BeTrue\(\)\). instead`
+		Expect(t).WithOffset(2).ShouldNot(BeFalse()) // want `ginkgo-linter: avoid double negative assertion\. Consider using .Expect\(t\)\.WithOffset\(2\)\.Should\(BeTrue\(\)\). instead`
 	})
 })
diff --git a/testdata/src/a/comparetypes/comparetypes.gomega_test.go b/testdata/src/a/comparetypes/comparetypes.gomega_test.go
@@ -43,17 +43,17 @@ var _ = Describe("compare different types", func() {
 		gomega.Expect(a).ShouldNot(gomega.Equal(5))
 		gomega.Expect(a).ShouldNot(gomega.Equal(uint(5)))           // want `ginkgo-linter: use Equal with different types: Comparing int with uint; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
 		gomega.Expect(fint64()).ShouldNot(gomega.Equal(uint64(5)))  // want `ginkgo-linter: use Equal with different types: Comparing int64 with uint64; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
-		gomega.Expect(fmytype()).ShouldNot(gomega.Equal(uint64(5))) // want `ginkgo-linter: use Equal with different types: Comparing a/comparetypes_test.mytype with uint64; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
+		gomega.Expect(fmytype()).ShouldNot(gomega.Equal(uint64(5))) // want `ginkgo-linter: use Equal with different types: Comparing a/comparetypes_test\.mytype with uint64; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
 		gomega.Expect(a).ShouldNot(gomega.Equal(int32(5)))          // want `ginkgo-linter: use Equal with different types: Comparing int with int32; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
 		gomega.Expect(a).ShouldNot(gomega.Equal(uint8(5)))          // want `ginkgo-linter: use Equal with different types: Comparing int with uint8; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
-		gomega.Expect(a).ShouldNot(gomega.Equal(mytype(5)))         // want `ginkgo-linter: use Equal with different types: Comparing int with a/comparetypes_test.mytype; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
-		gomega.Expect(5).ShouldNot(gomega.Equal(mytype(5)))         // want `ginkgo-linter: use Equal with different types: Comparing int with a/comparetypes_test.mytype; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
+		gomega.Expect(a).ShouldNot(gomega.Equal(mytype(5)))         // want `ginkgo-linter: use Equal with different types: Comparing int with a/comparetypes_test\.mytype; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
+		gomega.Expect(5).ShouldNot(gomega.Equal(mytype(5)))         // want `ginkgo-linter: use Equal with different types: Comparing int with a/comparetypes_test\.mytype; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
 
 		b := int16(5)
 		gomega.Expect(a).ShouldNot(gomega.Equal(b)) // want `ginkgo-linter: use Equal with different types: Comparing int with int16; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
 
 		c := mytype(5)
-		gomega.Expect(a).ShouldNot(gomega.Equal(c)) // want `ginkgo-linter: use Equal with different types: Comparing int with a/comparetypes_test.mytype; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
+		gomega.Expect(a).ShouldNot(gomega.Equal(c)) // want `ginkgo-linter: use Equal with different types: Comparing int with a/comparetypes_test\.mytype; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
 	})
 
 	It("compare interfaces", func() {
@@ -72,8 +72,8 @@ var _ = Describe("compare different types", func() {
 
 		gomega.Expect(pa).Should(gomega.HaveValue(gomega.Equal(uint64(5))))                                                                // want `ginkgo-linter: use Equal with different types: Comparing int with uint64; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
 		gomega.Expect(a).Should(gomega.Not(gomega.Equal(uint64(5))))                                                                       // want `ginkgo-linter: use Equal with different types: Comparing int with uint64; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
-		gomega.Expect(a).Should(gomega.And(gomega.Equal(uint64(5)), gomega.Not(gomega.Equal(int32(6)))))                                   // want `ginkgo-linter: use Equal with different types: Comparing int with uint64; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)` `ginkgo-linter: use Equal with different types: Comparing int with int32; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
-		gomega.Expect(a).Should(gomega.Or(gomega.Equal(uint64(5)), gomega.Not(gomega.Equal(int32(6))), gomega.Not(gomega.Equal(int8(4))))) // want `ginkgo-linter: use Equal with different types: Comparing int with uint64; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)` `ginkgo-linter: use Equal with different types: Comparing int with int32; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)` `ginkgo-linter: use Equal with different types: Comparing int with int8; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
+		gomega.Expect(a).Should(gomega.And(gomega.Equal(uint64(5)), gomega.Not(gomega.Equal(int32(6)))))                                   // want `ginkgo-linter: multiple issues: use Equal with different types: Comparing int with uint64; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\); use Equal with different types: Comparing int with int32; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
+		gomega.Expect(a).Should(gomega.Or(gomega.Equal(uint64(5)), gomega.Not(gomega.Equal(int32(6))), gomega.Not(gomega.Equal(int8(4))))) // want `ginkgo-linter: multiple issues: use Equal with different types: Comparing int with uint64; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\); use Equal with different types: Comparing int with int32; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\); use Equal with different types: Comparing int with int8; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
 		gomega.Expect(a).Should(gomega.WithTransform(func(i int) int { return i + 1 }, gomega.Equal(uint(6))))                             // want `ginkgo-linter: use Equal with different types: Comparing int with uint; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
 	})