Skip to content

Commit

Permalink
Revert "feat(40197): handle uncalled function checks in binary expres…
Browse files Browse the repository at this point in the history 
…sions (#40260)"

This reverts commit eaf4f46.
  • Loading branch information
@sandersn
sandersn committed Oct 7, 2020
1 parent eaf4f46 commit cf3e28e
Show file tree
Hide file tree
Showing 15 changed files with 24 additions and 1,069 deletions.
4 changes: 2 additions & 2 deletions src/compiler/binder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,7 @@ namespace ts {
}
// We create a return control flow graph for IIFEs and constructors. For constructors
// we use the return control flow graph in strict property initialization checks.
currentReturnTarget = isIIFE || node.kind === SyntaxKind.Constructor || (isInJSFile(node) && (node.kind === SyntaxKind.FunctionDeclaration || node.kind === SyntaxKind.FunctionExpression)) ? createBranchLabel() : undefined;
currentReturnTarget = isIIFE || node.kind === SyntaxKind.Constructor || (isInJSFile && (node.kind === SyntaxKind.FunctionDeclaration || node.kind === SyntaxKind.FunctionExpression)) ? createBranchLabel() : undefined;
currentExceptionTarget = undefined;
currentBreakTarget = undefined;
currentContinueTarget = undefined;
Expand All @@ -686,7 +686,7 @@ namespace ts {
if (currentReturnTarget) {
addAntecedent(currentReturnTarget, currentFlow);
currentFlow = finishFlowLabel(currentReturnTarget);
if (node.kind === SyntaxKind.Constructor || (isInJSFile(node) && (node.kind === SyntaxKind.FunctionDeclaration || node.kind === SyntaxKind.FunctionExpression))) {
if (node.kind === SyntaxKind.Constructor || (isInJSFile && (node.kind === SyntaxKind.FunctionDeclaration || node.kind === SyntaxKind.FunctionExpression))) {
(<FunctionLikeDeclaration>node).returnFlowNode = currentFlow;
}
}
Expand Down
53 changes: 19 additions & 34 deletions src/compiler/checker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30006,9 +30006,6 @@ namespace ts {
workStacks.leftType[stackIndex] = leftType;
const operator = node.operatorToken.kind;
if (operator === SyntaxKind.AmpersandAmpersandToken || operator === SyntaxKind.BarBarToken || operator === SyntaxKind.QuestionQuestionToken) {
if (operator === SyntaxKind.AmpersandAmpersandToken) {
checkTestingKnownTruthyCallableType(node.left, leftType);
}
checkTruthinessOfType(leftType, node.left);
}
advanceState(CheckBinaryExpressionState.FinishCheck);
Expand Down Expand Up @@ -30542,7 +30539,7 @@ namespace ts {

function checkConditionalExpression(node: ConditionalExpression, checkMode?: CheckMode): Type {
const type = checkTruthinessExpression(node.condition);
checkTestingKnownTruthyCallableType(node.condition, type, node.whenTrue);
checkTestingKnownTruthyCallableType(node.condition, node.whenTrue, type);
const type1 = checkExpression(node.whenTrue, checkMode);
const type2 = checkExpression(node.whenFalse, checkMode);
return getUnionType([type1, type2], UnionReduction.Subtype);
Expand Down Expand Up @@ -33778,7 +33775,7 @@ namespace ts {
// Grammar checking
checkGrammarStatementInAmbientContext(node);
const type = checkTruthinessExpression(node.expression);
checkTestingKnownTruthyCallableType(node.expression, type, node.thenStatement);
checkTestingKnownTruthyCallableType(node.expression, node.thenStatement, type);
checkSourceElement(node.thenStatement);

if (node.thenStatement.kind === SyntaxKind.EmptyStatement) {
Expand All @@ -33788,16 +33785,16 @@ namespace ts {
checkSourceElement(node.elseStatement);
}

function checkTestingKnownTruthyCallableType(condExpr: Expression, type: Type, body?: Statement | Expression) {
function checkTestingKnownTruthyCallableType(condExpr: Expression, body: Statement | Expression, type: Type) {
if (!strictNullChecks) {
return;
}

const location = isBinaryExpression(condExpr) ? condExpr.right : condExpr;
const testedNode = isIdentifier(location) ? location
: isPropertyAccessExpression(location) ? location.name
: isBinaryExpression(location) && isIdentifier(location.right) ? location.right
: undefined;
const testedNode = isIdentifier(condExpr)
? condExpr
: isPropertyAccessExpression(condExpr)
? condExpr.name
: undefined;

if (!testedNode) {
return;
Expand All @@ -33818,34 +33815,27 @@ namespace ts {
return;
}

const testedSymbol = getSymbolAtLocation(testedNode);
if (!testedSymbol) {
const testedFunctionSymbol = getSymbolAtLocation(testedNode);
if (!testedFunctionSymbol) {
return;
}

const isUsed = isBinaryExpression(condExpr.parent) ? isFunctionUsedInBinaryExpressionChain(condExpr.parent, testedSymbol)
: body ? isFunctionUsedInConditionBody(condExpr, body, testedNode, testedSymbol)
: false;
if (!isUsed) {
error(location, Diagnostics.This_condition_will_always_return_true_since_the_function_is_always_defined_Did_you_mean_to_call_it_instead);
}
}

function isFunctionUsedInConditionBody(expr: Expression, body: Statement | Expression, testedNode: Node, testedSymbol: Symbol): boolean {
return !!forEachChild(body, function check(childNode): boolean | undefined {
const functionIsUsedInBody = forEachChild(body, function check(childNode): boolean | undefined {
if (isIdentifier(childNode)) {
const childSymbol = getSymbolAtLocation(childNode);
if (childSymbol && childSymbol === testedSymbol) {
if (childSymbol && childSymbol === testedFunctionSymbol) {
// If the test was a simple identifier, the above check is sufficient
if (isIdentifier(expr)) {
if (isIdentifier(condExpr)) {
return true;
}
// Otherwise we need to ensure the symbol is called on the same target
let testedExpression = testedNode.parent;
let childExpression = childNode.parent;
while (testedExpression && childExpression) {

if (isIdentifier(testedExpression) && isIdentifier(childExpression) ||
testedExpression.kind === SyntaxKind.ThisKeyword && childExpression.kind === SyntaxKind.ThisKeyword) {
testedExpression.kind === SyntaxKind.ThisKeyword && childExpression.kind === SyntaxKind.ThisKeyword
) {
return getSymbolAtLocation(testedExpression) === getSymbolAtLocation(childExpression);
}

Expand All @@ -33862,18 +33852,13 @@ namespace ts {
}
}
}

return forEachChild(childNode, check);
});
}

function isFunctionUsedInBinaryExpressionChain(node: Node, testedSymbol: Symbol): boolean {
while (isBinaryExpression(node) && node.operatorToken.kind === SyntaxKind.AmpersandAmpersandToken) {
if (isCallExpression(node.right) && testedSymbol === getSymbolAtLocation(node.right.expression)) {
return true;
}
node = node.parent;
if (!functionIsUsedInBody) {
error(condExpr, Diagnostics.This_condition_will_always_return_true_since_the_function_is_always_defined_Did_you_mean_to_call_it_instead);
}
return false;
}

function checkDoStatement(node: DoStatement) {
Expand Down
6 changes: 3 additions & 3 deletions src/compiler/emitter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2811,7 +2811,7 @@ namespace ts {
if (isSimilarNode && currentSourceFile) {
pos = skipTrivia(currentSourceFile.text, pos);
}
if (isSimilarNode && contextNode.pos !== startPos) {
if (emitLeadingCommentsOfPosition && isSimilarNode && contextNode.pos !== startPos) {
const needsIndent = indentLeading && currentSourceFile && !positionsAreOnSameLine(startPos, pos, currentSourceFile);
if (needsIndent) {
increaseIndent();
Expand All @@ -2822,7 +2822,7 @@ namespace ts {
}
}
pos = writeTokenText(token, writer, pos);
if (isSimilarNode && contextNode.end !== pos) {
if (emitTrailingCommentsOfPosition && isSimilarNode && contextNode.end !== pos) {
emitTrailingCommentsOfPosition(pos, /*prefixSpace*/ true);
}
return pos;
Expand Down Expand Up @@ -3468,7 +3468,7 @@ namespace ts {
// "comment1" is not considered to be leading comment for node.initializer
// but rather a trailing comment on the previous node.
const initializer = node.initializer;
if ((getEmitFlags(initializer) & EmitFlags.NoLeadingComments) === 0) {
if (emitTrailingCommentsOfPosition && (getEmitFlags(initializer) & EmitFlags.NoLeadingComments) === 0) {
const commentRange = getCommentRange(initializer);
emitTrailingCommentsOfPosition(commentRange.pos);
}
Expand Down
108 changes: 0 additions & 108 deletions tests/baselines/reference/truthinessCallExpressionCoercion2.errors.txt
View file

This file was deleted.

Loading

0 comments on commit cf3e28e

Please sign in to comment.