diff --git a/java/java-analysis-impl/resources/messages/InspectionGadgetsBundle.properties b/java/java-analysis-impl/resources/messages/InspectionGadgetsBundle.properties index 9f30111db4f4..0a73368bc03d 100644 --- a/java/java-analysis-impl/resources/messages/InspectionGadgetsBundle.properties +++ b/java/java-analysis-impl/resources/messages/InspectionGadgetsBundle.properties @@ -2334,11 +2334,16 @@ inspection.enhanced.for.with.record.pattern.can.be.used.maximum.depth.disabled=D inspection.pattern.variable.can.be.used.display.name=Pattern variable can be used inspection.pattern.variable.can.be.used.message=Variable ''{0}'' can be replaced with pattern variable +inspection.pattern.variable.can.be.used.instead.of.cast.message=Cast expression can be replaced with pattern variable inspection.pattern.variable.can.be.used.fix.family.name=Replace with pattern variable +inspection.pattern.variable.instead.of.cast.can.be.used.fix.family.name=Replace cast expressions with pattern variable inspection.pattern.variable.can.be.used.fix.name=Replace ''{0}'' with pattern variable inspection.pattern.variable.can.be.used.existing.message=Existing pattern variable ''{0}'' can be used instead of ''{1}'' +inspection.pattern.variable.can.be.used.existing.cast.message=Existing pattern variable ''{0}'' can be used instead of cast expression inspection.pattern.variable.can.be.used.existing.fix.family.name=Replace with existing pattern variable inspection.pattern.variable.can.be.used.existing.fix.name=Replace ''{0}'' with existing pattern variable ''{1}'' +inspection.pattern.variable.instead.of.cast.can.be.used.existing.fix.name=Replace cast expression with existing pattern variable ''{0}'' +inspection.pattern.variable.can.be.used.report.cast.only=Report cast expressions which can be replaced with new pattern variables inspection.cast.can.be.replaced.with.variable.display.name=Cast can be replaced with variable inspection.cast.can.be.replaced.with.variable.message=Variable ''{0}'' can be used instead of ''{1}'' 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 3f53c6f1c590..7bdf8c6e03b8 100644 --- a/java/java-impl-inspections/src/com/intellij/codeInspection/PatternVariableCanBeUsedInspection.java +++ b/java/java-impl-inspections/src/com/intellij/codeInspection/PatternVariableCanBeUsedInspection.java @@ -1,7 +1,8 @@ -// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package com.intellij.codeInspection; import com.intellij.codeInsight.daemon.impl.analysis.HighlightControlFlowUtil; +import com.intellij.codeInspection.options.OptPane; import com.intellij.modcommand.ModPsiUpdater; import com.intellij.modcommand.PsiUpdateModCommandQuickFix; import com.intellij.openapi.project.Project; @@ -23,11 +24,23 @@ import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import java.util.ArrayList; import java.util.List; import java.util.Set; +import static com.intellij.codeInspection.options.OptPane.checkbox; + public final class PatternVariableCanBeUsedInspection extends AbstractBaseJavaLocalInspectionTool implements CleanupLocalInspectionTool { + @SuppressWarnings("PublicField") + public boolean reportOnCastOnly = false; + + @Override + public @NotNull OptPane getOptionsPane() { + return OptPane.pane(checkbox("reportOnCastOnly", + InspectionGadgetsBundle.message("inspection.pattern.variable.can.be.used.report.cast.only"))); + } + @Override public @NotNull Set<@NotNull JavaFeature> requiredFeatures() { return Set.of(JavaFeature.PATTERNS); @@ -118,6 +131,42 @@ public final class PatternVariableCanBeUsedInspection extends AbstractBaseJavaLo return null; } + @Override + public void visitTypeCastExpression(@NotNull PsiTypeCastExpression expression) { + InstanceOfCandidateResult result = findInstanceOfCandidateResult(expression); + if (result == null) return; + if (result.instanceOf() != null) { + PsiPattern pattern = result.instanceOf().getPattern(); + PsiPatternVariable existingPatternVariable = JavaPsiPatternUtil.getPatternVariable(pattern); + if (pattern != null && existingPatternVariable == null) { + return; + } + if (existingPatternVariable != null) { + if (!isFinalOrEffectivelyFinal(existingPatternVariable)) { + return; + } + holder.registerProblem(result.castTypeElement(), + InspectionGadgetsBundle.message("inspection.pattern.variable.can.be.used.existing.cast.message", + existingPatternVariable.getName()), + new ExistingPatternVariableCanBeUsedFix(null, existingPatternVariable)); + } + else { + if (!reportOnCastOnly) { + if (isOnTheFly) { + holder.registerProblem(expression, + InspectionGadgetsBundle.message("inspection.pattern.variable.can.be.used.instead.of.cast.message"), + ProblemHighlightType.INFORMATION, + new CastExpressionsCanBeReplacedWithPatternVariableFix(result.instanceOf())); + } + return; + } + holder.registerProblem(result.castTypeElement(), + InspectionGadgetsBundle.message("inspection.pattern.variable.can.be.used.instead.of.cast.message"), + new CastExpressionsCanBeReplacedWithPatternVariableFix(result.instanceOf())); + } + } + } + @Override public void visitLocalVariable(@NotNull PsiLocalVariable variable) { PsiIdentifier identifier = variable.getNameIdentifier(); @@ -164,15 +213,20 @@ public final class PatternVariableCanBeUsedInspection extends AbstractBaseJavaLo } } } - }; } private static class ExistingPatternVariableCanBeUsedFix extends PsiUpdateModCommandQuickFix { - private final @NotNull String myName; + private final @Nullable String myName; private final @NotNull String myPatternName; - private ExistingPatternVariableCanBeUsedFix(@NotNull String name, @NotNull PsiPatternVariable existingVariable) { + /** + * Creates a fix for using an existing pattern variable. + * + * @param name The name of the variable being replaced (null if there is no such a variable, only cast expressions) + * @param existingVariable The existing pattern variable to use + */ + private ExistingPatternVariableCanBeUsedFix(@Nullable String name, @NotNull PsiPatternVariable existingVariable) { myName = name; myPatternName = existingVariable.getName(); } @@ -181,9 +235,12 @@ public final class PatternVariableCanBeUsedInspection extends AbstractBaseJavaLo @NotNull @Override public String getName() { - return InspectionGadgetsBundle.message("inspection.pattern.variable.can.be.used.existing.fix.name", myName, myPatternName); + if (myName != null) { + return InspectionGadgetsBundle.message("inspection.pattern.variable.can.be.used.existing.fix.name", myName, myPatternName); + } + return InspectionGadgetsBundle.message("inspection.pattern.variable.instead.of.cast.can.be.used.existing.fix.name", myPatternName); } - + @Nls(capitalization = Nls.Capitalization.Sentence) @NotNull @Override @@ -193,7 +250,15 @@ public final class PatternVariableCanBeUsedInspection extends AbstractBaseJavaLo @Override protected void applyFix(@NotNull Project project, @NotNull PsiElement element, @NotNull ModPsiUpdater updater) { - if (!myName.endsWith("()")) { + if (myName == null) { + PsiElement castExpression = PsiTreeUtil.getParentOfType(element, PsiTypeCastExpression.class); + if (castExpression == null) return; + while (castExpression.getParent() instanceof PsiParenthesizedExpression parenthesizedExpression) { + castExpression = parenthesizedExpression; + } + new CommentTracker().replace(castExpression, myPatternName); + } + else if (!myName.endsWith("()")) { PsiLocalVariable variable = PsiTreeUtil.getParentOfType(element, PsiLocalVariable.class); if (variable == null) return; if (VariableAccessUtils.variableIsAssigned(variable)) { @@ -212,8 +277,93 @@ public final class PatternVariableCanBeUsedInspection extends AbstractBaseJavaLo } } - private static class PatternVariableCanBeUsedFix extends PsiUpdateModCommandQuickFix { + private static class CastExpressionsCanBeReplacedWithPatternVariableFix extends PsiUpdateModCommandQuickFix { + + @NotNull private final SmartPsiElementPointer myInstanceOfPointer; + + private CastExpressionsCanBeReplacedWithPatternVariableFix(@NotNull PsiInstanceOfExpression instanceOf) { + myInstanceOfPointer = SmartPointerManager.createPointer(instanceOf); + } + + @Override + public @NotNull String getFamilyName() { + return InspectionGadgetsBundle.message("inspection.pattern.variable.instead.of.cast.can.be.used.fix.family.name"); + } + + @Override + protected void applyFix(@NotNull Project project, @NotNull PsiElement element, @NotNull ModPsiUpdater updater) { + PsiTypeCastExpression castExpression = PsiTreeUtil.getParentOfType(element, false, PsiTypeCastExpression.class); + if (castExpression == null) return; + PsiTypeElement originalTypeElement = castExpression.getCastType(); + if (originalTypeElement == null) return; + PsiInstanceOfExpression originalInstanceOf = myInstanceOfPointer.getElement(); + if (originalInstanceOf == null) return; + PsiInstanceOfExpression instanceOf = PsiTreeUtil.findSameElementInCopy(originalInstanceOf, element.getContainingFile()); + if (instanceOf.getPattern() instanceof PsiDeconstructionPattern) return; + PsiTypeElement instanceOfType = instanceOf.getCheckType(); + PsiTypeElement typeElement = originalTypeElement; + if (instanceOfType != null && instanceOfType.getType() instanceof PsiClassType classType && !classType.isRaw()) { + typeElement = instanceOfType; + } + PsiIfStatement psiIfStatement = PsiTreeUtil.getParentOfType(instanceOf, PsiIfStatement.class); + if (psiIfStatement == null) return; + var visitor = new JavaRecursiveElementVisitor() { + final List myCasts = new ArrayList<>(); + + @Override + public void visitTypeCastExpression(@NotNull PsiTypeCastExpression expression) { + PsiTypeElement castType = expression.getCastType(); + if (castType == null) return; + if (!castType.textMatches(originalTypeElement)) { + return; + } + InstanceOfCandidateResult result = findInstanceOfCandidateResult(expression); + if (result != null && result.instanceOf == instanceOf) { + myCasts.add(expression); + } + } + }; + + psiIfStatement.accept(visitor); + List casts = visitor.myCasts; + if (casts.isEmpty()) return; + + CommentTracker ct = new CommentTracker(); + StringBuilder text = generateTextForInstanceOf(null, ct, instanceOf, typeElement); + if (text == null) return; + PsiElement replaced = ct.replace(instanceOf, text.toString()); + if (!(replaced instanceof PsiInstanceOfExpression instanceOfExpression)) { + return; + } + PsiPrimaryPattern pattern = instanceOfExpression.getPattern(); + if (!(pattern instanceof PsiTypeTestPattern typeTestPattern)) { + return; + } + PsiPatternVariable patternVariable = typeTestPattern.getPatternVariable(); + if (patternVariable == null) return; + String variableName = patternVariable.getName(); + for (PsiTypeCastExpression cast : casts) { + PsiElement currentCast = cast; + CommentTracker castCt = new CommentTracker(); + while (currentCast.getParent() instanceof PsiParenthesizedExpression parenthesizedExpression) { + currentCast = parenthesizedExpression; + } + castCt.replace(currentCast, variableName); + } + final JavaCodeStyleManager codeStyleManager = JavaCodeStyleManager.getInstance(project); + List names = new VariableNameGenerator(instanceOfExpression, VariableKind.LOCAL_VARIABLE) + .byType(patternVariable.getType()).byName( + codeStyleManager.suggestUniqueVariableName(variableName, instanceOfExpression, true)).generateAll(true); + names.remove(patternVariable.getName()); + updater.rename(patternVariable, names); + } + } + + private static class PatternVariableCanBeUsedFix extends PsiUpdateModCommandQuickFix { + @NotNull + private final SmartPsiElementPointer myInstanceOfPointer; + @NotNull private final String myName; private PatternVariableCanBeUsedFix(@NotNull String name, @NotNull PsiInstanceOfExpression instanceOf) { @@ -237,7 +387,7 @@ public final class PatternVariableCanBeUsedInspection extends AbstractBaseJavaLo @Override protected void applyFix(@NotNull Project project, @NotNull PsiElement element, @NotNull ModPsiUpdater updater) { - if (!(element.getParent() instanceof PsiLocalVariable variable)) return; + if (!(element.getParent() instanceof PsiLocalVariable variable)) return; PsiTypeCastExpression cast = ObjectUtils.tryCast(PsiUtil.skipParenthesizedExprDown(variable.getInitializer()), PsiTypeCastExpression.class); if (cast == null) return; @@ -250,22 +400,8 @@ public final class PatternVariableCanBeUsedInspection extends AbstractBaseJavaLo typeElement = instanceOfType; } CommentTracker ct = new CommentTracker(); - StringBuilder text = new StringBuilder(ct.text(instanceOf.getOperand())); - text.append(" instanceof "); - PsiModifierList modifierList = variable.getModifierList(); - JavaCodeStyleSettings codeStyleSettings = JavaCodeStyleSettings.getInstance(variable.getContainingFile()); - if (modifierList != null && modifierList.getTextLength() > 0) { - modifierList.setModifierProperty(PsiModifier.FINAL, codeStyleSettings.GENERATE_FINAL_LOCALS); - text.append(ct.text(modifierList)).append(' '); - } - else if (codeStyleSettings.GENERATE_FINAL_LOCALS) { - text.append("final "); - } - text.append(typeElement.getText()).append(' '); - if (instanceOf.getPattern() instanceof PsiDeconstructionPattern) { - return; - } - text.append(variable.getName()); + StringBuilder text = generateTextForInstanceOf(variable, ct, instanceOf, typeElement); + if (text == null) return; PsiElement replaced = ct.replace(instanceOf, text.toString()); ct.deleteAndRestoreComments(variable); if (!(replaced instanceof PsiInstanceOfExpression instanceOfExpression)) { @@ -288,4 +424,51 @@ public final class PatternVariableCanBeUsedInspection extends AbstractBaseJavaLo updater.rename(patternVariable, names); } } + + private static @Nullable InstanceOfCandidateResult findInstanceOfCandidateResult(@NotNull PsiTypeCastExpression expression) { + PsiTypeElement castTypeElement = expression.getCastType(); + if (castTypeElement == null) return null; + PsiExpression operand = expression.getOperand(); + if (operand == null) return null; + PsiType castType = castTypeElement.getType(); + if (castType instanceof PsiPrimitiveType) return null; + PsiType operandType = operand.getType(); + if (operandType == null || castType.isAssignableFrom(operandType)) return null; + PsiInstanceOfExpression instanceOf = InstanceOfUtils.findPatternCandidate(expression, null); + return new InstanceOfCandidateResult(castTypeElement, instanceOf); + } + + private record InstanceOfCandidateResult(@NotNull PsiTypeElement castTypeElement, + @Nullable PsiInstanceOfExpression instanceOf) { + } + + private static @Nullable StringBuilder generateTextForInstanceOf(@Nullable PsiLocalVariable variable, + @NotNull CommentTracker ct, + @NotNull PsiInstanceOfExpression instanceOf, + @NotNull PsiTypeElement typeElement) { + StringBuilder text = new StringBuilder(ct.text(instanceOf.getOperand())); + text.append(" instanceof "); + PsiModifierList modifierList = variable != null ? variable.getModifierList() : null; + JavaCodeStyleSettings codeStyleSettings = JavaCodeStyleSettings.getInstance(typeElement.getContainingFile()); + if (modifierList != null && modifierList.getTextLength() > 0) { + modifierList.setModifierProperty(PsiModifier.FINAL, codeStyleSettings.GENERATE_FINAL_LOCALS); + text.append(ct.text(modifierList)).append(' '); + } + else if (codeStyleSettings.GENERATE_FINAL_LOCALS) { + text.append("final "); + } + text.append(typeElement.getText()).append(' '); + if (instanceOf.getPattern() instanceof PsiDeconstructionPattern) { + return null; + } + if (variable == null) { + String name = new VariableNameGenerator(instanceOf, VariableKind.LOCAL_VARIABLE) + .byType(typeElement.getType()).generate(true); + text.append(name); + } + else { + text.append(variable.getName()); + } + return text; + } } diff --git a/java/java-tests/testData/inspection/patternVariableCanBeUsed/afterExistingCast.java b/java/java-tests/testData/inspection/patternVariableCanBeUsed/afterExistingCast.java new file mode 100644 index 000000000000..e544aa355fc5 --- /dev/null +++ b/java/java-tests/testData/inspection/patternVariableCanBeUsed/afterExistingCast.java @@ -0,0 +1,12 @@ +// "Replace cast expression with existing pattern variable 'i'" "true-preview" +import java.util.*; + +class X { + void test(Object obj) { + if (obj instanceof Integer i) { + doSomething(i); + } + } + + void doSomething(Integer i) {} +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/patternVariableCanBeUsed/afterExistingCastInsideCondition.java b/java/java-tests/testData/inspection/patternVariableCanBeUsed/afterExistingCastInsideCondition.java new file mode 100644 index 000000000000..9dec7ff77a41 --- /dev/null +++ b/java/java-tests/testData/inspection/patternVariableCanBeUsed/afterExistingCastInsideCondition.java @@ -0,0 +1,11 @@ +// "Replace cast expression with existing pattern variable 'i'" "true-preview" + +class X { + void test(Object obj) { + if (obj instanceof Integer i && i.intValue() == 1) { + doSomething(1); + } + } + + void doSomething(Integer i) {} +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/patternVariableCanBeUsed/afterReplaceCastsWithNewName.java b/java/java-tests/testData/inspection/patternVariableCanBeUsed/afterReplaceCastsWithNewName.java new file mode 100644 index 000000000000..797d392b3882 --- /dev/null +++ b/java/java-tests/testData/inspection/patternVariableCanBeUsed/afterReplaceCastsWithNewName.java @@ -0,0 +1,12 @@ +// "Replace cast expressions with pattern variable" "true" +import java.util.*; + +class X { + void test(Object obj) { + if (obj instanceof Integer integer && integer.intValue() == 1) { + doSomething(integer); + } + } + + void doSomething(Integer i) {} +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeExistingCast.java b/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeExistingCast.java new file mode 100644 index 000000000000..fbeb3f49093d --- /dev/null +++ b/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeExistingCast.java @@ -0,0 +1,12 @@ +// "Replace cast expression with existing pattern variable 'i'" "true-preview" +import java.util.*; + +class X { + void test(Object obj) { + if (obj instanceof Integer i) { + doSomething((Integer)obj); + } + } + + void doSomething(Integer i) {} +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeExistingCastInsideCondition.java b/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeExistingCastInsideCondition.java new file mode 100644 index 000000000000..2ba41f37c74e --- /dev/null +++ b/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeExistingCastInsideCondition.java @@ -0,0 +1,11 @@ +// "Replace cast expression with existing pattern variable 'i'" "true-preview" + +class X { + void test(Object obj) { + if (obj instanceof Integer i && ((Integer)obj).intValue() == 1) { + doSomething(1); + } + } + + void doSomething(Integer i) {} +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeReplaceCastsWithNewName.java b/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeReplaceCastsWithNewName.java new file mode 100644 index 000000000000..d37593d274ea --- /dev/null +++ b/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeReplaceCastsWithNewName.java @@ -0,0 +1,12 @@ +// "Replace cast expressions with pattern variable" "true" +import java.util.*; + +class X { + void test(Object obj) { + if (obj instanceof Integer && ((Integer)obj).intValue() == 1) { + doSomething((Integer)obj); + } + } + + void doSomething(Integer i) {} +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/PatternVariableCanBeUsedInspectionTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/PatternVariableCanBeUsedInspectionTest.java index 671f5a069767..4cdbc731af4b 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/PatternVariableCanBeUsedInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/PatternVariableCanBeUsedInspectionTest.java @@ -12,7 +12,9 @@ import org.jetbrains.annotations.NotNull; public class PatternVariableCanBeUsedInspectionTest extends LightQuickFixParameterizedTestCase { @Override protected LocalInspectionTool @NotNull [] configureLocalInspectionTools() { - return new LocalInspectionTool[]{new PatternVariableCanBeUsedInspection()}; + PatternVariableCanBeUsedInspection inspection = new PatternVariableCanBeUsedInspection(); + inspection.reportOnCastOnly = true; + return new LocalInspectionTool[]{inspection}; } @Override