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

Missing property fix #14097

Merged
merged 8 commits into from
Feb 16, 2017
Merged

Missing property fix #14097

merged 8 commits into from
Feb 16, 2017

Conversation

aozgaa
Copy link
Contributor

@aozgaa aozgaa commented Feb 15, 2017

Implements #13071.

Note that there is considerably more work to do for stubbing out missing methods if we want to do any type inference on the parameters. There are additionally issues with inferring type parameters for the stubbed method from its type arguments.

Thoughts @mhegazy @billti @DanielRosenwasser ?

const binaryExpression = token.parent.parent as BinaryExpression;

const checker = context.program.getTypeChecker();
const widenedType = checker.getBaseTypeOfLiteralType(checker.getTypeAtLocation(binaryExpression.right));
Copy link
Contributor

Choose a reason for hiding this comment

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

you also want to widen it. i.e. null => any

Copy link
Contributor

@mhegazy mhegazy left a comment

Choose a reason for hiding this comment

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

Looks good. just add the getWidenedType call

}]
},
{
description: formatStringFromArgs(getLocaleSpecificMessage(Diagnostics.Add_index_accessor_for_missing_property_0), [token.getText()]),
Copy link
Member

@DanielRosenwasser DanielRosenwasser Feb 16, 2017

Choose a reason for hiding this comment

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

  1. "index accessor" -> "index signature"
  2. You either need a test for this fix, or you should get rid of it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed and test added

@mhegazy
Copy link
Contributor

mhegazy commented Feb 16, 2017

@aozgaa after merging in master please port this to release-2.2

@aozgaa aozgaa merged commit 8a5bebe into microsoft:master Feb 16, 2017
@aozgaa aozgaa deleted the MissingPropertyFix branch June 6, 2017 17:56
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
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.

4 participants