[red-knot] Add diagnostic for invalid unpacking

[dhruvmanila]

Summary

Part of #13773

This PR adds diagnostics when there is a length mismatch during unpacking between the number of target expressions and the number of types for the unpack value expression.

There are 3 cases of diagnostics here where the first two occurs when there isn't a starred expression and the last one occurs when there's a starred expression:

1. Number of target expressions is less than the number of types that needs to be unpacked
2. Number of target expressions is greater then the number of types that needs to be unpacked
3. When there's a starred expression as one of the target expression and the number of target expressions is greater than the number of types

Examples for all each of the above cases:

# red-knot: Too many values to unpack (expected 2, got 3) [lint:invalid-assignment]
a, b = (1, 2, 3)

# red-knot: Not enough values to unpack (expected 2, got 1) [lint:invalid-assignment]
a, b = (1,)

# red-knot: Not enough values to unpack (expected 3 or more, got 2) [lint:invalid-assignment]
a, *b, c, d = (1, 2)

The (3) case is a bit special because it uses a distinct wording "expected n or more" instead of "expected n" because of the starred expression.

Location

The diagnostic location is the target expression that's being unpacked. For nested targets, the location will be the nested expression. For example:

(a, (b, c), d) = (1, (2, 3, 4), 5)
#   ^^^^^^
#   red-knot: Too many values to unpack (expected 2, got 3) [lint:invalid-assignment]

For future improvements, it would be useful to show the context for why this unpacking failed. For example, for why the expected number of targets is n, we can highlight the relevant elements for the value expression.

In the ecosystem, Pyright uses the target expressions for location while mypy uses the value expression for the location. For example:

if 1:
#          mypy: Too many values to unpack (2 expected, 3 provided)  [misc]
#          vvvvvvvvv
	a, b = (1, 2, 3)
#   ^^^^
#   Pyright: Expression with type "tuple[Literal[1], Literal[2], Literal[3]]" cannot be assigned to target tuple
#     Type "tuple[Literal[1], Literal[2], Literal[3]]" is incompatible with target tuple
#       Tuple size mismatch; expected 2 but received 3 [reportAssignmentType]
#   red-knot: Too many values to unpack (expected 2, got 3) [lint:invalid-assignment]

Test Plan

Update existing test cases TODO with the error directives.

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

[dhruvmanila]

Additional changes:

• Expanded the comments in this method
• Avoid &mut self as it's not required (not sure why clippy didn't catch that)
• Pass the expr which is required for the diagnostic location

[T-256]

Re-commenttng here,
Since "c" doesn't have enough values to unpack, I think LiteralString will never put at here.

[carljm]

I agree it would be better if this were just Unknown | Literal[3, 5].

[carljm]

I think to do this consistently would mean changing a bunch of other cases, though -- it would mean that e.g. for (a, b, c) = (1, 2) we would infer Unknown (or Never would also be reasonable) for all of a, b, c, rather than inferring Literal[1] for a and Literal[2] for b. That is, today we try to match up and infer as many types as we can, even if the overall unpacking is an error; this would mean changing that.

The advantage of the current approach is that it can catch more errors at once, assuming the parts that we are able to line up are actually lined up correctly. But that may not be a reasonable assumption; if you have a mis-alignment it could actually be early in the unpacking, in which case we are just inferring wrong types for many of the targets, potentially leading to false positives in subsequent code.

It looks like our current behavior is more like pyright; mypy always just infers Any for all targets in a mis-aligned unpacking.

I do think that the mypy behavior is more principled and we should probably switch to that, though I also don't think it's high priority to make that change.

[dhruvmanila]

Yes, I did that for the advantage that you've mentioned and I'm fine changing the behavior as it's an error case and should be less often in practice. I'll expand the original issue with this task.

[carljm]

Very nice!

[carljm]

I agree it would be better if this were just Unknown | Literal[3, 5].