From ab0a788fc8ac3aa4663aa42cbda81c2e8155936f Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Fri, 9 Sep 2016 16:24:43 -0700 Subject: [PATCH] Disallow comma operator when LHS is pure --- src/compiler/checker.ts | 79 +++++++++++++++++++ src/compiler/diagnosticMessages.json | 4 + ...nmentToParenthesizedExpression1.errors.txt | 7 +- .../reference/commaOperator1.errors.txt | 33 ++++++++ .../compiler/commaOperatorLeftSideUnused.ts | 45 +++++++++++ 5 files changed, 166 insertions(+), 2 deletions(-) create mode 100644 tests/baselines/reference/commaOperator1.errors.txt create mode 100644 tests/cases/compiler/commaOperatorLeftSideUnused.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index f5eb12a419e..efa9dd478a1 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -13209,6 +13209,82 @@ namespace ts { return sourceType; } + function isSideEffectFree(node: Node): boolean { + switch (node.kind) { + case SyntaxKind.Identifier: + case SyntaxKind.StringLiteral: + case SyntaxKind.RegularExpressionLiteral: + case SyntaxKind.NoSubstitutionTemplateLiteral: + case SyntaxKind.NumericLiteral: + case SyntaxKind.TrueKeyword: + case SyntaxKind.FalseKeyword: + case SyntaxKind.NullKeyword: + case SyntaxKind.UndefinedKeyword: + case SyntaxKind.FunctionExpression: + case SyntaxKind.ArrowFunction: + return true; + + case SyntaxKind.BinaryExpression: + if (isAssignmentOperator((node as BinaryExpression).operatorToken.kind)) { + return false; + } + return isSideEffectFree((node as BinaryExpression).left) && + isSideEffectFree((node as BinaryExpression).right); + + case SyntaxKind.PrefixUnaryExpression: + case SyntaxKind.PostfixUnaryExpression: + switch ((node as PrefixUnaryExpression).operator) { + case SyntaxKind.ExclamationToken: + case SyntaxKind.PlusToken: + case SyntaxKind.MinusToken: + case SyntaxKind.TildeToken: + return isSideEffectFree((node as PrefixUnaryExpression).operand); + } + return false; + + case SyntaxKind.ParenthesizedExpression: + case SyntaxKind.TypeAssertionExpression: + case SyntaxKind.TypeOfExpression: + case SyntaxKind.AsExpression: + case SyntaxKind.NonNullExpression: + // All the nodes which just have a .expression we should check + return isSideEffectFree((node as ParenthesizedExpression).expression); + + case SyntaxKind.ConditionalExpression: + return isSideEffectFree((node as ConditionalExpression).condition) && + isSideEffectFree((node as ConditionalExpression).whenTrue) && + isSideEffectFree((node as ConditionalExpression).whenFalse); + + case SyntaxKind.ObjectLiteralExpression: + // Note: negated forEach condition since we want to bail early to 'true', + // in other words the callback is computing "Could have side effects?" + return !forEach((node as ObjectLiteralExpression).properties, prop => { + if (isComputedPropertyName(prop.name) && !isSideEffectFree((prop.name as ComputedPropertyName).expression)) { + return true; + } + + switch (prop.kind) { + case SyntaxKind.ShorthandPropertyAssignment: + case SyntaxKind.MethodDeclaration: + return false; + case SyntaxKind.PropertyAssignment: + return !isSideEffectFree((prop as PropertyAssignment).initializer); + default: + Debug.fail('Unhandled object literal property kind ' + prop.kind); + } + }); + + case SyntaxKind.ArrayLiteralExpression: + // See previous comment about negated callback values + return !forEach((node as ArrayLiteralExpression).elements, elem => !isSideEffectFree(elem)); + + case SyntaxKind.VoidExpression: + // Explicit opt-out case (listed here to avoid accidental duplication above): 'void x' + default: + return false; + } + } + function isTypeEqualityComparableTo(source: Type, target: Type) { return (target.flags & TypeFlags.Nullable) !== 0 || isTypeComparableTo(source, target); } @@ -13370,6 +13446,9 @@ namespace ts { checkAssignmentOperator(rightType); return getRegularTypeOfObjectLiteral(rightType); case SyntaxKind.CommaToken: + if (isSideEffectFree(left) && !compilerOptions.allowUnreachableCode) { + error(left, Diagnostics.Left_side_of_comma_operator_is_unused_and_has_no_side_effects); + } return rightType; } diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index cddb132f56c..e6e6efd03be 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -1955,6 +1955,10 @@ "category": "Error", "code": 2692 }, + "Left side of comma operator is unused and has no side effects.": { + "category": "Error", + "code": 2693 + }, "Import declaration '{0}' is using private name '{1}'.": { "category": "Error", "code": 4000 diff --git a/tests/baselines/reference/assignmentToParenthesizedExpression1.errors.txt b/tests/baselines/reference/assignmentToParenthesizedExpression1.errors.txt index 0b1b1e04de8..534f74ed799 100644 --- a/tests/baselines/reference/assignmentToParenthesizedExpression1.errors.txt +++ b/tests/baselines/reference/assignmentToParenthesizedExpression1.errors.txt @@ -1,8 +1,11 @@ tests/cases/compiler/assignmentToParenthesizedExpression1.ts(2,1): error TS2364: Invalid left-hand side of assignment expression. +tests/cases/compiler/assignmentToParenthesizedExpression1.ts(2,2): error TS2693: Left operand of comma operator may not be a side-effect free expression. -==== tests/cases/compiler/assignmentToParenthesizedExpression1.ts (1 errors) ==== +==== tests/cases/compiler/assignmentToParenthesizedExpression1.ts (2 errors) ==== var x; (1, x)=0; ~~~~~~ -!!! error TS2364: Invalid left-hand side of assignment expression. \ No newline at end of file +!!! error TS2364: Invalid left-hand side of assignment expression. + ~ +!!! error TS2693: Left operand of comma operator may not be a side-effect free expression. \ No newline at end of file diff --git a/tests/baselines/reference/commaOperator1.errors.txt b/tests/baselines/reference/commaOperator1.errors.txt new file mode 100644 index 00000000000..c0668ceb2f5 --- /dev/null +++ b/tests/baselines/reference/commaOperator1.errors.txt @@ -0,0 +1,33 @@ +tests/cases/compiler/commaOperator1.ts(1,11): error TS2693: Left operand of comma operator may not be a side-effect free expression. +tests/cases/compiler/commaOperator1.ts(1,11): error TS2693: Left operand of comma operator may not be a side-effect free expression. +tests/cases/compiler/commaOperator1.ts(1,11): error TS2693: Left operand of comma operator may not be a side-effect free expression. +tests/cases/compiler/commaOperator1.ts(1,12): error TS2693: Left operand of comma operator may not be a side-effect free expression. +tests/cases/compiler/commaOperator1.ts(1,12): error TS2693: Left operand of comma operator may not be a side-effect free expression. +tests/cases/compiler/commaOperator1.ts(1,29): error TS2693: Left operand of comma operator may not be a side-effect free expression. +tests/cases/compiler/commaOperator1.ts(4,12): error TS2693: Left operand of comma operator may not be a side-effect free expression. +tests/cases/compiler/commaOperator1.ts(4,12): error TS2693: Left operand of comma operator may not be a side-effect free expression. + + +==== tests/cases/compiler/commaOperator1.ts (8 errors) ==== + var v1 = ((1, 2, 3), 4, 5, (6, 7)); + ~~~~~~~~~ +!!! error TS2693: Left operand of comma operator may not be a side-effect free expression. + ~~~~~~~~~~~~ +!!! error TS2693: Left operand of comma operator may not be a side-effect free expression. + ~~~~~~~~~~~~~~~ +!!! error TS2693: Left operand of comma operator may not be a side-effect free expression. + ~ +!!! error TS2693: Left operand of comma operator may not be a side-effect free expression. + ~~~~ +!!! error TS2693: Left operand of comma operator may not be a side-effect free expression. + ~ +!!! error TS2693: Left operand of comma operator may not be a side-effect free expression. + function f1() { + var a = 1; + return a, v1, a; + ~ +!!! error TS2693: Left operand of comma operator may not be a side-effect free expression. + ~~~~~ +!!! error TS2693: Left operand of comma operator may not be a side-effect free expression. + } + \ No newline at end of file diff --git a/tests/cases/compiler/commaOperatorLeftSideUnused.ts b/tests/cases/compiler/commaOperatorLeftSideUnused.ts new file mode 100644 index 00000000000..b52d122b5c0 --- /dev/null +++ b/tests/cases/compiler/commaOperatorLeftSideUnused.ts @@ -0,0 +1,45 @@ +var xx: any; + +function fn() { + let arr: any[] = []; + switch(arr.length) { + // Should error + case 0, 1: + return 'zero or one'; + default: + return 'more than one'; + } +} + +// Should error +let x = Math.pow((3, 5), 2); + +// Should error +let a = [(3 + 4), ((1 + 1, 8) * 4)]; + +// Should be OK +xx = x++, 2; +xx = x = 3, 2; + +// Should error +xx = x = (3, 2); + +// Error cases (object literals) +xx = ({ x: 3 }, 14); +xx = ({ y() { } }, 14); + +// OK cases (object literals) +xx = ({ m: Math.pow(2, 3) }), 14; +xx = ({ ['foo'.substr(3)]: 5 }), 14; + +// Error cases (array literals) +xx = ([1, 2], 4); +xx = ([], 4); +xx = ([[]], 4); +xx = ([{}], ''); +xx = ([false, true, (xx, xx)], 4); + +// OK cases (array literals) +xx = ([1, ''.substr(0)], ''); +xx = ([new Date()], ''); +xx = ([console.log], '');