From 1b3489639b1547968b3f956125960d451771a30b Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Wed, 1 Mar 2017 16:39:53 +0700 Subject: [PATCH] IDEA-168633 Warn if variable of type java.util.Optional is initialized with null --- .../OptionalAssignedToNullInspection.java | 136 ++++++++++++++++++ .../optionalNull/afterOptionalNull.java | 27 ++++ .../optionalNull/beforeOptionalNull.java | 27 ++++ .../OptionalAssignedToNullInspectionTest.java | 38 +++++ .../src/messages/InspectionsBundle.properties | 9 ++ .../siyeh/ig/psiutils/ExpressionUtils.java | 15 +- .../OptionalAssignedToNull.html | 9 ++ resources/src/META-INF/IdeaPlugin.xml | 5 + 8 files changed, 264 insertions(+), 2 deletions(-) create mode 100644 java/java-impl/src/com/intellij/codeInspection/OptionalAssignedToNullInspection.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalNull/afterOptionalNull.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalNull/beforeOptionalNull.java create mode 100644 java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/OptionalAssignedToNullInspectionTest.java create mode 100644 resources-en/src/inspectionDescriptions/OptionalAssignedToNull.html diff --git a/java/java-impl/src/com/intellij/codeInspection/OptionalAssignedToNullInspection.java b/java/java-impl/src/com/intellij/codeInspection/OptionalAssignedToNullInspection.java new file mode 100644 index 000000000000..0fd8165dec4c --- /dev/null +++ b/java/java-impl/src/com/intellij/codeInspection/OptionalAssignedToNullInspection.java @@ -0,0 +1,136 @@ +/* + * 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.codeInspection; + +import com.intellij.openapi.project.Project; +import com.intellij.openapi.util.text.StringUtil; +import com.intellij.psi.*; +import com.intellij.psi.impl.PsiDiamondTypeUtil; +import com.intellij.psi.util.PsiTypesUtil; +import com.siyeh.ig.psiutils.ExpressionUtils; +import com.siyeh.ig.psiutils.MethodCallUtils; +import com.siyeh.ig.psiutils.TypeUtils; +import org.jetbrains.annotations.Nls; +import org.jetbrains.annotations.NotNull; + +/** + * @author Tagir Valeev + */ +public class OptionalAssignedToNullInspection extends BaseJavaBatchLocalInspectionTool { + @NotNull + @Override + public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) { + return new JavaElementVisitor() { + @Override + public void visitAssignmentExpression(PsiAssignmentExpression expression) { + checkNulls(expression.getType(), expression.getRExpression(), + InspectionsBundle.message("inspection.null.value.for.optional.context.assignment")); + } + + @Override + public void visitMethodCallExpression(PsiMethodCallExpression call) { + PsiExpression[] args = call.getArgumentList().getExpressions(); + if (args.length == 0) return; + PsiMethod method = call.resolveMethod(); + if (method == null) return; + PsiParameter[] parameters = method.getParameterList().getParameters(); + if (parameters.length > args.length) return; + boolean varArgCall = MethodCallUtils.isVarArgCall(call); + if (!varArgCall && parameters.length < args.length) return; + for (int i = 0; i < args.length; i++) { + PsiParameter parameter = parameters[Math.min(parameters.length - 1, i)]; + PsiType type = parameter.getType(); + if (varArgCall && i >= parameters.length - 1 && type instanceof PsiEllipsisType) { + type = ((PsiEllipsisType)type).getComponentType(); + } + checkNulls(type, args[i], InspectionsBundle.message("inspection.null.value.for.optional.context.parameter")); + } + } + + @Override + public void visitLambdaExpression(PsiLambdaExpression lambda) { + PsiElement body = lambda.getBody(); + if (body instanceof PsiExpression) { + checkNulls(LambdaUtil.getFunctionalInterfaceReturnType(lambda), (PsiExpression)body, + InspectionsBundle.message("inspection.null.value.for.optional.context.lambda")); + } + } + + @Override + public void visitReturnStatement(PsiReturnStatement statement) { + checkNulls(PsiTypesUtil.getMethodReturnType(statement), statement.getReturnValue(), + InspectionsBundle.message("inspection.null.value.for.optional.context.return")); + } + + @Override + public void visitVariable(PsiVariable variable) { + checkNulls(variable.getType(), variable.getInitializer(), + InspectionsBundle.message("inspection.null.value.for.optional.context.declaration")); + } + + private void checkNulls(PsiType type, PsiExpression expression, String declaration) { + if (expression != null && TypeUtils.isOptional(type)) { + ExpressionUtils.possibleValues(expression).filter(ExpressionUtils::isNullLiteral) + .forEach(nullLiteral -> register(nullLiteral, (PsiClassType)type, declaration)); + } + } + + private void register(PsiExpression expression, PsiClassType type, String contextName) { + holder.registerProblem(expression, + InspectionsBundle.message("inspection.null.value.for.optional.message", contextName), + new ReplaceWithEmptyOptionalFix(type)); + } + }; + } + + private static class ReplaceWithEmptyOptionalFix implements LocalQuickFix { + private final String myTypeName; + private final String myTypeParameter; + private final String myMethodName; + + public ReplaceWithEmptyOptionalFix(PsiClassType type) { + myTypeName = type.rawType().getCanonicalText(); + PsiType[] parameters = type.getParameters(); + myTypeParameter = + parameters.length == 1 ? "<" + GenericsUtil.getVariableTypeByExpressionType(parameters[0]).getCanonicalText() + ">" : ""; + myMethodName = myTypeName.equals("com.google.common.base.Optional") ? "absent" : "empty"; + } + + @Nls + @NotNull + @Override + public String getName() { + return InspectionsBundle.message("inspection.null.value.for.optional.fix.name", + StringUtil.getShortName(myTypeName) + "." + myMethodName + "()"); + } + + @Nls + @NotNull + @Override + public String getFamilyName() { + return InspectionsBundle.message("inspection.null.value.for.optional.fix.family.name"); + } + + @Override + public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { + PsiElement element = descriptor.getStartElement(); + if (!(element instanceof PsiExpression)) return; + String emptyCall = myTypeName + "." + myTypeParameter + myMethodName + "()"; + PsiElement result = element.replace(JavaPsiFacade.getElementFactory(project).createExpressionFromText(emptyCall, element)); + PsiDiamondTypeUtil.removeRedundantTypeArguments(result); + } + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalNull/afterOptionalNull.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalNull/afterOptionalNull.java new file mode 100644 index 000000000000..b7857fb53adf --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalNull/afterOptionalNull.java @@ -0,0 +1,27 @@ +// "Fix all 'Null value for Optional type' problems in file" "true" +import java.util.List; +import java.util.Optional; +import java.util.OptionalInt; + +public class Test { + Optional field = Optional.empty(); + Optional field2 = Optional.empty(); + + public void test(List list) { + Optional s = list.size() > 0 ? Optional.of(list.get(0)) : Optional.empty(); + varArg(1, Optional.of(1), Optional.empty()); + m((Optional) Optional.empty()); + Optional.of("xyz").flatMap(x -> Optional.empty()).ifPresent(System.out::println); + } + + OptionalInt opt() { + return OptionalInt.empty(); + } + + void m(Optional opt) {} + void m(String s) {} + + void varArg(int x, Optional... opts) { + + }; +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalNull/beforeOptionalNull.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalNull/beforeOptionalNull.java new file mode 100644 index 000000000000..8ad0d932645e --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalNull/beforeOptionalNull.java @@ -0,0 +1,27 @@ +// "Fix all 'Null value for Optional type' problems in file" "true" +import java.util.List; +import java.util.Optional; +import java.util.OptionalInt; + +public class Test { + Optional field = null; + Optional field2 = null; + + public void test(List list) { + Optional s = list.size() > 0 ? Optional.of(list.get(0)) : null; + varArg(1, Optional.of(1), null); + m((Optional) null); + Optional.of("xyz").flatMap(x -> null).ifPresent(System.out::println); + } + + OptionalInt opt() { + return null; + } + + void m(Optional opt) {} + void m(String s) {} + + void varArg(int x, Optional... opts) { + + }; +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/OptionalAssignedToNullInspectionTest.java b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/OptionalAssignedToNullInspectionTest.java new file mode 100644 index 000000000000..2037692faa5a --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/OptionalAssignedToNullInspectionTest.java @@ -0,0 +1,38 @@ +/* + * 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.daemon.quickFix; + +import com.intellij.codeInspection.LocalInspectionTool; +import com.intellij.codeInspection.OptionalAssignedToNullInspection; +import org.jetbrains.annotations.NotNull; + + +public class OptionalAssignedToNullInspectionTest extends LightQuickFixParameterizedTestCase { + @NotNull + @Override + protected LocalInspectionTool[] configureLocalInspectionTools() { + return new LocalInspectionTool[]{ + new OptionalAssignedToNullInspection() + }; + } + + public void test() throws Exception { doAllTests(); } + + @Override + protected String getBasePath() { + return "/codeInsight/daemonCodeAnalyzer/quickFix/optionalNull"; + } +} \ No newline at end of file diff --git a/platform/platform-resources-en/src/messages/InspectionsBundle.properties b/platform/platform-resources-en/src/messages/InspectionsBundle.properties index 95f712fe4668..2ca495248c1e 100644 --- a/platform/platform-resources-en/src/messages/InspectionsBundle.properties +++ b/platform/platform-resources-en/src/messages/InspectionsBundle.properties @@ -787,3 +787,12 @@ inspection.collection.factories.message=Can be replaced with ''{0}.of'' call inspection.collection.factories.option.ignore.non.constant=Do not warn when content is non-constant inspection.collection.factories.fix.family.name=Replace with collection factory call inspection.collection.factories.fix.name=Replace with ''{0}.of'' call + +inspection.null.value.for.optional.message=Null is used for ''Optional'' type in {0} +inspection.null.value.for.optional.fix.family.name=Replace with empty Optional method +inspection.null.value.for.optional.fix.name=Replace with ''{0}'' +inspection.null.value.for.optional.context.assignment=assignment +inspection.null.value.for.optional.context.parameter=parameter +inspection.null.value.for.optional.context.lambda=lambda expression +inspection.null.value.for.optional.context.return=return statement +inspection.null.value.for.optional.context.declaration=declaration diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java index 0b04c00eb39f..6f22c39c8efc 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java @@ -236,7 +236,7 @@ public class ExpressionUtils { * Also the expression value is guaranteed to be equal to one of returned sub-expressions. * *

- * E.g. for {@code ((a) ? b : (c))} the stream will contain b and c. + * E.g. for {@code ((a) ? (Foo)b : (c))} the stream will contain b and c. *

* * @param expression expression to create a stream from @@ -253,7 +253,18 @@ public class ExpressionUtils { } return null; }).remove(e -> e instanceof PsiConditionalExpression || - e instanceof PsiParenthesizedExpression); + e instanceof PsiParenthesizedExpression) + .map(e -> { + if(e instanceof PsiTypeCastExpression) { + PsiExpression operand = ((PsiTypeCastExpression)e).getOperand(); + if(operand != null && !(e.getType() instanceof PsiPrimitiveType) && + (!(operand.getType() instanceof PsiPrimitiveType) || PsiType.NULL.equals(operand.getType()))) { + // Ignore to-primitive/from-primitive casts as they may actually change the value + return PsiUtil.skipParenthesizedExprDown(operand); + } + } + return e; + }); } public static boolean isZero(@Nullable PsiExpression expression) { diff --git a/resources-en/src/inspectionDescriptions/OptionalAssignedToNull.html b/resources-en/src/inspectionDescriptions/OptionalAssignedToNull.html new file mode 100644 index 000000000000..f498f2caad5d --- /dev/null +++ b/resources-en/src/inspectionDescriptions/OptionalAssignedToNull.html @@ -0,0 +1,9 @@ + + +

This inspection warns when null is assigned to Optional variable or returned from method returning + Optional. It's recommended to use Optional.empty() (or Optional.absent() for Guava) to denote + an empty value.

+ +

New in 2017.2 + + \ No newline at end of file diff --git a/resources/src/META-INF/IdeaPlugin.xml b/resources/src/META-INF/IdeaPlugin.xml index 6625b9dedb99..4f4a6d2f3e0b 100644 --- a/resources/src/META-INF/IdeaPlugin.xml +++ b/resources/src/META-INF/IdeaPlugin.xml @@ -907,6 +907,11 @@ groupKey="group.names.code.style.issues" enabledByDefault="true" level="WARNING" implementationClass="com.intellij.codeInspection.OptionalIsPresentInspection" displayName="Replace Optional.isPresent() checks with functional-style expressions"/> +