Nested class

[spkersten]

Fixes #565.

[JukkaL]

Hmm, this makes the lookup behavior even too flexible. This code is now accepted, even though it generates a NameError at runtime:

class A:
    class X: pass
    class B:
        y = X  # should be an error

Python doesn't look through the namespaces of enclosing classes (i.e., lexical scoping doesn't work for nested classes). A better way to fix this would be to use the namespace of the immediate enclosing class, but only when analyzing base classes (or function type annotations). For example:

class Top:
    class A:
        class B: pass
        class C(B):   # Use namespace of A to bind B (but ignore Top, A is not valid here)
            b = B  # This is an error: use namespace of C here (+ module namespace)

[spkersten]

I'm thinking of analysing the base classes before the setup is done for the analysis of (in you example) class C. That seems the most natural way too me and leaves the lookup function untouched. Simple switching the base class analysis and the setup (calls to Semanal.setup_class_def_analysis and Semanal.analyze_base_classes) doesn't work as the type variables are bound during the setup and that needs to happen before the base classes are analysed. I'm now looking at a way to refactor that code in an elegant way to make the switch possible (and make it more readable as a corollary).

[spkersten]

Now, the issue should be fixed (and similar issues with class decorators and meta classes) without making the lookup too permissive.

In the process I found another case where mypy is too permissive (already before my changes):

class A:
    a = A()  # Should raise an error, but doesn't

class B:
    class C(B): pass  # Should raise an error, but doesn't

This is not so easy to fix because we do want to allow A and B to be used as type annotation within their class bodies (and as code within methods). I'll look into this next.

[spkersten]

This is still work in progress, but I'd like some ideas about something. I'm making some fixes lookup problems. One fix is for the problem that mypy currently allows the use of symbols before they're defined, like this:

x = A()
class A: pass

I made some changes to report errors in those cases (again, still WIP). The problem is that now hundreds of test cases are failing because they contain invalid Python code like the snippet above. Does anyone have a smart idea for fixing those test cases, other than fixing them manually one by one?

[spkersten]

@kirbyfan64 See the Travis output: https://travis-ci.org/JukkaL/mypy/jobs/61891814

Note that some failures are 'real'. I'll try to resolve those first.

[JukkaL]

Ah, yes, the test cases assume that you can refer to a class before defining it :-(

Fixing most of the test cases could be possible via a script that works like this. Not sure if this would actually be less work than fixing the test cases manually, but at least it would be less boring:

1. Run the tests and parse test output to find out which test cases fail (record .test file path and test case name).

2. Write a script that moves class definitions in the affected test cases towards the beginning of the test input. It could work like this, using shallow parsing (disregarding many implementation details):

for each failing test case:
    parse test case file to find the input section of test case
    x = the location of the first non-import statement
        (first line after the last line beginning with `import` or `from`)
    skip empty lines after import statements
    while test case input has a class definition:
        find the next class; just look for lines starting with 'class ', and assume 
            that the class definition extends until the next non-empty line that 
            doesn't have leading spaces/tabs
        move the class to the location x
        update x to refer to the end of the copied class

The above steps aren't guaranteed to work, but they should (hopefully) work for the vast majority of cases. I can volunteer to write the script for step 2.

[spkersten]

I've removed the last bunch of commits, because they were really related to a separate issue. I thought that was quick to fix, but the test cases make it more difficult. I also want to refactor the handling environments/scopes and lookup before addressing it.

So, now the PR only contains a fix for #565 and can be merged (after a review) as far as I'm concerned.

Apologies for making a mess of this PR.

[JukkaL]

No problem! Now the PR looks good. Would you like to create a new issue for using names before they are defined?

[spkersten]

I've made #686 for the using-names-before-their-definition issue.