Skip to content

Use control flow analysis to check 'super(...)' call before 'this' access #38612

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

Merged
merged 4 commits into from
May 17, 2020
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
10 changes: 8 additions & 2 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -985,7 +985,7 @@ namespace ts {
return initFlowNode({ flags: FlowFlags.SwitchClause, antecedent, switchStatement, clauseStart, clauseEnd });
}

function createFlowMutation(flags: FlowFlags, antecedent: FlowNode, node: Node): FlowNode {
function createFlowMutation(flags: FlowFlags, antecedent: FlowNode, node: Expression | VariableDeclaration | ArrayBindingElement): FlowNode {
setFlowNodeReferenced(antecedent);
const result = initFlowNode({ flags, antecedent, node });
if (currentExceptionTarget) {
Expand Down Expand Up @@ -1341,7 +1341,7 @@ namespace ts {
// is potentially an assertion and is therefore included in the control flow.
if (node.expression.kind === SyntaxKind.CallExpression) {
const call = <CallExpression>node.expression;
if (isDottedName(call.expression)) {
if (isDottedName(call.expression) && call.expression.kind !== SyntaxKind.SuperKeyword) {
currentFlow = createFlowCall(currentFlow, call);
}
}
Expand Down Expand Up @@ -1747,6 +1747,9 @@ namespace ts {
}
else {
bindEachChild(node);
if (node.expression.kind === SyntaxKind.SuperKeyword) {
currentFlow = createFlowCall(currentFlow, node);
}
}
}
if (node.expression.kind === SyntaxKind.PropertyAccessExpression) {
Expand Down Expand Up @@ -2464,6 +2467,9 @@ namespace ts {
node.flowNode = currentFlow;
}
return checkStrictModeIdentifier(<Identifier>node);
case SyntaxKind.SuperKeyword:
node.flowNode = currentFlow;
break;
case SyntaxKind.PrivateIdentifier:
return checkPrivateIdentifier(node as PrivateIdentifier);
case SyntaxKind.PropertyAccessExpression:
Expand Down
96 changes: 56 additions & 40 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -911,6 +911,7 @@ namespace ts {
const sharedFlowNodes: FlowNode[] = [];
const sharedFlowTypes: FlowType[] = [];
const flowNodeReachable: (boolean | undefined)[] = [];
const flowNodePostSuper: (boolean | undefined)[] = [];
const potentialThisCollisions: Node[] = [];
const potentialNewTargetCollisions: Node[] = [];
const potentialWeakMapCollisions: Node[] = [];
Expand Down Expand Up @@ -20134,7 +20135,7 @@ namespace ts {
noCacheCheck = false;
}
if (flags & (FlowFlags.Assignment | FlowFlags.Condition | FlowFlags.ArrayMutation)) {
flow = (<FlowAssignment | FlowCondition | FlowArrayMutation | PreFinallyFlow>flow).antecedent;
flow = (<FlowAssignment | FlowCondition | FlowArrayMutation>flow).antecedent;
}
else if (flags & FlowFlags.Call) {
const signature = getEffectsSignature((<FlowCall>flow).node);
Expand Down Expand Up @@ -20184,6 +20185,51 @@ namespace ts {
}
}

// Return true if the given flow node is preceded by a 'super(...)' call in every possible code path
// leading to the node.
function isPostSuperFlowNode(flow: FlowNode, noCacheCheck: boolean): boolean {
while (true) {
const flags = flow.flags;
if (flags & FlowFlags.Shared) {
if (!noCacheCheck) {
const id = getFlowNodeId(flow);
const postSuper = flowNodePostSuper[id];
return postSuper !== undefined ? postSuper : (flowNodePostSuper[id] = isPostSuperFlowNode(flow, /*noCacheCheck*/ true));
}
noCacheCheck = false;
}
if (flags & (FlowFlags.Assignment | FlowFlags.Condition | FlowFlags.ArrayMutation | FlowFlags.SwitchClause)) {
flow = (<FlowAssignment | FlowCondition | FlowArrayMutation | FlowSwitchClause>flow).antecedent;
}
else if (flags & FlowFlags.Call) {
if ((<FlowCall>flow).node.expression.kind === SyntaxKind.SuperKeyword) {
return true;
}
flow = (<FlowCall>flow).antecedent;
}
else if (flags & FlowFlags.BranchLabel) {
// A branching point is post-super if every branch is post-super.
return every((<FlowLabel>flow).antecedents, f => isPostSuperFlowNode(f, /*noCacheCheck*/ false));
}
else if (flags & FlowFlags.LoopLabel) {
// A loop is post-super if the control flow path that leads to the top is post-super.
flow = (<FlowLabel>flow).antecedents![0];
}
else if (flags & FlowFlags.ReduceLabel) {
const target = (<FlowReduceLabel>flow).target;
const saveAntecedents = target.antecedents;
target.antecedents = (<FlowReduceLabel>flow).antecedents;
const result = isPostSuperFlowNode((<FlowReduceLabel>flow).antecedent, /*noCacheCheck*/ false);
target.antecedents = saveAntecedents;
return result;
}
else {
// Unreachable nodes are considered post-super to silence errors
return !!(flags & FlowFlags.Unreachable);
}
}
}

function getFlowTypeOfReference(reference: Node, declaredType: Type, initialType = declaredType, flowContainer?: Node, couldBeUninitialized?: boolean) {
let key: string | undefined;
let keySet = false;
Expand Down Expand Up @@ -21583,31 +21629,10 @@ namespace ts {
}
}

function findFirstSuperCall(n: Node): SuperCall | undefined {
if (isSuperCall(n)) {
return n;
}
else if (isFunctionLike(n)) {
return undefined;
}
return forEachChild(n, findFirstSuperCall);
}

/**
* Return a cached result if super-statement is already found.
* Otherwise, find a super statement in a given constructor function and cache the result in the node-links of the constructor
*
* @param constructor constructor-function to look for super statement
*/
function getSuperCallInConstructor(constructor: ConstructorDeclaration): SuperCall | undefined {
const links = getNodeLinks(constructor);

// Only trying to find super-call if we haven't yet tried to find one. Once we try, we will record the result
if (links.hasSuperCall === undefined) {
links.superCall = findFirstSuperCall(constructor.body!);
links.hasSuperCall = links.superCall ? true : false;
}
return links.superCall!;
function findFirstSuperCall(node: Node): SuperCall | undefined {
return isSuperCall(node) ? node :
isFunctionLike(node) ? undefined :
forEachChild(node, findFirstSuperCall);
}

/**
Expand All @@ -21630,17 +21655,7 @@ namespace ts {
// If a containing class does not have extends clause or the class extends null
// skip checking whether super statement is called before "this" accessing.
if (baseTypeNode && !classDeclarationExtendsNull(containingClassDecl)) {
const superCall = getSuperCallInConstructor(<ConstructorDeclaration>container);

// We should give an error in the following cases:
// - No super-call
// - "this" is accessing before super-call.
// i.e super(this)
// this.x; super();
// We want to make sure that super-call is done before accessing "this" so that
// "this" is not accessed as a parameter of the super-call.
if (!superCall || superCall.end > node.pos) {
// In ES6, super inside constructor of class-declaration has to precede "this" accessing
if (node.flowNode && !isPostSuperFlowNode(node.flowNode, /*noCacheCheck*/ false)) {
error(node, diagnosticMessage);
}
}
Expand Down Expand Up @@ -21865,7 +21880,8 @@ namespace ts {
function checkSuperExpression(node: Node): Type {
const isCallExpression = node.parent.kind === SyntaxKind.CallExpression && (<CallExpression>node.parent).expression === node;

let container = getSuperContainer(node, /*stopOnFunctions*/ true);
const immediateContainer = getSuperContainer(node, /*stopOnFunctions*/ true);
let container = immediateContainer;
let needToCaptureLexicalThis = false;

// adjust the container reference in case if super is used inside arrow functions with arbitrarily deep nesting
Expand Down Expand Up @@ -21901,7 +21917,7 @@ namespace ts {
return errorType;
}

if (!isCallExpression && container.kind === SyntaxKind.Constructor) {
if (!isCallExpression && immediateContainer.kind === SyntaxKind.Constructor) {
checkThisBeforeSuper(node, container, Diagnostics.super_must_be_called_before_accessing_a_property_of_super_in_the_constructor_of_a_derived_class);
}

Expand Down Expand Up @@ -29898,7 +29914,7 @@ namespace ts {
if (getClassExtendsHeritageElement(containingClassDecl)) {
captureLexicalThis(node.parent, containingClassDecl);
const classExtendsNull = classDeclarationExtendsNull(containingClassDecl);
const superCall = getSuperCallInConstructor(node);
const superCall = findFirstSuperCall(node.body!);
if (superCall) {
if (classExtendsNull) {
error(superCall, Diagnostics.A_constructor_cannot_contain_a_super_call_when_its_class_extends_null);
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ namespace ts {
* returns a falsey value, then returns false.
* If no such value is found, the callback is applied to each element of array and `true` is returned.
*/
export function every<T>(array: readonly T[], callback: (element: T, index: number) => boolean): boolean {
export function every<T>(array: readonly T[] | undefined, callback: (element: T, index: number) => boolean): boolean {
if (array) {
for (let i = 0; i < array.length; i++) {
if (!callback(array[i], i)) {
Expand Down
21 changes: 3 additions & 18 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2791,34 +2791,21 @@ namespace ts {
}

export type FlowNode =
| AfterFinallyFlow
| PreFinallyFlow
| FlowStart
| FlowLabel
| FlowAssignment
| FlowCall
| FlowCondition
| FlowSwitchClause
| FlowArrayMutation;
| FlowArrayMutation
| FlowCall
| FlowReduceLabel;

export interface FlowNodeBase {
flags: FlowFlags;
id?: number; // Node id used by flow type cache in checker
}

export interface FlowLock {
locked?: boolean;
}

export interface AfterFinallyFlow extends FlowNodeBase, FlowLock {
antecedent: FlowNode;
}

export interface PreFinallyFlow extends FlowNodeBase {
antecedent: FlowNode;
lock: FlowLock;
}

// FlowStart represents the start of a control flow. For a function expression or arrow
// function, the node property references the function (which in turn has a flowNode
// property for the containing control flow).
Expand Down Expand Up @@ -4316,8 +4303,6 @@ namespace ts {
resolvedJsxElementAttributesType?: Type; // resolved element attributes type of a JSX openinglike element
resolvedJsxElementAllAttributesType?: Type; // resolved all element attributes type of a JSX openinglike element
resolvedJSDocType?: Type; // Resolved type of a JSDoc type reference
hasSuperCall?: boolean; // recorded result when we try to find super-call. We only try to find one if this flag is undefined, indicating that we haven't made an attempt.
superCall?: SuperCall; // Cached first super-call found in the constructor. Used in checking whether super is called before this-accessing
switchTypes?: Type[]; // Cached array of switch case expression types
jsxNamespace?: Symbol | false; // Resolved jsx namespace symbol for this node
contextFreeType?: Type; // Cached context-free type used by the first pass of inference; used when a function's return is partially contextually sensitive
Expand Down
12 changes: 1 addition & 11 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1743,21 +1743,11 @@ declare namespace ts {
Label = 12,
Condition = 96
}
export type FlowNode = AfterFinallyFlow | PreFinallyFlow | FlowStart | FlowLabel | FlowAssignment | FlowCall | FlowCondition | FlowSwitchClause | FlowArrayMutation;
export type FlowNode = FlowStart | FlowLabel | FlowAssignment | FlowCall | FlowCondition | FlowSwitchClause | FlowArrayMutation | FlowCall | FlowReduceLabel;
export interface FlowNodeBase {
flags: FlowFlags;
id?: number;
}
export interface FlowLock {
locked?: boolean;
}
export interface AfterFinallyFlow extends FlowNodeBase, FlowLock {
antecedent: FlowNode;
}
export interface PreFinallyFlow extends FlowNodeBase {
antecedent: FlowNode;
lock: FlowLock;
}
export interface FlowStart extends FlowNodeBase {
node?: FunctionExpression | ArrowFunction | MethodDeclaration;
}
Expand Down
12 changes: 1 addition & 11 deletions tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1743,21 +1743,11 @@ declare namespace ts {
Label = 12,
Condition = 96
}
export type FlowNode = AfterFinallyFlow | PreFinallyFlow | FlowStart | FlowLabel | FlowAssignment | FlowCall | FlowCondition | FlowSwitchClause | FlowArrayMutation;
export type FlowNode = FlowStart | FlowLabel | FlowAssignment | FlowCall | FlowCondition | FlowSwitchClause | FlowArrayMutation | FlowCall | FlowReduceLabel;
export interface FlowNodeBase {
flags: FlowFlags;
id?: number;
}
export interface FlowLock {
locked?: boolean;
}
export interface AfterFinallyFlow extends FlowNodeBase, FlowLock {
antecedent: FlowNode;
}
export interface PreFinallyFlow extends FlowNodeBase {
antecedent: FlowNode;
lock: FlowLock;
}
export interface FlowStart extends FlowNodeBase {
node?: FunctionExpression | ArrowFunction | MethodDeclaration;
}
Expand Down

This file was deleted.

Loading