Repository: python/mypy
Pull request: Improve annotated support with non types
Closes #9315

This PR aims to support Annotated to create type Aliases when passing a non type as annotation, like this:

```python
class Meta:
    ...

x = Annotated[int, Meta()]
```

You can test with: 

```
pytest -k testAnnotatedSecondParamNonType --pdb
```

Problem statement:
Type alias for Annotated[some_type, non_type] not supported
The following code:

```py
from typing_extensions import Annotated
class Positive:...
PositiveInt = Annotated[int, Positive()]
```

triggers the following error:
```
test.py:3: error: Invalid type alias: expression is not a valid type
```

Mypy version: mypy 0.782
Python version: Python 3.7.7 (default, May 10 2020, 19:54:23)
[Clang 11.0.3 (clang-1103.0.32.59)] on darwin
Mypy config:
```ini
[mypy]
disallow_subclassing_any = True
disallow_any_generics = True
disallow_untyped_calls = True
disallow_untyped_defs = True
disallow_incomplete_defs = True
disallow_untyped_decorators = True
no_implicit_optional = True
warn_return_any = True
strict_equality = True
ignore_missing_imports = False
strict_optional = True
warn_unused_ignores = True
warn_redundant_casts = True
warn_unused_configs = True
warn_unreachable = True
check_untyped_defs = True
new_semantic_analyzer = True
```

Hints from issue discussion:
I'd recommend changing the issue topic to "Type alias for Annotated[some_type, non_type] not supported".

Because `A = Annotated[int, Positive]` works, and so does `x: Annotated[int, Positive()]`, but indeed `B = Annotated[int, Positive()]` does not.
I'd like to try fixing this bug 😊 Can I get some hints on where to start?
Check out typeanal (where mypy tries to understand type annotations) and exprtotype. Throwing in breakpoints is usually a good way to start fixing things in mypy.

Commit message: Don't try to convert annotated's second param to type

Patch to review:
diff --git a/mypy/exprtotype.py b/mypy/exprtotype.py
index a3b762a1b64f..6ac09c28d8ab 100644
--- a/mypy/exprtotype.py
+++ b/mypy/exprtotype.py
@@ -61,7 +61,10 @@ def expr_to_unanalyzed_type(expr: Expression, _parent: Optional[Expression] = No
                 args = expr.index.items
             else:
                 args = [expr.index]
-            base.args = tuple(expr_to_unanalyzed_type(arg, expr) for arg in args)
+            if base.name == 'Annotated':
+                base.args = (expr_to_unanalyzed_type(args[0], expr), args[1])
+            else:
+                base.args = tuple(expr_to_unanalyzed_type(arg, expr) for arg in args)
             if not base.args:
                 base.empty_tuple_index = True
             return base
diff --git a/test-data/unit/check-annotated.test b/test-data/unit/check-annotated.test
index aeb1a1985e6d..58dc33460cc0 100644
--- a/test-data/unit/check-annotated.test
+++ b/test-data/unit/check-annotated.test
@@ -116,3 +116,13 @@ Alias = Annotated[Union[T, str], ...]
 x: Alias[int]
 reveal_type(x)  # N: Revealed type is 'Union[builtins.int, builtins.str]'
 [builtins fixtures/tuple.pyi]
+
+[case testAnnotatedSecondParamNonType]
+from typing_extensions import Annotated
+
+class Meta:
+    ...
+
+x = Annotated[int, Meta()]
+reveal_type(x)  # N: Revealed type is 'def () -> builtins.int'
+[builtins fixtures/tuple.pyi]


Code review comments:
diff_hunk: @@ -61,7 +61,10 @@ def expr_to_unanalyzed_type(expr: Expression, _parent: Optional[Expression] = No
                 args = expr.index.items
             else:
                 args = [expr.index]
-            base.args = tuple(expr_to_unanalyzed_type(arg, expr) for arg in args)
+            if base.name == 'Annotated':
+                base.args = (expr_to_unanalyzed_type(args[0], expr), args[1])
path: mypy/exprtotype.py
text: @author:
this seems to work, but it will probably fail the type checking since now the args are of different types.

@author:
it feels a bit hacky, let me know what you think!

@author:
the check is potentially wrong too, but I think it might be worth checking this PR and maybe try to find a better way to do this 😊

@user1:
You're correct that you're failing type checking. You can probably fix the first type error by narrowing / guarding expr.base to the type that it is when its typing.Annotated (probably `isinstance(expr.base, RefExpr)`?)

The second type error is trickier and might indicate a fundamental problem. If there isn't a way around this, we could band aid this problem by just returning `expr_to_unanalyzed_type(args[0], expr)`. I'd imagine the implication of this is that mypy and mypy plugins won't recognise the alias as being Annotated, but at least we no longer have the false positive.

Finally, Annotated is variadic, so it can have arbitrarily many parameters, so you don't want to just hardcode it to two.

@author:
> You're correct that you're failing type checking. You can probably fix the first type error by narrowing / guarding expr.base to the type that it is when its typing.Annotated (probably isinstance(expr.base, RefExpr)?)

Done :)

> The second type error is trickier and might indicate a fundamental problem. If there isn't a way around this, we could band aid this problem by just returning expr_to_unanalyzed_type(args[0], expr). I'd imagine the implication of this is that mypy and mypy plugins won't recognise the alias as being Annotated, but that's not a regression and we no longer have the false positive. 

I followed your suggestion for now, mostly to see if there's any other failing test.
I wonder if need the annotations anywhere else (I guess it might useful for plugins 🤔)

Happy to change it to something else, but I'm not sure what can we change now (I'm still unfamiliar with mypy's source code) :)