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

Strict property initialization checks in classes #20075

Merged
merged 19 commits into from
Nov 20, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 9 additions & 7 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -517,16 +517,15 @@ namespace ts {
const isIIFE = containerFlags & ContainerFlags.IsFunctionExpression && !hasModifier(node, ModifierFlags.Async) && !!getImmediatelyInvokedFunctionExpression(node);
// A non-async IIFE is considered part of the containing control flow. Return statements behave
// similarly to break statements that exit to a label just past the statement body.
if (isIIFE) {
currentReturnTarget = createBranchLabel();
}
else {
if (!isIIFE) {
currentFlow = { flags: FlowFlags.Start };
if (containerFlags & (ContainerFlags.IsFunctionExpression | ContainerFlags.IsObjectLiteralOrClassExpressionMethod)) {
(<FlowStart>currentFlow).container = <FunctionExpression | ArrowFunction | MethodDeclaration>node;
}
currentReturnTarget = undefined;
}
// We create a return control flow graph for IIFEs and constructors. For constructors
// we use the return control flow graph in strict property intialization checks.
currentReturnTarget = isIIFE || node.kind === SyntaxKind.Constructor ? createBranchLabel() : undefined;
currentBreakTarget = undefined;
currentContinueTarget = undefined;
activeLabels = undefined;
Expand All @@ -541,11 +540,14 @@ namespace ts {
if (node.kind === SyntaxKind.SourceFile) {
node.flags |= emitFlags;
}
if (isIIFE) {
if (currentReturnTarget) {
addAntecedent(currentReturnTarget, currentFlow);
currentFlow = finishFlowLabel(currentReturnTarget);
if (node.kind === SyntaxKind.Constructor) {
(<ConstructorDeclaration>node).returnFlowNode = currentFlow;
}
}
else {
if (!isIIFE) {
currentFlow = saveCurrentFlow;
}
currentBreakTarget = saveBreakTarget;
Expand Down
128 changes: 84 additions & 44 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ namespace ts {
const allowSyntheticDefaultImports = getAllowSyntheticDefaultImports(compilerOptions);
const strictNullChecks = getStrictOptionValue(compilerOptions, "strictNullChecks");
const strictFunctionTypes = getStrictOptionValue(compilerOptions, "strictFunctionTypes");
const strictPropertyInitialization = getStrictOptionValue(compilerOptions, "strictPropertyInitialization");
const noImplicitAny = getStrictOptionValue(compilerOptions, "noImplicitAny");
const noImplicitThis = getStrictOptionValue(compilerOptions, "noImplicitThis");

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

function checkPropertyAccessExpressionOrQualifiedName(node: PropertyAccessExpression | QualifiedName, left: Expression | QualifiedName, right: Identifier) {
const type = checkNonNullExpression(left);
if (isTypeAny(type) || type === silentNeverType) {
return type;
}

const apparentType = getApparentType(getWidenedType(type));
if (apparentType === unknownType || (type.flags & TypeFlags.TypeParameter && isTypeAny(apparentType))) {
// handle cases when type is Type parameter with invalid or any constraint
let propType: Type;
const leftType = checkNonNullExpression(left);
const apparentType = getApparentType(getWidenedType(leftType));
if (isTypeAny(apparentType) || apparentType === silentNeverType) {
return apparentType;
}
const assignmentKind = getAssignmentTargetKind(node);
const prop = getPropertyOfType(apparentType, right.escapedText);
if (!prop) {
const indexInfo = getIndexInfoOfType(apparentType, IndexKind.String);
if (indexInfo && indexInfo.type) {
if (indexInfo.isReadonly && (isAssignmentTarget(node) || isDeleteTarget(node))) {
error(node, Diagnostics.Index_signature_in_type_0_only_permits_reading, typeToString(apparentType));
if (!(indexInfo && indexInfo.type)) {
if (right.escapedText && !checkAndReportErrorForExtendingInterface(node)) {
reportNonexistentProperty(right, leftType.flags & TypeFlags.TypeParameter && (leftType as TypeParameter).isThisType ? apparentType : leftType);
}
return getFlowTypeOfPropertyAccess(node, /*prop*/ undefined, indexInfo.type, getAssignmentTargetKind(node));
return unknownType;
}
if (right.escapedText && !checkAndReportErrorForExtendingInterface(node)) {
reportNonexistentProperty(right, type.flags & TypeFlags.TypeParameter && (type as TypeParameter).isThisType ? apparentType : type);
if (indexInfo.isReadonly && (isAssignmentTarget(node) || isDeleteTarget(node))) {
error(node, Diagnostics.Index_signature_in_type_0_only_permits_reading, typeToString(apparentType));
}
return unknownType;
propType = indexInfo.type;
}

checkPropertyNotUsedBeforeDeclaration(prop, node, right);

markPropertyAsReferenced(prop, node, left.kind === SyntaxKind.ThisKeyword);

getNodeLinks(node).resolvedSymbol = prop;

checkPropertyAccessibility(node, left, apparentType, prop);

const propType = getDeclaredOrApparentType(prop, node);
const assignmentKind = getAssignmentTargetKind(node);

if (assignmentKind) {
if (isReferenceToReadonlyEntity(<Expression>node, prop) || isReferenceThroughNamespaceImport(<Expression>node)) {
error(right, Diagnostics.Cannot_assign_to_0_because_it_is_a_constant_or_a_read_only_property, idText(right));
return unknownType;
else {
checkPropertyNotUsedBeforeDeclaration(prop, node, right);
markPropertyAsReferenced(prop, node, left.kind === SyntaxKind.ThisKeyword);
getNodeLinks(node).resolvedSymbol = prop;
checkPropertyAccessibility(node, left, apparentType, prop);
if (assignmentKind) {
if (isReferenceToReadonlyEntity(<Expression>node, prop) || isReferenceThroughNamespaceImport(<Expression>node)) {
error(right, Diagnostics.Cannot_assign_to_0_because_it_is_a_constant_or_a_read_only_property, idText(right));
return unknownType;
}
}
propType = getDeclaredOrApparentType(prop, node);
}
return getFlowTypeOfPropertyAccess(node, prop, propType, assignmentKind);
}

/**
* Only compute control flow type if this is a property access expression that isn't an
* assignment target, and the referenced property was declared as a variable, property,
* accessor, or optional method.
*/
function getFlowTypeOfPropertyAccess(node: PropertyAccessExpression | QualifiedName, prop: Symbol | undefined, type: Type, assignmentKind: AssignmentKind) {
// Only compute control flow type if this is a property access expression that isn't an
// assignment target, and the referenced property was declared as a variable, property,
// accessor, or optional method.
if (node.kind !== SyntaxKind.PropertyAccessExpression ||
assignmentKind === AssignmentKind.Definite ||
prop && !(prop.flags & (SymbolFlags.Variable | SymbolFlags.Property | SymbolFlags.Accessor)) && !(prop.flags & SymbolFlags.Method && type.flags & TypeFlags.Union)) {
return type;
prop && !(prop.flags & (SymbolFlags.Variable | SymbolFlags.Property | SymbolFlags.Accessor)) && !(prop.flags & SymbolFlags.Method && propType.flags & TypeFlags.Union)) {
return propType;
}
// If strict null checks and strict property initialization checks are enabled, if we have
// a this.xxx property access, if the property is an instance property without an initializer,
// and if we are in a constructor of the same class as the property declaration, assume that
// the property is uninitialized at the top of the control flow.
let assumeUninitialized = false;
if (strictNullChecks && strictPropertyInitialization && left.kind === SyntaxKind.ThisKeyword) {
const declaration = prop && prop.valueDeclaration;
if (declaration && isInstancePropertyWithoutInitializer(declaration)) {
const flowContainer = getControlFlowContainer(node);
if (flowContainer.kind === SyntaxKind.Constructor && flowContainer.parent === declaration.parent) {
assumeUninitialized = true;
}
}
}
const flowType = getFlowTypeOfReference(node, propType, assumeUninitialized ? getOptionalType(propType) : propType);
if (assumeUninitialized && !(getFalsyFlags(propType) & TypeFlags.Undefined) && getFalsyFlags(flowType) & TypeFlags.Undefined) {
error(right, Diagnostics.Property_0_is_used_before_being_assigned, symbolToString(prop));
// Return the declared type to reduce follow-on errors
return propType;
}
const flowType = getFlowTypeOfReference(node, type);
return assignmentKind ? getBaseTypeOfLiteralType(flowType) : flowType;
}

Expand Down Expand Up @@ -22483,6 +22489,7 @@ namespace ts {
if (produceDiagnostics) {
checkIndexConstraints(type);
checkTypeForDuplicateIndexSignatures(node);
checkPropertyInitialization(node);
}
}

Expand Down Expand Up @@ -22639,6 +22646,39 @@ namespace ts {
return ok;
}

function checkPropertyInitialization(node: ClassLikeDeclaration) {
if (!strictNullChecks || !strictPropertyInitialization || node.flags & NodeFlags.Ambient) {
return;
}
const constructor = findConstructorDeclaration(node);
for (const member of node.members) {
if (isInstancePropertyWithoutInitializer(member)) {
const propName = (<PropertyDeclaration>member).name;
if (isIdentifier(propName)) {
const type = getTypeOfSymbol(getSymbolOfNode(member));
if (!(type.flags & TypeFlags.Any || getFalsyFlags(type) & TypeFlags.Undefined)) {
if (!constructor || !isPropertyInitializedInConstructor(propName, type, constructor)) {
error(member.name, Diagnostics.Property_0_has_no_initializer_and_is_not_definitely_assigned_in_the_constructor, declarationNameToString(propName));
}
}
}
}
}
}

function isInstancePropertyWithoutInitializer(node: Node) {
return node.kind === SyntaxKind.PropertyDeclaration &&
!hasModifier(node, ModifierFlags.Static | ModifierFlags.Abstract) &&
!(<PropertyDeclaration>node).initializer;
}

function isPropertyInitializedInConstructor(propName: Identifier, propType: Type, constructor: ConstructorDeclaration) {
const reference = createPropertyAccess(createThis(), propName);
reference.flowNode = constructor.returnFlowNode;
const flowType = getFlowTypeOfReference(reference, propType, getOptionalType(propType));
return !(getFalsyFlags(flowType) & TypeFlags.Undefined);
}

function checkInterfaceDeclaration(node: InterfaceDeclaration) {
// Grammar checking
if (!checkGrammarDecoratorsAndModifiers(node)) checkGrammarInterfaceDeclaration(node);
Expand Down
7 changes: 7 additions & 0 deletions src/compiler/commandLineParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,13 @@ namespace ts {
category: Diagnostics.Strict_Type_Checking_Options,
description: Diagnostics.Enable_strict_checking_of_function_types
},
{
name: "strictPropertyInitialization",
type: "boolean",
showInSimplifiedHelpView: true,
category: Diagnostics.Strict_Type_Checking_Options,
description: Diagnostics.Enable_strict_checking_of_property_initialization_in_classes
},
{
name: "noImplicitThis",
type: "boolean",
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1927,7 +1927,7 @@ namespace ts {
: moduleKind === ModuleKind.System;
}

export type StrictOptionName = "noImplicitAny" | "noImplicitThis" | "strictNullChecks" | "strictFunctionTypes" | "alwaysStrict";
export type StrictOptionName = "noImplicitAny" | "noImplicitThis" | "strictNullChecks" | "strictFunctionTypes" | "strictPropertyInitialization" | "alwaysStrict";

export function getStrictOptionValue(compilerOptions: CompilerOptions, flag: StrictOptionName): boolean {
return compilerOptions[flag] === undefined ? compilerOptions.strict : compilerOptions[flag];
Expand Down
12 changes: 12 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -1952,6 +1952,14 @@
"category": "Error",
"code": 2563
},
"Property '{0}' has no initializer and is not definitely assigned in the constructor.": {
"category": "Error",
"code": 2564
},
"Property '{0}' is used before being assigned.": {
"category": "Error",
"code": 2565
},
"JSX element attributes type '{0}' may not be a union type.": {
"category": "Error",
"code": 2600
Expand Down Expand Up @@ -3391,6 +3399,10 @@
"category": "Message",
"code": 6186
},
"Enable strict checking of property initialization in classes.": {
"category": "Message",
"code": 6187
},
"Variable '{0}' implicitly has an '{1}' type.": {
"category": "Error",
"code": 7005
Expand Down
2 changes: 2 additions & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -947,6 +947,7 @@ namespace ts {
kind: SyntaxKind.Constructor;
parent?: ClassDeclaration | ClassExpression;
body?: FunctionBody;
/* @internal */ returnFlowNode?: FlowNode;
}

/** For when we encounter a semicolon in a class declaration. ES6 allows these as class elements. */
Expand Down Expand Up @@ -3854,6 +3855,7 @@ namespace ts {
strict?: boolean;
strictFunctionTypes?: boolean; // Always combine with strict property
strictNullChecks?: boolean; // Always combine with strict property
strictPropertyInitialization?: boolean; // Always combine with strict property
/* @internal */ stripInternal?: boolean;
suppressExcessPropertyErrors?: boolean;
suppressImplicitAnyIndexErrors?: boolean;
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2287,6 +2287,7 @@ declare namespace ts {
strict?: boolean;
strictFunctionTypes?: boolean;
strictNullChecks?: boolean;
strictPropertyInitialization?: boolean;
suppressExcessPropertyErrors?: boolean;
suppressImplicitAnyIndexErrors?: boolean;
target?: ScriptTarget;
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2287,6 +2287,7 @@ declare namespace ts {
strict?: boolean;
strictFunctionTypes?: boolean;
strictNullChecks?: boolean;
strictPropertyInitialization?: boolean;
suppressExcessPropertyErrors?: boolean;
suppressImplicitAnyIndexErrors?: boolean;
target?: ScriptTarget;
Expand Down
5 changes: 3 additions & 2 deletions tests/baselines/reference/indexedAccessTypeConstraints.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ type Data<T> = {
};

class Parent<M> {
private data: Data<M>;
constructor(private data: Data<M>) {}
getData(): Data<M> {
return this.data;
}
Expand Down Expand Up @@ -50,7 +50,8 @@ var __extends = (this && this.__extends) || (function () {
})();
exports.__esModule = true;
var Parent = /** @class */ (function () {
function Parent() {
function Parent(data) {
this.data = data;
}
Parent.prototype.getData = function () {
return this.data;
Expand Down
18 changes: 9 additions & 9 deletions tests/baselines/reference/indexedAccessTypeConstraints.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,20 @@ class Parent<M> {
>Parent : Symbol(Parent, Decl(indexedAccessTypeConstraints.ts, 8, 2))
>M : Symbol(M, Decl(indexedAccessTypeConstraints.ts, 10, 13))

private data: Data<M>;
>data : Symbol(Parent.data, Decl(indexedAccessTypeConstraints.ts, 10, 17))
constructor(private data: Data<M>) {}
>data : Symbol(Parent.data, Decl(indexedAccessTypeConstraints.ts, 11, 16))
>Data : Symbol(Data, Decl(indexedAccessTypeConstraints.ts, 4, 1))
>M : Symbol(M, Decl(indexedAccessTypeConstraints.ts, 10, 13))

getData(): Data<M> {
>getData : Symbol(Parent.getData, Decl(indexedAccessTypeConstraints.ts, 11, 26))
>getData : Symbol(Parent.getData, Decl(indexedAccessTypeConstraints.ts, 11, 41))
>Data : Symbol(Data, Decl(indexedAccessTypeConstraints.ts, 4, 1))
>M : Symbol(M, Decl(indexedAccessTypeConstraints.ts, 10, 13))

return this.data;
>this.data : Symbol(Parent.data, Decl(indexedAccessTypeConstraints.ts, 10, 17))
>this.data : Symbol(Parent.data, Decl(indexedAccessTypeConstraints.ts, 11, 16))
>this : Symbol(Parent, Decl(indexedAccessTypeConstraints.ts, 8, 2))
>data : Symbol(Parent.data, Decl(indexedAccessTypeConstraints.ts, 10, 17))
>data : Symbol(Parent.data, Decl(indexedAccessTypeConstraints.ts, 11, 16))
}
}

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

return this.getData().get('content');
>this.getData().get : Symbol(get, Decl(indexedAccessTypeConstraints.ts, 6, 16))
>this.getData : Symbol(Parent.getData, Decl(indexedAccessTypeConstraints.ts, 11, 26))
>this.getData : Symbol(Parent.getData, Decl(indexedAccessTypeConstraints.ts, 11, 41))
>this : Symbol(Foo, Decl(indexedAccessTypeConstraints.ts, 15, 1))
>getData : Symbol(Parent.getData, Decl(indexedAccessTypeConstraints.ts, 11, 26))
>getData : Symbol(Parent.getData, Decl(indexedAccessTypeConstraints.ts, 11, 41))
>get : Symbol(get, Decl(indexedAccessTypeConstraints.ts, 6, 16))
}
}
Expand All @@ -81,9 +81,9 @@ export class Bar<C, T extends IData<C>> extends Parent<T> {

return this.getData().get('content');
>this.getData().get : Symbol(get, Decl(indexedAccessTypeConstraints.ts, 6, 16))
>this.getData : Symbol(Parent.getData, Decl(indexedAccessTypeConstraints.ts, 11, 26))
>this.getData : Symbol(Parent.getData, Decl(indexedAccessTypeConstraints.ts, 11, 41))
>this : Symbol(Bar, Decl(indexedAccessTypeConstraints.ts, 21, 1))
>getData : Symbol(Parent.getData, Decl(indexedAccessTypeConstraints.ts, 11, 26))
>getData : Symbol(Parent.getData, Decl(indexedAccessTypeConstraints.ts, 11, 41))
>get : Symbol(get, Decl(indexedAccessTypeConstraints.ts, 6, 16))
}
}
Expand Down
Loading