Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLI: no need to implicitly specify =true for commandline flags #177

Merged
merged 4 commits into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ var _ = Describe("checking something", Focus, func() {
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.
***This rule is disabled by default***. Use the `--forbid-focus-container` command line flag to enable it.

### Comparing values from different types [BUG]

Expand All @@ -202,7 +202,7 @@ using casting, or use the `BeEquivalentTo` matcher.

The linter can't guess what is the best solution in each case, and so it won't auto-fix this warning.

To suppress this warning entirely, use the `--suppress-type-compare-assertion=true` command line parameter.
To suppress this warning entirely, use the `--suppress-type-compare-assertion` command line parameter.

To suppress a specific file or line, use the `// ginkgo-linter:ignore-type-compare-warning` comment (see [below](#suppress-warning-from-the-code))

Expand Down Expand Up @@ -274,7 +274,7 @@ a Gomega object as their first parameter, and returns nothing, e.g. this is a va
***Note***: This rule **does not** support auto-fix.

### Avoid Spec Pollution: Don't Initialize Variables in Container Nodes [BUG/STYLE]:
***Note***: Only applied when the `--forbid-spec-pollution=true` flag is set (disabled by default).
***Note***: Only applied when the `--forbid-spec-pollution` flag is set (disabled by default).

According to [ginkgo documentation](https://onsi.github.io/ginkgo/#avoid-spec-pollution-dont-initialize-variables-in-container-nodes),
no variable should be assigned within a container node (`Describe`, `Context`, `When` or their `F`, `P` or `X` forms)
Expand Down Expand Up @@ -451,7 +451,7 @@ Expect("abc").ShouldNot(BeEmpty()) // => Expect("abc").ToNot(BeEmpty())
```
This rule support auto fixing.

***This rule is disabled by default***. Use the `--force-expect-to=true` command line flag to enable it.
***This rule is disabled by default***. Use the `--force-expect-to` command line flag to enable it.

### Async timing interval: multiple timeout or polling intervals [STYLE]
***Note***: Only applied when the `suppress-async-assertion` flag is **not set** *and* the `validate-async-intervals`
Expand Down Expand Up @@ -522,20 +522,20 @@ will trigger a warning with a suggestion to replace the mather to
```go
Expect(myErrorFunc()).To(Succeed())
```
***This rule is disabled by default***. Use the `--force-succeed=true` command line flag to enable it.
***This rule is disabled by default***. Use the `--force-succeed` command line flag to enable it.

***Note***: This rule **does** support auto-fix, when the `--fix` command line parameter is used.

## Suppress the linter
### Suppress warning from command line
* Use the `--suppress-len-assertion=true` flag to suppress the wrong length and cap assertions warning
* Use the `--suppress-nil-assertion=true` flag to suppress the wrong nil assertion warning
* 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 `--forbid-focus-container=true` flag to activate the focused container assertion (deactivated by default)
* Use the `--suppress-type-compare-assertion=true` to suppress the type compare assertion warning
* Use the `--allow-havelen-0=true` flag to avoid warnings about `HaveLen(0)`; Note: this parameter is only supported from
* Use the `--suppress-len-assertion` flag to suppress the wrong length and cap assertions warning
* Use the `--suppress-nil-assertion` flag to suppress the wrong nil assertion warning
* Use the `--suppress-err-assertion` flag to suppress the wrong error assertion warning
* Use the `--suppress-compare-assertion` flag to suppress the wrong comparison assertion warning
* Use the `--suppress-async-assertion` flag to suppress the function call in async assertion warning
* Use the `--forbid-focus-container` flag to activate the focused container assertion (deactivated by default)
* Use the `--suppress-type-compare-assertion` to suppress the type compare assertion warning
* Use the `--allow-havelen-0` flag to avoid warnings about `HaveLen(0)`; Note: this parameter is only supported from
command line, and not from a comment.

### Suppress warning from the code
Expand Down
26 changes: 12 additions & 14 deletions analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,19 @@ func NewAnalyzer() *analysis.Analyzer {

a := NewAnalyzerWithConfig(config)

var ignored bool
a.Flags.Init("ginkgolinter", flag.ExitOnError)
a.Flags.Var(&config.SuppressLen, "suppress-len-assertion", "Suppress warning for wrong length assertions")
a.Flags.Var(&config.SuppressNil, "suppress-nil-assertion", "Suppress warning for wrong nil assertions")
a.Flags.Var(&config.SuppressErr, "suppress-err-assertion", "Suppress warning for wrong error assertions")
a.Flags.Var(&config.SuppressCompare, "suppress-compare-assertion", "Suppress warning for wrong comparison assertions")
a.Flags.Var(&config.SuppressAsync, "suppress-async-assertion", "Suppress warning for function call in async assertion, like Eventually")
a.Flags.Var(&config.ValidateAsyncIntervals, "validate-async-intervals", "best effort validation of async intervals (timeout and polling); ignored the suppress-async-assertion flag is true")
a.Flags.Var(&config.SuppressTypeCompare, "suppress-type-compare-assertion", "Suppress warning for comparing values from different types, like int32 and uint32")
a.Flags.Var(&config.AllowHaveLen0, "allow-havelen-0", "Do not warn for HaveLen(0); default = false")
a.Flags.Var(&config.ForceExpectTo, "force-expect-to", "force using `Expect` with `To`, `ToNot` or `NotTo`. reject using `Expect` with `Should` or `ShouldNot`; default = false (not forced)")
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(&config.ForbidFocus, "forbid-focus-container", "trigger a warning for ginkgo focus containers like FDescribe, FContext, FWhen or FIt; default = false.")
a.Flags.Var(&config.ForbidSpecPollution, "forbid-spec-pollution", "trigger a warning for variable assignments in ginkgo containers like Describe, Context and When, instead of in BeforeEach(); default = false.")
a.Flags.Var(&config.ForceSucceedForFuncs, "force-succeed", "force using the Succeed matcher for error functions, and the HaveOccurred matcher for non-function error values")
a.Flags.BoolVar(&config.SuppressLen, "suppress-len-assertion", config.SuppressLen, "Suppress warning for wrong length assertions")
a.Flags.BoolVar(&config.SuppressNil, "suppress-nil-assertion", config.SuppressNil, "Suppress warning for wrong nil assertions")
a.Flags.BoolVar(&config.SuppressErr, "suppress-err-assertion", config.SuppressErr, "Suppress warning for wrong error assertions")
a.Flags.BoolVar(&config.SuppressCompare, "suppress-compare-assertion", config.SuppressCompare, "Suppress warning for wrong comparison assertions")
a.Flags.BoolVar(&config.SuppressAsync, "suppress-async-assertion", config.SuppressAsync, "Suppress warning for function call in async assertion, like Eventually")
a.Flags.BoolVar(&config.ValidateAsyncIntervals, "validate-async-intervals", config.ValidateAsyncIntervals, "best effort validation of async intervals (timeout and polling); ignored the suppress-async-assertion flag is true")
a.Flags.BoolVar(&config.SuppressTypeCompare, "suppress-type-compare-assertion", config.SuppressTypeCompare, "Suppress warning for comparing values from different types, like int32 and uint32")
a.Flags.BoolVar(&config.AllowHaveLen0, "allow-havelen-0", config.AllowHaveLen0, "Do not warn for HaveLen(0); default = false")
a.Flags.BoolVar(&config.ForceExpectTo, "force-expect-to", config.ForceExpectTo, "force using `Expect` with `To`, `ToNot` or `NotTo`. reject using `Expect` with `Should` or `ShouldNot`; default = false (not forced)")
a.Flags.BoolVar(&config.ForbidFocus, "forbid-focus-container", config.ForbidFocus, "trigger a warning for ginkgo focus containers like FDescribe, FContext, FWhen or FIt; default = false.")
a.Flags.BoolVar(&config.ForbidSpecPollution, "forbid-spec-pollution", config.ForbidSpecPollution, "trigger a warning for variable assignments in ginkgo containers like Describe, Context and When, instead of in BeforeEach(); default = false.")
a.Flags.BoolVar(&config.ForceSucceedForFuncs, "force-succeed", config.ForceSucceedForFuncs, "force using the Succeed matcher for error functions, and the HaveOccurred matcher for non-function error values")

return a
}
Expand Down
11 changes: 3 additions & 8 deletions internal/expression/actual/actual.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,8 @@ type Actual struct {
actualOffset int
}

func New(origExpr, cloneExpr *ast.CallExpr, orig *ast.CallExpr, clone *ast.CallExpr, pass *analysis.Pass, handler gomegahandler.Handler, timePkg string, errMethodExists bool) (*Actual, bool) {
funcName, ok := handler.GetActualFuncName(orig)
if !ok {
return nil, false
}

arg, actualOffset := getActualArgPayload(orig, clone, pass, funcName, errMethodExists)
func New(origExpr, cloneExpr *ast.CallExpr, orig *ast.CallExpr, clone *ast.CallExpr, pass *analysis.Pass, timePkg string, info *gomegahandler.GomegaBasicInfo) (*Actual, bool) {
arg, actualOffset := getActualArgPayload(orig, clone, pass, info)
if arg == nil {
return nil, false
}
Expand All @@ -45,7 +40,7 @@ func New(origExpr, cloneExpr *ast.CallExpr, orig *ast.CallExpr, clone *ast.CallE
isTuple = tpl.Len() > 1
}

isAsyncExpr := gomegainfo.IsAsyncActualMethod(funcName)
isAsyncExpr := gomegainfo.IsAsyncActualMethod(info.MethodName)

var asyncArg *AsyncArg
if isAsyncExpr {
Expand Down
7 changes: 4 additions & 3 deletions internal/expression/actual/actualarg.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"golang.org/x/tools/go/analysis"

"github.com/nunnatsa/ginkgolinter/internal/expression/value"
"github.com/nunnatsa/ginkgolinter/internal/gomegahandler"
"github.com/nunnatsa/ginkgolinter/internal/gomegainfo"
"github.com/nunnatsa/ginkgolinter/internal/reverseassertion"
)
Expand Down Expand Up @@ -40,15 +41,15 @@ func (a ArgType) Is(val ArgType) bool {
return a&val != 0
}

func getActualArgPayload(origActualExpr, actualExprClone *ast.CallExpr, pass *analysis.Pass, actualMethodName string, errMethodExists bool) (ArgPayload, int) {
origArgExpr, argExprClone, actualOffset, isGomegaExpr := getActualArg(origActualExpr, actualExprClone, actualMethodName, pass)
func getActualArgPayload(origActualExpr, actualExprClone *ast.CallExpr, pass *analysis.Pass, info *gomegahandler.GomegaBasicInfo) (ArgPayload, int) {
origArgExpr, argExprClone, actualOffset, isGomegaExpr := getActualArg(origActualExpr, actualExprClone, info.MethodName, pass)
if !isGomegaExpr {
return nil, 0
}

var arg ArgPayload

if errMethodExists {
if info.HasErrorMethod {
arg = &ErrorMethodPayload{}
} else if value.IsExprError(pass, origArgExpr) {
arg = newErrPayload(origArgExpr, argExprClone, pass)
Expand Down
24 changes: 14 additions & 10 deletions internal/expression/expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ type GomegaExpression struct {
origAssertionFuncName string
actualFuncName string

isAsync bool
isAsync bool
isUsingGomegaVar bool

actual *actual.Actual
matcher *matcher.Matcher
Expand All @@ -36,25 +37,23 @@ type GomegaExpression struct {
}

func New(origExpr *ast.CallExpr, pass *analysis.Pass, handler gomegahandler.Handler, timePkg string) (*GomegaExpression, bool) {
actualMethodName, ok := handler.GetActualFuncName(origExpr)
if !ok || !gomegainfo.IsActualMethod(actualMethodName) {
info, ok := handler.GetGomegaBasicInfo(origExpr)
if !ok || !gomegainfo.IsActualMethod(info.MethodName) {
return nil, false
}

origSel, ok := origExpr.Fun.(*ast.SelectorExpr)
if !ok || !gomegainfo.IsAssertionFunc(origSel.Sel.Name) {
return &GomegaExpression{
orig: origExpr,
actualFuncName: actualMethodName,
actualFuncName: info.MethodName,
}, true
}

exprClone := astcopy.CallExpr(origExpr)
selClone := exprClone.Fun.(*ast.SelectorExpr)

errMethodExists := false

origActual := handler.GetActualExpr(origSel, &errMethodExists)
origActual := handler.GetActualExpr(origSel)
if origActual == nil {
return nil, false
}
Expand All @@ -64,7 +63,7 @@ func New(origExpr *ast.CallExpr, pass *analysis.Pass, handler gomegahandler.Hand
return nil, false
}

actl, ok := actual.New(origExpr, exprClone, origActual, actualClone, pass, handler, timePkg, errMethodExists)
actl, ok := actual.New(origExpr, exprClone, origActual, actualClone, pass, timePkg, info)
if !ok {
return nil, false
}
Expand All @@ -89,9 +88,10 @@ func New(origExpr *ast.CallExpr, pass *analysis.Pass, handler gomegahandler.Hand

assertionFuncName: origSel.Sel.Name,
origAssertionFuncName: origSel.Sel.Name,
actualFuncName: actualMethodName,
actualFuncName: info.MethodName,

isAsync: actl.IsAsync(),
isAsync: actl.IsAsync(),
isUsingGomegaVar: info.UseGomegaVar,

actual: actl,
matcher: mtchr,
Expand Down Expand Up @@ -135,6 +135,10 @@ func (e *GomegaExpression) IsAsync() bool {
return e.isAsync
}

func (e *GomegaExpression) IsUsingGomegaVar() bool {
return e.isUsingGomegaVar
}

func (e *GomegaExpression) ReverseAssertionFuncLogic() {
assertionFunc := e.clone.Fun.(*ast.SelectorExpr).Sel
newName := reverseassertion.ChangeAssertionLogic(assertionFunc.Name)
Expand Down
6 changes: 3 additions & 3 deletions internal/expression/matcher/matcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,13 @@ func New(origMatcher, matcherClone *ast.CallExpr, pass *analysis.Pass, handler g
reverse := false
var assertFuncName string
for {
ok := false
assertFuncName, ok = handler.GetActualFuncName(origMatcher)
info, ok := handler.GetGomegaBasicInfo(origMatcher)
if !ok {
return nil, false
}

if assertFuncName != "Not" {
if info.MethodName != "Not" {
assertFuncName = info.MethodName
break
}

Expand Down
4 changes: 2 additions & 2 deletions internal/ginkgohandler/handling.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ const (
func handleGinkgoSpecs(expr ast.Expr, config types.Config, pass *analysis.Pass, ginkgoHndlr Handler) bool {
goDeeper := false
if exp, ok := expr.(*ast.CallExpr); ok {
if bool(config.ForbidFocus) && checkFocusContainer(pass, ginkgoHndlr, exp) {
if config.ForbidFocus && checkFocusContainer(pass, ginkgoHndlr, exp) {
goDeeper = true
}

if bool(config.ForbidSpecPollution) && checkAssignmentsInContainer(pass, ginkgoHndlr, exp) {
if config.ForbidSpecPollution && checkAssignmentsInContainer(pass, ginkgoHndlr, exp) {
goDeeper = true
}
}
Expand Down
48 changes: 27 additions & 21 deletions internal/gomegahandler/dothandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,34 @@ type dotHandler struct {
pass *analysis.Pass
}

// GetActualFuncName returns the name of the gomega function, e.g. `Expect`
func (h dotHandler) GetActualFuncName(expr *ast.CallExpr) (string, bool) {
switch actualFunc := expr.Fun.(type) {
case *ast.Ident:
return actualFunc.Name, true
case *ast.SelectorExpr:
if h.isGomegaVar(actualFunc.X) {
return actualFunc.Sel.Name, true
}
// GetGomegaBasicInfo returns the name of the gomega function, e.g. `Expect` + some additional info
func (h dotHandler) GetGomegaBasicInfo(expr *ast.CallExpr) (*GomegaBasicInfo, bool) {
info := &GomegaBasicInfo{}
for {
switch actualFunc := expr.Fun.(type) {
case *ast.Ident:
info.MethodName = actualFunc.Name
return info, true
case *ast.SelectorExpr:
if h.isGomegaVar(actualFunc.X) {
info.UseGomegaVar = true
info.MethodName = actualFunc.Sel.Name
return info, true
}

if x, ok := actualFunc.X.(*ast.CallExpr); ok {
return h.GetActualFuncName(x)
}
if actualFunc.Sel.Name == "Error" {
info.HasErrorMethod = true
}

case *ast.CallExpr:
return h.GetActualFuncName(actualFunc)
if x, ok := actualFunc.X.(*ast.CallExpr); ok {
expr = x
} else {
return nil, false
}
default:
return nil, false
}
}
return "", false
}

// ReplaceFunction replaces the function with another one, for fix suggestions
Expand All @@ -51,7 +61,7 @@ func (dotHandler) GetNewWrapperMatcher(name string, existing *ast.CallExpr) *ast
}
}

func (h dotHandler) GetActualExpr(assertionFunc *ast.SelectorExpr, errMethodExists *bool) *ast.CallExpr {
func (h dotHandler) GetActualExpr(assertionFunc *ast.SelectorExpr) *ast.CallExpr {
actualExpr, ok := assertionFunc.X.(*ast.CallExpr)
if !ok {
return nil
Expand All @@ -66,11 +76,7 @@ func (h dotHandler) GetActualExpr(assertionFunc *ast.SelectorExpr, errMethodExis
return actualExpr
}
} else {
if fun.Sel.Name == "Error" {
*errMethodExists = true
}

return h.GetActualExpr(fun, errMethodExists)
return h.GetActualExpr(fun)
}
}
return nil
Expand Down
10 changes: 8 additions & 2 deletions internal/gomegahandler/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,23 @@ const (
// in imported with "." name, custom name or without any name.
type Handler interface {
// GetActualFuncName returns the name of the gomega function, e.g. `Expect`
GetActualFuncName(*ast.CallExpr) (string, bool)
GetGomegaBasicInfo(*ast.CallExpr) (*GomegaBasicInfo, bool)
// ReplaceFunction replaces the function with another one, for fix suggestions
ReplaceFunction(*ast.CallExpr, *ast.Ident)

GetActualExpr(assertionFunc *ast.SelectorExpr, errMethodExists *bool) *ast.CallExpr
GetActualExpr(assertionFunc *ast.SelectorExpr) *ast.CallExpr

GetActualExprClone(origFunc, funcClone *ast.SelectorExpr) *ast.CallExpr

GetNewWrapperMatcher(name string, existing *ast.CallExpr) *ast.CallExpr
}

type GomegaBasicInfo struct {
MethodName string
UseGomegaVar bool
HasErrorMethod bool
}

// GetGomegaHandler returns a gomegar handler according to the way gomega was imported in the specific file
func GetGomegaHandler(file *ast.File, pass *analysis.Pass) Handler {
for _, imp := range file.Imports {
Expand Down
Loading
Loading