x/tools/gopls/internal/analysis/modernize: add pass to use reflect.TypeAssert

[adonovan]

https://go.dev/cl/702735 replaced v.Interface().(T) with reflect.TypeAssert[T](v), where v is a reflect.Value, and demonstrated a significant performance improvement. Although the new code is 7 chars longer, it is more direct and arguably clearer. We should add a modernizer to automate this transformation when v.Interface().(T) appears in a two-value (x, ok := ...) context in a file using go1.25 or later. The doc comment for TypeAssert states that they are "semantically equivalent".

[gabyhelp]

Related Issues

• reflect: TypeAssert[T] is not identical to Interface().(T) #74404 (closed)
• reflect: add TypeAssert #62121 (closed)

Related Code Changes

• reflect: add example for TypeAssert

Related Documentation

• Package reflect > func TypeAssert

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

[mvdan]

FWIW the main point of the new API was performance, not clarity or brevity.

[linsite]

I think there are 4 patterns that should be considered depending on the context of v.Interface().(T):

1. Directly used as selector and without assert check

From

v.Interface().(T).method()

To

vi, _ := reflect.TypeAssert[T](v) // 'vi' should be generated carefully to avoid name conflict.
vi.method()

The same pattern applies to the following contexts:

v.Interface().(T).member = x

v.Interface().(T).member.foo.bar = x

2. Directly used as an argument and without assert check

From

f(v.Interface().(T))

to

vi, _ := reflect.TypeAssert[T](v)
f(vi)

The same pattern applies to the following contexts:

• • RHS of an assignment statement

x := v.Interface().(T) // the 'vi' is not needed any more

• • a member or property assignment in a var declaration of any type:

// struct
var v = Foo {
  m: v.Interface().(T)
}

v := Foo {
   m: v.Interface().(T)
}

v.m = v.Interface().(T)

// slice or array
v := []T{v.Interface().(T)}

v[i] = v.Interface().(T)

// map
v := map[string]T{"foo": v.Interface().(T)}

v["foo"] = v.Interface().(T)

3. Indirectly used and with assert check

vi, ok := v.Interface().(T)

to

vi, ok := reflect.TypeAssert[T](v)

4. Indirectly used and without assert check
   from

vi := v.Interface().(T)

to

vi, _ := reflect.TypeAssert[T](v)

Those are the patterns should be ignored.

1. v.Interface().(type) in a type-switch statement
2. Files using Go version earlier than 1.25

[adonovan]

Thanks for tackling the analysis! It is much easier to review transformations in this form than in code.

Regarding pattern 1: the transformation from v.Interface().(T).method() to x, _ := reflect.TypeAssert[T](v); x.method() may be a performance improvement (the stated motive for TypeAssert), but the replacement is about twice as long and breaks out of expression context. I wonder how often users would agree that the performance gain justifies the loss in brevity. Case 2a has the same issue.

Cases 2b (x := v.Interface().(T)) and 3 are already in statement form, so they are clear wins in both clarity and performance.

Case 4 is slightly questionable because it removes the implicit assertion of ok, delaying the detection of dynamic type errors, which could make latent bugs harder to find. (In general it's ok for a fix to assume that a program is bug-free, but the problem with source code transformations is that style is a fractal of nuance.)

I am inclined to implement only cases 2b and 3. What do you think?

[linsite]

Thanks for tackling the analysis! It is much easier to review transformations in this form than in code.

Regarding pattern 1: the transformation from v.Interface().(T).method() to x, _ := reflect.TypeAssert[T](v); x.method() may be a performance improvement (the stated motive for TypeAssert), but the replacement is about twice as long and breaks out of expression context. I wonder how often users would agree that the performance gain justifies the loss in brevity. Case 2a has the same issue.

Cases 2b (x := v.Interface().(T)) and 3 are already in statement form, so they are clear wins in both clarity and performance.

Case 4 is slightly questionable because it removes the implicit assertion of ok, delaying the detection of dynamic type errors, which could make latent bugs harder to find. (In general it's ok for a fix to assume that a program is bug-free, but the problem with source code transformations is that style is a fractal of nuance.)

I am inclined to implement only cases 2b and 3. What do you think?

Thanks for the review. Yes, I agree with keeping the transformation simple and bug-free.