Update WSL to v1.2.5 (#811)

* Update WSL to v1.2.4

* Fix false positive multiline case
* Fix false positive slice expression
* Fix false positive index expression
* Support to configure/allow cuddle declarations
* Support to configurre/allow case blocks to end with whitespace
* Support cuddle defer http body close

* Re-generate README.md

* Update WSL to v1.2.5

* Support output comments for example functions

* Fix bad field tag for config

.golangci.example.yml

@@ -220,6 +220,10 @@ linters-settings:
     allow-assign-and-call: true
     # Allow multiline assignments to be cuddled. Default is true.
     allow-multiline-assign: true
+    # Allow case blocks to end with a whitespace.
+    allow-case-traling-whitespace: true
+    # Allow declarations (var) to be cuddled.
+    allow-cuddle-declarations: false
 
 linters:
   enable:

README.md

@@ -819,6 +819,10 @@ linters-settings:
     allow-assign-and-call: true
     # Allow multiline assignments to be cuddled. Default is true.
     allow-multiline-assign: true
+    # Allow case blocks to end with a whitespace.
+    allow-case-traling-whitespace: true
+    # Allow declarations (var) to be cuddled.
+    allow-cuddle-declarations: false
 
 linters:
   enable:

go.mod

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

go.sum

@@ -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/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/bombsimon/wsl v1.2.1 h1:DcLf3V66dJi4a+KHt+F1FdOeBZ05adHqTMYFvjgv06k=
+github.com/bombsimon/wsl v1.2.5 h1:9gTOkIwVtoDZywvX802SDHokeX4kW1cKnV8ZTVAPkRs=
-github.com/bombsimon/wsl v1.2.1/go.mod h1:43lEF/i0kpXbLCeDXL9LMT8c92HyBywXb0AsgMHYngM=
+github.com/bombsimon/wsl v1.2.5/go.mod h1:43lEF/i0kpXbLCeDXL9LMT8c92HyBywXb0AsgMHYngM=
 github.com/cespare/xxhash v1.1.0/go.mod h1:XrSqR1VqqWfGrhpAt58auRo0WTKS1nRRg3ghfAqPWnc=
 github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
 github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk=

pkg/config/config.go

@@ -255,9 +255,11 @@ type GocognitSettings struct {
 }
 
 type WSLSettings struct {
-	StrictAppend               bool `mapstructure:"strict-append"`
+	StrictAppend                bool `mapstructure:"strict-append"`
-	AllowAssignAndCallCuddle   bool `mapstructure:"allow-assign-and-call"`
+	AllowAssignAndCallCuddle    bool `mapstructure:"allow-assign-and-call"`
-	AllowMultiLineAssignCuddle bool `mapstructure:"allow-multiline-assign"`
+	AllowMultiLineAssignCuddle  bool `mapstructure:"allow-multiline-assign"`
+	AllowCaseTrailingWhitespace bool `mapstructure:"allow-case-trailing-whitespace"`
+	AllowCuddleDeclaration      bool `mapstructure:"allow-cuddle-declarations"`
 }
 
 var defaultLintersSettings = LintersSettings{
@@ -289,9 +291,11 @@ var defaultLintersSettings = LintersSettings{
 		MinComplexity: 30,
 	},
 	WSL: WSLSettings{
-		StrictAppend:               true,
+		StrictAppend:                true,
-		AllowAssignAndCallCuddle:   true,
+		AllowAssignAndCallCuddle:    true,
-		AllowMultiLineAssignCuddle: true,
+		AllowMultiLineAssignCuddle:  true,
+		AllowCaseTrailingWhitespace: true,
+		AllowCuddleDeclaration:      false,
 	},
 }
 

pkg/golinters/wsl.go

@@ -37,11 +37,13 @@ func NewWSL() *goanalysis.Linter {
 				files        = []string{}
 				linterCfg    = lintCtx.Cfg.LintersSettings.WSL
 				processorCfg = wsl.Configuration{
-					StrictAppend:               linterCfg.StrictAppend,
+					StrictAppend:                linterCfg.StrictAppend,
-					AllowAssignAndCallCuddle:   linterCfg.AllowAssignAndCallCuddle,
+					AllowAssignAndCallCuddle:    linterCfg.AllowAssignAndCallCuddle,
-					AllowMultiLineAssignCuddle: linterCfg.AllowMultiLineAssignCuddle,
+					AllowMultiLineAssignCuddle:  linterCfg.AllowMultiLineAssignCuddle,
-					AllowCuddleWithCalls:       []string{"Lock", "RLock"},
+					AllowCaseTrailingWhitespace: linterCfg.AllowCaseTrailingWhitespace,
-					AllowCuddleWithRHS:         []string{"Unlock", "RUnlock"},
+					AllowCuddleDeclaration:      linterCfg.AllowCuddleDeclaration,
+					AllowCuddleWithCalls:        []string{"Lock", "RLock"},
+					AllowCuddleWithRHS:          []string{"Unlock", "RUnlock"},
 				}
 			)
 

test/linters_test.go

@@ -105,6 +105,7 @@ func testOneSource(t *testing.T, sourcePath string) {
 		"--print-issued-lines=false",
 		"--print-linter-name=false",
 		"--out-format=line-number",
+		"--max-same-issues=10",
 	}
 
 	rc := extractRunContextFromComments(t, sourcePath)

test/testdata/wsl.go

@@ -100,3 +100,75 @@ func f3() int {
 }
 
 func onelineShouldNotError() error { return nil }
+
+func multilineCase() {
+	// Multiline cases
+	switch {
+	case true,
+		false:
+		fmt.Println("ok")
+	case false ||
+		true:
+		fmt.Println("ok")
+	case true,
+		false:
+		fmt.Println("ok")
+	}
+}
+
+func sliceExpr() {
+	// Index- and slice expressions.
+	var aSlice = []int{1, 2, 3}
+
+	start := 2
+	if v := aSlice[start]; v == 1 {
+		fmt.Println("ok")
+	}
+
+	notOk := 1
+	if v := aSlice[start]; v == 1 { // ERROR "if statements should only be cuddled with assignments used in the if statement itself"
+		fmt.Println("notOk")
+		fmt.Println(notOk)
+	}
+
+	end := 2
+	if len(aSlice[start:end]) > 2 {
+		fmt.Println("ok")
+	}
+}
+
+func indexExpr() {
+	var aMap = map[string]struct{}{"key": {}}
+
+	key := "key"
+	if k, ok := aMap[key]; ok {
+		fmt.Println(k)
+	}
+
+	xxx := "xxx"
+	if _, ok := aMap[key]; ok { // ERROR "if statements should only be cuddled with assignments used in the if statement itself"
+		fmt.Println("not ok")
+		fmt.Println(xxx)
+	}
+}
+
+func allowTrailing(i int) {
+	switch i {
+	case 1:
+		fmt.Println("one")
+
+	case 2:
+		fmt.Println("two")
+
+	case 3:
+		fmt.Println("three")
+	}
+}
+
+// ExampleSomeOutput simulates an example function.
+func ExampleSomeOutput() {
+	fmt.Println("Hello, world")
+
+	// Output:
+	// Hello, world
+}

vendor/github.com/bombsimon/wsl/wsl.go

@@ -7,6 +7,7 @@ import (
 	"go/token"
 	"io/ioutil"
 	"reflect"
+	"strings"
 )
 
 type Configuration struct {
@@ -49,6 +50,29 @@ type Configuration struct {
 	//  }
 	AllowMultiLineAssignCuddle bool
 
+	// AllowCaseTrailingWhitespace will allow case blocks to end with a
+	// whitespace. Sometimes this might actually improve readability. This
+	// defaults to false but setting it to true will enable the following
+	// example:
+	//  switch {
+	//  case 1:
+	//      fmt.Println(1)
+	//
+	// case 2:
+	//     fmt.Println(2)
+	//
+	// case 3:
+	//     fmt:println(3)
+	// }
+	AllowCaseTrailingWhitespace bool
+
+	// AllowCuddleDeclaration will allow multiple var/declaration statements to
+	// be cuddled. This defaults to false but setting it to true will enable the
+	// following example:
+	//  var foo bool
+	//  var err error
+	AllowCuddleDeclaration 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:
@@ -69,11 +93,12 @@ type Configuration struct {
 // DefaultConfig returns default configuration
 func DefaultConfig() Configuration {
 	return Configuration{
-		StrictAppend:               true,
+		StrictAppend:                true,
-		AllowAssignAndCallCuddle:   true,
+		AllowAssignAndCallCuddle:    true,
-		AllowMultiLineAssignCuddle: true,
+		AllowMultiLineAssignCuddle:  true,
-		AllowCuddleWithCalls:       []string{"Lock", "RLock"},
+		AllowCaseTrailingWhitespace: false,
-		AllowCuddleWithRHS:         []string{"Unlock", "RUnlock"},
+		AllowCuddleWithCalls:        []string{"Lock", "RLock"},
+		AllowCuddleWithRHS:          []string{"Unlock", "RUnlock"},
 	}
 }
 
@@ -113,6 +138,7 @@ func NewProcessor() *Processor {
 
 // ProcessFiles takes a string slice with file names (full paths) and lints
 // them.
+// nolint: gocritic
 func (p *Processor) ProcessFiles(filenames []string) ([]Result, []string) {
 	for _, filename := range filenames {
 		data, err := ioutil.ReadFile(filename)
@@ -147,7 +173,7 @@ func (p *Processor) process(filename string, data []byte) {
 	for _, d := range p.file.Decls {
 		switch v := d.(type) {
 		case *ast.FuncDecl:
-			p.parseBlockBody(v.Body)
+			p.parseBlockBody(v.Name, v.Body)
 		case *ast.GenDecl:
 			// `go fmt` will handle proper spacing for GenDecl such as imports,
 			// constants etc.
@@ -159,14 +185,14 @@ func (p *Processor) process(filename string, data []byte) {
 
 // parseBlockBody will parse any kind of block statements such as switch cases
 // and if statements. A list of Result is returned.
-func (p *Processor) parseBlockBody(block *ast.BlockStmt) {
+func (p *Processor) parseBlockBody(ident *ast.Ident, block *ast.BlockStmt) {
 	// Nothing to do if there's no value.
 	if reflect.ValueOf(block).IsNil() {
 		return
 	}
 
 	// Start by finding leading and trailing whitespaces.
-	p.findLeadingAndTrailingWhitespaces(block, nil)
+	p.findLeadingAndTrailingWhitespaces(ident, block, nil)
 
 	// Parse the block body contents.
 	p.parseBlockStatements(block.List)
@@ -182,7 +208,7 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) {
 		if as, isAssignStmt := stmt.(*ast.AssignStmt); isAssignStmt {
 			for _, rhs := range as.Rhs {
 				if fl, isFuncLit := rhs.(*ast.FuncLit); isFuncLit {
-					p.parseBlockBody(fl.Body)
+					p.parseBlockBody(nil, fl.Body)
 				}
 			}
 		}
@@ -244,14 +270,6 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) {
 			calledOrAssignedOnLineAbove = append(calledOnLineAbove, assignedOnLineAbove...)
 		)
 
-		/*
-			DEBUG:
-			fmt.Println("LHS: ", leftHandSide)
-			fmt.Println("RHS: ", rightHandSide)
-			fmt.Println("Assigned above: ", assignedOnLineAbove)
-			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) {
@@ -282,6 +300,7 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) {
 			//     t.X = true
 			//     return t
 			// }
+			// nolint: gocritic
 			if i == len(statements)-1 && i == 1 {
 				if p.nodeEnd(stmt)-p.nodeStart(previousStatement) <= 2 {
 					return true
@@ -351,7 +370,9 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) {
 
 			p.addError(t.Pos(), "assignments should only be cuddled with other assignments")
 		case *ast.DeclStmt:
-			p.addError(t.Pos(), "declarations should never be cuddled")
+			if !p.config.AllowCuddleDeclaration {
+				p.addError(t.Pos(), "declarations should never be cuddled")
+			}
 		case *ast.ExprStmt:
 			switch previousStatement.(type) {
 			case *ast.DeclStmt, *ast.ReturnStmt:
@@ -394,6 +415,25 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) {
 				continue
 			}
 
+			// Special treatment of deferring body closes after error checking
+			// according to best practices. See
+			// https://github.com/bombsimon/wsl/issues/31 which links to
+			// discussion about error handling after HTTP requests. This is hard
+			// coded and very specific but for now this is to be seen as a
+			// special case. What this does is that it *only* allows a defer
+			// statement with `Close` on the right hand side to be cuddled with
+			// an if-statement to support this:
+			//  resp, err := client.Do(req)
+			//  if err != nil {
+			//      return err
+			//  }
+			//  defer resp.Body.Close()
+			if _, ok := previousStatement.(*ast.IfStmt); ok {
+				if atLeastOneInListsMatch(rightHandSide, []string{"Close"}) {
+					continue
+				}
+			}
+
 			if moreThanOneStatementAbove() {
 				p.addError(t.Pos(), "only one cuddle assignment allowed before defer statement")
 
@@ -517,7 +557,7 @@ func (p *Processor) firstBodyStatement(i int, allStmt []ast.Stmt) ast.Node {
 			}
 		}
 
-		p.parseBlockBody(statementBodyContent)
+		p.parseBlockBody(nil, statementBodyContent)
 	case []ast.Stmt:
 		// The Body field for an *ast.CaseClause or *ast.CommClause is of type
 		// []ast.Stmt. We must check leading and trailing whitespaces and then
@@ -530,7 +570,7 @@ func (p *Processor) firstBodyStatement(i int, allStmt []ast.Stmt) ast.Node {
 			nextStatement = allStmt[i+1]
 		}
 
-		p.findLeadingAndTrailingWhitespaces(stmt, nextStatement)
+		p.findLeadingAndTrailingWhitespaces(nil, stmt, nextStatement)
 		p.parseBlockStatements(statementBodyContent)
 	default:
 		p.addWarning(
@@ -664,6 +704,13 @@ func (p *Processor) findRHS(node ast.Node) []string {
 		return p.findRHS(t.Call)
 	case *ast.SendStmt:
 		return p.findLHS(t.Value)
+	case *ast.IndexExpr:
+		rhs = append(rhs, p.findRHS(t.Index)...)
+		rhs = append(rhs, p.findRHS(t.X)...)
+	case *ast.SliceExpr:
+		rhs = append(rhs, p.findRHS(t.X)...)
+		rhs = append(rhs, p.findRHS(t.Low)...)
+		rhs = append(rhs, p.findRHS(t.High)...)
 	default:
 		if x, ok := maybeX(t); ok {
 			return p.findRHS(x)
@@ -679,7 +726,7 @@ func (p *Processor) findRHS(node ast.Node) []string {
 // if it exists. If the node doesn't have an X field nil and false is returned.
 // Known fields with X that are handled:
 // IndexExpr, ExprStmt, SelectorExpr, StarExpr, ParentExpr, TypeAssertExpr,
-// RangeStmt, UnaryExpr, ParenExpr, SLiceExpr, IncDecStmt.
+// RangeStmt, UnaryExpr, ParenExpr, SliceExpr, IncDecStmt.
 func maybeX(node interface{}) (ast.Node, bool) {
 	maybeHasX := reflect.Indirect(reflect.ValueOf(node)).FieldByName("X")
 	if !maybeHasX.IsValid() {
@@ -726,7 +773,8 @@ func atLeastOneInListsMatch(listOne, listTwo []string) bool {
 // findLeadingAndTrailingWhitespaces will find leading and trailing whitespaces
 // in a node. The method takes comments in consideration which will make the
 // parser more gentle.
-func (p *Processor) findLeadingAndTrailingWhitespaces(stmt, nextStatement ast.Node) {
+// nolint: gocognit
+func (p *Processor) findLeadingAndTrailingWhitespaces(ident *ast.Ident, stmt, nextStatement ast.Node) {
 	var (
 		allowedLinesBeforeFirstStatement = 1
 		commentMap                       = ast.NewCommentMap(p.fileSet, stmt, p.file.Comments)
@@ -811,11 +859,16 @@ func (p *Processor) findLeadingAndTrailingWhitespaces(stmt, nextStatement ast.No
 			return
 		}
 
+		// If we allow case to end white whitespace just return.
+		if p.config.AllowCaseTrailingWhitespace {
+			return
+		}
+
 		switch n := nextStatement.(type) {
 		case *ast.CaseClause:
-			blockEndPos = n.Colon
+			blockEndPos = n.Case
 		case *ast.CommClause:
-			blockEndPos = n.Colon
+			blockEndPos = n.Case
 		default:
 			// We're not at the end of the case?
 			return
@@ -824,7 +877,7 @@ func (p *Processor) findLeadingAndTrailingWhitespaces(stmt, nextStatement ast.No
 		blockEndLine = p.fileSet.Position(blockEndPos).Line
 	}
 
-	if p.nodeEnd(lastStatement) != blockEndLine-1 {
+	if p.nodeEnd(lastStatement) != blockEndLine-1 && !isExampleFunc(ident) {
 		p.addError(
 			blockEndPos,
 			"block should not end with a whitespace (or comment)",
@@ -832,6 +885,10 @@ func (p *Processor) findLeadingAndTrailingWhitespaces(stmt, nextStatement ast.No
 	}
 }
 
+func isExampleFunc(ident *ast.Ident) bool {
+	return ident != nil && strings.HasPrefix(ident.Name, "Example")
+}
+
 func (p *Processor) nodeStart(node ast.Node) int {
 	return p.fileSet.Position(node.Pos()).Line
 }

vendor/modules.txt

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