From d9f36ee97a28a190f5fde9061f90b315dcf14b29 Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Mon, 19 Jun 2017 19:30:01 +0200 Subject: [PATCH] IPP: don't replace with List/ImmutableList.of() when arguments are possibly null --- .../ReplaceWithArraysAsListIntention.java | 46 +++++++++++++++---- ...laceWithArraysAsListIntentionJdk9Test.java | 34 ++++++++++++++ 2 files changed, 70 insertions(+), 10 deletions(-) diff --git a/plugins/IntentionPowerPak/src/com/siyeh/ipp/collections/ReplaceWithArraysAsListIntention.java b/plugins/IntentionPowerPak/src/com/siyeh/ipp/collections/ReplaceWithArraysAsListIntention.java index 58b31d131799..e517ac7aceb5 100644 --- a/plugins/IntentionPowerPak/src/com/siyeh/ipp/collections/ReplaceWithArraysAsListIntention.java +++ b/plugins/IntentionPowerPak/src/com/siyeh/ipp/collections/ReplaceWithArraysAsListIntention.java @@ -15,16 +15,21 @@ */ package com.siyeh.ipp.collections; +import com.intellij.codeInsight.NullableNotNullManager; import com.intellij.codeInsight.intention.HighPriorityAction; import com.intellij.psi.*; import com.intellij.psi.util.PsiUtil; import com.siyeh.IntentionPowerPackBundle; import com.siyeh.ig.PsiReplacementUtil; import com.siyeh.ig.psiutils.ClassUtils; +import com.siyeh.ig.psiutils.ExpressionUtils; +import com.siyeh.ig.psiutils.ParenthesesUtils; import com.siyeh.ipp.base.Intention; import com.siyeh.ipp.base.PsiElementPredicate; import org.jetbrains.annotations.NotNull; +import java.util.Arrays; + /** * @author Bas Leijdekkers */ @@ -71,20 +76,21 @@ public class ReplaceWithArraysAsListIntention extends Intention implements HighP } private static String getReplacementMethodText(String methodName, PsiMethodCallExpression context) { - if (methodName.equals("emptyList") && context.getArgumentList().getExpressions().length == 1 && - !PsiUtil.isLanguageLevel9OrHigher(context) && ClassUtils.findClass("com.google.common.collect.ImmutableList", context) == null) { + final PsiExpression[] arguments = context.getArgumentList().getExpressions(); + if (methodName.equals("emptyList") && arguments.length == 1 && + !PsiUtil.isLanguageLevel9OrHigher(context) && ClassUtils.findClass("com.google.common.collect.ImmutableList", context) == null) { return "java.util.Collections.singletonList"; } if (methodName.equals("emptyList") || methodName.equals("singletonList")) { - if (PsiUtil.isLanguageLevel9OrHigher(context)) { - return "java.util.List.of"; - } - else if (ClassUtils.findClass("com.google.common.collect.ImmutableList", context) != null) { - return "com.google.common.collect.ImmutableList.of"; - } - else { - return "java.util.Arrays.asList"; + if (Arrays.stream(arguments).noneMatch(e -> isPossiblyNull(e))) { + if (PsiUtil.isLanguageLevel9OrHigher(context)) { + return "java.util.List.of"; + } + else if (ClassUtils.findClass("com.google.common.collect.ImmutableList", context) != null) { + return "com.google.common.collect.ImmutableList.of"; + } } + return "java.util.Arrays.asList"; } if (methodName.equals("emptySet") || methodName.equals("singleton")) { if (PsiUtil.isLanguageLevel9OrHigher(context)) { @@ -104,4 +110,24 @@ public class ReplaceWithArraysAsListIntention extends Intention implements HighP } return null; } + + private static boolean isPossiblyNull(PsiExpression expression) { + expression = ParenthesesUtils.stripParentheses(expression); + if (expression instanceof PsiReferenceExpression) { + final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)expression; + final PsiElement target = referenceExpression.resolve(); + if (target instanceof PsiModifierListOwner) { + final PsiModifierListOwner modifierListOwner = (PsiModifierListOwner)target; + return NullableNotNullManager.getInstance(expression.getProject()).isNullable(modifierListOwner, false); + } + } + else if (ExpressionUtils.isNullLiteral(expression)) { + return true; + } + else if (expression instanceof PsiConditionalExpression) { + final PsiConditionalExpression conditionalExpression = (PsiConditionalExpression)expression; + return isPossiblyNull(conditionalExpression.getThenExpression()) || isPossiblyNull(conditionalExpression.getElseExpression()); + } + return false; + } } diff --git a/plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/collections/ReplaceWithArraysAsListIntentionJdk9Test.java b/plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/collections/ReplaceWithArraysAsListIntentionJdk9Test.java index 66a0fe574c92..c587c644dac4 100644 --- a/plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/collections/ReplaceWithArraysAsListIntentionJdk9Test.java +++ b/plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/collections/ReplaceWithArraysAsListIntentionJdk9Test.java @@ -40,6 +40,40 @@ public class ReplaceWithArraysAsListIntentionJdk9Test extends IPPTestCase { ); } + public void testNullArgument() { + doTest( + "import java.util.*;" + + "class X {" + + " List f() {" + + " return Collections.emptyList(null/*_Replace with 'java.util.Arrays.asList()'*/, null);" + + " }" + + "}", + + "import java.util.*;" + + "class X {" + + " List f() {" + + " return Arrays.asList(null, null);" + + " }" + + "}"); + } + + public void testNullableArgument() { + doTest( + "import java.util.*;" + + "class X {" + + " List f(@org.jetbrains.annotations.Nullable String a) {" + + " return Collections.emptyList(a/*_Replace with 'java.util.Arrays.asList()'*/);" + + " }" + + "}", + + "import java.util.*;" + + "class X {" + + " List f(@org.jetbrains.annotations.Nullable String a) {" + + " return Arrays.asList(a);" + + " }" + + "}"); + } + public void testReplaceSingletonList() { doTest( "import java.util.*;" +