Skip to content

Commit 148dc4e

Browse files
ahejlsbergahejlsberg
authored
Merge pull request #20075 from Microsoft/strictPropertyInitialization
Strict property initialization checks in classes
2 parents 40c3213 + 87a8d41 commit 148dc4e

File tree

29 files changed

+1052
-91
lines changed

29 files changed

+1052
-91
lines changed

‎src/compiler/binder.ts

+9-7
Original file line numberDiff line numberDiff line change
@@ -517,16 +517,15 @@ namespace ts {
517517
const isIIFE = containerFlags & ContainerFlags.IsFunctionExpression && !hasModifier(node, ModifierFlags.Async) && !!getImmediatelyInvokedFunctionExpression(node);
518518
// A non-async IIFE is considered part of the containing control flow. Return statements behave
519519
// similarly to break statements that exit to a label just past the statement body.
520-
if (isIIFE) {
521-
currentReturnTarget = createBranchLabel();
522-
}
523-
else {
520+
if (!isIIFE) {
524521
currentFlow = { flags: FlowFlags.Start };
525522
if (containerFlags & (ContainerFlags.IsFunctionExpression | ContainerFlags.IsObjectLiteralOrClassExpressionMethod)) {
526523
(<FlowStart>currentFlow).container = <FunctionExpression | ArrowFunction | MethodDeclaration>node;
527524
}
528-
currentReturnTarget = undefined;
529525
}
526+
// We create a return control flow graph for IIFEs and constructors. For constructors
527+
// we use the return control flow graph in strict property intialization checks.
528+
currentReturnTarget = isIIFE || node.kind === SyntaxKind.Constructor ? createBranchLabel() : undefined;
530529
currentBreakTarget = undefined;
531530
currentContinueTarget = undefined;
532531
activeLabels = undefined;
@@ -541,11 +540,14 @@ namespace ts {
541540
if (node.kind === SyntaxKind.SourceFile) {
542541
node.flags |= emitFlags;
543542
}
544-
if (isIIFE) {
543+
if (currentReturnTarget) {
545544
addAntecedent(currentReturnTarget, currentFlow);
546545
currentFlow = finishFlowLabel(currentReturnTarget);
546+
if (node.kind === SyntaxKind.Constructor) {
547+
(<ConstructorDeclaration>node).returnFlowNode = currentFlow;
548+
}
547549
}
548-
else {
550+
if (!isIIFE) {
549551
currentFlow = saveCurrentFlow;
550552
}
551553
currentBreakTarget = saveBreakTarget;

‎src/compiler/checker.ts

+84-44
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ namespace ts {
6969
const allowSyntheticDefaultImports = getAllowSyntheticDefaultImports(compilerOptions);
7070
const strictNullChecks = getStrictOptionValue(compilerOptions, "strictNullChecks");
7171
const strictFunctionTypes = getStrictOptionValue(compilerOptions, "strictFunctionTypes");
72+
const strictPropertyInitialization = getStrictOptionValue(compilerOptions, "strictPropertyInitialization");
7273
const noImplicitAny = getStrictOptionValue(compilerOptions, "noImplicitAny");
7374
const noImplicitThis = getStrictOptionValue(compilerOptions, "noImplicitThis");
7475

@@ -12283,7 +12284,7 @@ namespace ts {
1228312284
// on empty arrays are possible without implicit any errors and new element types can be inferred without
1228412285
// type mismatch errors.
1228512286
const resultType = getObjectFlags(evolvedType) & ObjectFlags.EvolvingArray && isEvolvingArrayOperationTarget(reference) ? anyArrayType : finalizeEvolvingArrayType(evolvedType);
12286-
if (reference.parent.kind === SyntaxKind.NonNullExpression && getTypeWithFacts(resultType, TypeFacts.NEUndefinedOrNull).flags & TypeFlags.Never) {
12287+
if (reference.parent && reference.parent.kind === SyntaxKind.NonNullExpression && getTypeWithFacts(resultType, TypeFacts.NEUndefinedOrNull).flags & TypeFlags.Never) {
1228712288
return declaredType;
1228812289
}
1228912290
return resultType;
@@ -15561,63 +15562,68 @@ namespace ts {
1556115562
}
1556215563

1556315564
function checkPropertyAccessExpressionOrQualifiedName(node: PropertyAccessExpression | QualifiedName, left: Expression | QualifiedName, right: Identifier) {
15564-
const type = checkNonNullExpression(left);
15565-
if (isTypeAny(type) || type === silentNeverType) {
15566-
return type;
15567-
}
15568-
15569-
const apparentType = getApparentType(getWidenedType(type));
15570-
if (apparentType === unknownType || (type.flags & TypeFlags.TypeParameter && isTypeAny(apparentType))) {
15571-
// handle cases when type is Type parameter with invalid or any constraint
15565+
let propType: Type;
15566+
const leftType = checkNonNullExpression(left);
15567+
const apparentType = getApparentType(getWidenedType(leftType));
15568+
if (isTypeAny(apparentType) || apparentType === silentNeverType) {
1557215569
return apparentType;
1557315570
}
15571+
const assignmentKind = getAssignmentTargetKind(node);
1557415572
const prop = getPropertyOfType(apparentType, right.escapedText);
1557515573
if (!prop) {
1557615574
const indexInfo = getIndexInfoOfType(apparentType, IndexKind.String);
15577-
if (indexInfo && indexInfo.type) {
15578-
if (indexInfo.isReadonly && (isAssignmentTarget(node) || isDeleteTarget(node))) {
15579-
error(node, Diagnostics.Index_signature_in_type_0_only_permits_reading, typeToString(apparentType));
15575+
if (!(indexInfo && indexInfo.type)) {
15576+
if (right.escapedText && !checkAndReportErrorForExtendingInterface(node)) {
15577+
reportNonexistentProperty(right, leftType.flags & TypeFlags.TypeParameter && (leftType as TypeParameter).isThisType ? apparentType : leftType);
1558015578
}
15581-
return getFlowTypeOfPropertyAccess(node, /*prop*/ undefined, indexInfo.type, getAssignmentTargetKind(node));
15579+
return unknownType;
1558215580
}
15583-
if (right.escapedText && !checkAndReportErrorForExtendingInterface(node)) {
15584-
reportNonexistentProperty(right, type.flags & TypeFlags.TypeParameter && (type as TypeParameter).isThisType ? apparentType : type);
15581+
if (indexInfo.isReadonly && (isAssignmentTarget(node) || isDeleteTarget(node))) {
15582+
error(node, Diagnostics.Index_signature_in_type_0_only_permits_reading, typeToString(apparentType));
1558515583
}
15586-
return unknownType;
15584+
propType = indexInfo.type;
1558715585
}
15588-
15589-
checkPropertyNotUsedBeforeDeclaration(prop, node, right);
15590-
15591-
markPropertyAsReferenced(prop, node, left.kind === SyntaxKind.ThisKeyword);
15592-
15593-
getNodeLinks(node).resolvedSymbol = prop;
15594-
15595-
checkPropertyAccessibility(node, left, apparentType, prop);
15596-
15597-
const propType = getDeclaredOrApparentType(prop, node);
15598-
const assignmentKind = getAssignmentTargetKind(node);
15599-
15600-
if (assignmentKind) {
15601-
if (isReferenceToReadonlyEntity(<Expression>node, prop) || isReferenceThroughNamespaceImport(<Expression>node)) {
15602-
error(right, Diagnostics.Cannot_assign_to_0_because_it_is_a_constant_or_a_read_only_property, idText(right));
15603-
return unknownType;
15586+
else {
15587+
checkPropertyNotUsedBeforeDeclaration(prop, node, right);
15588+
markPropertyAsReferenced(prop, node, left.kind === SyntaxKind.ThisKeyword);
15589+
getNodeLinks(node).resolvedSymbol = prop;
15590+
checkPropertyAccessibility(node, left, apparentType, prop);
15591+
if (assignmentKind) {
15592+
if (isReferenceToReadonlyEntity(<Expression>node, prop) || isReferenceThroughNamespaceImport(<Expression>node)) {
15593+
error(right, Diagnostics.Cannot_assign_to_0_because_it_is_a_constant_or_a_read_only_property, idText(right));
15594+
return unknownType;
15595+
}
1560415596
}
15597+
propType = getDeclaredOrApparentType(prop, node);
1560515598
}
15606-
return getFlowTypeOfPropertyAccess(node, prop, propType, assignmentKind);
15607-
}
15608-
15609-
/**
15610-
* Only compute control flow type if this is a property access expression that isn't an
15611-
* assignment target, and the referenced property was declared as a variable, property,
15612-
* accessor, or optional method.
15613-
*/
15614-
function getFlowTypeOfPropertyAccess(node: PropertyAccessExpression | QualifiedName, prop: Symbol | undefined, type: Type, assignmentKind: AssignmentKind) {
15599+
// Only compute control flow type if this is a property access expression that isn't an
15600+
// assignment target, and the referenced property was declared as a variable, property,
15601+
// accessor, or optional method.
1561515602
if (node.kind !== SyntaxKind.PropertyAccessExpression ||
1561615603
assignmentKind === AssignmentKind.Definite ||
15617-
prop && !(prop.flags & (SymbolFlags.Variable | SymbolFlags.Property | SymbolFlags.Accessor)) && !(prop.flags & SymbolFlags.Method && type.flags & TypeFlags.Union)) {
15618-
return type;
15604+
prop && !(prop.flags & (SymbolFlags.Variable | SymbolFlags.Property | SymbolFlags.Accessor)) && !(prop.flags & SymbolFlags.Method && propType.flags & TypeFlags.Union)) {
15605+
return propType;
15606+
}
15607+
// If strict null checks and strict property initialization checks are enabled, if we have
15608+
// a this.xxx property access, if the property is an instance property without an initializer,
15609+
// and if we are in a constructor of the same class as the property declaration, assume that
15610+
// the property is uninitialized at the top of the control flow.
15611+
let assumeUninitialized = false;
15612+
if (strictNullChecks && strictPropertyInitialization && left.kind === SyntaxKind.ThisKeyword) {
15613+
const declaration = prop && prop.valueDeclaration;
15614+
if (declaration && isInstancePropertyWithoutInitializer(declaration)) {
15615+
const flowContainer = getControlFlowContainer(node);
15616+
if (flowContainer.kind === SyntaxKind.Constructor && flowContainer.parent === declaration.parent) {
15617+
assumeUninitialized = true;
15618+
}
15619+
}
15620+
}
15621+
const flowType = getFlowTypeOfReference(node, propType, assumeUninitialized ? getOptionalType(propType) : propType);
15622+
if (assumeUninitialized && !(getFalsyFlags(propType) & TypeFlags.Undefined) && getFalsyFlags(flowType) & TypeFlags.Undefined) {
15623+
error(right, Diagnostics.Property_0_is_used_before_being_assigned, symbolToString(prop));
15624+
// Return the declared type to reduce follow-on errors
15625+
return propType;
1561915626
}
15620-
const flowType = getFlowTypeOfReference(node, type);
1562115627
return assignmentKind ? getBaseTypeOfLiteralType(flowType) : flowType;
1562215628
}
1562315629

@@ -22483,6 +22489,7 @@ namespace ts {
2248322489
if (produceDiagnostics) {
2248422490
checkIndexConstraints(type);
2248522491
checkTypeForDuplicateIndexSignatures(node);
22492+
checkPropertyInitialization(node);
2248622493
}
2248722494
}
2248822495

@@ -22639,6 +22646,39 @@ namespace ts {
2263922646
return ok;
2264022647
}
2264122648

22649+
function checkPropertyInitialization(node: ClassLikeDeclaration) {
22650+
if (!strictNullChecks || !strictPropertyInitialization || node.flags & NodeFlags.Ambient) {
22651+
return;
22652+
}
22653+
const constructor = findConstructorDeclaration(node);
22654+
for (const member of node.members) {
22655+
if (isInstancePropertyWithoutInitializer(member)) {
22656+
const propName = (<PropertyDeclaration>member).name;
22657+
if (isIdentifier(propName)) {
22658+
const type = getTypeOfSymbol(getSymbolOfNode(member));
22659+
if (!(type.flags & TypeFlags.Any || getFalsyFlags(type) & TypeFlags.Undefined)) {
22660+
if (!constructor || !isPropertyInitializedInConstructor(propName, type, constructor)) {
22661+
error(member.name, Diagnostics.Property_0_has_no_initializer_and_is_not_definitely_assigned_in_the_constructor, declarationNameToString(propName));
22662+
}
22663+
}
22664+
}
22665+
}
22666+
}
22667+
}
22668+
22669+
function isInstancePropertyWithoutInitializer(node: Node) {
22670+
return node.kind === SyntaxKind.PropertyDeclaration &&
22671+
!hasModifier(node, ModifierFlags.Static | ModifierFlags.Abstract) &&
22672+
!(<PropertyDeclaration>node).initializer;
22673+
}
22674+
22675+
function isPropertyInitializedInConstructor(propName: Identifier, propType: Type, constructor: ConstructorDeclaration) {
22676+
const reference = createPropertyAccess(createThis(), propName);
22677+
reference.flowNode = constructor.returnFlowNode;
22678+
const flowType = getFlowTypeOfReference(reference, propType, getOptionalType(propType));
22679+
return !(getFalsyFlags(flowType) & TypeFlags.Undefined);
22680+
}
22681+
2264222682
function checkInterfaceDeclaration(node: InterfaceDeclaration) {
2264322683
// Grammar checking
2264422684
if (!checkGrammarDecoratorsAndModifiers(node)) checkGrammarInterfaceDeclaration(node);

‎src/compiler/commandLineParser.ts

+7
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,13 @@ namespace ts {
277277
category: Diagnostics.Strict_Type_Checking_Options,
278278
description: Diagnostics.Enable_strict_checking_of_function_types
279279
},
280+
{
281+
name: "strictPropertyInitialization",
282+
type: "boolean",
283+
showInSimplifiedHelpView: true,
284+
category: Diagnostics.Strict_Type_Checking_Options,
285+
description: Diagnostics.Enable_strict_checking_of_property_initialization_in_classes
286+
},
280287
{
281288
name: "noImplicitThis",
282289
type: "boolean",

‎src/compiler/core.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -1967,7 +1967,7 @@ namespace ts {
19671967
: moduleKind === ModuleKind.System;
19681968
}
19691969

1970-
export type StrictOptionName = "noImplicitAny" | "noImplicitThis" | "strictNullChecks" | "strictFunctionTypes" | "alwaysStrict";
1970+
export type StrictOptionName = "noImplicitAny" | "noImplicitThis" | "strictNullChecks" | "strictFunctionTypes" | "strictPropertyInitialization" | "alwaysStrict";
19711971

19721972
export function getStrictOptionValue(compilerOptions: CompilerOptions, flag: StrictOptionName): boolean {
19731973
return compilerOptions[flag] === undefined ? compilerOptions.strict : compilerOptions[flag];

‎src/compiler/diagnosticMessages.json

+12
Original file line numberDiff line numberDiff line change
@@ -1952,6 +1952,14 @@
19521952
"category": "Error",
19531953
"code": 2563
19541954
},
1955+
"Property '{0}' has no initializer and is not definitely assigned in the constructor.": {
1956+
"category": "Error",
1957+
"code": 2564
1958+
},
1959+
"Property '{0}' is used before being assigned.": {
1960+
"category": "Error",
1961+
"code": 2565
1962+
},
19551963
"JSX element attributes type '{0}' may not be a union type.": {
19561964
"category": "Error",
19571965
"code": 2600
@@ -3391,6 +3399,10 @@
33913399
"category": "Message",
33923400
"code": 6186
33933401
},
3402+
"Enable strict checking of property initialization in classes.": {
3403+
"category": "Message",
3404+
"code": 6187
3405+
},
33943406
"Variable '{0}' implicitly has an '{1}' type.": {
33953407
"category": "Error",
33963408
"code": 7005

‎src/compiler/types.ts

+2
Original file line numberDiff line numberDiff line change
@@ -947,6 +947,7 @@ namespace ts {
947947
kind: SyntaxKind.Constructor;
948948
parent?: ClassDeclaration | ClassExpression;
949949
body?: FunctionBody;
950+
/* @internal */ returnFlowNode?: FlowNode;
950951
}
951952

952953
/** For when we encounter a semicolon in a class declaration. ES6 allows these as class elements. */
@@ -3854,6 +3855,7 @@ namespace ts {
38543855
strict?: boolean;
38553856
strictFunctionTypes?: boolean; // Always combine with strict property
38563857
strictNullChecks?: boolean; // Always combine with strict property
3858+
strictPropertyInitialization?: boolean; // Always combine with strict property
38573859
/* @internal */ stripInternal?: boolean;
38583860
suppressExcessPropertyErrors?: boolean;
38593861
suppressImplicitAnyIndexErrors?: boolean;

‎tests/baselines/reference/api/tsserverlibrary.d.ts

+1
Original file line numberDiff line numberDiff line change
@@ -2287,6 +2287,7 @@ declare namespace ts {
22872287
strict?: boolean;
22882288
strictFunctionTypes?: boolean;
22892289
strictNullChecks?: boolean;
2290+
strictPropertyInitialization?: boolean;
22902291
suppressExcessPropertyErrors?: boolean;
22912292
suppressImplicitAnyIndexErrors?: boolean;
22922293
target?: ScriptTarget;

‎tests/baselines/reference/api/typescript.d.ts

+1
Original file line numberDiff line numberDiff line change
@@ -2287,6 +2287,7 @@ declare namespace ts {
22872287
strict?: boolean;
22882288
strictFunctionTypes?: boolean;
22892289
strictNullChecks?: boolean;
2290+
strictPropertyInitialization?: boolean;
22902291
suppressExcessPropertyErrors?: boolean;
22912292
suppressImplicitAnyIndexErrors?: boolean;
22922293
target?: ScriptTarget;

‎tests/baselines/reference/indexedAccessTypeConstraints.js

+3-2
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ type Data<T> = {
1010
};
1111

1212
class Parent<M> {
13-
private data: Data<M>;
13+
constructor(private data: Data<M>) {}
1414
getData(): Data<M> {
1515
return this.data;
1616
}
@@ -50,7 +50,8 @@ var __extends = (this && this.__extends) || (function () {
5050
})();
5151
exports.__esModule = true;
5252
var Parent = /** @class */ (function () {
53-
function Parent() {
53+
function Parent(data) {
54+
this.data = data;
5455
}
5556
Parent.prototype.getData = function () {
5657
return this.data;

‎tests/baselines/reference/indexedAccessTypeConstraints.symbols

+9-9
Original file line numberDiff line numberDiff line change
@@ -29,20 +29,20 @@ class Parent<M> {
2929
>Parent : Symbol(Parent, Decl(indexedAccessTypeConstraints.ts, 8, 2))
3030
>M : Symbol(M, Decl(indexedAccessTypeConstraints.ts, 10, 13))
3131

32-
private data: Data<M>;
33-
>data : Symbol(Parent.data, Decl(indexedAccessTypeConstraints.ts, 10, 17))
32+
constructor(private data: Data<M>) {}
33+
>data : Symbol(Parent.data, Decl(indexedAccessTypeConstraints.ts, 11, 16))
3434
>Data : Symbol(Data, Decl(indexedAccessTypeConstraints.ts, 4, 1))
3535
>M : Symbol(M, Decl(indexedAccessTypeConstraints.ts, 10, 13))
3636

3737
getData(): Data<M> {
38-
>getData : Symbol(Parent.getData, Decl(indexedAccessTypeConstraints.ts, 11, 26))
38+
>getData : Symbol(Parent.getData, Decl(indexedAccessTypeConstraints.ts, 11, 41))
3939
>Data : Symbol(Data, Decl(indexedAccessTypeConstraints.ts, 4, 1))
4040
>M : Symbol(M, Decl(indexedAccessTypeConstraints.ts, 10, 13))
4141

4242
return this.data;
43-
>this.data : Symbol(Parent.data, Decl(indexedAccessTypeConstraints.ts, 10, 17))
43+
>this.data : Symbol(Parent.data, Decl(indexedAccessTypeConstraints.ts, 11, 16))
4444
>this : Symbol(Parent, Decl(indexedAccessTypeConstraints.ts, 8, 2))
45-
>data : Symbol(Parent.data, Decl(indexedAccessTypeConstraints.ts, 10, 17))
45+
>data : Symbol(Parent.data, Decl(indexedAccessTypeConstraints.ts, 11, 16))
4646
}
4747
}
4848

@@ -59,9 +59,9 @@ export class Foo<C> extends Parent<IData<C>> {
5959

6060
return this.getData().get('content');
6161
>this.getData().get : Symbol(get, Decl(indexedAccessTypeConstraints.ts, 6, 16))
62-
>this.getData : Symbol(Parent.getData, Decl(indexedAccessTypeConstraints.ts, 11, 26))
62+
>this.getData : Symbol(Parent.getData, Decl(indexedAccessTypeConstraints.ts, 11, 41))
6363
>this : Symbol(Foo, Decl(indexedAccessTypeConstraints.ts, 15, 1))
64-
>getData : Symbol(Parent.getData, Decl(indexedAccessTypeConstraints.ts, 11, 26))
64+
>getData : Symbol(Parent.getData, Decl(indexedAccessTypeConstraints.ts, 11, 41))
6565
>get : Symbol(get, Decl(indexedAccessTypeConstraints.ts, 6, 16))
6666
}
6767
}
@@ -81,9 +81,9 @@ export class Bar<C, T extends IData<C>> extends Parent<T> {
8181

8282
return this.getData().get('content');
8383
>this.getData().get : Symbol(get, Decl(indexedAccessTypeConstraints.ts, 6, 16))
84-
>this.getData : Symbol(Parent.getData, Decl(indexedAccessTypeConstraints.ts, 11, 26))
84+
>this.getData : Symbol(Parent.getData, Decl(indexedAccessTypeConstraints.ts, 11, 41))
8585
>this : Symbol(Bar, Decl(indexedAccessTypeConstraints.ts, 21, 1))
86-
>getData : Symbol(Parent.getData, Decl(indexedAccessTypeConstraints.ts, 11, 26))
86+
>getData : Symbol(Parent.getData, Decl(indexedAccessTypeConstraints.ts, 11, 41))
8787
>get : Symbol(get, Decl(indexedAccessTypeConstraints.ts, 6, 16))
8888
}
8989
}

0 commit comments

Comments
 (0)