Refactor: move ginkgo containers logic to the handler

Cleanup the linter code by moving the ginkgo related logic (check for focus containers, and avoid test polution check), to the ginkgo linter package, in order to get cleaner main linter code.
nunnatsa · Nov 4, 2024 · 1309578 · 1309578
1 parent 05ca79e
commit 1309578
Show file tree

Hide file tree

Showing 7 changed files with 382 additions and 279 deletions.
diff --git a/internal/ginkgohandler/dothandler.go b/internal/ginkgohandler/dothandler.go
@@ -0,0 +1,36 @@
+package ginkgohandler
+
+import (
+	"go/ast"
+
+	"golang.org/x/tools/go/analysis"
+
+	"github.com/nunnatsa/ginkgolinter/types"
+)
+
+// dotHandler is used when importing ginkgo with dot; i.e.
+// import . "github.com/onsi/ginkgo"
+type dotHandler struct{}
+
+func (h dotHandler) HandleGinkgoSpecs(expr ast.Expr, config types.Config, pass *analysis.Pass) bool {
+	return handleGinkgoSpecs(expr, config, pass, h)
+}
+
+func (h dotHandler) getFocusContainerName(exp *ast.CallExpr) (bool, *ast.Ident) {
+	if fun, ok := exp.Fun.(*ast.Ident); ok {
+		return isFocusContainer(fun.Name), fun
+	}
+	return false, nil
+}
+
+func (h dotHandler) isWrapContainer(exp *ast.CallExpr) bool {
+	if fun, ok := exp.Fun.(*ast.Ident); ok {
+		return isWrapContainer(fun.Name)
+	}
+	return false
+}
+
+func (h dotHandler) isFocusSpec(exp ast.Expr) bool {
+	id, ok := exp.(*ast.Ident)
+	return ok && id.Name == focusSpec
+}
diff --git a/internal/ginkgohandler/ginkgoinfo.go b/internal/ginkgohandler/ginkgoinfo.go
@@ -0,0 +1,63 @@
+package ginkgohandler
+
+const ( // container names
+	describe  = "Describe"
+	pdescribe = "PDescribe"
+	xdescribe = "XDescribe"
+	fdescribe = "FDescribe"
+
+	when  = "When"
+	pwhen = "PWhen"
+	xwhen = "XWhen"
+	fwhen = "FWhen"
+
+	contextContainer = "Context"
+	pcontext         = "PContext"
+	xcontext         = "XContext"
+	fcontext         = "FContext"
+
+	it  = "It"
+	pit = "PIt"
+	xit = "XIt"
+	fit = "FIt"
+
+	describeTable  = "DescribeTable"
+	pdescribeTable = "PDescribeTable"
+	xdescribeTable = "XDescribeTable"
+	fdescribeTable = "FDescribeTable"
+
+	entry  = "Entry"
+	pentry = "PEntry"
+	xentry = "XEntry"
+	fentry = "FEntry"
+)
+
+func isFocusContainer(name string) bool {
+	switch name {
+	case fdescribe, fcontext, fwhen, fit, fdescribeTable, fentry:
+		return true
+	}
+	return false
+}
+
+func isContainer(name string) bool {
+	switch name {
+	case it, when, contextContainer, describe, describeTable, entry,
+		pit, pwhen, pcontext, pdescribe, pdescribeTable, pentry,
+		xit, xwhen, xcontext, xdescribe, xdescribeTable, xentry:
+		return true
+	}
+	return isFocusContainer(name)
+}
+
+func isWrapContainer(name string) bool {
+	switch name {
+	case when, contextContainer, describe,
+		fwhen, fcontext, fdescribe,
+		pwhen, pcontext, pdescribe,
+		xwhen, xcontext, xdescribe:
+		return true
+	}
+
+	return false
+}
diff --git a/internal/ginkgohandler/handler.go b/internal/ginkgohandler/handler.go
@@ -2,6 +2,10 @@ package ginkgohandler
 
 import (
 	"go/ast"
+
+	"golang.org/x/tools/go/analysis"
+
+	"github.com/nunnatsa/ginkgolinter/types"
 )
 
 const (
@@ -14,116 +18,31 @@ const (
 // 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)
-	IsWrapContainer(*ast.CallExpr) bool
-	IsFocusSpec(ident ast.Expr) bool
+	HandleGinkgoSpecs(ast.Expr, types.Config, *analysis.Pass) bool
+	getFocusContainerName(*ast.CallExpr) (bool, *ast.Ident)
+	isWrapContainer(*ast.CallExpr) bool
+	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 != importPath && imp.Path.Value != importPathV2 {
-			continue
-		}
+		switch imp.Path.Value {
+
+		case importPath, importPathV2:
+			switch name := imp.Name.String(); {
+			case name == ".":
+				return dotHandler{}
+			case name == "<nil>": // import with no local name
+				return nameHandler("ginkgo")
+			default:
+				return nameHandler(name)
+			}
 
-		switch name := imp.Name.String(); {
-		case name == ".":
-			return dotHandler{}
-		case name == "<nil>": // import with no local name
-			return nameHandler("ginkgo")
 		default:
-			return nameHandler(name)
-		}
-	}
-
-	return nil // no ginkgo import; this file does not use ginkgo
-}
-
-// dotHandler is used when importing ginkgo with dot; i.e.
-// import . "github.com/onsi/ginkgo"
-type dotHandler struct{}
-
-func (h dotHandler) GetFocusContainerName(exp *ast.CallExpr) (bool, *ast.Ident) {
-	if fun, ok := exp.Fun.(*ast.Ident); ok {
-		return isFocusContainer(fun.Name), fun
-	}
-	return false, nil
-}
-
-func (h dotHandler) IsWrapContainer(exp *ast.CallExpr) bool {
-	if fun, ok := exp.Fun.(*ast.Ident); ok {
-		return IsWrapContainer(fun.Name)
-	}
-	return false
-}
-
-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"
-//
-// or with a custom name; e.g.
-// import customname "github.com/onsi/ginkgo"
-type nameHandler string
-
-func (h nameHandler) GetFocusContainerName(exp *ast.CallExpr) (bool, *ast.Ident) {
-	if sel, ok := exp.Fun.(*ast.SelectorExpr); ok {
-		if id, ok := sel.X.(*ast.Ident); ok && id.Name == string(h) {
-			return isFocusContainer(sel.Sel.Name), sel.Sel
-		}
-	}
-	return false, nil
-}
-
-func (h nameHandler) IsWrapContainer(exp *ast.CallExpr) bool {
-	if sel, ok := exp.Fun.(*ast.SelectorExpr); ok {
-		if id, ok := sel.X.(*ast.Ident); ok && id.Name == string(h) {
-			return IsWrapContainer(sel.Sel.Name)
-		}
-	}
-	return false
-
-}
-
-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
+			continue
 		}
 	}
 
-	return false
-}
-
-func isFocusContainer(name string) bool {
-	switch name {
-	case "FDescribe", "FContext", "FWhen", "FIt", "FDescribeTable", "FEntry":
-		return true
-	}
-	return false
-}
-
-func IsContainer(name string) bool {
-	switch name {
-	case "It", "When", "Context", "Describe", "DescribeTable", "Entry",
-		"PIt", "PWhen", "PContext", "PDescribe", "PDescribeTable", "PEntry",
-		"XIt", "XWhen", "XContext", "XDescribe", "XDescribeTable", "XEntry":
-		return true
-	}
-	return isFocusContainer(name)
-}
-
-func IsWrapContainer(name string) bool {
-	switch name {
-	case "When", "Context", "Describe",
-		"FWhen", "FContext", "FDescribe",
-		"PWhen", "PContext", "PDescribe",
-		"XWhen", "XContext", "XDescribe":
-		return true
-	}
-
-	return false
+	return nil
 }
diff --git a/internal/ginkgohandler/handler_test.go b/internal/ginkgohandler/handler_test.go
@@ -113,38 +113,38 @@ func TestGetGinkgoHandler_no_ginkgo(t *testing.T) {
 
 func TestDotHandler_GetFocusContainerName_happy(t *testing.T) {
 	exp := &ast.CallExpr{
-		Fun: ast.NewIdent("FIt"),
+		Fun: ast.NewIdent(fit),
 	}
 
 	h := dotHandler{}
 
-	isFocus, name := h.GetFocusContainerName(exp)
+	isFocus, name := h.getFocusContainerName(exp)
 
 	if !isFocus {
 		t.Error("h.GetFocusContainerName(exp) should return true")
 	}
 
 	if name == nil {
 		t.Error("should return valid ast.Ident object")
-	} else if name.Name != "FIt" {
+	} else if name.Name != fit {
 		t.Error("function name should be 'FIt'")
 	}
 }
 
 func TestDotHandler_GetFocusContainerName_no_focus(t *testing.T) {
 	exp := &ast.CallExpr{
-		Fun: ast.NewIdent("It"),
+		Fun: ast.NewIdent(it),
 	}
 
 	h := dotHandler{}
-	isFocus, name := h.GetFocusContainerName(exp)
+	isFocus, name := h.getFocusContainerName(exp)
 	if isFocus {
 		t.Error("h.GetFocusContainerName(exp) should return false")
 	}
 
 	if name == nil {
 		t.Error("should return valid ast.Ident object")
-	} else if name.Name != "It" {
+	} else if name.Name != it {
 		t.Error("function name should be 'It'")
 	}
 }
@@ -154,13 +154,13 @@ func TestDotHandler_GetFocusContainerName_selector(t *testing.T) {
 		Fun: &ast.SelectorExpr{
 			Sel: ast.NewIdent("ginkgo"),
 			X: &ast.CallExpr{
-				Fun: ast.NewIdent("FIt"),
+				Fun: ast.NewIdent(fit),
 			},
 		},
 	}
 
 	h := dotHandler{}
-	isFocus, name := h.GetFocusContainerName(exp)
+	isFocus, name := h.getFocusContainerName(exp)
 	if isFocus {
 		t.Error("h.GetFocusContainerName(exp) should return false")
 	}
@@ -173,52 +173,52 @@ func TestDotHandler_GetFocusContainerName_selector(t *testing.T) {
 func TestNameHandler_GetFocusContainerName_happy(t *testing.T) {
 	exp := &ast.CallExpr{
 		Fun: &ast.SelectorExpr{
-			Sel: ast.NewIdent("FIt"),
+			Sel: ast.NewIdent(fit),
 			X:   ast.NewIdent("ginkgo"),
 		},
 	}
 
 	h := nameHandler("ginkgo")
-	isFocus, name := h.GetFocusContainerName(exp)
+	isFocus, name := h.getFocusContainerName(exp)
 	if !isFocus {
 		t.Error("h.GetFocusContainerName(exp) should return true")
 	}
 
 	if name == nil {
 		t.Error("should return a valid ast.Ident object")
-	} else if name.Name != "FIt" {
+	} else if name.Name != fit {
 		t.Error("function name should be 'FIt'")
 	}
 }
 
 func TestNameHandler_GetFocusContainerName_no_focus(t *testing.T) {
 	exp := &ast.CallExpr{
 		Fun: &ast.SelectorExpr{
-			Sel: ast.NewIdent("It"),
+			Sel: ast.NewIdent(it),
 			X:   ast.NewIdent("ginkgo"),
 		},
 	}
 
 	h := nameHandler("ginkgo")
-	isFocus, name := h.GetFocusContainerName(exp)
+	isFocus, name := h.getFocusContainerName(exp)
 	if isFocus {
 		t.Error("h.GetFocusContainerName(exp) should return false")
 	}
 
 	if name == nil {
 		t.Error("should return a valid ast.Ident object")
-	} else if name.Name != "It" {
+	} else if name.Name != it {
 		t.Error("function name should be 'FIt'")
 	}
 }
 
 func TestNameHandler_GetFocusContainerName_ident(t *testing.T) {
 	exp := &ast.CallExpr{
-		Fun: ast.NewIdent("FIt"),
+		Fun: ast.NewIdent(fit),
 	}
 
 	h := nameHandler("ginkgo")
-	isFocus, name := h.GetFocusContainerName(exp)
+	isFocus, name := h.getFocusContainerName(exp)
 	if isFocus {
 		t.Error("h.GetFocusContainerName(exp) should return false")
 	}