Skip to content

Commit

Permalink
Only enforce err cuddling if the statement above was an assignment
Browse files Browse the repository at this point in the history
Fixes #78
  • Loading branch information
bombsimon committed Apr 17, 2020
1 parent 320b4e5 commit 3e16ba4
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 0 deletions.
12 changes: 12 additions & 0 deletions wsl.go
Expand Up @@ -398,6 +398,18 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) {
if !cuddledWithLastStmt {
checkingErr := atLeastOneInListsMatch(rightAndLeftHandSide, p.config.ErrorVariableNames)
if checkingErr {
// We only want to enforce cuddling error checks if the
// error was assigned on the line above. See
// https://github.com/bombsimon/wsl/issues/78.
// This is needed since `assignedOnLineAbove` is not
// actually just assignments but everything from LHS in the
// previous statement. This means that if previous line was
// `if err ...`, `err` will now be in the list
// `assignedOnLineAbove`.
if _, ok := previousStatement.(*ast.AssignStmt); !ok {
continue
}

if checkingErrInitializedInline() {
continue
}
Expand Down
44 changes: 44 additions & 0 deletions wsl_test.go
Expand Up @@ -1674,6 +1674,50 @@ func TestWithConfig(t *testing.T) {
}
}`),
},
{
description: "only warn about cuddling errors if it's an expression above",
customConfig: &Configuration{
ForceCuddleErrCheckAndAssign: true,
ErrorVariableNames: []string{"err"},
},
code: []byte(`package main
var ErrSomething error
func validateErr(err error) {
if err == nil {
return
}
if errors.Is(err, ErrSomething) {
return
}
// Should be valid since the if actually is cuddled so the error
// check assumes something right is going on here. If
// anotherThingToCheck wasn't used in the if statement we would
// get a regular if cuddle error.
anotherThingToCheck := someFunc()
if multiCheck(anotherThingToCheck, err) {
fmt.Println("this must be OK, err is not assigned above")
}
notUsedInNextIf := someFunc()
if multiCheck(anotherThingToCheck, err) {
fmt.Println("this should not warn, we didn't assign err above")
}
// This fails under the cuddle if rule.
if err != nil {
return
}
if !errors.Is(err, ErrSomething) {
return
}
}`),
expectedErrorStrings: []string{reasonOnlyCuddleIfWithAssign},
},
}

for _, tc := range cases {
Expand Down

0 comments on commit 3e16ba4

Please sign in to comment.