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

Add checker for different usage of type in func arguments #31

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

a-urth
Copy link

@a-urth a-urth commented Feb 7, 2019

Add checker consistent usage of types in function declarations

func some(a int, b int, c int) {
    ...
}

vs

func some(a, b, c int) {
    ...
}

@a-urth
Copy link
Author

a-urth commented Feb 7, 2019

If @quasilyte is ok with this addition I'll update readme as well.

@cristaloleg
Copy link
Contributor

@quasilyte
Copy link
Owner

@cristaloleg maybe @a-urth wants to have an opportunity to change combined types into separate definitions. Some programmers dislike a, b, c T notation.

@a-urth, is that so?

I need some time to think about it and ask others about their opinion.
I'll post back soon.

@a-urth
Copy link
Author

a-urth commented Feb 7, 2019

Yes, but it suggest one way of doing it while go-consistent check for one already used most in the system.

@a-urth
Copy link
Author

a-urth commented Feb 7, 2019

@quasilyte exactly. We have this discussion in our project and want to stick to the way which is already mostly used.

Me personally on the side of combining types, which I found to be idiomatic in go, but i can relate to why type should be used for each parameter. Its kinda consistent :D

@quasilyte
Copy link
Owner

The argument I've got from several sources:

Sometimes f(x, y T) is more readable, especially in those small signatures, but for all other cases more verbose form is preferred.

The bad part is that this preference allows both forms to co-exist in one codebase.
But if we have a way to disable checks, it's probably not a big deal.

@quasilyte
Copy link
Owner

I still think that implementing this as a check for lintpack is more useful as it can have more options.
As an example of such standalone check or a bundle of checks there is https://github.com/quasilyte/go-police, it can be built with https://github.com/go-lintpack/lintpack in the same way as https://github.com/go-critic/go-critic, or you can build a linter that includes both gocritic and your personal checks.

I can help you porting that check, if you have any questions, there is https://t.me/go_critic_eng and https://t.me/go_critic_ru (depending on the language you prefer).

The problem with go-consistent is that it works sub-optimal when both alternative may be desired to live inside a single project. I don't know how to approach that yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants