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

Populate inheritied properties and methods in completion list inside a class #7158

Closed
mhegazy opened this issue Feb 19, 2016 · 21 comments
Closed
Assignees
Labels
Fixed A PR has been merged for this issue Help Wanted You can do this Suggestion An idea for TypeScript VS Code Tracked There is a VS Code equivalent to this issue

Comments

@mhegazy
Copy link
Contributor

mhegazy commented Feb 19, 2016

abstract calss B {
    abstract getValue() : number;
}

class C extends B {
    | // <= expect getValue to show.
}
@mhegazy mhegazy added Suggestion An idea for TypeScript Help Wanted You can do this labels Feb 19, 2016
@RyanCavanaugh RyanCavanaugh added this to the Community milestone Feb 19, 2016
@masaeedu
Copy link
Contributor

Should the same be done for ancestor interface members as well? Also, once the member has been implemented, should it stop being displayed in the completion list?

@mhegazy
Copy link
Contributor Author

mhegazy commented Feb 22, 2016

Should the same be done for ancestor interface members as well?

I do not know about this one. currently you get no completion in an interface; I would start with class only.

Also, once the member has been implemented, should it stop being displayed in the completion list?

That is correct

@masaeedu
Copy link
Contributor

currently you get no completion in an interface

What I mean is that you should get completion in a class deriving from an interface, just as you would in a class deriving from an abstract class. For example, if you had:

interface B {
    getValue() : number;
}

class C extends B {
    | // <= expect getValue to show.
}

You would still get the same completion options.

@mhegazy
Copy link
Contributor Author

mhegazy commented Feb 22, 2016

i also should add, that in the absence of these completions, only applicable keywords should be shown, i.e. public, private, protected, static, abstract, readonly, get and set.

@mihailik
Copy link
Contributor

constructor too

@masaeedu
Copy link
Contributor

@mihailik I'm not changing the completion options that would normally be populated at that location, just adding some extras. Right now the list isn't limited to the applicable keywords you and @mhegazy have mentioned (this is actually marked as a TODO in the code). I think this is best left to a separate PR.

@masaeedu
Copy link
Contributor

@mhegazy Could we open a new issue with the exact specification for what should and shouldn't be suggested in a class body? Unimplemented abstract ancestor members are PRed in #7190.

@DanielRosenwasser
Copy link
Member

Just because a base member is not abstract doesn't mean you can't override it. So only taking abstract members probably isn't the right approach.

So let's say all base class members which are not currently overridden should be listed as suggestions in the body of a class. But it's not quite that simple. If a member exists, you might still be defining an overload, so you definitely don't want to remove that from a completion list.

So as a first step, the completion list should consist of the above suggested keywords, alongside any base members. Don't worry about filtering candidates out for now.

For examples of how to approach this, take a look at tryGetObjectLikeCompletionSymbols or tryGetImportOrExportClauseCompletionSymbols.

@masaeedu
Copy link
Contributor

Gotcha, thanks. So the set of members to suggest (in addition to the keywords), is what would be suggested at this.|, correct? I could recursively add all base type members, but there's edge cases to consider like private members.

@DanielRosenwasser
Copy link
Member

@masaeedu yup, that sounds correct.

@tinganho
Copy link
Contributor

Could it be possible if this autocompletes the signature and not just the identifier?

abstract calss B {
    abstract getValue(p1: number, p2: number) : number;
}

class C extends B {
    | // <= expect 'getValue(p1: number, p2: number): number' to show.
}

@mhegazy
Copy link
Contributor Author

mhegazy commented May 11, 2016

Could it be possible if this autocompletes the signature and not just the identifier?

the API does not support a template-like completion at the moment. so either do identifiers only, or add a concept of replaceText to put the whole signature.

@heruan
Copy link

heruan commented Aug 8, 2016

I agree with @tinganho, a template-like completion for signatures would be greatly appreciated since in TypeScript it's very common to extend classes or implement interfaces.

@TzviPM
Copy link

TzviPM commented Mar 23, 2017

Is anybody working on this issue currently?

@RyanCavanaugh
Copy link
Member

@omniroot nope - you'll see someone in the "assigned" field before anyone begins work on it

@pmunin
Copy link

pmunin commented Jul 17, 2017

Not sure I'm following the process here - the issue marked as closed, however I still see this problem in current version of VSCode:
Version 1.14.1
Commit 2648980a697a4c8fb5777dcfb2ab110cec8a2f58
Date 2017-07-13T19:18:47.188Z
Shell 1.6.6
Renderer 56.0.2924.87
Node 7.4.0
Typescript 2.4.1

How can I see from the issue, when it's planned to be in PROD?

@bugproof
Copy link

  • VSCode Version: Code 1.16.1 (27492b6bf3acb0775d82d2f87b25a93490673c6d, 2017-09-14T16:38:23.027Z)
  • OS Version: Windows_NT x64 10.0.15063

doesn't work for me as well, I'm using React Native and it doesn't seem to complete this.setState for instance or any method from React.Component.

@pmunin
Copy link

pmunin commented Sep 21, 2017

I guess someone needs to reopen or recreate the issue

@doronnac
Copy link

@sheetalkamat From what I can tell, method override autocompletion is not a desirable feature in VSCode and Typescript. Can you please confirm?

@mhegazy
Copy link
Contributor Author

mhegazy commented Oct 16, 2017

We believe the issue has been fixed. if you are still running into issues please file a new ticket and provide enough information for us to reproduce it locally.

@ChiriVulpes
Copy link

Is this not what this issue was for? I can't find any other issues that seem to talk about what I expect to happen, that all type & visibility information would be automatically inserted

Is this a bug or missing feature? Is there already an open issue for this?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Fixed A PR has been merged for this issue Help Wanted You can do this Suggestion An idea for TypeScript VS Code Tracked There is a VS Code equivalent to this issue
Projects
None yet
Development

No branches or pull requests