Skip to content

Commit

Permalink
Error on binding patterns and improve error location
Browse files Browse the repository at this point in the history
  • Loading branch information
Andy Hanson committed Mar 1, 2018
1 parent 0a32978 commit f69d5b4
Show file tree
Hide file tree
Showing 13 changed files with 122 additions and 29 deletions.
15 changes: 7 additions & 8 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18824,6 +18824,7 @@ namespace ts {

function checkObjectLiteralAssignment(node: ObjectLiteralExpression, sourceType: Type): Type {
const properties = node.properties;
checkGrammarForDisallowedTrailingComma(properties, Diagnostics.A_rest_parameter_or_binding_pattern_may_not_have_a_trailing_comma);
if (strictNullChecks && properties.length === 0) {
return checkNonNullType(sourceType, node);
}
Expand Down Expand Up @@ -18882,6 +18883,8 @@ namespace ts {
}

function checkArrayLiteralAssignment(node: ArrayLiteralExpression, sourceType: Type, checkMode?: CheckMode): Type {
const elements = node.elements;
checkGrammarForDisallowedTrailingComma(elements, Diagnostics.A_rest_parameter_or_binding_pattern_may_not_have_a_trailing_comma);
if (languageVersion < ScriptTarget.ES2015 && compilerOptions.downlevelIteration) {
checkExternalEmitHelpers(node, ExternalEmitHelpers.Read);
}
Expand All @@ -18890,7 +18893,6 @@ namespace ts {
// present (aka the tuple element property). This call also checks that the parentType is in
// fact an iterable or array (depending on target language).
const elementType = checkIteratedTypeOrElementType(sourceType, node, /*allowStringInput*/ false, /*allowAsyncIterables*/ false) || unknownType;
const elements = node.elements;
for (let i = 0; i < elements.length; i++) {
checkArrayLiteralDestructuringElementAssignment(node, sourceType, i, elementType, checkMode);
}
Expand Down Expand Up @@ -26067,11 +26069,9 @@ namespace ts {
return grammarErrorOnNode(asyncModifier, Diagnostics._0_modifier_cannot_be_used_here, "async");
}

function checkGrammarForDisallowedTrailingComma(list: NodeArray<Node>): boolean {
function checkGrammarForDisallowedTrailingComma(list: NodeArray<Node>, diag = Diagnostics.Trailing_comma_not_allowed): boolean {
if (list && list.hasTrailingComma) {
const start = list.end - ",".length;
const end = list.end;
return grammarErrorAtPos(list[0], start, end - start, Diagnostics.Trailing_comma_not_allowed);
return grammarErrorAtPos(list[0], list.end - ",".length, ",".length, diag);
}
}

Expand All @@ -26093,9 +26093,7 @@ namespace ts {
if (i !== (parameterCount - 1)) {
return grammarErrorOnNode(parameter.dotDotDotToken, Diagnostics.A_rest_parameter_must_be_last_in_a_parameter_list);
}
if (parameters.hasTrailingComma) {
return grammarErrorOnNode(parameter.dotDotDotToken, Diagnostics.A_rest_parameter_may_not_have_a_trailing_comma);
}
checkGrammarForDisallowedTrailingComma(parameters, Diagnostics.A_rest_parameter_or_binding_pattern_may_not_have_a_trailing_comma);

if (isBindingPattern(parameter.name)) {
return grammarErrorOnNode(parameter.name, Diagnostics.A_rest_element_cannot_contain_a_binding_pattern);
Expand Down Expand Up @@ -26709,6 +26707,7 @@ namespace ts {
if (node !== last(elements)) {
return grammarErrorOnNode(node, Diagnostics.A_rest_element_must_be_last_in_a_destructuring_pattern);
}
checkGrammarForDisallowedTrailingComma(elements, Diagnostics.A_rest_parameter_or_binding_pattern_may_not_have_a_trailing_comma);

if (node.name.kind === SyntaxKind.ArrayBindingPattern || node.name.kind === SyntaxKind.ObjectBindingPattern) {
return grammarErrorOnNode(node.name, Diagnostics.A_rest_element_cannot_contain_a_binding_pattern);
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"category": "Error",
"code": 1012
},
"A rest parameter may not have a trailing comma.": {
"A rest parameter or binding pattern may not have a trailing comma.": {
"category": "Error",
"code": 1013
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
tests/cases/conformance/es7/trailingCommasInBindingPatterns.ts(1,12): error TS1013: A rest parameter or binding pattern may not have a trailing comma.
tests/cases/conformance/es7/trailingCommasInBindingPatterns.ts(2,12): error TS1013: A rest parameter or binding pattern may not have a trailing comma.
tests/cases/conformance/es7/trailingCommasInBindingPatterns.ts(4,7): error TS1013: A rest parameter or binding pattern may not have a trailing comma.
tests/cases/conformance/es7/trailingCommasInBindingPatterns.ts(5,7): error TS1013: A rest parameter or binding pattern may not have a trailing comma.


==== tests/cases/conformance/es7/trailingCommasInBindingPatterns.ts (4 errors) ====
const [...a,] = [];
~
!!! error TS1013: A rest parameter or binding pattern may not have a trailing comma.
const {...b,} = {};
~
!!! error TS1013: A rest parameter or binding pattern may not have a trailing comma.
let c, d;
([...c,] = []);
~
!!! error TS1013: A rest parameter or binding pattern may not have a trailing comma.
({...d,} = {});
~
!!! error TS1013: A rest parameter or binding pattern may not have a trailing comma.

23 changes: 23 additions & 0 deletions tests/baselines/reference/trailingCommasInBindingPatterns.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
//// [trailingCommasInBindingPatterns.ts]
const [...a,] = [];
const {...b,} = {};
let c, d;
([...c,] = []);
({...d,} = {});


//// [trailingCommasInBindingPatterns.js]
var __rest = (this && this.__rest) || function (s, e) {
var t = {};
for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p) && e.indexOf(p) < 0)
t[p] = s[p];
if (s != null && typeof Object.getOwnPropertySymbols === "function")
for (var i = 0, p = Object.getOwnPropertySymbols(s); i < p.length; i++) if (e.indexOf(p[i]) < 0)
t[p[i]] = s[p[i]];
return t;
};
var a = [].slice(0);
var b = __rest({}, []);
var c, d;
(c = [].slice(0));
(d = __rest({}, []));
17 changes: 17 additions & 0 deletions tests/baselines/reference/trailingCommasInBindingPatterns.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
=== tests/cases/conformance/es7/trailingCommasInBindingPatterns.ts ===
const [...a,] = [];
>a : Symbol(a, Decl(trailingCommasInBindingPatterns.ts, 0, 7))

const {...b,} = {};
>b : Symbol(b, Decl(trailingCommasInBindingPatterns.ts, 1, 7))

let c, d;
>c : Symbol(c, Decl(trailingCommasInBindingPatterns.ts, 2, 3))
>d : Symbol(d, Decl(trailingCommasInBindingPatterns.ts, 2, 6))

([...c,] = []);
>c : Symbol(c, Decl(trailingCommasInBindingPatterns.ts, 2, 3))

({...d,} = {});
>d : Symbol(d, Decl(trailingCommasInBindingPatterns.ts, 2, 6))

28 changes: 28 additions & 0 deletions tests/baselines/reference/trailingCommasInBindingPatterns.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
=== tests/cases/conformance/es7/trailingCommasInBindingPatterns.ts ===
const [...a,] = [];
>a : any[]
>[] : undefined[]

const {...b,} = {};
>b : {}
>{} : {}

let c, d;
>c : any
>d : any

([...c,] = []);
>([...c,] = []) : undefined[]
>[...c,] = [] : undefined[]
>[...c,] : undefined[]
>...c : any
>c : any
>[] : undefined[]

({...d,} = {});
>({...d,} = {}) : {}
>{...d,} = {} : {}
>{...d,} : any
>d : any
>{} : {}

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
tests/cases/conformance/es7/trailingCommasInFunctionParametersAndArguments.ts(5,13): error TS1013: A rest parameter may not have a trailing comma.
tests/cases/conformance/es7/trailingCommasInFunctionParametersAndArguments.ts(5,20): error TS1013: A rest parameter or binding pattern may not have a trailing comma.


==== tests/cases/conformance/es7/trailingCommasInFunctionParametersAndArguments.ts (1 errors) ====
Expand All @@ -7,8 +7,8 @@ tests/cases/conformance/es7/trailingCommasInFunctionParametersAndArguments.ts(5,
f1(1,);

function f2(...args,) {}
~~~
!!! error TS1013: A rest parameter may not have a trailing comma.
~
!!! error TS1013: A rest parameter or binding pattern may not have a trailing comma.

f2(...[],);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
tests/cases/compiler/unusedLocalsAndObjectSpread2.ts(5,6): error TS6133: 'rest' is declared but its value is never read.
tests/cases/compiler/unusedLocalsAndObjectSpread2.ts(8,10): error TS6133: 'foo' is declared but its value is never read.
tests/cases/compiler/unusedLocalsAndObjectSpread2.ts(12,8): error TS6133: 'rest' is declared but its value is never read.
tests/cases/compiler/unusedLocalsAndObjectSpread2.ts(12,12): error TS6133: 'rest' is declared but its value is never read.


==== tests/cases/compiler/unusedLocalsAndObjectSpread2.ts (3 errors) ====
declare let props: any;
const {
children, // here!
active: _a, // here!
...rest,
...rest
~~~~
!!! error TS6133: 'rest' is declared but its value is never read.
} = props;
Expand All @@ -19,10 +19,10 @@ tests/cases/compiler/unusedLocalsAndObjectSpread2.ts(12,8): error TS6133: 'rest'
const {
children,
active: _a,
...rest,
~~~~
...rest
~~~~
!!! error TS6133: 'rest' is declared but its value is never read.
} = props;
} = props;
}

export const asdf = 123;
6 changes: 3 additions & 3 deletions tests/baselines/reference/unusedLocalsAndObjectSpread2.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ declare let props: any;
const {
children, // here!
active: _a, // here!
...rest,
...rest
} = props;

function foo() {
const {
children,
active: _a,
...rest,
} = props;
...rest
} = props;
}

export const asdf = 123;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const {
active: _a, // here!
>_a : Symbol(_a, Decl(unusedLocalsAndObjectSpread2.ts, 2, 13))

...rest,
...rest
>rest : Symbol(rest, Decl(unusedLocalsAndObjectSpread2.ts, 3, 15))

} = props;
Expand All @@ -25,10 +25,10 @@ function foo() {
active: _a,
>_a : Symbol(_a, Decl(unusedLocalsAndObjectSpread2.ts, 9, 17))

...rest,
...rest
>rest : Symbol(rest, Decl(unusedLocalsAndObjectSpread2.ts, 10, 19))

} = props;
} = props;
>props : Symbol(props, Decl(unusedLocalsAndObjectSpread2.ts, 0, 11))
}

Expand Down
6 changes: 3 additions & 3 deletions tests/baselines/reference/unusedLocalsAndObjectSpread2.types
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const {
>active : any
>_a : any

...rest,
...rest
>rest : any

} = props;
Expand All @@ -27,10 +27,10 @@ function foo() {
>active : any
>_a : any

...rest,
...rest
>rest : any

} = props;
} = props;
>props : any
}

Expand Down
6 changes: 3 additions & 3 deletions tests/cases/compiler/unusedLocalsAndObjectSpread2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@ declare let props: any;
const {
children, // here!
active: _a, // here!
...rest,
...rest
} = props;

function foo() {
const {
children,
active: _a,
...rest,
} = props;
...rest
} = props;
}

export const asdf = 123;
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
const [...a,] = [];
const {...b,} = {};
let c, d;
([...c,] = []);
({...d,} = {});

0 comments on commit f69d5b4

Please sign in to comment.