Skip to content

Commit

Permalink
Improve handling of if
Browse files Browse the repository at this point in the history
  • Loading branch information
bombsimon committed Jun 6, 2024
1 parent 6d6e70e commit 1e3a52a
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 69 deletions.
9 changes: 9 additions & 0 deletions testdata/src/default_config/if.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,12 @@ func fn3() {
panic(err)
}
}

func fn4() {
if foo := 1; foo != 2 {
panic(foo)
}
if foo := 2; foo != 2 { // want "missing whitespace decreases readability"
panic(bar)
}
}
10 changes: 10 additions & 0 deletions testdata/src/default_config/if.go.golden
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,13 @@ func fn3() {
panic(err)
}
}

func fn4() {
if foo := 1; foo != 2 {
panic(foo)
}

if foo := 2; foo != 2 { // want "missing whitespace decreases readability"
panic(bar)
}
}
7 changes: 7 additions & 0 deletions testdata/src/default_config/leading_whitespace.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,10 @@ func fn5() {
// Comment without space before or after.
fmt.Println("Hello, World")
}

func fn6() {
if true { // want "unnecessary whitespace decreases readability"

_ = 1
}
}
6 changes: 6 additions & 0 deletions testdata/src/default_config/leading_whitespace.go.golden
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,9 @@ func fn5() {
// Comment without space before or after.
fmt.Println("Hello, World")
}

func fn6() {
if true { // want "unnecessary whitespace decreases readability"
_ = 1
}
}
9 changes: 1 addition & 8 deletions testdata/src/wip/wip.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,3 @@
package testpkg

func fn1() {
if foo := 1; foo != 2 {
panic(foo)
}
if foo := 2; foo != 2 { // want "missing whitespace decreases readability"
panic(bar)
}
}
func fn1() {}
10 changes: 1 addition & 9 deletions testdata/src/wip/wip.go.golden
Original file line number Diff line number Diff line change
@@ -1,11 +1,3 @@
package testpkg

func fn1() {
if foo := 1; foo != 2 {
panic(foo)
}

if foo := 2; foo != 2 { // want "missing whitespace decreases readability"
panic(bar)
}
}
func fn1() {}
128 changes: 76 additions & 52 deletions wsl.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,42 +71,58 @@ func (w *WSL) CheckIf(stmt *ast.IfStmt, cursor *Cursor) {
reset := cursor.Save()

currentIdents := allIdents(cursor.Stmt())

previousIdents := []*ast.Ident{}

var previousNode ast.Node = nil

if cursor.Previous() {
previousIdents = allIdents(cursor.Stmt())
cursor.Next()
previousNode = cursor.Stmt()
previousIdents = allIdents(previousNode)

cursor.Next() // Move forward again
}

_, prevIsAssign := previousNode.(*ast.AssignStmt)
_, prevIsDecl := previousNode.(*ast.DeclStmt)
n := w.numberOfStatementsAbove(cursor)

if n > 0 {
intersects := identIntersection(currentIdents, previousIdents)
if prevIsAssign || prevIsDecl {
intersects := identIntersection(currentIdents, previousIdents)

// No idents above share name with one in the if statement.
if len(intersects) == 0 {
w.addError(
stmt.Pos(),
stmt.Pos(),
stmt.Pos(),
MessageAddWhitespace,
)
}
// No idents above share name with one in the if statement.
if len(intersects) == 0 {
w.addError(
stmt.Pos(),
stmt.Pos(),
stmt.Pos(),
MessageAddWhitespace,
)
}

// Idents on the line above exist in the current condition so that
// should remain cuddled.
if len(intersects) > 0 {
cursor.Previous()
// Idents on the line above exist in the current condition so that
// should remain cuddled.
if len(intersects) > 0 {
cursor.Previous()

w.addError(
cursor.Stmt().Pos(),
cursor.Stmt().Pos(),
cursor.Stmt().Pos(),
MessageAddWhitespace,
)
}
// TODO: Features:
// * Allow idents that is used first in the block
// * OR - if configured - anywhere in the block.
} else {
w.addError(
cursor.Stmt().Pos(),
cursor.Stmt().Pos(),
cursor.Stmt().Pos(),
MessageAddWhitespace,
)
}
// TODO: Features:
// * Allow idents that is used first in the block
// * OR - if configured - anywhere in the block.
}

if w.Config.Errcheck && n == 0 && len(previousIdents) > 0 {
Expand Down Expand Up @@ -148,6 +164,8 @@ func (w *WSL) CheckIf(stmt *ast.IfStmt, cursor *Cursor) {

reset()

w.CheckUnnecessaryBlockLeadingNewline(stmt.Body)

// if
w.CheckBlock(stmt.Body)

Expand All @@ -157,46 +175,52 @@ func (w *WSL) CheckIf(stmt *ast.IfStmt, cursor *Cursor) {
w.CheckIf(v, cursor)
// else
case *ast.BlockStmt:
w.CheckUnnecessaryBlockLeadingNewline(v)
w.CheckBlock(v)
}
}

func (w *WSL) CheckBlock(block *ast.BlockStmt) {
cursor := NewCursor(0, block.List)
cursor := NewCursor(-1, block.List)
for cursor.Next() {
fmt.Printf("%d: %T\n", cursor.currentIdx, cursor.Stmt())
w.CheckStmt(cursor.Stmt(), cursor)
}
}

//nolint:gocritic // This is not commented out code, it's examples
switch s := cursor.Stmt().(type) {
// if a {} else if b {} else {}
case *ast.IfStmt:
w.CheckIf(s, cursor)
// for {} / for a; b; c {}
case *ast.ForStmt:
// for _, _ = range a {}
case *ast.RangeStmt:
// switch {} // switch a {}
case *ast.SwitchStmt:
// switch a.(type) {}
case *ast.TypeSwitchStmt:
// return a
case *ast.ReturnStmt:
// continue / break
case *ast.BranchStmt:
// var a
case *ast.DeclStmt:
// a := a
case *ast.AssignStmt:
// a++ / a--
case *ast.IncDecStmt:
// defer func() {}
case *ast.DeferStmt:
// go func() {}
case *ast.GoStmt:
case *ast.ExprStmt:
default:
fmt.Printf("Not implemented: %T\n", s)
}
func (w *WSL) CheckStmt(stmt ast.Stmt, cursor *Cursor) {
//nolint:gocritic // This is not commented out code, it's examples
switch s := stmt.(type) {
// if a {} else if b {} else {}
case *ast.IfStmt:
w.CheckIf(s, cursor)
// for {} / for a; b; c {}
case *ast.ForStmt:
// for _, _ = range a {}
case *ast.RangeStmt:
// switch {} // switch a {}
case *ast.SwitchStmt:
// switch a.(type) {}
case *ast.TypeSwitchStmt:
// return a
case *ast.ReturnStmt:
// continue / break
case *ast.BranchStmt:
// var a
case *ast.DeclStmt:
// a := a
case *ast.AssignStmt:
// a++ / a--
case *ast.IncDecStmt:
// defer func() {}
case *ast.DeferStmt:
// go func() {}
case *ast.GoStmt:
case *ast.ExprStmt:
case *ast.BlockStmt:
w.CheckBlock(s)
default:
fmt.Printf("Not implemented: %T\n", s)
}
}

Expand Down

0 comments on commit 1e3a52a

Please sign in to comment.