Skip to content

Commit

Permalink
WIP: Removal fixer
Browse files Browse the repository at this point in the history
* Remove newline in end of block
* Remove newline in start of block (and comments...)

This is WIP because we need to preserve the comments so they don't get
deleted when we remove beginning of blocks.
  • Loading branch information
bombsimon committed Feb 15, 2023
1 parent 07a9043 commit eab3161
Show file tree
Hide file tree
Showing 8 changed files with 112 additions and 78 deletions.
30 changes: 12 additions & 18 deletions analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package wsl

import (
"flag"
"go/token"

"golang.org/x/tools/go/analysis"
)
Expand Down Expand Up @@ -55,40 +54,35 @@ func run(pass *analysis.Pass) (interface{}, error) {

for _, v := range processor.Result {
var (
pos token.Pos
end token.Pos
newText []byte
textEdits []analysis.TextEdit
)

//nolint:exhaustive // Not while TODO
switch v.Type {
case WhitespaceShouldAddBefore:
pos = v.FixNode.Pos()
end = v.FixNode.Pos()
newText = []byte("\n")
case WhitespaceShouldAddAfter:
pos = v.FixNode.End()
end = v.FixNode.End()
newText = []byte("\n")
case WhitespaceShouldRemoveEnd:
newText = []byte("\n}")
case WhitespaceShouldRemoveBeginning:
newText = []byte("{\n")
default:
//nolint:gocritic // We need TODOs while iterating...
//nolint:gocritic // Not while TODO
// TODO
continue
}

if !v.NoFix {
textEdits = []analysis.TextEdit{
{
Pos: pos,
End: end,
NewText: newText,
},
}
textEdits = []analysis.TextEdit{
{
Pos: v.FixRangeStart,
End: v.FixRangeEnd,
NewText: newText,
},
}

d := analysis.Diagnostic{
Pos: v.Node.Pos(),
Pos: v.ReportAt,
Category: "",
Message: v.Reason,
SuggestedFixes: []analysis.SuggestedFix{
Expand Down
8 changes: 6 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,17 @@ go 1.19

require github.com/stretchr/testify v1.8.1

require (
golang.org/x/mod v0.8.0 // indirect
golang.org/x/sys v0.5.0 // indirect
)

require (
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/kr/text v0.2.0 // indirect
github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
golang.org/x/tools v0.0.0-20200403190813-44a64ad78b9b
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 // indirect
golang.org/x/tools v0.6.0
gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
27 changes: 7 additions & 20 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,13 @@ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/
github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk=
github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
golang.org/x/mod v0.2.0 h1:KU7oHjnv3XNWfa5COkzUifxZmxp1TyI7ImMXqFxLwvQ=
golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
golang.org/x/tools v0.0.0-20200403190813-44a64ad78b9b h1:AFZdJUT7jJYXQEC29hYH/WZkoV7+KhwxQGmdZ19yYoY=
golang.org/x/tools v0.0.0-20200403190813-44a64ad78b9b/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/mod v0.8.0 h1:LUYupSeNrTNCGzR/hVBk2NHZO4hXcVaW1k4Qx7rjPx8=
golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
golang.org/x/sync v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o=
golang.org/x/sys v0.5.0 h1:MUK/U/4lj1t1oPg0HfuXDN/Z1wv31ZJ/YcPiGccS4DU=
golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/tools v0.6.0 h1:BOw41kyTf3PuCW1pVQf8+Cyg8pMlkYB1oo9iJ6D/lKM=
golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f h1:BLraFXnmrev5lT+xlilqcH8XK9/i0At2xKjWk4p6zsU=
gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
Expand Down
2 changes: 1 addition & 1 deletion testdata/fix_advanced.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package testpkg

func Advanced() {
var foo = 1
var bar = 2 // want "declarations should never be cuddled"
var bar = 2 // want "declarations should never be cuddled"
var biz int // want "declarations should never be cuddled"

x := []string{}
Expand Down
26 changes: 26 additions & 0 deletions testdata/remove_whitespace.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package testpkg

import "fmt"

func RemoveWhitespaceNoComments() { // want "block should not start with a whitespace"

a := 1
if a < 2 { // want "block should not start with a whitespace"

a = 2

} // want "block should not end with a whitespace"

switch { // want "block should not start with a whitespace"

case true:
fmt.Println("true")

default:
fmt.Println("false")

} // want "block should not end with a whitespace"

_ = a

} // want "block should not end with a whitespace"
20 changes: 20 additions & 0 deletions testdata/remove_whitespace.go.golden
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package testpkg

import "fmt"

func RemoveWhitespaceNoComments() {
a := 1
if a < 2 {
a = 2
} // want "block should not end with a whitespace"

switch {
case true:
fmt.Println("true")

default:
fmt.Println("false")
} // want "block should not end with a whitespace"

_ = a
} // want "block should not end with a whitespace"
75 changes: 39 additions & 36 deletions wsl.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,11 +222,12 @@ func DefaultConfig() Configuration {

// Result represents the result of one error.
type Result struct {
Node ast.Node
FixNode ast.Node
Reason string
NoFix bool
Type ErrorType
ReportAt token.Pos
FixRangeStart token.Pos
FixRangeEnd token.Pos
Reason string
NoFix bool
Type ErrorType
}

// Processor is the type that keeps track of the file and fileset and holds the
Expand Down Expand Up @@ -416,20 +417,20 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) {
// it was and use *that* statement's position
if p.config.ForceExclusiveShortDeclarations && cuddledWithLastStmt {
if p.isShortDecl(stmt) && !p.isShortDecl(previousStatement) {
p.addError(
stmt,
nil,
p.addErrorRange(
stmt.Pos(),
stmt.End(),
stmt.End(),
reasonShortDeclNotExclusive,
WhitespaceShouldAddAfter,
false,
)
} else if p.isShortDecl(previousStatement) && !p.isShortDecl(stmt) {
p.addError(
previousStatement,
nil,
p.addErrorRange(
previousStatement.Pos(),
previousStatement.Pos(),
previousStatement.Pos(),
reasonShortDeclNotExclusive,
WhitespaceShouldAddBefore,
false,
)
}
}
Expand All @@ -449,7 +450,7 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) {
// If the variable on the line above is allowed to be
// cuddled, break two lines above so we keep the proper
// cuddling.
p.addWhitespaceBeforeFixOtherNodeError(n1, n2, reason)
p.addErrorRange(n1.Pos(), n2.Pos(), n2.Pos(), reason, WhitespaceShouldAddBefore)
} else {
// If not, break here so we separate the cuddled variable.
p.addWhitespaceBeforeError(n1, reason)
Expand Down Expand Up @@ -1140,7 +1141,15 @@ func (p *Processor) findLeadingAndTrailingWhitespaces(ident *ast.Ident, stmt, ne
// And now if the first statement is passed the number of allowed lines,
// then we had extra WS, possibly before the first comment group.
if p.nodeStart(firstStatement) > blockStartLine+allowedLinesBeforeFirstStatement {
p.addError(stmt, nil, reasonBlockStartsWithWS, WhitespaceShouldRemoveBeginning, false)
// TODO: We need to pick a better start of the range so we don't delete
// potential comments.
p.addErrorRange(
blockStartPos,
blockStartPos,
firstStatement.Pos(),
reasonBlockStartsWithWS,
WhitespaceShouldRemoveBeginning,
)
}

// If the blockEndLine is not 0 we're a regular block (not case).
Expand All @@ -1163,7 +1172,13 @@ func (p *Processor) findLeadingAndTrailingWhitespaces(ident *ast.Ident, stmt, ne
}

if p.nodeEnd(lastStatement) != blockEndLine-1 && !isExampleFunc(ident) {
p.addError(stmt, nil, reasonBlockEndsWithWS, WhitespaceShouldRemoveEnd, false)
p.addErrorRange(
stmt.End(),
lastStatement.End(),
stmt.End(),
reasonBlockEndsWithWS,
WhitespaceShouldRemoveEnd,
)
}

return
Expand Down Expand Up @@ -1278,32 +1293,20 @@ func isEmptyLabeledStmt(node ast.Node) bool {
}

func (p *Processor) addWhitespaceBeforeError(node ast.Node, reason string) {
p.addError(node, nil, reason, WhitespaceShouldAddBefore, false)
}

func (p *Processor) addWhitespaceBeforeFixOtherNodeError(reportNode, fixNode ast.Node, reason string) {
p.addError(reportNode, fixNode, reason, WhitespaceShouldAddBefore, false)
p.addErrorRange(node.Pos(), node.Pos(), node.Pos(), reason, WhitespaceShouldAddBefore)
}

func (p *Processor) addWhitespaceAfterError(node ast.Node, reason string) {
p.addError(node, nil, reason, WhitespaceShouldAddAfter, false)
p.addErrorRange(node.Pos(), node.End(), node.End(), reason, WhitespaceShouldAddAfter)
}

// Add an error for the file and line number for the current token.Pos with the
// given reason.
//
//nolint:unparam // We will potentially use this in the future.
func (p *Processor) addError(reportNode, fixNode ast.Node, reason string, errType ErrorType, noFix bool) {
if fixNode == nil {
fixNode = reportNode
}

func (p *Processor) addErrorRange(reportAt, start, end token.Pos, reason string, errType ErrorType) {
p.Result = append(p.Result, Result{
Node: reportNode,
FixNode: fixNode,
Reason: reason,
NoFix: noFix,
Type: errType,
ReportAt: reportAt,
FixRangeStart: start,
FixRangeEnd: end,
Reason: reason,
Type: errType,
})
}

Expand Down
2 changes: 1 addition & 1 deletion wsl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2064,7 +2064,7 @@ func TestTODO(t *testing.T) {
t.Logf("WARNINGS: %s", p.Warnings)

for _, r := range p.Result {
fmt.Println(r.Node, r.Reason)
fmt.Println(r.FixRangeStart, r.Reason)
}

require.Len(t, p.Result, len(tc.expectedErrorStrings), "correct amount of errors found")
Expand Down

0 comments on commit eab3161

Please sign in to comment.