Closed
Description
Found after running modernize (#70815) on std + cmd:
- don't apply
maps
transformations tomaps
package itself or its in-package tests (CL 653595). - don't import
slices
(or almost any other package) fromruntime
or its dependencies (CL 653595). - remove unused imports such as "sort" (split out as x/tools/go/analysis/internal/checker: -fix should remove unneeded imports #72035)
-
for i := 0; i < 1e3; i++
->for range 1e3
requires an int conversion or change of literal (CL 653616) -
for i := 0; i < n; i++
->for range n
is invalid if the loop body contains i++ (I thought we already checked this?). Example: path/filepath.scanChunk (Fixed by https://go.dev/cl/650815 ) -
var i int; for i = 0; i < n; i++
->for range n
removes uses of i that might make local var i unreferenced (example) (https://go.dev/cl/650815) - some comments go missing (e.g. in minmax) due to our old nemesis (see issue go/ast: Free-floating comments are single-biggest issue when manipulating the AST #20744).
See also:
Refactoring is hard. :( We may need to be selective about which categories we apply without review until all these bugs are fixed. Perhaps we also need a flag to select categories too.
Mistakes that break the build are of low severity, because the mistake is impossible to miss and the remedy is obvious. Loss of commentary is of medium severity as it may go undetected due to reviewer fatigue in a large CL. The really severe problems are latent behavior changes such as the "loop body contains i++" case above (now fixed).