Skip to content

Fixes type assertion argument in type predicate functions bug. #3814

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

Closed

Conversation

tinganho
Copy link
Contributor

Ref: #3812.

@tinganho tinganho force-pushed the typePredicateAssignmentBug branch from 50c2610 to 9c443aa Compare July 10, 2015 18:08
else if (arg.kind === SyntaxKind.AsExpression || arg.kind === SyntaxKind.TypeAssertionExpression) {
if ((<AssertionExpression>arg).expression) {
if ((<AssertionExpression>arg).expression.kind === SyntaxKind.Identifier &&
getSymbolAtLocation((<AssertionExpression>arg).expression) === symbol) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the same check as above, it's just doing the check on a different expression. I would instead structure it like this:

let arg = expr.arguments[signature.typePredicate.parameterIndex];
if (arg.kind === SyntaxKind.AsExpression || arg.kind === SyntaxKind.TypeAssertionExpression) {
    arg = (<AssertionExpression>arg).expression;
}
if (arg.kind === SyntaxKind.Identifier && getSymbolAtLocation(arg) === symbol) {
    if (!assumeTrue) ...
}

That way you do not need the narrowType inner function

@JsonFreeman
Copy link
Contributor

Something I don't get here. Why would you call a type predicate function and cast its input? Presumably if you are casting, you know what type it is already.

@tinganho
Copy link
Contributor Author

I was probably to fast on making a fix without even thinking about it. It doesn't make any sense at all.

@tinganho tinganho closed this Jul 10, 2015
@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.

3 participants