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

Allow structural reference-finding in the language service #6388

Closed
DanielRosenwasser opened this issue Jan 7, 2016 · 5 comments
Closed
Labels
Suggestion An idea for TypeScript Visual Studio Integration with Visual Studio

Comments

@DanielRosenwasser
Copy link
Member

Consider the following:

interface A {
    a: string;
    b: number;
}

declare function f(x: A): void;

var v = { a: "hello", b: 10 };
f(v);

This code works because { a: string; b: number } is structurally compatible with A.

Imagine that a user tries to rename a in either the interface or the object literal. Right now, renaming either one will cause an error, because the two are not considered the same property in their containing type. This leads to an unpleasant refactoring experience, because code actually stops working correctly.

We should consider fixing this by testing whether the containing type is structurally compatible.

This change should definitely affect:

  • Finding references
  • Finding rename locations
  • Finding document highlight spans
@DanielRosenwasser DanielRosenwasser added the Bug A bug in TypeScript label Jan 7, 2016
@mhegazy mhegazy added Suggestion An idea for TypeScript and removed Bug A bug in TypeScript labels Jan 7, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Jan 7, 2016

The scope of this change is more than a bug fix. marking it as a suggestion. there is also a VS/Editor aspect here as there is a new UI/option to be provided to the user at rename time.

@DanielRosenwasser DanielRosenwasser added the Visual Studio Integration with Visual Studio label Jan 7, 2016
@DanielRosenwasser
Copy link
Member Author

Note to implementors: a basic, but useful, useful test case would be

interface A {
    a: string;
}
function f(): Promise<A> {
    return Promise.resolve({
        a: '',
    });
}

taken from #6244.

@zpdDG4gta8XKpMCd
Copy link

I think it should stay as it is now. v is of a literal type that happened to be very similar to the interface, that's it, they are not related.

Correct me if I am wrong, your latter example would work aout of the box, had TypeScript crave the type out of the context which it doesn't currently do. You would've killed many birds with a single shot if it did.

@DanielRosenwasser
Copy link
Member Author

The proposal is an option to look for compatible types when performing a rename/search, so this wouldn't affect you if you didn't want it to.

It depends what you mean by "crave", because what it comes down to in our current algorithms is stepping through levels of type argument inference and contextual typing. If there's a solid and general idea about figuring that out, it'd be worth considering.

@RyanCavanaugh
Copy link
Member

Closing due to lack of additional feedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Suggestion An idea for TypeScript Visual Studio Integration with Visual Studio
Projects
None yet
Development

No branches or pull requests

4 participants