lint: move every checker in it's own dir and own package about go-critic HOT 9 CLOSED

It's not very frequent to split packages like this in Go.

For example:

• there is no expr, stmt and decl packages, only ast which includes all of them.
• compiler optimizations are all part of ssa package, it defines multiple optimization passes just like we define multiple checkers inside one package.

There are no circular imports in Go, you can't import A from B if B imports A.
This means that code re-use can get complicated and require much architectural thinking to overcome it.

For example, lint.Warning required by all checkers, so each checker package will import lint.
But we also need lint.Warning in user code (one of them is cmd/kfulint, there can be more later).
So, we will get circular dependency.

Checker-related functionality is not "global", it only occupies checker method set.
All unexported functions that are global (util.go) are global by design,
to be re-used between different checkers.

Also note that long package names in Go are also not idiomatic.
They're usually 3-8 chars long, and consist of 1 word (with exception of foo_test idioms).

In C++ you would not move each checker to a separate namespace either, right?

Anyway, I'm voting for long consideration until this is done.

I agree that lint.NewXYZ looks weird in linter main, but that can change in near future,
we don't figured out good alternative yet, and package.New does not solve the problem, it only changes the names.

from go-critic.

When we spoken earlier about linter-specific directory split, I though about test files from testdata.

from go-critic.

Oh, circular imports are pain.. Doesn't go have patterns to resolve them, like predefined structures?
In c++ i would have been put every "private function" in cpp file to hide them. Not now, but later huge amount of "visible" functions in packet can be a problem.

from go-critic.

Which functions do you refer to?

from go-critic.

In checker A I see all exported/unexported functions from checker B.

from go-critic.

But you shouldn't ever have an instance of other checker inside other checker in a first place.
Such code will never pass the review.

And if your autocomplete somehow gives you B methods for A type, that's editor problem, not architectural.

from go-critic.

Every abstraction and incapsulation is a convention for a degree.
In C++ you can include cpp files, and you can do #define private public; you can also do many things with unsafe tricks and assembler that break incapsulation (can "private" save from invalid pointer cast or arbitrary memory read?).
This doesn't automatically mean that all C++ code is bad, right? (Well, this is arguable...)

from go-critic.

Oh, my bad. I hadn't thought about it.
I rly forgot that every function in cheker refers to it's own type.
Sry for waisting your time.

from go-critic.

@fexolm, that's not a time wasting, it's a discussion. :)
You're welcome.

from go-critic.