From 3605bcd824b49d63bc7f138ef28f1dc6bbb47be5 Mon Sep 17 00:00:00 2001 From: Marcin Mikosik Date: Thu, 9 Oct 2025 10:45:14 +0000 Subject: [PATCH] [java] IDEA-251649 Sort 'Cast argument to' quickfixes #IDEA-251649 Fixed Merge-request: IJ-MR-173597 Merged-by: Marcin Mikosik GitOrigin-RevId: 197b9c4e3fb9be1d753b895b36a44c08cb7afdef --- .../impl/analysis/HighlightFixUtil.java | 1 - .../quickfix/ArgumentFixerActionFactory.java | 72 ++++++++--- .../impl/quickfix/CastMethodArgumentFix.java | 48 +++++++ .../CastMethodParametersOrderTest.java | 120 ++++++++++++++++++ 4 files changed, 221 insertions(+), 20 deletions(-) create mode 100644 java/java-tests/testSrc/com/intellij/codeInsight/daemon/impl/quickfix/CastMethodParametersOrderTest.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightFixUtil.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightFixUtil.java index 92e6b4c761c7..5143ca37b76b 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightFixUtil.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightFixUtil.java @@ -754,7 +754,6 @@ public final class HighlightFixUtil { @NotNull JavaResolveResult @NotNull [] resolveResults) { PsiExpressionList list = methodCall.getArgumentList(); MethodCandidateInfo[] candidates = toMethodCandidates(resolveResults); - CastMethodArgumentFix.REGISTRAR.registerCastActions(candidates, methodCall, sink); WrapWithAdapterMethodCallFix.registerCastActions(candidates, methodCall, sink); WrapObjectWithOptionalOfNullableFix.REGISTAR.registerCastActions(candidates, methodCall, sink); WrapExpressionFix.registerWrapAction(candidates, list.getExpressions(), sink); diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/ArgumentFixerActionFactory.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/ArgumentFixerActionFactory.java index 1ac3984366c2..ec54e297a4ce 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/ArgumentFixerActionFactory.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/ArgumentFixerActionFactory.java @@ -11,6 +11,7 @@ import com.intellij.psi.util.PsiTypesUtil; import com.intellij.psi.util.PsiUtil; import com.intellij.psi.util.TypeConversionUtil; import com.intellij.util.IncorrectOperationException; +import com.intellij.util.containers.ContainerUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -60,12 +61,13 @@ public abstract class ArgumentFixerActionFactory { try { PsiType expectedTypeByParent = PsiTypesUtil.getExpectedTypeByParent(call); Map> suggestedCasts = new HashMap<>(); - // find to which type we can cast this param to get valid method call + // find to which type we can cast this param to get a valid method call + List actualCasts = new ArrayList<>(); for (CandidateInfo candidate : methodCandidates) { PsiMethod method = (PsiMethod)candidate.getElement(); PsiSubstitutor substitutor = candidate.getSubstitutor(); PsiParameter[] parameters = method.getParameterList().getParameters(); - Map potentialCasts = new HashMap<>(); + List potentialCasts = new ArrayList<>(); for (int i = 0; i < expressions.length; i++) { PsiExpression expression = expressions[i]; PsiType exprType = PsiUtil.skipParenthesizedExprDown(expression) instanceof PsiFunctionalExpression fn ? @@ -82,33 +84,34 @@ public abstract class ArgumentFixerActionFactory { } // strict compare since even widening cast may help if (Comparing.equal(exprType, parameterType)) continue; - potentialCasts.put(i, parameterType); + potentialCasts.add(new ArgumentCast(i, parameterType)); } if (!potentialCasts.isEmpty()) { PsiCall newCall = call; - for (Map.Entry entry : potentialCasts.entrySet()) { - newCall = replaceWithCast(expressions, newCall, entry, newCall == call); + for (var argumentCast : potentialCasts) { + newCall = replaceWithCast(expressions, newCall, argumentCast, newCall == call); if (newCall == null) { break; } } if (newCall != null) { doCheckNewCall(expectedTypeByParent, newCall, () -> { - for (Map.Entry entry : potentialCasts.entrySet()) { - registerCastIntention(info, list, suggestedCasts, entry); + for (var argumentCast : potentialCasts) { + collectCast(actualCasts, suggestedCasts, argumentCast); } potentialCasts.clear(); }); } - - for (Map.Entry entry : potentialCasts.entrySet()) { - PsiCall callWithSingleCast = replaceWithCast(expressions, call, entry, true); + + for (var argumentCast : potentialCasts) { + PsiCall callWithSingleCast = replaceWithCast(expressions, call, argumentCast, true); if (callWithSingleCast == null) continue; - doCheckNewCall(expectedTypeByParent, callWithSingleCast, () -> registerCastIntention(info, list, suggestedCasts, entry)); + doCheckNewCall(expectedTypeByParent, callWithSingleCast, () -> collectCast(actualCasts, suggestedCasts, argumentCast)); } } } + putCompilableCastsFirst(actualCasts, expressions).forEach(cast -> registerCastIntention(info, list, cast)); } catch (IncorrectOperationException e) { LOG.error(e); @@ -128,20 +131,17 @@ public abstract class ArgumentFixerActionFactory { private void registerCastIntention(@NotNull Consumer info, PsiExpressionList list, - Map> suggestedCasts, - Map.Entry entry) { - suggestedCasts.get(entry.getKey()).add(entry.getValue().getCanonicalText()); - IntentionAction action = createFix(list, entry.getKey(), entry.getValue()); + ArgumentCast argumentCast) { + IntentionAction action = createFix(list, argumentCast.argumentIndex(), argumentCast.castType()); if (action != null) { info.accept(action); } } - private @Nullable PsiCall replaceWithCast(PsiExpression[] expressions, @NotNull PsiCall origCall, Map.Entry entry, + private @Nullable PsiCall replaceWithCast(PsiExpression[] expressions, @NotNull PsiCall origCall, ArgumentCast argumentCast, boolean shouldCopy) { - Integer i = entry.getKey(); - PsiType parameterType = entry.getValue(); - PsiExpression modifiedExpression = getModifiedArgument(expressions[i], parameterType); + int i = argumentCast.argumentIndex(); + PsiExpression modifiedExpression = getModifiedArgument(expressions[i], argumentCast.castType()); if (modifiedExpression == null) return null; PsiExpressionList argumentList = origCall.getArgumentList(); if (argumentList == null) return null; @@ -151,7 +151,41 @@ public abstract class ArgumentFixerActionFactory { return newCall; } + private static void collectCast(@NotNull List<@NotNull ArgumentCast> actualCasts, + @NotNull Map<@NotNull Integer, @NotNull Set<@NotNull String>> suggestedCasts, + @NotNull ArgumentCast argumentCast) { + actualCasts.add(argumentCast); + suggestedCasts.get(argumentCast.argumentIndex()).add(argumentCast.castType().getCanonicalText()); + } + + private List putCompilableCastsFirst(@NotNull List<@NotNull ArgumentCast> casts, + @NotNull PsiExpression @NotNull [] expressions) { + List compilableCasts = new ArrayList<>(); + List nonCompilableCasts = new ArrayList<>(); + for (var argumentCast : casts) { + if (doesFixCauseOtherCompilationErrors(expressions[argumentCast.argumentIndex()], argumentCast.castType())) { + nonCompilableCasts.add(argumentCast); + } + else { + compilableCasts.add(argumentCast); + } + } + return ContainerUtil.concat(compilableCasts, nonCompilableCasts); + } + + protected boolean doesFixCauseOtherCompilationErrors(@NotNull PsiExpression expression, @NotNull PsiType parameterType) { + return false; + } + public abstract boolean areTypesConvertible(@NotNull PsiType exprType, @NotNull PsiType parameterType, @NotNull PsiElement context); public abstract IntentionAction createFix(PsiExpressionList list, int i, PsiType parameterType); + + /** + * Cast of an argument in a method call to a type. + * @param argumentIndex index of the argument that is cast + * @param castType type in a cast expression + */ + private record ArgumentCast(int argumentIndex, @NotNull PsiType castType) { + } } \ No newline at end of file diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/CastMethodArgumentFix.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/CastMethodArgumentFix.java index 1de3281fb8b8..76c2e99d6f8f 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/CastMethodArgumentFix.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/CastMethodArgumentFix.java @@ -9,9 +9,19 @@ import com.intellij.modcommand.Presentation; import com.intellij.psi.*; import com.intellij.psi.util.PsiUtil; import com.intellij.util.IncorrectOperationException; +import com.intellij.util.containers.ContainerUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import java.util.Collection; +import java.util.List; + +import static com.intellij.codeInsight.ExceptionUtil.collectUnhandledExceptions; +import static com.intellij.codeInsight.ExceptionUtil.isHandledBy; +import static com.intellij.psi.LambdaUtil.getFunctionalInterfaceMethod; +import static com.intellij.psi.util.PsiUtil.skipParenthesizedExprDown; +import static java.util.Arrays.asList; + public final class CastMethodArgumentFix extends MethodArgumentFix { private CastMethodArgumentFix(PsiExpressionList list, int i, PsiType toType, final ArgumentFixerActionFactory factory) { super(list, i, toType, factory); @@ -69,6 +79,44 @@ public final class CastMethodArgumentFix extends MethodArgumentFix { return parameterType instanceof PsiEllipsisType ellipsisType && areTypesConvertible(exprType, ellipsisType.getComponentType(), context); } + + /** + * @return true when the parameterType is a functional interface + * AND argument is a functional interface instance + * AND that instance throws an unchecked exception that is not declared by functional interface SAM. + */ + @Override + protected boolean doesFixCauseOtherCompilationErrors(@NotNull PsiExpression expression, @NotNull PsiType parameterType) { + var declaredExceptions = declaredExceptionsOfFunctionalInterface(parameterType); + if (declaredExceptions == null) return false; + var unhandledExceptions = exceptionsThrownByFunctionalExpression(skipParenthesizedExprDown(expression)); + return ContainerUtil.exists(unhandledExceptions, unhandled -> !isHandledBy(unhandled, declaredExceptions)); + } + + private static @NotNull Collection<@NotNull PsiClassType> exceptionsThrownByFunctionalExpression(PsiExpression expression) { + if (expression instanceof PsiLambdaExpression lambdaExpression) { + PsiElement body = lambdaExpression.getBody(); + if (body != null) { + return collectUnhandledExceptions(body, body); + } + } + else if (expression instanceof PsiMethodReferenceExpression methodReferenceExpression) { + PsiElement target = methodReferenceExpression.resolve(); + if (target instanceof PsiMethod psiMethod) { + return asList(psiMethod.getThrowsList().getReferencedTypes()); + } + } + return List.of(); + } + + private static @NotNull PsiClassType @Nullable [] declaredExceptionsOfFunctionalInterface(@NotNull PsiType parameterType) { + if (!(parameterType instanceof PsiClassType classType)) return null; + var psiSubstitutor = classType.resolveGenerics().getSubstitutor(); + var psiMethod = getFunctionalInterfaceMethod(parameterType); + if (psiMethod == null) return null; + var throwListTypes = psiMethod.getThrowsList().getReferencedTypes(); + return ContainerUtil.map(throwListTypes, type -> (PsiClassType)psiSubstitutor.substitute(type)).toArray(PsiClassType[]::new); + } } public static final ArgumentFixerActionFactory REGISTRAR = new MyFixerActionFactory(); diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/impl/quickfix/CastMethodParametersOrderTest.java b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/impl/quickfix/CastMethodParametersOrderTest.java new file mode 100644 index 000000000000..8f857ac432c4 --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/impl/quickfix/CastMethodParametersOrderTest.java @@ -0,0 +1,120 @@ +// Copyright 2000-2025 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.daemon.quickFix.LightQuickFixTestCase; +import com.intellij.util.containers.ContainerUtil; +import org.jetbrains.annotations.NotNull; + +import java.util.List; + +import static com.intellij.testFramework.assertions.Assertions.assertThat; + +public class CastMethodParametersOrderTest extends LightQuickFixTestCase { + + /** + * Verifies that 'cast argument to' fixes, which generate code that contains further compilation errors, + * are placed at the bottom of the quick-fix list. + */ + public void testOrderOfCastMethodArgumentQuickFixesForLambdaArgument() { + String code = """ + import java.io.IOException; + import java.util.concurrent.Callable; + import java.util.function.Supplier; + public class AClass { + void foo1() { + m1(() -> { throw new IOException(); }); + } + void m1(Supplier a) {} + void m1(Callable a) {} + } + """; + var expectedQuickFixes = List.of( + "Cast argument to 'Callable'", + "Cast argument to 'Supplier'"); + assertThatFollowingQuickFixesAreAvailableInOrder(code, expectedQuickFixes); + } + + /** + * Verifies that 'cast argument to' fixes, which generate code that contains further compilation errors, + * are placed at the bottom of the quick-fix list. + */ + public void testOrderOfCastMethodArgumentQuickFixesForMethodReferenceArgument() { + String code = """ + import java.io.IOException; + import java.util.concurrent.Callable; + import java.util.function.Supplier; + public class AClass { + void foo2() { + m1(this::returnString); + } + public String returnString() throws IOException { + return null; + } + void m1(Supplier a) {} + void m1(Callable a) {} + } + """; + List expectedQuickFixes = List.of( + "Cast argument to 'Callable'", + "Cast argument to 'Supplier'"); + assertThatFollowingQuickFixesAreAvailableInOrder(code, expectedQuickFixes); + } + + /** + * Verifies that 'cast argument to' fixes, which generate code that contains further compilation errors, + * are placed at the bottom of the quick-fix list. + */ + public void testOrderOfCastMethodArgumentQuickFixesForParameterWithGenericThrow() { + String code = """ + import java.io.IOException; + import java.util.concurrent.Callable; + import java.util.function.Supplier; + public class AClass { + void foo1() { + m1(() -> { throw new IOException(); }); + } + void m1(Supplier a) {} + void m1(Callable a) {} + void m1(ThrowingRunnable a) {} + } + interface ThrowingRunnable { + void run() throws T; + } + """; + List expectedQuickFixes = List.of( + "Cast argument to 'Callable'", + "Cast argument to 'ThrowingRunnable'", + "Cast argument to 'Supplier'"); + assertThatFollowingQuickFixesAreAvailableInOrder(code, expectedQuickFixes); + } + + public void testOrderOfCastMethodArgumentQuickFixesForParameterWithRawGenericThrow() { + String code = """ + import java.io.IOException; + import java.util.concurrent.Callable; + import java.util.function.Supplier; + public class AClass { + void foo1() { + m1(() -> { throw new IOException(); }); + } + void m1(Supplier a) {} + void m1(Callable a) {} + void m1(ThrowingRunnable a) {} + } + interface ThrowingRunnable { + void run() throws T; + } + """; + List expectedQuickFixes = List.of( + "Cast argument to 'Callable'", + "Cast argument to 'ThrowingRunnable'", + "Cast argument to 'Supplier'"); + assertThatFollowingQuickFixesAreAvailableInOrder(code, expectedQuickFixes); + } + + private void assertThatFollowingQuickFixesAreAvailableInOrder(String code, List<@NotNull String> expectedQuickFixes) { + configureFromFileText("AClass.java", code); + var actionNames = ContainerUtil.map(getAvailableActions(), a -> a.getText()); + assertThat(actionNames).containsSubsequence(expectedQuickFixes); + } +}