From ca9fffa746f8144cf82f2c096966c7fec8393de1 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Wed, 30 Aug 2017 14:25:55 +0700 Subject: [PATCH] IDEA-178242 Extract Set from comparison chain: find duplicates within class --- .../ExtractSetFromComparisonChainAction.java | 157 +++++++++++++----- .../afterDuplicate.java | 35 ++++ .../beforeDuplicate.java | 29 ++++ .../src/messages/CodeInsightBundle.properties | 1 + 4 files changed, 184 insertions(+), 38 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/extractSetFromComparison/afterDuplicate.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/extractSetFromComparison/beforeDuplicate.java diff --git a/java/java-impl/src/com/intellij/codeInsight/intention/impl/ExtractSetFromComparisonChainAction.java b/java/java-impl/src/com/intellij/codeInsight/intention/impl/ExtractSetFromComparisonChainAction.java index 2c0f73fa0fc5..5a97845bb76b 100644 --- a/java/java-impl/src/com/intellij/codeInsight/intention/impl/ExtractSetFromComparisonChainAction.java +++ b/java/java-impl/src/com/intellij/codeInsight/intention/impl/ExtractSetFromComparisonChainAction.java @@ -16,10 +16,15 @@ package com.intellij.codeInsight.intention.impl; import com.intellij.codeInsight.CodeInsightBundle; +import com.intellij.codeInsight.FileModificationService; import com.intellij.codeInsight.PsiEquivalenceUtil; import com.intellij.codeInsight.intention.PsiElementBaseIntentionAction; +import com.intellij.openapi.application.ApplicationManager; +import com.intellij.openapi.application.ApplicationNamesInfo; +import com.intellij.openapi.application.WriteAction; import com.intellij.openapi.editor.Editor; import com.intellij.openapi.project.Project; +import com.intellij.openapi.ui.Messages; import com.intellij.openapi.util.Pair; import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.*; @@ -35,7 +40,7 @@ import com.intellij.psi.util.PsiUtil; import com.intellij.refactoring.rename.inplace.MemberInplaceRenamer; import com.intellij.util.ArrayUtil; import com.intellij.util.IncorrectOperationException; -import com.intellij.util.ObjectUtils; +import com.intellij.util.ThrowableRunnable; import com.siyeh.ig.callMatcher.CallMatcher; import com.siyeh.ig.psiutils.ClassUtils; import com.siyeh.ig.psiutils.ExpressionUtils; @@ -48,9 +53,9 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.text.MessageFormat; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Objects; +import java.util.*; + +import static com.intellij.util.ObjectUtils.tryCast; /** * @author Tagir Valeev @@ -75,34 +80,93 @@ public class ExtractSetFromComparisonChainAction extends PsiElementBaseIntention CommonClassNames.JAVA_UTIL_COLLECTIONS + ".unmodifiableSet(" + "java.util.EnumSet.of({0}))"; + @Override + public boolean startInWriteAction() { + return false; + } + @Override public void invoke(@NotNull Project project, Editor editor, @NotNull PsiElement element) throws IncorrectOperationException { + if (!FileModificationService.getInstance().preparePsiElementForWrite(element)) return; + + PsiDocumentManager.getInstance(project).commitAllDocuments(); + List comparisons = comparisons(element).toList(); if (comparisons.size() < 2) return; - PsiExpression firstComparison = comparisons.get(0).myComparison; - PsiExpression lastComparison = comparisons.get(comparisons.size() - 1).myComparison; - PsiExpression disjunction = ObjectUtils.tryCast(firstComparison.getParent(), PsiPolyadicExpression.class); - if (disjunction == null) return; - PsiExpression stringExpression = comparisons.get(0).myExpression; - PsiClass containingClass = ClassUtils.getContainingStaticClass(disjunction); + PsiClass containingClass = ClassUtils.getContainingStaticClass(element); if (containingClass == null) return; + List> copies = findCopies(comparisons, containingClass); JavaCodeStyleManager manager = JavaCodeStyleManager.getInstance(project); PsiElementFactory factory = JavaPsiFacade.getElementFactory(project); LinkedHashSet suggestions = getSuggestions(comparisons); - String name = manager.suggestUniqueVariableName(suggestions.iterator().next(), containingClass, false); - String fieldInitializer = StreamEx.of(comparisons).map(cmp -> cmp.myConstant.getText()).joining(","); - String pattern = getInitializer(comparisons.get(0).myType, containingClass); - String elementType = comparisons.get(0).myType.getCanonicalText(); - String initializer = MessageFormat.format(pattern, fieldInitializer, elementType); - String modifiers = containingClass.isInterface() ? "" : "private static final "; - String type = CommonClassNames.JAVA_UTIL_SET + - (PsiUtil.isLanguageLevel5OrHigher(containingClass) ? "<" + elementType + ">" : ""); - PsiField field = factory.createFieldFromText(modifiers + type + " " + - name + "=" + initializer + ";", containingClass); - field = (PsiField)containingClass.add(field); - PsiDiamondTypeUtil.removeRedundantTypeArguments(field); - CodeStyleManager.getInstance(project).reformat(manager.shortenClassReferences(field)); + class Extractor implements ThrowableRunnable { + PsiElement result; + PsiField field; + + @Override + public void run() { + if (!containingClass.isValid()) return; + String name = manager.suggestUniqueVariableName(suggestions.iterator().next(), containingClass, false); + String fieldInitializer = StreamEx.of(comparisons).map(cmp -> cmp.myConstant.getText()).joining(","); + String pattern = ExtractSetFromComparisonChainAction.this.getInitializer(comparisons.get(0).myType, containingClass); + String elementType = comparisons.get(0).myType.getCanonicalText(); + String initializer = MessageFormat.format(pattern, fieldInitializer, elementType); + String modifiers = containingClass.isInterface() ? "" : "private static final "; + String type = CommonClassNames.JAVA_UTIL_SET + (PsiUtil.isLanguageLevel5OrHigher(containingClass) ? "<" + elementType + ">" : ""); + field = factory.createFieldFromText(modifiers + type + " " + name + "=" + initializer + ";", containingClass); + field = (PsiField)containingClass.add(field); + PsiDiamondTypeUtil.removeRedundantTypeArguments(field); + CodeStyleManager.getInstance(project).reformat(manager.shortenClassReferences(field)); + + result = replace(comparisons, field); + } + } + Extractor extractor = new Extractor(); + WriteAction.run(extractor); + PsiField field = extractor.field; + if (field == null || !field.isValid()) return; + + if (!copies.isEmpty()) { + int answer = ApplicationManager.getApplication().isUnitTestMode() ? Messages.YES : + Messages.showYesNoDialog(project, + CodeInsightBundle.message("intention.extract.set.from.comparison.chain.duplicates", + ApplicationNamesInfo.getInstance().getProductName(), + copies.size()), "Process Duplicates", + Messages.getQuestionIcon()); + if (answer == Messages.YES) { + WriteAction.run(() -> { + for (List copy : copies) { + replace(copy, field); + } + }); + } + } + + PsiElement result = extractor.result; + if (result == null || !result.isValid()) return; + PsiReferenceExpression fieldRef = + tryCast(ReferencesSearch.search(field, new LocalSearchScope(result)).findFirst(), PsiReferenceExpression.class); + if (fieldRef == null) return; + + PsiDocumentManager.getInstance(project).doPostponedOperationsAndUnblockDocument(editor.getDocument()); + editor.getCaretModel().moveToOffset(fieldRef.getTextOffset()); + editor.getSelectionModel().removeSelection(); + new MemberInplaceRenamer(field, field, editor).performInplaceRefactoring(suggestions); + } + + private static PsiElement replace(List comparisons, PsiField field) { + if (comparisons.isEmpty()) return null; + Project project = field.getProject(); + PsiClass containingClass = field.getContainingClass(); + if (containingClass == null) return null; + String name = field.getName(); + if (name == null) return null; + PsiExpression stringExpression = comparisons.get(0).myExpression; + PsiExpression firstComparison = comparisons.get(0).myComparison; + PsiExpression lastComparison = comparisons.get(comparisons.size() - 1).myComparison; + PsiExpression disjunction = tryCast(firstComparison.getParent(), PsiPolyadicExpression.class); + if (disjunction == null) return null; int startOffset = firstComparison.getStartOffsetInParent(); int endOffset = lastComparison.getStartOffsetInParent() + lastComparison.getTextLength(); String origText = disjunction.getText(); @@ -111,20 +175,37 @@ public class ExtractSetFromComparisonChainAction extends PsiElementBaseIntention String replacementText = origText.substring(0, startOffset) + fieldReference + ".contains(" + stringExpression.getText() + ")" + origText.substring(endOffset); - PsiExpression replacement = factory.createExpressionFromText(replacementText, disjunction); + PsiExpression replacement = JavaPsiFacade.getElementFactory(project).createExpressionFromText(replacementText, disjunction); if (replacement instanceof PsiMethodCallExpression && disjunction.getParent() instanceof PsiParenthesizedExpression) { disjunction = (PsiExpression)disjunction.getParent(); } - PsiElement result = disjunction.replace(replacement); + return disjunction.replace(replacement); + } - PsiReferenceExpression fieldRef = - ObjectUtils.tryCast(ReferencesSearch.search(field, new LocalSearchScope(result)).findFirst(), PsiReferenceExpression.class); - if (fieldRef == null) return; - - PsiDocumentManager.getInstance(project).doPostponedOperationsAndUnblockDocument(editor.getDocument()); - editor.getCaretModel().moveToOffset(fieldRef.getTextOffset()); - editor.getSelectionModel().removeSelection(); - new MemberInplaceRenamer(field, field, editor).performInplaceRefactoring(suggestions); + private static List> findCopies(@NotNull List comparisons, + @NotNull PsiClass aClass) { + Set orig = StreamEx.of(comparisons).map(c -> c.myConstantRepresentation).toSet(); + List> copies = new ArrayList<>(); + Set processedOperands = new HashSet<>(); + aClass.accept(new JavaRecursiveElementWalkingVisitor() { + @Override + public void visitPolyadicExpression(PsiPolyadicExpression expression) { + super.visitPolyadicExpression(expression); + if (!expression.getOperationTokenType().equals(JavaTokenType.OROR)) return; + for (PsiExpression operand : expression.getOperands()) { + if (processedOperands.contains(operand)) continue; + List otherComparisons = comparisons(operand).toList(); + otherComparisons.stream().map(c -> c.myComparison).forEach(processedOperands::add); + if (otherComparisons.size() == comparisons.size() && + otherComparisons.get(0).myExpression != comparisons.get(0).myExpression && + otherComparisons.get(0).myType.equals(comparisons.get(0).myType) + && StreamEx.of(otherComparisons).map(c -> c.myConstantRepresentation).toSet().equals(orig)) { + copies.add(otherComparisons); + } + } + } + }); + return copies; } @NotNull @@ -235,7 +316,7 @@ public class ExtractSetFromComparisonChainAction extends PsiElementBaseIntention static ExpressionToConstantComparison create(PsiExpression candidate) { candidate = PsiUtil.skipParenthesizedExprDown(candidate); - PsiMethodCallExpression call = ObjectUtils.tryCast(candidate, PsiMethodCallExpression.class); + PsiMethodCallExpression call = tryCast(candidate, PsiMethodCallExpression.class); if (call != null) { if (MethodCallUtils.isEqualsCall(call)) { PsiExpression qualifier = call.getMethodExpression().getQualifierExpression(); @@ -248,7 +329,7 @@ public class ExtractSetFromComparisonChainAction extends PsiElementBaseIntention } return null; } - PsiBinaryExpression binOp = ObjectUtils.tryCast(candidate, PsiBinaryExpression.class); + PsiBinaryExpression binOp = tryCast(candidate, PsiBinaryExpression.class); if (binOp != null && JavaTokenType.EQEQ.equals(binOp.getOperationTokenType())) { return fromComparison(candidate, binOp.getLOperand(), binOp.getROperand()); } @@ -265,13 +346,13 @@ public class ExtractSetFromComparisonChainAction extends PsiElementBaseIntention @Nullable private static ExpressionToConstantComparison tryExtract(PsiExpression candidate, PsiExpression constant, PsiExpression nonConstant) { - String constantValue = ObjectUtils.tryCast(ExpressionUtils.computeConstantExpression(constant), String.class); + String constantValue = tryCast(ExpressionUtils.computeConstantExpression(constant), String.class); if (constantValue != null) { return new ExpressionToConstantComparison(candidate, nonConstant, constant, constantValue); } - PsiReferenceExpression ref = ObjectUtils.tryCast(PsiUtil.skipParenthesizedExprDown(constant), PsiReferenceExpression.class); + PsiReferenceExpression ref = tryCast(PsiUtil.skipParenthesizedExprDown(constant), PsiReferenceExpression.class); if (ref != null) { - PsiEnumConstant enumConstant = ObjectUtils.tryCast(ref.resolve(), PsiEnumConstant.class); + PsiEnumConstant enumConstant = tryCast(ref.resolve(), PsiEnumConstant.class); if (enumConstant != null && enumConstant.getName() != null) { return new ExpressionToConstantComparison(candidate, nonConstant, ref, enumConstant.getName()); } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/extractSetFromComparison/afterDuplicate.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/extractSetFromComparison/afterDuplicate.java new file mode 100644 index 000000000000..a794a90ed46a --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/extractSetFromComparison/afterDuplicate.java @@ -0,0 +1,35 @@ +// "Extract Set from comparison chain" "true" + +import java.util.Collections; +import java.util.EnumSet; +import java.util.Set; + +public class Test { + private static final Set STATUSES = Collections.unmodifiableSet(EnumSet.of(Status.VALID, Status.PENDING)); + + enum Status { + VALID, PENDING, INVALID, UNKNOWN; + } + + void test1(Status status) { + if(STATUSES.contains(status)) { + System.out.println("ok"); + } + } + + static class Another { + static final String STATUSES = ""; + + void test2(Status st) { + if(st == null || Test.STATUSES.contains(st) || Math.random() > 0.5) { + System.out.println("Replace here as well"); + } + } + + void test3(Status st2) { + if(st2 == Status.VALID || st2 == Status.PENDING || st2 == Status.UNKNOWN) { + System.out.println("Do not replace as we test three statuses"); + } + } + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/extractSetFromComparison/beforeDuplicate.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/extractSetFromComparison/beforeDuplicate.java new file mode 100644 index 000000000000..1bb24c43f709 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/extractSetFromComparison/beforeDuplicate.java @@ -0,0 +1,29 @@ +// "Extract Set from comparison chain" "true" + +public class Test { + enum Status { + VALID, PENDING, INVALID, UNKNOWN; + } + + void test1(Status status) { + if(status == Status.VALID || status == Status.PENDING) { + System.out.println("ok"); + } + } + + static class Another { + static final String STATUSES = ""; + + void test2(Status st) { + if(st == null || Status.PENDING == st || Status.VALID == st || Math.random() > 0.5) { + System.out.println("Replace here as well"); + } + } + + void test3(Status st2) { + if(st2 == Status.VALID || st2 == Status.PENDING || st2 == Status.UNKNOWN) { + System.out.println("Do not replace as we test three statuses"); + } + } + } +} diff --git a/platform/platform-resources-en/src/messages/CodeInsightBundle.properties b/platform/platform-resources-en/src/messages/CodeInsightBundle.properties index 4e3741798e72..c183189d70a6 100644 --- a/platform/platform-resources-en/src/messages/CodeInsightBundle.properties +++ b/platform/platform-resources-en/src/messages/CodeInsightBundle.properties @@ -551,6 +551,7 @@ inlay.hints.disable.custom.option=Disable ''{0}'' inlay.hints.enable.custom.option=Enable ''{0}'' intention.extract.set.from.comparison.chain.family=Extract Set from comparison chain +intention.extract.set.from.comparison.chain.duplicates={0} has detected {1} code {1,choice,1#fragment|2#fragments} in this class that can be replaced using newly created Set. Would you like to replace {1,choice,1#it|2#them}? block.comment.intersects.existing.comment=Selected region intersects existing comment block.comment.wrapping.suffix=Selected region contains block comment suffix