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

fix: stable format for if/for/while/do/catch/switch with trailing comment #595

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jtkiesel
Copy link
Contributor

@jtkiesel jtkiesel commented Aug 6, 2023

What changed with this PR:

If/for/while/do/catch/switch statements whose parentheses have trailing comments are now formatted stably, and in general are formatted more closely to the way that Prettier JavaScript does.

Example

Input

class Example {

  void ifStatements() {
    if (true) // comment
      System.out.println("Oops");

    if (true) {
      // comment
    }

    if (true) // comment
    {}

    if (true) // comment
    {
      System.out.println("Oops");
    }

    if (true) // comment
    {
      if (true) {}
    }

    if (true) // comment
    ;

    if (true) /*comment*/;
  }

  void forLoops() {
    for (int i = 0; i < 1; i++) // comment
      System.out.println("Oops");

    for (int i = 0; i < 1; i++) {
      // comment
    }

    for (;;)/*comment*/;

    for (int i = 0;;) //comment
    ;

    for (int i = 0; i < 1;) // comment
    {}

    for (;;) // comment
    {}

    for (; i < 1;) // comment
    {}

    for (;; i++) // comment
    {}

    for (
      int i = 0;
      i < 1;
      i++ // hi
    ) // comment
    {
      System.out.println("Oops");
    }

    for (int i = 0; i < 1; i++) // comment
    {
      if (true) {}
    }

    for (int i = 0; i < 1; i++) /*comment*/;

    for (String s : strings) // comment
      System.out.println("Oops");

    for (String s : strings) {
      // comment
    }

    for (String s : strings) // comment
    {}

    for (
      String s : strings // comment
    ) {}

    for (String s : strings) // comment
    {
      System.out.println("Oops");
    }

    for (String s : strings) // comment
    {
      if (true) {}
    }
  }

  void whileLoops() {
    while (true) // comment
      System.out.println("Oops");

    while (true) {
      // comment
    }

    while (true) // comment
    {}

    while (true) // comment
    {
      System.out.println("Oops");
    }

    while (
      true // test
    ) // comment
    {
      if (true) {}
    }

    while (true) // comment
    ;

    while (true) /*comment*/;
  }

  void doLoops() {
    do {
      // comment
    } while (true);

    do // comment
    {} while (true);

    do // comment
    {
      System.out.println("Oops");
    } while (true);

    do // comment
    {
      if (true) {}
    } while (true);

    do {} while (true) // comment
    ;

    do {} while (true) /*comment*/;

    do {
      System.out.println("Oops");
    } while (true) // comment
    ;

    do {
      if (true) {}
    } while (true) // comment
    ;
  }

  void switchStatements() {
    switch (value) {
      // comment
    }

    switch (value) // comment
    {}

    switch (value) // comment
    {
      case "a":
        System.out.println("Oops");
    }
  }

  void catchStatements() {
    try {} catch (Exception e) {
      // comment
    }

    try {} catch (Exception e) // comment
    {}

    try {} catch (Exception e) // comment
    {
      System.out.println("Oops");
    }

    try {} catch (Exception e) // comment
    {
      if (true) {}
    }
  }
}

Output

class Example {

  void ifStatements() {
    if (true)
      // comment
      System.out.println("Oops");

    if (true) {
      // comment
    }

    if (true) {
      // comment
    }

    if (true) {
      // comment
      System.out.println("Oops");
    }

    if (true) {
      // comment
      if (true) {}
    }

    if (
      true // comment
    );

    if (true/*comment*/);
  }

  void forLoops() {
    for (
      int i = 0;
      i < 1;
      i++ // comment
    )
      System.out.println("Oops");

    for (int i = 0; i < 1; i++) {
      // comment
    }

    /*comment*/
    for (;;);

    for (
      int i = 0; //comment
      ;

    );

    for (
      int i = 0;
      i < 1; // comment

    ) {}

    // comment
    for (;;) {}

    for (
      ;
      i < 1; // comment

    ) {}

    for (
      ;
      ;
      i++ // comment
    ) {}

    for (
      int i = 0;
      i < 1;
      i++ // hi
      // comment
    ) {
      System.out.println("Oops");
    }

    for (
      int i = 0;
      i < 1;
      i++ // comment
    ) {
      if (true) {}
    }

    for (int i = 0; i < 1; i++/*comment*/);

    for (String s : strings)
      // comment
      System.out.println("Oops");

    for (String s : strings) {
      // comment
    }

    for (String s : strings) {
      // comment
    }

    for (String s : strings) {} // comment

    for (String s : strings) {
      // comment
      System.out.println("Oops");
    }

    for (String s : strings) {
      // comment
      if (true) {}
    }
  }

  void whileLoops() {
    while (true)
      // comment
      System.out.println("Oops");

    while (true) {
      // comment
    }

    while (true) {
      // comment
    }

    while (true) {
      // comment
      System.out.println("Oops");
    }

    while (
      true // test
    ) {
      // comment
      if (true) {}
    }

    while (
      true // comment
    );

    while (true/*comment*/);
  }

  void doLoops() {
    do {
      // comment
    } while (true);

    do {} while (true); // comment

    do { // comment
      System.out.println("Oops");
    } while (true);

    do { // comment
      if (true) {}
    } while (true);

    do {} while (
      true // comment
    );

    do {} while (true/*comment*/);

    do {
      System.out.println("Oops");
    } while (
      true // comment
    );

    do {
      if (true) {}
    } while (
      true // comment
    );
  }

  void switchStatements() {
    switch (value) {
      // comment
    }

    switch (
      value // comment
    ) {}

    switch (
      value // comment
    ) {
      case "a":
        System.out.println("Oops");
    }
  }

  void catchStatements() {
    try {} catch (Exception e) {
      // comment
    }

    try {} catch (
      Exception e // comment
    ) {}

    try {} catch (
      Exception e // comment
    ) {
      System.out.println("Oops");
    }

    try {} catch (
      Exception e // comment
    ) {
      if (true) {}
    }
  }
}

Relative issues or prs:

Closes #592

@jtkiesel jtkiesel force-pushed the fix/stable-format-if-expression-trailing-comment branch from 01ee02b to 4a36754 Compare August 6, 2023 02:38
@@ -192,12 +193,19 @@ export class BlocksAndStatementPrettierVisitor extends BaseCstPrettierPrinter {
}

ifStatement(ctx: IfStatementCtx) {
handleCommentsIfStatement(ctx);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this issue only happening on if statements ?

Copy link
Contributor Author

@jtkiesel jtkiesel Aug 12, 2023

Choose a reason for hiding this comment

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

Looks like it also happens with for/while/do loops and switch/catch statements. I'll see about fixing those cases as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Latest push now handles for/while/do loops and switch/catch statements as well.

@DanielFran DanielFran added $100 https://www.jhipster.tech/bug-bounties/ $$ bug-bounty $$ https://www.jhipster.tech/bug-bounties/ labels Aug 12, 2023
@jtkiesel jtkiesel force-pushed the fix/stable-format-if-expression-trailing-comment branch from 4a36754 to 020b09e Compare August 15, 2023 04:19
@jtkiesel jtkiesel changed the title fix: stable format for if expression with trailing comment fix: stable format for if/for/while/do/catch/switch with trailing comment Aug 15, 2023
@clementdessoude
Copy link
Contributor

Thank you, @jtkiesel ! It seems that quite a lot of logic is added in order to handle this edge case. I would like to dig a bit more into the issue to see if we can simplify some things before merging this. My objective would be to look at it by the end of the week (and the other pending PR) so we could release a new version of Prettier this weekend :)

@jtkiesel jtkiesel force-pushed the fix/stable-format-if-expression-trailing-comment branch from 020b09e to a356f53 Compare August 15, 2023 21:00
@clementdessoude
Copy link
Contributor

I looked at it more closely yesterday, but was not able to wrap it up, sorry. I propose to make a new release with the changes that were made since 2.2.0 (#600), and finish this afterwards.

@jtkiesel
Copy link
Contributor Author

Sounds good! I'll try to take another look at it this week as well, and see if I can simplify it at all.

@koppor
Copy link
Contributor

koppor commented Nov 17, 2023

@clementdessoude @jtkiesel Did you find time to look into this? 😇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
$$ bug-bounty $$ https://www.jhipster.tech/bug-bounties/ $100 https://www.jhipster.tech/bug-bounties/
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Formatting not stable with comment in if statement
4 participants