Update WSL to v1.2.1 (#794)

* Update WSL to v1.2.1

* Add new tests for fixed false positives, don't derive defaults from WSL
This commit is contained in:
Simon Sawert 2019-10-08 03:22:44 +02:00 committed by Trevor Pounds
parent 7577d548a3
commit d4b4ad8dfe
12 changed files with 344 additions and 126 deletions

View File

@ -211,6 +211,15 @@ linters-settings:
whitespace: whitespace:
multi-if: false # Enforces newlines (or comments) after every multi-line if statement multi-if: false # Enforces newlines (or comments) after every multi-line if statement
multi-func: false # Enforces newlines (or comments) after every multi-line function signature multi-func: false # Enforces newlines (or comments) after every multi-line function signature
wsl:
# If true append is only allowed to be cuddled if appending value is
# matching variables, fields or types on line above. Default is true.
strict-append: true
# Allow calls and assignments to be cuddled as long as the lines have any
# matching variables, fields or types. Default is true.
allow-assign-and-call: true
# Allow multiline assignments to be cuddled. Default is true.
allow-multiline-assign: true
linters: linters:
enable: enable:

View File

@ -804,6 +804,15 @@ linters-settings:
whitespace: whitespace:
multi-if: false # Enforces newlines (or comments) after every multi-line if statement multi-if: false # Enforces newlines (or comments) after every multi-line if statement
multi-func: false # Enforces newlines (or comments) after every multi-line function signature multi-func: false # Enforces newlines (or comments) after every multi-line function signature
wsl:
# If true append is only allowed to be cuddled if appending value is
# matching variables, fields or types on line above. Default is true.
strict-append: true
# Allow calls and assignments to be cuddled as long as the lines have any
# matching variables, fields or types. Default is true.
allow-assign-and-call: true
# Allow multiline assignments to be cuddled. Default is true.
allow-multiline-assign: true
linters: linters:
enable: enable:

2
go.mod
View File

@ -4,7 +4,7 @@ go 1.12
require ( require (
github.com/OpenPeeDeeP/depguard v1.0.1 github.com/OpenPeeDeeP/depguard v1.0.1
github.com/bombsimon/wsl v1.0.1 github.com/bombsimon/wsl v1.2.1
github.com/fatih/color v1.7.0 github.com/fatih/color v1.7.0
github.com/go-critic/go-critic v0.3.5-0.20190904082202-d79a9f0c64db github.com/go-critic/go-critic v0.3.5-0.20190904082202-d79a9f0c64db
github.com/go-lintpack/lintpack v0.5.2 github.com/go-lintpack/lintpack v0.5.2

4
go.sum
View File

@ -11,8 +11,8 @@ github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRF
github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8= github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8=
github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q= github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=
github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8= github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8=
github.com/bombsimon/wsl v1.0.1 h1:GkrpOj7ajds8re6EJXN+k6UtatYSktupigQ2ZfOUIOU= github.com/bombsimon/wsl v1.2.1 h1:DcLf3V66dJi4a+KHt+F1FdOeBZ05adHqTMYFvjgv06k=
github.com/bombsimon/wsl v1.0.1/go.mod h1:DSRwCD8c7NecM11/LnZcTS8nS8OND5ybj04DWM4l8mc= github.com/bombsimon/wsl v1.2.1/go.mod h1:43lEF/i0kpXbLCeDXL9LMT8c92HyBywXb0AsgMHYngM=
github.com/cespare/xxhash v1.1.0/go.mod h1:XrSqR1VqqWfGrhpAt58auRo0WTKS1nRRg3ghfAqPWnc= github.com/cespare/xxhash v1.1.0/go.mod h1:XrSqR1VqqWfGrhpAt58auRo0WTKS1nRRg3ghfAqPWnc=
github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw= github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk= github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk=

View File

@ -176,6 +176,7 @@ type LintersSettings struct {
MultiFunc bool `mapstructure:"multi-func"` MultiFunc bool `mapstructure:"multi-func"`
} }
WSL WSLSettings
Lll LllSettings Lll LllSettings
Unparam UnparamSettings Unparam UnparamSettings
Nakedret NakedretSettings Nakedret NakedretSettings
@ -249,6 +250,12 @@ type GocognitSettings struct {
MinComplexity int `mapstructure:"min-complexity"` MinComplexity int `mapstructure:"min-complexity"`
} }
type WSLSettings struct {
StrictAppend bool `mapstructure:"strict-append"`
AllowAssignAndCallCuddle bool `mapstructure:"allow-assign-and-call"`
AllowMultiLineAssignCuddle bool `mapstructure:"allow-multiline-assign"`
}
var defaultLintersSettings = LintersSettings{ var defaultLintersSettings = LintersSettings{
Lll: LllSettings{ Lll: LllSettings{
LineLength: 120, LineLength: 120,
@ -277,6 +284,11 @@ var defaultLintersSettings = LintersSettings{
Gocognit: GocognitSettings{ Gocognit: GocognitSettings{
MinComplexity: 30, MinComplexity: 30,
}, },
WSL: WSLSettings{
StrictAppend: true,
AllowAssignAndCallCuddle: true,
AllowMultiLineAssignCuddle: true,
},
} }
type Linters struct { type Linters struct {

View File

@ -33,13 +33,25 @@ func NewWSL() *goanalysis.Linter {
nil, nil,
).WithContextSetter(func(lintCtx *linter.Context) { ).WithContextSetter(func(lintCtx *linter.Context) {
analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { analyzer.Run = func(pass *analysis.Pass) (interface{}, error) {
files := []string{} var (
files = []string{}
linterCfg = lintCtx.Cfg.LintersSettings.WSL
processorCfg = wsl.Configuration{
StrictAppend: linterCfg.StrictAppend,
AllowAssignAndCallCuddle: linterCfg.AllowAssignAndCallCuddle,
AllowMultiLineAssignCuddle: linterCfg.AllowMultiLineAssignCuddle,
AllowCuddleWithCalls: []string{"Lock", "RLock"},
AllowCuddleWithRHS: []string{"Unlock", "RUnlock"},
}
)
for _, file := range pass.Files { for _, file := range pass.Files {
files = append(files, pass.Fset.Position(file.Pos()).Filename) files = append(files, pass.Fset.Position(file.Pos()).Filename)
} }
wslErrors, _ := wsl.ProcessFiles(files) wslErrors, _ := wsl.NewProcessorWithConfig(processorCfg).
ProcessFiles(files)
if len(wslErrors) == 0 { if len(wslErrors) == 0 {
return nil, nil return nil, nil
} }

22
test/testdata/wsl.go vendored
View File

@ -52,6 +52,26 @@ func main() {
_, cf2 := context.WithCancel(context.Background()) _, cf2 := context.WithCancel(context.Background())
defer cf1() // ERROR "only one cuddle assignment allowed before defer statement" defer cf1() // ERROR "only one cuddle assignment allowed before defer statement"
defer cf2() defer cf2()
err := multiline(
"spanning",
"multiple",
)
if err != nil {
panic(err)
}
notErr := multiline(
"spanning",
"multiple",
)
if err != nil { // ERROR "if statements should only be cuddled with assignments used in the if statement itself"
panic(notErr)
}
}
func multiline(s ...string) error {
return nil
} }
func f1() int { func f1() int {
@ -78,3 +98,5 @@ func f3() int {
return sum + notSum return sum + notSum
} }
func onelineShouldNotError() error { return nil }

View File

@ -14,6 +14,13 @@ good, making it harder for other people to read and understand. The linter will
warn about newlines in and around blocks, in the beginning of files and other warn about newlines in and around blocks, in the beginning of files and other
places in the code. places in the code.
**I know this linter is aggressive** and a lot of projects I've tested it on
have failed miserably. For this linter to be useful at all I want to be open to
new ideas, configurations and discussions! Also note that some of the warnings
might be bugs or unintentional false positives so I would love an
[issue](https://github.com/bombsimon/wsl/issues/new) to fix, discuss, change or
make something configurable!
## Usage ## Usage
Install by using `go get -u github.com/bombsimon/wsl/cmd/...`. Install by using `go get -u github.com/bombsimon/wsl/cmd/...`.
@ -26,6 +33,9 @@ By default, the linter will run on `./...` which means all go files in the
current path and all subsequent paths, including test files. To disable linting current path and all subsequent paths, including test files. To disable linting
test files, use `-n` or `--no-test`. test files, use `-n` or `--no-test`.
This linter can also be used as a part of
[golangci-lint](https://github.com/golangci/golangci-lint)
## Rules ## Rules
Note that this linter doesn't take in consideration the issues that will be Note that this linter doesn't take in consideration the issues that will be
@ -335,6 +345,11 @@ fmt.Println(a)
foo := true foo := true
someFunc(false) someFunc(false)
if someBool() {
fmt.Println("doing things")
}
x.Calling()
``` ```
**Do** **Do**
@ -350,6 +365,12 @@ someFunc(foo)
bar := false bar := false
someFunc(true) someFunc(true)
if someBool() {
fmt.Println("doing things")
}
x.Calling()
``` ```
#### Ranges #### Ranges

View File

@ -1,7 +1,7 @@
module github.com/bombsimon/wsl module github.com/bombsimon/wsl
require ( require (
github.com/davecgh/go-spew v1.1.1 github.com/davecgh/go-spew v1.1.1 // indirect
github.com/kr/pretty v0.1.0 // indirect github.com/kr/pretty v0.1.0 // indirect
github.com/stretchr/testify v1.4.0 github.com/stretchr/testify v1.4.0
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect

View File

@ -18,7 +18,5 @@ gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogR
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v2 v2.2.3 h1:fvjTMHxHEw/mxHbtzPi3JCcKXQRAnQTBRo6YCJSVHKI=
gopkg.in/yaml.v2 v2.2.3/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v2 v2.2.4 h1:/eiJrUcujPVeJ3xlSWaiNi3uSVmDGBK1pDHUHAnao1I= gopkg.in/yaml.v2 v2.2.4 h1:/eiJrUcujPVeJ3xlSWaiNi3uSVmDGBK1pDHUHAnao1I=
gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=

View File

@ -9,26 +9,111 @@ import (
"reflect" "reflect"
) )
type result struct { type Configuration struct {
// StrictAppend will do strict checking when assigning from append (x =
// append(x, y)). If this is set to true the append call must append either
// a variable assigned, called or used on the line above. Example on not
// allowed when this is true:
//
// x := []string{}
// y := "not going in X"
// x = append(x, "not y") // This is not allowed with StrictAppend
// z := "going in X"
//
// x = append(x, z) // This is allowed with StrictAppend
//
// m := transform(z)
// x = append(x, z) // So is this because Z is used above.
StrictAppend bool
// AllowAssignAndCallCuddle allows assignments to be cuddled with variables
// used in calls on line above and calls to be cuddled with assignments of
// variables used in call on line above.
// Example supported with this set to true:
//
// x.Call()
// x = Assign()
// x.AnotherCall()
// x = AnotherAssign()
AllowAssignAndCallCuddle bool
// AllowMultiLineAssignCuddle allows cuddling to assignments even if they
// span over multiple lines. This defaults to true which allows the
// following example:
//
// err := function(
// "multiple", "lines",
// )
// if err != nil {
// // ...
// }
AllowMultiLineAssignCuddle bool
// AllowCuddleWithCalls is a list of call idents that everything can be
// cuddled with. Defaults to calls looking like locks to support a flow like
// this:
//
// mu.Lock()
// allow := thisAssignment
AllowCuddleWithCalls []string
// AllowCuddleWithRHS is a list of right hand side variables that is allowed
// to be cuddled with anything. Defaults to assignments or calls looking
// like unlocks to support a flow like this:
//
// allow := thisAssignment()
// mu.Unlock()
AllowCuddleWithRHS []string
}
// DefaultConfig returns default configuration
func DefaultConfig() Configuration {
return Configuration{
StrictAppend: true,
AllowAssignAndCallCuddle: true,
AllowMultiLineAssignCuddle: true,
AllowCuddleWithCalls: []string{"Lock", "RLock"},
AllowCuddleWithRHS: []string{"Unlock", "RUnlock"},
}
}
// Result represents the result of one error.
type Result struct {
FileName string FileName string
LineNumber int LineNumber int
Position token.Position Position token.Position
Reason string Reason string
} }
type processor struct { // String returns the filename, line number and reason of a Result.
result []result func (r *Result) String() string {
return fmt.Sprintf("%s:%d: %s", r.FileName, r.LineNumber, r.Reason)
}
type Processor struct {
config Configuration
result []Result
warnings []string warnings []string
fileSet *token.FileSet fileSet *token.FileSet
file *ast.File file *ast.File
} }
// NewProcessor will create a Processor.
func NewProcessorWithConfig(cfg Configuration) *Processor {
return &Processor{
result: []Result{},
config: cfg,
}
}
// NewProcessor will create a Processor.
func NewProcessor() *Processor {
return NewProcessorWithConfig(DefaultConfig())
}
// ProcessFiles takes a string slice with file names (full paths) and lints // ProcessFiles takes a string slice with file names (full paths) and lints
// them. // them.
func ProcessFiles(filenames []string) ([]result, []string) { func (p *Processor) ProcessFiles(filenames []string) ([]Result, []string) {
p := NewProcessor()
// Iterate over all files.
for _, filename := range filenames { for _, filename := range filenames {
data, err := ioutil.ReadFile(filename) data, err := ioutil.ReadFile(filename)
if err != nil { if err != nil {
@ -41,20 +126,13 @@ func ProcessFiles(filenames []string) ([]result, []string) {
return p.result, p.warnings return p.result, p.warnings
} }
// NewProcessor will create a processor. func (p *Processor) process(filename string, data []byte) {
func NewProcessor() *processor {
return &processor{
result: []result{},
}
}
func (p *processor) process(filename string, data []byte) {
fileSet := token.NewFileSet() fileSet := token.NewFileSet()
file, err := parser.ParseFile(fileSet, filename, data, parser.ParseComments) file, err := parser.ParseFile(fileSet, filename, data, parser.ParseComments)
// If the file is not parsable let's add a syntax error and move on. // If the file is not parsable let's add a syntax error and move on.
if err != nil { if err != nil {
p.result = append(p.result, result{ p.result = append(p.result, Result{
FileName: filename, FileName: filename,
LineNumber: 0, LineNumber: 0,
Reason: fmt.Sprintf("invalid syntax, file cannot be linted (%s)", err.Error()), Reason: fmt.Sprintf("invalid syntax, file cannot be linted (%s)", err.Error()),
@ -81,7 +159,7 @@ func (p *processor) process(filename string, data []byte) {
// parseBlockBody will parse any kind of block statements such as switch cases // parseBlockBody will parse any kind of block statements such as switch cases
// and if statements. A list of Result is returned. // and if statements. A list of Result is returned.
func (p *processor) parseBlockBody(block *ast.BlockStmt) { func (p *Processor) parseBlockBody(block *ast.BlockStmt) {
// Nothing to do if there's no value. // Nothing to do if there's no value.
if reflect.ValueOf(block).IsNil() { if reflect.ValueOf(block).IsNil() {
return return
@ -96,7 +174,8 @@ func (p *processor) parseBlockBody(block *ast.BlockStmt) {
// parseBlockStatements will parse all the statements found in the body of a // parseBlockStatements will parse all the statements found in the body of a
// node. A list of Result is returned. // node. A list of Result is returned.
func (p *processor) parseBlockStatements(statements []ast.Stmt) { // nolint: gocognit
func (p *Processor) parseBlockStatements(statements []ast.Stmt) {
for i, stmt := range statements { for i, stmt := range statements {
// TODO: How to tell when and where func literals may exist to enforce // TODO: How to tell when and where func literals may exist to enforce
// linting. // linting.
@ -128,10 +207,26 @@ func (p *processor) parseBlockStatements(statements []ast.Stmt) {
// made over multiple lines we should not allow cuddling. // made over multiple lines we should not allow cuddling.
var assignedOnLineAbove []string var assignedOnLineAbove []string
// We want to keep track of what was called on the line above to support
// special handling of things such as mutexes.
var calledOnLineAbove []string
// Check if the previous statement spans over multiple lines.
var isMultiLineAssignment = p.nodeStart(previousStatement) != p.nodeStart(stmt)-1
// Ensure previous line is not a multi line assignment and if not get // Ensure previous line is not a multi line assignment and if not get
// all assigned variables. // rightAndLeftHandSide assigned variables.
if p.nodeStart(previousStatement) == p.nodeStart(stmt)-1 { if !isMultiLineAssignment {
assignedOnLineAbove = p.findLhs(previousStatement) assignedOnLineAbove = p.findLHS(previousStatement)
calledOnLineAbove = p.findRHS(previousStatement)
}
// If previous assignment is multi line and we allow it, fetch
// assignments (but only assignments).
if isMultiLineAssignment && p.config.AllowMultiLineAssignCuddle {
if _, ok := previousStatement.(*ast.AssignStmt); ok {
assignedOnLineAbove = p.findLHS(previousStatement)
}
} }
// We could potentially have a block which require us to check the first // We could potentially have a block which require us to check the first
@ -139,32 +234,44 @@ func (p *processor) parseBlockStatements(statements []ast.Stmt) {
var assignedFirstInBlock []string var assignedFirstInBlock []string
if firstBodyStatement != nil { if firstBodyStatement != nil {
assignedFirstInBlock = p.findLhs(firstBodyStatement) assignedFirstInBlock = p.findLHS(firstBodyStatement)
} }
lhs := p.findLhs(stmt) var (
rhs := p.findRhs(stmt) leftHandSide = p.findLHS(stmt)
all := append(lhs, rhs...) rightHandSide = p.findRHS(stmt)
rightAndLeftHandSide = append(leftHandSide, rightHandSide...)
calledOrAssignedOnLineAbove = append(calledOnLineAbove, assignedOnLineAbove...)
)
/* /*
DEBUG: DEBUG:
fmt.Println("LHS: ", lhs) fmt.Println("LHS: ", leftHandSide)
fmt.Println("RHS: ", rhs) fmt.Println("RHS: ", rightHandSide)
fmt.Println("Assigned above: ", assignedOnLineAbove) fmt.Println("Assigned above: ", assignedOnLineAbove)
fmt.Println("Assigned first: ", assignedFirstInBlock) fmt.Println("Assigned first: ", assignedFirstInBlock)
*/ */
// If we called some kind of lock on the line above we allow cuddling
// anything.
if atLeastOneInListsMatch(calledOnLineAbove, p.config.AllowCuddleWithCalls) {
continue
}
// If we call some kind of unlock on this line we allow cuddling with
// anything.
if atLeastOneInListsMatch(rightHandSide, p.config.AllowCuddleWithRHS) {
continue
}
moreThanOneStatementAbove := func() bool { moreThanOneStatementAbove := func() bool {
if i < 2 { if i < 2 {
return false return false
} }
statementBeforePreviousStatement := statements[i-2] statementBeforePreviousStatement := statements[i-2]
if p.nodeStart(previousStatement)-1 == p.nodeEnd(statementBeforePreviousStatement) {
return true
}
return false return p.nodeStart(previousStatement)-1 == p.nodeEnd(statementBeforePreviousStatement)
} }
isLastStatementInBlockOfOnlyTwoLines := func() bool { isLastStatementInBlockOfOnlyTwoLines := func() bool {
@ -188,17 +295,15 @@ func (p *processor) parseBlockStatements(statements []ast.Stmt) {
case *ast.IfStmt: case *ast.IfStmt:
if len(assignedOnLineAbove) == 0 { if len(assignedOnLineAbove) == 0 {
p.addError(t.Pos(), "if statements should only be cuddled with assignments") p.addError(t.Pos(), "if statements should only be cuddled with assignments")
continue continue
} }
if moreThanOneStatementAbove() { if moreThanOneStatementAbove() {
p.addError(t.Pos(), "only one cuddle assignment allowed before if statement") p.addError(t.Pos(), "only one cuddle assignment allowed before if statement")
continue continue
} }
if !atLeastOneInListsMatch(all, assignedOnLineAbove) { if !atLeastOneInListsMatch(rightAndLeftHandSide, assignedOnLineAbove) {
if !atLeastOneInListsMatch(assignedOnLineAbove, assignedFirstInBlock) { if !atLeastOneInListsMatch(assignedOnLineAbove, assignedFirstInBlock) {
p.addError(t.Pos(), "if statements should only be cuddled with assignments used in the if statement itself") p.addError(t.Pos(), "if statements should only be cuddled with assignments used in the if statement itself")
} }
@ -218,16 +323,32 @@ func (p *processor) parseBlockStatements(statements []ast.Stmt) {
case *ast.AssignStmt: case *ast.AssignStmt:
// append is usually an assignment but should not be allowed to be // append is usually an assignment but should not be allowed to be
// cuddled with anything not appended. // cuddled with anything not appended.
if len(rhs) > 0 && rhs[len(rhs)-1] == "append" { if len(rightHandSide) > 0 && rightHandSide[len(rightHandSide)-1] == "append" {
if !atLeastOneInListsMatch(assignedOnLineAbove, rhs) { if p.config.StrictAppend {
p.addError(t.Pos(), "append only allowed to cuddle with appended value") if !atLeastOneInListsMatch(calledOrAssignedOnLineAbove, rightHandSide) {
p.addError(t.Pos(), "append only allowed to cuddle with appended value")
}
} }
continue
} }
if _, ok := previousStatement.(*ast.AssignStmt); ok { if _, ok := previousStatement.(*ast.AssignStmt); ok {
continue continue
} }
// If the assignment is from a type or variable called on the line
// above we can allow it by setting AllowAssignAndCallCuddle to
// true.
// Example (x is used):
// x.function()
// a.Field = x.anotherFunction()
if p.config.AllowAssignAndCallCuddle {
if atLeastOneInListsMatch(calledOrAssignedOnLineAbove, rightAndLeftHandSide) {
continue
}
}
p.addError(t.Pos(), "assignments should only be cuddled with other assignments") p.addError(t.Pos(), "assignments should only be cuddled with other assignments")
case *ast.DeclStmt: case *ast.DeclStmt:
p.addError(t.Pos(), "declarations should never be cuddled") p.addError(t.Pos(), "declarations should never be cuddled")
@ -235,22 +356,34 @@ func (p *processor) parseBlockStatements(statements []ast.Stmt) {
switch previousStatement.(type) { switch previousStatement.(type) {
case *ast.DeclStmt, *ast.ReturnStmt: case *ast.DeclStmt, *ast.ReturnStmt:
p.addError(t.Pos(), "expressions should not be cuddled with declarations or returns") p.addError(t.Pos(), "expressions should not be cuddled with declarations or returns")
case *ast.IfStmt, *ast.RangeStmt, *ast.SwitchStmt:
p.addError(t.Pos(), "expressions should not be cuddled with blocks")
}
// If the expression is called on a type or variable used or
// assigned on the line we can allow it by setting
// AllowAssignAndCallCuddle to true.
// Example of allowed cuddled (x is used):
// a.Field = x.func()
// x.function()
if p.config.AllowAssignAndCallCuddle {
if atLeastOneInListsMatch(calledOrAssignedOnLineAbove, rightAndLeftHandSide) {
continue
}
} }
// If we assigned variables on the line above but didn't use them in // If we assigned variables on the line above but didn't use them in
// this expression we there should probably be a newline between // this expression there should probably be a newline between them.
// them. if len(assignedOnLineAbove) > 0 && !atLeastOneInListsMatch(rightAndLeftHandSide, assignedOnLineAbove) {
if len(assignedOnLineAbove) > 0 && !atLeastOneInListsMatch(all, assignedOnLineAbove) {
p.addError(t.Pos(), "only cuddled expressions if assigning variable or using from line above") p.addError(t.Pos(), "only cuddled expressions if assigning variable or using from line above")
} }
case *ast.RangeStmt: case *ast.RangeStmt:
if moreThanOneStatementAbove() { if moreThanOneStatementAbove() {
p.addError(t.Pos(), "only one cuddle assignment allowed before range statement") p.addError(t.Pos(), "only one cuddle assignment allowed before range statement")
continue continue
} }
if !atLeastOneInListsMatch(all, assignedOnLineAbove) { if !atLeastOneInListsMatch(rightAndLeftHandSide, assignedOnLineAbove) {
if !atLeastOneInListsMatch(assignedOnLineAbove, assignedFirstInBlock) { if !atLeastOneInListsMatch(assignedOnLineAbove, assignedFirstInBlock) {
p.addError(t.Pos(), "ranges should only be cuddled with assignments used in the iteration") p.addError(t.Pos(), "ranges should only be cuddled with assignments used in the iteration")
} }
@ -270,16 +403,16 @@ func (p *processor) parseBlockStatements(statements []ast.Stmt) {
// Be extra nice with RHS, it's common to use this for locks: // Be extra nice with RHS, it's common to use this for locks:
// m.Lock() // m.Lock()
// defer m.Unlock() // defer m.Unlock()
previousRhs := p.findRhs(previousStatement) previousRHS := p.findRHS(previousStatement)
if atLeastOneInListsMatch(rhs, previousRhs) { if atLeastOneInListsMatch(rightHandSide, previousRHS) {
continue continue
} }
if !atLeastOneInListsMatch(all, assignedOnLineAbove) { if !atLeastOneInListsMatch(rightAndLeftHandSide, assignedOnLineAbove) {
p.addError(t.Pos(), "defer statements should only be cuddled with expressions on same variable") p.addError(t.Pos(), "defer statements should only be cuddled with expressions on same variable")
} }
case *ast.ForStmt: case *ast.ForStmt:
if len(all) == 0 { if len(rightAndLeftHandSide) == 0 {
p.addError(t.Pos(), "for statement without condition should never be cuddled") p.addError(t.Pos(), "for statement without condition should never be cuddled")
continue continue
@ -294,7 +427,7 @@ func (p *processor) parseBlockStatements(statements []ast.Stmt) {
// The same rule applies for ranges as for if statements, see // The same rule applies for ranges as for if statements, see
// comments regarding variable usages on the line before or as the // comments regarding variable usages on the line before or as the
// first line in the block for details. // first line in the block for details.
if !atLeastOneInListsMatch(all, assignedOnLineAbove) { if !atLeastOneInListsMatch(rightAndLeftHandSide, assignedOnLineAbove) {
if !atLeastOneInListsMatch(assignedOnLineAbove, assignedFirstInBlock) { if !atLeastOneInListsMatch(assignedOnLineAbove, assignedFirstInBlock) {
p.addError(t.Pos(), "for statements should only be cuddled with assignments used in the iteration") p.addError(t.Pos(), "for statements should only be cuddled with assignments used in the iteration")
} }
@ -306,7 +439,7 @@ func (p *processor) parseBlockStatements(statements []ast.Stmt) {
continue continue
} }
if !atLeastOneInListsMatch(all, assignedOnLineAbove) { if !atLeastOneInListsMatch(rightAndLeftHandSide, assignedOnLineAbove) {
p.addError(t.Pos(), "go statements can only invoke functions assigned on line above") p.addError(t.Pos(), "go statements can only invoke functions assigned on line above")
} }
case *ast.SwitchStmt: case *ast.SwitchStmt:
@ -316,8 +449,8 @@ func (p *processor) parseBlockStatements(statements []ast.Stmt) {
continue continue
} }
if !atLeastOneInListsMatch(all, assignedOnLineAbove) { if !atLeastOneInListsMatch(rightAndLeftHandSide, assignedOnLineAbove) {
if len(all) == 0 { if len(rightAndLeftHandSide) == 0 {
p.addError(t.Pos(), "anonymous switch statements should never be cuddled") p.addError(t.Pos(), "anonymous switch statements should never be cuddled")
} else { } else {
p.addError(t.Pos(), "switch statements should only be cuddled with variables switched") p.addError(t.Pos(), "switch statements should only be cuddled with variables switched")
@ -331,7 +464,7 @@ func (p *processor) parseBlockStatements(statements []ast.Stmt) {
} }
// Allowed to type assert on variable assigned on line above. // Allowed to type assert on variable assigned on line above.
if !atLeastOneInListsMatch(rhs, assignedOnLineAbove) { if !atLeastOneInListsMatch(rightHandSide, assignedOnLineAbove) {
// Allow type assertion on variables used in the first case // Allow type assertion on variables used in the first case
// immediately. // immediately.
if !atLeastOneInListsMatch(assignedOnLineAbove, assignedFirstInBlock) { if !atLeastOneInListsMatch(assignedOnLineAbove, assignedFirstInBlock) {
@ -353,7 +486,7 @@ func (p *processor) parseBlockStatements(statements []ast.Stmt) {
// directly as the first argument inside a body. // directly as the first argument inside a body.
// The body will then be parsed as a *ast.BlockStmt (regular block) or as a list // The body will then be parsed as a *ast.BlockStmt (regular block) or as a list
// of []ast.Stmt (case block). // of []ast.Stmt (case block).
func (p *processor) firstBodyStatement(i int, allStmt []ast.Stmt) ast.Node { func (p *Processor) firstBodyStatement(i int, allStmt []ast.Stmt) ast.Node {
stmt := allStmt[i] stmt := allStmt[i]
// Start by checking if the statement has a body (probably if-statement, // Start by checking if the statement has a body (probably if-statement,
@ -409,7 +542,7 @@ func (p *processor) firstBodyStatement(i int, allStmt []ast.Stmt) ast.Node {
return firstBodyStatement return firstBodyStatement
} }
func (p *processor) findLhs(node ast.Node) []string { func (p *Processor) findLHS(node ast.Node) []string {
var lhs []string var lhs []string
if node == nil { if node == nil {
@ -429,36 +562,36 @@ func (p *processor) findLhs(node ast.Node) []string {
return []string{t.Name} return []string{t.Name}
case *ast.AssignStmt: case *ast.AssignStmt:
for _, v := range t.Lhs { for _, v := range t.Lhs {
lhs = append(lhs, p.findLhs(v)...) lhs = append(lhs, p.findLHS(v)...)
} }
case *ast.GenDecl: case *ast.GenDecl:
for _, v := range t.Specs { for _, v := range t.Specs {
lhs = append(lhs, p.findLhs(v)...) lhs = append(lhs, p.findLHS(v)...)
} }
case *ast.ValueSpec: case *ast.ValueSpec:
for _, v := range t.Names { for _, v := range t.Names {
lhs = append(lhs, p.findLhs(v)...) lhs = append(lhs, p.findLHS(v)...)
} }
case *ast.BlockStmt: case *ast.BlockStmt:
for _, v := range t.List { for _, v := range t.List {
lhs = append(lhs, p.findLhs(v)...) lhs = append(lhs, p.findLHS(v)...)
} }
case *ast.BinaryExpr: case *ast.BinaryExpr:
return append( return append(
p.findLhs(t.X), p.findLHS(t.X),
p.findLhs(t.Y)..., p.findLHS(t.Y)...,
) )
case *ast.DeclStmt: case *ast.DeclStmt:
return p.findLhs(t.Decl) return p.findLHS(t.Decl)
case *ast.IfStmt: case *ast.IfStmt:
return p.findLhs(t.Cond) return p.findLHS(t.Cond)
case *ast.TypeSwitchStmt: case *ast.TypeSwitchStmt:
return p.findLhs(t.Assign) return p.findLHS(t.Assign)
case *ast.SendStmt: case *ast.SendStmt:
return p.findLhs(t.Chan) return p.findLHS(t.Chan)
default: default:
if x, ok := maybeX(t); ok { if x, ok := maybeX(t); ok {
return p.findLhs(x) return p.findLHS(x)
} }
p.addWarning("UNKNOWN LHS", t.Pos(), t) p.addWarning("UNKNOWN LHS", t.Pos(), t)
@ -467,7 +600,7 @@ func (p *processor) findLhs(node ast.Node) []string {
return lhs return lhs
} }
func (p *processor) findRhs(node ast.Node) []string { func (p *Processor) findRHS(node ast.Node) []string {
var rhs []string var rhs []string
if node == nil { if node == nil {
@ -485,53 +618,55 @@ func (p *processor) findRhs(node ast.Node) []string {
return []string{t.Name} return []string{t.Name}
case *ast.SelectorExpr: case *ast.SelectorExpr:
// TODO: Should this be RHS? // TODO: Should this be RHS?
// Needed for defer as of now // t.X is needed for defer as of now and t.Sel needed for special
return p.findRhs(t.X) // functions such as Lock()
rhs = p.findRHS(t.X)
rhs = append(rhs, p.findRHS(t.Sel)...)
case *ast.AssignStmt: case *ast.AssignStmt:
for _, v := range t.Rhs { for _, v := range t.Rhs {
rhs = append(rhs, p.findRhs(v)...) rhs = append(rhs, p.findRHS(v)...)
} }
case *ast.CallExpr: case *ast.CallExpr:
for _, v := range t.Args { for _, v := range t.Args {
rhs = append(rhs, p.findRhs(v)...) rhs = append(rhs, p.findRHS(v)...)
} }
rhs = append(rhs, p.findRhs(t.Fun)...) rhs = append(rhs, p.findRHS(t.Fun)...)
case *ast.CompositeLit: case *ast.CompositeLit:
for _, v := range t.Elts { for _, v := range t.Elts {
rhs = append(rhs, p.findRhs(v)...) rhs = append(rhs, p.findRHS(v)...)
} }
case *ast.IfStmt: case *ast.IfStmt:
rhs = append(rhs, p.findRhs(t.Cond)...) rhs = append(rhs, p.findRHS(t.Cond)...)
rhs = append(rhs, p.findRhs(t.Init)...) rhs = append(rhs, p.findRHS(t.Init)...)
case *ast.BinaryExpr: case *ast.BinaryExpr:
return append( return append(
p.findRhs(t.X), p.findRHS(t.X),
p.findRhs(t.Y)..., p.findRHS(t.Y)...,
) )
case *ast.TypeSwitchStmt: case *ast.TypeSwitchStmt:
return p.findRhs(t.Assign) return p.findRHS(t.Assign)
case *ast.ReturnStmt: case *ast.ReturnStmt:
for _, v := range t.Results { for _, v := range t.Results {
rhs = append(rhs, p.findRhs(v)...) rhs = append(rhs, p.findRHS(v)...)
} }
case *ast.BlockStmt: case *ast.BlockStmt:
for _, v := range t.List { for _, v := range t.List {
rhs = append(rhs, p.findRhs(v)...) rhs = append(rhs, p.findRHS(v)...)
} }
case *ast.SwitchStmt: case *ast.SwitchStmt:
return p.findRhs(t.Tag) return p.findRHS(t.Tag)
case *ast.GoStmt: case *ast.GoStmt:
return p.findRhs(t.Call) return p.findRHS(t.Call)
case *ast.ForStmt: case *ast.ForStmt:
return p.findRhs(t.Cond) return p.findRHS(t.Cond)
case *ast.DeferStmt: case *ast.DeferStmt:
return p.findRhs(t.Call) return p.findRHS(t.Call)
case *ast.SendStmt: case *ast.SendStmt:
return p.findLhs(t.Value) return p.findLHS(t.Value)
default: default:
if x, ok := maybeX(t); ok { if x, ok := maybeX(t); ok {
return p.findRhs(x) return p.findRHS(x)
} }
p.addWarning("UNKNOWN RHS", t.Pos(), t) p.addWarning("UNKNOWN RHS", t.Pos(), t)
@ -591,13 +726,15 @@ func atLeastOneInListsMatch(listOne, listTwo []string) bool {
// findLeadingAndTrailingWhitespaces will find leading and trailing whitespaces // findLeadingAndTrailingWhitespaces will find leading and trailing whitespaces
// in a node. The method takes comments in consideration which will make the // in a node. The method takes comments in consideration which will make the
// parser more gentle. // parser more gentle.
func (p *processor) findLeadingAndTrailingWhitespaces(stmt, nextStatement ast.Node) { func (p *Processor) findLeadingAndTrailingWhitespaces(stmt, nextStatement ast.Node) {
var ( var (
allowedLinesBeforeFirstStatement = 1 allowedLinesBeforeFirstStatement = 1
commentMap = ast.NewCommentMap(p.fileSet, stmt, p.file.Comments) commentMap = ast.NewCommentMap(p.fileSet, stmt, p.file.Comments)
blockStatements []ast.Stmt blockStatements []ast.Stmt
blockStartLine int blockStartLine int
blockEndLine int blockEndLine int
blockStartPos token.Pos
blockEndPos token.Pos
) )
// Depending on the block type, get the statements in the block and where // Depending on the block type, get the statements in the block and where
@ -605,14 +742,14 @@ func (p *processor) findLeadingAndTrailingWhitespaces(stmt, nextStatement ast.No
switch t := stmt.(type) { switch t := stmt.(type) {
case *ast.BlockStmt: case *ast.BlockStmt:
blockStatements = t.List blockStatements = t.List
blockStartLine = p.fileSet.Position(t.Lbrace).Line blockStartPos = t.Lbrace
blockEndLine = p.fileSet.Position(t.Rbrace).Line blockEndPos = t.Rbrace
case *ast.CaseClause: case *ast.CaseClause:
blockStatements = t.Body blockStatements = t.Body
blockStartLine = p.fileSet.Position(t.Colon).Line blockStartPos = t.Colon
case *ast.CommClause: case *ast.CommClause:
blockStatements = t.Body blockStatements = t.Body
blockStartLine = p.fileSet.Position(t.Colon).Line blockStartPos = t.Colon
default: default:
p.addWarning("whitespace node type not implemented ", stmt.Pos(), stmt) p.addWarning("whitespace node type not implemented ", stmt.Pos(), stmt)
@ -624,6 +761,14 @@ func (p *processor) findLeadingAndTrailingWhitespaces(stmt, nextStatement ast.No
return return
} }
blockStartLine = p.fileSet.Position(blockStartPos).Line
blockEndLine = p.fileSet.Position(blockEndPos).Line
// No whitespace possible if LBrace and RBrace is on the same line.
if blockStartLine == blockEndLine {
return
}
var ( var (
firstStatement = blockStatements[0] firstStatement = blockStatements[0]
lastStatement = blockStatements[len(blockStatements)-1] lastStatement = blockStatements[len(blockStatements)-1]
@ -633,13 +778,9 @@ func (p *processor) findLeadingAndTrailingWhitespaces(stmt, nextStatement ast.No
// the beginning of a block before the first statement. // the beginning of a block before the first statement.
if c, ok := commentMap[firstStatement]; ok { if c, ok := commentMap[firstStatement]; ok {
for _, commentGroup := range c { for _, commentGroup := range c {
var ( // If the comment group is on the same line as the block start
start = p.fileSet.Position(commentGroup.Pos()).Line
)
// If the comment group is on the same lince as the block start
// (LBrace) we should not consider it. // (LBrace) we should not consider it.
if start == blockStartLine { if p.nodeStart(commentGroup) == blockStartLine {
continue continue
} }
@ -654,10 +795,9 @@ func (p *processor) findLeadingAndTrailingWhitespaces(stmt, nextStatement ast.No
} }
} }
if p.fileSet.Position(firstStatement.Pos()).Line != blockStartLine+allowedLinesBeforeFirstStatement { if p.nodeStart(firstStatement) != blockStartLine+allowedLinesBeforeFirstStatement {
p.addErrorOffset( p.addError(
firstStatement.Pos(), blockStartPos,
-1,
"block should not start with a whitespace", "block should not start with a whitespace",
) )
} }
@ -673,52 +813,47 @@ func (p *processor) findLeadingAndTrailingWhitespaces(stmt, nextStatement ast.No
switch n := nextStatement.(type) { switch n := nextStatement.(type) {
case *ast.CaseClause: case *ast.CaseClause:
blockEndLine = p.fileSet.Position(n.Colon).Line blockEndPos = n.Colon
case *ast.CommClause: case *ast.CommClause:
blockEndLine = p.fileSet.Position(n.Colon).Line blockEndPos = n.Colon
default: default:
// We're not at the end of the case? // We're not at the end of the case?
return return
} }
blockEndLine = p.fileSet.Position(blockEndPos).Line
} }
if p.fileSet.Position(lastStatement.End()).Line != blockEndLine-1 { if p.nodeEnd(lastStatement) != blockEndLine-1 {
p.addErrorOffset( p.addError(
lastStatement.End(), blockEndPos,
1,
"block should not end with a whitespace (or comment)", "block should not end with a whitespace (or comment)",
) )
} }
} }
func (p *processor) nodeStart(node ast.Node) int { func (p *Processor) nodeStart(node ast.Node) int {
return p.fileSet.Position(node.Pos()).Line return p.fileSet.Position(node.Pos()).Line
} }
func (p *processor) nodeEnd(node ast.Node) int { func (p *Processor) nodeEnd(node ast.Node) int {
return p.fileSet.Position(node.End()).Line return p.fileSet.Position(node.End()).Line
} }
// Add an error for the file and line number for the current token.Pos with the // Add an error for the file and line number for the current token.Pos with the
// given reason. // given reason.
func (p *processor) addError(pos token.Pos, reason string) { func (p *Processor) addError(pos token.Pos, reason string) {
p.addErrorOffset(pos, 0, reason)
}
// Add an error for the file for the current token.Pos with the given offset and
// reason. The offset will be added to the token.Pos line.
func (p *processor) addErrorOffset(pos token.Pos, offset int, reason string) {
position := p.fileSet.Position(pos) position := p.fileSet.Position(pos)
p.result = append(p.result, result{ p.result = append(p.result, Result{
FileName: position.Filename, FileName: position.Filename,
LineNumber: position.Line + offset, LineNumber: position.Line,
Position: position, Position: position,
Reason: reason, Reason: reason,
}) })
} }
func (p *processor) addWarning(w string, pos token.Pos, t interface{}) { func (p *Processor) addWarning(w string, pos token.Pos, t interface{}) {
position := p.fileSet.Position(pos) position := p.fileSet.Position(pos)
p.warnings = append(p.warnings, p.warnings = append(p.warnings,

2
vendor/modules.txt vendored
View File

@ -4,7 +4,7 @@ github.com/BurntSushi/toml
github.com/OpenPeeDeeP/depguard github.com/OpenPeeDeeP/depguard
# github.com/StackExchange/wmi v0.0.0-20180116203802-5d049714c4a6 # github.com/StackExchange/wmi v0.0.0-20180116203802-5d049714c4a6
github.com/StackExchange/wmi github.com/StackExchange/wmi
# github.com/bombsimon/wsl v1.0.1 # github.com/bombsimon/wsl v1.2.1
github.com/bombsimon/wsl github.com/bombsimon/wsl
# github.com/davecgh/go-spew v1.1.1 # github.com/davecgh/go-spew v1.1.1
github.com/davecgh/go-spew/spew github.com/davecgh/go-spew/spew