Improve return type parse error #17096

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

Jump to bottom

Merged

sandersn merged 5 commits into master from improve-return-type-parse-error

Jul 12, 2017

Member

sandersn commented Jul 11, 2017

Better error for wrong return token (: vs =>) in types.

Who really remembers the right return token to use for { (): string } vs () => string ? Now the compiler (1) suggests the right token (2) parses the return type as a return type.

This is a huge improvement from the previous behaviour when the first example was wrongly written as { () => string }. Before, => was parsed sort of like ;. { (); string } gives you a type literal with a call signature and a member named string. Very confusing.

Note that parsing both tokens would be ambiguous in expression position, so impossible to give a better message from the parser in this context. For example:

let f = (x: number) => number => x + 1;
                    ~~
                    Should be ':'

But the parser doesn't know that 'number' isn't an argument named 'number'.

sandersn added 4 commits

July 11, 2017 09:51


          Switch fillSignature boolean params to single enum

fcc9823


          Better error for wrong return (: vs =>) in types

b6ad43d

It's very ambiguous in expression position, so impossible to give a
better message from the parser. For example:

let f = (x: number) => number => x + 1;
                    ~~
                    Should be ':'

But the parser doesn't know that 'number' isn't an expression now.


          Test:better error for wrong return token (: vs =>)

3638ff1


          Make enum private and fix fillSignature predicate

8856ddf

msftclas added the cla-not-required label

sandersn requested a review from weswigham

July 11, 2017 17:51

weswigham requested changes

View reviewed changes

src/compiler/parser.ts Outdated

@@ @@ -3254,7 +3268,7 @@ namespace ts { @@
                       function parseParenthesizedArrowFunctionExpressionHead(allowAmbiguity: boolean): ArrowFunction {
                           const node = <ArrowFunction>createNode(SyntaxKind.ArrowFunction);
                           node.modifiers = parseModifiersForArrowFunction();
-                          const isAsync = !!(getModifierFlags(node) & ModifierFlags.Async);
+                          const isAsync = (getModifierFlags(node) & ModifierFlags.Async) ? SignatureContext.Await : 0;

Member

weswigham Jul 11, 2017

Rather than 0, could you add a SignatureContext.None member of value 0 and use it here? We do this with many of our other enums. Helps with finding references, refactoring and such.

Member Author

sandersn Jul 11, 2017

Done

src/compiler/parser.ts Outdated

@@ @@ -3263,7 +3277,7 @@ namespace ts { @@
                           // a => (b => c)
                           // And think that "(b =>" was actually a parenthesized arrow function with a missing
                           // close paren.
-                          fillSignature(SyntaxKind.ColonToken, /*yieldContext*/ false, /*awaitContext*/ isAsync, /*requireCompleteParameterList*/ !allowAmbiguity, node);
+                          fillSignature(SyntaxKind.ColonToken, isAsync | (allowAmbiguity ? 0 : SignatureContext.RequireCompleteParameterList), node);

Member

weswigham Jul 11, 2017

SignatureContext.None in place of 0 again

src/compiler/parser.ts Outdated

@@ @@ -4386,16 +4400,16 @@ namespace ts { @@
                           parseExpected(SyntaxKind.FunctionKeyword);
                           node.asteriskToken = parseOptionalToken(SyntaxKind.AsteriskToken);
-                          const isGenerator = !!node.asteriskToken;
-                          const isAsync = !!(getModifierFlags(node) & ModifierFlags.Async);
+                          const isGenerator = node.asteriskToken ? SignatureContext.Yield : 0;

Member

weswigham Jul 11, 2017

SignatureContext.None in place of 0 again

src/compiler/parser.ts Outdated

-                          const isAsync = hasModifier(node, ModifierFlags.Async);
-                          fillSignature(SyntaxKind.ColonToken, /*yieldContext*/ isGenerator, /*awaitContext*/ isAsync, /*requireCompleteParameterList*/ false, node);
-                          node.body = parseFunctionBlockOrSemicolon(isGenerator, isAsync, Diagnostics.or_expected);
+                          const isGenerator = node.asteriskToken ? SignatureContext.Yield : 0;

Member

weswigham Jul 11, 2017

SignatureContext.None in place of 0 again

src/compiler/parser.ts Outdated

    
            @@ -5158,7 +5172,7 @@ namespace ts {
          
                          node.decorators = decorators;

                          node.modifiers = modifiers;

                          parseExpected(SyntaxKind.ConstructorKeyword);

                          fillSignature(SyntaxKind.ColonToken, /*yieldContext*/ false, /*awaitContext*/ false, /*requireCompleteParameterList*/ false, node);

                          fillSignature(SyntaxKind.ColonToken, /*context*/ 0, node);

Member

weswigham Jul 11, 2017

SignatureContext.None in place of 0 again

src/compiler/parser.ts Outdated

-                          const isAsync = hasModifier(method, ModifierFlags.Async);
-                          fillSignature(SyntaxKind.ColonToken, /*yieldContext*/ isGenerator, /*awaitContext*/ isAsync, /*requireCompleteParameterList*/ false, method);
-                          method.body = parseFunctionBlockOrSemicolon(isGenerator, isAsync, diagnosticMessage);
+                          const isGenerator = asteriskToken ? SignatureContext.Yield : 0;

Member

weswigham Jul 11, 2017

SignatureContext.None in place of 0 again

src/compiler/parser.ts Outdated

    
            @@ -5226,7 +5240,7 @@ namespace ts {
          
                          node.decorators = decorators;

                          node.modifiers = modifiers;

                          node.name = parsePropertyName();

                          fillSignature(SyntaxKind.ColonToken, /*yieldContext*/ false, /*awaitContext*/ false, /*requireCompleteParameterList*/ false, node);

                          fillSignature(SyntaxKind.ColonToken, /*context*/ 0, node);

Member

weswigham Jul 11, 2017

SignatureContext.None in place of 0 again

src/compiler/parser.ts Outdated

    
                          const isGenerator = asteriskToken ? SignatureContext.Yield : 0;

                          const isAsync = hasModifier(method, ModifierFlags.Async) ? SignatureContext.Await : 0;

                          fillSignature(SyntaxKind.ColonToken, isGenerator | isAsync, method);

                          method.body = parseFunctionBlockOrSemicolon(!!isGenerator, !!isAsync, diagnosticMessage);

Member

weswigham Jul 11, 2017

Is it not worth having parseFunctionBlockOrSemicolon just take flags, too?

Member Author

sandersn Jul 11, 2017

Well, it delegates immediately to parseFunction for the flags, so I changed that one too, and added another member to the flags enum.


          Rename SignatureFlags enum and improve its usage

1b1f257

As requested in the PR comments

weswigham approved these changes

View reviewed changes

sandersn merged commit 50f3910 into master

sandersn deleted the improve-return-type-parse-error branch

July 12, 2017 14:18

microsoft locked and limited conversation to collaborators

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet