Description
(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 useTypeReg
instead. - The comments for
archive/zip.FileHeader.{Compressed,Uncompressed}Size
say to use the corresponding*64
fields instead. - The comments for
image.ZR
andimage.ZP
say to use zero struct literals instead. - The comment for
go/importer.For
says to useForCompiler
, and is implemented as a trivial call toForCompiler
with an additional argument. - The comment for the
io.SEEK_*
constants says to use correspondingio.Seek*
constants instead. - The comments for
syscall.StringByte{Slice,Ptr}
say to use the correspondingByte*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 thatgo fix
should replace occurrences of that constant or variable with the expressionEXPR
. Free variables in the expression are interpreted as package names, andgo fix
may rewrite them to avoid collisions with non-packages.//go:fix-to .EXPR
on struct field indicates thatgo 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 thatgo 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 aconst
orvar
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 thatgo 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 onereturn
statement and nodefer
statements.//go:forward
on a type alias indicates thatgo 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
Type
Projects
Status