Lint: intention preview throws exception for ReplaceStringQuickFix [271575376]

Fixed

Bug

Status Update

No update yet.

Description

gh...@google.com

created issue #1

Mar 3, 2023 04:56PM

While doing some other work in ReplaceStringQuickFix I was able to induce the following exception during intention preview:

com.intellij.util.IncorrectOperationException: Must not change PSI outside command or undo-transparent action. See com.intellij.openapi.command.WriteCommandAction or com.intellij.openapi.command.CommandProcessor
    ...
    at com.intellij.psi.impl.source.codeStyle.CodeStyleManagerImpl.reformatRange(CodeStyleManagerImpl.java:97)
    at com.android.tools.idea.lint.common.ReplaceStringQuickFix.apply(ReplaceStringQuickFix.java:322)
    at com.intellij.codeInsight.intention.IntentionAction.generatePreview(IntentionAction.java:145)
    ...
    at com.intellij.codeInsight.intention.impl.preview.IntentionPreviewComputable.invokePreview(IntentionPreviewComputable.kt:144)
    at com.intellij.codeInsight.intention.impl.preview.IntentionPreviewComputable.generatePreview(IntentionPreviewComputable.kt:103)

I think part of the problem is this line in ReplaceStringQuickFix:

if (myRange != null) {
  file = myRange.getContainingFile();

That line has the effect of changing the PsiFile back to a physical file (replacing the non-physical file synthesized by the intention preview code). The line was originally added to support applying Lint fixes to a different file than the one being edited (ag/12351963). ~~Probably we can disable intention preview in this case.~~

Comments

gh...@google.com <gh...@google.com> #2Mar 3, 2023 09:19PM

Marked as fixed.

I can reproduce this (thanks for the repro project!)

It looks like the problem is that the desugared api list from r8 contains this entry:

java/util/Collection#removeIf(Ljava/util/function/Predicate;)Z

but the bytecode here doesn't match -- it's java/util/ArrayList. Collection isn't a directly implemented interface or a direct super class, it's an interface on the super super class. The most efficient thing runtime wise would be for the signature list to inline this method on all implemented subclasses. But I should probably at least for now go and make the desugared API lookup do something similar to what it does for API lookup -- search through all super classes and interfaces as well. This isn't a new problem, so I'm very surprised this hasn't come up before (or it has, and I've forgotten).

Issue 271575376

Description

Issue summary

Comments

gh...@google.com <gh...@google.com> #2Mar 3, 2023 09:19PM

Add comment

Issue metadata