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.

Sign up for GitHub

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

Disallow void branches #7261

Closed
wants to merge 7 commits into from
Closed

Disallow void branches #7261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
aa4ea10
Add checking for 'void' in '||', '&&', and branching statements.
DanielRosenwasser Feb 26, 2016
cf3182d
Accepted baselines.
DanielRosenwasser Feb 26, 2016
72fe336
Error on void tested expressions for logical not and conditional expr…
DanielRosenwasser Feb 27, 2016
0c79f97
Accepted baselines.
DanielRosenwasser Feb 27, 2016
6cd31f7
Minor function rename.
DanielRosenwasser Mar 9, 2016
5992d53
Merge branch 'master' into noVoidBranches
DanielRosenwasser Apr 8, 2016
eacae36
Merge branch 'master' into noVoidBranches
DanielRosenwasser May 25, 2016
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
83 changes: 71 additions & 12 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12011,10 +12011,13 @@ namespace ts {
case SyntaxKind.MinusToken:
case SyntaxKind.TildeToken:
if (maybeTypeOfKind(operandType, TypeFlags.ESSymbol)) {
error(node.operand, Diagnostics.The_0_operator_cannot_be_applied_to_type_symbol, tokenToString(node.operator));
error(node.operand, Diagnostics.The_0_operator_cannot_be_applied_to_type_1, tokenToString(node.operator), typeToString(esSymbolType));
}
return numberType;
case SyntaxKind.ExclamationToken:
if (operandType === voidType) {
error(node.operand, Diagnostics.The_0_operator_cannot_be_applied_to_type_1, tokenToString(node.operator), typeToString(operandType));
}
return booleanType;
case SyntaxKind.PlusPlusToken:
case SyntaxKind.MinusMinusToken:
Expand Down Expand Up @@ -12312,7 +12315,7 @@ namespace ts {
if ((leftType.flags & TypeFlags.Boolean) &&
(rightType.flags & TypeFlags.Boolean) &&
(suggestedOperator = getSuggestedBooleanOperator(operatorToken.kind)) !== undefined) {
error(errorNode || operatorToken, Diagnostics.The_0_operator_is_not_allowed_for_boolean_types_Consider_using_1_instead, tokenToString(operatorToken.kind), tokenToString(suggestedOperator));
error(errorNode || operatorToken, Diagnostics.The_0_operator_is_not_allowed_for_boolean_types_Consider_using_1_instead, tokenToString(operator), tokenToString(suggestedOperator));
}
else {
// otherwise just check each operand separately and report errors as normal
Expand Down Expand Up @@ -12355,7 +12358,7 @@ namespace ts {
}

// Symbols are not allowed at all in arithmetic expressions
if (resultType && !checkForDisallowedESSymbolOperand(operator)) {
if (resultType && checkForDisallowedESSymbolOperand(operator)) {
return resultType;
}
}
Expand Down Expand Up @@ -12392,8 +12395,14 @@ namespace ts {
case SyntaxKind.InKeyword:
return checkInExpression(left, right, leftType, rightType);
case SyntaxKind.AmpersandAmpersandToken:
if (checkForDisallowedVoidLeftOperand(operator, left, leftType)) {
return leftType;
}
return addNullableKind(rightType, getNullableKind(leftType));
case SyntaxKind.BarBarToken:
if (checkForDisallowedVoidLeftOperand(operator, left, leftType)) {
return rightType;
}
return getUnionType([getNonNullableType(leftType), rightType]);
case SyntaxKind.EqualsToken:
checkAssignmentOperator(rightType);
Expand All @@ -12402,17 +12411,34 @@ namespace ts {
return rightType;
}

// Return true if there was no error, false if there was an error.
/** @returns true if there was an error, false otherwise. */
function checkForDisallowedESSymbolOperand(operator: SyntaxKind): boolean {
const offendingSymbolOperand =
maybeTypeOfKind(leftType, TypeFlags.ESSymbol) ? left :
maybeTypeOfKind(rightType, TypeFlags.ESSymbol) ? right :
undefined;
if (offendingSymbolOperand) {
error(offendingSymbolOperand, Diagnostics.The_0_operator_cannot_be_applied_to_type_symbol, tokenToString(operator));
return false;
error(offendingSymbolOperand, Diagnostics.The_0_operator_cannot_be_applied_to_type_1, tokenToString(operator), typeToString(esSymbolType));
return true;
}

return false;
}

/** @returns true if there was an error, false otherwise. */
function checkForDisallowedVoidLeftOperand(operatorKind: SyntaxKind, leftOperand: Expression, operandType: Type): boolean {
if (operandType !== voidType) {
return false;
}
switch (operatorKind) {
case SyntaxKind.AmpersandAmpersandToken:
case SyntaxKind.BarBarToken:
Debug.assert(leftOperand === (leftOperand.parent as BinaryExpression).left, "Operand should be left side.");
error(leftOperand, Diagnostics.The_left_hand_side_of_a_0_expression_cannot_have_type_1, tokenToString(operatorKind), typeToString(operandType));
break;
default:
Debug.fail("Not '&&' or '||'");
}
return true;
}

Expand Down Expand Up @@ -12517,7 +12543,12 @@ namespace ts {
}

function checkConditionalExpression(node: ConditionalExpression, contextualMapper?: TypeMapper): Type {
checkExpression(node.condition);
const conditionType = checkExpression(node.condition);

if (conditionType === voidType) {
error(node.condition, Diagnostics.The_tested_expression_of_a_conditional_expression_cannot_have_type_0, typeToString(conditionType));
}

const type1 = checkExpression(node.whenTrue, contextualMapper);
const type2 = checkExpression(node.whenFalse, contextualMapper);
return getUnionType([type1, type2]);
Expand Down Expand Up @@ -14090,7 +14121,9 @@ namespace ts {

function checkFunctionDeclaration(node: FunctionDeclaration): void {
if (produceDiagnostics) {
checkFunctionOrMethodDeclaration(node) || checkGrammarForGenerator(node);
checkGrammarForGenerator(node);

checkFunctionOrMethodDeclaration(node);

checkCollisionWithCapturedSuperVariable(node, node.name);
checkCollisionWithCapturedThisVariable(node, node.name);
Expand Down Expand Up @@ -14582,7 +14615,9 @@ namespace ts {
// Grammar checking
checkGrammarStatementInAmbientContext(node);

checkExpression(node.expression);
const conditionType = checkExpression(node.expression);
checkConditionOfBranchingStatement(node, node.expression, conditionType);

checkSourceElement(node.thenStatement);

if (node.thenStatement.kind === SyntaxKind.EmptyStatement) {
Expand All @@ -14597,14 +14632,16 @@ namespace ts {
checkGrammarStatementInAmbientContext(node);

checkSourceElement(node.statement);
checkExpression(node.expression);
const conditionType = checkExpression(node.expression);
checkConditionOfBranchingStatement(node, node.expression, conditionType);
}

function checkWhileStatement(node: WhileStatement) {
// Grammar checking
checkGrammarStatementInAmbientContext(node);

checkExpression(node.expression);
const conditionType = checkExpression(node.expression);
checkConditionOfBranchingStatement(node, node.expression, conditionType);
checkSourceElement(node.statement);
}

Expand All @@ -14625,11 +14662,33 @@ namespace ts {
}
}

if (node.condition) checkExpression(node.condition);
if (node.condition) {
const conditionType = checkExpression(node.condition);
checkConditionOfBranchingStatement(node, node.condition, conditionType);
}
if (node.incrementor) checkExpression(node.incrementor);
checkSourceElement(node.statement);
}

function checkConditionOfBranchingStatement(
statementOrConditional: IfStatement | IterationStatement,
condition: Expression,
conditionType: Type): void {

// Any type apart from 'void' is okay.
if (conditionType !== voidType) {
return;
}

// Don't report an error for '||'.
// We'll have reported errors elsewhere.
if (condition.kind === SyntaxKind.BinaryExpression && (condition as BinaryExpression).operatorToken.kind === SyntaxKind.BarBarToken) {
return;
}

error(statementOrConditional, Diagnostics.The_condition_of_a_0_statement_cannot_have_type_1, tokenToString(statementOrConditional.kind), voidType);
}

function checkForOfStatement(node: ForOfStatement): void {
checkGrammarForInOrForOfStatement(node);

Expand Down
14 changes: 13 additions & 1 deletion src/compiler/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1495,7 +1495,7 @@
"category": "Error",
"code": 2468
},
"The '{0}' operator cannot be applied to type 'symbol'.": {
"The '{0}' operator cannot be applied to type '{1}'.": {
"category": "Error",
"code": 2469
},
Expand Down Expand Up @@ -2312,6 +2312,18 @@
"category": "Error",
"code": 5064
},
"The left-hand side of a '{0}' expression cannot have type '{1}'.": {
"category": "Error",
"code": 5065
},
"The condition of a '{0}' statement cannot have type '{1}'.": {
"category": "Error",
"code": 5066
},
"The tested expression of a conditional expression cannot have type '{0}'.": {
"category": "Error",
"code": 5067
},
"Concatenate and emit output to single file.": {
"category": "Message",
"code": 6001
Expand Down
82 changes: 82 additions & 0 deletions tests/baselines/reference/conditionalOperatorConditionIsObjectType.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
tests/cases/conformance/expressions/conditonalOperator/conditionalOperatorConditionIsObjectType.ts(36,1): error TS5065: The tested expression of a conditional expression cannot have type 'void'.
tests/cases/conformance/expressions/conditonalOperator/conditionalOperatorConditionIsObjectType.ts(39,1): error TS5065: The tested expression of a conditional expression cannot have type 'void'.
tests/cases/conformance/expressions/conditonalOperator/conditionalOperatorConditionIsObjectType.ts(58,20): error TS5065: The tested expression of a conditional expression cannot have type 'void'.
tests/cases/conformance/expressions/conditonalOperator/conditionalOperatorConditionIsObjectType.ts(61,23): error TS5065: The tested expression of a conditional expression cannot have type 'void'.
tests/cases/conformance/expressions/conditonalOperator/conditionalOperatorConditionIsObjectType.ts(63,32): error TS5065: The tested expression of a conditional expression cannot have type 'void'.


==== tests/cases/conformance/expressions/conditonalOperator/conditionalOperatorConditionIsObjectType.ts (5 errors) ====
//Cond ? Expr1 : Expr2, Cond is of object type, Expr1 and Expr2 have the same type
var condObject: Object;

var exprAny1: any;
var exprBoolean1: boolean;
var exprNumber1: number;
var exprString1: string;
var exprIsObject1: Object;

var exprAny2: any;
var exprBoolean2: boolean;
var exprNumber2: number;
var exprString2: string;
var exprIsObject2: Object;

function foo() { };
class C { static doIt: () => void };

//Cond is an object type variable
condObject ? exprAny1 : exprAny2;
condObject ? exprBoolean1 : exprBoolean2;
condObject ? exprNumber1 : exprNumber2;
condObject ? exprString1 : exprString2;
condObject ? exprIsObject1 : exprIsObject2;
condObject ? exprString1 : exprBoolean1; // union

//Cond is an object type literal
((a: string) => a.length) ? exprAny1 : exprAny2;
((a: string) => a.length) ? exprBoolean1 : exprBoolean2;
({}) ? exprNumber1 : exprNumber2;
({ a: 1, b: "s" }) ? exprString1 : exprString2;
({ a: 1, b: "s" }) ? exprIsObject1 : exprIsObject2;
({ a: 1, b: "s" }) ? exprString1: exprBoolean1; // union

//Cond is an object type expression
foo() ? exprAny1 : exprAny2;
~~~~~
!!! error TS5065: The tested expression of a conditional expression cannot have type 'void'.
new Date() ? exprBoolean1 : exprBoolean2;
new C() ? exprNumber1 : exprNumber2;
C.doIt() ? exprString1 : exprString2;
~~~~~~~~
!!! error TS5065: The tested expression of a conditional expression cannot have type 'void'.
condObject.valueOf() ? exprIsObject1 : exprIsObject2;
new Date() ? exprString1 : exprBoolean1; // union

//Results shoud be same as Expr1 and Expr2
var resultIsAny1 = condObject ? exprAny1 : exprAny2;
var resultIsBoolean1 = condObject ? exprBoolean1 : exprBoolean2;
var resultIsNumber1 = condObject ? exprNumber1 : exprNumber2;
var resultIsString1 = condObject ? exprString1 : exprString2;
var resultIsObject1 = condObject ? exprIsObject1 : exprIsObject2;
var resultIsStringOrBoolean1 = condObject ? exprString1 : exprBoolean1; // union

var resultIsAny2 = ((a: string) => a.length) ? exprAny1 : exprAny2;
var resultIsBoolean2 = ((a: string) => a.length) ? exprBoolean1 : exprBoolean2;
var resultIsNumber2 = ({}) ? exprNumber1 : exprNumber2;
var resultIsString2 = ({ a: 1, b: "s" }) ? exprString1 : exprString2;
var resultIsObject2 = ({ a: 1, b: "s" }) ? exprIsObject1 : exprIsObject2;
var resultIsStringOrBoolean2 = ({ a: 1, b: "s" }) ? exprString1 : exprBoolean1; // union

var resultIsAny3 = foo() ? exprAny1 : exprAny2;
~~~~~
!!! error TS5065: The tested expression of a conditional expression cannot have type 'void'.
var resultIsBoolean3 = new Date() ? exprBoolean1 : exprBoolean2;
var resultIsNumber3 = new C() ? exprNumber1 : exprNumber2;
var resultIsString3 = C.doIt() ? exprString1 : exprString2;
~~~~~~~~
!!! error TS5065: The tested expression of a conditional expression cannot have type 'void'.
var resultIsObject3 = condObject.valueOf() ? exprIsObject1 : exprIsObject2;
var resultIsStringOrBoolean3 = C.doIt() ? exprString1 : exprBoolean1; // union
~~~~~~~~
!!! error TS5065: The tested expression of a conditional expression cannot have type 'void'.

Loading