From 13e4c24bba9bbca2ec00806c6f4e15888b4467cf Mon Sep 17 00:00:00 2001 From: Mikhail Pyltsin Date: Mon, 2 Dec 2024 12:42:24 +0100 Subject: [PATCH] [java-inspections] IDEA-364128 Inspection PatternVariableCanBeUsedInspection shouldn't propose converting for variable - skip cast for variables - not extend parameters for cast GitOrigin-RevId: 6eb3a63d923fd1625c2c593afb65ce5762a1c0e4 --- .../PatternVariableCanBeUsedInspection.java | 55 ++++++++++++++++--- .../afterCastNotWildcardToWildcard.java | 14 +++++ .../afterCastWildcardWithExtends.java | 16 ++++++ .../beforeCastNoVariables.java | 17 ++++++ .../beforeCastNotWildcardToWildcard.java | 15 +++++ .../beforeCastWildcardWithExtends.java | 17 ++++++ 6 files changed, 127 insertions(+), 7 deletions(-) create mode 100644 java/java-tests/testData/inspection/patternVariableCanBeUsed/afterCastNotWildcardToWildcard.java create mode 100644 java/java-tests/testData/inspection/patternVariableCanBeUsed/afterCastWildcardWithExtends.java create mode 100644 java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeCastNoVariables.java create mode 100644 java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeCastNotWildcardToWildcard.java create mode 100644 java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeCastWildcardWithExtends.java diff --git a/java/java-impl-inspections/src/com/intellij/codeInspection/PatternVariableCanBeUsedInspection.java b/java/java-impl-inspections/src/com/intellij/codeInspection/PatternVariableCanBeUsedInspection.java index 46ddb663b17e..fe38ed074a46 100644 --- a/java/java-impl-inspections/src/com/intellij/codeInspection/PatternVariableCanBeUsedInspection.java +++ b/java/java-impl-inspections/src/com/intellij/codeInspection/PatternVariableCanBeUsedInspection.java @@ -13,10 +13,8 @@ import com.intellij.psi.codeStyle.JavaCodeStyleSettings; import com.intellij.psi.codeStyle.VariableKind; import com.intellij.psi.impl.light.LightRecordMethod; import com.intellij.psi.impl.source.tree.JavaSharedImplUtil; +import com.intellij.psi.util.*; import com.intellij.psi.util.InheritanceUtil; -import com.intellij.psi.util.JavaPsiPatternUtil; -import com.intellij.psi.util.PsiTreeUtil; -import com.intellij.psi.util.PsiUtil; import com.intellij.util.ArrayUtil; import com.intellij.util.ObjectUtils; import com.siyeh.InspectionGadgetsBundle; @@ -27,6 +25,7 @@ import org.jetbrains.annotations.Nullable; import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.Set; import static com.intellij.codeInspection.options.OptPane.checkbox; @@ -136,6 +135,7 @@ public final class PatternVariableCanBeUsedInspection extends AbstractBaseJavaLo @Override public void visitTypeCastExpression(@NotNull PsiTypeCastExpression expression) { + if (expression.getParent() instanceof PsiVariable) return; InstanceOfCandidateResult result = findInstanceOfCandidateResult(expression); if (result == null) return; if (result.instanceOf() != null) { @@ -388,12 +388,53 @@ public final class PatternVariableCanBeUsedInspection extends AbstractBaseJavaLo PsiTypeElement typeElement = instanceOfType; if (instanceOfType != null && instanceOfType.getType() instanceof PsiClassType instanceOfClassType && instanceOfClassType.isRaw()) { if (originalTypeElement.getType() instanceof PsiClassType originalClassType && !originalClassType.isRaw()) { - PsiClass instanceOfClass = PsiUtil.resolveClassInClassTypeOnly(instanceOfClassType); - PsiClass originalClass = PsiUtil.resolveClassInClassTypeOnly(originalClassType); + PsiClassType.ClassResolveResult instanceOfClassResult = instanceOfClassType.resolveGenerics(); + PsiClass instanceOfClass = instanceOfClassResult.getElement(); + PsiClassType.ClassResolveResult originalClassResult = originalClassType.resolveGenerics(); + PsiClass originalClass = originalClassResult.getElement(); if (originalClass != null && originalClass.getQualifiedName() != null) { if (InheritanceUtil.isInheritor(instanceOfClass, false, originalClass.getQualifiedName())) { - PsiClassType genericType = GenericsUtil.getExpectedGenericType(instanceOfClass, instanceOfClass, originalClassType); - typeElement = JavaPsiFacade.getElementFactory(instanceOfClass.getProject()).createTypeElement(genericType); + PsiElementFactory factory = JavaPsiFacade.getElementFactory(instanceOfClass.getProject()); + PsiSubstitutor classSubstitutor; + if (originalClass.getManager().areElementsEquivalent(originalClass, instanceOfClass)) { + classSubstitutor = PsiSubstitutor.EMPTY; + for (PsiTypeParameter parameter : originalClass.getTypeParameters()) { + classSubstitutor = classSubstitutor.put(parameter, factory.createType(parameter)); + } + } + else { + classSubstitutor = JavaClassSupers.getInstance() + .getSuperClassSubstitutor(originalClass, instanceOfClass, instanceOfClass.getResolveScope(), PsiSubstitutor.EMPTY); + } + if (classSubstitutor == null) return typeElement; + PsiSubstitutor target = instanceOfClassResult.getSubstitutor(); + for (Map.Entry originalTypeEntry : originalClassResult.getSubstitutor().getSubstitutionMap() + .entrySet()) { + PsiType keyType = classSubstitutor.getSubstitutionMap().get(originalTypeEntry.getKey()); + if (keyType instanceof PsiClassType classType && classType.resolve() instanceof PsiTypeParameter targetTypeParameter) { + PsiType value = originalTypeEntry.getValue(); + if (value != null && !(value instanceof PsiWildcardType valueWildCard && !valueWildCard.isBounded())) { + PsiType previousValue = target.getSubstitutionMap().get(targetTypeParameter); + if (previousValue != null && (!(previousValue instanceof PsiWildcardType wildcardType) || wildcardType.isBounded())) { + continue; + } + target = target.put(targetTypeParameter, value); + } + } + } + for (Map.Entry entry : target.getSubstitutionMap().entrySet()) { + if (entry.getValue() == null) { + target = target.put(entry.getKey(), PsiWildcardType.createUnbounded(originalClass.getManager())); + } + } + for (PsiTypeParameter parameter : instanceOfClass.getTypeParameters()) { + if (target.getSubstitutionMap().containsKey(parameter)) { + continue; + } + target = target.put(parameter, PsiWildcardType.createUnbounded(originalClass.getManager())); + } + PsiType substituted = factory.createType(instanceOfClass, target); + typeElement = factory.createTypeElement(substituted); } } } diff --git a/java/java-tests/testData/inspection/patternVariableCanBeUsed/afterCastNotWildcardToWildcard.java b/java/java-tests/testData/inspection/patternVariableCanBeUsed/afterCastNotWildcardToWildcard.java new file mode 100644 index 000000000000..d11913b8b43b --- /dev/null +++ b/java/java-tests/testData/inspection/patternVariableCanBeUsed/afterCastNotWildcardToWildcard.java @@ -0,0 +1,14 @@ +// "Replace 'c' with pattern variable" "true-preview" +package pkg; + +import java.util.*; + +public class ComplexCast { + public static void is(Map o) { + if (o instanceof AbstractMap c) { + final Object o1 = c.get(null); + System.out.println(c.size()); + } + } +} + diff --git a/java/java-tests/testData/inspection/patternVariableCanBeUsed/afterCastWildcardWithExtends.java b/java/java-tests/testData/inspection/patternVariableCanBeUsed/afterCastWildcardWithExtends.java new file mode 100644 index 000000000000..bb8c60d26b6b --- /dev/null +++ b/java/java-tests/testData/inspection/patternVariableCanBeUsed/afterCastWildcardWithExtends.java @@ -0,0 +1,16 @@ +// "Replace 'a1' with pattern variable" "true" +package pkg; + +import java.util.Objects; + +class A { + private T t; + + @Override + public final boolean equals(Object o) { + if (!(o instanceof A a1)) return false; + + return Objects.equals(t, a1.t); + } +} + diff --git a/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeCastNoVariables.java b/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeCastNoVariables.java new file mode 100644 index 000000000000..3a7126153490 --- /dev/null +++ b/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeCastNoVariables.java @@ -0,0 +1,17 @@ +// "Replace cast expressions with pattern variable" "false" +package pkg; + +import java.util.Objects; + +class A { + private T t; + + @Override + public final boolean equals(Object o) { + if (!(o instanceof A)) return false; + + final A a1 = (A) o; + return Objects.equals(t, a1.t); + } +} + diff --git a/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeCastNotWildcardToWildcard.java b/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeCastNotWildcardToWildcard.java new file mode 100644 index 000000000000..59d51635e728 --- /dev/null +++ b/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeCastNotWildcardToWildcard.java @@ -0,0 +1,15 @@ +// "Replace 'c' with pattern variable" "true-preview" +package pkg; + +import java.util.*; + +public class ComplexCast { + public static void is(Map o) { + if (o instanceof AbstractMap) { + AbstractMap c = (AbstractMap) o; + final Object o1 = c.get(null); + System.out.println(c.size()); + } + } +} + diff --git a/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeCastWildcardWithExtends.java b/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeCastWildcardWithExtends.java new file mode 100644 index 000000000000..b06e5e5eb084 --- /dev/null +++ b/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeCastWildcardWithExtends.java @@ -0,0 +1,17 @@ +// "Replace 'a1' with pattern variable" "true" +package pkg; + +import java.util.Objects; + +class A { + private T t; + + @Override + public final boolean equals(Object o) { + if (!(o instanceof A)) return false; + + final A a1 = (A) o; + return Objects.equals(t, a1.t); + } +} +