From 83c6c5222e2fe8b63c6183d3c16f31f191392c69 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Thu, 11 Jan 2018 13:46:35 +0700 Subject: [PATCH] IDEA-184240 Unnecessary array-to-collection wrapping should be detected --- .../afterAsListForEachEllipsisType.java | 11 +++ .../afterAsListForEachInline.java | 10 +++ .../afterAsListForEachSimple.java | 14 ++++ .../beforeAsListForEachEllipsisType.java | 11 +++ .../beforeAsListForEachInline.java | 10 +++ .../beforeAsListForEachSimple.java | 14 ++++ .../beforeAsListForEachUnused.java | 8 +++ .../beforeAsListForEachUsed.java | 15 ++++ ...edundantCollectionOperationInspection.java | 72 ++++++++++++++++++- 9 files changed, 164 insertions(+), 1 deletion(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/afterAsListForEachEllipsisType.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/afterAsListForEachInline.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/afterAsListForEachSimple.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/beforeAsListForEachEllipsisType.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/beforeAsListForEachInline.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/beforeAsListForEachSimple.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/beforeAsListForEachUnused.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/beforeAsListForEachUsed.java diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/afterAsListForEachEllipsisType.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/afterAsListForEachEllipsisType.java new file mode 100644 index 000000000000..9f36e1ecb7e4 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/afterAsListForEachEllipsisType.java @@ -0,0 +1,11 @@ +// "Unwrap" "true" +import java.util.Arrays; + +class Test { + void test(String[] data, Object[] data2, boolean b) { + Object[] list = b ? data : data2; + for(Object obj : list) { + System.out.println(obj); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/afterAsListForEachInline.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/afterAsListForEachInline.java new file mode 100644 index 000000000000..c108d662cd48 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/afterAsListForEachInline.java @@ -0,0 +1,10 @@ +// "Unwrap" "true" +import java.util.Arrays; + +class Test { + void test(String[] data) { + for(String s : data) { + System.out.println("hello "+s); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/afterAsListForEachSimple.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/afterAsListForEachSimple.java new file mode 100644 index 000000000000..c3c42beb1556 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/afterAsListForEachSimple.java @@ -0,0 +1,14 @@ +// "Unwrap" "true" +import java.util.Arrays; + +class Test { + void test(String[] data) { + String[] list = data; + for(String s : list) { + System.out.println("hello "+s); + } + for(String s : list) { + System.out.println("goodbye "+s); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/beforeAsListForEachEllipsisType.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/beforeAsListForEachEllipsisType.java new file mode 100644 index 000000000000..2765b1e04192 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/beforeAsListForEachEllipsisType.java @@ -0,0 +1,11 @@ +// "Unwrap" "true" +import java.util.Arrays; + +class Test { + void test(String[] data, Object[] data2, boolean b) { + List list = Arrays.asList(b ? data : data2); + for(Object obj : list) { + System.out.println(obj); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/beforeAsListForEachInline.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/beforeAsListForEachInline.java new file mode 100644 index 000000000000..7d8bcc21fb54 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/beforeAsListForEachInline.java @@ -0,0 +1,10 @@ +// "Unwrap" "true" +import java.util.Arrays; + +class Test { + void test(String[] data) { + for(String s : Arrays.asList(data)) { + System.out.println("hello "+s); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/beforeAsListForEachSimple.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/beforeAsListForEachSimple.java new file mode 100644 index 000000000000..3de68a9b9557 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/beforeAsListForEachSimple.java @@ -0,0 +1,14 @@ +// "Unwrap" "true" +import java.util.Arrays; + +class Test { + void test(String[] data) { + List list = Arrays.asList(data); + for(String s : list) { + System.out.println("hello "+s); + } + for(String s : list) { + System.out.println("goodbye "+s); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/beforeAsListForEachUnused.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/beforeAsListForEachUnused.java new file mode 100644 index 000000000000..3fbe78cbc9e5 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/beforeAsListForEachUnused.java @@ -0,0 +1,8 @@ +// "Unwrap" "false" +import java.util.Arrays; + +class Test { + void test(String[] data) { + List list = Arrays.asList(data); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/beforeAsListForEachUsed.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/beforeAsListForEachUsed.java new file mode 100644 index 000000000000..666a13df21e8 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/beforeAsListForEachUsed.java @@ -0,0 +1,15 @@ +// "Unwrap" "false" +import java.util.Arrays; + +class Test { + void test(String[] data) { + List list = Arrays.asList(data); + for(String s : list) { + System.out.println("hello "+s); + } + for(String s : list) { + System.out.println("goodbye "+s); + } + System.out.println(list); + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/redundancy/RedundantCollectionOperationInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/redundancy/RedundantCollectionOperationInspection.java index ffc33fd011c4..9f0bce707d87 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/redundancy/RedundantCollectionOperationInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/redundancy/RedundantCollectionOperationInspection.java @@ -52,7 +52,8 @@ public class RedundantCollectionOperationInspection extends AbstractBaseJavaLoca .register(CONTAINS_ALL, ContainsAllSingletonHandler::handler) .register(CONTAINS, SingletonContainsHandler::handler) .register(CONTAINS, ContainsBeforeAddRemoveHandler::handler) - .register(REMOVE_BY_INDEX, RedundantIndexOfHandler::handler); + .register(REMOVE_BY_INDEX, RedundantIndexOfHandler::handler) + .register(AS_LIST, RedundantAsListForIterationHandler::handler); @NotNull @Override @@ -377,6 +378,75 @@ public class RedundantCollectionOperationInspection extends AbstractBaseJavaLoca } } + private static class RedundantAsListForIterationHandler implements RedundantCollectionOperationHandler { + @Override + public String getProblemName() { + return "Unnecessary 'Arrays.asList' call"; + } + + @NotNull + @Override + public String getFixName() { + return "Unwrap"; + } + + @Override + public void performFix(@NotNull Project project, @NotNull PsiMethodCallExpression call) { + PsiExpression[] args = call.getArgumentList().getExpressions(); + if (args.length != 1) return; + PsiElement parent = PsiUtil.skipParenthesizedExprUp(call.getParent()); + CommentTracker ct = new CommentTracker(); + if (parent instanceof PsiLocalVariable) { + PsiTypeElement typeElement = ((PsiLocalVariable)parent).getTypeElement(); + if (!typeElement.isInferredType()) { + PsiType type = args[0].getType(); + if (type == null) return; + if(type instanceof PsiEllipsisType) { + type = ((PsiEllipsisType)type).toArrayType(); + } + if (!typeElement.isInferredType()) { + typeElement.replace(JavaPsiFacade.getElementFactory(project).createTypeElement(type)); + } + } + } + ct.replaceAndRestoreComments(call, ct.markUnchanged(args[0])); + } + + static RedundantAsListForIterationHandler handler(PsiMethodCallExpression call) { + if (MethodCallUtils.isVarArgCall(call)) return null; + PsiExpression arg = call.getArgumentList().getExpressions()[0]; + if (!(arg.getType() instanceof PsiArrayType)) return null; + if (isAllowedContext(call)) { + return new RedundantAsListForIterationHandler(); + } + PsiElement parent = PsiUtil.skipParenthesizedExprUp(call.getParent()); + if (parent instanceof PsiLocalVariable) { + PsiLocalVariable localVariable = (PsiLocalVariable)parent; + if (!(localVariable.getParent() instanceof PsiDeclarationStatement) || + ((PsiDeclarationStatement)localVariable.getParent()).getDeclaredElements().length != 1) { + return null; + } + PsiCodeBlock block = PsiTreeUtil.getParentOfType(localVariable, PsiCodeBlock.class); + if (block != null && VariableAccessUtils.variableIsUsed(localVariable, block) && + PsiTreeUtil.processElements(block, element -> { + if (!(element instanceof PsiReferenceExpression)) return true; + PsiReferenceExpression ref = (PsiReferenceExpression)element; + if (!(ref.isReferenceTo(localVariable))) return true; + return isAllowedContext(ref); + })) { + return new RedundantAsListForIterationHandler(); + } + } + return null; + } + + private static boolean isAllowedContext(PsiExpression expression) { + PsiElement parent = PsiUtil.skipParenthesizedExprUp(expression.getParent()); + return parent instanceof PsiForeachStatement && + PsiTreeUtil.isAncestor(((PsiForeachStatement)parent).getIteratedValue(), expression, false); + } + } + private static class RedundantCollectionOperationFix implements LocalQuickFix { private final RedundantCollectionOperationHandler myHandler;