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

Intellij Plugin breaks method inlining and variable extraction from inside if condition #1101

Open
DidierLoiseau opened this issue May 23, 2024 · 3 comments
Labels

Comments

@DidierLoiseau
Copy link

Consider the following code and try inlining isEmpty2:

import java.util.Collection;
import java.util.List;

public class InliningWithJavaFormat {
  static boolean isEmpty(Collection<?> c) {
    return c == null || c.isEmpty();
  }

  static boolean isEmpty2(Collection<?> c) {
    return isEmpty(c);
  }

  public static void main(String[] args){
    var pojo = new MyPojo();
    if (pojo != null && !isEmpty2(pojo.getCollection())) {
      System.out.println("empty");
    }
  }

  static class MyPojo {
    Collection<String> getCollection() {
      return List.of();
    }
  }
}

Not only does it fail (without reporting it), but it adds an additional if (true):

  public static void main(String[] args){
    var pojo = new MyPojo();
    if (pojo != null && !isEmpty2(pojo.getCollection())) {
      if (true) {
        System.out.println("empty");
      }
    }
  }

with the plugin disabled, it works:

  public static void main(String[] args){
    var pojo = new MyPojo();
    if (pojo != null) {
      Collection<?> c = pojo.getCollection();
      if (!isEmpty(c)) {
        System.out.println("empty");
      }
    }
  }

(not the ideal result but at least it does not fail)

In addition, when I do that on my actual project I get an error notification from the plugin and the formatting gets broken:
error notification
(I did not try to reproduce this with an MRE, I guess it is a side effect of the first issue)

@DidierLoiseau
Copy link
Author

I just noticed that the same issue occurs if you simply try to extract a local variable from within the if condition instead of performing an inlining. IntelliJ would normally split the if in a similar way.

I guess this is what IntelliJ tries to do under the hood for the inlining (as shown when the plugin is disabled), and the at the source of the problem.

@DidierLoiseau DidierLoiseau changed the title Intellij Plugin breaks method inlining inside if condition Intellij Plugin breaks method inlining and variable extraction from inside if condition May 30, 2024
@nrayburn-tech
Copy link
Contributor

For the first example, the code fails at the line here https://github.com/JetBrains/intellij-community/blob/cd9bfbd98a7dca730fbc469156ce1ed30364afba/java/java-analysis-impl/src/com/siyeh/ig/psiutils/CodeBlockSurrounder.java#L884 due to a ClassCastException.

After formatting, CodeStyleManager.getInstance(project).reformat() is returning an instance of PsiKeywordImpl instead of the expected PsiIfStatement. Not sure why this is happening. I don't see where we return any PSI elements directly, we only return a string. So, something about how that string is being parsed into PSI elements isn't working as expected.

Because the exception is thrown, the remaining refactor logic from IntelliJ doesn't seem to be running.

@amaembo
Copy link

amaembo commented Oct 17, 2024

IntelliJ developer is here. We will work around the problem on the IntelliJ side (ClassCastException in CodeBlockSurrounder). However, the root cause is not identified, and similar problem may still happen in other contexts. Please track IDEA-340109 for details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants