Skip to content
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

throw the error when there is flow shorthand annotation comment with other comments #7429

Closed
strager opened this issue Jan 25, 2020 · 7 comments
Labels
area:comments Issues with how Prettier prints comments area:flow comment types lang:javascript Issues affecting JS type:bug Issues identifying ugly output, or a defect in the program

Comments

@strager
Copy link

strager commented Jan 25, 2020

Prettier 1.19.1
Playground link

--parser babel

Input:

f(
  // comment
  (0 /*: number */)
);

Output:

ensureAllCommentsPrinted/<@https://prettier.io/lib/standalone.js:15543:15
ensureAllCommentsPrinted@https://prettier.io/lib/standalone.js:15541:17
coreFormat@https://prettier.io/lib/standalone.js:15592:29
format@https://prettier.io/lib/standalone.js:15832:75
formatWithCursor@https://prettier.io/lib/standalone.js:15848:14
withPlugins/<@https://prettier.io/lib/standalone.js:31794:17
format@https://prettier.io/lib/standalone.js:31802:14
formatCode@https://prettier.io/worker.js:234:21
handleMessage@https://prettier.io/worker.js:185:18
self.onmessage@https://prettier.io/worker.js:155:14

Expected behavior:
Prettier exits successfully. Prettier does not change the formatting of this code.

Observations:
The following changes to the input make the crash disappear:

  • Remove the : in the comment on line 3
  • Remove the ( and ) on line 3
  • Remove the comment on line 2
@sosukesuzuki sosukesuzuki added lang:javascript Issues affecting JS type:bug Issues identifying ugly output, or a defect in the program labels Jan 25, 2020
@sosukesuzuki
Copy link
Member

Thanks for reporting! Prettier seems to throw the error when there is flow shorthand annotation comment with other comments.

Playground link

// Input
(0 /*: number */ /* comment */)
Error: Comment "comment" was not printed. Please report this error!
    at https://prettier.io/lib/standalone.js:15543:15
    at Array.forEach (<anonymous>)
    at ensureAllCommentsPrinted (https://prettier.io/lib/standalone.js:15541:17)
    at coreFormat (https://prettier.io/lib/standalone.js:15592:5)
    at format (https://prettier.io/lib/standalone.js:15832:75)
    at formatWithCursor (https://prettier.io/lib/standalone.js:15848:14)
    at https://prettier.io/lib/standalone.js:31794:17
    at Object.format (https://prettier.io/lib/standalone.js:31802:14)
    at formatCode (https://prettier.io/worker.js:234:21)
    at handleMessage (https://prettier.io/worker.js:185:18)

@sosukesuzuki sosukesuzuki changed the title Prettier crash: "Comment was not printed" throw the error when there is flow shorthand annotation comment with other comments Jan 25, 2020
@strager
Copy link
Author

strager commented Jan 28, 2020

If a Flow shorthand annotation comment is detected, Prettier disables the default comment handling and implements custom handling for Flow comments. (This custom handling was implemented in pull request #5280.) The problem is that the custom handling only prints one comment (the first trailing comment), and forgets about all other comments. Prettier's sanity checks detect the fact that some comments were not printed, and reports an error.

This issue does not reproduce when using Flow's parser (--parser flow).

I see three solutions:

  1. Extend the custom handling for Flow comments to print all other comments too.
  2. In Prettier, transform the AST given by Babel to make it look like the AST given by Flow. In particular, wrap each commented node in a TypeCastExpression node. (See the AST comparisons below.) Then, remove the custom handling for Flow comments.
  3. In Babel, add an option to parse Flow shorthand annotation comments into TypeCastExpression nodes, mimicking Flow's parser. (See the AST comparisons below.) Then, enable this feature in Prettier's use of Babel, and remove Prettier's custom handling for Flow comments.

Which of the above solutions sounds best? Does anyone have any other ideas on how to fix this crash?

cc @swac, @j-f1, @lydell

AST comparison

Input program

f(
  // comment
  (0 /*: number */)
);

--parser babel AST (abridged)

Node {
  type: 'File',
  program:
   Node {
     type: 'Program',
     sourceType: 'module',
     interpreter: null,
     body:
      [ Node {
          type: 'ExpressionStatement',
          expression:
           Node {
             type: 'CallExpression',
             callee:
              Node {
                type: 'Identifier',
                name: 'f' },
             arguments:
              [ Node {
                  type: 'NumericLiteral',
                  extra:
                   { rawValue: 0, raw: '0', parenthesized: true, parenStart: 18 },
                  value: 0,
                  leadingComments:
                   [ { type: 'CommentLine',
                       value: ' comment' } ],
                  trailingComments:
                   [ { type: 'CommentBlock',
                       value: ': number ' } ] } ] } } ],
     directives: [] },
  comments:
   [ { type: 'CommentLine',
       value: ' comment' },
     { type: 'CommentBlock',
       value: ': number ' } ] }

--parser flow AST (abridged)

{ type: 'Program',
  body:
   [ { type: 'ExpressionStatement',
       expression:
        { type: 'CallExpression',
          callee:
           { type: 'Identifier',
             name: 'f',
             typeAnnotation: null,
             optional: false },
          typeArguments: null,
          arguments:
           [ { type: 'TypeCastExpression',
               expression:
                { type: 'Literal',
                  value: 0,
                  raw: '0' },
               typeAnnotation:
                { type: 'TypeAnnotation',
                  typeAnnotation:
                   { type: 'NumberTypeAnnotation' } } } ] },
       directive: null } ],
  comments:
   [ { type: 'Line',
       value: ' comment' } ] }

@fisker
Copy link
Member

fisker commented Jan 31, 2020

I was trying to fix this, before I saw comment by @strager , spend a lot of time figure how it works... I tried a solution similar to solution1. Finnally make it work for babel but typescript parser still fail. will look into it later

@fisker
Copy link
Member

fisker commented Feb 1, 2020

Prettier 1.19.1
Playground link

--parser flow

Input:

f(
(1 
// comment
 /*: number */
/* comment*/
)
);

Output:

f(
  (1 /*: // comment
  number */)
  /* comment*/
);

Is this valid code?

@thorn0
Copy link
Member

thorn0 commented Feb 1, 2020

Seems to be valid. Use Flow's playground.

@fisker
Copy link
Member

fisker commented Feb 1, 2020

Opened a new issue, maybe we should fix this first

@thorn0 thorn0 added the area:comments Issues with how Prettier prints comments label May 7, 2020
@thorn0
Copy link
Member

thorn0 commented Oct 24, 2022

Not an issue anymore because support for Flow comments was removed in #13687

@thorn0 thorn0 closed this as completed Oct 24, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area:comments Issues with how Prettier prints comments area:flow comment types lang:javascript Issues affecting JS type:bug Issues identifying ugly output, or a defect in the program
Projects
None yet
Development

No branches or pull requests

4 participants