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.

Sign up for GitHub

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
wants to merge 3 commits into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9c443aa
Fixes type assertion argument in type predicate functions bug
tinganho Jul 10, 2015
c47f391
Accept baselines
tinganho Jul 10, 2015
0df07e0
Remove todo
tinganho Jul 10, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 37 additions & 21 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5998,20 +5998,37 @@ namespace ts {
if (type.flags & TypeFlags.Any) {
return type;
}

let signature = getResolvedSignature(expr);

if (signature.typePredicate &&
expr.arguments[signature.typePredicate.parameterIndex] &&
getSymbolAtLocation(expr.arguments[signature.typePredicate.parameterIndex]) === symbol) {
expr.arguments[signature.typePredicate.parameterIndex]) {

let arg = expr.arguments[signature.typePredicate.parameterIndex];
if (arg.kind === SyntaxKind.Identifier && getSymbolAtLocation(arg) === symbol) {
return narrowType();
}
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


if (!assumeTrue) {
if (type.flags & TypeFlags.Union) {
return getUnionType(filter((<UnionType>type).types, t => !isTypeSubtypeOf(t, signature.typePredicate.type)));
return narrowType();
}
}
return type;
}
return getNarrowedType(type, signature.typePredicate.type);

function narrowType(): Type {
if (!assumeTrue) {
if (type.flags & TypeFlags.Union) {
return getUnionType(filter((<UnionType>type).types, t => !isTypeSubtypeOf(t, signature.typePredicate.type)));
}
return type;
}
return getNarrowedType(type, signature.typePredicate.type);
}
}

return type;
}

Expand Down Expand Up @@ -6765,7 +6782,7 @@ namespace ts {
let restArrayType = checkExpression((<SpreadElementExpression>e).expression, contextualMapper);
let restElementType = getIndexTypeOfType(restArrayType, IndexKind.Number) ||
(languageVersion >= ScriptTarget.ES6 ? getElementTypeOfIterable(restArrayType, /*errorNode*/ undefined) : undefined);
if (restElementType) {
if (restElementType) {
elementTypes.push(restElementType);
}
}
Expand Down Expand Up @@ -8012,8 +8029,7 @@ namespace ts {
return args;
}


/**
/**
* Returns the effective argument count for a node that works like a function invocation.
* If 'node' is a Decorator, the number of arguments is derived from the decoration
* target and the signature:
Expand Down Expand Up @@ -8090,11 +8106,11 @@ namespace ts {
let classSymbol = getSymbolOfNode(node);
return getTypeOfSymbol(classSymbol);
}

// fall-through

case SyntaxKind.PropertyDeclaration:
case SyntaxKind.MethodDeclaration:
case SyntaxKind.MethodDeclaration:
case SyntaxKind.GetAccessor:
case SyntaxKind.SetAccessor:
// For a property or method decorator, the `target` is the
Expand Down Expand Up @@ -8137,14 +8153,14 @@ namespace ts {
// For a constructor parameter decorator, the `propertyKey` will be `undefined`.
return anyType;
}

// For a non-constructor parameter decorator, the `propertyKey` will be either
// a string or a symbol, based on the name of the parameter's containing method.

// fall-through

case SyntaxKind.PropertyDeclaration:
case SyntaxKind.MethodDeclaration:
case SyntaxKind.MethodDeclaration:
case SyntaxKind.GetAccessor:
case SyntaxKind.SetAccessor:
// The `propertyKey` for a property or method decorator will be a
Expand Down Expand Up @@ -8172,8 +8188,8 @@ namespace ts {
return unknownType;
}


default:
default:
Debug.fail("Unsupported decorator target.");
return unknownType;
}
Expand Down Expand Up @@ -11459,19 +11475,19 @@ namespace ts {
forEach(node.declarationList.declarations, checkSourceElement);
}

function checkGrammarDisallowedModifiersOnObjectLiteralExpressionMethod(node: Node) {
function checkGrammarDisallowedModifiersOnObjectLiteralExpressionMethod(node: Node) {
// We only disallow modifier on a method declaration if it is a property of object-literal-expression
if (node.modifiers && node.parent.kind === SyntaxKind.ObjectLiteralExpression){
if (isAsyncFunctionLike(node)) {
if (node.modifiers.length > 1) {
return grammarErrorOnFirstToken(node, Diagnostics.Modifiers_cannot_appear_here);
return grammarErrorOnFirstToken(node, Diagnostics.Modifiers_cannot_appear_here);
}
}
else {
return grammarErrorOnFirstToken(node, Diagnostics.Modifiers_cannot_appear_here);
}
}
}
}

function checkExpressionStatement(node: ExpressionStatement) {
// Grammar checking
Expand Down Expand Up @@ -14501,7 +14517,7 @@ namespace ts {
}
flags |= NodeFlags.Ambient;
lastDeclare = modifier;
break;
break;

case SyntaxKind.AbstractKeyword:
if (flags & NodeFlags.Abstract) {
Expand Down Expand Up @@ -14537,7 +14553,7 @@ namespace ts {
}
flags |= NodeFlags.Async;
lastAsync = modifier;
break;
break;
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -847,8 +847,8 @@ namespace ts {
}

export interface TypeAssertion extends UnaryExpression {
type: TypeNode;
expression: UnaryExpression;
type: TypeNode;
}

export type AssertionExpression = TypeAssertion | AsExpression;
Expand Down Expand Up @@ -1755,10 +1755,10 @@ namespace ts {
StringLike = String | StringLiteral,
NumberLike = Number | Enum,
ObjectType = Class | Interface | Reference | Tuple | Anonymous,
UnionOrIntersection = Union | Intersection,
UnionOrIntersection = Union | Intersection,
StructuredType = ObjectType | Union | Intersection,
/* @internal */
RequiresWidening = ContainsUndefinedOrNull | ContainsObjectLiteral
RequiresWidening = ContainsUndefinedOrNull | ContainsObjectLiteral
}

// Properties common to all types
Expand Down
47 changes: 0 additions & 47 deletions tests/baselines/reference/tsxAttributeResolution9.symbols
View file
Open in desktop

This file was deleted.

49 changes: 0 additions & 49 deletions tests/baselines/reference/tsxAttributeResolution9.types
View file
Open in desktop

This file was deleted.

18 changes: 17 additions & 1 deletion tests/baselines/reference/typeGuardFunction.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@ if(isA(union)) {
union.propA;
}

// Type assertions
if (isA(<A>union)) {
a = union;
}
if (isA(union as A)) {
a = union;
}

// Call signature
interface I1 {
(p1: A): p1 is C;
Expand Down Expand Up @@ -81,7 +89,8 @@ acceptingTypeGuardFunction(isA);

// Binary expressions
let union2: C | B;
let union3: boolean | B = isA(union2) || union2;
let union3: boolean | B = isA(union2) || union2;


//// [typeGuardFunction.js]
var __extends = (this && this.__extends) || function (d, b) {
Expand Down Expand Up @@ -122,6 +131,13 @@ var union;
if (isA(union)) {
union.propA;
}
// Type assertions
if (isA(union)) {
a = union;
}
if (isA(union)) {
a = union;
}
if (isC_multipleParams(a, 0)) {
a.propC;
}
Expand Down
Loading