Repository: python/mypy
Pull request: Check duplicate bases when defining TypedDict
### Description

Closes https://github.com/python/mypy/issues/3673

Adds a trivial for-loop and a helper set to check duplicates of `typeddict_bases`.

## Test Plan

Adds a new test case `testCannotCreateTypedDictWithDuplicateBases`

Problem statement:
Duplicate Base Class for TypedDict
Consider the following code:

```py
from mypy_extensions import TypedDict

class MovieBase(TypedDict):
    name: str
    year: int


class Movie(MovieBase, MovieBase):
    based_on: str
```

Notice that the `Movie` class inherits `MovieBase` twice. 

mypy gives the following errors:
```
n.py:8: error: Cannot overwrite TypedDict field "name" while merging
n.py:8: error: Cannot overwrite TypedDict field "year" while merging
```

while there's no runtime error.

Note that if `MovieBase` was not a `TypedDict`, there would be a runtime TypeError: `duplicate base class MovieBase`



I think the right thing here would be to report duplicate base class.

Hints from issue discussion:

Commit message: Fix type check

Patch to review:
diff --git a/mypy/semanal_typeddict.py b/mypy/semanal_typeddict.py
index f70bbe427124..46565e98cec7 100644
--- a/mypy/semanal_typeddict.py
+++ b/mypy/semanal_typeddict.py
@@ -67,16 +67,24 @@ def analyze_typeddict_classdef(self, defn: ClassDef) -> Tuple[bool, Optional[Typ
                 defn.analyzed.line = defn.line
                 defn.analyzed.column = defn.column
                 return True, info
+
             # Extending/merging existing TypedDicts
-            if any(not isinstance(expr, RefExpr) or
-                   expr.fullname not in TPDICT_NAMES and
-                   not self.is_typeddict(expr) for expr in defn.base_type_exprs):
-                self.fail("All bases of a new TypedDict must be TypedDict types", defn)
-            typeddict_bases = list(filter(self.is_typeddict, defn.base_type_exprs))
+            typeddict_bases = []
+            typeddict_bases_set = set()
+            for expr in defn.base_type_exprs:
+                if (not isinstance(expr, RefExpr) or
+                        expr.fullname not in TPDICT_NAMES and not self.is_typeddict(expr)):
+                    self.fail("All bases of a new TypedDict must be TypedDict types", defn)
+                elif self.is_typeddict(expr):
+                    if expr.fullname not in typeddict_bases_set:
+                        typeddict_bases_set.add(expr.fullname)
+                        typeddict_bases.append(expr)
+                    else:
+                        assert isinstance(expr.node, TypeInfo)
+                        self.fail('Duplicate base class "%s"' % expr.node.name, defn)
             keys: List[str] = []
             types = []
             required_keys = set()
-
             # Iterate over bases in reverse order so that leftmost base class' keys take precedence
             for base in reversed(typeddict_bases):
                 assert isinstance(base, RefExpr)
diff --git a/test-data/unit/check-typeddict.test b/test-data/unit/check-typeddict.test
index 04bbc9b2b4d4..eae26f6c98c0 100644
--- a/test-data/unit/check-typeddict.test
+++ b/test-data/unit/check-typeddict.test
@@ -171,6 +171,18 @@ p: Point2D
 reveal_type(p)  # N: Revealed type is "TypedDict('__main__.Point2D', {'x': builtins.int, 'y': builtins.int})"
 [builtins fixtures/dict.pyi]
 
+[case testCannotCreateTypedDictWithDuplicateBases]
+# https://github.com/python/mypy/issues/3673
+from typing import TypedDict
+
+class A(TypedDict):
+    x: str
+    y: int
+
+class B(A, A): # E: Duplicate base class "A"
+    z: str
+[typing fixtures/typing-typeddict.pyi]
+
 [case testCannotCreateTypedDictWithClassWithOtherStuff]
 # flags: --python-version 3.6
 from mypy_extensions import TypedDict


Code review comments:
diff_hunk: @@ -67,16 +67,24 @@ def analyze_typeddict_classdef(self, defn: ClassDef) -> Tuple[bool, Optional[Typ
                 defn.analyzed.line = defn.line
                 defn.analyzed.column = defn.column
                 return True, info
+
             # Extending/merging existing TypedDicts
-            if any(not isinstance(expr, RefExpr) or
-                   expr.fullname not in TPDICT_NAMES and
-                   not self.is_typeddict(expr) for expr in defn.base_type_exprs):
-                self.fail("All bases of a new TypedDict must be TypedDict types", defn)
-            typeddict_bases = list(filter(self.is_typeddict, defn.base_type_exprs))
+            typeddict_bases = []
+            typeddict_bases_set = set()
+            for expr in defn.base_type_exprs:
+                if (not isinstance(expr, RefExpr) or
path: mypy/semanal_typeddict.py
text: @user1:
I feel like there's two many nots here for this code to be readable. It looks like there are three cases: (1) the base class is TypedDict of some sort (expr.fullname in TPDICT_NAMES); (2) the base class is another TypedDict; (3) anything else, which produces an error. The code would be more readable if we covered those three cases in order, without any nots.