IDEA-168201 Intention to extract a set from "foo".equals(xyz) || "bar".equals(xyz)

java/java-impl/src/com/intellij/codeInsight/intention/impl/ExtractSetFromComparisonChainAction.java

@@ -0,0 +1,185 @@
+/*
+ * Copyright 2000-2017 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.intellij.codeInsight.intention.impl;
+
+import com.intellij.codeInsight.intention.PsiElementBaseIntentionAction;
+import com.intellij.openapi.editor.Editor;
+import com.intellij.openapi.project.Project;
+import com.intellij.openapi.util.Pair;
+import com.intellij.openapi.util.text.StringUtil;
+import com.intellij.psi.*;
+import com.intellij.psi.codeStyle.CodeStyleManager;
+import com.intellij.psi.codeStyle.JavaCodeStyleManager;
+import com.intellij.psi.codeStyle.SuggestedNameInfo;
+import com.intellij.psi.codeStyle.VariableKind;
+import com.intellij.psi.impl.PsiDiamondTypeUtil;
+import com.intellij.psi.search.LocalSearchScope;
+import com.intellij.psi.search.searches.ReferencesSearch;
+import com.intellij.psi.util.PsiTreeUtil;
+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.siyeh.ig.psiutils.ClassUtils;
+import com.siyeh.ig.psiutils.EquivalenceChecker;
+import com.siyeh.ig.psiutils.ExpressionUtils;
+import com.siyeh.ig.psiutils.MethodCallUtils;
+import one.util.streamex.IntStreamEx;
+import one.util.streamex.MoreCollectors;
+import one.util.streamex.StreamEx;
+import org.jetbrains.annotations.Nls;
+import org.jetbrains.annotations.NotNull;
+
+import java.text.MessageFormat;
+import java.util.LinkedHashSet;
+import java.util.List;
+
+/**
+ * @author Tagir Valeev
+ */
+public class ExtractSetFromComparisonChainAction extends PsiElementBaseIntentionAction {
+  private static final String INITIALIZER_FORMAT_JAVA8 = "new " +
+                                                         CommonClassNames.JAVA_UTIL_HASH_SET +
+                                                         "<" +
+                                                         CommonClassNames.JAVA_LANG_STRING +
+                                                         ">(" +
+                                                         CommonClassNames.JAVA_UTIL_ARRAYS +
+                                                         ".asList({0}))";
+  private static final String INITIALIZER_FORMAT_JAVA9 = CommonClassNames.JAVA_UTIL_SET + ".of({0})";
+
+  @Override
+  public void invoke(@NotNull Project project, Editor editor, @NotNull PsiElement element) throws IncorrectOperationException {
+    List<PsiExpression> disjuncts = disjuncts(element).toList();
+    if (disjuncts.size() < 2) return;
+    PsiExpression disjunction = ObjectUtils.tryCast(disjuncts.get(0).getParent(), PsiPolyadicExpression.class);
+    if (disjunction == null) return;
+    PsiExpression stringExpression = getValueComparedToString(disjuncts.get(0));
+    if (stringExpression == null) return;
+    PsiClass containingClass = ClassUtils.getContainingStaticClass(disjunction);
+    if (containingClass == null) return;
+    JavaCodeStyleManager manager = JavaCodeStyleManager.getInstance(project);
+    PsiElementFactory factory = JavaPsiFacade.getElementFactory(project);
+    LinkedHashSet<String> suggestions = getSuggestions(disjuncts, stringExpression);
+    String name = manager.suggestUniqueVariableName(suggestions.iterator().next(), containingClass, false);
+    String fieldInitializer = qualifiers(disjuncts).map(PsiElement::getText).joining(",");
+    String pattern = PsiUtil.isLanguageLevel9OrHigher(containingClass) ? INITIALIZER_FORMAT_JAVA9 : INITIALIZER_FORMAT_JAVA8;
+    String initializer = MessageFormat.format(pattern, fieldInitializer);
+    PsiField field = factory.createFieldFromText("private static final " +
+                                                 CommonClassNames.JAVA_UTIL_SET + "<" + CommonClassNames.JAVA_LANG_STRING + "> " +
+                                                 name + "=" + initializer + ";", containingClass);
+    field = (PsiField)containingClass.add(field);
+    PsiDiamondTypeUtil.removeRedundantTypeArguments(field);
+    CodeStyleManager.getInstance(project).reformat(manager.shortenClassReferences(field));
+
+    int startOffset = disjuncts.get(0).getStartOffsetInParent();
+    int endOffset = disjuncts.get(disjuncts.size() - 1).getStartOffsetInParent() + disjuncts.get(disjuncts.size() - 1).getTextLength();
+    String origText = disjunction.getText();
+    String fieldReference = PsiResolveHelper.SERVICE.getInstance(project).resolveReferencedVariable(name, disjunction) == field ?
+                            name : containingClass.getQualifiedName() + "." + name;
+    String replacementText = origText.substring(0, startOffset) +
+                             fieldReference + ".contains(" + stringExpression.getText() + ")" +
+                             origText.substring(endOffset);
+    PsiExpression replacement = factory.createExpressionFromText(replacementText, disjunction);
+    if (replacement instanceof PsiMethodCallExpression && disjunction.getParent() instanceof PsiParenthesizedExpression) {
+      disjunction = (PsiExpression)disjunction.getParent();
+    }
+    PsiElement result = 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);
+  }
+
+  @Override
+  public boolean isAvailable(@NotNull Project project, Editor editor, @NotNull PsiElement element) {
+    return disjuncts(element).count() > 1;
+  }
+
+  @NotNull
+  @Override
+  public String getText() {
+    return getFamilyName();
+  }
+
+  @Nls
+  @NotNull
+  @Override
+  public String getFamilyName() {
+    return "Extract Set from comparison chain";
+  }
+
+  @NotNull
+  private static LinkedHashSet<String> getSuggestions(List<PsiExpression> disjuncts, PsiExpression stringExpression) {
+    Project project = stringExpression.getProject();
+    JavaCodeStyleManager manager = JavaCodeStyleManager.getInstance(project);
+    PsiElementFactory factory = JavaPsiFacade.getElementFactory(project);
+    SuggestedNameInfo info = manager.suggestVariableName(VariableKind.STATIC_FINAL_FIELD, null, stringExpression,
+                                                         factory.createTypeFromText(CommonClassNames.JAVA_LANG_STRING, stringExpression),
+                                                         false);
+    // Suggestions like OBJECT and AN_OBJECT appear because Object.equals argument type is an Object,
+    // such names are rarely appropriate
+    LinkedHashSet<String> suggestions =
+      StreamEx.of(info.names).without("OBJECT", "AN_OBJECT").map(StringUtil::pluralize).nonNull().toCollection(LinkedHashSet::new);
+    Pair<String, String> prefixSuffix = qualifiers(disjuncts).map(ExpressionUtils::computeConstantExpression).select(String.class).collect(
+      MoreCollectors.pairing(MoreCollectors.commonPrefix(), MoreCollectors.commonSuffix(), Pair::create));
+    StreamEx.of(prefixSuffix.first, prefixSuffix.second).flatMap(str -> StreamEx.split(str, "\\W+").limit(3))
+      .filter(str -> str.length() >= 3 && StringUtil.isJavaIdentifier(str))
+      .flatMap(str -> StreamEx.of(manager.suggestVariableName(VariableKind.STATIC_FINAL_FIELD, str, null, null).names))
+      .limit(5)
+      .map(StringUtil::pluralize)
+      .forEach(suggestions::add);
+    suggestions.add("STRINGS");
+    return suggestions;
+  }
+
+  private static StreamEx<PsiExpression> qualifiers(List<PsiExpression> expressions) {
+    return StreamEx.of(expressions).map(PsiUtil::skipParenthesizedExprDown).select(PsiMethodCallExpression.class)
+      .map(call -> call.getMethodExpression().getQualifierExpression()).nonNull();
+  }
+
+  private static StreamEx<PsiExpression> disjuncts(PsiElement element) {
+    PsiPolyadicExpression disjunction = PsiTreeUtil.getParentOfType(element, PsiPolyadicExpression.class);
+    if (disjunction == null || disjunction.getOperationTokenType() != JavaTokenType.OROR) return StreamEx.empty();
+    PsiExpression[] operands = disjunction.getOperands();
+    int offset = element.getTextOffset() - disjunction.getTextOffset();
+    int index = IntStreamEx.ofIndices(operands, op -> op.getStartOffsetInParent() + op.getTextLength() > offset)
+      .findFirst().orElse(operands.length - 1);
+    PsiExpression comparedValue = getValueComparedToString(operands[index]);
+    if (comparedValue == null) return StreamEx.empty();
+    EquivalenceChecker checker = EquivalenceChecker.getCanonicalPsiEquivalence();
+    List<PsiExpression> prefix = IntStreamEx.rangeClosed(index - 1, 0, -1)
+      .elements(operands)
+      .takeWhile(op -> checker.expressionsAreEquivalent(getValueComparedToString(op), comparedValue))
+      .toList();
+    List<PsiExpression> suffix = StreamEx.of(operands, index + 1, operands.length)
+      .takeWhile(op -> checker.expressionsAreEquivalent(getValueComparedToString(op), comparedValue))
+      .toList();
+    return StreamEx.ofReversed(prefix).append(operands[index]).append(suffix);
+  }
+
+  private static PsiExpression getValueComparedToString(PsiExpression expression) {
+    PsiMethodCallExpression call = ObjectUtils.tryCast(PsiUtil.skipParenthesizedExprDown(expression), PsiMethodCallExpression.class);
+    if (call == null || !MethodCallUtils.isEqualsCall(call)) return null;
+    if (!(ExpressionUtils.computeConstantExpression(call.getMethodExpression().getQualifierExpression()) instanceof String)) return null;
+    return ArrayUtil.getFirstElement(call.getArgumentList().getExpressions());
+  }
+}

java/java-impl/src/com/intellij/psi/impl/source/codeStyle/JavaCodeStyleManagerImpl.java

@@ -34,6 +34,7 @@ import com.intellij.util.BitUtil;
 import com.intellij.util.IncorrectOperationException;
 import com.intellij.util.containers.ContainerUtil;
 import gnu.trove.THashSet;
+import one.util.streamex.StreamEx;
 import org.jetbrains.annotations.NonNls;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -586,8 +587,9 @@ public class JavaCodeStyleManagerImpl extends JavaCodeStyleManager {
               final PsiExpression qualifierExpression = methodExpr.getQualifierExpression();
               if (qualifierExpression instanceof PsiReferenceExpression &&
                   ((PsiReferenceExpression)qualifierExpression).resolve() instanceof PsiVariable) {
-                names = ArrayUtil.append(names, StringUtil
-                  .sanitizeJavaIdentifier(changeIfNotIdentifier(qualifierExpression.getText() + StringUtil.capitalize(propertyName))));
+                String name = qualifierExpression.getText() + StringUtil.capitalize(propertyName);
+                String[] propertySuggestions = getSuggestionsByName(name, variableKind, false, correctKeywords);
+                names = StreamEx.of(names).append(propertySuggestions).distinct().toArray(String[]::new);
               }
               return new NamesByExprInfo(propertyName, names);
             }

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/extractSetFromComparison/afterAdditionalLeft.java

@@ -0,0 +1,14 @@
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+// "Extract Set from comparison chain" "true"
+public class Test {
+    private static final Set<String> NAMES = new HashSet<>(Arrays.asList("foo", "bar", "baz"));
+
+    void testOr(String name) {
+    if(name == null || NAMES.contains(name)) {
+      System.out.println("foobarbaz");
+    }
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/extractSetFromComparison/afterAdditionalRight.java

@@ -0,0 +1,18 @@
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+// "Extract Set from comparison chain" "true"
+public class Test {
+    private static final Set<String> NAMES = new HashSet<>(Arrays.asList("foo", "bar", "baz"));
+
+    interface Person {
+    String getName();
+  }
+
+  void testOr(Person person) {
+    if(NAMES.contains(person.getName()) || person.getName() == null) {
+      System.out.println("foobarbaz");
+    }
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/extractSetFromComparison/afterParentheses.java

@@ -0,0 +1,17 @@
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+// "Extract Set from comparison chain" "true"
+public class Test {
+  public static final String BAR = "bar";
+    private static final Set<String> PROPERTIES = new HashSet<>(Arrays.asList("foo", BAR, "baz"));
+
+    void testOr(int i, String property) {
+    int PROPERTIES;
+
+    if(i > 0 && Test.PROPERTIES.contains(property)) {
+      System.out.println("foobarbaz");
+    }
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/extractSetFromComparison/afterSimple.java

@@ -0,0 +1,14 @@
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+// "Extract Set from comparison chain" "true"
+public class Test {
+    private static final Set<String> S = new HashSet<>(Arrays.asList("foo", "bar", "baz"));
+
+    void testOr(String s) {
+    if(S.contains(s)) {
+      System.out.println("foobarbaz");
+    }
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/extractSetFromComparison/beforeAdditionalLeft.java

@@ -0,0 +1,8 @@
+// "Extract Set from comparison chain" "true"
+public class Test {
+  void testOr(String name) {
+    if(name == null || <caret>"foo".equals(name) || "bar".equals(name) || "baz".equals(name)) {
+      System.out.println("foobarbaz");
+    }
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/extractSetFromComparison/beforeAdditionalLeftWrongPlace.java

@@ -0,0 +1,8 @@
+// "Extract Set from comparison chain" "false"
+public class Test {
+  void testOr(String s) {
+    if(s == nul<caret>l || "foo".equals(s) || "bar".equals(s) || "baz".equals(s)) {
+      System.out.println("foobarbaz");
+    }
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/extractSetFromComparison/beforeAdditionalRight.java

@@ -0,0 +1,12 @@
+// "Extract Set from comparison chain" "true"
+public class Test {
+  interface Person {
+    String getName();
+  }
+
+  void testOr(Person person) {
+    if("foo".equals(person.getName()) || "bar".equals(person.getName()) || "baz".equals(person.getName()<caret>) || person.getName() == null) {
+      System.out.println("foobarbaz");
+    }
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/extractSetFromComparison/beforeAdditionalRightWrongPlace.java

@@ -0,0 +1,8 @@
+// "Extract Set from comparison chain" "false"
+public class Test {
+  void testOr(String s) {
+    if("foo".equals(s) || "bar".equals(s) || "baz".equals(s) || s<caret> == null) {
+      System.out.println("foobarbaz");
+    }
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/extractSetFromComparison/beforeParentheses.java

@@ -0,0 +1,12 @@
+// "Extract Set from comparison chain" "true"
+public class Test {
+  public static final String BAR = "bar";
+
+  void testOr(int i, String property) {
+    int PROPERTIES;
+
+    if(i > 0 && ("foo"<caret>.equals(property) || BAR.equals(property) || "baz".equals(property))) {
+      System.out.println("foobarbaz");
+    }
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/extractSetFromComparison/beforeSimple.java

@@ -0,0 +1,8 @@
+// "Extract Set from comparison chain" "true"
+public class Test {
+  void testOr(String s) {
+    if("foo"<caret>.equals(s) || "bar".equals(s) || "baz".equals(s)) {
+      System.out.println("foobarbaz");
+    }
+  }
+}

java/java-tests/testSrc/com/intellij/codeInsight/intention/ExtractSetFromComparisonChainActionTest.java

@@ -0,0 +1,28 @@
+/*
+ * Copyright 2000-2017 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.intellij.codeInsight.intention;
+
+import com.intellij.codeInsight.daemon.LightIntentionActionTestCase;
+
+public class ExtractSetFromComparisonChainActionTest extends LightIntentionActionTestCase {
+
+  public void test() throws Exception { doAllTests(); }
+
+  @Override
+  protected String getBasePath() {
+    return "/codeInsight/daemonCodeAnalyzer/quickFix/extractSetFromComparison";
+  }
+}

resources-en/src/intentionDescriptions/ExtractSetFromComparisonChainAction/after.java.template

@@ -0,0 +1,10 @@
+import java.util.*;
+
+public class Test {
+    <spot>private static final Set<String> FRUITS =
+        new HashSet<>(Arrays.asList("Apple", "Pear", "Banana"))</spot>;
+
+    boolean check(String fruit) {
+        return <spot>FRUITS.contains(fruit)</spot>;
+    }
+}

resources-en/src/intentionDescriptions/ExtractSetFromComparisonChainAction/before.java.template

@@ -0,0 +1,7 @@
+public class Test {
+    boolean check(String fruit) {
+        return <spot>"Apple".equals(fruit) ||
+                "Pear".equals(fruit) ||
+                "Banana".equals(fruit)</spot>;
+    }
+}

resources-en/src/intentionDescriptions/ExtractSetFromComparisonChainAction/description.html

@@ -0,0 +1,6 @@
+<html>
+<body>
+<p>Extracts a <code>Set&lt;String&gt;</code> from series of comparisons like <code>"a".equals(str) || "b".equals(str) || "c".equals(str)</code></p>
+<!-- tooltip end -->
+</body>
+</html>

resources/src/META-INF/IdeaPlugin.xml

@@ -1176,6 +1176,10 @@
       <className>com.intellij.codeInsight.intention.impl.SurroundAutoCloseableAction</className>
       <category>Java/Try Statements</category>
     </intentionAction>
+    <intentionAction>
+      <className>com.intellij.codeInsight.intention.impl.ExtractSetFromComparisonChainAction</className>
+      <category>Java/Declaration</category>
+    </intentionAction>
 
     <lang.parserDefinition language="JAVA" implementationClass="com.intellij.lang.java.JavaParserDefinition"/>