Skip to content

Commit ab0a788

Browse files
RyanCavanaughRyanCavanaugh
committed
Disallow comma operator when LHS is pure
1 parent b052d69 commit ab0a788

File tree

5 files changed

+166
-2
lines changed

5 files changed

+166
-2
lines changed

‎src/compiler/checker.ts

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13209,6 +13209,82 @@ namespace ts {
1320913209
return sourceType;
1321013210
}
1321113211

13212+
function isSideEffectFree(node: Node): boolean {
13213+
switch (node.kind) {
13214+
case SyntaxKind.Identifier:
13215+
case SyntaxKind.StringLiteral:
13216+
case SyntaxKind.RegularExpressionLiteral:
13217+
case SyntaxKind.NoSubstitutionTemplateLiteral:
13218+
case SyntaxKind.NumericLiteral:
13219+
case SyntaxKind.TrueKeyword:
13220+
case SyntaxKind.FalseKeyword:
13221+
case SyntaxKind.NullKeyword:
13222+
case SyntaxKind.UndefinedKeyword:
13223+
case SyntaxKind.FunctionExpression:
13224+
case SyntaxKind.ArrowFunction:
13225+
return true;
13226+
13227+
case SyntaxKind.BinaryExpression:
13228+
if (isAssignmentOperator((node as BinaryExpression).operatorToken.kind)) {
13229+
return false;
13230+
}
13231+
return isSideEffectFree((node as BinaryExpression).left) &&
13232+
isSideEffectFree((node as BinaryExpression).right);
13233+
13234+
case SyntaxKind.PrefixUnaryExpression:
13235+
case SyntaxKind.PostfixUnaryExpression:
13236+
switch ((node as PrefixUnaryExpression).operator) {
13237+
case SyntaxKind.ExclamationToken:
13238+
case SyntaxKind.PlusToken:
13239+
case SyntaxKind.MinusToken:
13240+
case SyntaxKind.TildeToken:
13241+
return isSideEffectFree((node as PrefixUnaryExpression).operand);
13242+
}
13243+
return false;
13244+
13245+
case SyntaxKind.ParenthesizedExpression:
13246+
case SyntaxKind.TypeAssertionExpression:
13247+
case SyntaxKind.TypeOfExpression:
13248+
case SyntaxKind.AsExpression:
13249+
case SyntaxKind.NonNullExpression:
13250+
// All the nodes which just have a .expression we should check
13251+
return isSideEffectFree((node as ParenthesizedExpression).expression);
13252+
13253+
case SyntaxKind.ConditionalExpression:
13254+
return isSideEffectFree((node as ConditionalExpression).condition) &&
13255+
isSideEffectFree((node as ConditionalExpression).whenTrue) &&
13256+
isSideEffectFree((node as ConditionalExpression).whenFalse);
13257+
13258+
case SyntaxKind.ObjectLiteralExpression:
13259+
// Note: negated forEach condition since we want to bail early to 'true',
13260+
// in other words the callback is computing "Could have side effects?"
13261+
return !forEach((node as ObjectLiteralExpression).properties, prop => {
13262+
if (isComputedPropertyName(prop.name) && !isSideEffectFree((prop.name as ComputedPropertyName).expression)) {
13263+
return true;
13264+
}
13265+
13266+
switch (prop.kind) {
13267+
case SyntaxKind.ShorthandPropertyAssignment:
13268+
case SyntaxKind.MethodDeclaration:
13269+
return false;
13270+
case SyntaxKind.PropertyAssignment:
13271+
return !isSideEffectFree((prop as PropertyAssignment).initializer);
13272+
default:
13273+
Debug.fail('Unhandled object literal property kind ' + prop.kind);
13274+
}
13275+
});
13276+
13277+
case SyntaxKind.ArrayLiteralExpression:
13278+
// See previous comment about negated callback values
13279+
return !forEach((node as ArrayLiteralExpression).elements, elem => !isSideEffectFree(elem));
13280+
13281+
case SyntaxKind.VoidExpression:
13282+
// Explicit opt-out case (listed here to avoid accidental duplication above): 'void x'
13283+
default:
13284+
return false;
13285+
}
13286+
}
13287+
1321213288
function isTypeEqualityComparableTo(source: Type, target: Type) {
1321313289
return (target.flags & TypeFlags.Nullable) !== 0 || isTypeComparableTo(source, target);
1321413290
}
@@ -13370,6 +13446,9 @@ namespace ts {
1337013446
checkAssignmentOperator(rightType);
1337113447
return getRegularTypeOfObjectLiteral(rightType);
1337213448
case SyntaxKind.CommaToken:
13449+
if (isSideEffectFree(left) && !compilerOptions.allowUnreachableCode) {
13450+
error(left, Diagnostics.Left_side_of_comma_operator_is_unused_and_has_no_side_effects);
13451+
}
1337313452
return rightType;
1337413453
}
1337513454

‎src/compiler/diagnosticMessages.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1955,6 +1955,10 @@
19551955
"category": "Error",
19561956
"code": 2692
19571957
},
1958+
"Left side of comma operator is unused and has no side effects.": {
1959+
"category": "Error",
1960+
"code": 2693
1961+
},
19581962
"Import declaration '{0}' is using private name '{1}'.": {
19591963
"category": "Error",
19601964
"code": 4000

‎tests/baselines/reference/assignmentToParenthesizedExpression1.errors.txt

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
tests/cases/compiler/assignmentToParenthesizedExpression1.ts(2,1): error TS2364: Invalid left-hand side of assignment expression.
2+
tests/cases/compiler/assignmentToParenthesizedExpression1.ts(2,2): error TS2693: Left operand of comma operator may not be a side-effect free expression.
23

34

4-
==== tests/cases/compiler/assignmentToParenthesizedExpression1.ts (1 errors) ====
5+
==== tests/cases/compiler/assignmentToParenthesizedExpression1.ts (2 errors) ====
56
var x;
67
(1, x)=0;
78
~~~~~~
8-
!!! error TS2364: Invalid left-hand side of assignment expression.
9+
!!! error TS2364: Invalid left-hand side of assignment expression.
10+
~
11+
!!! error TS2693: Left operand of comma operator may not be a side-effect free expression.

‎tests/baselines/reference/commaOperator1.errors.txt

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
tests/cases/compiler/commaOperator1.ts(1,11): error TS2693: Left operand of comma operator may not be a side-effect free expression.
2+
tests/cases/compiler/commaOperator1.ts(1,11): error TS2693: Left operand of comma operator may not be a side-effect free expression.
3+
tests/cases/compiler/commaOperator1.ts(1,11): error TS2693: Left operand of comma operator may not be a side-effect free expression.
4+
tests/cases/compiler/commaOperator1.ts(1,12): error TS2693: Left operand of comma operator may not be a side-effect free expression.
5+
tests/cases/compiler/commaOperator1.ts(1,12): error TS2693: Left operand of comma operator may not be a side-effect free expression.
6+
tests/cases/compiler/commaOperator1.ts(1,29): error TS2693: Left operand of comma operator may not be a side-effect free expression.
7+
tests/cases/compiler/commaOperator1.ts(4,12): error TS2693: Left operand of comma operator may not be a side-effect free expression.
8+
tests/cases/compiler/commaOperator1.ts(4,12): error TS2693: Left operand of comma operator may not be a side-effect free expression.
9+
10+
11+
==== tests/cases/compiler/commaOperator1.ts (8 errors) ====
12+
var v1 = ((1, 2, 3), 4, 5, (6, 7));
13+
~~~~~~~~~
14+
!!! error TS2693: Left operand of comma operator may not be a side-effect free expression.
15+
~~~~~~~~~~~~
16+
!!! error TS2693: Left operand of comma operator may not be a side-effect free expression.
17+
~~~~~~~~~~~~~~~
18+
!!! error TS2693: Left operand of comma operator may not be a side-effect free expression.
19+
~
20+
!!! error TS2693: Left operand of comma operator may not be a side-effect free expression.
21+
~~~~
22+
!!! error TS2693: Left operand of comma operator may not be a side-effect free expression.
23+
~
24+
!!! error TS2693: Left operand of comma operator may not be a side-effect free expression.
25+
function f1() {
26+
var a = 1;
27+
return a, v1, a;
28+
~
29+
!!! error TS2693: Left operand of comma operator may not be a side-effect free expression.
30+
~~~~~
31+
!!! error TS2693: Left operand of comma operator may not be a side-effect free expression.
32+
}
33+

‎tests/cases/compiler/commaOperatorLeftSideUnused.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
var xx: any;
2+
3+
function fn() {
4+
let arr: any[] = [];
5+
switch(arr.length) {
6+
// Should error
7+
case 0, 1:
8+
return 'zero or one';
9+
default:
10+
return 'more than one';
11+
}
12+
}
13+
14+
// Should error
15+
let x = Math.pow((3, 5), 2);
16+
17+
// Should error
18+
let a = [(3 + 4), ((1 + 1, 8) * 4)];
19+
20+
// Should be OK
21+
xx = x++, 2;
22+
xx = x = 3, 2;
23+
24+
// Should error
25+
xx = x = (3, 2);
26+
27+
// Error cases (object literals)
28+
xx = ({ x: 3 }, 14);
29+
xx = ({ y() { } }, 14);
30+
31+
// OK cases (object literals)
32+
xx = ({ m: Math.pow(2, 3) }), 14;
33+
xx = ({ ['foo'.substr(3)]: 5 }), 14;
34+
35+
// Error cases (array literals)
36+
xx = ([1, 2], 4);
37+
xx = ([], 4);
38+
xx = ([[]], 4);
39+
xx = ([{}], '');
40+
xx = ([false, true, (xx, xx)], 4);
41+
42+
// OK cases (array literals)
43+
xx = ([1, ''.substr(0)], '');
44+
xx = ([new Date()], '');
45+
xx = ([console.log], '');

0 commit comments

Comments
 (0)