Check for duplication in union type annotations

[mbyrnepr2]

Current problem

Currently we emit nothing about the following pattern, which can be simplified:

ANSWER: int | int = 42

Desired solution

Emit a warning when this pattern is detected. A union typehint containing the same type multiple times can be simplified. Potential name: typehint-duplicate-union-type.

Additional context

This is the same issue as reported in python/mypy#13448 although it doesn't look like it will be implemented by the type-checker.

[Pierre-Sassoulas]

+1 for me as the mypy team explicitly said linter should do it.

[orSolocate]

Could possibly work on it but need some more info:

Should it raise a message for any redundant type? Are there types where redundant union type is a fine to do?

Should this be implemented only for assignments for starters?

Does astroid recognizes a union pattern in its annotation node attribute?

[mbyrnepr2]

Should it raise a message for any redundant type? Are there types where redundant union type is a fine to do?

I think emitting a message any place where there is a redundant type in a union is the aim. I don't think there is ever a situation where redundant types are considered best practice.

Should this be implemented only for assignments for starters?

I don't think we should limit ourselves; but open to thoughts on splitting it into separate merge requests, for example.

Does astroid recognizes a union pattern in its annotation node attribute?

Yes, If I have understand your question. For the simple example in the description:

(Pdb) node
<AssignName.ANSWER l.1 at 0x10bc6aaa0>
(Pdb) node.parent
<AnnAssign l.1 at 0x10bc6a980>
(Pdb) node.parent.annotation
<BinOp l.1 at 0x10bc6aa40>
(Pdb) node.parent.annotation.left
<Name.int l.1 at 0x10bc6a860>
(Pdb) node.parent.annotation.right
<Name.int l.1 at 0x10bc6a740>
(Pdb) 

Thanks for looking into it!

[orSolocate]

I think emitting a message any place where there is a redundant type in a union is the aim. I don't think there is ever a situation where redundant types are considered best practice.

I think you are right, found this is Python docs:

Redundant arguments are skipped, e.g.:

Union[int, str, int] == Union[int, str] == int | str

Which means it is only a readability issue since the compiler omits the redundant type anyway. and makes sense to make it a [Refactor] message.

I would make the checker catch both the | case and the Union[..] case.

Another corner case is multiple multiple ( :D) typehints. I will explain where it can be a problem:

Consider this intuitive algorithm:

For every assignment with type hints:

      for every typehint:
      
           raise message if *any other* type hint is of the same type
           

run this algorithm on:

ANSWER: int | int | int = 42

and you get 2 messages (instead of 1?). one for the first int being same type and second int, and the other message for the second int being the same type as third int.

should I keep the types that a message was raised for them and skip'em if encounter them again?

[mbyrnepr2]

Sounds good @orSolocate! For point 3. I think we only need to emit one message for that situation. Perhaps we can obtain a list of the types in the union & check for duplication?

[orSolocate]

@Pierre-Sassoulas - you can assign me for this issue.

@mbyrnepr2 - the constructor could add the type of every duplicated (a message was raised for) type, and then skip those type if encounters them again. (for the same AnnAssign node I mean..