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

* Use CFA graph to check this/super accesses are preceded by super() call

* Accept cleaned-up API baselines

* Accept new baselines

* Add tests
This commit is contained in:
Anders Hejlsberg
2020-05-16 19:58:17 -07:00
committed by GitHub
parent bc0d2ec26c
commit 3c1f37e913
13 changed files with 948 additions and 104 deletions

View File

@@ -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) {
@@ -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);
}
}
@@ -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) {
@@ -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:

View File

@@ -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[] = [];
@@ -20148,7 +20149,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);
@@ -20198,6 +20199,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;
@@ -21597,31 +21643,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);
}
/**
@@ -21644,17 +21669,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);
}
}
@@ -21879,7 +21894,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
@@ -21915,7 +21931,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);
}
@@ -29912,7 +29928,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);

View File

@@ -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)) {

View File

@@ -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).
@@ -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