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

Fix handling of generics and class constructors that define variables - #666

Merged
Mikhail Arkhipov (MikhailArkhipov) merged 51 commits into
microsoft:masterfrom
MikhailArkhipov:462
Feb 28, 2019
Merged

Mikhail Arkhipov (MikhailArkhipov) merged 51 commits into
microsoft:masterfrom
MikhailArkhipov:462

Conversation

@MikhailArkhipov

Copy link
Copy Markdown

Fixes #462
Fixes #338
Fixes #337

  • The issue was with incorrect interpretation of arguments to generic class constructor. Instead of type the actual specific instance is being passed so we need to extract type arguments from the instance (such as dict) and create specific type based in the passed instance content and type.
  • Added support for generic->specific argument match for generic functions in non-generic classes.
  • Support standalone generic-returning functions (+enabled test).
  • Fixed processing of constructors that define class variables + test.
  • Split typing and generic tests into separate classes.

@MikhailArkhipov Mikhail Arkhipov (MikhailArkhipov) changed the title 462 Fix handling of generics and class constructors that define variables Feb 27, 2019
@MikhailArkhipov

Copy link
Copy Markdown
Author

Eeew PR number 666...

Comment thread src/LanguageServer/Impl/Program.cs Outdated
var classGenericParameters = selfClassType.GenericParameters.Keys.ToArray();
if (classGenericParameters.Length > 0) {
// Declaring class is specific and provides definitions of generic parameters
typeArgs = classGenericParameters

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What if there is a mix of class generic parameters and method generic parameters?

@MikhailArkhipov Mikhail Arkhipov (MikhailArkhipov) Feb 28, 2019 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Its OK. Specific class stores dictionary of the original generic parameter names (such as K, V in Dict) and their assigned specific types (int, str) if Dict[K, V] was created via

from typing import Dict

d1 = {1:'a', 2:'b'}
d2 = Dict(d1)

So here we are trying to match names of the generic function parameters to those of the class so we can figure out which specific type to use. Generally speaking method generic names must either be different or they match the class. Per Python T is basically a variable with the assigned type declared in the class scope. So function that is generic of T will be using T defined by the class.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I've been thinking of a case when function has a different generic parameter. Something like this:

from typing import TypeVar, Generic, Callable

T = TypeVar('T')
V = TypeVar('V')

class C(Generic[T]):
    def __init__(self, value: T) -> None:
        self.value = value

    def f(self, input: V, callback: Callable[[T, V], str]) -> str: 
        return callback(self.value, input)

def g(s1: int, s2: str) -> str:
    return str(s1) + s2

print(C(1).f("b", g))

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

In this particular case it doesn't matter since annotation says str and hence

image

Now, if we are to eval this (we don't support Callable just yet though, that's #535)... it is still a string since eval with call callback and either use annotation or eval with arguments and yield a string.

There are probably some convoluted examples, but I'd wait for users to give them to us :-)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry, I meant def f(self, input: V, callback: Callable[[T, V], V]) -> V in the example above. But since we have separate task for it, that's not for this PR.

Comment thread src/LanguageServer/Impl/Sources/DefinitionSource.cs
Comment thread src/LanguageServer/Impl/Sources/DefinitionSource.cs
Mikhail Arkhipov added 2 commits February 27, 2019 20:11
public bool VisibleToChildren { get; }

public IReadOnlyList<IScope> Children => _childScopes ?? Array.Empty<IScope>() as IReadOnlyList<IScope>;
public IReadOnlyList<IScope> Children => _childScopes?.ToArray() ?? Array.Empty<IScope>();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

_childScopes looks like a good candidate for ImmutableArray, cause the child scope is never removed. And it can be safely returned from the Children property as is. But it's up to you.

@MikhailArkhipov
Mikhail Arkhipov (MikhailArkhipov) merged commit eaa08a4 into microsoft:master Feb 28, 2019
Jake Bailey (jakebailey) pushed a commit to jakebailey/python-language-server that referenced this pull request Nov 1, 2019
…microsoft#666)

* Fix microsoft#601

* Fix microsoft#607

* Update test

* Fix version conditionals
Implement platform handling
Specialize os.path

* Optional/None

* Make sure constructors are processed

* Null checks

* Merge issues

* Merge branch 'master' of https://github.com/Microsoft/python-language-server into 601

* Using

* Merge master

* Attach object as base to 3x classes

* using

* Partial

* Handle or expression in parameters

* Optimistically return instance if callable turns out to be a property or a dynamic member.

* Remove async
Better handle inner functions

* Pass ctor arguments

* Test updates

* Tests

* Fix SO

* Generic functions - partial

* Generic args

* Tests and formatting

* Fix NRE

* Don't eval stub functions

* Fix doc strings

* Handle standalone generic + enable test
Split typing ang generics tests

* Tests update

* More gotodef tests

* Property handle constructors defining members

* fix function doc

* Debug code

* Add as name check

* Fix import

* Add AsyncLocal

* Revert "Add AsyncLocal"

This reverts commit 5a808b6.
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.

3 participants