Skip to content

x/tools/gopls/internal/analysis: add "modernizer" analyzers #70815

Closed
1 of 2 issues completed
Closed
@adonovan

Description

@adonovan

[Note: please create new issues for bugs and feature requests rather than expanding the scope of this one.]

Between generics, iterators, min and max, new standard packages such as slices, maps, and cmp, there are a large number of new ways to write Go code more concisely and no less clearly. We should develop "modernizer" analyzer(s) that identify such opportunities and offer suggested fixes.

Many of these are already implemented in staticcheck (see S1000 et seq); we would just need to enable them.

When the appropriate fix is to inline a call to a deprecated function whose body illustrates the recommended approach (e.g. a call to a more modern function, as in the case of the functions in the golang.org/x/exp/maps package), these will be handled by annotating the deprecated function as prescribed by proposal #32816, and letting the inliner take care of it.

Examples:

  • b.N → b.Loop in Benchmarks (https://go.dev/cl/638337)
  • interface{} → any (CL 636276)
  • explicit loops → slices.{Equal,Compare,Index,Contains}{,Func}. The challenge is treating the return false and return true steps in the slices package as wildcards that match "equal and opposite" actions--not just return statements--in the user code. (CL 640576)
  • explicit loops → maps.{Clone,Collect.Insert,Copy} (https://go.dev/cl/638875)
  • maps.Keys(m) + slices.Collect (or loop equivalent of Keys+Collect) + sort -> slices.Sorted(maps.Keys(m)). Eliminate temporaries when used once in range _. Downside: de-optimizes preallocation of correct array size; perhaps best left until resolution of proposal: maps: helper to loop over the maps in predictable order #68598.
  • various append + s[::] combinations -> slices.Grow, Clip, Concat & Clone (https://go.dev/cl/637735), etc.
  • append(s[:i], s[i+k:]...) -> slices.Delete(s, i, i+k), where k is a non-negative constant
  • exp/{slices,maps} -> std {slices,maps} (see CL 635680 + issue cmd/fix: automate migrations for simple deprecations #32816)
  • exp/maps.Clear → clear etc (https://go.dev/cl/635680)
  • if/else → min/max (CL 635777)
  • awkward logic in slice.Sort's less function → if cmp := cmp.Compare(x, y); cmp != 0 { return cmp < 0 }
  • sort.Slice -> slices.Sort, avoiding the less func when it is the natural order (https://go.dev/cl/635681)
  • sort.Slice -> slices.SortFunc, eliminating the indices
  • json:omitempty → json:omitzero for basic types
  • SetFinalizer → AddCleanup
  • use strings.Lines instead of strings.Split("\n") (but: "\r"?!) or text.scanner(strings.NewReader)/Scan/Text
  • []byte(fmt.Sprintf...)fmt.Appendf(nil, ...) (https://go.dev/cl/638436)
  • implementing {Text,Binary}Appender for types that currently implement {Text,Binary}Marshaler
  • replacing ctx, cancel := context.WithCancel(context.Background()); defer cancel() in tests with ctx := t.Context().
  • use cipher.NewGCMWithRandomNonce where possible (example)
  • reflect.DeepEqual(x, y []comparable) -> slices.Equal(x, y) (CL 649457. Not sound! Changes nil behavior.)
  • for range strings.Split -> for range strings.SplitSeq (CL 647438)
  • ditto for strings.Fields{,Seq} (split out to separate issue x/tools/gopls/internal/analysis/modernizer: replace "range strings.Fields" with FieldsSeq #72033)
  • for i := 0; i < n; i++ {} => for i := range n {} (CL 646916)

They should of course all respect the effective version of Go determined by go.mod and build tags.

@aclements

Sub-issues

Metadata

Metadata

Labels

AnalysisIssues related to static analysis (vet, x/tools/go/analysis)FeatureRequestIssues asking for a new feature that does not need a proposal.RefactoringIssues related to refactoring toolsToolsThis label describes issues relating to any tools in the x/tools repository.goplsIssues related to the Go language server, gopls.gopls/analysisIssues related to running analysis in gopls

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions