-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Contextually type implemented properties #6118
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
Changes from 6 commits
c63b031
f81cac5
c097494
6388186
6bf5c3f
055ae8c
52d1be1
e70dd7f
4523557
16a3c00
bed2c6c
6e25554
dec4d6d
8f79de1
8ac5082
635471d
c1d0d18
872f848
526a09b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2595,7 +2595,14 @@ namespace ts { | |
|
||
// Use the type of the initializer expression if one is present | ||
if (declaration.initializer) { | ||
return checkExpressionCached(declaration.initializer); | ||
let mapper: TypeMapper; | ||
if (declaration.kind === SyntaxKind.PropertyDeclaration) { | ||
const type = getTypeOfBasePropertyDeclaration(<PropertyDeclaration>declaration); | ||
if (type) { | ||
mapper = createTypeMapper([undefinedType, nullType], [type, type]); | ||
} | ||
} | ||
return checkExpressionCached(declaration.initializer, mapper); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
} | ||
|
||
// If it is a short-hand property assignment, use the type of the identifier | ||
|
@@ -2895,6 +2902,40 @@ namespace ts { | |
return unknownType; | ||
} | ||
|
||
function getTypeOfBasePropertyDeclaration(declaration: PropertyDeclaration) { | ||
if (declaration.parent.kind === SyntaxKind.ClassDeclaration) { | ||
const property = getPropertyOfBaseTypeDeclaration(<ClassLikeDeclaration>declaration.parent, declaration.symbol.name); | ||
if (property) { | ||
return getTypeOfSymbol(property); | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you use an explicit return? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
} | ||
|
||
function getPropertyOfBaseTypeDeclaration(declaration: ClassLikeDeclaration, propertyName: string): Symbol { | ||
const property = getFirstPropertyOfTypes(getBaseTypes(<InterfaceType>getTypeOfSymbol(getSymbolOfNode(declaration))), propertyName); | ||
if (property) { | ||
return property; | ||
} | ||
const implementedTypeNodes = getClassImplementsHeritageClauseElements(declaration); | ||
if (implementedTypeNodes) { | ||
return getFirstPropertyOfTypes(map(implementedTypeNodes, getTypeFromTypeReference), propertyName); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here and below; explicit return There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
} | ||
|
||
function getFirstPropertyOfTypes(types: Type[], propertyName: string) { | ||
for (const t of types) { | ||
if (t !== unknownType) { | ||
const property = getPropertyOfType(t, propertyName); | ||
if (!property || property.valueDeclaration.flags & NodeFlags.Private) { | ||
continue; | ||
} | ||
if (property.name === propertyName) { | ||
return property; | ||
} | ||
} | ||
} | ||
} | ||
|
||
function getTargetType(type: ObjectType): Type { | ||
return type.flags & TypeFlags.Reference ? (<TypeReference>type).target : type; | ||
} | ||
|
@@ -4933,7 +4974,7 @@ namespace ts { | |
// Returns true if the given expression contains (at any level of nesting) a function or arrow expression | ||
// that is subject to contextual typing. | ||
function isContextSensitive(node: Expression | MethodDeclaration | ObjectLiteralElement): boolean { | ||
Debug.assert(node.kind !== SyntaxKind.MethodDeclaration || isObjectLiteralMethod(node)); | ||
Debug.assert(node.kind !== SyntaxKind.MethodDeclaration || isMethod(node)); | ||
switch (node.kind) { | ||
case SyntaxKind.FunctionExpression: | ||
case SyntaxKind.ArrowFunction: | ||
|
@@ -6894,7 +6935,7 @@ namespace ts { | |
return expression; | ||
} | ||
|
||
function checkIdentifier(node: Identifier): Type { | ||
function checkIdentifier(node: Identifier, contextualMapper?: TypeMapper): Type { | ||
const symbol = getResolvedSymbol(node); | ||
|
||
// As noted in ECMAScript 6 language spec, arrow functions never have an arguments objects. | ||
|
@@ -6925,7 +6966,11 @@ namespace ts { | |
checkCollisionWithCapturedThisVariable(node, node); | ||
checkBlockScopedBindingCapturedInLoop(node, symbol); | ||
|
||
return getNarrowedTypeOfSymbol(getExportSymbolOfValueSymbolIfExported(symbol), node); | ||
const type = getNarrowedTypeOfSymbol(getExportSymbolOfValueSymbolIfExported(symbol), node); | ||
if (type === undefinedType || type == nullType) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This checks for the undefined type instead of the symbol with the name "undefined". I think this is wrong, so I changed it to |
||
return (contextualMapper || identityMapper)(type); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ahejlsberg can correct me, but I think the contextualMapper is just for inferential typing. |
||
} | ||
return type; | ||
} | ||
|
||
function isInsideFunction(node: Node, threshold: Node): boolean { | ||
|
@@ -7170,10 +7215,14 @@ namespace ts { | |
} | ||
} | ||
|
||
function checkNullKeyword(nullNode: Node, contextualMapper: TypeMapper) { | ||
return (contextualMapper || identityMapper)(nullType); | ||
} | ||
|
||
// Return contextual type of parameter or undefined if no contextual type is available | ||
function getContextuallyTypedParameterType(parameter: ParameterDeclaration): Type { | ||
const func = parameter.parent; | ||
if (isFunctionExpressionOrArrowFunction(func) || isObjectLiteralMethod(func)) { | ||
if (isFunctionExpressionOrArrowFunction(func) || isMethod(func)) { | ||
if (isContextSensitive(func)) { | ||
const contextualSignature = getContextualSignature(func); | ||
if (contextualSignature) { | ||
|
@@ -7214,6 +7263,12 @@ namespace ts { | |
return type; | ||
} | ||
} | ||
if (declaration.kind === SyntaxKind.PropertyDeclaration) { | ||
const type = getTypeOfBasePropertyDeclaration(<PropertyDeclaration>declaration); | ||
if (type) { | ||
return type; | ||
} | ||
} | ||
if (isBindingPattern(declaration.name)) { | ||
return getTypeFromBindingPattern(<BindingPattern>declaration.name, /*includePatternInType*/ true); | ||
} | ||
|
@@ -7544,10 +7599,18 @@ namespace ts { | |
// all identical ignoring their return type, the result is same signature but with return type as | ||
// union type of return types from these signatures | ||
function getContextualSignature(node: FunctionExpression | MethodDeclaration): Signature { | ||
Debug.assert(node.kind !== SyntaxKind.MethodDeclaration || isObjectLiteralMethod(node)); | ||
const type = isObjectLiteralMethod(node) | ||
? getContextualTypeForObjectLiteralMethod(node) | ||
: getApparentTypeOfContextualType(node); | ||
Debug.assert(node.kind !== SyntaxKind.MethodDeclaration || isMethod(node)); | ||
let type: Type; | ||
if (isFunctionExpressionOrArrowFunction(node)) { | ||
type = getApparentTypeOfContextualType(node); | ||
} | ||
else if (isObjectLiteralMethod(node)) { | ||
type = getContextualTypeForObjectLiteralMethod(node); | ||
} | ||
else if (isMethod(node)) { | ||
type = getTypeOfBasePropertyDeclaration(node); | ||
} | ||
|
||
if (!type) { | ||
return undefined; | ||
} | ||
|
@@ -7640,7 +7703,7 @@ namespace ts { | |
function checkArrayLiteral(node: ArrayLiteralExpression, contextualMapper?: TypeMapper): Type { | ||
const elements = node.elements; | ||
let hasSpreadElement = false; | ||
const elementTypes: Type[] = []; | ||
let elementTypes: Type[] = []; | ||
const inDestructuringPattern = isAssignmentTarget(node); | ||
for (const e of elements) { | ||
if (inDestructuringPattern && e.kind === SyntaxKind.SpreadElementExpression) { | ||
|
@@ -7702,7 +7765,15 @@ namespace ts { | |
} | ||
} | ||
} | ||
return createArrayType(elementTypes.length ? getUnionType(elementTypes) : undefinedType); | ||
if (!elementTypes.length) { | ||
const mapper = contextualMapper || identityMapper; | ||
const mappedType = mapper(undefinedType); | ||
if (mappedType === undefinedType) { | ||
return createArrayType(undefinedType); | ||
} | ||
elementTypes = (<TypeReference>mappedType).typeArguments; | ||
} | ||
return createArrayType(getUnionType(elementTypes)); | ||
} | ||
|
||
function isNumericName(name: DeclarationName): boolean { | ||
|
@@ -7762,7 +7833,7 @@ namespace ts { | |
return links.resolvedType; | ||
} | ||
|
||
function checkObjectLiteral(node: ObjectLiteralExpression, contextualMapper?: TypeMapper): Type { | ||
function checkObjectLiteral(node: ObjectLiteralExpression, contextualMapper: TypeMapper): Type { | ||
const inDestructuringPattern = isAssignmentTarget(node); | ||
// Grammar checking | ||
checkGrammarObjectLiteralExpression(node, inDestructuringPattern); | ||
|
@@ -7885,7 +7956,21 @@ namespace ts { | |
} | ||
} | ||
} | ||
const result = propTypes.length ? getUnionType(propTypes) : undefinedType; | ||
let result: Type; | ||
if (!propTypes.length) { | ||
const mapper = contextualMapper || identityMapper; | ||
const mappedType = mapper(undefinedType); | ||
if (mappedType === undefinedType) { | ||
result = undefinedType; | ||
} | ||
else { | ||
const resolvedType = <ResolvedType>mappedType; | ||
result = kind === IndexKind.String ? resolvedType.stringIndexType : resolvedType.numberIndexType; | ||
} | ||
} | ||
else { | ||
result = getUnionType(propTypes); | ||
} | ||
typeFlags |= result.flags; | ||
return result; | ||
} | ||
|
@@ -10862,7 +10947,7 @@ namespace ts { | |
return checkExpression((<PropertyAssignment>node).initializer, contextualMapper); | ||
} | ||
|
||
function checkObjectLiteralMethod(node: MethodDeclaration, contextualMapper?: TypeMapper): Type { | ||
function checkObjectLiteralMethod(node: MethodDeclaration, contextualMapper: TypeMapper): Type { | ||
// Grammar checking | ||
checkGrammarMethod(node); | ||
|
||
|
@@ -10937,13 +11022,13 @@ namespace ts { | |
function checkExpressionWorker(node: Expression, contextualMapper: TypeMapper): Type { | ||
switch (node.kind) { | ||
case SyntaxKind.Identifier: | ||
return checkIdentifier(<Identifier>node); | ||
return checkIdentifier(<Identifier>node, contextualMapper); | ||
case SyntaxKind.ThisKeyword: | ||
return checkThisExpression(node); | ||
case SyntaxKind.SuperKeyword: | ||
return checkSuperExpression(node); | ||
case SyntaxKind.NullKeyword: | ||
return nullType; | ||
return checkNullKeyword(node, contextualMapper); | ||
case SyntaxKind.TrueKeyword: | ||
case SyntaxKind.FalseKeyword: | ||
return booleanType; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
tests/cases/conformance/expressions/contextualTyping/implementedPropertyContextualTyping1.ts(21,3): error TS2322: Type 'number' is not assignable to type 'string'. | ||
tests/cases/conformance/expressions/contextualTyping/implementedPropertyContextualTyping1.ts(24,3): error TS2322: Type 'number' is not assignable to type 'string'. | ||
tests/cases/conformance/expressions/contextualTyping/implementedPropertyContextualTyping1.ts(28,3): error TS2322: Type 'number' is not assignable to type 'string'. | ||
tests/cases/conformance/expressions/contextualTyping/implementedPropertyContextualTyping1.ts(31,3): error TS2322: Type 'number' is not assignable to type 'string'. | ||
|
||
|
||
==== tests/cases/conformance/expressions/contextualTyping/implementedPropertyContextualTyping1.ts (4 errors) ==== | ||
interface Event { | ||
time: number; | ||
} | ||
interface Base { | ||
superHandle: (e: Event) => number; | ||
} | ||
interface Listener extends Base { | ||
handle: (e: Event) => void; | ||
} | ||
interface Ringer { | ||
ring: (times: number) => void; | ||
} | ||
|
||
abstract class Watcher { | ||
abstract watch(e: Event): number; | ||
} | ||
|
||
class Alarm extends Watcher implements Listener, Ringer { | ||
str: string; | ||
handle = e => { | ||
this.str = e.time; // error | ||
~~~~~~~~ | ||
!!! error TS2322: Type 'number' is not assignable to type 'string'. | ||
} | ||
superHandle = e => { | ||
this.str = e.time; // error | ||
~~~~~~~~ | ||
!!! error TS2322: Type 'number' is not assignable to type 'string'. | ||
return e.time; | ||
} | ||
ring(times) { | ||
this.str = times; // error | ||
~~~~~~~~ | ||
!!! error TS2322: Type 'number' is not assignable to type 'string'. | ||
} | ||
watch(e) { | ||
this.str = e.time; // error | ||
~~~~~~~~ | ||
!!! error TS2322: Type 'number' is not assignable to type 'string'. | ||
return e.time; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
//// [implementedPropertyContextualTyping1.ts] | ||
interface Event { | ||
time: number; | ||
} | ||
interface Base { | ||
superHandle: (e: Event) => number; | ||
} | ||
interface Listener extends Base { | ||
handle: (e: Event) => void; | ||
} | ||
interface Ringer { | ||
ring: (times: number) => void; | ||
} | ||
|
||
abstract class Watcher { | ||
abstract watch(e: Event): number; | ||
} | ||
|
||
class Alarm extends Watcher implements Listener, Ringer { | ||
str: string; | ||
handle = e => { | ||
this.str = e.time; // error | ||
} | ||
superHandle = e => { | ||
this.str = e.time; // error | ||
return e.time; | ||
} | ||
ring(times) { | ||
this.str = times; // error | ||
} | ||
watch(e) { | ||
this.str = e.time; // error | ||
return e.time; | ||
} | ||
} | ||
|
||
//// [implementedPropertyContextualTyping1.js] | ||
var __extends = (this && this.__extends) || function (d, b) { | ||
for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p]; | ||
function __() { this.constructor = d; } | ||
d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __()); | ||
}; | ||
var Watcher = (function () { | ||
function Watcher() { | ||
} | ||
return Watcher; | ||
}()); | ||
var Alarm = (function (_super) { | ||
__extends(Alarm, _super); | ||
function Alarm() { | ||
var _this = this; | ||
_super.apply(this, arguments); | ||
this.handle = function (e) { | ||
_this.str = e.time; // error | ||
}; | ||
this.superHandle = function (e) { | ||
_this.str = e.time; // error | ||
return e.time; | ||
}; | ||
} | ||
Alarm.prototype.ring = function (times) { | ||
this.str = times; // error | ||
}; | ||
Alarm.prototype.watch = function (e) { | ||
this.str = e.time; // error | ||
return e.time; | ||
}; | ||
return Alarm; | ||
}(Watcher)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the first argument never changes, you could factor it out to a constant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not so sure this works - doesn't this map all undefined and null types in the expression? So if I have
won't
x
inDerived
get the type(x: string) => void
meaninga
will have the type(x: (x: string) => void) => void
.Maybe I misunderstood.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
Also, I think the TypeMappers are supposed to be for generics.