-
Notifications
You must be signed in to change notification settings - Fork 17.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
x/tools/gopls: completion gives unneeded generic type instantiation snippet #51783
Comments
I guess it is more question of "will type inference work for this function call", and if "yes" then don't give the type param snippet. If go/types can't tell us that, then I feel like some simple heuristics can cover common cases. For example, if all the type params are referenced in the param list, then type inference will work (is that correct?). |
@findleyr Can we hook in to type param inference to determine a priori if a generic function's type params will/might be inferable after the normal params are filled in? I know sometimes it depends on the arguments, but my feeling is that type param inference works most of the time if it can work at all for a given function. In a similar vein, the heuristic I suggested above would not work in all cases, but I think it would be an improvement (i.e. if type inference will "probably" work, don't give the type param snippet). |
The completion of generic functions is currently very poor. In most cases, type parameters are unnecessary. If you use completion now, you must also delete the type parameters after completion. The standard library already has the "slices" and "maps" packages, so providing a completion experience for generic functions would be very helpful. If it is difficult to determine whether type parameters should be completed, it is also acceptable to remove the type parameters in complete (it is only needed in rare cases). |
@muirdm suggested here essentially the same as I suggested in #64480 (comment), apologies that this slipped through the cracks during generics work. go/types does not provide precisely what we need, but as I commented on the other issue, I think a simple heuristic is probably good enough. We should fix this soon. |
Consider slices.Clone, which is representative of a large number of library functions that are cautiously written for the reasons given in https://go.dev/blog/deconstructing-type-parameters. Its type is:
The heuristic proposed above suggests that we can omit the suffix of type parameters that are free (referenced by) the parameter types, but in this case, E is not free, to ensure that all of these are valid: https://go.dev/play/p/HM9slI86sI5 type strings []string
type stringy string
type stringys []stringy
{
var x []string
slices.Clone(x)
slices.Clone[[]string](x)
slices.Clone[[]string, string](x)
slices.Clone[strings](x) // non-trivial
slices.Clone[strings, string](x) // non-trivial
}
{
var x strings
slices.Clone(x)
slices.Clone[[]string](x) // non-trivial
slices.Clone[[]string, string](x) // non-trivial
slices.Clone[strings](x)
slices.Clone[strings, string](x)
}
{
var x stringys
slices.Clone(x)
slices.Clone[[]stringy](x) // non-trivial
slices.Clone[[]stringy, stringy](x) // non-trivial
slices.Clone[stringys, stringy](x)
slices.Clone[stringys, stringy](x)
} (The instantiations that would not be inferred are labeled "non-trivial".) So I think this heuristic will not work for such cautiously-written library functions. |
E can infer from S, so the E also is free.
I don’t think we need to consider whether the type parameter can be omitted in all scenarios, the type parameter should always be specified in rare cases. This problem doesn't seem to be solved quickly. I thought of a simple solution to alleviate this problem, we can change the tabstop range. slices.Clone[${1:S ~[]E}, ${2:E any}](${3:s S}) change to slices.Clone${1:[${2:S ~[]E}, ${3:E any}]}(${4:s S}) Then we can simply ignore the type parameter if we don't need it |
Change https://go.dev/cl/554855 mentions this issue: |
Completing to "foo" at <> inserts "int(foo[T float64|int](a T))" (ignore the extra type conversion), but most likely the type argument can be inferred so gopls should offer just "foo(a int)" instead.
The text was updated successfully, but these errors were encountered: