Skip to content

Disable isStartOfType's lookahead when called from isStartOfParameter #18296

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.

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

Merged
merged 3 commits into from
Sep 7, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/compiler/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1283,7 +1283,7 @@ namespace ts {
args[i] = arguments[i];
}

return t => reduceLeft<(t: T) => T, T>(args, (u, f) => f(u), t);
return t => reduceLeft(args, (u, f) => f(u), t);
}
else if (d) {
return t => d(c(b(a(t))));
Expand Down
9 changes: 5 additions & 4 deletions src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2237,7 +2237,8 @@ namespace ts {
return token() === SyntaxKind.DotDotDotToken ||
isIdentifierOrPattern() ||
isModifierKind(token()) ||
token() === SyntaxKind.AtToken || isStartOfType();
token() === SyntaxKind.AtToken ||
isStartOfType(/*inStartOfParameter*/ true);
}

function parseParameter(): ParameterDeclaration {
Expand Down Expand Up @@ -2698,7 +2699,7 @@ namespace ts {
}
}

function isStartOfType(): boolean {
function isStartOfType(inStartOfParameter?: boolean): boolean {
switch (token()) {
case SyntaxKind.AnyKeyword:
case SyntaxKind.StringKeyword:
Expand Down Expand Up @@ -2728,11 +2729,11 @@ namespace ts {
case SyntaxKind.DotDotDotToken:
return true;
case SyntaxKind.MinusToken:
return lookAhead(nextTokenIsNumericLiteral);
return !inStartOfParameter && lookAhead(nextTokenIsNumericLiteral);
case SyntaxKind.OpenParenToken:
// Only consider '(' the start of a type if followed by ')', '...', an identifier, a modifier,
// or something that starts a type. We don't want to consider things like '(1)' a type.
return lookAhead(isStartOfParenthesizedOrFunctionType);
return !inStartOfParameter && lookAhead(isStartOfParenthesizedOrFunctionType);
default:
return isIdentifier();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
tests/cases/compiler/fatarrowfunctionsOptionalArgsErrors2.ts(1,15): error TS1003: Identifier expected.
tests/cases/compiler/fatarrowfunctionsOptionalArgsErrors2.ts(1,12): error TS2304: Cannot find name 'a'.
tests/cases/compiler/fatarrowfunctionsOptionalArgsErrors2.ts(1,12): error TS2695: Left side of comma operator is unused and has no side effects.
tests/cases/compiler/fatarrowfunctionsOptionalArgsErrors2.ts(1,16): error TS2304: Cannot find name 'b'.
tests/cases/compiler/fatarrowfunctionsOptionalArgsErrors2.ts(1,16): error TS2695: Left side of comma operator is unused and has no side effects.
tests/cases/compiler/fatarrowfunctionsOptionalArgsErrors2.ts(1,19): error TS2304: Cannot find name 'c'.
tests/cases/compiler/fatarrowfunctionsOptionalArgsErrors2.ts(1,23): error TS1005: ';' expected.
tests/cases/compiler/fatarrowfunctionsOptionalArgsErrors2.ts(1,26): error TS2304: Cannot find name 'a'.
tests/cases/compiler/fatarrowfunctionsOptionalArgsErrors2.ts(1,28): error TS2304: Cannot find name 'b'.
tests/cases/compiler/fatarrowfunctionsOptionalArgsErrors2.ts(1,30): error TS2304: Cannot find name 'c'.
tests/cases/compiler/fatarrowfunctionsOptionalArgsErrors2.ts(2,12): error TS2695: Left side of comma operator is unused and has no side effects.
Expand All @@ -18,16 +21,22 @@ tests/cases/compiler/fatarrowfunctionsOptionalArgsErrors2.ts(4,17): error TS1005
tests/cases/compiler/fatarrowfunctionsOptionalArgsErrors2.ts(4,20): error TS2304: Cannot find name 'a'.


==== tests/cases/compiler/fatarrowfunctionsOptionalArgsErrors2.ts (18 errors) ====
==== tests/cases/compiler/fatarrowfunctionsOptionalArgsErrors2.ts (21 errors) ====
var tt1 = (a, (b, c)) => a+b+c;
~
!!! error TS1003: Identifier expected.
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm. so we had this test originally? why did not we catch this the first time around then?

Copy link
Member Author

Choose a reason for hiding this comment

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

I noticed the baseline change but (1) I thought that removal of 3 errors was actually an improvement (2) didn't think it was indicative on any further problem. I also looked at the emitted code and thought it was better -- closer to what was written at least.

Copy link
Contributor

Choose a reason for hiding this comment

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

well.. the new error is a parse error. so a bit worse than the 3 semantic ones.

~
!!! error TS2304: Cannot find name 'a'.
~
!!! error TS2695: Left side of comma operator is unused and has no side effects.
~
!!! error TS2304: Cannot find name 'b'.
~
!!! error TS2695: Left side of comma operator is unused and has no side effects.
~
!!! error TS2304: Cannot find name 'c'.
~~
!!! error TS1005: ';' expected.
~
!!! error TS2304: Cannot find name 'a'.
~
!!! error TS2304: Cannot find name 'b'.
~
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,8 @@ var tt2 = ((a), b, c) => a+b+c;
var tt3 = ((a)) => a;

//// [fatarrowfunctionsOptionalArgsErrors2.js]
var tt1 = function (a, ) {
if ( === void 0) { = (b, c); }
return a + b + c;
};
var tt1 = (a, (b, c));
a + b + c;
var tt2 = ((a), b, c);
a + b + c;
var tt3 = ((a));
Expand Down
17 changes: 13 additions & 4 deletions tests/baselines/reference/parser512325.errors.txt
Original file line number Diff line number Diff line change
@@ -1,21 +1,30 @@
tests/cases/conformance/parser/ecmascript5/RegressionTests/parser512325.ts(1,14): error TS1003: Identifier expected.
tests/cases/conformance/parser/ecmascript5/RegressionTests/parser512325.ts(1,11): error TS2304: Cannot find name 'a'.
tests/cases/conformance/parser/ecmascript5/RegressionTests/parser512325.ts(1,11): error TS2695: Left side of comma operator is unused and has no side effects.
tests/cases/conformance/parser/ecmascript5/RegressionTests/parser512325.ts(1,15): error TS2304: Cannot find name 'b'.
tests/cases/conformance/parser/ecmascript5/RegressionTests/parser512325.ts(1,15): error TS2695: Left side of comma operator is unused and has no side effects.
tests/cases/conformance/parser/ecmascript5/RegressionTests/parser512325.ts(1,18): error TS2304: Cannot find name 'c'.
tests/cases/conformance/parser/ecmascript5/RegressionTests/parser512325.ts(1,22): error TS1005: ';' expected.
tests/cases/conformance/parser/ecmascript5/RegressionTests/parser512325.ts(1,25): error TS2304: Cannot find name 'a'.
tests/cases/conformance/parser/ecmascript5/RegressionTests/parser512325.ts(1,27): error TS2304: Cannot find name 'b'.
tests/cases/conformance/parser/ecmascript5/RegressionTests/parser512325.ts(1,29): error TS2304: Cannot find name 'c'.


==== tests/cases/conformance/parser/ecmascript5/RegressionTests/parser512325.ts (6 errors) ====
==== tests/cases/conformance/parser/ecmascript5/RegressionTests/parser512325.ts (9 errors) ====
var tt = (a, (b, c)) => a+b+c;
~
!!! error TS1003: Identifier expected.
~
!!! error TS2304: Cannot find name 'a'.
~
!!! error TS2695: Left side of comma operator is unused and has no side effects.
~
!!! error TS2304: Cannot find name 'b'.
~
!!! error TS2695: Left side of comma operator is unused and has no side effects.
~
!!! error TS2304: Cannot find name 'c'.
~~
!!! error TS1005: ';' expected.
~
!!! error TS2304: Cannot find name 'a'.
~
!!! error TS2304: Cannot find name 'b'.
~
Expand Down
6 changes: 2 additions & 4 deletions tests/baselines/reference/parser512325.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,5 @@
var tt = (a, (b, c)) => a+b+c;

//// [parser512325.js]
var tt = function (a, ) {
if ( === void 0) { = (b, c); }
return a + b + c;
};
var tt = (a, (b, c));
a + b + c;
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
tests/cases/conformance/parser/ecmascript5/ArrowFunctionExpressions/parserArrowFunctionExpression5.ts(1,2): error TS2304: Cannot find name 'bar'.
tests/cases/conformance/parser/ecmascript5/ArrowFunctionExpressions/parserArrowFunctionExpression5.ts(1,6): error TS2304: Cannot find name 'x'.


==== tests/cases/conformance/parser/ecmascript5/ArrowFunctionExpressions/parserArrowFunctionExpression5.ts (2 errors) ====
(bar(x,
~~~
!!! error TS2304: Cannot find name 'bar'.
~
!!! error TS2304: Cannot find name 'x'.
() => {},
() => {}
)
)

10 changes: 10 additions & 0 deletions tests/baselines/reference/parserArrowFunctionExpression5.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
//// [parserArrowFunctionExpression5.ts]
(bar(x,
() => {},
() => {}
)
)


//// [parserArrowFunctionExpression5.js]
(bar(x, function () { }, function () { }));
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
(bar(x,
() => {},
() => {}
)
)