Skip to content

improve error info for arrow function at right of binary expression #26085

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

Closed
Kingwl wants to merge 1 commit into from
Closed

improve error info for arrow function at right of binary expression #26085

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions src/compiler/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4125,6 +4125,10 @@
"category": "Error",
"code": 17017
},
"Invalid arrow-function arguments, parentheses around the arrow-function may help.": {
Copy link
Copy Markdown
Member

@DanielRosenwasser DanielRosenwasser Aug 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

Arrow functions are not syntactically valid here. Consider wrapping the function in parentheses.

"category": "Error",
"code": 17018
},

"Circularity detected while resolving configuration: {0}": {
"category": "Error",
Expand Down
21 changes: 20 additions & 1 deletion src/compiler/parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3791,7 +3791,18 @@ namespace ts {
}
}
else {
leftOperand = makeBinaryExpression(leftOperand, parseTokenNode(), parseBinaryExpressionOrHigher(newPrecedence));
const op = parseTokenNode<Token<BinaryOperator>>();
Copy link
Copy Markdown
Member

@DanielRosenwasser DanielRosenwasser Aug 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leave a comment like

// Much of the time, users will write code along the lines of
//
//   let x = foo || () => { /*...*/ }
//
// However, arrow functions aren't valid in those positions in the ECMAScript grammar.
// Despite that, we'll try to parse it out anyway and give a decent error message.

if (isParenthesizedArrowFunctionExpression() !== Tristate.False) {
const maybeArrow = lookAhead(tryParseParenthesizedArrowFunctionExpression);
if (maybeArrow) {
parseErrorAtRange(maybeArrow, Diagnostics.Invalid_arrow_function_arguments_parentheses_around_the_arrow_function_may_help);
}
}
else if (lookAhead(isStartOfSimpleArrowFunction)) {
const maybeArrow = tryParseAsyncSimpleArrowFunctionExpression() || lookAhead(() => parseSimpleArrowFunctionExpression(<Identifier>parseBinaryExpressionOrHigher(/*precedence*/ 0)));
parseErrorAtRange(maybeArrow, Diagnostics.Invalid_arrow_function_arguments_parentheses_around_the_arrow_function_may_help);
}
leftOperand = makeBinaryExpression(leftOperand, op, parseBinaryExpressionOrHigher(newPrecedence));
Copy link
Copy Markdown
Member

@DanielRosenwasser DanielRosenwasser Aug 22, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're currently ignoring it if it's present, but I think that you actually want to gracefully parse the arrow functions you've been able to find above.

Throwing away the result and trying to parse as before actually provides a slightly more confusing error experience.

}
}

Expand Down Expand Up @@ -6015,6 +6026,14 @@ namespace ts {
return nextToken() === SyntaxKind.SlashToken;
}

function isStartOfSimpleArrowFunction () {
Copy link
Copy Markdown
Member

@DanielRosenwasser DanielRosenwasser Aug 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: not space before parentheses

if (token() === SyntaxKind.AsyncKeyword) {
nextToken();
return isIdentifier() && nextToken() === SyntaxKind.EqualsGreaterThanToken || token() === SyntaxKind.EqualsGreaterThanToken;
Copy link
Copy Markdown
Member

@DanielRosenwasser DanielRosenwasser Aug 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you leave a comment on the intent here? Is this supposed to cover async => 100?

}
return isIdentifier() && nextToken() === SyntaxKind.EqualsGreaterThanToken;
}

function parseNamespaceExportDeclaration(node: NamespaceExportDeclaration): NamespaceExportDeclaration {
node.kind = SyntaxKind.NamespaceExportDeclaration;
parseExpected(SyntaxKind.AsKeyword);
Expand Down
5 changes: 4 additions & 1 deletion tests/baselines/reference/ambiguousGenericAssertion1.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,20 @@
tests/cases/compiler/ambiguousGenericAssertion1.ts(4,10): error TS1109: Expression expected.
tests/cases/compiler/ambiguousGenericAssertion1.ts(4,14): error TS17018: Invalid arrow-function arguments, parentheses around the arrow-function may help.
tests/cases/compiler/ambiguousGenericAssertion1.ts(4,15): error TS2304: Cannot find name 'x'.
tests/cases/compiler/ambiguousGenericAssertion1.ts(4,16): error TS1005: ')' expected.
tests/cases/compiler/ambiguousGenericAssertion1.ts(4,19): error TS1005: ',' expected.
tests/cases/compiler/ambiguousGenericAssertion1.ts(4,21): error TS1005: ';' expected.


==== tests/cases/compiler/ambiguousGenericAssertion1.ts (5 errors) ====
==== tests/cases/compiler/ambiguousGenericAssertion1.ts (6 errors) ====
function f<T>(x: T): T { return null; }
var r = <T>(x: T) => x;
var r2 = < <T>(x: T) => T>f; // valid
var r3 = <<T>(x: T) => T>f; // ambiguous, appears to the parser as a << operation
~~
!!! error TS1109: Expression expected.
~~~~~~~~~~~~~
!!! error TS17018: Invalid arrow-function arguments, parentheses around the arrow-function may help.
~
!!! error TS2304: Cannot find name 'x'.
~
Expand Down
127 changes: 127 additions & 0 deletions tests/baselines/reference/incorrectArrowFunction.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
tests/cases/compiler/incorrectArrowFunction.ts(7,8): error TS17018: Invalid arrow-function arguments, parentheses around the arrow-function may help.
tests/cases/compiler/incorrectArrowFunction.ts(7,10): error TS1109: Expression expected.
tests/cases/compiler/incorrectArrowFunction.ts(7,12): error TS1005: ';' expected.
tests/cases/compiler/incorrectArrowFunction.ts(8,8): error TS17018: Invalid arrow-function arguments, parentheses around the arrow-function may help.
tests/cases/compiler/incorrectArrowFunction.ts(8,9): error TS2304: Cannot find name 'a'.
tests/cases/compiler/incorrectArrowFunction.ts(8,11): error TS1005: ';' expected.
tests/cases/compiler/incorrectArrowFunction.ts(9,8): error TS17018: Invalid arrow-function arguments, parentheses around the arrow-function may help.
tests/cases/compiler/incorrectArrowFunction.ts(9,10): error TS2304: Cannot find name 'a'.
tests/cases/compiler/incorrectArrowFunction.ts(9,13): error TS1005: ';' expected.
tests/cases/compiler/incorrectArrowFunction.ts(10,8): error TS17018: Invalid arrow-function arguments, parentheses around the arrow-function may help.
tests/cases/compiler/incorrectArrowFunction.ts(10,13): error TS1005: ';' expected.
tests/cases/compiler/incorrectArrowFunction.ts(11,8): error TS17018: Invalid arrow-function arguments, parentheses around the arrow-function may help.
tests/cases/compiler/incorrectArrowFunction.ts(11,10): error TS2304: Cannot find name 'a'.
tests/cases/compiler/incorrectArrowFunction.ts(11,11): error TS1005: ')' expected.
tests/cases/compiler/incorrectArrowFunction.ts(11,13): error TS2693: 'number' only refers to a type, but is being used as a value here.
tests/cases/compiler/incorrectArrowFunction.ts(11,19): error TS1005: ';' expected.
tests/cases/compiler/incorrectArrowFunction.ts(11,21): error TS1128: Declaration or statement expected.
tests/cases/compiler/incorrectArrowFunction.ts(12,8): error TS17018: Invalid arrow-function arguments, parentheses around the arrow-function may help.
tests/cases/compiler/incorrectArrowFunction.ts(12,12): error TS2304: Cannot find name 'a'.
tests/cases/compiler/incorrectArrowFunction.ts(12,15): error TS1005: ')' expected.
tests/cases/compiler/incorrectArrowFunction.ts(12,17): error TS2693: 'Foo' only refers to a type, but is being used as a value here.
tests/cases/compiler/incorrectArrowFunction.ts(12,20): error TS1005: ';' expected.
tests/cases/compiler/incorrectArrowFunction.ts(12,22): error TS1128: Declaration or statement expected.
tests/cases/compiler/incorrectArrowFunction.ts(13,8): error TS17018: Invalid arrow-function arguments, parentheses around the arrow-function may help.
tests/cases/compiler/incorrectArrowFunction.ts(13,9): error TS2304: Cannot find name 'async'.
tests/cases/compiler/incorrectArrowFunction.ts(13,18): error TS1005: ';' expected.
tests/cases/compiler/incorrectArrowFunction.ts(14,8): error TS17018: Invalid arrow-function arguments, parentheses around the arrow-function may help.
tests/cases/compiler/incorrectArrowFunction.ts(14,21): error TS1109: Expression expected.
tests/cases/compiler/incorrectArrowFunction.ts(15,8): error TS17018: Invalid arrow-function arguments, parentheses around the arrow-function may help.
tests/cases/compiler/incorrectArrowFunction.ts(15,9): error TS2304: Cannot find name 'async'.
tests/cases/compiler/incorrectArrowFunction.ts(15,15): error TS1005: ';' expected.


==== tests/cases/compiler/incorrectArrowFunction.ts (31 errors) ====
declare const foo;

interface Foo {
a: number
}

true || () => 1;
~~~~~~~~
!!! error TS17018: Invalid arrow-function arguments, parentheses around the arrow-function may help.
~
!!! error TS1109: Expression expected.
~~
!!! error TS1005: ';' expected.
true || a => 1;
~~~~~~~
!!! error TS17018: Invalid arrow-function arguments, parentheses around the arrow-function may help.
~
!!! error TS2304: Cannot find name 'a'.
~~
!!! error TS1005: ';' expected.
true || (a) => 2;
~~~~~~~~~
!!! error TS17018: Invalid arrow-function arguments, parentheses around the arrow-function may help.
~
!!! error TS2304: Cannot find name 'a'.
~~
!!! error TS1005: ';' expected.
true || foo => 1;
~~~~~~~~~
!!! error TS17018: Invalid arrow-function arguments, parentheses around the arrow-function may help.
~~
!!! error TS1005: ';' expected.
true || (a: number) => 1;
~~~~~~~~~~~~~~~~~
!!! error TS17018: Invalid arrow-function arguments, parentheses around the arrow-function may help.
~
!!! error TS2304: Cannot find name 'a'.
~
!!! error TS1005: ')' expected.
~~~~~~
!!! error TS2693: 'number' only refers to a type, but is being used as a value here.
~
!!! error TS1005: ';' expected.
~~
!!! error TS1128: Declaration or statement expected.
true || ({ a }: Foo) => 1;
~~~~~~~~~~~~~~~~~~
!!! error TS17018: Invalid arrow-function arguments, parentheses around the arrow-function may help.
~
!!! error TS2304: Cannot find name 'a'.
~
!!! error TS1005: ')' expected.
~~~
!!! error TS2693: 'Foo' only refers to a type, but is being used as a value here.
~
!!! error TS1005: ';' expected.
~~
!!! error TS1128: Declaration or statement expected.
true || async () => 1;
~~~~~~~~~~~~~~
!!! error TS17018: Invalid arrow-function arguments, parentheses around the arrow-function may help.
~~~~~
!!! error TS2304: Cannot find name 'async'.
~~
!!! error TS1005: ';' expected.
true || async a => 1;
~~~~~~~~~~~~~
!!! error TS17018: Invalid arrow-function arguments, parentheses around the arrow-function may help.
~
!!! error TS1109: Expression expected.
true || async => 1;
~~~~~~~~~~~
Copy link
Copy Markdown
Member

@DanielRosenwasser DanielRosenwasser Aug 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You really don't want the error span to include whitespace.

!!! error TS17018: Invalid arrow-function arguments, parentheses around the arrow-function may help.
~~~~~
!!! error TS2304: Cannot find name 'async'.
~~
!!! error TS1005: ';' expected.

true || (() => 1);
true || (a => 1);
true || ((a) => 2);
true || (foo => 1);
true || ((a: number) => 1);
true || (({ a }: Foo) => 1);
true || (async () => 1);
true || (async a => 1);
true || (async => 1);
(() => 1) || (() => 2);

true || (false);
true || false;
true || foo;

109 changes: 109 additions & 0 deletions tests/baselines/reference/incorrectArrowFunction.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
//// [incorrectArrowFunction.ts]
declare const foo;

interface Foo {
a: number
}

true || () => 1;
true || a => 1;
true || (a) => 2;
true || foo => 1;
true || (a: number) => 1;
true || ({ a }: Foo) => 1;
true || async () => 1;
true || async a => 1;
true || async => 1;

true || (() => 1);
true || (a => 1);
true || ((a) => 2);
true || (foo => 1);
true || ((a: number) => 1);
true || (({ a }: Foo) => 1);
true || (async () => 1);
true || (async a => 1);
true || (async => 1);
(() => 1) || (() => 2);

true || (false);
true || false;
true || foo;


//// [incorrectArrowFunction.js]
var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, generator) {
return new (P || (P = Promise))(function (resolve, reject) {
function fulfilled(value) { try { step(generator.next(value)); } catch (e) { reject(e); } }
function rejected(value) { try { step(generator["throw"](value)); } catch (e) { reject(e); } }
function step(result) { result.done ? resolve(result.value) : new P(function (resolve) { resolve(result.value); }).then(fulfilled, rejected); }
step((generator = generator.apply(thisArg, _arguments || [])).next());
});
};
var __generator = (this && this.__generator) || function (thisArg, body) {
var _ = { label: 0, sent: function() { if (t[0] & 1) throw t[1]; return t[1]; }, trys: [], ops: [] }, f, y, t, g;
return g = { next: verb(0), "throw": verb(1), "return": verb(2) }, typeof Symbol === "function" && (g[Symbol.iterator] = function() { return this; }), g;
function verb(n) { return function (v) { return step([n, v]); }; }
function step(op) {
if (f) throw new TypeError("Generator is already executing.");
while (_) try {
if (f = 1, y && (t = op[0] & 2 ? y["return"] : op[0] ? y["throw"] || ((t = y["return"]) && t.call(y), 0) : y.next) && !(t = t.call(y, op[1])).done) return t;
if (y = 0, t) op = [op[0] & 2, t.value];
switch (op[0]) {
case 0: case 1: t = op; break;
case 4: _.label++; return { value: op[1], done: false };
case 5: _.label++; y = op[1]; op = [0]; continue;
case 7: op = _.ops.pop(); _.trys.pop(); continue;
default:
if (!(t = _.trys, t = t.length > 0 && t[t.length - 1]) && (op[0] === 6 || op[0] === 2)) { _ = 0; continue; }
if (op[0] === 3 && (!t || (op[1] > t[0] && op[1] < t[3]))) { _.label = op[1]; break; }
if (op[0] === 6 && _.label < t[1]) { _.label = t[1]; t = op; break; }
if (t && _.label < t[2]) { _.label = t[2]; _.ops.push(op); break; }
if (t[2]) _.ops.pop();
_.trys.pop(); continue;
}
op = body.call(thisArg, _);
} catch (e) { op = [6, e]; y = 0; } finally { f = t = 0; }
if (op[0] & 5) throw op[1]; return { value: op[0] ? op[1] : void 0, done: true };
}
};
var _this = this;
true || ();
1;
true || a;
1;
true || (a);
2;
true || foo;
1;
true || (a);
number;
1;
true || ({ a: a });
Foo;
1;
true || async();
1;
true || ;
true || async;
1;
true || (function () { return 1; });
true || (function (a) { return 1; });
true || (function (a) { return 2; });
true || (function (foo) { return 1; });
true || (function (a) { return 1; });
true || (function (_a) {
var a = _a.a;
return 1;
});
true || (function () { return __awaiter(_this, void 0, void 0, function () { return __generator(this, function (_a) {
return [2 /*return*/, 1];
}); }); });
true || (function (a) { return __awaiter(_this, void 0, void 0, function () { return __generator(this, function (_a) {
return [2 /*return*/, 1];
}); }); });
true || (function (async) { return 1; });
(function () { return 1; }) || (function () { return 2; });
true || (false);
true || false;
true || foo;
56 changes: 56 additions & 0 deletions tests/baselines/reference/incorrectArrowFunction.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
=== tests/cases/compiler/incorrectArrowFunction.ts ===
declare const foo;
>foo : Symbol(foo, Decl(incorrectArrowFunction.ts, 0, 13))

interface Foo {
>Foo : Symbol(Foo, Decl(incorrectArrowFunction.ts, 0, 18))

a: number
>a : Symbol(Foo.a, Decl(incorrectArrowFunction.ts, 2, 15))
}

true || () => 1;
true || a => 1;
true || (a) => 2;
true || foo => 1;
>foo : Symbol(foo, Decl(incorrectArrowFunction.ts, 0, 13))

true || (a: number) => 1;
true || ({ a }: Foo) => 1;
>a : Symbol(a, Decl(incorrectArrowFunction.ts, 11, 10))

true || async () => 1;
true || async a => 1;
true || async => 1;

true || (() => 1);
true || (a => 1);
>a : Symbol(a, Decl(incorrectArrowFunction.ts, 17, 9))

true || ((a) => 2);
>a : Symbol(a, Decl(incorrectArrowFunction.ts, 18, 10))

true || (foo => 1);
>foo : Symbol(foo, Decl(incorrectArrowFunction.ts, 19, 9))

true || ((a: number) => 1);
>a : Symbol(a, Decl(incorrectArrowFunction.ts, 20, 10))

true || (({ a }: Foo) => 1);
>a : Symbol(a, Decl(incorrectArrowFunction.ts, 21, 11))
>Foo : Symbol(Foo, Decl(incorrectArrowFunction.ts, 0, 18))

true || (async () => 1);
true || (async a => 1);
>a : Symbol(a, Decl(incorrectArrowFunction.ts, 23, 14))

true || (async => 1);
>async : Symbol(async, Decl(incorrectArrowFunction.ts, 24, 9))

(() => 1) || (() => 2);

true || (false);
true || false;
true || foo;
>foo : Symbol(foo, Decl(incorrectArrowFunction.ts, 0, 13))

Loading