Build flags and other options

[mvdan]

Quite a number of linters accept flags that change their behavior. The most common case is whether or not to load test files via go/loader.

Others include build tags, or what Go versions to check against (e.g. for new std stuff).

With the current API, none of those are possible. Some ideas:

• Should a linter somehow report its options via the interface, having ways to use them (e.g. funcs)?
• Instead of the above, should linters only be allowed to receive options?

[mvdan]

Or perhaps we could even make this package aware of all the possible options. The question then is - could a linter have an option that is specific to it, and not applicable to most linters?

I don't know the answer to that question, but I think assuming that there aren't could limit some linters.

[mvdan]

Actually, whether or not tests should be included happens earlier, in the go/loader.Config.FromArgs call. As long as linters work fine with and without the tests included, they should not have to know about whether or not they were actually included.

[thomasheller]

Should a linter somehow report its options via the interface, having ways to use them (e.g. funcs)?

I'm not convinced yet why metalint would need to know about the options each linter accepts. There are multiple ways (and combinations thereof) how options could work:

1. Have metalint accept a few generic options (like include tests, Go version etc.). Pass those options to every linter through Checker. It's up to the linter whether they change their behavior depending on those options. There's no pain in making some options available to a linter that doesn't need them.
2. Have metalint accept arbitrary options for specific linters (option to pass option X with value Y to linter Z). Pass the option and value through Checker to the designated linter. Linters would receive an interface that works similar to the flag package, so they can query if a specific option was set without metalint caring about details.
3. Have metalint accept specific options (some for multiple linters, some for specific linters). Actually do the work of "translating" the options - this is how gometalinter does it. It knows which options can be applied to which linter, and passes the values of specific options to specific linters. In this case, we would need to keep track of supported options.

could a linter have an option that is specific to it, and not applicable to most linters?

This sure can happen. gocyclo has -over N for example, lll has -l MAXLENGTH.

On the other hand, -tests is probably the most universal option, that seems so apply to every linter.

As long as linters work fine with and without the tests included, they should not have to know about whether or not they were actually included.

Agreed.

[mvdan]

Have metalint accept a few generic options

This was my initial idea, as I'm a big fan of linters that "just work".

Have metalint accept arbitrary options for specific linters

Thought of this one too, but IMO it's too chaotic. For example, what if two linters separately accept the same flag name, meaning different things?

Have metalint accept specific options (some for multiple linters, some for specific linters)

I think this is a valid option just like the first one.

It all depends on whether we want the interface and metalint to be as simple as possible, at the cost of losing configurability. I'm not sure right now, but I'm leaning towards allowing specific options through the Checker constructors. For example, what we have now in metalint is:

var linters = [...]struct {
        name    string
        checker lint.Checker
}{
        {"unparam", &unparam.Checker{}},
        {"interfacer", &interfacer.Checker{}},
}

It could just as well be, with no changes to the interface:

var treshold = flag.Int(...)
[...]
        {"interfacer", &interfacer.Checker{Treshold: *treshold}},

[mvdan]

Of course, each Checker implementation could also hide the actual struct type and just export a constructor func, like NewChecker(...) lint.Checker.

And we should probably encourage something like Rob Pike's and Dave Cheney's option funcs: https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis

[thomasheller]

For example, what if two linters separately accept the same flag name, meaning different things?

That wouldn't happen, because it would go something like:

$ metalint -pass linterA:samename:value1 -pass linterB:samename:value2 ...

Still, this should probably be simpler.

[mvdan]

Ouch, that's verbose. Even with that, it would still be somewhat confusing.

I think we should let the caller decide what to do when calling the constructors with the options. They can follow each linter's flag names and use a prefix system like you suggested too, but I'd likely not do that for the metalint command.

[mvdan]

Now that go/packages and go/analysis are a thing, this repo lost its purpose.