Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Treat short declarations as declarations, not assignments (enhancement) #102

Closed
mholiday-nyt opened this issue Apr 9, 2021 · 11 comments
Closed
Labels
golangci-only Fixed in wsl but not merged to golangci-lint

Comments

@mholiday-nyt
Copy link
Contributor

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

@bombsimon
Copy link
Owner

Interesting! We had a quite long discussion in #82 about a similar question but that was to group := and var. We moved on to #83 and I ended up adding a flag to allow that. I didn't really consider this option.

I would be open to a PR for the first case, I encourage everyone who wants to use whitespaces! I would however not enforce this by default but as an opt in. The second one is violating the enforcing of cuddle error checks to where the error is defined, I'm not sure how they would work together. Maybe just have the new flag only allowing assignment grouping override any other flag.

What do you thing this kind of configuration would be referenced to and how would it work? ForceExclusiveAssignment maybe and then just having assignments with := having to be separated from everything except other assignments?

@mholiday-nyt
Copy link
Contributor Author

Yeah. One thing is that I find it clearer to talk about := as "short declarations" because they're not the same as assignments with =; they actually introduce a new name, which is my whole thought around separating them (even though mentally I still think of := as the Pascal assignment operator)

Yes, it would definitely be opt-in, and yes, it would force these things to be grouped apart

You can see my style in https://github.com/matt4biz/envy for example; I've found over the years that I have about 30% of the total lines as blank lines, even across different languages (well, at least since giving up punch cards ;-)

I particularly like separating declarations into bunches and having spaces around all block constructs; it's all about making the code easier to speed-read

@bombsimon
Copy link
Owner

Sounds reasonable to group it that way! I like your code style in general, we seem to have very similar idea.

Feel free to start working on a PR with an opt-in option to enforce shorthand declarations to be cuddled only with other shorthand declarations!

bombsimon pushed a commit that referenced this issue Apr 20, 2021
This option treats a short declaration with := as different than a plain
assignment with = and so prevents them cuddling together, as well as not
allowing a short declaration to cuddle with a block, for example.
@bombsimon bombsimon added the golangci-only Fixed in wsl but not merged to golangci-lint label Apr 20, 2021
@bombsimon
Copy link
Owner

I'll hold of drafting a new release until we decided on #104 as well so we can merge them to golangci-lint at the same time!

@bombsimon
Copy link
Owner

Both PRs merged and I created v3.3.0 so it should be easy to update golangci-lint. If you want me to do it I can do it tomorrow!

Thanks for your issues and PRs, really appreciated!

@matt4biz
Copy link

😎

@mholiday-nyt
Copy link
Contributor Author

Ahh, having looked at your last merge to golangci-lint, I see it's a bit more than just updating a version

LMK if you'd like me to help / review the PR

@bombsimon
Copy link
Owner

I'll create a PR later today and let you know when it's ready!

@bombsimon
Copy link
Owner

Filed golangci/golangci-lint#1922

@mholiday-nyt
Copy link
Contributor Author

PR looks good to me; thanks again, this has been really easy!

@bombsimon
Copy link
Owner

Merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
golangci-only Fixed in wsl but not merged to golangci-lint
Projects
None yet
Development

No branches or pull requests

3 participants