What is the URL of the page with the issue?
https://pkg.go.dev/golang.org/x/tools/internal/gofix
What is your user agent?
Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/18.5 Safari/605.1.15
Screenshot
No response
What did you do?
(Reporting based on request by @aclements)
I read the descriptions under the Functions and Constants sections and find them confusing; it is conflating inlining and deprecation/migration.
The description for the second example: Inlining can be used to move off of a deprecated function:
Similarly, the third example: It can also be used to move off of an obsolete package, as when the import path has changed or a higher major version is available:
Replacing a call pkg.F() by pkg2.F(nil) can have no effect on the program, so this mechanism provides a low-risk way to update large numbers of calls. We recommend, where possible, expressing the old API in terms of the new one to enable automatic migration.
Under Constants:
this analyzer will recommend that uses of Ptr should be replaced with Pointer.
As with functions, inlining can be used to replace deprecated constants and constants in obsolete packages.
What did you see happen?
Main issue described above. But I can add a few nitpicks as well:
- The latest version is not formatting Functions and Constants as headings, but as ## Functions
and ## Constants
- As I commented on #73605, the explanation below
You can use this (officially unsupported) command to apply gofix fixes en masse:
$ go run golang.org/x/tools/internal/gofix/cmd/gofix@latest -test ./...
I believe I previously used this tool to fix (modernize) my code base in bulk. But the -test
flag does not match the tool's own documentation to mean apply gofix en masse. The doc says:
-fix
apply all suggested fixes
-test
indicates whether test files should be analyzed, too (default true)
But I'm now not sure if this is the actual tool to use for such fixes.
What did you expect to see?
A clearer description of the gofix analyzer and the associated //go:fix inline
tag.
Comment From: gabyhelp
Related Issues
- x/tools/internal/refactor/inline: analyzer generates suggested fixes that move comments incorrectly #67336 (closed)
- cmd/fix: automate migrations for simple deprecations #32816 (closed)
(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)
Comment From: jba
The smaller points we'll address.
I don't understand your larger point about conflation. You quote the passages that you find confusing verbatim but don't explain why they confuse you. I don't see this as conflation; it is describing how to implement one thing in terms of another. There is no conflation between hammers and fastening in "a hammer can be used to fasten wood together." If you have clearer prose, please suggest it.
Comment From: meling
I find it confusing that an inlining label is promoted as a way to deprecate and migrate to new APIs, especially since the documentation doesn't mention inlining for performance, which is what I mainly associate with inlining.
But my own confusion aside. Let me propose the following prose to provide some initial context for the reader:
Package gofix defines an Analyzer that inlines calls to functions and uses of constants marked with an inline directive.
Analyzer gofix ¶ ~gofix: apply fixes based on go:fix comment directives~ (maybe move elsewhere?)
The gofix analyzer replaces calls to functions and uses of constants with their definitions when annotated with the "//go:fix inline" directive. Inlining a (small) function may improve performance at the expense of a larger binary. Inlining can also be used to deprecate a function or constant by specifying an appropriate replacement, without breaking existing code.
The above would at least give the reader some context that the hammer can also be used for deprecating/migrating to a new API.