From 4edfba595544c41ffdc28adc42f81917ed8c698a Mon Sep 17 00:00:00 2001 From: Nahshon Unna-Tsameret Date: Mon, 24 Jul 2023 13:21:20 +0300 Subject: [PATCH] disable the focused container rule by default Enabling this rule by default may case issues in projects with a complex build systems. This PR change this rule to be opt-in. The `--suppress-focus-container` flag is now depracated and ignored. Instead, this PR introduces the `--forbid-focus-container` flag, with default value of `false`. --- README.md | 3 ++- ginkgo_linter.go | 6 ++++- ginkgo_linter_test.go | 24 +++++++++---------- testdata/src/a/focus/focus_name_test.go | 8 +++---- testdata/src/a/focus/focus_noname_test.go | 8 +++---- testdata/src/a/focus/focus_test.go | 21 ++++------------ testdata/src/a/focusconfig/focus_name_test.go | 8 +++---- .../src/a/focusconfig/focus_noname_test.go | 8 +++---- testdata/src/a/focusconfig/focus_test.go | 21 ++++++++++++---- .../a/{focus => focusconfig}/supress_test.go | 0 tests/e2e.sh | 18 +++++++------- 11 files changed, 65 insertions(+), 60 deletions(-) rename testdata/src/a/{focus => focusconfig}/supress_test.go (100%) diff --git a/README.md b/README.md index 8492fba..3edf065 100644 --- a/README.md +++ b/README.md @@ -159,6 +159,8 @@ This rule finds ginkgo focus containers in the code. ginkgo supports the `FDescribe`, `FContext`, `FWhen` and `FIt` 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() { @@ -304,7 +306,6 @@ Expect(c1 == x1).Should(BeTrue()) // ==> Expect(x1).Should(Equal(c1)) * Use the `--suppress-err-assertion=true` flag to suppress the wrong error assertion warning * Use the `--suppress-compare-assertion=true` flag to suppress the wrong comparison assertion warning * Use the `--suppress-async-assertion=true` flag to suppress the function call in async assertion warning -* Use the `--suppress-focus-container=true` flag to suppress the focus container warning * Use the `--allow-havelen-0=true` flag to avoid warnings about `HaveLen(0)`; Note: this parameter is only supported from command line, and not from a comment. diff --git a/ginkgo_linter.go b/ginkgo_linter.go index 5e5ac00..24c3ffb 100644 --- a/ginkgo_linter.go +++ b/ginkgo_linter.go @@ -80,6 +80,7 @@ func NewAnalyzer() *analysis.Analyzer { SuppressNil: false, SuppressErr: false, SuppressCompare: false, + SuppressFocus: true, AllowHaveLen0: false, }, } @@ -90,15 +91,18 @@ func NewAnalyzer() *analysis.Analyzer { Run: linter.run, } + var ignored bool a.Flags.Init("ginkgolinter", flag.ExitOnError) a.Flags.Var(&linter.config.SuppressLen, "suppress-len-assertion", "Suppress warning for wrong length assertions") a.Flags.Var(&linter.config.SuppressNil, "suppress-nil-assertion", "Suppress warning for wrong nil assertions") a.Flags.Var(&linter.config.SuppressErr, "suppress-err-assertion", "Suppress warning for wrong error assertions") a.Flags.Var(&linter.config.SuppressCompare, "suppress-compare-assertion", "Suppress warning for wrong comparison assertions") a.Flags.Var(&linter.config.SuppressAsync, "suppress-async-assertion", "Suppress warning for function call in async assertion, like Eventually") - a.Flags.Var(&linter.config.SuppressFocus, "suppress-focus-container", "Suppress warning for ginkgo focus containers like FDescribe, FContext, FWhen or FIt") a.Flags.Var(&linter.config.AllowHaveLen0, "allow-havelen-0", "Do not warn for HaveLen(0); default = false") + a.Flags.BoolVar(&ignored, "suppress-focus-container", true, "Suppress warning for ginkgo focus containers like FDescribe, FContext, FWhen or FIt. Deprecated and ignored: use --forbid-focus-container instead") + a.Flags.Var(&linter.config.SuppressFocus, "forbid-focus-container", "trigger a warning for ginkgo focus containers like FDescribe, FContext, FWhen or FIt; default = false.") + return a } diff --git a/ginkgo_linter_test.go b/ginkgo_linter_test.go index 4c1ba07..6de3dd4 100644 --- a/ginkgo_linter_test.go +++ b/ginkgo_linter_test.go @@ -88,48 +88,48 @@ func TestFlags(t *testing.T) { for _, tc := range []struct { testName string testData []string - flags []string + flags map[string]string }{ { testName: "test the suppress-len-assertion flag", testData: []string{"a/configlen"}, - flags: []string{"suppress-len-assertion"}, + flags: map[string]string{"suppress-len-assertion": "true"}, }, { testName: "test the suppress-nil-assertion flag", testData: []string{"a/confignil"}, - flags: []string{"suppress-nil-assertion"}, + flags: map[string]string{"suppress-nil-assertion": "true"}, }, { testName: "test the suppress-err-assertion flag", testData: []string{"a/configerr"}, - flags: []string{"suppress-err-assertion"}, + flags: map[string]string{"suppress-err-assertion": "true"}, }, { testName: "test the suppress-compare-assertion flag", testData: []string{"a/configcompare"}, - flags: []string{"suppress-compare-assertion"}, + flags: map[string]string{"suppress-compare-assertion": "true"}, }, { testName: "test the allow-havelen-0 flag", testData: []string{"a/havelen0config"}, - flags: []string{"allow-havelen-0"}, + flags: map[string]string{"allow-havelen-0": "true"}, }, { testName: "test the suppress-async-assertion flag", testData: []string{"a/asyncconfig"}, - flags: []string{"suppress-async-assertion"}, + flags: map[string]string{"suppress-async-assertion": "true"}, }, { - testName: "test the suppress-focus-container flag", - testData: []string{"a/focusconfig"}, - flags: []string{"suppress-focus-container"}, + testName: "test the forbid-focus-container flag", + testData: []string{"a/focus"}, + flags: map[string]string{"forbid-focus-container": "true"}, }, } { t.Run(tc.testName, func(tt *testing.T) { analyzer := ginkgolinter.NewAnalyzer() - for _, flag := range tc.flags { - err := analyzer.Flags.Set(flag, "true") + for flag, value := range tc.flags { + err := analyzer.Flags.Set(flag, value) if err != nil { tt.Errorf(`failed to set the "%s" flag; %v`, flag, err) return diff --git a/testdata/src/a/focus/focus_name_test.go b/testdata/src/a/focus/focus_name_test.go index a622374..47b8506 100644 --- a/testdata/src/a/focus/focus_name_test.go +++ b/testdata/src/a/focus/focus_name_test.go @@ -5,7 +5,7 @@ import ( . "github.com/onsi/gomega" ) -var _ = tester.FDescribe("should warn", func() { // 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 "Describe"` +var _ = tester.FDescribe("should warn", func() { tester.When("should ignore", func() { tester.It("should ignore", func() { Expect(len("abcd")).Should(Equal(4)) // want `ginkgo-linter: wrong length assertion; consider using .Expect\("abcd"\)\.Should\(HaveLen\(4\)\). instead` @@ -23,7 +23,7 @@ var _ = tester.FDescribe("should warn", func() { // want `ginkgo-linter: Focus c }) var _ = tester.Describe("should ignore", func() { - tester.FWhen("should warn", func() { // 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 "When"` + tester.FWhen("should warn", func() { tester.Context("should ignore", func() { tester.It("should ignore", func() { Expect("abcd").Should(HaveLen(4)) @@ -35,7 +35,7 @@ var _ = tester.Describe("should ignore", func() { }) }) - tester.FContext("should warn", func() { // 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 "Context"` + tester.FContext("should warn", func() { tester.Context("should ignore", func() { tester.It("should ignore", func() { Expect("abcd").Should(HaveLen(4)) @@ -48,7 +48,7 @@ var _ = tester.Describe("should ignore", func() { }) tester.Context("ignore", func() { - tester.FIt("should warn", func() { // 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 "It"` + tester.FIt("should warn", func() { Expect("abcd").Should(HaveLen(4)) }) }) diff --git a/testdata/src/a/focus/focus_noname_test.go b/testdata/src/a/focus/focus_noname_test.go index a91fb1e..a2b6e27 100644 --- a/testdata/src/a/focus/focus_noname_test.go +++ b/testdata/src/a/focus/focus_noname_test.go @@ -5,7 +5,7 @@ import ( . "github.com/onsi/gomega" ) -var _ = ginkgo.FDescribe("should warn", func() { // 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 "Describe"` +var _ = ginkgo.FDescribe("should warn", func() { ginkgo.When("should ignore", func() { ginkgo.It("should ignore", func() { Expect("abcd").Should(HaveLen(4)) @@ -23,7 +23,7 @@ var _ = ginkgo.FDescribe("should warn", func() { // want `ginkgo-linter: Focus c }) var _ = ginkgo.Describe("should ignore", func() { - ginkgo.FWhen("should warn", func() { // 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 "When"` + ginkgo.FWhen("should warn", func() { ginkgo.Context("should ignore", func() { ginkgo.It("should ignore", func() { Expect("abcd").Should(HaveLen(4)) @@ -35,7 +35,7 @@ var _ = ginkgo.Describe("should ignore", func() { }) }) - ginkgo.FContext("should warn", func() { // 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 "Context"` + ginkgo.FContext("should warn", func() { ginkgo.Context("should ignore", func() { ginkgo.It("should ignore", func() { Expect("abcd").Should(HaveLen(4)) @@ -48,7 +48,7 @@ var _ = ginkgo.Describe("should ignore", func() { }) ginkgo.Context("ignore", func() { - ginkgo.FIt("should warn", func() { // 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 "It"` + ginkgo.FIt("should warn", func() { Expect("abcd").Should(HaveLen(4)) }) }) diff --git a/testdata/src/a/focus/focus_test.go b/testdata/src/a/focus/focus_test.go index da7fa6d..7e75066 100644 --- a/testdata/src/a/focus/focus_test.go +++ b/testdata/src/a/focus/focus_test.go @@ -5,7 +5,7 @@ import ( . "github.com/onsi/gomega" ) -var _ = FDescribe("should warn", func() { // 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 "Describe"` +var _ = FDescribe("should warn", func() { When("should ignore", func() { It("should ignore", func() { Expect("abcd").Should(HaveLen(4)) @@ -23,7 +23,7 @@ var _ = FDescribe("should warn", func() { // want `ginkgo-linter: Focus containe }) var _ = Describe("should ignore", func() { - FWhen("should warn", func() { // 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 "When"` + FWhen("should warn", func() { Context("should ignore", func() { It("should ignore", func() { Expect("abcd").Should(HaveLen(4)) @@ -35,7 +35,7 @@ var _ = Describe("should ignore", func() { }) }) - FContext("should warn", func() { // 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 "Context"` + FContext("should warn", func() { Context("should ignore", func() { It("should ignore", func() { Expect("abcd").Should(HaveLen(4)) @@ -48,21 +48,8 @@ var _ = Describe("should ignore", func() { }) Context("ignore", func() { - FIt("should warn", func() { // 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 "It"` + FIt("should warn", func() { Expect("abcd").Should(HaveLen(4)) }) }) }) - -var _ = Describe("suppress", func() { - // ginkgo-linter:ignore-focus-container-warning - FContext("supress", func() { - // ginkgo-linter:ignore-focus-container-warning - FWhen("suppress", func() { - // ginkgo-linter:ignore-focus-container-warning - FIt("suppress", func() { - Expect(len("abcd")).Should(Equal(4)) // want `ginkgo-linter: wrong length assertion; consider using .Expect\("abcd"\)\.Should\(HaveLen\(4\)\). instead` - }) - }) - }) -}) diff --git a/testdata/src/a/focusconfig/focus_name_test.go b/testdata/src/a/focusconfig/focus_name_test.go index 47b8506..a622374 100644 --- a/testdata/src/a/focusconfig/focus_name_test.go +++ b/testdata/src/a/focusconfig/focus_name_test.go @@ -5,7 +5,7 @@ import ( . "github.com/onsi/gomega" ) -var _ = tester.FDescribe("should warn", func() { +var _ = tester.FDescribe("should warn", func() { // 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 "Describe"` tester.When("should ignore", func() { tester.It("should ignore", func() { Expect(len("abcd")).Should(Equal(4)) // want `ginkgo-linter: wrong length assertion; consider using .Expect\("abcd"\)\.Should\(HaveLen\(4\)\). instead` @@ -23,7 +23,7 @@ var _ = tester.FDescribe("should warn", func() { }) var _ = tester.Describe("should ignore", func() { - tester.FWhen("should warn", func() { + tester.FWhen("should warn", func() { // 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 "When"` tester.Context("should ignore", func() { tester.It("should ignore", func() { Expect("abcd").Should(HaveLen(4)) @@ -35,7 +35,7 @@ var _ = tester.Describe("should ignore", func() { }) }) - tester.FContext("should warn", func() { + tester.FContext("should warn", func() { // 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 "Context"` tester.Context("should ignore", func() { tester.It("should ignore", func() { Expect("abcd").Should(HaveLen(4)) @@ -48,7 +48,7 @@ var _ = tester.Describe("should ignore", func() { }) tester.Context("ignore", func() { - tester.FIt("should warn", func() { + tester.FIt("should warn", func() { // 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 "It"` Expect("abcd").Should(HaveLen(4)) }) }) diff --git a/testdata/src/a/focusconfig/focus_noname_test.go b/testdata/src/a/focusconfig/focus_noname_test.go index a2b6e27..a91fb1e 100644 --- a/testdata/src/a/focusconfig/focus_noname_test.go +++ b/testdata/src/a/focusconfig/focus_noname_test.go @@ -5,7 +5,7 @@ import ( . "github.com/onsi/gomega" ) -var _ = ginkgo.FDescribe("should warn", func() { +var _ = ginkgo.FDescribe("should warn", func() { // 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 "Describe"` ginkgo.When("should ignore", func() { ginkgo.It("should ignore", func() { Expect("abcd").Should(HaveLen(4)) @@ -23,7 +23,7 @@ var _ = ginkgo.FDescribe("should warn", func() { }) var _ = ginkgo.Describe("should ignore", func() { - ginkgo.FWhen("should warn", func() { + ginkgo.FWhen("should warn", func() { // 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 "When"` ginkgo.Context("should ignore", func() { ginkgo.It("should ignore", func() { Expect("abcd").Should(HaveLen(4)) @@ -35,7 +35,7 @@ var _ = ginkgo.Describe("should ignore", func() { }) }) - ginkgo.FContext("should warn", func() { + ginkgo.FContext("should warn", func() { // 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 "Context"` ginkgo.Context("should ignore", func() { ginkgo.It("should ignore", func() { Expect("abcd").Should(HaveLen(4)) @@ -48,7 +48,7 @@ var _ = ginkgo.Describe("should ignore", func() { }) ginkgo.Context("ignore", func() { - ginkgo.FIt("should warn", func() { + ginkgo.FIt("should warn", func() { // 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 "It"` Expect("abcd").Should(HaveLen(4)) }) }) diff --git a/testdata/src/a/focusconfig/focus_test.go b/testdata/src/a/focusconfig/focus_test.go index 7e75066..da7fa6d 100644 --- a/testdata/src/a/focusconfig/focus_test.go +++ b/testdata/src/a/focusconfig/focus_test.go @@ -5,7 +5,7 @@ import ( . "github.com/onsi/gomega" ) -var _ = FDescribe("should warn", func() { +var _ = FDescribe("should warn", func() { // 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 "Describe"` When("should ignore", func() { It("should ignore", func() { Expect("abcd").Should(HaveLen(4)) @@ -23,7 +23,7 @@ var _ = FDescribe("should warn", func() { }) var _ = Describe("should ignore", func() { - FWhen("should warn", func() { + FWhen("should warn", func() { // 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 "When"` Context("should ignore", func() { It("should ignore", func() { Expect("abcd").Should(HaveLen(4)) @@ -35,7 +35,7 @@ var _ = Describe("should ignore", func() { }) }) - FContext("should warn", func() { + FContext("should warn", func() { // 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 "Context"` Context("should ignore", func() { It("should ignore", func() { Expect("abcd").Should(HaveLen(4)) @@ -48,8 +48,21 @@ var _ = Describe("should ignore", func() { }) Context("ignore", func() { - FIt("should warn", func() { + FIt("should warn", func() { // 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 "It"` Expect("abcd").Should(HaveLen(4)) }) }) }) + +var _ = Describe("suppress", func() { + // ginkgo-linter:ignore-focus-container-warning + FContext("supress", func() { + // ginkgo-linter:ignore-focus-container-warning + FWhen("suppress", func() { + // ginkgo-linter:ignore-focus-container-warning + FIt("suppress", func() { + Expect(len("abcd")).Should(Equal(4)) // want `ginkgo-linter: wrong length assertion; consider using .Expect\("abcd"\)\.Should\(HaveLen\(4\)\). instead` + }) + }) + }) +}) diff --git a/testdata/src/a/focus/supress_test.go b/testdata/src/a/focusconfig/supress_test.go similarity index 100% rename from testdata/src/a/focus/supress_test.go rename to testdata/src/a/focusconfig/supress_test.go diff --git a/tests/e2e.sh b/tests/e2e.sh index c8f7be8..097e724 100755 --- a/tests/e2e.sh +++ b/tests/e2e.sh @@ -6,20 +6,20 @@ cp ginkgolinter testdata/src/a cd testdata/src/a # no suppress -[[ $(./ginkgolinter a/... 2>&1 | wc -l) == 2398 ]] +[[ $(./ginkgolinter a/... 2>&1 | wc -l) == 2374 ]] # suppress all but nil -[[ $(./ginkgolinter --suppress-len-assertion=true --suppress-err-assertion=true --suppress-compare-assertion=true --suppress-async-assertion=true --suppress-focus-container=true a/... 2>&1 | wc -l) == 1452 ]] +[[ $(./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 --suppress-focus-container=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) == 744 ]] # suppress all but err -[[ $(./ginkgolinter --suppress-nil-assertion=true --suppress-len-assertion=true --suppress-compare-assertion=true --suppress-async-assertion=true --suppress-focus-container=true a/... 2>&1 | wc -l) == 212 ]] +[[ $(./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 -[[ $(./ginkgolinter --suppress-nil-assertion=true --suppress-err-assertion=true --suppress-len-assertion=true --suppress-async-assertion=true --suppress-focus-container=true a/... 2>&1 | wc -l) == 222 ]] +[[ $(./ginkgolinter --suppress-nil-assertion=true --suppress-err-assertion=true --suppress-len-assertion=true --suppress-async-assertion=true a/... 2>&1 | wc -l) == 222 ]] # suppress all but async -[[ $(./ginkgolinter --suppress-nil-assertion=true --suppress-err-assertion=true --suppress-len-assertion=true --suppress-compare-assertion=true --suppress-focus-container=true a/... 2>&1 | wc -l) == 119 ]] +[[ $(./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 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=false a/... 2>&1 | wc -l) == 112 ]] # allow HaveLen(0) -[[ $(./ginkgolinter --allow-havelen-0=true a/... 2>&1 | wc -l) == 2385 ]] +[[ $(./ginkgolinter --allow-havelen-0=true a/... 2>&1 | wc -l) == 2361 ]] # 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 --suppress-focus-container=true a/... 2>&1 | wc -l) == 88 ]] +[[ $(./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) == 112 ]]