x/tools/gopls/internal/analysis/modernize: use new reflect iterator APIs

[aclements]

#66631 adds iterator forms of several existing reflection APIs. We should add a modernizer to rewrite the non-iterator forms to the iterator forms when used in a simple loop.

Example rewrite:

var v Value
for i := range v.NumFields() {
	f := v.Field(i)
	// ... use f ...

	// Optional:
	sf := v.Type().Field(i)
	// ... use sf ...
}

var v Value
for sf, f := range v.Fields() {
	// ... use f and sf ...
}

[gabyhelp]

Related Issues

• x/tools/gopls/internal/analysis/modernizer: replace "range strings.Fields" with FieldsSeq #72033 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[adonovan]

Ideally this modernizer would be generalized so that it works for a whole table of data types with Len() + At(i) style APIs that have since been augmented with iterators, e.g. all these in go/types:

• Interface.Methods
• Interface.ExplicitMethods
• Interface.EmbeddedTypes
• Named.Methods
• Scope.Children
• Struct.Fields
• Tuple.Variables
• MethodSet.Methods
• Union.Terms
• TypeParamList.TypeParams
• TypeList.Types

[aclements]

That's a great idea. Some of the new reflect APIs may be a little trickier because we chose to make them return a Seq2 with a useful key. Using just the value from the Seq2 fits into that pattern and would be a fine first step, but they'll need a little extra work to make use of the key.

[adonovan]

One question that came up during the implementation is how we avoid deoptimizing code that has intentionally avoided using iterators because of their run-time overheads. Do we need a suppression mechanism?

[gopherbot]

Change https://go.dev/cl/708775 mentions this issue: go/analysis/passes/modernize: moderniser for iterator APIs

[aclements]

I think I would like to see examples of such code before we go down the path of a suppression mechanism. Ideally no one should have to choose between performance and clarity, so I would like to think hard about optimizations if there is a performance penalty for this.