[java-inspections] Fix boxing in ternary branches when NPE is possible (IDEA-267511)

GitOrigin-RevId: 95bd6ad84ace485bca5699192c4dfc161397736b

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java

@@ -242,7 +242,7 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec
     return Collections.emptyList();
   }
 
-  protected @NotNull List<LocalQuickFix> createUnboxingNullableFixes(@NotNull PsiExpression qualifier, PsiExpression expression, boolean onTheFly) {
+  protected @NotNull List<LocalQuickFix> createUnboxingNullableFixes(@NotNull PsiExpression qualifier, PsiElement anchor, boolean onTheFly) {
     return Collections.emptyList();
   }
 
@@ -656,7 +656,7 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec
           anchor = Objects.requireNonNull(((PsiTypeCastExpression)anchor).getOperand());
         }
         if (anchor != null) {
-          LocalQuickFix[] fixes = createUnboxingNullableFixes(anchor, expression, reporter.isOnTheFly()).toArray(LocalQuickFix.EMPTY_ARRAY);
+          LocalQuickFix[] fixes = createUnboxingNullableFixes(anchor, element, reporter.isOnTheFly()).toArray(LocalQuickFix.EMPTY_ARRAY);
           reporter.registerProblem(anchor, problem.getMessage(expressions), fixes);
         }
       });

java/java-impl-inspections/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java

@@ -7,6 +7,7 @@ import com.intellij.codeInsight.daemon.impl.quickfix.DeleteSideEffectsAwareFix;
 import com.intellij.codeInsight.daemon.impl.quickfix.SimplifyBooleanExpressionFix;
 import com.intellij.codeInsight.daemon.impl.quickfix.UnwrapSwitchLabelFix;
 import com.intellij.codeInspection.*;
+import com.intellij.codeInspection.dataFlow.fix.BoxPrimitiveInTernaryFix;
 import com.intellij.codeInspection.dataFlow.fix.FindDfaProblemCauseFix;
 import com.intellij.codeInspection.dataFlow.fix.ReplaceWithBooleanEqualsFix;
 import com.intellij.codeInspection.dataFlow.fix.SurroundWithRequireNonNullFix;
@@ -23,6 +24,7 @@ import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.psi.util.PsiUtil;
 import com.intellij.psi.util.TypeConversionUtil;
 import com.intellij.util.IncorrectOperationException;
+import com.intellij.util.ObjectUtils;
 import com.intellij.util.SmartList;
 import com.intellij.util.containers.ContainerUtil;
 import com.intellij.util.ui.JBInsets;
@@ -221,11 +223,12 @@ public class DataFlowInspection extends DataFlowInspectionBase {
   }
 
   @Override
-  protected @NotNull List<LocalQuickFix> createUnboxingNullableFixes(@NotNull PsiExpression qualifier, PsiExpression expression, boolean onTheFly) {
+  protected @NotNull List<LocalQuickFix> createUnboxingNullableFixes(@NotNull PsiExpression qualifier, PsiElement anchor, boolean onTheFly) {
     List<LocalQuickFix> result = new SmartList<>();
     if (TypeConversionUtil.isBooleanType(qualifier.getType())) {
       result.add(new ReplaceWithBooleanEqualsFix(qualifier));
     }
+    ContainerUtil.addIfNotNull(result, BoxPrimitiveInTernaryFix.makeFix(ObjectUtils.tryCast(anchor, PsiExpression.class)));
     addCreateNullBranchFix(qualifier, result);
     return result;
   }

java/java-impl/src/com/intellij/codeInspection/dataFlow/fix/BoxPrimitiveInTernaryFix.java

@@ -0,0 +1,88 @@
+// Copyright 2000-2022 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
+package com.intellij.codeInspection.dataFlow.fix;
+
+import com.intellij.codeInsight.intention.FileModifier;
+import com.intellij.codeInspection.LocalQuickFix;
+import com.intellij.codeInspection.ProblemDescriptor;
+import com.intellij.java.JavaBundle;
+import com.intellij.openapi.project.Project;
+import com.intellij.psi.*;
+import com.intellij.psi.util.PsiTreeUtil;
+import com.intellij.psi.util.PsiUtil;
+import com.siyeh.ig.jdk.AutoBoxingInspection;
+import com.siyeh.ig.psiutils.ExpectedTypeUtils;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+import java.util.Objects;
+
+
+public class BoxPrimitiveInTernaryFix implements LocalQuickFix {
+  private final SmartPsiElementPointer<PsiExpression> myPointer;
+
+  private BoxPrimitiveInTernaryFix(PsiExpression expression) {
+    myPointer = SmartPointerManager.createPointer(expression);
+  }
+
+  @Override
+  public @Nullable FileModifier getFileModifierForPreview(@NotNull PsiFile target) {
+    PsiExpression expression = myPointer.getElement();
+    return expression == null ? null : new BoxPrimitiveInTernaryFix(PsiTreeUtil.findSameElementInCopy(expression, target));
+  }
+
+  public static @Nullable BoxPrimitiveInTernaryFix makeFix(@Nullable PsiExpression npeExpression) {
+    if (npeExpression == null) return null;
+    PsiConditionalExpression parentConditional = getParentConditional(npeExpression);
+    if (parentConditional == null) return null;
+    PsiType type = Objects.requireNonNull(parentConditional.getType());
+    PsiType expectedType = ExpectedTypeUtils.findExpectedType(parentConditional, false);
+    if (expectedType instanceof PsiClassType && expectedType.isAssignableFrom(type)) {
+      return new BoxPrimitiveInTernaryFix(npeExpression);
+    }
+    return null;
+  }
+
+  private static PsiConditionalExpression getParentConditional(@NotNull PsiExpression expression) {
+    PsiElement parent = PsiUtil.skipParenthesizedExprUp(expression.getParent());
+    if (!(parent instanceof PsiConditionalExpression)) return null;
+    PsiConditionalExpression conditional = (PsiConditionalExpression)parent;
+    PsiType type = conditional.getType();
+    if (type == null) return null;
+    PsiExpression thenExpression = conditional.getThenExpression();
+    if (thenExpression == null) return null;
+    PsiType thenType = thenExpression.getType();
+    if (thenType == null) return null;
+    PsiExpression elseExpression = conditional.getElseExpression();
+    if (elseExpression == null) return null;
+    PsiType elseType = elseExpression.getType();
+    if (elseType == null) return null;
+    if (elseType instanceof PsiPrimitiveType && thenType instanceof PsiClassType) {
+      if (PsiTreeUtil.isAncestor(thenExpression, expression, false) && thenType.isAssignableFrom(elseType)) return conditional;
+    }
+    else if (elseType instanceof PsiClassType && thenType instanceof PsiPrimitiveType) {
+      if (PsiTreeUtil.isAncestor(elseExpression, expression, false) && elseType.isAssignableFrom(thenType)) return conditional;
+    }
+    return null;
+  }
+
+  @Override
+  public @NotNull String getFamilyName() {
+    return JavaBundle.message("intention.family.name.box.primitive.in.conditional.branch");
+  }
+
+  @Override
+  public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
+    PsiExpression expression = myPointer.getElement();
+    if (expression == null) return;
+    PsiConditionalExpression conditional = getParentConditional(expression);
+    if (conditional == null) return;
+    PsiExpression thenExpression = conditional.getThenExpression();
+    if (thenExpression != null) {
+      AutoBoxingInspection.replaceWithBoxing(thenExpression);
+    }
+    PsiExpression elseExpression = conditional.getElseExpression();
+    if (elseExpression != null) {
+      AutoBoxingInspection.replaceWithBoxing(elseExpression);
+    }
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/boxPrimitiveInTernary/afterNullableBooleanMethodInBranch.java

@@ -0,0 +1,10 @@
+// "Box primitive value in conditional branch" "true"
+class Test {
+  public static void main(String strictArgument) {
+    Boolean flag = Math.random() > 0.5 ? getBoolean() : Boolean.FALSE;
+  }
+
+  static Boolean getBoolean() {
+    return Math.random() > 0.5 ? true : null;
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/boxPrimitiveInTernary/afterTwoTernariesBoolean.java

@@ -0,0 +1,6 @@
+// "Box primitive value in conditional branch" "true"
+class Test {
+  public static void main(String strictArgument) {
+    Boolean strict = strictArgument == null ? Boolean.FALSE : strictArgument.isEmpty() ? true : null;
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/boxPrimitiveInTernary/afterTwoTernariesInt.java

@@ -0,0 +1,7 @@
+// "Box primitive value in conditional branch" "true"
+class Test {
+  public static void main(String strictArgument) {
+    Integer test = Math.random() > 0.5 ? Integer.valueOf(1) :
+                   Math.random() > 0.5 ? 2 : null;
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/boxPrimitiveInTernary/beforeNullableBooleanMethodInBranch.java

@@ -0,0 +1,10 @@
+// "Box primitive value in conditional branch" "true"
+class Test {
+  public static void main(String strictArgument) {
+    Boolean flag = Math.random() > 0.5 ? <caret>getBoolean() : false;
+  }
+
+  static Boolean getBoolean() {
+    return Math.random() > 0.5 ? true : null;
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/boxPrimitiveInTernary/beforeTwoTernariesBoolean.java

@@ -0,0 +1,6 @@
+// "Box primitive value in conditional branch" "true"
+class Test {
+  public static void main(String strictArgument) {
+    Boolean strict = strictArgument == null ? false : strictArgument.isEmpty() ? true : <caret>null;
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/boxPrimitiveInTernary/beforeTwoTernariesBooleanPrimitiveExpected.java

@@ -0,0 +1,6 @@
+// "Box primitive value in conditional branch" "false"
+class Test {
+  public static void main(String strictArgument) {
+    boolean strict = strictArgument == null ? false : strictArgument.isEmpty() ? true : <caret>null;
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/boxPrimitiveInTernary/beforeTwoTernariesInt.java

@@ -0,0 +1,7 @@
+// "Box primitive value in conditional branch" "true"
+class Test {
+  public static void main(String strictArgument) {
+    Integer test = Math.random() > 0.5 ? 1 :
+                   Math.random() > 0.5 ? 2 : <caret>null;
+  }
+}

java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/quickFix/BoxPrimitiveInTernaryFixTest.java

@@ -0,0 +1,20 @@
+// Copyright 2000-2021 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.java.codeInsight.daemon.quickFix;
+
+import com.intellij.codeInsight.daemon.quickFix.LightQuickFixParameterizedTestCase;
+import com.intellij.codeInspection.LocalInspectionTool;
+import com.intellij.codeInspection.dataFlow.DataFlowInspection;
+import org.jetbrains.annotations.NotNull;
+
+public class BoxPrimitiveInTernaryFixTest extends LightQuickFixParameterizedTestCase {
+
+  @Override
+  protected LocalInspectionTool @NotNull [] configureLocalInspectionTools() {
+    return new LocalInspectionTool[]{new DataFlowInspection()};
+  }
+
+  @Override
+  protected String getBasePath() {
+    return "/codeInsight/daemonCodeAnalyzer/quickFix/boxPrimitiveInTernary";
+  }
+}

java/openapi/resources/messages/JavaBundle.properties

@@ -1757,4 +1757,5 @@ unresolved.class.reference.repair.message=Try to resolve class reference
 remove.var.keyword.text=Remove 'var'
 
 intention.family.name.upgrade.jdk=Upgrade JDK
-intention.name.upgrade.jdk.to=Upgrade JDK to {0}+
+intention.name.upgrade.jdk.to=Upgrade JDK to {0}+
+intention.family.name.box.primitive.in.conditional.branch=Box primitive value in conditional branch

plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/jdk/AutoBoxingInspection.java

@@ -37,7 +37,7 @@ import java.util.Map;
 
 public class AutoBoxingInspection extends BaseInspection {
 
-  @SuppressWarnings({"PublicField"})
+  @SuppressWarnings("PublicField")
   public boolean ignoreAddedToCollection = false;
 
   @NonNls static final Map<String, String> s_boxingClasses = new HashMap<>(8);
@@ -89,6 +89,110 @@ public class AutoBoxingInspection extends BaseInspection {
     return new AutoBoxingFix();
   }
 
+  /**
+   * Replaces expression of primitive type with explicitly boxed expression
+   * @param expression expression to box
+   */
+  public static void replaceWithBoxing(@NotNull PsiExpression expression) {
+    final PsiType type = expression.getType();
+    if (!(type instanceof PsiPrimitiveType)) return;
+    final PsiType expectedType = ((PsiPrimitiveType)type).getBoxedType(expression);
+    if (expectedType == null) {
+      return;
+    }
+    final String expectedTypeText = expectedType.getCanonicalText();
+    final String classToConstruct;
+    if (s_boxingClasses.containsValue(expectedTypeText)) {
+      classToConstruct = expectedTypeText;
+    }
+    else {
+      final String expressionTypeText = type.getCanonicalText();
+      classToConstruct = s_boxingClasses.get(expressionTypeText);
+    }
+    if (shortcutReplace(expression, classToConstruct)) {
+      return;
+    }
+    final PsiExpression strippedExpression = PsiUtil.skipParenthesizedExprDown(expression);
+    if (strippedExpression == null) {
+      return;
+    }
+    CommentTracker commentTracker = new CommentTracker();
+    @NonNls final String expressionText = strippedExpression.getText();
+    @NonNls final String newExpression;
+    if ("true".equals(expressionText)) {
+      newExpression = "java.lang.Boolean.TRUE";
+    }
+    else if ("false".equals(expressionText)) {
+      newExpression = "java.lang.Boolean.FALSE";
+    }
+    else {
+      commentTracker.markUnchanged(strippedExpression);
+      newExpression = classToConstruct + ".valueOf(" + expressionText + ')';
+    }
+    final PsiElement parent = expression.getParent();
+    if (parent instanceof PsiTypeCastExpression) {
+      final PsiTypeCastExpression typeCastExpression = (PsiTypeCastExpression)parent;
+      PsiReplacementUtil.replaceExpression(typeCastExpression, newExpression, commentTracker);
+    } else {
+      PsiReplacementUtil.replaceExpression(expression, newExpression, commentTracker);
+    }
+  }
+
+  private static boolean shortcutReplace(PsiExpression expression, String classToConstruct) {
+    if (!(expression instanceof PsiMethodCallExpression)) {
+      return false;
+    }
+    final PsiMethodCallExpression methodCallExpression = (PsiMethodCallExpression)expression;
+    final PsiReferenceExpression methodExpression = methodCallExpression.getMethodExpression();
+    final PsiExpression qualifierExpression = methodExpression.getQualifierExpression();
+    if (qualifierExpression == null) {
+      return false;
+    }
+    if (classToConstruct.equals(CommonClassNames.JAVA_LANG_INTEGER)) {
+      if (MethodCallUtils.isCallToMethod(methodCallExpression, CommonClassNames.JAVA_LANG_INTEGER, PsiType.INT, "intValue")) {
+        expression.replace(qualifierExpression);
+        return true;
+      }
+    }
+    else if (classToConstruct.equals(CommonClassNames.JAVA_LANG_SHORT)) {
+      if (MethodCallUtils.isCallToMethod(methodCallExpression, CommonClassNames.JAVA_LANG_SHORT, PsiType.SHORT, "shortValue")) {
+        expression.replace(qualifierExpression);
+        return true;
+      }
+    }
+    else if (classToConstruct.equals(CommonClassNames.JAVA_LANG_BYTE)) {
+      if (MethodCallUtils.isCallToMethod(methodCallExpression, CommonClassNames.JAVA_LANG_BYTE, PsiType.BYTE, "byteValue")) {
+        expression.replace(qualifierExpression);
+        return true;
+      }
+    }
+    else if (classToConstruct.equals(CommonClassNames.JAVA_LANG_CHARACTER)) {
+      if (MethodCallUtils.isCallToMethod(methodCallExpression, CommonClassNames.JAVA_LANG_CHARACTER, PsiType.CHAR, "charValue")) {
+        expression.replace(qualifierExpression);
+        return true;
+      }
+    }
+    else if (classToConstruct.equals(CommonClassNames.JAVA_LANG_LONG)) {
+      if (MethodCallUtils.isCallToMethod(methodCallExpression, CommonClassNames.JAVA_LANG_LONG, PsiType.LONG, "longValue")) {
+        expression.replace(qualifierExpression);
+        return true;
+      }
+    }
+    else if (classToConstruct.equals(CommonClassNames.JAVA_LANG_FLOAT)) {
+      if (MethodCallUtils.isCallToMethod(methodCallExpression, CommonClassNames.JAVA_LANG_FLOAT, PsiType.FLOAT, "floatValue")) {
+        expression.replace(qualifierExpression);
+        return true;
+      }
+    }
+    else if (classToConstruct.equals(CommonClassNames.JAVA_LANG_DOUBLE)) {
+      if (MethodCallUtils.isCallToMethod(methodCallExpression, CommonClassNames.JAVA_LANG_DOUBLE, PsiType.DOUBLE, "doubleValue")) {
+        expression.replace(qualifierExpression);
+        return true;
+      }
+    }
+    return false;
+  }
+
   private static class AutoBoxingFix extends InspectionGadgetsFix {
     @Override
     @NotNull
@@ -99,105 +203,7 @@ public class AutoBoxingInspection extends BaseInspection {
     @Override
     public void doFix(Project project, ProblemDescriptor descriptor) {
       final PsiExpression expression = (PsiExpression)descriptor.getPsiElement();
-      final PsiType expectedType = ExpectedTypeUtils.findExpectedType(expression, false, true);
-      if (expectedType == null) {
-        return;
-      }
-      final String expectedTypeText = expectedType.getCanonicalText();
-      final String classToConstruct;
-      if (s_boxingClasses.containsValue(expectedTypeText)) {
-        classToConstruct = expectedTypeText;
-      }
-      else {
-        final PsiType type = expression.getType();
-        if (type == null) {
-          return;
-        }
-        final String expressionTypeText = type.getCanonicalText();
-        classToConstruct = s_boxingClasses.get(expressionTypeText);
-      }
-      if (shortcutReplace(expression, classToConstruct)) {
-        return;
-      }
-      final PsiExpression strippedExpression = PsiUtil.skipParenthesizedExprDown(expression);
-      if (strippedExpression == null) {
-        return;
-      }
-      CommentTracker commentTracker = new CommentTracker();
-      @NonNls final String expressionText = strippedExpression.getText();
-      @NonNls final String newExpression;
-      if ("true".equals(expressionText)) {
-        newExpression = "java.lang.Boolean.TRUE";
-      }
-      else if ("false".equals(expressionText)) {
-        newExpression = "java.lang.Boolean.FALSE";
-      }
-      else {
-        commentTracker.markUnchanged(strippedExpression);
-        newExpression = classToConstruct + ".valueOf(" + expressionText + ')';
-      }
-      final PsiElement parent = expression.getParent();
-      if (parent instanceof PsiTypeCastExpression) {
-        final PsiTypeCastExpression typeCastExpression = (PsiTypeCastExpression)parent;
-        PsiReplacementUtil.replaceExpression(typeCastExpression, newExpression, commentTracker);
-      } else {
-        PsiReplacementUtil.replaceExpression(expression, newExpression, commentTracker);
-      }
-    }
-
-    private static boolean shortcutReplace(PsiExpression expression, String classToConstruct) {
-      if (!(expression instanceof PsiMethodCallExpression)) {
-        return false;
-      }
-      final PsiMethodCallExpression methodCallExpression = (PsiMethodCallExpression)expression;
-      final PsiReferenceExpression methodExpression = methodCallExpression.getMethodExpression();
-      final PsiExpression qualifierExpression = methodExpression.getQualifierExpression();
-      if (qualifierExpression == null) {
-        return false;
-      }
-      if (classToConstruct.equals(CommonClassNames.JAVA_LANG_INTEGER)) {
-        if (MethodCallUtils.isCallToMethod(methodCallExpression, CommonClassNames.JAVA_LANG_INTEGER, PsiType.INT, "intValue")) {
-          expression.replace(qualifierExpression);
-          return true;
-        }
-      }
-      else if (classToConstruct.equals(CommonClassNames.JAVA_LANG_SHORT)) {
-        if (MethodCallUtils.isCallToMethod(methodCallExpression, CommonClassNames.JAVA_LANG_SHORT, PsiType.SHORT, "shortValue")) {
-          expression.replace(qualifierExpression);
-          return true;
-        }
-      }
-      else if (classToConstruct.equals(CommonClassNames.JAVA_LANG_BYTE)) {
-        if (MethodCallUtils.isCallToMethod(methodCallExpression, CommonClassNames.JAVA_LANG_BYTE, PsiType.BYTE, "byteValue")) {
-          expression.replace(qualifierExpression);
-          return true;
-        }
-      }
-      else if (classToConstruct.equals(CommonClassNames.JAVA_LANG_CHARACTER)) {
-        if (MethodCallUtils.isCallToMethod(methodCallExpression, CommonClassNames.JAVA_LANG_CHARACTER, PsiType.CHAR, "charValue")) {
-          expression.replace(qualifierExpression);
-          return true;
-        }
-      }
-      else if (classToConstruct.equals(CommonClassNames.JAVA_LANG_LONG)) {
-        if (MethodCallUtils.isCallToMethod(methodCallExpression, CommonClassNames.JAVA_LANG_LONG, PsiType.LONG, "longValue")) {
-          expression.replace(qualifierExpression);
-          return true;
-        }
-      }
-      else if (classToConstruct.equals(CommonClassNames.JAVA_LANG_FLOAT)) {
-        if (MethodCallUtils.isCallToMethod(methodCallExpression, CommonClassNames.JAVA_LANG_FLOAT, PsiType.FLOAT, "floatValue")) {
-          expression.replace(qualifierExpression);
-          return true;
-        }
-      }
-      else if (classToConstruct.equals(CommonClassNames.JAVA_LANG_DOUBLE)) {
-        if (MethodCallUtils.isCallToMethod(methodCallExpression, CommonClassNames.JAVA_LANG_DOUBLE, PsiType.DOUBLE, "doubleValue")) {
-          expression.replace(qualifierExpression);
-          return true;
-        }
-      }
-      return false;
+      replaceWithBoxing(expression);
     }
   }