Skip to content
This repository was archived by the owner on Apr 14, 2022. It is now read-only.

Adding super as a specialized keyword #1702

Merged
merged 23 commits into from
Nov 1, 2019

Conversation

bschnurr
Copy link
Member

@bschnurr bschnurr commented Oct 17, 2019

super
Fix for #1232
Fix for #1130

-adding multi param support
-adding tests around gotoDefinition (still broken)
refactoing python 2.7 two param super(..)  I was using the mro from the frist param always, but it should be the second.
- new helper functions IsInstanceOf() and IsSubClass()
removing IsInstanceOf
removing nested if
Added Distinct completion tests
@MikhailArkhipov
Copy link

I am wondering if persisting super is actually necessary. What is the scenario that gets broken if we don't cache it? Caching is for libraries and we don't do navigation or completions there.

@jakebailey
Copy link
Member

jakebailey commented Oct 31, 2019

The short example is a library which has something to the effect of:

class Foo(Bar):
    def sup(self):
        return super()

Or:

x = super(B, A())

The return values or variable values are instances of this super proxy. I can't imagine that this is common (would need to write some quick logging and see what happens on a real codebase), but this means that our database output depends on how we happen to hack together the super type and what default cases happen in the module storage.

@MikhailArkhipov
Copy link

I think in

def sup(self):
        return super()

sup should have StaticReturnValue evaluated to actual value of super() and the qualified name of the evaluated call should be persisted (i.e. overload model should have actual return type of super).

@jakebailey
Copy link
Member

Yes, but the issue is that if we don't modify any persistence code, the super proxy gets persisted as "instance of class super defined in the module where it was used", which falls apart because there isn't actually a class called "super" in the module where it was used.

Storing the entirety of the super proxy's state is certainly possible, as the MRO in IPythonSuperType is just of more types, whose qualified names we can store in a list, then restore fully to the original state.

@MikhailArkhipov
Copy link

MikhailArkhipov commented Oct 31, 2019

OK, then perhaps qualified name of the IPythonSuperType should be the actual underlying value. Like with PythonTypeWrapper? Such as t:baseClass? It probably does not need dedicated model.

@jakebailey
Copy link
Member

Yes, that's what I'm thinking. s:[some,list,that,is,mro] or similar.

- changing qualified name to :SuperType
- trimming the leading ":" from builttin types
@bschnurr bschnurr force-pushed the bschnurr/add_super_support branch from f2175f9 to 475f557 Compare October 31, 2019 18:55
moving createSuper(..) to PythonSuperTypes
@jakebailey
Copy link
Member

LGTM, caching stuff appears to work fine. All tests pass (sans stuff that was already broken in master).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants