Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type 'Scope' is not assignable to type 'this' #8164

Closed
yortus opened this issue Apr 19, 2016 · 12 comments
Closed

Type 'Scope' is not assignable to type 'this' #8164

yortus opened this issue Apr 19, 2016 · 12 comments
Labels
By Design Deprecated - use "Working as Intended" or "Design Limitation" instead Question An issue which isn't directly actionable in code

Comments

@yortus
Copy link
Contributor

yortus commented Apr 19, 2016

TypeScript Version:

1.9.0-dev.20160419
1.8.7

Code

class Scope {

    constructor(private locals: string[], private parent: Scope) {
    }

    find(name: string) {
        var scope = this;
        while (scope && scope.locals.indexOf(name) === -1) {
            scope = scope.parent; // ERROR: Type 'Scope' is not assignable to type 'this'
        }
        return scope;
    }
}


let outer = new Scope(['a', 'b'], null);
let inner = new Scope(['b', 'c'], outer);
assert(inner.find('a') === outer);
assert(inner.find('b') === inner);
assert(inner.find('d') === null);

Expected behavior:

  • compile time: no compile errors
  • runtime: all assertions pass.

Actual behavior:

  • compile time: compile error as shown in code comment.
  • runtime: all assertions pass as expected.
@yortus
Copy link
Contributor Author

yortus commented Apr 19, 2016

If an explicit type annotation is added, the error goes away:

    ...
    find(name: string) {
        var scope: Scope = this; // NB: type annotation added here
    ...

...but that type annotation seems superfluous given that this is already known to be a Scope.

@kitsonk
Copy link
Contributor

kitsonk commented Apr 19, 2016

I maybe wrong, but type this !== Scope. It if guarding against you accidentally assigning something that could not be Scope in the future (e.g. if you extended Scope).

The problem really seems to be is you can't pass this as a type for an argument. This code works, but obviously isn't what you desire, as it won't guard against the argument:

class Scope {

    private parent: this;

    constructor(private locals: string[], parent: any) {
        this.parent = parent;
    }

    find(name: string) {
        var scope = this;
        while (scope && scope.locals.indexOf(name) === -1) {
            scope = scope.parent;
        }
        return scope;
    }
}

@yortus
Copy link
Contributor Author

yortus commented Apr 19, 2016

@kitsonk thanks for the input

It if guarding against you accidentally assigning something that could not be Scope in the future (e.g. if you extended Scope).

I don't suppose you could come up with a little example of this? I'm not really understanding the type error. Especially since the code works and is reasonably idiomatic as JavaScript, I would have thought.

@kitsonk
Copy link
Contributor

kitsonk commented Apr 19, 2016

Since 1.7, in classes and interfaces, there is a special this types, which is then being inferred on the assignment you are making of var scope = this. It is useful because it allow you to operate on things knowing that their type will change in the future:

class Animal {
  walk() {
    console.log('walking...');
    return this;
  }
}

class Dog extends Animal {
  bark() {
    console.log('barking...');
    return this;
  }
}

const dog = new Dog();
dog
  .walk() // < 1.7 would have returned type Animal
  .bark(); // so you wouldn't have been able to do this

So something of type this can be narrowed to Scope but something of type Scope cannot ever be widened into type this (because who knows at runtime what type this will be?)

@yortus
Copy link
Contributor Author

yortus commented Apr 19, 2016

But even if the instance was a subclass of Scope, every operation on the scope variable in the body of the find method would still work. Perhaps it would require too much analysis for the compiler to see that?

@kitsonk
Copy link
Contributor

kitsonk commented Apr 19, 2016

Yes, that is why type this can be narrowed to Scope, but what your code is doing is taking something that is specifically Scope (this.parent) and assign it to something that is type this. What you do with it afterwards is irrelevant to the compiler. Scope will always be a subset of this and this will always be a superset of Scope and Scope will never be equal to this in the mind of the compiler.

@yortus
Copy link
Contributor Author

yortus commented Apr 19, 2016

The obvious thing to try next is to change the constructor annotations to:

    constructor(private locals: string[], private parent: this) {
    }

...but that produces another error: a 'this' type is available only in a non-static member...

Then I tried (similar to your first suggestion):

class Scope {
    private parent: this;

    constructor(private locals: string[], parent: Scope) {
        this.parent = parent; // ERROR: Type 'Scope' is not assignable to type this
    }

In the end I've just added a type assertion on the var scope line to keep the compiler happy.

EDIT: I should add that Scope is a private class and will not have any subclasses. That's why I find having to reason about potential subclasses a hassle in this simple case.

@kitsonk
Copy link
Contributor

kitsonk commented Apr 19, 2016

Yes, I ran into the same issues you did in "fixing" it, in that you cannot type guard this in arguments, which I think has been brought up before, but I can't remember the reason why it isn't offered. That seems like the best way.

I think though the var scope: Scope = this is the most "accurate" solution at the moment and represents the intent of your code. It does imply though that the return type of find() will be Scope instead of polymorphic this, which sounds like in your use case is perfectly fine.

@mhegazy
Copy link
Contributor

mhegazy commented Apr 19, 2016

as @kitsonk explained, this is Scope but Scope is not this. I would think of this as T extends Scope.

@mhegazy mhegazy closed this as completed Apr 19, 2016
@mhegazy mhegazy added Question An issue which isn't directly actionable in code By Design Deprecated - use "Working as Intended" or "Design Limitation" instead labels Apr 19, 2016
@yortus
Copy link
Contributor Author

yortus commented Apr 20, 2016

@mhegazy why is it not allowed to do the following (the first thing I tried to fix the Scope/this mismatch):

constructor(private locals: string[], private parent: this) { }

That gives the error a 'this' type is available only in a non-static member...

@mhegazy
Copy link
Contributor

mhegazy commented Apr 20, 2016

issue #5863 tracks adding it.

Polymorphic this types are an instance type feature, the constructor is part of the static side of the class.

@yortus
Copy link
Contributor Author

yortus commented Apr 20, 2016

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
By Design Deprecated - use "Working as Intended" or "Design Limitation" instead Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

3 participants