Background: gopls has a couple dozen Analyzers that flag features of Go code that could be improved by using newer features of the languages and library, and that suggest fixes to do so. We would like to make this functionality available in go fix (see #71859).

Proposal: we propose to move the x/tools/gopls/internal/modernize directory to x/tools/go/analysis/passes/modernize.

package modernize // import "golang.org/x/tools/go/passes/modernize"

// Suite lists all modernize analyzers that are safe to be enabled by default.
var Suite = []*analysis.Analyzer{
    AnyAnalyzer,
    // AppendClippedAnalyzer, // not nil-preserving!
    BLoopAnalyzer,
    FmtAppendfAnalyzer,
    ForVarAnalyzer,
    MapsLoopAnalyzer,
    MinMaxAnalyzer,
    OmitZeroAnalyzer,
    RangeIntAnalyzer,
    SlicesContainsAnalyzer,
    // SlicesDeleteAnalyzer, // not nil-preserving!
    SlicesSortAnalyzer,
    StringsCutPrefixAnalyzer,
    StringsSeqAnalyzer,
    TestingContextAnalyzer,
    WaitGroupAnalyzer,
}

var (
    AnyAnalyzer,
    AppendClippedAnalyzer,
    BLoopAnalyzer,
    FmtAppendfAnalyzer,
    ForVarAnalyzer,
    MapsLoopAnalyzer,
    MinMaxAnalyzer,
    OmitZeroAnalyzer,
    RangeIntAnalyzer,
    SlicesContainsAnalyzer,
    SlicesDeleteAnalyzer,
    SlicesSortAnalyzer,
    StringsCutPrefixAnalyzer,
    StringsSeqAnalyzer,
    TestingContextAnalyzer,
    WaitGroupAnalyzer
    *analysis.Analyzer
)

Comment From: gabyhelp

Related Issues

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

Comment From: cespare

At work we have an internal linter which is essentially just all the vet passes that we want to enable (including some that aren't go vet defaults) plus a handful of custom, internal passes. This is all glued together with multichecker.Main. Doing it this way makes it easier to have fine-tuned control over the vet checks and also is more efficient because it reduces duplicated work on the common analyses that need to happen for all these different passes.

Anyway, I'm happy to see this change because it will let us put some of the modernize checks into our linter too. We've been doing some modernization passes on an ad hoc basis but haven't yet figured out whether/how we want to lock in these changes into our automatically-enforced regime.

More generally: it's great when all Analyzer passes that are used by the tools you folks write are exported for use elsewhere.

Comment From: adonovan

Anyway, I'm happy to see this change because it will let us put some of the modernize checks into our linter too. We've been doing some modernization passes on an ad hoc basis but haven't yet figured out whether/how we want to lock in these changes into our automatically-enforced regime.

More generally: it's great when all Analyzer passes that are used by the tools you folks write are exported for use elsewhere.

Glad to hear it, this completely aligns with my thinking. There is a recent proposal (https://github.com/golang/go/issues/75227) to expose a global variable that defines "the list of analyzers actually used by go vet", which I think is another good step in that direction. We'll likely want an analogous one for "used by go fix" in light of https://github.com/golang/go/issues/71859.

Comment From: gopherbot

Change https://go.dev/cl/701775 mentions this issue: internal/moreiters: move from gopls/internal/util/moreiters

Comment From: gopherbot

Change https://go.dev/cl/701776 mentions this issue: internal/analysisinternal/generated: move from gopls/internal/analysis

Comment From: gopherbot

Change https://go.dev/cl/701777 mentions this issue: internal/astutil: move declarations from gopls/internal/util/astutil

Comment From: gopherbot

Change https://go.dev/cl/701778 mentions this issue: internal/analysisinternal: simplify DeleteStmt using Cursor

Comment From: gopherbot

Change https://go.dev/cl/701779 mentions this issue: gopls/internal/analysis/modernize: add test of no gopls imports

Comment From: aclements

This proposal has been added to the active column of the proposals project and will now be reviewed at the weekly proposal review meetings. — aclements for the proposal review group

Comment From: kwjw

@adonovan What do you envision for new modernizers that might arise from future changes to Go?

For example, I imagine that modernizers could be written for #45624 and #51945, should these proposals be accepted.

One of the useful parts of writing analyzers in gopls is the freedom to iterate on ideas and gather feedback from users without being locked into Go compatibility.

Will new modernizers go through the proposal process individually, before being implemented (now that the first big batch of modernizers is written and new ones will trickle in slowly as the need arises)? Or will they be written for gopls first and then "graduate" to x/tools/go (potentially in batches) via the proposal process?

Comment From: adonovan

What do you envision for new modernizers that might arise from future changes to Go? One of the useful parts of writing analyzers in gopls is the freedom to iterate on ideas and gather feedback from users without being locked into Go compatibility.

I agree. We'll continue to develop them in gopls, and later promote useful, debugged analyzers to the published modernizer package (and then to its Suite variable) as they mature, though of course that requires a--hopefully lightweight--proposal review process for each one or each batch.

Comment From: aclements

Based on the discussion above, this proposal seems like a likely accept. — aclements for the proposal review group

The proposal details are given in https://github.com/golang/go/issues/75266#issue-3385309735

Comment From: aclements

No change in consensus, so accepted. 🎉 This issue now tracks the work of implementing the proposal. — aclements for the proposal review group

The proposal details are given in https://github.com/golang/go/issues/75266#issue-3385309735

Comment From: gopherbot

Change https://go.dev/cl/706918 mentions this issue: go/analysis/passes/modernize: publish