From f7c30e6971e6006af0ce50e34e3871fcf72ac225 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Mon, 17 Jul 2023 13:22:46 +0200 Subject: [PATCH] [java-intentions] MethodThrowsFix: ModCommand GitOrigin-RevId: e9fab99693bcfcd8729703edc3c96a8d8a31840a --- .../messages/JavaAnalysisBundle.properties | 2 +- .../daemon/impl/quickfix/MethodThrowsFix.java | 178 ++++++------------ .../DuplicateThrowsInspection.java | 4 +- .../impl/config/QuickFixFactoryImpl.java | 8 +- 4 files changed, 63 insertions(+), 129 deletions(-) diff --git a/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties b/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties index 36670bde96d3..7e60a7506c0d 100644 --- a/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties +++ b/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties @@ -92,7 +92,7 @@ dataflow.too.complex={0} is too complex to analyze by data flow algorithm delete.repeated.0=Delete repeated ''{0}'' delete.repeated.interface=Delete repeated interface -exception.removal.will.break.source.code.proceed.anyway=Exception removal will break source code. Proceed anyway? +exception.handler.will.become.unreachable=Catch section will become unreachable feature.generics=Generics feature.annotations=Annotations diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/MethodThrowsFix.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/MethodThrowsFix.java index 92b8f7492b6e..965109d37773 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/MethodThrowsFix.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/MethodThrowsFix.java @@ -1,45 +1,34 @@ -// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package com.intellij.codeInsight.daemon.impl.quickfix; import com.intellij.codeInsight.ExceptionUtil; import com.intellij.codeInsight.daemon.QuickFixBundle; -import com.intellij.codeInsight.intention.preview.IntentionPreviewInfo; +import com.intellij.codeInsight.intention.preview.IntentionPreviewUtils; import com.intellij.codeInsight.javadoc.JavaDocUtil; -import com.intellij.codeInspection.LocalQuickFixAndIntentionActionOnPsiElement; +import com.intellij.codeInspection.ModCommands; import com.intellij.find.findUsages.JavaFindUsagesHelper; import com.intellij.find.findUsages.JavaMethodFindUsagesOptions; -import com.intellij.ide.highlighter.JavaFileType; import com.intellij.java.analysis.JavaAnalysisBundle; -import com.intellij.openapi.application.WriteAction; -import com.intellij.openapi.editor.Editor; -import com.intellij.openapi.progress.ProgressManager; +import com.intellij.modcommand.ModCommand; +import com.intellij.modcommand.ModShowConflicts; +import com.intellij.modcommand.PsiBasedModCommandAction; import com.intellij.openapi.project.Project; -import com.intellij.openapi.ui.Messages; -import com.intellij.openapi.util.TextRange; import com.intellij.openapi.util.text.StringUtil; -import com.intellij.openapi.vfs.ReadonlyStatusHandler; import com.intellij.psi.*; -import com.intellij.psi.codeStyle.CodeStyleManager; import com.intellij.psi.codeStyle.JavaCodeStyleManager; import com.intellij.psi.javadoc.PsiDocComment; import com.intellij.psi.util.PsiFormatUtil; import com.intellij.psi.util.PsiFormatUtilBase; -import com.intellij.psi.util.PsiTreeUtil; -import com.intellij.refactoring.RefactoringBundle; -import com.intellij.util.ObjectUtils; import com.intellij.util.SmartList; import com.intellij.util.containers.ContainerUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.PropertyKey; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.List; +import java.util.*; import java.util.stream.Stream; -public abstract class MethodThrowsFix extends LocalQuickFixAndIntentionActionOnPsiElement { +public abstract class MethodThrowsFix extends PsiBasedModCommandAction { protected final String myThrowsCanonicalText; private final String myMethodName; private final @PropertyKey(resourceBundle = QuickFixBundle.BUNDLE) String myMessageKey; @@ -54,58 +43,22 @@ public abstract class MethodThrowsFix extends LocalQuickFixAndIntentionActionOnP PsiFormatUtilBase.SHOW_NAME | (showClassName ? PsiFormatUtilBase.SHOW_CONTAINING_CLASS : 0), 0); } - @Override - public void invoke(@NotNull Project project, - @NotNull PsiFile file, - @Nullable Editor editor, - @NotNull PsiElement startElement, - @NotNull PsiElement endElement) { - processMethod(project, (PsiMethod)startElement); - } - - abstract void processMethod(@NotNull Project project, @NotNull PsiMethod method); - - @Override - public @NotNull IntentionPreviewInfo generatePreview(@NotNull Project project, @NotNull Editor editor, @NotNull PsiFile copyFile) { - PsiMethod startElement = ObjectUtils.tryCast(getStartElement(), PsiMethod.class); - if (startElement == null) return IntentionPreviewInfo.EMPTY; - PsiFile file = startElement.getContainingFile(); - if (copyFile.getOriginalFile() == file) { - processMethod(project, PsiTreeUtil.findSameElementInCopy(startElement, copyFile)); - return IntentionPreviewInfo.DIFF; - } - PsiMethod copy = (PsiMethod)startElement.copy(); - processMethod(project, copy); - TextRange range = copy.getThrowsList().getTextRangeInParent(); - CodeStyleManager.getInstance(project).reformatRange(copy, range.getStartOffset(), range.getEndOffset(), true); - String origText = startElement.getText(); - String copyText = copy.getText(); - PsiCodeBlock copyBody = copy.getBody(); - PsiCodeBlock origBody = startElement.getBody(); - if (copyBody != null && origBody != null) { - origText = origText.substring(0, origBody.getStartOffsetInParent() + 1); - copyText = copyText.substring(0, copyBody.getStartOffsetInParent() + 1); - } - return new IntentionPreviewInfo.CustomDiff(JavaFileType.INSTANCE, file.getName(), origText, copyText); - } - public static class Add extends MethodThrowsFix { public Add(@NotNull PsiMethod method, @NotNull PsiClassType exceptionType, boolean showClassName) { super("fix.throws.list.add.exception", method, exceptionType, showClassName); } @Override - void processMethod(@NotNull Project project, @NotNull PsiMethod myMethod) { + protected @NotNull ModCommand perform(@NotNull ActionContext context, @NotNull PsiMethod myMethod) { PsiJavaCodeReferenceElement[] referenceElements = myMethod.getThrowsList().getReferenceElements(); boolean alreadyThrows = ContainerUtil.exists(referenceElements, referenceElement -> referenceElement.getCanonicalText().equals(myThrowsCanonicalText)); - if (!alreadyThrows) { - final PsiElementFactory factory = JavaPsiFacade.getElementFactory(myMethod.getProject()); - final PsiClassType type = (PsiClassType)factory.createTypeFromText(myThrowsCanonicalText, myMethod); - PsiJavaCodeReferenceElement ref = factory.createReferenceElementByType(type); - ref = (PsiJavaCodeReferenceElement)JavaCodeStyleManager.getInstance(project).shortenClassReferences(ref); - myMethod.getThrowsList().add(ref); - } + if (alreadyThrows) return ModCommands.nop(); + final PsiElementFactory factory = JavaPsiFacade.getElementFactory(myMethod.getProject()); + final PsiClassType type = (PsiClassType)factory.createTypeFromText(myThrowsCanonicalText, myMethod); + JavaCodeStyleManager manager = JavaCodeStyleManager.getInstance(context.project()); + PsiElement ref = manager.shortenClassReferences(factory.createReferenceElementByType(type)); + return ModCommands.psiUpdate(myMethod, method -> method.getThrowsList().add(ref)); } } @@ -115,9 +68,12 @@ public abstract class MethodThrowsFix extends LocalQuickFixAndIntentionActionOnP } @Override - void processMethod(@NotNull Project project, @NotNull PsiMethod method) { + protected @NotNull ModCommand perform(@NotNull ActionContext context, @NotNull PsiMethod method) { PsiJavaCodeReferenceElement[] referenceElements = method.getThrowsList().getReferenceElements(); - Arrays.stream(referenceElements).filter(referenceElement -> referenceElement.getCanonicalText().equals(myThrowsCanonicalText)).findFirst().ifPresent(PsiElement::delete); + return Arrays.stream(referenceElements) + .filter(referenceElement -> referenceElement.getCanonicalText().equals(myThrowsCanonicalText)).findFirst() + .map(ref -> ModCommands.psiUpdate(ref, PsiElement::delete)) + .orElse(ModCommands.nop()); } } @@ -127,69 +83,53 @@ public abstract class MethodThrowsFix extends LocalQuickFixAndIntentionActionOnP } @Override - public boolean startInWriteAction() { - return false; - } - - @Override - public void invoke(@NotNull Project project, - @NotNull PsiFile file, - @Nullable Editor editor, - @NotNull PsiElement startElement, - @NotNull PsiElement endElement) { - if (!ReadonlyStatusHandler.ensureFilesWritable(project, file.getVirtualFile())) { - return; - } - final PsiMethod method = (PsiMethod)startElement; - + protected @NotNull ModCommand perform(@NotNull ActionContext context, @NotNull PsiMethod method) { + Project project = context.project(); PsiClassType exception = JavaPsiFacade.getElementFactory(project).createTypeByFQClassName(myThrowsCanonicalText, method.getResolveScope()); - if (!ExceptionUtil.isUncheckedException(exception)) { + ModCommand action = ModCommands.psiUpdate(method, m -> processMethod(project, m)); + if (!IntentionPreviewUtils.isIntentionPreviewActive() && !ExceptionUtil.isUncheckedException(exception)) { JavaMethodFindUsagesOptions ops = new JavaMethodFindUsagesOptions(project); ops.isSearchForTextOccurrences = false; ops.isImplicitToString = false; ops.isSkipImportStatements = true; - boolean breakSourceCode = !ProgressManager.getInstance().runProcessWithProgressSynchronously(() -> JavaFindUsagesHelper.processElementUsages( + Map conflicts = new HashMap<>(); + JavaFindUsagesHelper.processElementUsages( method, ops, usage -> { - PsiElement element = usage.getElement(); - if (!(element instanceof PsiReferenceExpression)) return true; - PsiElement parent = element.getParent(); - if (parent instanceof PsiCallExpression) { - ExceptionUtil.HandlePlace place = ExceptionUtil.getHandlePlace(parent, exception, null); - if (place instanceof ExceptionUtil.HandlePlace.TryCatch) { - PsiParameter parameter = ((ExceptionUtil.HandlePlace.TryCatch)place).getParameter(); - PsiCodeBlock block = ((ExceptionUtil.HandlePlace.TryCatch)place).getTryStatement().getTryBlock(); - if (block != null) { - PsiCallExpression call = (PsiCallExpression)parent; - Collection types = ExceptionUtil.collectUnhandledExceptions(block, null, call); - if (types.contains(exception)) { - return true; - } - List thrownCheckedExceptions = new ArrayList<>(ExceptionUtil.getThrownCheckedExceptions(call)); - thrownCheckedExceptions.remove(exception); + PsiElement element = usage.getElement(); + if (!(element instanceof PsiReferenceExpression)) return true; + PsiElement parent = element.getParent(); + if (parent instanceof PsiCallExpression) { + ExceptionUtil.HandlePlace place = ExceptionUtil.getHandlePlace(parent, exception, null); + if (place instanceof ExceptionUtil.HandlePlace.TryCatch tryCatch) { + PsiParameter parameter = tryCatch.getParameter(); + PsiCodeBlock block = tryCatch.getTryStatement().getTryBlock(); + if (block != null) { + PsiCallExpression call = (PsiCallExpression)parent; + Collection types = ExceptionUtil.collectUnhandledExceptions(block, null, call); + if (types.contains(exception)) { + return true; + } + List thrownCheckedExceptions = new ArrayList<>(ExceptionUtil.getThrownCheckedExceptions(call)); + thrownCheckedExceptions.remove(exception); - PsiType caughtExceptionType = parameter.getType(); - if (Stream.concat(types.stream(), thrownCheckedExceptions.stream()).noneMatch(ex -> caughtExceptionType.isAssignableFrom(ex))) { - return false; + PsiType caughtExceptionType = parameter.getType(); + if (Stream.concat(types.stream(), thrownCheckedExceptions.stream()) + .noneMatch(ex -> caughtExceptionType.isAssignableFrom(ex))) { + conflicts.put(parameter, new ModShowConflicts.Conflict(List.of( + JavaAnalysisBundle.message("exception.handler.will.become.unreachable")))); + } } } } - } - return true; - }), JavaAnalysisBundle.message("processing.method.usages"), true, project); + return true; + }); - if (breakSourceCode && Messages.showYesNoDialog(project, JavaAnalysisBundle - .message("exception.removal.will.break.source.code.proceed.anyway"), RefactoringBundle.getCannotRefactorMessage(null), null) == Messages.NO) { - return; - } + return new ModShowConflicts(conflicts, action); } - - WriteAction.run(() -> { - processMethod(project, method); - }); + return action; } - @Override void processMethod(@NotNull Project project, @NotNull PsiMethod method) { PsiType exceptionType = JavaPsiFacade.getElementFactory(project).createTypeFromText(myThrowsCanonicalText, null); for (PsiElement element : extractRefsToRemove(method, exceptionType)) { @@ -222,12 +162,6 @@ public abstract class MethodThrowsFix extends LocalQuickFixAndIntentionActionOnP } } - @NotNull - @Override - public final String getText() { - return QuickFixBundle.message(myMessageKey, StringUtil.getShortName(myThrowsCanonicalText), myMethodName); - } - @Override @NotNull public final String getFamilyName() { @@ -235,10 +169,8 @@ public abstract class MethodThrowsFix extends LocalQuickFixAndIntentionActionOnP } @Override - public final boolean isAvailable(@NotNull Project project, - @NotNull PsiFile file, - @NotNull PsiElement startElement, - @NotNull PsiElement endElement) { - return !(((PsiMethod)startElement).getThrowsList() instanceof PsiCompiledElement); // can happen in Kotlin + protected @Nullable Presentation getPresentation(@NotNull ActionContext context, @NotNull PsiMethod element) { + if (element.getThrowsList() instanceof PsiCompiledElement) return null; // can happen in Kotlin + return Presentation.of(QuickFixBundle.message(myMessageKey, StringUtil.getShortName(myThrowsCanonicalText), myMethodName)); } } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/duplicateThrows/DuplicateThrowsInspection.java b/java/java-analysis-impl/src/com/intellij/codeInspection/duplicateThrows/DuplicateThrowsInspection.java index 9ea7e2764445..89ea6dba0847 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/duplicateThrows/DuplicateThrowsInspection.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/duplicateThrows/DuplicateThrowsInspection.java @@ -1,4 +1,4 @@ -// Copyright 2000-2022 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package com.intellij.codeInspection.duplicateThrows; import com.intellij.codeInsight.daemon.impl.quickfix.MethodThrowsFix; @@ -79,7 +79,7 @@ public class DuplicateThrowsInspection extends AbstractBaseJavaLocalInspectionTo } } if (problem != null) { - holder.registerProblem(ref, problem, new MethodThrowsFix.RemoveFirst(method, type, false)); + holder.problem(ref, problem).fix(new MethodThrowsFix.RemoveFirst(method, type, false)).register(); } } } diff --git a/java/java-impl/src/com/intellij/codeInsight/intention/impl/config/QuickFixFactoryImpl.java b/java/java-impl/src/com/intellij/codeInsight/intention/impl/config/QuickFixFactoryImpl.java index 34448a3d6cff..93ac686ad9dd 100644 --- a/java/java-impl/src/com/intellij/codeInsight/intention/impl/config/QuickFixFactoryImpl.java +++ b/java/java-impl/src/com/intellij/codeInsight/intention/impl/config/QuickFixFactoryImpl.java @@ -29,6 +29,7 @@ import com.intellij.java.JavaBundle; import com.intellij.lang.annotation.HighlightSeverity; import com.intellij.lang.java.request.CreateConstructorFromUsage; import com.intellij.lang.java.request.CreateMethodFromUsage; +import com.intellij.modcommand.ModCommandAction; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.editor.Document; @@ -132,9 +133,10 @@ public final class QuickFixFactoryImpl extends QuickFixFactory { @NotNull PsiClassType exceptionClass, boolean shouldThrow, boolean showContainingClass) { - return shouldThrow - ? new MethodThrowsFix.Add(method, exceptionClass, showContainingClass) - : new MethodThrowsFix.Remove(method, exceptionClass, showContainingClass); + ModCommandAction action = shouldThrow + ? new MethodThrowsFix.Add(method, exceptionClass, showContainingClass) + : new MethodThrowsFix.Remove(method, exceptionClass, showContainingClass); + return LocalQuickFixAndIntentionActionOnPsiElement.from(action, method); } @NotNull