x/tools/gopls/internal/settings: enable more staticcheck passes

[adonovan]

This program triggers no diagnostic from staticcheck's SA4003 analyzer:

func f(x uint8) {
	if x < 0 {
		println(0)
	}
	if x >= 0 {
		println(0)
	}
}

I would have expected it to be on by default and to report that both comparisons are redundant. Why does this not happen?

[adonovan]

Ok, well the answer to that question is easy: the default value of the staticcheck configuration is false.

Next question: why is it false? Let's enable the subset of its analyzers that produce reliable signals that your code has a problem (or, if the code is not necessarily wrong, as with modernizers, let's report the diagnostic at "hint" severity). What is that subset?

[dmitshur]

What is that subset?

As noted at https://staticcheck.dev/docs/configuration/, "Staticcheck tries to provide a good out-of-the-box experience, [...]", that is, this question is already being considered upstream. So it might work well to reuse the decisions made (and maintained) there, or at least use that as a starting point.

Edit: It looks like one of the places the default check list is visible is https://staticcheck.dev/docs/configuration/options/, under the default value for "checks".

CC @dominikh.

[adonovan]

So it might work well to reuse the decisions made (and maintained) there, or at least use that as a starting point.

I feel sure that we have discussed this question before but I can't find the thread. I remember now that at least one reason, perhaps the major one, not to enable staticcheck by default was that the buildir (SSA) pass uses facts, meaning that it is applied to all transitive dependencies, making it computationally quite expensive. We did some work in the x/tools/go/ssa to eliminate most of the costs involved, but last I heard they were not yet ported to buildir.

So I think the buildir-free subset of staticcheck may be a reasonable starting point.

[findleyr]

My recollection is that we would enable the buildir-free subset of staticcheck by default. I'm not sure if this is recorded anywhere; thanks for filing this issue.

[adonovan]

This is fixed, as #71038, which is really a dup of this issue.