Check deferred nodes after all first passes in an import cycle #2264

gvanrossum · 2016-10-17T16:49:01Z

Might fix #481.

JukkaL · 2016-10-19T13:26:39Z

test-data/unit/check-modules.test

+[file b.py]
+import a
+def g() -> int:
+    return a.y


Reveal the type of a.y (to verify that it doesn't somehow fall back to Any).

JukkaL · 2016-10-19T13:26:54Z

test-data/unit/check-modules.test

+[file a.py]
+import b
+def f() -> int:
+    return b.x


Reveal the type of b.x.

JukkaL · 2016-10-19T13:40:01Z

mypy/checker.py

+                                                in options.strict_optional_whitelist)
+
+    def check_first_pass(self) -> None:
+        """Type check the entire file, but defer functions with forward references."""


Be more explicit about forward reference mean. In particular, they mean references to definitions whose types haven't been inferred yet.

JukkaL · 2016-10-19T14:49:32Z

Looks good to me! This hopefully fixes one of the biggest pain points mypy users with large code bases have had.

I'd like to see some more test cases before merging:

Add test case where two passes aren't enough, to verify that we still
generate cannot determine type errors when needed.
Add test case for referring to a decorated function whose type might not be inferred
(another common case where we had trouble before).
Add test case where we generate an error only on the second pass (to verify
that the import context is shown correctly). For example, something like this (partial example):

def f() -> None:
    a = b.x  # Type of b.x only available during second pass
    a + 1   # Error that we can't report during first pass

gvanrossum · 2016-10-19T18:59:49Z

I'll add more tests. I also discovered that reveal_type() needs to check for current_node_deferred.

gvanrossum · 2016-10-22T05:10:56Z

Rebased, generalized the multi-pass machinery, and added a test that would require 3 passes.

Still needs tests for (2) and (3). Also, there are still some print() calls left.

gvanrossum · 2016-10-22T17:40:29Z

I could use a hint for (2).

gvanrossum · 2016-10-23T01:13:44Z

I think I have all the required tests now. Jukka, can you double-check?

codecov-io · 2016-10-23T01:26:06Z

Current coverage is 84.90% (diff: 100%)

Merging #2264 into master will increase coverage by 1.80%

@@             master      #2264   diff @@
==========================================
  Files            72         72           
  Lines         19037      20953   +1916   
  Methods           0          0           
  Messages          0          0           
  Branches       3919       4824    +905   
==========================================
+ Hits          15819      17790   +1971   
+ Misses         2618       2588     -30   
+ Partials        600        575     -25

Powered by Codecov. Last update 15adf8b...f1b8872

JukkaL · 2016-10-24T13:25:59Z

mypy/checker.py

+            return False
+        self.errors.set_file(self.path)
+        self.pass_num += 1
+        print('---', self.path, 'pass', self.pass_num + 1, '---')


Remove print.

JukkaL · 2016-10-24T13:26:24Z

mypy/checker.py

+        for node, type_name in todo:
+            if node in done:
+                continue
+            print(type_name, '.', node.fullname() or node.name())


Remove another print.

JukkaL · 2016-10-24T13:29:00Z

test-data/unit/check-modules.test

+tmp/a.py:4: error: Cannot determine type of 'x2'
+
+[case testErrorInPassTwo]
+# flags: --hide-error-context


Don't hide error context to test the it works correctly for errors reported in the second pass.

JukkaL · 2016-10-24T13:30:21Z

test-data/unit/check-modules.test

+import a
+x = 1 + 1
+[out]
+tmp/a.py:4: error: Unsupported operand types for + ("int" and "str")


Maybe also test this the other way as well (error reported in b.py in addition a.py).

JukkaL · 2016-10-24T13:32:29Z

test-data/unit/check-modules.test

+[file b.py]
+from typing import Any
+import a
+def deco(f: Any) -> Any:


This decorator probably doesn't trigger a second pass, since simple decorator signatures are special cased in semantic analysis. A more complex decorator signature such as Callable[[T], int] -> Callable[[T]], str] is needed.

JukkaL · 2016-10-24T13:33:42Z

test-data/unit/check-modules.test

+[file a.py]
+import b
+def g() -> None:
+    f()


Make sure that the decorator function returns an interesting type (not Any) and do reveal_type(f()) here to verify that decorator got applied correctly.

These verify that deferral due to a cross-module dependency works.

…irst time around

gvanrossum · 2016-10-24T16:22:28Z

Ready for review/merge.

JukkaL · 2016-10-24T16:39:05Z

LGTM now -- thanks for the updates!

gvanrossum · 2016-10-25T18:23:27Z

OK, rebased and made all requested amendments.

This was referenced Oct 17, 2016

Generated classes does not work with forward references #1701

Closed

Make it easier to work with circular module dependencies #481

Closed

JukkaL reviewed Oct 19, 2016

View reviewed changes

test-data/unit/check-modules.test

[file a.py]

import b

def f() -> int:

return b.x

Copy link

Collaborator

JukkaL Oct 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reveal the type of b.x.

JukkaL reviewed Oct 19, 2016

View reviewed changes

gvanrossum mentioned this pull request Oct 20, 2016

Add --warn-no-return, and major binder refactoring #1748

Merged

gvanrossum force-pushed the checker-per-file branch from e5b8733 to 1471cb4 Compare October 22, 2016 05:10

JukkaL reviewed Oct 24, 2016

View reviewed changes

Guido van Rossum added 11 commits October 24, 2016 08:43

Use a separate TypeChecker per file.

cd22b73

Change the TypeChecker toplevel interface.

4bd65c0

Do deferred (second) type checker pass after all first passes in an SCC.

9b46001

Add simple tests.

d4d26c6

These verify that deferral due to a cross-module dependency works.

Fix test failures due to semantic_analysis_only flag.

4afdc13

Add TODOs about getting rid of BuildResult and all_types.

be3dfa1

Also hide cross-module notes with --hide-error-context

de3a076

Expand docstring for check_first_pass()

ed10d6d

Suppress reveal_type() output when current node is being deferred

30c84fa

Add reveal_type() calls to testSymmetricImportCycle*

f68a7d4

WIP skip deferred queue duplicates

fd5bc22

Guido van Rossum added 9 commits October 24, 2016 08:43

Make it easy to do more than 2 passes

53f635b

Add a test that would require three passes

182059b

Make lint happy

981c36e

Test case for error reported by pass two

bb5361f

Test case for deferred decorator

8abacf1

Explicit test case that reveal_type() shuts up in deferred function f…

6aea45a

…irst time around

Remove print calls

19345a0

Remove --hide-error-context from new tests and adjust expected output

bb212eb

Add reversed version of testErrorInPassTwo

331f845

gvanrossum force-pushed the checker-per-file branch from f1b8872 to 3357393 Compare October 24, 2016 15:44

Make decorator more complicated (both b.deco and a.g are now deferred)

d1778da

gvanrossum force-pushed the checker-per-file branch from 3357393 to d1778da Compare October 24, 2016 15:46

JukkaL merged commit 68c6e96 into master Oct 24, 2016

gvanrossum deleted the checker-per-file branch October 24, 2016 17:05

gvanrossum mentioned this pull request Oct 24, 2016

Add a new, lower priority for imports inside "if MYPY" #2167

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Check deferred nodes after all first passes in an import cycle #2264

Check deferred nodes after all first passes in an import cycle #2264

gvanrossum commented Oct 17, 2016

JukkaL Oct 19, 2016

JukkaL Oct 19, 2016

JukkaL Oct 19, 2016

JukkaL commented Oct 19, 2016

gvanrossum commented Oct 19, 2016

gvanrossum commented Oct 22, 2016

gvanrossum commented Oct 22, 2016

gvanrossum commented Oct 23, 2016

codecov-io commented Oct 23, 2016 •

edited

Loading

JukkaL Oct 24, 2016

JukkaL Oct 24, 2016

JukkaL Oct 24, 2016

JukkaL Oct 24, 2016 •

edited

Loading

JukkaL Oct 24, 2016

JukkaL Oct 24, 2016

gvanrossum commented Oct 24, 2016

JukkaL commented Oct 24, 2016

gvanrossum commented Oct 25, 2016 via email

Check deferred nodes after all first passes in an import cycle #2264

Check deferred nodes after all first passes in an import cycle #2264

Conversation

gvanrossum commented Oct 17, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JukkaL commented Oct 19, 2016

gvanrossum commented Oct 19, 2016

gvanrossum commented Oct 22, 2016

gvanrossum commented Oct 22, 2016

gvanrossum commented Oct 23, 2016

codecov-io commented Oct 23, 2016 • edited Loading

Current coverage is 84.90% (diff: 100%)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JukkaL Oct 24, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gvanrossum commented Oct 24, 2016

JukkaL commented Oct 24, 2016

gvanrossum commented Oct 25, 2016 via email

codecov-io commented Oct 23, 2016 •

edited

Loading

JukkaL Oct 24, 2016 •

edited

Loading