From 14ebae29060f7c7be67d31a559986aa8afe7cbb0 Mon Sep 17 00:00:00 2001 From: Simon Sawert Date: Sun, 10 Nov 2019 15:00:07 +0100 Subject: [PATCH] Bump wsl to v1.2.7 --- .golangci.example.yml | 6 +- README.md | 6 +- go.mod | 2 +- go.sum | 4 +- pkg/config/config.go | 22 +- pkg/golinters/wsl.go | 15 +- test/testdata/wsl.go | 40 +- vendor/github.com/bombsimon/wsl/.travis.yml | 6 +- vendor/github.com/bombsimon/wsl/README.md | 670 +++----------------- vendor/github.com/bombsimon/wsl/wsl.go | 209 ++++-- vendor/modules.txt | 2 +- 11 files changed, 310 insertions(+), 672 deletions(-) diff --git a/.golangci.example.yml b/.golangci.example.yml index 7735dc1b..8c64ead1 100644 --- a/.golangci.example.yml +++ b/.golangci.example.yml @@ -223,10 +223,12 @@ 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 + # Allow trailing comments in ending of blocks + allow-trailing-comment: false + # Force newlines in end of case at this limit (0 = never). + force-case-trailing-whitespace: 0 linters: enable: diff --git a/README.md b/README.md index e85f9d2e..ae67a4c0 100644 --- a/README.md +++ b/README.md @@ -824,10 +824,12 @@ 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 + # Allow trailing comments in ending of blocks + allow-trailing-comment: false + # Force newlines in end of case at this limit (0 = never). + force-case-trailing-whitespace: 0 linters: enable: diff --git a/go.mod b/go.mod index 263d2c88..180b09af 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.12 require ( github.com/OpenPeeDeeP/depguard v1.0.1 - github.com/bombsimon/wsl v1.2.5 + github.com/bombsimon/wsl v1.2.7 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 diff --git a/go.sum b/go.sum index 01daee95..aa0a9f87 100644 --- a/go.sum +++ b/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.5 h1:9gTOkIwVtoDZywvX802SDHokeX4kW1cKnV8ZTVAPkRs= -github.com/bombsimon/wsl v1.2.5/go.mod h1:43lEF/i0kpXbLCeDXL9LMT8c92HyBywXb0AsgMHYngM= +github.com/bombsimon/wsl v1.2.7 h1:hckBZYt27e6Ff6yqV/SurMUe48BmXzJk/KVjxDyWtUE= +github.com/bombsimon/wsl v1.2.7/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= diff --git a/pkg/config/config.go b/pkg/config/config.go index 1b8de675..2ddfc60b 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -255,11 +255,12 @@ type GocognitSettings struct { } type WSLSettings struct { - StrictAppend bool `mapstructure:"strict-append"` - AllowAssignAndCallCuddle bool `mapstructure:"allow-assign-and-call"` - AllowMultiLineAssignCuddle bool `mapstructure:"allow-multiline-assign"` - AllowCaseTrailingWhitespace bool `mapstructure:"allow-case-trailing-whitespace"` - AllowCuddleDeclaration bool `mapstructure:"allow-cuddle-declarations"` + StrictAppend bool `mapstructure:"strict-append"` + AllowAssignAndCallCuddle bool `mapstructure:"allow-assign-and-call"` + AllowMultiLineAssignCuddle bool `mapstructure:"allow-multiline-assign"` + AllowCuddleDeclaration bool `mapstructure:"allow-cuddle-declarations"` + AllowTrailingComment bool `mapstructure:"allow-trailing-comment"` + CaseForceTrailingWhitespaceLimit int `mapstructure:"force-case-trailing-whitespace:"` } var defaultLintersSettings = LintersSettings{ @@ -291,11 +292,12 @@ var defaultLintersSettings = LintersSettings{ MinComplexity: 30, }, WSL: WSLSettings{ - StrictAppend: true, - AllowAssignAndCallCuddle: true, - AllowMultiLineAssignCuddle: true, - AllowCaseTrailingWhitespace: true, - AllowCuddleDeclaration: false, + StrictAppend: true, + AllowAssignAndCallCuddle: true, + AllowMultiLineAssignCuddle: true, + AllowCuddleDeclaration: false, + AllowTrailingComment: false, + CaseForceTrailingWhitespaceLimit: 0, }, } diff --git a/pkg/golinters/wsl.go b/pkg/golinters/wsl.go index 174a673b..a2cb6b2a 100644 --- a/pkg/golinters/wsl.go +++ b/pkg/golinters/wsl.go @@ -37,13 +37,14 @@ func NewWSL() *goanalysis.Linter { files = []string{} linterCfg = lintCtx.Cfg.LintersSettings.WSL processorCfg = wsl.Configuration{ - StrictAppend: linterCfg.StrictAppend, - AllowAssignAndCallCuddle: linterCfg.AllowAssignAndCallCuddle, - AllowMultiLineAssignCuddle: linterCfg.AllowMultiLineAssignCuddle, - AllowCaseTrailingWhitespace: linterCfg.AllowCaseTrailingWhitespace, - AllowCuddleDeclaration: linterCfg.AllowCuddleDeclaration, - AllowCuddleWithCalls: []string{"Lock", "RLock"}, - AllowCuddleWithRHS: []string{"Unlock", "RUnlock"}, + StrictAppend: linterCfg.StrictAppend, + AllowAssignAndCallCuddle: linterCfg.AllowAssignAndCallCuddle, + AllowMultiLineAssignCuddle: linterCfg.AllowMultiLineAssignCuddle, + AllowCuddleDeclaration: linterCfg.AllowCuddleDeclaration, + AllowTrailingComment: linterCfg.AllowTrailingComment, + CaseForceTrailingWhitespaceLimit: linterCfg.CaseForceTrailingWhitespaceLimit, + AllowCuddleWithCalls: []string{"Lock", "RLock"}, + AllowCuddleWithRHS: []string{"Unlock", "RUnlock"}, } ) diff --git a/test/testdata/wsl.go b/test/testdata/wsl.go index 81fc71c8..0a50dafd 100644 --- a/test/testdata/wsl.go +++ b/test/testdata/wsl.go @@ -159,7 +159,7 @@ func allowTrailing(i int) { case 2: fmt.Println("two") - + // Comments OK too! case 3: fmt.Println("three") } @@ -172,3 +172,41 @@ func ExampleSomeOutput() { // Output: // Hello, world } + +func IncDecStmt() { + counter := 0 + for range make([]int, 5) { + counter++ + } + + type t struct { + counter int + } + + x := t{5} + + x.counter-- + if x.counter > 0 { + fmt.Println("not yet 0") + } +} + +func AnonymousBlock() { + func(a, b int) { // ERROR "block should not start with a whitespace" + + fmt.Println(a + b) + }(1, 1) +} + +func MultilineComment() { + if true { + /* + Ok to start block with + a + long + multiline + cmoment + */ + fmt.Println("true") + } +} diff --git a/vendor/github.com/bombsimon/wsl/.travis.yml b/vendor/github.com/bombsimon/wsl/.travis.yml index d84a5d6c..5e2e26ed 100644 --- a/vendor/github.com/bombsimon/wsl/.travis.yml +++ b/vendor/github.com/bombsimon/wsl/.travis.yml @@ -2,9 +2,9 @@ language: go go: - - "1.13" - - "1.12" - - "1.11" + - 1.13.x + - 1.12.x + - 1.11.x env: global: diff --git a/vendor/github.com/bombsimon/wsl/README.md b/vendor/github.com/bombsimon/wsl/README.md index a8fb0d1e..774feed2 100644 --- a/vendor/github.com/bombsimon/wsl/README.md +++ b/vendor/github.com/bombsimon/wsl/README.md @@ -21,595 +21,105 @@ 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! +## Installation + +### By `go get` (local installation) + +You can do that by using: + +```sh +go get -u github.com/bombsimon/wsl/cmd/... +``` + +### By golangci-lint (CI automation) + +`wsl` is already integrated with +[golangci-lint](https://github.com/golangci/golangci-lint). Please refer to the +instructions there. + ## Usage -Install by using `go get -u github.com/bombsimon/wsl/cmd/...`. +How to use depends on how you install `wsl`. -Run with `wsl [--no-test] [files...]` or `wsl ./package/...`. The "..." -wildcard is not used like other `go` commands but instead can only be to a -relative or absolute path. +### With local binary + +The general command format for `wsl` is: + +```sh +$ wsl [flags] [files...] +$ wsl [flags] + +# Examples + +$ wsl ./main.go +$ wsl --no-test ./main.go +$ wsl -allow-declaration ./main.go +$ wsl --no-test --allow-cuddle-declaration ./main.go +$ wsl --no-test --allow-trailing-comment ./myProject/... +``` + +The "..." wildcard is not used like other `go` commands but instead can only +be to a relative or absolute path. 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 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) +### By `golangci-lint` (CI automation) -## Rules +The recommended command is: -Note that this linter doesn't take in consideration the issues that will be -fixed with `gofmt` so ensure that the code is properly formatted. - -### Never use empty lines - -Even though this linter was built to **promote** the usage of empty lines, there -are a few places where they should never be used. - -Never use empty lines in the start or end of a block! - -**Don't** - -```go -if someBooleanValue { - - fmt.Println("i like starting newlines") -} - -if someOtherBooleanValue { - fmt.Println("also trailing") - -} - -switch { - -case 1: - fmt.Println("switch is also a block") -} - -switch { -case 1: - - fmt.Println("not in a case") -case 2: - fmt.Println("or at the end") - -} - -func neverNewlineAfterReturn() { - return true - -} - -func notEvenWithComments() { - return false - // I just forgot to say this... -} +```sh +golangci-lint --disable-all --enable wsl ``` -**Do** - -```go -if someBooleanValue { - fmt.Println("this is tight and nice") -} - -switch { -case 1: - fmt.Println("no blank lines here") -case 2: - // Comments are fine here! - fmt.Println("or here") -} - -func returnCuddleded() { - // My comment goes above the last statement! - return true -} -``` - -### Use empty lines - -There's an easy way to improve logic and readability by enforcing whitespaces at -the right places. Usually this is around blocks and after declarations. - -#### If - -If statements should only be cuddled with assignments/declarations of variables -used in the condition and only one assignment should be cuddled. Never cuddle if -with anything but assignments. - -**Don't** - -```go -notConditional := "x" -if somethingElse == "y" { - fmt.Println("what am i checking?") -} - -first := 1 -second := 2 -third := 3 -forever := 4 -if forever { - return true -} - -if false { - fmt.Println("first if") -} -if true { - fmt.Println("second if is cuddled") -} -``` - -**Do** - -```go -val, err := SomeThing() -if err != nil { - // err is assigned on line above -} - -first := 1 -second := 2 -third := 3 -forever := 4 - -if forever > 3 { - return fmt.Sprintf("group multiple assignments away from if") -} - -// Or separate from your condition. -first := 1 -second := 2 -third := 3 - -forever := 4 -if forever > 3 { - return fmt.Sprintf("group multiple assignments away from if") -} - -if false { - // This is one statement -} - -if true { - // This is another one, don't cuddled them! -} -``` - -#### Return - -Return should be placed on a separate line from other statement unless the block -consists of only two lines (including the return). - -**Don't** - -```go -doSomething() -add := 1+2 -fmt.Sprintf(add) -return false - -if true { - stmt.X = true - stmt.Y = false - return true -} -``` - -**Do** - -```go -doSomething() - -add := 1+2 -fmt.Sprintf(add)) - -return false - -if true { - stmt.X = "only one line without return, may cuddled" - return true -} - -if thisWorksToo { - whitespace := true - - return false -} -``` - -#### Branch statement - -The same rule as for return - -**Don't** - -```go -for i := range make([]int, 5) { - if i > 2 { - sendToOne(i) - sendToSecond(i) - continue - } -} -``` - -**Do** - -```go -for i := range make([]int, 5) { - if i > 2 { - sendToOne(i) - sendToSecond(i) - - continue - } - - if statement == "is short" { - sendToOne(i) - break - } -} -``` - -#### Assignment - -Assignments may only be cuddled with other assignments. - -**Don't** - -```go -assignOne := "one" -callFunc(assignOne) -assignTwo := "two") -callFunc(assignTwo) - -if true { - return -} -assigningClose := "bad" - -var x = 2 -y := 3 -``` - -**Do** - -```go -assignOne := "one" -assignTwo := "two") - -callFunc(assignOne) -callFunc(assignTwo) - -// Or group assignment and call by usage. -assignOne := "one" -callFunc(assignOne) - -assignTwo := "two") -callFunc(assignTwo) - -if true { - return -} - -notAssigningClose := "not bad" - -var x = 2 - -y := 3 -``` - -#### Declarations - -Declarations should never be cuddled with anything, not even other declarations. - -**Don't** - -```go -var x int -var y int - -z := 2 -var a -``` - -**Do** - -```go -// Group declarations, they'll align nice and tidy! -var ( - x int - y int -) - -z := 2 - -var a -``` - -#### Expressions - -Expressions (function calls) may never be cuddled with declarations or return -statements. Expressions may also not be cuddled with assignments if not passed -to the expression func. - -**Don't** - -```go -var a bool -fmt.Println(a) - -foo := true -someFunc(false) - -if someBool() { - fmt.Println("doing things") -} -x.Calling() -``` - -**Do** - -```go -var b bool - -fmt.Println(b) - -foo := true -someFunc(foo) - -bar := false - -someFunc(true) - -if someBool() { - fmt.Println("doing things") -} - -x.Calling() -``` - -#### Ranges - -Range statements may only be cuddled with assignments that are used in the -range. Just like if statements this only applies if it's a single assignment -cuddled and not multiple. - -Ranges may also be cuddled with assignments that are used directly in the block -as first statement. - -**Don't** - -```go -noRangeList := []string{"a", "b", "c"} -for _, x := range anotherList { - fmt.Println(x) -} - -oneList := []int{1, 2, 3} -twoList := []int{4, 5, 6} -for i := range twoList { - fmt.Println("too much assignments!") -} - -myCount := 0 -for _, v := range aList { - fmt.Sprintf("first statement doesn't use assignment") -} -``` - -**Do** - -```go -rangeList := []string{"a", "b", "c"} -for _, x := range rangeList { - fmt.Println(x) -} - -oneList := []int{1, 2, 3} - -twoList := []int{4, 5, 6} -for i := range twoList { - fmt.Println("too much assignments!") -} - -myCount := 0 -for _, v := range aList { - myCount += v - - fmt.Sprintf("first statement uses cuddled assignment") -} -``` - -#### Defer - -Defer is almost handled like return statements but there are cases where -grouping defer statements with each other or expression calls may improve -readability. - -Defer statements may be cuddled with other defer statements as many times as you -like. It may also be cuddled with assignments above or expression variables on -the line above. - -**Don't** - -```go -first := getFirst() -defer first.Close() -second := getSecond() // This will fail -defer second.Close() - -first := getFirst() -second := getSecond() -defer first.Close() // Too many assignments above -defer second.Close() - -m1.Lock() -defer m2.RUnlock() // Not the expression above -``` - -**Do** - -```go -first := getFirst() -second := getSecond() - -defer first.Close() -defer second.Close() - -// Or group by usage. -first := getFirst() -defer first.Close() - -second := getSecond() -defer second.Close() - -m.Lock() -defer m.Unlock() -``` - -#### For loops - -For statements works similar like ranges except that anonymous (infinite) loops -may never be cuddled. Just like the range statement, variables used in the for -or in first statement in the body may be cuddled. - -**Don't** - -```go -t := true -for notT { - fmt.Println("where's t used?") -} - -n := 0 -m := 1 -for x < 100 { - n += x // m not used in for or body -} - -n := 1 -for { - fmt.Println("never cuddled for without condition") -} -``` - -**Do** - -```go -t := true -for t { - fmt.Println("t used in for") -} - -n := 0 -for x < 100 { - n += x // n used in first block statement. -} - -n := 1 - -for { - fmt.Println("never cuddled for without condition") -} -``` - -#### Go - -Go routines may only be executed if there's a maximum of one assignments above -and that assignment is used in the expression. - -**Don't** - -```go -first := func() {} -second := func() {} -go second() - -notUsed := func() {} -go first() - -x := "1" -go func() { - fmt.Println("where's x used!=") -}() -``` - -**Do** - -```go -first := func() {} -go first() - -notUsed := func() {} - -first := func() {} -go first() -``` - -#### Switch and Type switch - -The same rules applies for switch and type switch statements with the exception -of anonymous type switches. That means type switches where a new variable is not -assigned. It's also allowed to cuddled type switches with variables used if it's -used as the first argument in the first case. - -Type switches may only be cuddled with one assignment above and if that -assignment is used in the switch. - -**Don't** - -```go -notSome := SomeInt() -switch some { -case 1: - fmt.Println("1") -default: - fmt.Println("not 1") -} - -notSwitched := SomeInt() -switch { -case 1 > 2: - fmt.Println("whitespace between assignments") -} - -n := 0 -switch v := some.(type): -case typeOne: - x := v.X // n not used in switch or body -case typeTwo: - x := v.X -} -``` - -**Do** - -```go -some := SomeInt() -switch some { -case 1: - fmt.Println("1") -default: - fmt.Println("not 1") -} - -notSwitched := SomeInt() - -switch { -case 1 > 2: - fmt.Println("whitespace between assignments") -} - -n := 0 -switch v := some.(type): -case typeOne: - n = v.X // n is used first in block, OK to cuddle -case typeTwo: - n = v.Y -} -``` +For more information, please refer to +[golangci-lint](https://github.com/golangci/golangci-lint)'s documentation. + +## Issues and configuration + +The linter suppers a few ways to configure it to satisfy more than one kind of +code style. These settings could be set either with flags or with YAML +configuration if used via `golangci-lint`. + +The supported configuration can be found [in the documentation](doc/configuration.md). + +Below are the available checklist for any hit from `wsl`. If you do not see any, +feel free to raise an [issue](https://github.com/bombsimon/wsl/issues/new). + +> **Note**: this linter doesn't take in consideration the issues that will be +> fixed with `go fmt -s` so ensure that the code is properly formatted before +> use. + +* [Anonymous switch statements should never be cuddled](doc/rules.md#anonymous-switch-statements-should-never-be-cuddled) +* [Append only allowed to cuddle with appended value](doc/rules.md#append-only-allowed-to-cuddle-with-appended-value) +* [Assignments should only be cuddled with other assignments](doc/rules.md#assignments-should-only-be-cuddled-with-other-assignments) +* [Block should not end with a whitespace (or comment)](doc/rules.md#block-should-not-end-with-a-whitespace-or-comment) +* [Block should not start with a whitespace](doc/rules.md#block-should-not-start-with-a-whitespace) +* [Case block should end with newline at this size](doc/rules.md#case-block-should-end-with-newline-at-this-size) +* [Branch statements should not be cuddled if block has more than two lines](doc/rules.md#branch-statements-should-not-be-cuddled-if-block-has-more-than-two-lines) +* [Declarations should never be cuddled](doc/rules.md#declarations-should-never-be-cuddled) +* [Defer statements should only be cuddled with expressions on same variable](doc/rules.md#defer-statements-should-only-be-cuddled-with-expressions-on-same-variable) +* [Expressions should not be cuddled with blocks](doc/rules.md#expressions-should-not-be-cuddled-with-blocks) +* [Expressions should not be cuddled with declarations or returns](doc/rules.md#expressions-should-not-be-cuddled-with-declarations-or-returns) +* [For statement without condition should never be cuddled](doc/rules.md#for-statement-without-condition-should-never-be-cuddled) +* [For statements should only be cuddled with assignments used in the iteration](doc/rules.md#for-statements-should-only-be-cuddled-with-assignments-used-in-the-iteration) +* [Go statements can only invoke functions assigned on line above](doc/rules.md#go-statements-can-only-invoke-functions-assigned-on-line-above) +* [If statements should only be cuddled with assignments](doc/rules.md#if-statements-should-only-be-cuddled-with-assignments) +* [If statements should only be cuddled with assignments used in the if + statement + itself](doc/rules.md#if-statements-should-only-be-cuddled-with-assignments-used-in-the-if-statement-itself) +* [Only cuddled expressions if assigning variable or using from line + above](doc/rules.md#only-cuddled-expressions-if-assigning-variable-or-using-from-line-above) +* [Only one cuddle assignment allowed before defer statement](doc/rules.md#only-one-cuddle-assignment-allowed-before-defer-statement) +* [Only one cuddle assginment allowed before for statement](doc/rules.md#only-one-cuddle-assignment-allowed-before-for-statement) +* [Only one cuddle assignment allowed before go statement](doc/rules.md#only-one-cuddle-assignment-allowed-before-go-statement) +* [Only one cuddle assignment allowed before if statement](doc/rules.md#only-one-cuddle-assignment-allowed-before-if-statement) +* [Only one cuddle assignment allowed before range statement](doc/rules.md#only-one-cuddle-assignment-allowed-before-range-statement) +* [Only one cuddle assignment allowed before switch statement](doc/rules.md#only-one-cuddle-assignment-allowed-before-switch-statement) +* [Only one cuddle assignment allowed before type switch statement](doc/rules.md#only-one-cuddle-assignment-allowed-before-type-switch-statement) +* [Ranges should only be cuddled with assignments used in the iteration](doc/rules.md#ranges-should-only-be-cuddled-with-assignments-used-in-the-iteration) +* [Return statements should not be cuddled if block has more than two lines](doc/rules.md#return-statements-should-not-be-cuddled-if-block-has-more-than-two-lines) +* [Stmt type not implemented](doc/rules.md#stmt-type-not-implemented) +* [Switch statements should only be cuddled with variables switched](doc/rules.md#switch-statements-should-only-be-cuddled-with-variables-switched) +* [Type switch statements should only be cuddled with variables switched](doc/rules.md#type-switch-statements-should-only-be-cuddled-with-variables-switched) diff --git a/vendor/github.com/bombsimon/wsl/wsl.go b/vendor/github.com/bombsimon/wsl/wsl.go index fe62b62c..ea8d4eb5 100644 --- a/vendor/github.com/bombsimon/wsl/wsl.go +++ b/vendor/github.com/bombsimon/wsl/wsl.go @@ -50,21 +50,12 @@ 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 + // If the number of lines in a case block is equal to or lager than this + // number, the case *must* end white a newline. + CaseForceTrailingWhitespaceLimit int + + // AllowTrailingComment will allow blocks to end with comments. + AllowTrailingComment bool // AllowCuddleDeclaration will allow multiple var/declaration statements to // be cuddled. This defaults to false but setting it to true will enable the @@ -93,12 +84,13 @@ type Configuration struct { // DefaultConfig returns default configuration func DefaultConfig() Configuration { return Configuration{ - StrictAppend: true, - AllowAssignAndCallCuddle: true, - AllowMultiLineAssignCuddle: true, - AllowCaseTrailingWhitespace: false, - AllowCuddleWithCalls: []string{"Lock", "RLock"}, - AllowCuddleWithRHS: []string{"Unlock", "RUnlock"}, + StrictAppend: true, + AllowAssignAndCallCuddle: true, + AllowMultiLineAssignCuddle: true, + AllowTrailingComment: false, + CaseForceTrailingWhitespaceLimit: 0, + AllowCuddleWithCalls: []string{"Lock", "RLock"}, + AllowCuddleWithRHS: []string{"Unlock", "RUnlock"}, } } @@ -203,14 +195,12 @@ func (p *Processor) parseBlockBody(ident *ast.Ident, block *ast.BlockStmt) { // nolint: gocognit func (p *Processor) parseBlockStatements(statements []ast.Stmt) { for i, stmt := range statements { - // TODO: How to tell when and where func literals may exist to enforce - // linting. - if as, isAssignStmt := stmt.(*ast.AssignStmt); isAssignStmt { - for _, rhs := range as.Rhs { - if fl, isFuncLit := rhs.(*ast.FuncLit); isFuncLit { - p.parseBlockBody(nil, fl.Body) - } - } + // Start by checking if this statement is another block (other than if, + // for and range). This could be assignment to a function, defer or go + // call with an inline function or similar. If this is found we start by + // parsing this body block before moving on. + for _, stmtBlocks := range p.findBlockStmt(stmt) { + p.parseBlockBody(nil, stmtBlocks) } firstBodyStatement := p.firstBodyStatement(i, statements) @@ -322,11 +312,15 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) { continue } - if !atLeastOneInListsMatch(rightAndLeftHandSide, assignedOnLineAbove) { - if !atLeastOneInListsMatch(assignedOnLineAbove, assignedFirstInBlock) { - p.addError(t.Pos(), "if statements should only be cuddled with assignments used in the if statement itself") - } + if atLeastOneInListsMatch(rightAndLeftHandSide, assignedOnLineAbove) { + continue } + + if atLeastOneInListsMatch(assignedOnLineAbove, assignedFirstInBlock) { + continue + } + + p.addError(t.Pos(), "if statements should only be cuddled with assignments used in the if statement itself") case *ast.ReturnStmt: if isLastStatementInBlockOfOnlyTwoLines() { continue @@ -473,6 +467,10 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) { } } case *ast.GoStmt: + if _, ok := previousStatement.(*ast.GoStmt); ok { + continue + } + if moreThanOneStatementAbove() { p.addError(t.Pos(), "only one cuddle assignment allowed before go statement") @@ -595,9 +593,10 @@ func (p *Processor) findLHS(node ast.Node) []string { *ast.ReturnStmt, *ast.GoStmt, *ast.CaseClause, *ast.CommClause, *ast.CallExpr, *ast.UnaryExpr, *ast.BranchStmt, *ast.TypeSpec, *ast.ChanType, - *ast.DeferStmt, *ast.TypeAssertExpr, *ast.IncDecStmt, - *ast.RangeStmt: - // Nothing to add to LHS + *ast.DeferStmt, *ast.TypeAssertExpr, *ast.RangeStmt: + // Nothing to add to LHS + case *ast.IncDecStmt: + return p.findLHS(t.X) case *ast.Ident: return []string{t.Name} case *ast.AssignStmt: @@ -722,6 +721,33 @@ func (p *Processor) findRHS(node ast.Node) []string { return rhs } +func (p *Processor) findBlockStmt(node ast.Node) []*ast.BlockStmt { + var blocks []*ast.BlockStmt + + switch t := node.(type) { + case *ast.AssignStmt: + for _, x := range t.Rhs { + blocks = append(blocks, p.findBlockStmt(x)...) + } + case *ast.CallExpr: + blocks = append(blocks, p.findBlockStmt(t.Fun)...) + case *ast.FuncLit: + blocks = append(blocks, t.Body) + case *ast.ExprStmt: + blocks = append(blocks, p.findBlockStmt(t.X)...) + case *ast.ReturnStmt: + for _, x := range t.Results { + blocks = append(blocks, p.findBlockStmt(x)...) + } + case *ast.DeferStmt: + blocks = append(blocks, p.findBlockStmt(t.Call)...) + case *ast.GoStmt: + blocks = append(blocks, p.findBlockStmt(t.Call)...) + } + + return blocks +} + // maybeX extracts the X field from an AST node and returns it with a true value // if it exists. If the node doesn't have an X field nil and false is returned. // Known fields with X that are handled: @@ -839,7 +865,10 @@ func (p *Processor) findLeadingAndTrailingWhitespaces(ident *ast.Ident, stmt, ne break } - allowedLinesBeforeFirstStatement += len(commentGroup.List) + // Support both /* multiline */ and //single line comments + for _, c := range commentGroup.List { + allowedLinesBeforeFirstStatement += len(strings.Split(c.Text, "\n")) + } } } @@ -850,38 +879,92 @@ func (p *Processor) findLeadingAndTrailingWhitespaces(ident *ast.Ident, stmt, ne ) } - // If the blockEndLine is 0 we're a case clause. If we don't have any - // nextStatement the trailing whitespace will be handled when parsing the - // switch. If we do have a next statement we can see where it starts by - // getting it's colon position. - if blockEndLine == 0 { - if nextStatement == nil { - return + // If the blockEndLine is not 0 we're a regular block (not case). + if blockEndLine != 0 { + if p.config.AllowTrailingComment { + if lastComment, ok := commentMap[lastStatement]; ok { + var ( + lastCommentGroup = lastComment[len(lastComment)-1] + lastCommentLine = lastCommentGroup.List[len(lastCommentGroup.List)-1] + countNewlines = 0 + ) + + countNewlines += len(strings.Split(lastCommentLine.Text, "\n")) + + // No newlines between trailing comments and end of block. + if p.nodeStart(lastCommentLine)+countNewlines != blockEndLine-1 { + return + } + } } - // If we allow case to end white whitespace just return. - if p.config.AllowCaseTrailingWhitespace { - return + if p.nodeEnd(lastStatement) != blockEndLine-1 && !isExampleFunc(ident) { + p.addError(blockEndPos, "block should not end with a whitespace (or comment)") } - switch n := nextStatement.(type) { - case *ast.CaseClause: - blockEndPos = n.Case - case *ast.CommClause: - blockEndPos = n.Case - default: - // We're not at the end of the case? - return - } - - blockEndLine = p.fileSet.Position(blockEndPos).Line + return } - if p.nodeEnd(lastStatement) != blockEndLine-1 && !isExampleFunc(ident) { - p.addError( - blockEndPos, - "block should not end with a whitespace (or comment)", - ) + // If we don't have any nextStatement the trailing whitespace will be + // handled when parsing the switch. If we do have a next statement we can + // see where it starts by getting it's colon position. We set the end of the + // current case to the position of the next case. + switch n := nextStatement.(type) { + case *ast.CaseClause: + blockEndPos = n.Case + case *ast.CommClause: + blockEndPos = n.Case + default: + // No more cases + return + } + + blockEndLine = p.fileSet.Position(blockEndPos).Line - 1 + + var ( + blockSize = blockEndLine - blockStartLine + caseTrailingCommentLines int + ) + + // TODO: I don't know what comments are bound to in cases. For regular + // blocks the last comment is bound to the last statement but for cases + // they are bound to the case clause expression. This will however get us all + // comments and depending on the case expression this gets tricky. + // + // To handle this I get the comment map from the current statement (the case + // itself) and iterate through all groups and all comment within all groups. + // I then get the comments after the last statement but before the next case + // clause and just map each line of comment that way. + for _, commentGroups := range commentMap { + for _, commentGroup := range commentGroups { + for _, comment := range commentGroup.List { + commentLine := p.fileSet.Position(comment.Pos()).Line + + // Ignore comments before the last statement. + if commentLine <= p.nodeStart(lastStatement) { + continue + } + + // Ignore comments after the end of this case. + if commentLine > blockEndLine { + continue + } + + // This allows /* multiline */ comments with newlines as well + // as regular (//) ones + caseTrailingCommentLines += len(strings.Split(comment.Text, "\n")) + } + } + } + + hasTrailingWhitespace := p.nodeEnd(lastStatement)+caseTrailingCommentLines != blockEndLine + + // If the force trailing limit is configured and we don't end with a newline. + if p.config.CaseForceTrailingWhitespaceLimit > 0 && !hasTrailingWhitespace { + // Check if the block size is too big to miss the newline. + if blockSize >= p.config.CaseForceTrailingWhitespaceLimit { + p.addError(lastStatement.Pos(), "case block should end with newline at this size") + } } } diff --git a/vendor/modules.txt b/vendor/modules.txt index 93b4abdc..765a0903 100644 --- a/vendor/modules.txt +++ b/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.5 +# github.com/bombsimon/wsl v1.2.7 github.com/bombsimon/wsl # github.com/davecgh/go-spew v1.1.1 github.com/davecgh/go-spew/spew