Skip to content

Commit

Permalink
Support to cuddling if used anywhere in block
Browse files Browse the repository at this point in the history
  • Loading branch information
bombsimon committed Jan 9, 2025
1 parent 51074f9 commit b5e8e09
Show file tree
Hide file tree
Showing 8 changed files with 181 additions and 39 deletions.
4 changes: 4 additions & 0 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const (
CheckReturn
CheckSwitch
CheckTypeSwitch
CheckWholeBlock
)

/*
Expand Down Expand Up @@ -127,6 +128,7 @@ func DefaultChecks() CheckSet {
func AllChecks() CheckSet {
c := DefaultChecks()
c.Add(CheckErr)
c.Add(CheckWholeBlock)

return c
}
Expand Down Expand Up @@ -179,6 +181,8 @@ func CheckFromString(s string) (CheckType, error) {
return CheckSwitch, nil
case "type-switch":
return CheckTypeSwitch, nil
case "whole-block":
return CheckWholeBlock, nil
default:
return CheckInvalid, fmt.Errorf("invalid check '%s'", s)
}
Expand Down
12 changes: 10 additions & 2 deletions cursor.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,20 @@ var ErrCursourOutObFounds = errors.New("out of bounds")
type Cursor struct {
currentIdx int
statements []ast.Stmt
idents map[string]struct{}
}

func NewCursor(i int, statements []ast.Stmt) *Cursor {
func NewCursor(statements []ast.Stmt) *Cursor {
return &Cursor{
currentIdx: i,
currentIdx: -1,
statements: statements,
idents: make(map[string]struct{}),
}
}

func (c *Cursor) AddIdents(idents []*ast.Ident) {
for _, i := range idents {
c.idents[i.Name] = struct{}{}
}
}

Expand Down
10 changes: 5 additions & 5 deletions cursor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
)

func TestSavestate(t *testing.T) {
cursor := NewCursor(0, []ast.Stmt{
cursor := NewCursor([]ast.Stmt{
&ast.AssignStmt{},
&ast.AssignStmt{},
&ast.AssignStmt{},
Expand Down Expand Up @@ -36,14 +36,14 @@ func TestSavestate(t *testing.T) {
cursor.Next()
cursor.Next()

assert.Equal(t, 6, cursor.currentIdx)
assert.Equal(t, 5, cursor.currentIdx)
}()

assert.Equal(t, 4, cursor.currentIdx)
assert.Equal(t, 3, cursor.currentIdx)
}()

assert.Equal(t, 2, cursor.currentIdx)
assert.Equal(t, 1, cursor.currentIdx)

reset()
assert.Equal(t, 0, cursor.currentIdx)
assert.Equal(t, -1, cursor.currentIdx)
}
33 changes: 17 additions & 16 deletions testdata/src/with_config/no_check_decl/no_check_decl.go.golden
Original file line number Diff line number Diff line change
@@ -1,22 +1,23 @@
package testpkg

func fn1() {
var a = 1
var b = 2
const c = 3
const d = 4
import "fmt"

func Fn() {}

func fn1() {
a := 1
if true {
_ = 1
fmt.Println(a)
}

e := 5
var f = 6
g := 7
b := 2
for range make([]int, 10) {
_ = 1
if false {
Fn()

_ = a
_ = b
_ = c
_ = d
_ = e
_ = f
_ = g
Fn(b)
}
}
}

39 changes: 39 additions & 0 deletions testdata/src/with_config/whole_block/block.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package testpkg

func Fn(_ int) {}

func fn1() {
a := 1
if true {
if false {
b := 2
if true { // want "missing whitespace decreases readability"
Fn(a)
}

_ = b
}
}

y := 2
if true { // want "missing whitespace decreases readability"
if false {
a = 1
if true {
Fn(a)
}
}
}

i := 0
if true {
_ = 1
_ = 2
_ = 3

i++
}

_ = x
_ = y
}
41 changes: 41 additions & 0 deletions testdata/src/with_config/whole_block/block.go.golden
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package testpkg

func Fn(_ int) {}

func fn1() {
a := 1
if true {
if false {
b := 2

if true { // want "missing whitespace decreases readability"
Fn(a)
}

_ = b
}
}

y := 2

if true { // want "missing whitespace decreases readability"
if false {
a = 1
if true {
Fn(a)
}
}
}

i := 0
if true {
_ = 1
_ = 2
_ = 3

i++
}

_ = x
_ = y
}
75 changes: 59 additions & 16 deletions wsl.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,21 @@ func (w *WSL) checkCuddlingWithDecl(

// TODO: Features:
// * Allow idents that is used first in the block
// * OR - if configured - anywhere in the block.

// FEATURE: Enable identifier used anywhere in block.
// The cursor has already parsed the block we're comparing so let's see if
// the ident is used anywhere in the block.
if _, ok := w.Config.Checks[CheckWholeBlock]; ok {
anyIntersects := identsInMap(previousIdents, cursor.idents)
if len(anyIntersects) > 0 {
// fmt.Printf("Valid since we have %v any in the block\n", previousIdents)
if numStmtsAbove > maxAllowedStatements {
w.addError(previousNode.Pos(), previousNode.Pos(), previousNode.Pos(), MessageAddWhitespace)
}

return
}
}

// We're cuddled but the line immediately above doesn't contain any
// variables used in this statement.
Expand Down Expand Up @@ -230,20 +244,24 @@ func (w *WSL) CheckFunc(funcDecl *ast.FuncDecl) {
}

func (w *WSL) CheckIf(stmt *ast.IfStmt, cursor *Cursor) {
if _, ok := w.Config.Checks[CheckIf]; ok {
w.CheckCuddling(stmt, cursor, 1)
}

// if
w.CheckBlock(stmt.Body)
cursor.idents = w.CheckBlock(stmt.Body)

switch v := stmt.Else.(type) {
// else-if
case *ast.IfStmt:
w.CheckIf(v, cursor)
// else
case *ast.BlockStmt:
w.CheckBlock(v)
elseIdents := w.CheckBlock(v)

for k, v := range elseIdents {
cursor.idents[k] = v
}
}

if _, ok := w.Config.Checks[CheckIf]; ok {
w.CheckCuddling(stmt, cursor, 1)
}
}

Expand All @@ -258,7 +276,7 @@ func (w *WSL) CheckFor(stmt *ast.ForStmt, cursor *Cursor) {
}

func (w *WSL) CheckRange(stmt *ast.RangeStmt, cursor *Cursor) {
defer w.CheckBlock(stmt.Body)
cursor.idents = w.CheckBlock(stmt.Body)

if _, ok := w.Config.Checks[CheckRange]; !ok {
return
Expand All @@ -268,7 +286,7 @@ func (w *WSL) CheckRange(stmt *ast.RangeStmt, cursor *Cursor) {
}

func (w *WSL) CheckSwitch(stmt *ast.SwitchStmt, cursor *Cursor) {
defer w.CheckBlock(stmt.Body)
cursor.idents = w.CheckBlock(stmt.Body)

if _, ok := w.Config.Checks[CheckSwitch]; !ok {
return
Expand All @@ -278,7 +296,7 @@ func (w *WSL) CheckSwitch(stmt *ast.SwitchStmt, cursor *Cursor) {
}

func (w *WSL) CheckTypeSwitch(stmt *ast.TypeSwitchStmt, cursor *Cursor) {
defer w.CheckBlock(stmt.Body)
cursor.idents = w.CheckBlock(stmt.Body)

if _, ok := w.Config.Checks[CheckTypeSwitch]; !ok {
return
Expand Down Expand Up @@ -370,14 +388,17 @@ func (w *WSL) CheckDecl(stmt *ast.DeclStmt, cursor *Cursor) {
w.CheckCuddlingNoDecl(stmt, cursor, 1)
}

func (w *WSL) CheckBlock(block *ast.BlockStmt) {
func (w *WSL) CheckBlock(block *ast.BlockStmt) map[string]struct{} {
w.CheckBlockLeadingNewline(block)
w.CheckTrailingNewline(block)

cursor := NewCursor(-1, block.List)
cursor := NewCursor(block.List)
for cursor.Next() {
w.CheckStmt(cursor.Stmt(), cursor)
cursor.AddIdents(allIdents(cursor.Stmt()))
}

return cursor.idents
}

func (w *WSL) CheckReturn(stmt *ast.ReturnStmt, cursor *Cursor) {
Expand Down Expand Up @@ -462,7 +483,7 @@ func (w *WSL) CheckCase(stmt *ast.CaseClause, cursor *Cursor) {
return
}

cursor = NewCursor(-1, stmt.Body)
cursor = NewCursor(stmt.Body)
for cursor.Next() {
w.CheckStmt(cursor.Stmt(), cursor)
}
Expand Down Expand Up @@ -515,7 +536,7 @@ func (w *WSL) CheckStmt(stmt ast.Stmt, cursor *Cursor) {
w.CheckCase(s, cursor)
// { }
case *ast.BlockStmt:
w.CheckBlock(s)
cursor.idents = w.CheckBlock(s)
default:
fmt.Printf("Not implemented stmt: %T\n", s)
}
Expand All @@ -525,7 +546,7 @@ func (w *WSL) CheckExpr(expr ast.Expr, cursor *Cursor) {
switch s := expr.(type) {
// func() {}
case *ast.FuncLit:
w.CheckBlock(s.Body)
cursor.idents = w.CheckBlock(s.Body)
// Call(args...)
case *ast.CallExpr:
for _, e := range s.Args {
Expand Down Expand Up @@ -778,7 +799,17 @@ func allIdents(node ast.Node) []*ast.Ident {
for _, elt := range n.Elts {
idents = append(idents, allIdents(elt)...)
}
case *ast.BasicLit, *ast.FuncLit, *ast.IncDecStmt, *ast.BranchStmt,
case *ast.IncDecStmt:
idents = allIdents(n.X)
case *ast.CaseClause:
for _, expr := range n.List {
idents = append(idents, allIdents(expr)...)
}
case *ast.ReturnStmt:
for _, r := range n.Results {
idents = append(idents, allIdents(r)...)
}
case *ast.BasicLit, *ast.FuncLit, *ast.BranchStmt,
*ast.ArrayType:
default:
spew.Dump(node)
Expand All @@ -801,3 +832,15 @@ func identIntersection(a, b []*ast.Ident) []*ast.Ident {

return intersects
}

func identsInMap(a []*ast.Ident, m map[string]struct{}) []*ast.Ident {
intersects := []*ast.Ident{}

for _, as := range a {
if _, ok := m[as.Name]; ok {
intersects = append(intersects, as)
}
}

return intersects
}
6 changes: 6 additions & 0 deletions wsl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,12 @@ func TestWithConfig(t *testing.T) {
config.Checks.Remove(CheckDecl)
},
},
{
subdir: "whole_block",
configFn: func(config *Configuration) {
config.Checks.Add(CheckWholeBlock)
},
},
} {
t.Run(tc.subdir, func(t *testing.T) {
config := NewConfig()
Expand Down

0 comments on commit b5e8e09

Please sign in to comment.