Remaining TypeScript Printing Issues #1643

azz · 2017-05-21T03:30:39Z

Latest run through the TypeScript codebase's test suite resulted in this output:

13886 good, 886 bad

90      SyntaxError: ';' expected.
90      SyntaxError: Expression expected.
82      TypeError: Cannot read property 'type' of undefined
62      Error: Value undefined is not a valid document
58      prettier(input) !== prettier(prettier(input))
56      Error: did not recognize object of type "TSInterfaceHeritage"
46      SyntaxError: Invalid character.
44      SyntaxError: ',' expected.
42      ast(input) !== ast(prettier(input))
35      SyntaxError: Identifier expected.
24      SyntaxError: Declaration or statement expected.
23      SyntaxError: Type expected.
21      SyntaxError: '(' expected.
17      SyntaxError: ':' expected.
16      SyntaxError: Hexadecimal digit expected.
16      SyntaxError: Property or signature expected.
14      SyntaxError: '=' expected.
12      SyntaxError: Unterminated template literal.
10      SyntaxError: Parameter declaration expected.
8       SyntaxError: An extended Unicode escape value must be between 0x0 and 0x10FFFF inclusive.
8       SyntaxError: ')' expected.
8       SyntaxError: 'super' must be followed by an argument list or member access.
7       SyntaxError: Unexpected token. A constructor, method, accessor, or property was expected.
6       SyntaxError: Property assignment expected.
6       SyntaxError: '}' expected.
6       TypeError: Cannot read property 'some' of undefined
6       SyntaxError: Unterminated Unicode escape sequence.
6       SyntaxError: '{' expected.
5       SyntaxError: '{' or ';' expected.
4       Error: Comment "" was not printed. Please report this error!
4       SyntaxError: Unterminated string literal.
4       SyntaxError: Digit expected.
3       SyntaxError: Argument expression expected.
3       SyntaxError: 'as' expected.
3       Error: Comment location overlaps with node location
3       SyntaxError: Merge conflict marker encountered.
3       SyntaxError: Unterminated regular expression literal.
3       SyntaxError: An unary expression with the '-' operator is not allowed in the left-hand side of an exponentiation expression. Consider enclosing the expression in parentheses.
3       SyntaxError: '=>' expected.
3       SyntaxError: ']' expected.
2       SyntaxError: Enum member expected.
2       SyntaxError: '>' expected.
2       SyntaxError: Declaration expected.
2       SyntaxError: Array element destructuring pattern expected.
2       SyntaxError: Variable declaration expected.
2       SyntaxError: Expression or comma expected.
2       SyntaxError: '*/' expected.
1       SyntaxError: An unary expression with the 'typeof' operator is not allowed in the left-hand side of an exponentiation expression. Consider enclosing the expression in parentheses.
1       SyntaxError: Unexpected end of text.
1       SyntaxError: Binary digit expected.
1       SyntaxError: Type parameter declaration expected.
1       SyntaxError: Cannot read property 'pos' of undefined (undefined:undefined)
1       SyntaxError: JSX element 'number' has no corresponding closing tag.
1       SyntaxError: 'case' or 'default' expected.
1       SyntaxError: Invalid 'reference' directive syntax.
1       SyntaxError: JSX element 'T' has no corresponding closing tag.
1       SyntaxError: An AMD module cannot have multiple name assignments.
1       SyntaxError: 'of' expected.
1       SyntaxError: An unary expression with the 'delete' operator is not allowed in the left-hand side of an exponentiation expression. Consider enclosing the expression in parentheses.

Scrolling through the debug output, it appears that all of the SyntaxErrors are fine (actual syntax errors in the input), but the following are still issues:

12 ~~13~~ ~~20~~ ~~21~~ ~~58~~ prettier(input) !== prettier(prettier(input)) gist
Unstable yield re-print with TypeScript parser #1769, TypeScript: fix unstable optional method #1770, Unstable comments with TypeScript #1771 and TypeScript: upgrade typescript-eslint-parser to 992f1fa #1785
3 Error: Comment location overlaps with node location gist
0 ~~2~~ ~~0~~ ~~4~~ Error: Comment "<omitted>" was not printed. Please report this error! gist
Fixed by Add TSTupleType to ast-types #1685 and TypeScript: fix namespace comment printing #1765
0 ~~5~~ ~~82~~ TypeError: Cannot read property 'type' of undefined gist
Note: most/all of these are issues with decorators
Fixed by Handle TypeScript decorators #1678
0 ~~62~~ Error: Value undefined is not a valid document gist
Fixed by TypeScript: handle undefined variable declarations #1645 and Exponentiation tokens are not supported eslint/typescript-eslint-parser#280
0 ~~56~~ Error: did not recognize object of type "TSInterfaceHeritage" gist
Fixed by TypeScript: add TSInterfaceHeritage to ast-types #1644
0 ~~5~~ ~~15~~ ~~42~~ ast(input) !== ast(prettier(input)) gist
Fixed by TypeScript: handle ExperimentalSpreadOperator, optional method, and abstract interface #1763 and TypeScript: allow parens with TSFunctionType and ignore empty specifiers #1764
0 ~~6~~ TypeError: Cannot read property 'some' of undefined gist
Fixed by TypeScript: handle global namespace without modifiers #1646

15 16 23 26 31 44 313 errors

Looks like we're getting close!

The text was updated successfully, but these errors were encountered:

LukeSheard · 2017-05-21T03:38:32Z

@azz How do we run these TS tests? I just updated to master and I can't find any?

azz · 2017-05-21T03:45:23Z

@LukeSheard the tests aren't in this repo, they're in the TS repo: https://github.com/Microsoft/TypeScript/tree/master/tests

We just clone the TypeScript repo next to the prettier repo and run them that way.

I added a script node scripts/run-external-typescript-tests.js that runs them for you, but it currently only works properly on Windows.

lydell · 2017-05-21T08:16:18Z

Perhaps scripts/run-external-typescript-tests.js can be modified to ignore test cases that are invalid TypeScript as input, just like scripts/sync-flow-tests.js does.

azz · 2017-05-21T09:59:29Z

@lydell Good idea. Done in #1647.

vjeux · 2017-05-21T19:26:35Z

node scripts/run-external-typescript-tests.js
Error: this script currently only works on windows.

:(

lydell · 2017-05-21T19:41:05Z

Is there a reason the TypeScript tests aren't copied into the repo as the Flow tests are?

vjeux · 2017-05-21T19:49:26Z

@lydell there are 11k of them :x Only a hundred-ish for flow.

azz · 2017-05-21T23:02:33Z

@vjeux My MacBook died so I am unable to pursue fixing the script on Mac for the moment. I left a comment at the top of the script as to why it doesn't work.

vjeux · 2017-05-21T23:24:04Z

Oh noes, sad to hear!

vjeux · 2017-05-23T17:43:13Z

vjeux · 2017-05-23T18:03:40Z

../tslint/src/rules/semicolonRule.ts: SyntaxError: Expression expected. (94:43)
  92 |
  93 | class SemicolonWalker extends Lint.AbstractWalker<Options> {
> 94 |   private scanner?: ts.Scanner = undefined?;
     |                                            ^
  95 |   public walk(sourceFile: ts.SourceFile) {
  96 |     const cb = (node: ts.Node): void => {
  97 |       switch (node.kind) {

Any idea why it's not parsing?

jgoz · 2017-05-23T18:23:19Z

Any idea why it's not parsing?

The ? after undefined is not valid TS for value expressions (as far as I know). It's expecting a ternary expression there.

azz · 2017-05-24T00:07:02Z

So close!

13356 good, 531 skipped, 44 bad
100% of 13931 files processed without errors.

I think we need more accuracy on that percentage! (99.68%)

vjeux · 2017-05-24T00:25:07Z

On a related note, prettier for typescript is significantly slower than running it for normal js. I can't wait for the babylon integration to land, this would avoid having to parse it once and then completely reshuffle the ast.

vjeux · 2017-05-24T02:42:27Z

Okay I'm done for the day,

If we fix the following 5 issues, we should be good to go for all the projects (but angular):

I haven't finished yet to categorize all the angular errors, but I think that there are some unique ones that aren't tracked here.

This is really awesome! Even with those remaining errors, most of the files actually compile fine!

JamesHenry · 2017-05-25T10:57:58Z

On a related note, prettier for typescript is significantly slower than running it for normal js. I can't wait for the babylon integration to land, this would avoid having to parse it once and then completely reshuffle the ast.

@vjeux Would you mind sharing your method for evaluating this? Performance has never been an issue in typescript-eslint-parser, so it has accordingly not received much attention. There may well be some quick wins available in the short term

vjeux · 2017-05-25T15:02:18Z

@JamesHenry I've been running prettier on a bunch of files for both typescript and js and it feels significantly faster using flow. I haven't benchmarked it or anything yet :)

azz · 2017-05-27T02:53:00Z

Another one causing causing some prettier(input) !== prettier(prettier(input)) checks to fail: eslint/typescript-eslint-parser#296.

-class Foo<___proto__> {}
+class Foo<____proto__> {}

azz · 2017-05-28T11:58:09Z

With #1785 and #1786:

12759 good, 568 skipped, 15 bad
99.89% of 13342 files processed without errors.

JamesHenry · 2017-05-28T12:03:09Z

Nice!

vjeux · 2017-05-28T20:23:42Z

On master, angular has only those two issues remaining:

The file actually doesn't parse at all. See angular/angular@7256d0e#commitcomment-22311174

../angular//packages/compiler-cli/integrationtest/ngtools_src/feature2/feature2.module.ts: SyntaxError: ',' expected. (19:91)
  17 |   declarations: [FeatureComponent],
  18 |   imports: [RouterModule.forChild([
> 19 |     {path: '', component: FeatureComponent}, {path: 'd', loadChildren: './default.module'} {
     |                                                                                            ^
  20 |       path: 'e',
  21 |       loadChildren: 'feature/feature.module#FeatureModule'
  22 |     }

Need to investigate

../angular//packages/platform-browser/src/dom/dom_adapter.ts: Error: Comment "<T extends Node>" was not printed. Please report this error!
    at /Users/vjeux/random/prettier/index.js:46:13
    at Array.forEach (native)
    at ensureAllCommentsPrinted (/Users/vjeux/random/prettier/index.js:44:15)
    at format (/Users/vjeux/random/prettier/index.js:70:3)
    at formatWithShebang (/Users/vjeux/random/prettier/index.js:233:12)
    at Object.module.exports.format (/Users/vjeux/random/prettier/index.js:246:12)
    at format (/Users/vjeux/random/prettier/bin/prettier.js:180:25)
    at /Users/vjeux/random/prettier/bin/prettier.js:304:16
    at /Users/vjeux/random/prettier/bin/prettier.js:359:7
    at Array.forEach (native)

JamesHenry · 2017-05-28T20:26:33Z

@vjeux Love how prettier is in your "random" folder 😄

azz · 2017-06-02T15:56:13Z

@vjeux What's the priority on the three Error: Comment location overlaps with node location errors?

vjeux · 2017-06-02T15:59:38Z

@azz: we should fix them :) This one is also an error that we should fix:

../angular//packages/platform-browser/src/dom/dom_adapter.ts: Error: Comment "<T extends Node>" was not printed. Please report this error!

vjeux · 2017-07-06T11:19:16Z

Let's close this one now that TypeScript has been released.

azz added the lang:typescript Issues affecting TypeScript-specific constructs (not general JS issues) label May 21, 2017

This was referenced May 21, 2017

TypeScript: add TSInterfaceHeritage to ast-types #1644

Merged

TypeScript: handle undefined variable declarations #1645

Merged

TypeScript: handle global namespace without modifiers #1646

Merged

vjeux mentioned this issue May 21, 2017

Missing TypeScript Nodes (2) #1480

Closed

10 tasks

vjeux mentioned this issue May 22, 2017

Add typescript parser to command line help #1663

Closed

vjeux mentioned this issue May 24, 2017

Missing default modifier for TSNamespaceExportDeclaration eslint/typescript-eslint-parser#288

Closed

vjeux mentioned this issue May 24, 2017

Missing computed flag on TSMethodSignature eslint/typescript-eslint-parser#290

Closed

azz mentioned this issue May 28, 2017

Break on Property with long value and Literal key #1786

Merged

vjeux closed this as completed Jul 6, 2017

lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jul 7, 2018

lock bot locked as resolved and limited conversation to collaborators Jul 7, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remaining TypeScript Printing Issues #1643

Remaining TypeScript Printing Issues #1643

azz commented May 21, 2017 •

edited

Loading

LukeSheard commented May 21, 2017

azz commented May 21, 2017

lydell commented May 21, 2017 •

edited

Loading

azz commented May 21, 2017

vjeux commented May 21, 2017

lydell commented May 21, 2017

vjeux commented May 21, 2017

azz commented May 21, 2017 •

edited

Loading

vjeux commented May 21, 2017

vjeux commented May 23, 2017 •

edited

Loading

vjeux commented May 23, 2017 •

edited

Loading

jgoz commented May 23, 2017

azz commented May 24, 2017

vjeux commented May 24, 2017

vjeux commented May 24, 2017 •

edited

Loading

JamesHenry commented May 25, 2017

vjeux commented May 25, 2017

azz commented May 27, 2017

azz commented May 28, 2017

JamesHenry commented May 28, 2017

vjeux commented May 28, 2017 •

edited

Loading

JamesHenry commented May 28, 2017

azz commented Jun 2, 2017

vjeux commented Jun 2, 2017

vjeux commented Jul 6, 2017

Remaining TypeScript Printing Issues #1643

Remaining TypeScript Printing Issues #1643

Comments

azz commented May 21, 2017 • edited Loading

15 16 23 26 31 44 313 errors

LukeSheard commented May 21, 2017

azz commented May 21, 2017

lydell commented May 21, 2017 • edited Loading

azz commented May 21, 2017

vjeux commented May 21, 2017

lydell commented May 21, 2017

vjeux commented May 21, 2017

azz commented May 21, 2017 • edited Loading

vjeux commented May 21, 2017

vjeux commented May 23, 2017 • edited Loading

vjeux commented May 23, 2017 • edited Loading

jgoz commented May 23, 2017

azz commented May 24, 2017

vjeux commented May 24, 2017

vjeux commented May 24, 2017 • edited Loading

JamesHenry commented May 25, 2017

vjeux commented May 25, 2017

azz commented May 27, 2017

azz commented May 28, 2017

JamesHenry commented May 28, 2017

vjeux commented May 28, 2017 • edited Loading

JamesHenry commented May 28, 2017

azz commented Jun 2, 2017

vjeux commented Jun 2, 2017

vjeux commented Jul 6, 2017

azz commented May 21, 2017 •

edited

Loading

lydell commented May 21, 2017 •

edited

Loading

azz commented May 21, 2017 •

edited

Loading

vjeux commented May 23, 2017 •

edited

Loading

vjeux commented May 23, 2017 •

edited

Loading

vjeux commented May 24, 2017 •

edited

Loading

vjeux commented May 28, 2017 •

edited

Loading