Comments are not considered part of the block

[dionysius]

Hi, I found a part of my code which wsl should have reported linting errors, but hasn't because I found out that the comments are also considered as "block separators". Consider this arbitary file:

package root

import "fmt"

func init() {
	var foo string
	// some comment
	var bar string
	// if something
	err := fmt.Errorf("foo")
	if err != nil {
		fmt.Println("do-something")
		// bla
		return
	}
	// something else
	var baz string
	// and
	return
}

I would've expected that different wsl rules would be reported, but aren't.

$ go run github.com/bombsimon/wsl/cmd/wsl -w test.go
<no output>
$ go list -json github.com/bombsimon/wsl/cmd/wsl | jq -r '.Module.Version'
v1.2.8

I would've expected that the comments are part of that block, and a newline before the comment fulfills the separation.

for comparison this would satisfy wsl if there were no comments

package root

import "fmt"

func init() {
	var foo string

	// some comment
	var bar string

	// if something
	err := fmt.Errorf("foo")
	if err != nil {
		fmt.Println("do-something")

		// bla
		return
	}

	// something else
	var baz string

	// and
	return
}

[bombsimon]

Thanks for the report!

This is "kind of" intentional. I've known about this since the start and I haven't made up my mind if this is how I want it or not. Since the whole idea with this linter is to improve readability which I think is done by separating with newliens or comments I think I'm ok with this.

The same goes for leading comments. The linter doesn't allow you to start a block with a newline but it may start with comments. That's not because I see comment's as statements or nodes, it's because I don't think it has a bad impact on the readability.

However, with the work on #4 I need to take comments for a statement into consideration so maybe I should bundle each node with it's leading and trailing comments when they're found and have this configurable.

I'll definitely leave this ticket open, I hope people feel that this is the right way gives this a thumbs up. Depending on the work with grouping statements and comments together this might be an easy implementation in a future release.

[mholiday-nyt]

I find it odd that this fails:

func xyz() {
    // here's a starting comment with whitespace after

    if x == nil {
    }

    . . .

because ignoring the comment as an element then causes the whitespace after it to be "leading whitespace" within the block

[bombsimon]

Yes that doesn't sound right, I wonder if it's a result of mergin #73 or if it was this way before. I don't think that should be valid but working with comments, especially those not obvious bound to a node, is the trickiest part and one of the reasons I still not finished the fixer for wsl. Thisi one should be fairly simple though 🤞🏼

[mholiday-nyt]

I've found what I think is the fix for the above, I'll open another issue and PR for it

        // If we have multiple groups, add support for newline between each group.
        if p.config.AllowSeparatedLeadingComment {
-               if seenCommentGroups > 1 {
-                       allowedLinesBeforeFirstStatement += seenCommentGroups - 1
+               if seenCommentGroups > 0 {
+                       allowedLinesBeforeFirstStatement += seenCommentGroups
                }
        }

and to add the missing doc for that flag :0)

[bombsimon]

Nice find! Yeah that must be it, at least if one should trust the comment. :)

[mholiday-nyt]

Will continue the discussion in #104; that logic is not quite good enough :-(