confirmed that invalid type to not be reported assignable to the error interface in go 1.14
cc @griesemer

In the type checker, to avoid follow-on errors caused by earlier errors in the source, we usually accept an invalid operand or type as "valid", without reporting an error.

What happens here is that an invalid type always implements any interface without error, hence invalid is assignable to error. What we don't do here (and probably we should, for consistency) is to permit assigning an invalid operand/type to some other (non-interface) type. This is a trivial fix and would lead to consistent behavior, but it's not clear to me if it solves the gopls issue.

That said, we probably don't want to change the behavior of invalid operands/types in the type checker dep. on whether AssignableTo was called externally or internally.

@muirdm It should be easy to use a wrapper around AssignableTo (and whatever other API entry points that need it) which checks that the arguments are valid types, and if not, return the correct negative result.

Internally, we should make the type checker behavior consistent and accept invalid in both cases.

@findleyr Other thoughts?

It should be easy to use a wrapper around AssignableTo

Certainly yes. I wanted to understand the desired behavior of AssignabeTo before updating gopls.

an invalid type always implements any interface without error

This seems to be new behavior in Go 1.18, at least wrt AssignableTo. Ignoring the internal go/types use case of minimizing errors, why should an invalid type implement all interfaces? Intuitively, why is it more correct/useful for go/types to say that an invalid type can be assigned to/from anything?

The implementation of "implements" was completely rewritten for 1.18 as it needed to accommodate the new interface definition. So I'm not surprised the behavior changed slightly. Changing it back is trivial but it does lead to an unnecessary error in at least one of our (admittedly esoteric) test cases.

Usually, an invalid type is the result of an error in the package that's being type checked. That type may be stored/retained as the type of the erroneous variable or expression. It's standard practice throughout the type checker to ignore such invalid types. If we don't, a local error may lead to additional errors elsewhere where they are not helpful. We are not 100% consistent with this, which is ok because the primary impact (ignoring API calls for the moment) is that the type checker and thus compiler may report more errors than needed (only if there are errors in the first place).

Thus, being able to ignore invalid types or operands (which basically means, treating them as "correct") is a very useful feature and significantly reduces the number of follow-on errors in the presence of errors.

We could catch invalid types in the exported AssignableTo (rather than requiring clients to use a wrapper) but it would only work at the top level: in general an invalid type may appear deeply nested in some enclosing composite type, and we probably don't want to walk each type first.

In general, it's probably best for a client to not rely on a specific behavior for undefined types, but to exclude them as needed.

I don't have a strong opinion, but agree that we should be consistent in both cases. We don't have a coherent theory of Typ[Invalid], so it's hard to make a principled argument either way. I'd lean toward preserving the 1.17 behavior (and backporting the fix).

Two additional notes:

• We have explicit handling for Typ[Invalid] in AssertableTo
• The behavior of ConvertibleTo also changed in 1.18: https://go.dev/play/p/bBgW7xRuiMP (compare with selecting 1.17 in the backend dropdown)

Either way, it's probably a good idea to add explicit handling in gopls.

@findleyr You make an excellent observation with AssertableTo: we explicitly check for invalid types: since this effectively says that an interface cannot have an invalid type as a dynamic type, we should also not let an invalid type implement an interface (at least with respect to how these exported functions behave).

So let's maintain 1.17 behavior. (see below)

Change https://go.dev/cl/415334 mentions this issue: types2: exported predicates to return false for invalid type arguments

Per further discussion, maintaining 1.17 behavior is inconsistent as well. Decided to document that behavior is undefined for invalid types for now. We can consider applying https://go.dev/cl/415334 for 1.20.

Change https://go.dev/cl/415574 mentions this issue: go/types, types2: document that exported predicates are undefined for invalid type arguments

Change https://go.dev/cl/415595 mentions this issue: lsp/completion: fix ranking of *types.PkgName candidates

Some tools depend on current behavior. We need to have a good answer for tools first (they may want to use their own wrappers with check for invalid types and forward to go/types otherwise). Moving to 1.21.

This issue is currently labeled as early-in-cycle for Go 1.21.
That time is now, so a friendly reminder to look at it again.

The comment here still applies. Moving to 1.22.

Moving to 1.23.

Too late for 1.23. Moving to 1.24.

@findleyr Anything we can do here?

Unfortunately, no I don't think this is something we can change. Now that we have documented this and worked around the limination, we should close and move on.