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

feat: support 'this' type on methods and properties #2906

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

mattjohnsonpint
Copy link
Contributor

@mattjohnsonpint mattjohnsonpint commented Feb 5, 2025

Fixes #2904

Changes proposed in this pull request:

  • Fixes support for polymorphic this type on class methods and properties
  • Reports a "not implemented" for fields declared with the this type (would be nice, but is complicated)
  • Adds compiler tests for this feature

To clarify, the this type was already implemented, but was returning the base type instead of the calling instance type.

  • I've read the contributing guidelines
  • I've added my name and email to the NOTICE file

@mattjohnsonpint mattjohnsonpint marked this pull request as draft February 5, 2025 23:43
@mattjohnsonpint mattjohnsonpint marked this pull request as ready for review February 6, 2025 01:07
@mattjohnsonpint
Copy link
Contributor Author

FYI, I considered trying to bind the prototype instance to the derived type, but I couldn't quite get that way to work. So instead, I resolve the current "this" to use as the class instance instead. It works, but let me know if there's a better way to approach this. Thanks.

Copy link
Member

@HerrCai0907 HerrCai0907 left a comment

Choose a reason for hiding this comment

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

Could we create an override method for inheritance class
e.g.
for class A extends B, B has method foo return this, we resolve it as return B. A will generate override method foo return A.
Then resolve function should resolve to correct function.

@mattjohnsonpint
Copy link
Contributor Author

Could we create an override method for inheritance class ...

I explored this a bit. It would have side effects, even without using the this return type.

For example, given:

class X {
  private foo: i32 = 0;

  doSomething(): void {
    this.foo = 1;
  }
}

class Y extends X {
}

... we can't just transform it to this:

class X {
  private foo: i32 = 0;

  doSomething(): void {
    this.foo = 1;
  }
}

class Y extends X {
  doSomething(): void {
    this.foo = 1;
  }
}

... because field foo is declared private.

The resolver implementation does effectively create an alias of the member, but in doing so it uses the original prototype, bound to the base instance. It doesn't create an actual override.

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

Successfully merging this pull request may close these issues.

this type should return instance type when overridden
2 participants