make "Unpredictable BigDecimal constructor call" inspection and quickfix more accurate

2026-09-27 10:03:11 +07:00 · 2014-05-13 14:00:59 +02:00
parent 6ee9d29a68
commit c7bcc1180f
9 changed files with 184 additions and 39 deletions
@@ -1612,7 +1612,7 @@ unpredictable.big.decimal.constructor.call.display.name=Unpredictable BigDecimal
 unpredictable.big.decimal.constructor.call.problem.descriptor=Unpredictable <code>new #ref()</code> call #loc
 unpredictable.big.decimal.constructor.call.ignore.references.option=Ignore constructor calls with variable or method call arguments
 unpredictable.big.decimal.constructor.call.ignore.complex.literals.option=Ignore constructor calls with multiple literals (e.g. 0.1 + 0.2)
-unpredictable.big.decimal.constructor.call.quickfix=Replace with ''new BigDecimal("{0}")''
+unpredictable.big.decimal.constructor.call.quickfix=Replace with ''{0}''
 unnecessary.unary.minus.display.name=Unnecessary unary minus
 unnecessary.unary.minus.problem.descriptor=Unnecessary unary minus #loc
 unnecessary.unary.minus.quickfix=Remove unary minus and invert parent operation sign
@@ -1,5 +1,5 @@
 /*
- * Copyright 2007-2009 Bas Leijdekkers
+ * Copyright 2007-2014 Bas Leijdekkers
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
@@ -19,7 +19,6 @@ import com.intellij.codeInspection.ProblemDescriptor;
 import com.intellij.codeInspection.ui.MultipleCheckboxOptionsPanel;
 import com.intellij.openapi.project.Project;
 import com.intellij.psi.*;
-import com.intellij.util.IncorrectOperationException;
 import com.siyeh.InspectionGadgetsBundle;
 import com.siyeh.ig.BaseInspection;
 import com.siyeh.ig.BaseInspectionVisitor;
@@ -33,8 +32,8 @@ import javax.swing.*;
 public class UnpredictableBigDecimalConstructorCallInspection
  extends BaseInspection {

-  public boolean ignoreReferences = true;
-  public boolean ignoreComplexLiterals = false;
+  @SuppressWarnings("PublicField") public boolean ignoreReferences = true;
+  @SuppressWarnings("PublicField") public boolean ignoreComplexLiterals = false;

  @Override
  @NotNull
@@ -75,14 +74,16 @@ public class UnpredictableBigDecimalConstructorCallInspection
      return null;
    }
    final PsiExpression firstArgument = arguments[0];
-    if (!(firstArgument instanceof PsiLiteralExpression)) {
-      return null;
+    if (firstArgument instanceof PsiLiteralExpression) {
+      return new ReplaceDoubleArgumentWithStringFix("new BigDecimal(\"" + firstArgument.getText() + "\")");
    }
-    return new ReplaceDoubleArgumentWithStringFix(firstArgument.getText());
+    if (arguments.length == 1) {
+      return new ReplaceDoubleArgumentWithStringFix("BigDecimal.valueOf(" + firstArgument.getText() + ')');
+    }
+    return null;
  }

-  private class ReplaceDoubleArgumentWithStringFix
-    extends InspectionGadgetsFix {
+  private static class ReplaceDoubleArgumentWithStringFix extends InspectionGadgetsFix {

    private final String argumentText;

@@ -101,27 +102,25 @@ public class UnpredictableBigDecimalConstructorCallInspection
    @NotNull
    @Override
    public String getFamilyName() {
-      return "Replace with 'new BigDecimal'";
+      return "Replace with 'BigDecimal.valueOf()'";
    }

    @Override
-    protected void doFix(Project project, ProblemDescriptor descriptor)
-      throws IncorrectOperationException {
+    protected void doFix(Project project, ProblemDescriptor descriptor) {
      final PsiElement element = descriptor.getPsiElement();
-      final PsiNewExpression newExpression =
-        (PsiNewExpression)element.getParent();
-      final PsiExpressionList argumentList =
-        newExpression.getArgumentList();
+      final PsiNewExpression newExpression = (PsiNewExpression)element.getParent();
+      final PsiExpressionList argumentList = newExpression.getArgumentList();
      if (argumentList == null) {
        return;
      }
      final PsiExpression[] arguments = argumentList.getExpressions();
-      if (arguments.length == 0) {
-        return;
-      }
      final PsiExpression firstArgument = arguments[0];
-      PsiReplacementUtil.replaceExpression(firstArgument,
-                                           '"' + firstArgument.getText() + '"');
+      if (firstArgument instanceof PsiLiteralExpression) {
+        PsiReplacementUtil.replaceExpression(firstArgument, '"' + firstArgument.getText() + '"');
+      }
+      else if (arguments.length == 1) {
+        PsiReplacementUtil.replaceExpression(newExpression, "java.math.BigDecimal.valueOf(" + firstArgument.getText() + ')');
+      }
    }
  }

@@ -149,8 +148,11 @@ public class UnpredictableBigDecimalConstructorCallInspection
      if (constructor == null) {
        return;
      }
-      final PsiParameterList parameterList =
-        constructor.getParameterList();
+      final PsiClass containingClass = constructor.getContainingClass();
+      if (containingClass == null || !"java.math.BigDecimal".equals(containingClass.getQualifiedName())) {
+        return;
+      }
+      final PsiParameterList parameterList = constructor.getParameterList();
      final int length = parameterList.getParametersCount();
      if (length != 1 && length != 2) {
        return;
@@ -170,34 +172,30 @@ public class UnpredictableBigDecimalConstructorCallInspection
        return;
      }
      final PsiExpression firstArgument = arguments[0];
-      if (!checkArguments(firstArgument)) {
+      if (!checkExpression(firstArgument)) {
        return;
      }
      registerNewExpressionError(expression, expression);
    }

-    private boolean checkArguments(@Nullable PsiExpression firstArgument) {
-      if (firstArgument == null) {
+    private boolean checkExpression(@Nullable PsiExpression expression) {
+      if (expression == null) {
        return false;
      }
-      if (firstArgument instanceof PsiReferenceExpression) {
+      if (expression instanceof PsiReferenceExpression) {
        if (ignoreReferences) {
          return false;
        }
      }
-      else if (firstArgument instanceof PsiBinaryExpression) {
+      else if (expression instanceof PsiPolyadicExpression) {
        if (ignoreComplexLiterals) {
          return false;
        }
-        final PsiBinaryExpression binaryExpression =
-          (PsiBinaryExpression)firstArgument;
-        final PsiExpression lhs = binaryExpression.getLOperand();
-        if (!checkArguments(lhs)) {
-          return false;
-        }
-        final PsiExpression rhs = binaryExpression.getROperand();
-        if (!checkArguments(rhs)) {
-          return false;
+        final PsiPolyadicExpression polyadicExpression = (PsiPolyadicExpression)expression;
+        for (PsiExpression operand : polyadicExpression.getOperands()) {
+          if (!checkExpression(operand)) {
+            return false;
+          }
        }
      }
      return true;
@@ -4,7 +4,7 @@ Reports on calls to <b>BigDecimal</b>
 constructors which accept a <b>double</b> value.
 These constructors can have somewhat unpredictable results because many numbers
 cannot be represented exactly in a <b>double</b>.
-It is recommend to use the constructors which accept a String instead.
+It is recommended to use the <b>BigDecimal.valueOf()</b> method or the constructors which accept a <b>String</b> instead.
 <!-- tooltip end -->
 <p>

@@ -0,0 +1,8 @@
+import java.math.BigDecimal;
+
+class Constructor {
+
+  void foo() {
+    new BigDecimal("0.1");
+  }
+}
@@ -0,0 +1,8 @@
+import java.math.BigDecimal;
+
+class Constructor {
+
+  void foo() {
+    new BigDecimal<caret>(0.1);
+  }
+}
@@ -0,0 +1,9 @@
+package com.siyeh.igfixes.numeric.unpredictable_big_decimal;
+
+import java.math.BigDecimal;
+
+class Factory {
+  void foo(double val) {
+    BigDecimal bd = BigDecimal.valueOf(val);
+  }
+}
@@ -0,0 +1,9 @@
+package com.siyeh.igfixes.numeric.unpredictable_big_decimal;
+
+import java.math.BigDecimal;
+
+class Factory {
+  void foo(double val) {
+    BigDecimal bd = new <caret>BigDecimal(val);
+  }
+}
@@ -0,0 +1,53 @@
+/*
+ * Copyright 2000-2014 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.siyeh.ig.fixes.numeric;
+
+import com.siyeh.InspectionGadgetsBundle;
+import com.siyeh.ig.IGQuickFixesTestCase;
+import com.siyeh.ig.numeric.UnpredictableBigDecimalConstructorCallInspection;
+
+/**
+ * @author Bas Leijdekkers
+ */
+public class UnpredictableBigDecimalConstructorCallFixTest extends IGQuickFixesTestCase {
+
+  @Override
+  protected void setUp() throws Exception {
+    super.setUp();
+    final UnpredictableBigDecimalConstructorCallInspection inspection = new UnpredictableBigDecimalConstructorCallInspection();
+    inspection.ignoreReferences = false;
+    myFixture.enableInspections(inspection);
+    myFixture.addClass(
+      "package java.math;" +
+      "public class BigDecimal {" +
+      "  public BigDecimal(double d) {}" +
+      "}"
+    );
+  }
+
+  @Override
+  protected String getRelativePath() {
+    return "numeric/unpredictable_big_decimal";
+  }
+
+  public void testFactory() {
+    doTest(InspectionGadgetsBundle.message("unpredictable.big.decimal.constructor.call.quickfix", "BigDecimal.valueOf(val)"));
+  }
+
+  public void testConstructor() {
+    doTest(InspectionGadgetsBundle.message("unpredictable.big.decimal.constructor.call.quickfix", "new BigDecimal(\"0.1\")"));
+  }
+}
@@ -0,0 +1,60 @@
+/*
+ * Copyright 2000-2014 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.siyeh.ig.numeric;
+
+import com.intellij.codeInspection.InspectionProfileEntry;
+import com.siyeh.ig.LightInspectionTestCase;
+
+public class UnpredictableBigDecimalConstructorCallInspectionTest extends LightInspectionTestCase {
+
+  @Override
+  protected InspectionProfileEntry getInspection() {
+    return new UnpredictableBigDecimalConstructorCallInspection();
+  }
+
+  @Override
+  protected String[] getEnvironmentClasses() {
+    return new String[] {
+      "package java.math;" +
+      "public class BigDecimal {" +
+      "  public BigDecimal(double d) {}" +
+      "  public BigDecimal(int i) {}" +
+      "}"
+    };
+  }
+
+  public void testNotMathBigDecimal() {
+    doTest("class X {" +
+           "  void foo() {" +
+           "    new BigDecimal(.1);" +
+           "  }" +
+           "" +
+           "  class BigDecimal {" +
+           "    BigDecimal(double d) {}" +
+           "  }" +
+           "}");
+  }
+
+  public void testSimple() {
+    doTest("import java.math.*;" +
+           "class X {" +
+           "  void foo() {" +
+           "    new /*Unpredictable 'new BigDecimal()' call*/BigDecimal/**/(.1);" +
+           "    new BigDecimal(1);" +
+           "  }" +
+           "}");
+  }
+}