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.

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

Merged
sandersn merged 3 commits into from
Sep 7, 2017
Merged

Disable isStartOfType's lookahead when called from isStartOfParameter #18296

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7c69dd8
Disable lookahead in isStartOfParameter/isStartOfType
sandersn Sep 6, 2017
a5c2eac
Test:disable lookahead in isStartOfParameter
sandersn Sep 6, 2017
90d9f3d
Rename isStartOfType parameter used by isStartOfParameter
sandersn Sep 7, 2017
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
View file
Open in desktop
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
View file
Open in desktop
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(/*disableLookahead*/ true);
}

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

function isStartOfType(): boolean {
function isStartOfType(disableLookahead?: boolean): boolean {

@mhegazy mhegazy Sep 7, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we have a more declarative name, like inJSDocType or something?

@sandersn sandersn Sep 7, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

sure. done.

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 !disableLookahead && 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 !disableLookahead && lookAhead(isStartOfParenthesizedOrFunctionType);
default:
return isIdentifier();
}
Expand Down
17 changes: 13 additions & 4 deletions tests/baselines/reference/fatarrowfunctionsOptionalArgsErrors2.errors.txt
View file
Open in desktop
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.

@mhegazy mhegazy Sep 7, 2017

Copy link
Copy Markdown
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?

@sandersn sandersn Sep 7, 2017

Copy link
Copy Markdown
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.

@mhegazy mhegazy Sep 7, 2017

Copy link
Copy Markdown
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
6 changes: 2 additions & 4 deletions tests/baselines/reference/fatarrowfunctionsOptionalArgsErrors2.js
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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;
15 changes: 15 additions & 0 deletions tests/baselines/reference/parserArrowFunctionExpression5.errors.txt
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
//// [parserArrowFunctionExpression5.ts]
(bar(x,
() => {},
() => {}
)
)


//// [parserArrowFunctionExpression5.js]
(bar(x, function () { }, function () { }));
5 changes: 5 additions & 0 deletions ...conformance/parser/ecmascript5/ArrowFunctionExpressions/parserArrowFunctionExpression5.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
(bar(x,
() => {},
() => {}
)
)