[java-inspections] IDEA-285964 Replacement for 'Expression can be replaced with Double.compare()' sometimes breaks the code

Option is added to disable the inspection when semantics change is possible

GitOrigin-RevId: 97e5576582b65c3ba0cb3122d6274a37e271386d
This commit is contained in:
Tagir Valeev
2022-01-10 10:15:10 +07:00
committed by intellij-monorepo-bot
parent 71a587c4b2
commit 2d08b71a4e
7 changed files with 57 additions and 9 deletions

View File

@@ -336,6 +336,8 @@ inspection.suspicious.integer.div.assignment.option=Report suspicious but possib
inspection.unary.plus.unary.binary.option=Only report in confusing binary or unary expression context
inspection.unnecessary.super.qualifier.option=Ignore clarification 'super' qualifier
inspection.use.compare.method.fix.family.name=Replace with single comparison method
inspection.use.compare.method.option.double=Suggest Double.compare() and Float.compare()
inspection.use.compare.method.turn.off.double=Do not suggest Double.compare() and Float.compare() methods
inspection.visibility.accept.quickfix=Accept suggested access level
inspection.visibility.compose.suggestion=Can be {0}
inspection.visibility.option.constants=Suggest weaker visibility for constants

View File

@@ -2,6 +2,7 @@
package com.intellij.codeInspection;
import com.intellij.codeInsight.PsiEquivalenceUtil;
import com.intellij.codeInspection.ui.InspectionOptionsPanel;
import com.intellij.java.analysis.JavaAnalysisBundle;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.Pair;
@@ -10,6 +11,7 @@ import com.intellij.psi.*;
import com.intellij.psi.tree.IElementType;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.intellij.psi.util.TypeConversionUtil;
import com.siyeh.ig.psiutils.CommentTracker;
import com.siyeh.ig.psiutils.ControlFlowUtils;
import com.siyeh.ig.psiutils.ExpressionUtils;
@@ -21,6 +23,7 @@ import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import javax.swing.*;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
@@ -29,6 +32,14 @@ import java.util.Map;
import static com.intellij.util.ObjectUtils.tryCast;
public class UseCompareMethodInspection extends AbstractBaseJavaLocalInspectionTool {
public boolean suggestFloatingCompare = true;
@Override
public @Nullable JComponent createOptionsPanel() {
return InspectionOptionsPanel.singleCheckBox(
this, JavaAnalysisBundle.message("inspection.use.compare.method.option.double"), "suggestFloatingCompare");
}
@NotNull
@Override
public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) {
@@ -63,8 +74,12 @@ public class UseCompareMethodInspection extends AbstractBaseJavaLocalInspectionT
}
private void register(CompareInfo info, PsiElement nameElement) {
if (!suggestFloatingCompare && info.myMayChangeSemantics) return;
LocalQuickFix turnOffFloating = info.myMayChangeSemantics ? new SetInspectionOptionFix(
UseCompareMethodInspection.this, "suggestFloatingCompare",
JavaAnalysisBundle.message("inspection.use.compare.method.turn.off.double"), false) : null;
holder.registerProblem(nameElement, JavaAnalysisBundle.message("inspection.expression.can.be.replaced.with.message", info.myClass.getClassName() + ".compare"),
new ReplaceWithPrimitiveCompareFix(info.getReplacementText()));
new ReplaceWithPrimitiveCompareFix(info.getReplacementText()), turnOffFloating);
}
};
}
@@ -186,7 +201,8 @@ public class UseCompareMethodInspection extends AbstractBaseJavaLocalInspectionT
PsiClassType boxedType = leftType instanceof PsiPrimitiveType ? ((PsiPrimitiveType)leftType).getBoxedType(expression) :
tryCast(leftType, PsiClassType.class);
if (boxedType == null) return null;
return new CompareInfo(template, expression, canonicalPair.getFirst(), canonicalPair.getSecond(), boxedType);
return new CompareInfo(template, expression, canonicalPair.getFirst(), canonicalPair.getSecond(), boxedType,
TypeConversionUtil.isFloatOrDoubleType(boxedType));
}
private static Pair<PsiExpression, PsiExpression> getOperands(PsiExpression expression, IElementType expectedToken) {
@@ -226,7 +242,7 @@ public class UseCompareMethodInspection extends AbstractBaseJavaLocalInspectionT
if (left == null) return null;
PsiExpression right = extractPrimitive(boxedType, primitiveType, arg);
if (right == null) return null;
return new CompareInfo(call, call, left, right, boxedType);
return new CompareInfo(call, call, left, right, boxedType, false);
}
@Nullable
@@ -290,17 +306,20 @@ public class UseCompareMethodInspection extends AbstractBaseJavaLocalInspectionT
final @NotNull PsiExpression myLeft;
final @NotNull PsiExpression myRight;
final @NotNull PsiClassType myClass;
final boolean myMayChangeSemantics;
CompareInfo(@NotNull PsiElement template,
@NotNull PsiExpression toReplace,
@NotNull PsiExpression left,
@NotNull PsiExpression right,
@NotNull PsiClassType aClass) {
@NotNull PsiClassType aClass,
boolean mayChangeSemantics) {
myTemplate = template;
myToReplace = toReplace;
myLeft = left;
myRight = right;
myClass = aClass;
myMayChangeSemantics = mayChangeSemantics;
}
private @NotNull PsiElement replace(PsiElement toReplace, CommentTracker ct) {

View File

@@ -17,8 +17,11 @@ instead of more verbose or less efficient constructs.
</code></pre>
<!-- tooltip end -->
<p>
The <code>Double.compare()</code> and <code>Float.compare()</code> methods were introduced in Java 1.4. Methods for other primitive types
are available since Java 1.7.
Note that <code>Double.compare</code> and <code>Float.compare</code> slightly change the code semantics. In particular,
they make <code>-0.0</code> and <code>0.0</code> distinguishable (<code>Double.compare(-0.0, 0.0)</code> yields -1).
Also, they consistently process <code>NaN</code> value. In most of the cases, this semantics change actually improves the
code. Use the checkbox to disable this inspection for floating point numbers if semantics change is unacceptable
in your case.
</p>
<p><small>New in 2017.2</small></p>
</body>

View File

@@ -0,0 +1,6 @@
// "Fix all ''compare()' method can be used to compare numbers' problems in file" "true"
public class Test {
public int test(double a, double b) {
return Double.compare(a, b);
}
}

View File

@@ -0,0 +1,8 @@
// "Fix all ''compare()' method can be used to compare numbers' problems in file" "true"
public class Test {
public int test(double a, double b) {
<caret>if (a < b) return -1;
if (a > b) return 1;
return 0;
}
}

View File

@@ -0,0 +1,8 @@
// "Fix all ''compare()' method can be used to compare numbers' problems in file" "false"
public class Test {
public int test(double a, double b) {
<caret>if (a < b) return -1;
if (a > b) return 1;
return 0;
}
}

View File

@@ -27,9 +27,11 @@ import static com.intellij.testFramework.fixtures.LightJavaCodeInsightFixtureTes
public class UseCompareMethodInspectionTest extends LightQuickFixParameterizedTestCase {
@Override
protected LocalInspectionTool @NotNull [] configureLocalInspectionTools() {
return new LocalInspectionTool[]{
new UseCompareMethodInspection(),
};
UseCompareMethodInspection inspection = new UseCompareMethodInspection();
if (getTestName(false).endsWith("NoFloating.java")) {
inspection.suggestFloatingCompare = false;
}
return new LocalInspectionTool[]{inspection};
}
@Override