Propagate type narrowing to nested functions #15133

JukkaL · 2023-04-25T17:32:48Z

Use the heuristic suggested in #2608 and allow narrowed types of variables (but not attributes) to be propagated to nested functions if the variable is not assigned to after the definition of the nested function in the outer function.

Since we don't have a full control flow graph, we simply look for assignments that are textually after the nested function in the outer function. This can result in false negatives (at least in loops) and false positives (in if statements, and if the assigned type is narrow enough), but I expect these to be rare and not a significant issue. Type narrowing is already unsound, and the additional unsoundness seems minor, while the usability benefit is big.

This doesn't do the right thing for nested classes yet. I'll create an issue to track that.

mypy/literals.py

JukkaL · 2023-04-25T18:37:48Z

mypy_primer output looks good. The new errors in openlibrary are from fixed false negatives. Similarly, this error from spark is due to better inferred types:

python/pyspark/pandas/frame.py:6248: error: Argument "value" to "replace" of "Series" has incompatible type "int | float | str | list[Any] | tuple[Any, ...] | dict[Any, Any]"; expected "list[Any] | tuple[Any, ...]"  [arg-type]

Everything else seems to be caused by either fewer errors generated (good!) or some error code changing due to a union type being replaced with a narrowed, non-union type.

Co-authored-by: Alex Waygood <[email protected]>

hauntsaninja

This is awesome, thanks for making it happen! One possible improvement: this fails if you use the name after the function is defined, even if you're just loading it and not assigning to it

JukkaL · 2023-04-26T18:17:32Z

this fails if you use the name after the function is defined, even if you're just loading it and not assigning to it

I think that this mostly works, since we only stop narrowing if the name is used in an lvalue. However, I noticed that we didn't handle some cases correctly. I've pushed a fix. Also, there was minimal test coverage for this. I updated tests to cover this case better.

hauntsaninja · 2023-04-26T20:38:07Z

Ah yeah, sorry, it does work! Thanks for adding the tests and for fixing for index and member exprs.

(What happened is I'd noticed there weren't tests for that case, so I locally added the test case and it failed, but only now am I realising that it failed for fixture reasons since we don't have __add__ on str in fixtures)

ilevkivskyi

LG, but have couple comments on tests.

ilevkivskyi · 2023-04-27T08:18:55Z

test-data/unit/check-optional.test

+
+x: Optional[str] = "x"
+
+def cannot_narrow_top_level() -> None:


The actual narrowing is missing here. Until #2008 is fixed this would be optional despite r.h.s. So you need to write

x: Optional[str] if x is None: x = "x"

here as well.

Yeah this was an incomplete test case.

ilevkivskyi · 2023-04-27T08:25:41Z

test-data/unit/check-optional.test

+    cc[c] = 3
+    nested()
+
+[builtins fixtures/isinstance.pyi]


I think it would be good to add tests for edge cases in the visitor like x, y = foo() and/or x = y = foo().

Added some test cases.

github-actions · 2023-05-03T10:01:27Z

Diff from mypy_primer, showing the effect of this PR on open source code:

werkzeug (https://github.com/pallets/werkzeug)
+ src/werkzeug/local.py:495: error: Unused "type: ignore" comment  [unused-ignore]
+ src/werkzeug/local.py:506: error: Unused "type: ignore" comment  [unused-ignore]
+ src/werkzeug/local.py:515: error: Unused "type: ignore" comment  [unused-ignore]

jinja (https://github.com/pallets/jinja)
+ src/jinja2/utils.py:262: error: Unused "type: ignore" comment  [unused-ignore]
+ src/jinja2/runtime.py:933: error: Unused "type: ignore" comment  [unused-ignore]
+ src/jinja2/environment.py:925: error: Unused "type: ignore" comment  [unused-ignore]

psycopg (https://github.com/psycopg/psycopg)
+ psycopg/psycopg/rows.py:125: error: Unused "type: ignore" comment  [unused-ignore]
+ psycopg/psycopg/rows.py:172: error: Unused "type: ignore" comment  [unused-ignore]
+ psycopg/psycopg/rows.py:208: error: Unused "type: ignore" comment  [unused-ignore]

openlibrary (https://github.com/internetarchive/openlibrary)
+ openlibrary/plugins/upstream/account.py: note: In function "csv_string":
+ openlibrary/plugins/upstream/account.py:854: error: Argument 1 to "row_formatter" has incompatible type "Mapping[Any, Any]"; expected "dict[Any, Any]"  [arg-type]
+ openlibrary/plugins/upstream/account.py:856: error: Argument 1 to "row_formatter" has incompatible type "Mapping[Any, Any]"; expected "dict[Any, Any]"  [arg-type]

isort (https://github.com/pycqa/isort)
+ isort/sorting.py:120: error: Unused "type: ignore" comment  [unused-ignore]

graphql-core (https://github.com/graphql-python/graphql-core)
+ src/graphql/validation/validate.py:61: error: Unused "type: ignore" comment  [unused-ignore]
+ tests/test_user_registry.py:495: error: Unused "type: ignore" comment  [unused-ignore]
+ tests/test_user_registry.py:501: error: Unused "type: ignore" comment  [unused-ignore]

spark (https://github.com/apache/spark)
+ python/pyspark/rdd.py:1515: error: Unused "type: ignore" comment  [unused-ignore]
+ python/pyspark/rdd.py:2212: error: Unused "type: ignore" comment  [unused-ignore]
+ python/pyspark/rdd.py:2252: error: Unused "type: ignore" comment  [unused-ignore]
+ python/pyspark/rdd.py:2465: error: Unused "type: ignore" comment  [unused-ignore]
+ python/pyspark/rdd.py:2472: error: Unused "type: ignore" comment  [unused-ignore]
+ python/pyspark/sql/streaming/readwriter.py:1243: error: Unused "type: ignore" comment  [unused-ignore]
+ python/pyspark/sql/streaming/readwriter.py:1286: error: Unused "type: ignore" comment  [unused-ignore]
+ python/pyspark/sql/streaming/readwriter.py:1286: error: "SupportsProcess" has no attribute "open"  [attr-defined]
+ python/pyspark/sql/streaming/readwriter.py:1286: note: Error code "attr-defined" not covered by "type: ignore" comment
+ python/pyspark/sql/streaming/readwriter.py:1293: error: Redundant cast to "SupportsProcess"  [redundant-cast]
+ python/pyspark/sql/streaming/readwriter.py:1298: error: Unused "type: ignore" comment  [unused-ignore]
+ python/pyspark/sql/streaming/readwriter.py:1298: error: "SupportsProcess" has no attribute "close"  [attr-defined]
+ python/pyspark/sql/streaming/readwriter.py:1298: note: Error code "attr-defined" not covered by "type: ignore" comment
+ python/pyspark/streaming/dstream.py:803: error: Unused "type: ignore" comment  [unused-ignore]
+ python/pyspark/streaming/dstream.py:852: error: Redundant cast to "int"  [redundant-cast]
+ python/pyspark/pandas/frame.py:6247: error: Argument "value" to "replace" of "Series" has incompatible type "int | float | str | list[Any] | tuple[Any, ...] | dict[Any, Any]"; expected "list[Any] | tuple[Any, ...]"  [arg-type]
+ python/pyspark/pandas/namespace.py:2306: error: Redundant cast to "list[str]"  [redundant-cast]
+ python/pyspark/pandas/namespace.py:2309: error: Redundant cast to "list[str]"  [redundant-cast]

cwltool (https://github.com/common-workflow-language/cwltool)
+ cwltool/validate_js.py: note: In function "get_expressions":
+ cwltool/validate_js.py:97:17: error: Redundant cast to "ArraySchema"  [redundant-cast]

ibis (https://github.com/ibis-project/ibis)
- ibis/backends/base/__init__.py:754: error: "BaseBackend" has no attribute "compiler"; maybe "compile"?  [attr-defined]

hydra-zen (https://github.com/mit-ll-responsible-ai/hydra-zen)
- src/hydra_zen/structured_configs/_implementations.py:1832: error: Argument 1 to "list" has incompatible type "list[str | DataClass_ | Mapping[str, str | Sequence[str] | None]] | None"; expected "Iterable[Any]"  [arg-type]
- src/hydra_zen/structured_configs/_make_custom_builds.py:299: error: List item 0 has incompatible type "DataclassOptions | None"; expected "SupportsKeysAndGetItem[Any, Any]"  [list-item]

discord.py (https://github.com/Rapptz/discord.py)
- discord/app_commands/tree.py:332: error: Incompatible types in assignment (expression has type "Command[Any, [VarArg(Any), KwArg(Any)], Any] | ContextMenu | Group", target has type "ContextMenu")  [assignment]

prefect (https://github.com/PrefectHQ/prefect)
- src/prefect/_internal/compatibility/deprecated.py:199: error: "None" not callable  [misc]
- src/prefect/_internal/compatibility/experimental.py:232: error: "None" not callable  [misc]

urllib3 (https://github.com/urllib3/urllib3)
+ src/urllib3/util/request.py:226: error: Unused "type: ignore" comment  [unused-ignore]

schuelermine · 2023-05-03T20:44:34Z

Thank you for this fix!

cdce8p · 2023-05-03T21:45:42Z

This PR seems to cause a new crash with Home Assistant. I narrowed it down to this (tested with Python 3.11). Not sure why mypy_primer didn't caught it though.

from contextlib import suppress
import os
from typing import Any


def setup(config: Any) -> None:
    if config is not None:
        os.path.join(config, "a")

    def func() -> None:
        with suppress(RuntimeError):
            pass

reveal_type(1)

Traceback

(venv-311) $ mypy --no-incremental test.py 
test.py:11: error: INTERNAL ERROR -- Please try using mypy master on GitHub:
https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
Please report a bug at https://github.com/python/mypy/issues
version: 1.4.0+dev.61b9b9c75fd79090fcd127e5030f18342224e90b
Traceback (most recent call last):
  File "/.../venv-311/bin/mypy", line 8, in <module>
    sys.exit(console_entry())
  File "/.../mypy/mypy/__main__.py", line 15, in console_entry
    main()
  File "/.../mypy/mypy/main.py", line 95, in main
    res, messages, blockers = run_build(sources, options, fscache, t0, stdout, stderr)
  File "/.../mypy/mypy/main.py", line 174, in run_build
    res = build.build(sources, options, None, flush_errors, fscache, stdout, stderr)
  File "/.../mypy/mypy/build.py", line 194, in build
    result = _build(
  File "/.../mypy/mypy/build.py", line 267, in _build
    graph = dispatch(sources, manager, stdout)
  File "/.../mypy/mypy/build.py", line 2926, in dispatch
    process_graph(graph, manager)
  File "/.../mypy/mypy/build.py", line 3324, in process_graph
    process_stale_scc(graph, scc, manager)
  File "/.../mypy/mypy/build.py", line 3425, in process_stale_scc
    graph[id].type_check_first_pass()
  File "/.../mypy/mypy/build.py", line 2311, in type_check_first_pass
    self.type_checker().check_first_pass()
  File "/.../mypy/mypy/checker.py", line 475, in check_first_pass
    self.accept(d)
  File "/.../mypy/mypy/checker.py", line 585, in accept
    stmt.accept(self)
  File "/.../mypy/mypy/nodes.py", line 784, in accept
    return visitor.visit_func_def(self)
  File "/.../mypy/mypy/checker.py", line 963, in visit_func_def
    self._visit_func_def(defn)
  File "/.../mypy/mypy/checker.py", line 967, in _visit_func_def
    self.check_func_item(defn, name=defn.name)
  File "/.../mypy/mypy/checker.py", line 1039, in check_func_item
    self.check_func_def(defn, typ, name, allow_empty)
  File "/.../mypy/mypy/checker.py", line 1234, in check_func_def
    self.accept(item.body)
  File "/.../mypy/mypy/checker.py", line 585, in accept
    stmt.accept(self)
  File "/.../mypy/mypy/nodes.py", line 1218, in accept
    return visitor.visit_block(self)
  File "/.../mypy/mypy/checker.py", line 2659, in visit_block
    self.accept(s)
  File "/.../mypy/mypy/checker.py", line 585, in accept
    stmt.accept(self)
  File "/.../mypy/mypy/nodes.py", line 784, in accept
    return visitor.visit_func_def(self)
  File "/.../mypy/mypy/checker.py", line 963, in visit_func_def
    self._visit_func_def(defn)
  File "/.../mypy/mypy/checker.py", line 967, in _visit_func_def
    self.check_func_item(defn, name=defn.name)
  File "/.../mypy/mypy/checker.py", line 1039, in check_func_item
    self.check_func_def(defn, typ, name, allow_empty)
  File "/.../mypy/mypy/checker.py", line 1234, in check_func_def
    self.accept(item.body)
  File "/.../mypy/mypy/checker.py", line 585, in accept
    stmt.accept(self)
  File "/.../mypy/mypy/nodes.py", line 1218, in accept
    return visitor.visit_block(self)
  File "/.../mypy/mypy/checker.py", line 2659, in visit_block
    self.accept(s)
  File "/.../mypy/mypy/checker.py", line 585, in accept
    stmt.accept(self)
  File "/.../mypy/mypy/nodes.py", line 1574, in accept
    return visitor.visit_with_stmt(self)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/.../mypy/mypy/checker.py", line 4771, in visit_with_stmt
    with self.binder.frame_context(can_skip=True, try_frame=True):
  File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/contextlib.py", line 144, in __exit__
    next(self.gen)
  File "/.../mypy/mypy/binder.py", line 435, in frame_context
    self.pop_frame(can_skip, fall_through)
  File "/.../mypy/mypy/binder.py", line 247, in pop_frame
    self.last_pop_changed = self.update_from_options(options)
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/.../mypy/mypy/binder.py", line 223, in update_from_options
    type = join_simple(self.declarations[key], type, other)
                       ~~~~~~~~~~~~~~~~~^^^^^
KeyError: ('Var', <mypy.nodes.Var object at 0x104254e10>)
test.py:11: : note: use --pdb to drop into pdb

JukkaL · 2023-05-04T12:50:37Z

This PR seems to cause a new crash with Home Assistant

I can confirm the crash. Thanks for the report! I'm looking at it.

See #15133 (comment) for context.

…15181) See #15133 (comment) for context.

hauntsaninja · 2023-05-04T17:50:58Z

Thanks for testing dev versions!

JukkaL added 20 commits April 25, 2023 10:56

Add test case

e4ecb91

WIP debugging stuff

05e3958

Propagate narrowed types to nested functions in some cases

f3c87ca

Add test cases

01e044d

Support for and with statements

eecc877

Test more

030457e

Fix nested functions

7968859

Add unit tests

7603f24

More testing

a81856a

Check if a match statement assigns to a variable

c503b3c

Support walrus expression

a601098

Fix self check and isort

fe05131

Add docstrings

9d0b478

Minor tweak to test

639a9a1

Refactoring and comment updates

6dbf9f1

More refactoring

d463240

Test narrowing multiple variables

77b67df

Add another test case

339ae64

Don't leak frames + add assert to find frame leaks

56c3f3a

Test method

24faaa5

JukkaL requested a review from ilevkivskyi April 25, 2023 17:35

Fix mypyc build

ecc2d34

AlexWaygood reviewed Apr 25, 2023

View reviewed changes

mypy/literals.py Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

JukkaL and others added 4 commits April 25, 2023 12:37

Update mypy/literals.py

67098aa

Co-authored-by: Alex Waygood <[email protected]>

Fix tests when using compiled mypy

25cafbd

Fix unused import

40f3b53

Actually fix compiled mypy

4bfb5f1

This comment has been minimized.

Sign in to view

hauntsaninja reviewed Apr 26, 2023

View reviewed changes

JukkaL added 2 commits April 26, 2023 12:08

Add more tests for reading narrowed variable after nested function

0c4dce4

Fix dealing with member and index expressions after nested function

4fe08c2

This comment has been minimized.

Sign in to view

ilevkivskyi approved these changes Apr 27, 2023

View reviewed changes

JukkaL added 3 commits May 2, 2023 17:20

Merge branch 'master' into nested-func-optional

dc6516e

Fix test case

c323e06

More tests

c350905

JukkaL merged commit 8c14cba into master May 3, 2023

JukkaL deleted the nested-func-optional branch May 3, 2023 12:39

JukkaL added a commit that referenced this pull request May 4, 2023

Fix crash related to propagating type narrowing to nested functions

c95f97f

See #15133 (comment) for context.

JukkaL mentioned this pull request May 4, 2023

Fix crash related to propagating type narrowing to nested functions #15181

Merged

JukkaL added a commit that referenced this pull request May 4, 2023

Fix crash related to propagating type narrowing to nested functions (#…

13f35ad

…15181) See #15133 (comment) for context.

hauntsaninja mentioned this pull request Sep 6, 2023

False positive and negative with lambda narrowing #16056

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Propagate type narrowing to nested functions #15133

Propagate type narrowing to nested functions #15133

JukkaL commented Apr 25, 2023

This comment has been minimized.

JukkaL commented Apr 25, 2023

This comment has been minimized.

hauntsaninja left a comment •

edited

Loading

JukkaL commented Apr 26, 2023 •

edited

Loading

This comment has been minimized.

hauntsaninja commented Apr 26, 2023

ilevkivskyi left a comment

ilevkivskyi Apr 27, 2023

JukkaL May 3, 2023

ilevkivskyi Apr 27, 2023

JukkaL May 3, 2023

github-actions bot commented May 3, 2023

schuelermine commented May 3, 2023

cdce8p commented May 3, 2023

JukkaL commented May 4, 2023

hauntsaninja commented May 4, 2023


		x: Optional[str] = "x"

		def cannot_narrow_top_level() -> None:

Propagate type narrowing to nested functions #15133

Propagate type narrowing to nested functions #15133

Conversation

JukkaL commented Apr 25, 2023

This comment has been minimized.

JukkaL commented Apr 25, 2023

This comment has been minimized.

hauntsaninja left a comment • edited Loading

Choose a reason for hiding this comment

JukkaL commented Apr 26, 2023 • edited Loading

This comment has been minimized.

hauntsaninja commented Apr 26, 2023

ilevkivskyi left a comment

Choose a reason for hiding this comment

ilevkivskyi Apr 27, 2023

Choose a reason for hiding this comment

JukkaL May 3, 2023

Choose a reason for hiding this comment

ilevkivskyi Apr 27, 2023

Choose a reason for hiding this comment

JukkaL May 3, 2023

Choose a reason for hiding this comment

github-actions bot commented May 3, 2023

schuelermine commented May 3, 2023

cdce8p commented May 3, 2023

JukkaL commented May 4, 2023

hauntsaninja commented May 4, 2023

hauntsaninja left a comment •

edited

Loading

JukkaL commented Apr 26, 2023 •

edited

Loading