diff --git a/java/java-impl/src/com/intellij/codeInspection/OptionalAssignedToNullInspection.java b/java/java-impl/src/com/intellij/codeInspection/OptionalAssignedToNullInspection.java index b3cab8b18761..78e5136ba3a6 100644 --- a/java/java-impl/src/com/intellij/codeInspection/OptionalAssignedToNullInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/OptionalAssignedToNullInspection.java @@ -1,21 +1,32 @@ // Copyright 2000-2017 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package com.intellij.codeInspection; +import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel; 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 com.intellij.util.ObjectUtils; +import com.siyeh.ig.psiutils.*; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import javax.swing.*; /** * @author Tagir Valeev */ public class OptionalAssignedToNullInspection extends AbstractBaseJavaLocalInspectionTool { + public boolean WARN_ON_COMPARISON = true; + + @Nullable + @Override + public JComponent createOptionsPanel() { + return new SingleCheckboxOptionsPanel("Report comparison of Optional with null", this, "WARN_ON_COMPARISON"); + } + @NotNull @Override public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) { @@ -67,6 +78,18 @@ public class OptionalAssignedToNullInspection extends AbstractBaseJavaLocalInspe InspectionsBundle.message("inspection.null.value.for.optional.context.declaration")); } + @Override + public void visitBinaryExpression(PsiBinaryExpression binOp) { + if (!WARN_ON_COMPARISON) return; + PsiExpression value = ExpressionUtils.getValueComparedWithNull(binOp); + if (value != null && TypeUtils.isOptional(value.getType())) { + holder.registerProblem(binOp, "Optional value is compared with null", + new ReplaceWithIsPresentFix(), + new SetInspectionOptionFix(OptionalAssignedToNullInspection.this, "WARN_ON_COMPARISON", + "Do not warn when comparing Optional with null", false)); + } + } + private void checkNulls(PsiType type, PsiExpression expression, String declaration) { if (expression != null && TypeUtils.isOptional(type)) { ExpressionUtils.nonStructuralChildren(expression).filter(ExpressionUtils::isNullLiteral) @@ -115,8 +138,28 @@ public class OptionalAssignedToNullInspection extends AbstractBaseJavaLocalInspe PsiElement element = descriptor.getStartElement(); if (!(element instanceof PsiExpression)) return; String emptyCall = myTypeName + "." + myTypeParameter + myMethodName + "()"; - PsiElement result = element.replace(JavaPsiFacade.getElementFactory(project).createExpressionFromText(emptyCall, element)); + PsiElement result = new CommentTracker().replaceAndRestoreComments(element, emptyCall); PsiDiamondTypeUtil.removeRedundantTypeArguments(result); } } + + private static class ReplaceWithIsPresentFix implements LocalQuickFix { + @Nls + @NotNull + @Override + public String getFamilyName() { + return "Replace with 'isPresent()' call"; + } + + @Override + public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { + PsiBinaryExpression binOp = ObjectUtils.tryCast(descriptor.getStartElement(), PsiBinaryExpression.class); + if (binOp == null) return; + PsiExpression value = ExpressionUtils.getValueComparedWithNull(binOp); + if (value == null || !TypeUtils.isOptional(value.getType())) return; + CommentTracker ct = new CommentTracker(); + String negation = binOp.getOperationTokenType().equals(JavaTokenType.NE) ? "" : "!"; + ct.replaceAndRestoreComments(binOp, negation + ct.text(value, ParenthesesUtils.METHOD_CALL_PRECEDENCE) + ".isPresent()"); + } + } } diff --git a/java/java-impl/src/inspectionDescriptions/OptionalAssignedToNull.html b/java/java-impl/src/inspectionDescriptions/OptionalAssignedToNull.html index cb8f758780ae..c18f018a036e 100644 --- a/java/java-impl/src/inspectionDescriptions/OptionalAssignedToNull.html +++ b/java/java-impl/src/inspectionDescriptions/OptionalAssignedToNull.html @@ -1,8 +1,12 @@ -

This inspection warns when null is assigned to Optional variable or returned from method returning +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.

+ an empty value. +

+ Use checkbox below to report also comparisons like optional == null. While in rare cases (e.g. lazily initialized optional field) + this might be correct, usually optional variable is never null, and probably optional.isPresent() was intended. +

This inspection only reports if the project or module is configured to use a language level of 8 or higher.

New in 2017.2 diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalNull/afterOptionalComparison.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalNull/afterOptionalComparison.java new file mode 100644 index 000000000000..50b65275a995 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalNull/afterOptionalComparison.java @@ -0,0 +1,20 @@ +// "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 { + void test(Optional opt) { + if(!opt.isPresent()) { + System.out.println("null!"); + } + } + + void test2 (Optional opt) { + /*comment*/ + if(opt.isPresent()) { + System.out.println("null!"); + } + } + +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalNull/afterOptionalNull.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalNull/afterOptionalNull.java index 443b59a53b9d..149bfebb343b 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalNull/afterOptionalNull.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalNull/afterOptionalNull.java @@ -16,7 +16,7 @@ public class Test { } OptionalInt opt() { - return OptionalInt.empty(); + return (/*comment*/OptionalInt.empty()); } void m(Optional opt) {} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalNull/beforeOptionalComparison.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalNull/beforeOptionalComparison.java new file mode 100644 index 000000000000..2949a5ae68f7 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalNull/beforeOptionalComparison.java @@ -0,0 +1,19 @@ +// "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 { + void test(Optional opt) { + if(opt == null) { + System.out.println("null!"); + } + } + + void test2 (Optional opt) { + if(opt !=/*comment*/ null) { + System.out.println("null!"); + } + } + +} \ 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 index e179f2382f69..17412712034c 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalNull/beforeOptionalNull.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalNull/beforeOptionalNull.java @@ -16,7 +16,7 @@ public class Test { } OptionalInt opt() { - return null; + return (/*comment*/null); } void m(Optional opt) {}