Add support to cuddle assignments for a whole block

[jacobdrury]

Support to cuddle assignments for a whole block

Implements #24 #50

Right now you have to use the variable, type or field in the first statement in a block if you want to use it like this:

variable := "assigned"
[token] {
    // Must use 'variable' here
}

This PR will update this cuddle check to look in the entire body of the statement to check if the variable is used. Meaning the following will now be allowed:

var numbers []int
for i := 0; i < 10; i++ {
    if x == y {
        numbers = append(numbers, i)
    }
}

counter := 0
if somethingTrue {
    checker := getAChecker()
    if !checker {
        return
    }

    counter++ // Cuddled variable used in block, but not as first statement
}

var id2 string
switch i := objectID.(type) {
case int:
    if true {
        id2 = strconv.Itoa(i)
    }
case uint32:
    if true {
        id2 = strconv.Itoa(int(i))
    }
case string:
    if true {
        id2 = i
    }
}

[bombsimon]

This is great! Very nice using the ast.Inspect and not keeping track of variables in each scope manually.

I think this looks good as is other than I want it to be an opt-in and not enabled by default. I think we can call it allow-cuddle-used-in-block or similar.

[bombsimon]

I think we want to add this as an opt-in and keep the old behavior. Ideally we could use the same code/walk but just return after the first variable is found if we didn't opt in.

[jacobdrury]

I decided to use the old code if the new flag isn't enabled that way we don't introduce any unintended new behavior on accident by using the new ast.Inspect approach. Lmk if you want to re-visit this.

if firstBodyStatement != nil {
	if p.config.CuddleUsedInBlock {
		identifiersUsedInBlock = p.findUsedVariablesInStatement(stmt)
	} else {
		identifiersUsedInBlock = append(p.findLHS(firstBodyStatement), p.findRHS(firstBodyStatement)...)
	}
}

[deltics]

But the old behaviour is not what the rule describes: "must be used in the iteration". 🤔

If you wanted to keep the old behaviour it might be better to re-introduce it as a new rule with a more accurate error message so that the different messages can be interpreted more easily, rather than a message which may be misleading/inaccurate depending on the configuration of the rule producing that message.

[jacobdrury]

Thats why I originally introduced this as a fix rather than a new flag. There was no mention of the "first line only" rule. All the rules just said that it "must be used in the iteration"

[bombsimon]

tl;dr - there's a bunch of bugs and bad behavior here, no need to adress that for now. I think it should be addressed, but I'm fine isolating this PR to only address the new feature flag and don't try to fix this. The flag kan be added in a minor release directly and we can fix the old stuff in a future breaking change.

---

Yeah this is a bug and bad documentation, it stems from way back when each different kind of block was seen as separate. Code changed and never really got adapted (which is pretty clear from the whole code base tbh).

The only mention of it is here for range specifically. It's just a comment on the second example. It's also documented here as code docs, but that's not very clear from a user perspective.

I had a quick look now and it seems to be missing from switch statements but exist for type switch.

There are more bugs with defer and go statements passing the whole Body causing false negatives. These two passes but shouldn't according to the other logc.

y := 1
go func() {
	_ = 1
	y = 1
}()

y := 1
defer func() {
	_ = 1
	y = 1
}()

[jacobdrury]

@bombsimon PR suggestions pushed. Please let me know your thoughts!

[bombsimon]

This is great, thank you so much!! Didn't expect this to land in the current version so many years later but this is a great solution!

Just a nit on the docs.

[coveralls]

coverage: 94.19% (+0.1%) from 94.044%
when pulling dd00deb on jacobdrury:fix-assignment-cuddle-blocks
into ae57516 on bombsimon:master.

[bombsimon]

CI is not your code, it's related to Go 1.24 which I haven't built this project with yet

[jacobdrury]

Added your doc suggestion! Its ready to merge whenever!