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

EmptyBlock removes } else { when the if block was empty, breaking the logic #314

Open
valters opened this issue Jul 13, 2024 · 1 comment
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@valters
Copy link

valters commented Jul 13, 2024

What version of OpenRewrite are you using?

I am using

  • Maven plugin 5.35.0
  • rewrite-static-analysis 1.11.0

How are you running OpenRewrite?

Maven plugin.
Minimal project to repro the issue: https://github.com/valters/test-openrewrite
Please see steps in readme.

What is the smallest, simplest way to reproduce the problem?

Trying to rewrite code with moderately tricky if logic:

public class WithEmptyBlock {

  public static class Base {
    public boolean isExtended() {
      return false;
    }
  }

  public static class Extender extends Base {

    @Override
    public boolean isExtended() {
      return true;
    }

    public boolean isExtendedClass() {
      return true;
    }

    public boolean isSomeOtherClass() {
      return false;
    }
  }

  public void shouldNotFlipCondition(Base arg) {

    if (arg.isExtended()) {
      if (((Extender) arg).isExtendedClass()
          || ((Extender) arg).isSomeOtherClass()) {
      } else {
        System.out.println("This line should not be executed!");
        throw new RuntimeException("code should not reach here! if you see this, rewrite has messed up.");
      }
    }
  }
}

What did you expect to see?

Either if condition should be inverted, or the else keyword can't be removed.

What did you see instead?

Broken diff after rewrite:

diff --git a/source-problems/src/main/java/ch/vingolds/testopenrewrite/WithEmptyBlock.java b/source-problems/src/main/java/ch/vingolds/testopenrewrite/WithEmptyBlock.java
index 5b4c050..c1a9f75 100644
--- a/source-problems/src/main/java/ch/vingolds/testopenrewrite/WithEmptyBlock.java
+++ b/source-problems/src/main/java/ch/vingolds/testopenrewrite/WithEmptyBlock.java
@@ -29,7 +29,6 @@ public class WithEmptyBlock {
     if (arg.isExtended()) {
       if (((Extender) arg).isExtendedClass()
           || ((Extender) arg).isSomeOtherClass()) {
-      } else {
         System.out.println("This line should not be executed!");
         throw new RuntimeException("code should not reach here! if you see this, rewrite has messed up.");
       }

What is the full stack trace of any errors you encountered?

No error. Just incorrect output.

@valters valters added the bug Something isn't working label Jul 13, 2024
@valters valters changed the title NeedBraces removes } else { when the if block was empty, breaking the logic EmptyBlock removes } else { when the if block was empty, breaking the logic Jul 13, 2024
@timtebeek
Copy link
Contributor

Thanks a lot for the detailed report and reproducer! I think it would be best to turn this into a runnable unit test as seen here, and then work from there on a draft PR.

@Test
void emptyElseBlock() {
rewriteRun(
//language=java
java(
"""
public class A {
{
if (true) {
System.out.println("this");
} else {
}
}
}
""",
"""
public class A {
{
if (true) {
System.out.println("this");
}
}
}
"""
)
);
}

@timtebeek timtebeek moved this to Backlog in OpenRewrite Jul 13, 2024
@timtebeek timtebeek added the good first issue Good for newcomers label Jul 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
Status: Backlog
Development

No branches or pull requests

2 participants