Skip to content

cmd/fix: automate migrations for simple deprecations #32816

Closed
@bcmills

Description

@bcmills

(I've mentioned this in passing on #32014 and #27248, but I don't see an actual proposal filed yet — so here it is!)

Over time, the author of a package may notice certain patterns in the usage of their API that they want to make simpler or clearer for users. When that happens, they may suggest that users update their own code to use the new API.

In many cases, the process of updating is nearly trivial, so it would be nice if some tool in the standard distribution could apply those trivial updates mechanically. The go fix subcommand was used for similar updates to the language itself, so perhaps it could be adapted to work for at least the simple cases.

The very simplest sort of update rewrites a call to some function or use of some variable or constant to instead call some other function or use some other variable, constant, or expression.

To draw some examples from the standard library:

  • The comment for archive/tar.TypeRegA says to use TypeReg instead.
  • The comments for archive/zip.FileHeader.{Compressed,Uncompressed}Size say to use the corresponding *64 fields instead.
  • The comments for image.ZR and image.ZP say to use zero struct literals instead.
  • The comment for go/importer.For says to use ForCompiler, and is implemented as a trivial call to ForCompiler with an additional argument.
  • The comment for the io.SEEK_* constants says to use corresponding io.Seek* constants instead.
  • The comments for syscall.StringByte{Slice,Ptr} say to use the corresponding Byte*FromString functions, and are implemented as short wrappers around those functions.

Outside the standard library, this pattern can also occur when a package makes a breaking change: if the v3 API can express all of the same capabilities as v2, then v2 can be implemented as a wrapper around v3 — and references to the v2 identifiers can be replaced with direct references to the underlying v3 identifiers.


See the current proposal details in #32816 (comment) below.

Previous draft (prior to #32816 (comment)):

I propose that we establish a consistent convention to mark these kinds of mechanical replacements, and improve cmd/fix to be able to apply them.

I'm not particularly tied to any given convention, but in the spirit of having something concrete to discuss, I propose two tokens, //go:fix-to and //go:forward, as follows:

  • //go:fix-to EXPR on a constant or variable indicates that go fix should replace occurrences of that constant or variable with the expression EXPR. Free variables in the expression are interpreted as package names, and go fix may rewrite them to avoid collisions with non-packages.
  • //go:fix-to .EXPR on struct field indicates that go fix should replace references to that field with the given selector expression (which may be a field access OR a method call) on the same value. Free variables are again interpreted as package names. If the expression is a method call, it replaces only reads of the field.
  • //go:forward on an individual constant or variable indicates that go fix should replace the constant or variable with the expression from which it is initialized. If a constant is declared with a type but initialized from an untyped expression, the replacement is wrapped in a conversion to that type.
  • //go:forward on a const or var block indicates that every identifier within the block should be replaced by the expression from which it is initialized.
  • //go:forward on a method or function indicates that go fix should inline its body at the call site, renaming any local variables and labels as needed to avoid collisions. The function or method must have at most one return statement and no defer statements.
  • //go:forward on a type alias indicates that go fix should replace references to the alias name with the (immediate) type it aliases.

All of these rewrites must be acyclic: the result of a (transitively applied) rewrite must not refer to the thing being rewritten.

Note that there is a prototype example-based refactoring tool for Go in golang.org/x/tools/cmd/eg that supported a broader set of refactoring rules. It might be useful as a starting point.


For the above examples, the resulting declarations might look like:

package image

// Deprecated: Use a literal image.Point{} instead.
var ZP = Point{} //go:forward

// Deprecated: Use a literal image.Rectangle{} instead.
var ZR = Rectangle{} //go:forward
package importer

// Deprecated: Use ForCompiler, which populates a FileSet
// with the positions of objects created by the importer.
//
//go:forward
func For(compiler string, lookup Lookup) types.Importer {
	return ForCompiler(token.NewFileSet(), compiler, lookup)
}
package io

// Deprecated: Use io.SeekStart, io.SeekCurrent, and io.SeekEnd.
const (
	SEEK_SET int = 0 //go:fix-to io.SeekStart
	SEEK_CUR int = 1 //go:fix-to io.SeekCur
	SEEK_END int = 2 //go:fix-to io.SeekEnd
)
package syscall

// Deprecated: Use ByteSliceFromString instead.
//
//go:forward
func StringByteSlice(s string) []byte {
	a, err := ByteSliceFromString(s)
	if err != nil {
		panic("string with NUL passed to syscall.ByteSliceFromString")
	}
	return a
}

[…]

This proposal does not address the archive/tar use-case: callers producing archives might be able to transparently migrate from TypeRegA to TypeReg, but it isn't obvious that callers consuming archives can safely do so.

Nor does this proposal does not address the archive/zip.FileHeader use-case: the deprecation there indicates a loss of precision, and the caller should probably be prompted to make related fixes in the surrounding code.

Both of those cases might be addressable with a deeper example-based refactoring tool, but (I claim) are too subtle to handle directly in go fix.


CC @marwan-at-work @thepudds @jayconrod @ianthehat @myitcv @cep21

Metadata

Metadata

Assignees

Labels

FeatureRequestIssues asking for a new feature that does not need a proposal.NeedsFixThe path to resolution is known, but the work has not been done.ProposalProposal-Accepted

Type

No type

Projects

Status

Accepted

Relationships

None yet

Development

No branches or pull requests

Issue actions