diff --git a/README.md b/README.md index 3edf065..4193be6 100644 --- a/README.md +++ b/README.md @@ -153,24 +153,35 @@ The linter will not suggest a fix for this warning. This rule cannot be suppressed. -### Focus Container Found [BUG] -This rule finds ginkgo focus containers in the code. +### Focus Container / Focus individual spec found [BUG] +This rule finds ginkgo focus containers, or the `Focus` individual spec in the code. -ginkgo supports the `FDescribe`, `FContext`, `FWhen` and `FIt` containers to allow the developer to focus +ginkgo supports the `FDescribe`, `FContext`, `FWhen`, `FIt`, `FDescribeTable` and `FEntry` +containers to allow the developer to focus on a specific test or set of tests during test development or debug. -***This rule is disabled by default***. Use the `--forbid-focus-container=true` command line flag to enable it. - For example: ```go var _ = Describe("checking something", func() { - FIt("this test is the only one that will run", func(){ - ... - }) + FIt("this test is the only one that will run", func(){ + ... + }) +}) +``` +Alternatively, the `Focus` individual spec may be used for the same purpose, e.g. +```go +var _ = Describe("checking something", Focus, func() { + It("this test is the only one that will run", func(){ + ... + }) }) ``` -These container must not be part of the final source code, and should only be used locally by the developer. +These container, or the `Focus` spec, must not be part of the final source code, and should only be used locally by the developer. + +***This rule is disabled by default***. Use the `--forbid-focus-container=true` command line flag to enable it. + + ### Wrong Length Assertion [STYLE] The linter finds assertion of the golang built-in `len` function, with all kind of matchers, while there are already gomega matchers for these usecases; We want to assert the item, rather than its length. diff --git a/ginkgo_linter.go b/ginkgo_linter.go index 1635ce4..11cffac 100644 --- a/ginkgo_linter.go +++ b/ginkgo_linter.go @@ -37,6 +37,7 @@ const ( missingAssertionMessage = linterName + `: %q: missing assertion method. Expected "Should()", "To()", "ShouldNot()", "ToNot()" or "NotTo()"` missingAsyncAssertionMessage = linterName + `: %q: missing assertion method. Expected "Should()" or "ShouldNot()"` focusContainerFound = linterName + ": Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with %q" + focusSpecFound = linterName + ": Focus spec found. This is used only for local debug and should not be part of the actual source code, consider to remove it" ) const ( // gomega matchers beEmpty = "BeEmpty" @@ -232,12 +233,27 @@ func (l *ginkgoLinter) run(pass *analysis.Pass) (interface{}, error) { } func checkFocusContainer(pass *analysis.Pass, ginkgoHndlr ginkgohandler.Handler, exp *ast.CallExpr) bool { + foundFocus := false isFocus, id := ginkgoHndlr.GetFocusContainerName(exp) if isFocus { reportNewName(pass, id, id.Name[1:], focusContainerFound, id.Name) - return true + foundFocus = true } - return false + + if id != nil && ginkgohandler.IsContainer(id) { + for _, arg := range exp.Args { + if ginkgoHndlr.IsFocusSpec(arg) { + reportNoFix(pass, arg.Pos(), focusSpecFound) + foundFocus = true + } else if callExp, ok := arg.(*ast.CallExpr); ok { + if checkFocusContainer(pass, ginkgoHndlr, callExp) { // handle table entries + foundFocus = true + } + } + } + } + + return foundFocus } func checkExpression(pass *analysis.Pass, config types.Config, assertionExp *ast.CallExpr, actualExpr *ast.CallExpr, handler gomegahandler.Handler) bool { diff --git a/ginkgohandler/handler.go b/ginkgohandler/handler.go index 87703a9..c0829c4 100644 --- a/ginkgohandler/handler.go +++ b/ginkgohandler/handler.go @@ -4,16 +4,24 @@ import ( "go/ast" ) +const ( + importPath = `"github.com/onsi/ginkgo"` + importPathV2 = `"github.com/onsi/ginkgo/v2"` + + focusSpec = "Focus" +) + // Handler provide different handling, depend on the way ginkgo was imported, whether // in imported with "." name, custom name or without any name. type Handler interface { GetFocusContainerName(*ast.CallExpr) (bool, *ast.Ident) + IsFocusSpec(ident ast.Expr) bool } // GetGinkgoHandler returns a ginkgor handler according to the way ginkgo was imported in the specific file func GetGinkgoHandler(file *ast.File) Handler { for _, imp := range file.Imports { - if imp.Path.Value != `"github.com/onsi/ginkgo"` && imp.Path.Value != `"github.com/onsi/ginkgo/v2"` { + if imp.Path.Value != importPath && imp.Path.Value != importPathV2 { continue } @@ -41,6 +49,11 @@ func (h dotHandler) GetFocusContainerName(exp *ast.CallExpr) (bool, *ast.Ident) return false, nil } +func (h dotHandler) IsFocusSpec(exp ast.Expr) bool { + id, ok := exp.(*ast.Ident) + return ok && id.Name == focusSpec +} + // nameHandler is used when importing ginkgo without name; i.e. // import "github.com/onsi/ginkgo" // @@ -57,10 +70,28 @@ func (h nameHandler) GetFocusContainerName(exp *ast.CallExpr) (bool, *ast.Ident) return false, nil } +func (h nameHandler) IsFocusSpec(exp ast.Expr) bool { + if selExp, ok := exp.(*ast.SelectorExpr); ok { + if x, ok := selExp.X.(*ast.Ident); ok && x.Name == string(h) { + return selExp.Sel.Name == focusSpec + } + } + + return false +} + func isFocusContainer(name string) bool { switch name { - case "FDescribe", "FContext", "FWhen", "FIt": + case "FDescribe", "FContext", "FWhen", "FIt", "FDescribeTable", "FEntry": return true } return false } + +func IsContainer(id *ast.Ident) bool { + switch id.Name { + case "It", "When", "Context", "Describe", "DescribeTable", "Entry": + return true + } + return isFocusContainer(id.Name) +} diff --git a/testdata/src/a/focus/focus_key_test.go b/testdata/src/a/focus/focus_key_test.go new file mode 100644 index 0000000..ed495f6 --- /dev/null +++ b/testdata/src/a/focus/focus_key_test.go @@ -0,0 +1,16 @@ +package focus + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("focused Describe", Focus, func() { + When("focused when", Focus, func() { + Context("focused Context", Focus, func() { + It("focused It", Focus, func() { + Expect(len("1234")).Should(Equal(4)) // want `ginkgo-linter: wrong length assertion; consider using .Expect\("1234"\)\.Should\(HaveLen\(4\)\). instead` + }) + }) + }) +}) diff --git a/testdata/src/a/focus/table_test.go b/testdata/src/a/focus/table_test.go new file mode 100644 index 0000000..9969cb5 --- /dev/null +++ b/testdata/src/a/focus/table_test.go @@ -0,0 +1,22 @@ +package focus + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("test focused tables", func() { + FDescribeTable("focused table", func(s []int, l int) { + Expect(len(s)).Should(Equal(l)) // want `ginkgo-linter: wrong length assertion; consider using .Expect\(s\)\.Should\(HaveLen\(l\)\). instead` + }, + Entry([]int{1, 2, 3, 4}, 4), + FEntry([]int{1, 2, 3, 4, 5}, 5), + ) + + DescribeTable("non-focused table", func(s []int, l int) { + Expect(s).Should(HaveLen(l)) + }, + Entry([]int{1, 2, 3, 4}, 4), + FEntry([]int{1, 2, 3, 4, 5}, 5), + ) +}) diff --git a/testdata/src/a/focusconfig/focus_key_name_test.go b/testdata/src/a/focusconfig/focus_key_name_test.go new file mode 100644 index 0000000..adcec31 --- /dev/null +++ b/testdata/src/a/focusconfig/focus_key_name_test.go @@ -0,0 +1,16 @@ +package focus + +import ( + "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = ginkgo.Describe("focused Describe", ginkgo.Focus, func() { // want `ginkgo-linter: Focus spec found. This is used only for local debug and should not be part of the actual source code, consider to remove it` + ginkgo.When("focused when", ginkgo.Focus, func() { // want `ginkgo-linter: Focus spec found. This is used only for local debug and should not be part of the actual source code, consider to remove it` + ginkgo.Context("focused Context", ginkgo.Focus, func() { // want `ginkgo-linter: Focus spec found. This is used only for local debug and should not be part of the actual source code, consider to remove it` + ginkgo.It("focused It", ginkgo.Focus, func() { // want `ginkgo-linter: Focus spec found. This is used only for local debug and should not be part of the actual source code, consider to remove it` + Expect(len("1234")).Should(Equal(4)) // want `ginkgo-linter: wrong length assertion; consider using .Expect\("1234"\)\.Should\(HaveLen\(4\)\). instead` + }) + }) + }) +}) diff --git a/testdata/src/a/focusconfig/focus_key_test.go b/testdata/src/a/focusconfig/focus_key_test.go new file mode 100644 index 0000000..c06b334 --- /dev/null +++ b/testdata/src/a/focusconfig/focus_key_test.go @@ -0,0 +1,16 @@ +package focus + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("focused Describe", Focus, func() { // want `ginkgo-linter: Focus spec found. This is used only for local debug and should not be part of the actual source code, consider to remove it` + When("focused when", Focus, func() { // want `ginkgo-linter: Focus spec found. This is used only for local debug and should not be part of the actual source code, consider to remove it` + Context("focused Context", Focus, func() { // want `ginkgo-linter: Focus spec found. This is used only for local debug and should not be part of the actual source code, consider to remove it` + It("focused It", Focus, func() { // want `ginkgo-linter: Focus spec found. This is used only for local debug and should not be part of the actual source code, consider to remove it` + Expect(len("1234")).Should(Equal(4)) // want `ginkgo-linter: wrong length assertion; consider using .Expect\("1234"\)\.Should\(HaveLen\(4\)\). instead` + }) + }) + }) +}) diff --git a/testdata/src/a/focusconfig/table_name_test.go b/testdata/src/a/focusconfig/table_name_test.go new file mode 100644 index 0000000..f3c6b0b --- /dev/null +++ b/testdata/src/a/focusconfig/table_name_test.go @@ -0,0 +1,32 @@ +package focus + +import ( + gnk "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = gnk.Describe("test focused tables", func() { + gnk.FDescribeTable("focused table", func(s []int, l int) { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "DescribeTable"` + Expect(len(s)).Should(Equal(l)) // want `ginkgo-linter: wrong length assertion; consider using .Expect\(s\)\.Should\(HaveLen\(l\)\). instead` + }, + gnk.Entry("check slice not focused", []int{1, 2, 3, 4}, 4), + gnk.Entry("check slice focus spec", gnk.Focus, []int{1, 2, 3, 4}, 4), // want `ginkgo-linter: Focus spec found. This is used only for local debug and should not be part of the actual source code, consider to remove it` + gnk.FEntry("check slice focused", []int{1, 2, 3, 4, 5}, 5), // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "Entry"` + ) + + gnk.DescribeTable("non-focused table", func(s []int, l int) { + Expect(s).Should(HaveLen(l)) + }, + gnk.Entry("check slice not focused", []int{1, 2, 3, 4}, 4), + gnk.Entry("check slice focus spec", gnk.Focus, []int{1, 2, 3, 4}, 4), // want `ginkgo-linter: Focus spec found. This is used only for local debug and should not be part of the actual source code, consider to remove it` + gnk.FEntry("check slice focused", []int{1, 2, 3, 4, 5}, 5), // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "Entry"` + ) + + gnk.DescribeTable("spec focused table", gnk.Focus, func(s []int, l int) { // want `ginkgo-linter: Focus spec found. This is used only for local debug and should not be part of the actual source code, consider to remove it` + Expect(s).Should(HaveLen(l)) + }, + gnk.Entry("check slice not focused", []int{1, 2, 3, 4}, 4), + gnk.Entry("check slice focus spec", gnk.Focus, []int{1, 2, 3, 4}, 4), // want `ginkgo-linter: Focus spec found. This is used only for local debug and should not be part of the actual source code, consider to remove it` + gnk.FEntry("check slice focused", []int{1, 2, 3, 4, 5}, 5), // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "Entry"` + ) +}) diff --git a/testdata/src/a/focusconfig/table_test.go b/testdata/src/a/focusconfig/table_test.go new file mode 100644 index 0000000..5b8f04e --- /dev/null +++ b/testdata/src/a/focusconfig/table_test.go @@ -0,0 +1,32 @@ +package focus + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("test focused tables", func() { + FDescribeTable("focused table", func(s []int, l int) { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "DescribeTable"` + Expect(len(s)).Should(Equal(l)) // want `ginkgo-linter: wrong length assertion; consider using .Expect\(s\)\.Should\(HaveLen\(l\)\). instead` + }, + Entry("check slice not focused", []int{1, 2, 3, 4}, 4), + Entry("check slice focus spec", Focus, []int{1, 2, 3, 4}, 4), // want `ginkgo-linter: Focus spec found. This is used only for local debug and should not be part of the actual source code, consider to remove it` + FEntry("check slice focused", []int{1, 2, 3, 4, 5}, 5), // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "Entry"` + ) + + DescribeTable("non-focused table", func(s []int, l int) { + Expect(s).Should(HaveLen(l)) + }, + Entry("check slice not focused", []int{1, 2, 3, 4}, 4), + Entry("check slice focus spec", Focus, []int{1, 2, 3, 4}, 4), // want `ginkgo-linter: Focus spec found. This is used only for local debug and should not be part of the actual source code, consider to remove it` + FEntry("check slice focused", []int{1, 2, 3, 4, 5}, 5), // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "Entry"` + ) + + DescribeTable("spec focused table", Focus, func(s []int, l int) { // want `ginkgo-linter: Focus spec found. This is used only for local debug and should not be part of the actual source code, consider to remove it` + Expect(s).Should(HaveLen(l)) + }, + Entry("check slice not focused", []int{1, 2, 3, 4}, 4), + Entry("check slice focus spec", Focus, []int{1, 2, 3, 4}, 4), // want `ginkgo-linter: Focus spec found. This is used only for local debug and should not be part of the actual source code, consider to remove it` + FEntry("check slice focused", []int{1, 2, 3, 4, 5}, 5), // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "Entry"` + ) +}) diff --git a/tests/e2e.sh b/tests/e2e.sh index 40717af..d0c6115 100755 --- a/tests/e2e.sh +++ b/tests/e2e.sh @@ -6,11 +6,11 @@ cp ginkgolinter testdata/src/a cd testdata/src/a # no suppress -[[ $(./ginkgolinter a/... 2>&1 | wc -l) == 2374 ]] +[[ $(./ginkgolinter a/... 2>&1 | wc -l) == 2380 ]] # suppress all but nil [[ $(./ginkgolinter --suppress-len-assertion=true --suppress-err-assertion=true --suppress-compare-assertion=true --suppress-async-assertion=true a/... 2>&1 | wc -l) == 1452 ]] # suppress all but len -[[ $(./ginkgolinter --suppress-nil-assertion=true --suppress-err-assertion=true --suppress-compare-assertion=true --suppress-async-assertion=true a/... 2>&1 | wc -l) == 744 ]] +[[ $(./ginkgolinter --suppress-nil-assertion=true --suppress-err-assertion=true --suppress-compare-assertion=true --suppress-async-assertion=true a/... 2>&1 | wc -l) == 750 ]] # suppress all but err [[ $(./ginkgolinter --suppress-nil-assertion=true --suppress-len-assertion=true --suppress-compare-assertion=true --suppress-async-assertion=true a/... 2>&1 | wc -l) == 212 ]] # suppress all but compare @@ -18,8 +18,8 @@ cd testdata/src/a # suppress all but async [[ $(./ginkgolinter --suppress-nil-assertion=true --suppress-err-assertion=true --suppress-len-assertion=true --suppress-compare-assertion=true a/... 2>&1 | wc -l) == 119 ]] # suppress all but focus -[[ $(./ginkgolinter --suppress-nil-assertion=true --suppress-err-assertion=true --suppress-len-assertion=true --suppress-compare-assertion=true --suppress-async-assertion=true --forbid-focus-container=true a/... 2>&1 | wc -l) == 112 ]] +[[ $(./ginkgolinter --suppress-nil-assertion=true --suppress-err-assertion=true --suppress-len-assertion=true --suppress-compare-assertion=true --suppress-async-assertion=true --forbid-focus-container=true a/... 2>&1 | wc -l) == 143 ]] # allow HaveLen(0) -[[ $(./ginkgolinter --allow-havelen-0=true a/... 2>&1 | wc -l) == 2361 ]] +[[ $(./ginkgolinter --allow-havelen-0=true a/... 2>&1 | wc -l) == 2367 ]] # suppress all - should only return the few non-suppressble [[ $(./ginkgolinter --suppress-nil-assertion=true --suppress-len-assertion=true --suppress-err-assertion=true --suppress-compare-assertion=true --suppress-async-assertion=true --forbid-focus-container=false a/... 2>&1 | wc -l) == 88 ]]