Fix bug with the forbid-focus-container flag

The `--forbid-focus-container` flag behaved with the reverse logic. This PR fixes it.
nunnatsa · Jul 24, 2023 · f4e70b6 · f4e70b6
1 parent 4edfba5
commit f4e70b6
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 18 deletions.
diff --git a/ginkgo_linter.go b/ginkgo_linter.go
@@ -80,7 +80,7 @@ func NewAnalyzer() *analysis.Analyzer {
 			SuppressNil:     false,
 			SuppressErr:     false,
 			SuppressCompare: false,
-			SuppressFocus:   true,
+			ForbidFocus:     false,
 			AllowHaveLen0:   false,
 		},
 	}
@@ -101,7 +101,7 @@ func NewAnalyzer() *analysis.Analyzer {
 	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.")
+	a.Flags.Var(&linter.config.ForbidFocus, "forbid-focus-container", "trigger a warning for ginkgo focus containers like FDescribe, FContext, FWhen or FIt; default = false.")
 
 	return a
 }
@@ -169,7 +169,7 @@ func (l *ginkgoLinter) run(pass *analysis.Pass) (interface{}, error) {
 		}
 
 		ast.Inspect(file, func(n ast.Node) bool {
-			if ginkgoHndlr != nil && !fileConfig.SuppressFocus {
+			if ginkgoHndlr != nil && fileConfig.ForbidFocus {
 				spec, ok := n.(*ast.ValueSpec)
 				if ok {
 					for _, val := range spec.Values {
@@ -199,7 +199,7 @@ func (l *ginkgoLinter) run(pass *analysis.Pass) (interface{}, error) {
 				return true
 			}
 
-			if ginkgoHndlr != nil && !bool(config.SuppressFocus) && checkFocusContainer(pass, ginkgoHndlr, assertionExp) {
+			if ginkgoHndlr != nil && bool(config.ForbidFocus) && checkFocusContainer(pass, ginkgoHndlr, assertionExp) {
 				return true
 			}
 

diff --git a/ginkgo_linter_test.go b/ginkgo_linter_test.go
@@ -122,7 +122,7 @@ func TestFlags(t *testing.T) {
 		},
 		{
 			testName: "test the forbid-focus-container flag",
-			testData: []string{"a/focus"},
+			testData: []string{"a/focusconfig"},
 			flags:    map[string]string{"forbid-focus-container": "true"},
 		},
 	} {

diff --git a/tests/e2e.sh b/tests/e2e.sh
@@ -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=false 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) == 112 ]]
 # allow HaveLen(0)
 [[ $(./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 --forbid-focus-container=false a/... 2>&1 | wc -l) == 112 ]]
+[[ $(./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 ]]
diff --git a/types/config.go b/types/config.go
@@ -22,12 +22,12 @@ type Config struct {
 	SuppressErr     Boolean
 	SuppressCompare Boolean
 	SuppressAsync   Boolean
-	SuppressFocus   Boolean
+	ForbidFocus     Boolean
 	AllowHaveLen0   Boolean
 }
 
 func (s *Config) AllTrue() bool {
-	return bool(s.SuppressLen && s.SuppressNil && s.SuppressErr && s.SuppressCompare && s.SuppressAsync && s.SuppressFocus)
+	return bool(s.SuppressLen && s.SuppressNil && s.SuppressErr && s.SuppressCompare && s.SuppressAsync && !s.ForbidFocus)
 }
 
 func (s *Config) Clone() Config {
@@ -37,7 +37,7 @@ func (s *Config) Clone() Config {
 		SuppressErr:     s.SuppressErr,
 		SuppressCompare: s.SuppressCompare,
 		SuppressAsync:   s.SuppressAsync,
-		SuppressFocus:   s.SuppressFocus,
+		ForbidFocus:     s.ForbidFocus,
 		AllowHaveLen0:   s.AllowHaveLen0,
 	}
 }
@@ -56,12 +56,20 @@ func (s *Config) UpdateFromComment(commentGroup []*ast.CommentGroup) {
 				comment = strings.TrimSuffix(comment, "*/")
 				comment = strings.TrimSpace(comment)
 
-				s.SuppressLen = s.SuppressLen || (comment == suppressLengthAssertionWarning)
-				s.SuppressNil = s.SuppressNil || (comment == suppressNilAssertionWarning)
-				s.SuppressErr = s.SuppressErr || (comment == suppressErrAssertionWarning)
-				s.SuppressCompare = s.SuppressCompare || (comment == suppressCompareAssertionWarning)
-				s.SuppressAsync = s.SuppressAsync || (comment == suppressAsyncAsertWarning)
-				s.SuppressFocus = s.SuppressFocus || (comment == suppressFocusContainerWarning)
+				switch comment {
+				case suppressLengthAssertionWarning:
+					s.SuppressLen = true
+				case suppressNilAssertionWarning:
+					s.SuppressNil = true
+				case suppressErrAssertionWarning:
+					s.SuppressErr = true
+				case suppressCompareAssertionWarning:
+					s.SuppressCompare = true
+				case suppressAsyncAsertWarning:
+					s.SuppressAsync = true
+				case suppressFocusContainerWarning:
+					s.ForbidFocus = false
+				}
 			}
 		}
 	}

diff --git a/types/config_test.go b/types/config_test.go
@@ -11,7 +11,7 @@ func TestSuppress_AllTrue(t *testing.T) {
 		SuppressErr:     true,
 		SuppressAsync:   true,
 		SuppressCompare: true,
-		SuppressFocus:   true,
+		ForbidFocus:     false,
 	}
 
 	if !s.AllTrue() {
@@ -51,7 +51,7 @@ func TestSuppress_Clone(t *testing.T) {
 		SuppressErr:     true,
 		SuppressCompare: true,
 		SuppressAsync:   true,
-		SuppressFocus:   true,
+		ForbidFocus:     false,
 	}
 
 	clone := s.Clone()