Can we produce a "hit" list for explaining each hits and how to amend them? E.g. https://go-critic.github.io/overview

I do not mind contribute back. I see a lot of good potential for this linter. =)

EDIT: added mind

I'd like to see a configuration option that would prevent short declarations from cuddling with anything but themselves, e.g.

a := 1
b := 2
c := 3

would work, but this should fail with the option enabled:

a := 1
b = 2
c := 3

and it would override many other cuddle rules automatically, also preventing these sorts of things:

result, err := someFunc()
if err != nil {
    fmt.Errorf( . . . )
}

I'm apparently an even bigger fan of whitespace than most 😄

I'd be happy to work on a PR for this if you're open to it

Sorry, I'm not sure how to describe it better, so here goes an example:

var config Configuration
conf.Load(&config)

Another that comes to my mind:

var config Configuration
err = json.Unmarshal(body, &config)

When there is an empty line in between these two statements seem really disconnected, while I consider them more or less a single statement, which isn't possible due Go "limitations".

Hi!

I see that there exists a allow-trailing-comment flag to allow blocks to end with a comment. We would like to have a similar flag that would allow blocks to start with comments.

Here is our use case: we are working on an API and we use https://github.com/go-swagger/go-swagger to generate swagger documentation for our API. The syntax that all routes are defined for the doc is a comment starting with swagger: route and we usually place the handler description right at the start of our handler functions. This does trigger an error (we are using golangci-lint) and we would like to allow such comments instead of applying nolint: wsl to the whole handler function.

Here's a solution to avoid false positives on empty labels. Labels end position always falls into the next line, regardless if it is empty or not. So for empty labels we simply could compensate the end position to avoid the false positive.

This solves #92

For #65

This is the first time I've worked with an AST, it was pretty awesome... Sometimes frustrating.. but awesome :P

I set the new feature to Off by default.

I'd love to be able to enforce that error checking is cuddled with the error assignment.

For example, right now this doesn't trigger the linter:

thing, err := db.GetThing()

if err != nil {
  return err
}

But I'd like it to :)

Thanks for the awesome linter!

I see some options like --no-test -allow-declaration, but I'm uncertain what they do. I was looking for a way to disable some of the warnings about the: block should not start/end with a whitespace but I'm not sure if there is any such option

... I think. I am mighty confused by go modules.

In golangci-lint I am getting the following problem with their v1.21.0 tag:

# github.com/golangci/golangci-lint/pkg/golinters
/go/pkg/mod/github.com/golangci/[email protected]/pkg/golinters/wsl.go:43:6: unknown field 'AllowCaseTrailingWhitespace' in struct literal of type wsl.Configuration

This build worked before on the same commit of my code. I noticed my builds are using different versions of WSL even though they grab the same version of golintci-lint.. The build had worked with wsl v1.2.5. However, now by build is get v1.2.7 of wsl.

I suspect that go modules believes this is fine, because v1.2.7 should be backwards compatible to v1.2.5 if you are not using any new fields according to semantic versioning.

However, when I git diff v1.2.6 v1.2.7 wsl.go I see that the exported configuration property AllowCaseTrailingWhitespace was removed/renamed to CaseForceTrailingWhitespaceLimit, which is a breaking a change:

diff --git a/wsl.go b/wsl.go
index 8a1f45b..ea8d4eb 100644
--- a/wsl.go
+++ b/wsl.go
@@ -50,21 +50,9 @@ type Configuration struct {
        //  }
        AllowMultiLineAssignCuddle bool
 ...
-       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

So I believe that for Go modules, that change would have need to have been v2.x.x and using a new a package folder with v2 at the end of the folder path (or module declaration) as per https://github.com/golang/go/wiki/Modules#releasing-modules-v2-or-higher

Simplified version of my code:

for _, x := range a {
	if x == y {
		goto SKIP // do not append
	}

	clean = append(clean, x)
SKIP:
}       // block should not end with a whitespace (or comment)

I think wsl considers the label SKIP as a whitespace (or comment)...

var err error
x := 12345

I consider these two basically the same, and it's annoying that I have to split them. It makes the code look weird at some places. It's also weird when you realize that this is a perfectly fine way to do the same thing, just more verbose:

var (
    err error
    x	= 12345
)

I see this as just fine:

	// ...
	}
}
return

	// ...

	if err != nil
		return nil, err
	}
	return // "bad"
}

return statements should not be cuddled if block has more than two lines

This linter has been in a maintenance like mode for a while and never got the fixer implemented. One of the reasons the fixer never got added was that I had issues getting some of the rules to work properly. Both grouping of const/var and also due to comments.

Since this linter was created we now have gofumpt which addresses (and fixes!) a lot of the issues wsl reports on. There's also whitespace which only focuses on the empty lines in the start and end of a function. This linter too has support for suggested fixes. Lastly we have nlreturn which checks if your return statement has a preceding newline and can fix that.

So, to actually make this linter more resonable to use (less opinionated and more focused) and also make it easier to finally add a fixer, which I think is really important to make it useful, I want to drop support for all overlapping features and point anyone in need of those to the other alternatives listed above. This would mean that a lot of the rules wsl warns about would be removed. I also think it may help to make the error reporting more generic (similar to how "File is not gofumpt-ed").

I don't really have a plan for when to start on this but I'm writing this down to make it visible.

The rule for a block should not start with a whitespace should not apply to a multiline function signature. Instead a no cuddle rule should be in place instead.

Example

func (c *Client) GetSegment(
	ctx context.Context,
	uri *url.URL,
	out io.Writer,
) (written int64, err error) {

	req, err := http.NewRequestWithContext(ctx, http.MethodGet, uri.String(), http.NoBody)
	if err != nil {
		return 0, errors.Wrap(err, "failed to create request for getting segment")
	}
...
}

There is often a need to add a value to a wait group and then calling a go routine that will mark done on that wait group as soon as it finishes. Such as

wg.Add(1)
go goRoutineWithDone()

wg.Add(1)
go goRoutineWithDone2()

// waiting for all go routines to finish
wg.Wait()

Currently WSL sees this as a problem with needing a whitespace before the go call. I think this should be encouraged. Thoughts?

Is it by design, or in anyway intended that the following cuddled statements inside the anonymous function are not hit by wsl ?

package main

import "fmt"

func main() {
        fmt.Println(func () error {
                foo:="bar"
                fmt.Println("Just a statement")
                if foo == "bar" {
                        return fmt.Errorf("bar")
                }
                return nil
        }())
}

My original function:

func sortedKeys(m map[string]interface{}) sort.StringSlice {
	keys := make(sort.StringSlice, len(m))

    i := 0
	for k := range m { // ranges should only be cuddled with assignments used in the iteration
		keys[i] = k
		i++
	}

	sort.Sort(keys)

	return keys
}

I have found three possible fixes:

1. Isolate i := 0 but this is the index of the for loop

   	func sortedKeys(m map[string]interface{}) sort.StringSlice {
   		keys := make(sort.StringSlice, len(m))
   
   	    i := 0
   
   		for k := range m {
   			keys[i] = k
   			i++
   		}
   
   		sort.Sort(keys)
   
   		return keys
   	}
   

2. Inverse both assignments i := 0 and keys := .. but this is wired :-/

   	func sortedKeys(m map[string]interface{}) sort.StringSlice {
   	    i := 0
   
   		keys := make(sort.StringSlice, len(m))
   		for k := range m {
   			keys[i] = k
   			i++
   		}
   
   		sort.Sort(keys)
   
   		return keys
   	}
   

3. Putting both assignments in one line, but this is the least readable :-(

   	func sortedKeys(m map[string]interface{}) sort.StringSlice {
   		i, keys := 0, make(sort.StringSlice, len(m))
   		for k := range m {
   			keys[i] = k
   			i++
   		}
   
   		sort.Sort(keys)
   
   		return keys
   	}
   

I thing wsl should consider i++ as an assignment used in the iteration. @bombsimon What about your opinion?