Fix for NamedTuple in method [WIP]

[gvanrossum]

Fixes #2535.

[elazarg]

In lines 1654 and 741 we have:

node.kind = GDEF  # TODO locally defined namedtuple

and when working with classes, we have (line 788)

        if self.is_func_scope() or self.type:
            kind = MDEF
            if self.is_func_scope():
                kind = LDEF

is it not related? (I must say I do not completely understand how the symbol table works).

(The whole ad-hoc type creation (process_*_definition) screams for refactoring, but that's probably for a another PR).

[gvanrossum]

From grepping it looks like the distinction between LDEF, MDEF and GDEF is minimal, and the kind is only really used in pretty_name() in strconv.py, to format the name somewhat differently for debug output. There are a few other places that compare them but treat all three the same. We could probably replace them with a single 'DEF' and everything would continue to work except we would have to fix a whole lot of tests that compare the debug output (node rendering IIRC).

(The constants are also returned by kind_by_scope() and its return value is sometimes incorporated in symbol table nodes, but the only place where I see the kind retrieved from those nodes used is in strconv, and in a whole bunch of places that copy it into other nodes.)

Maybe @davidfstr is interested in the ad-hoc class creation refactoring you suggest? (Before he introduced TypedDict there was only one place doing this stuff.)

[davidfstr]

With respect to refactoring process_*_definition (and everything related to visit_assignment_stmt), there was a refactoring I had in mind that would move the entire call tree rooted at visit_assignment_stmt to a separate class, similar to ExpressionChecker (@ checkexpr.py).

More details about the refactoring - in particular the call tree - are in this postponed proposal: #2198

[ilevkivskyi]

There is one more thing about refactoring process_*_definition things. Now forward references only work for classes but not for things defined in assignment statements: type aliases, named tuples, newtypes, typeddicts (there are several issues about this on the tracker). I have an idea to fix this by just moving part of the logic that is now in second pass to the first pass in semanal.py. I am not working on this right now, but I think I will start soon (hopefully I will finish this before the end of the year).

[gvanrossum]

Beware, the first pass runs very early and under severe restrictions. It is mostly just concerned with computing the set of module dependencies.

[ilevkivskyi]

Yes, it also creates "stub" nodes for classes, so that they are "known" for type analyzer in second pass, thus avoiding Invalid type and later filling up the nodes with real info. I just want to implement a similar logic for other types.

[JukkaL]

Since we don't process imports during the first pass, you can't reliably detect references to things like TypedDict or NewType. They can be accessed with different names, and there could be other things named the same but meaning different things.

[gvanrossum]

Anyway, I'm just going to merge this after I've verified that the globals can't conflict.

[gvanrossum]

@JukkaL Regarding the current diff, it seems that simply NOT storing the info in self.globals[name] solve the problem too. Do you recall what that assignment is even for? I'm guessing it creates a global named A when you have a NamedTuple('A', ...) used in an expression that's not directly assigned to a variable. But why would we need that? Apparently everything still works, and the incremental fixup bug is gone (though I still have to confirm that with confirmed in our internal codebase).

[gvanrossum]

(Whoops, left the WIP in the commit message. Oh well.)