From 8fad6682680a9c6bac3e96b6294e41154d146166 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Tue, 14 May 2024 14:02:49 +0200 Subject: [PATCH] [java-inspection] IDEA-345669 Report value compared to itself with == or != GitOrigin-RevId: 601c0bb6c5beddaace52d3b094506d2229195d37 --- .../messages/JavaAnalysisBundle.properties | 2 + .../ExpressionComparedToItselfInspection.java | 64 +++++++++++++++++++ .../src/META-INF/InspectionGadgets.xml | 3 + .../ExpressionComparedToItself.html | 29 +++++++++ .../ExpressionComparedToItself.java | 10 +++ ...xpressionComparedToItselfNoSideEffect.java | 10 +++ ...ressionComparedToItselfInspectionTest.java | 30 +++++++++ 7 files changed, 148 insertions(+) create mode 100644 java/java-analysis-impl/src/com/siyeh/ig/controlflow/ExpressionComparedToItselfInspection.java create mode 100644 java/java-impl/src/inspectionDescriptions/ExpressionComparedToItself.html create mode 100644 java/java-tests/testData/ig/com/siyeh/igtest/controlflow/expression_compared_to_itself/ExpressionComparedToItself.java create mode 100644 java/java-tests/testData/ig/com/siyeh/igtest/controlflow/expression_compared_to_itself/ExpressionComparedToItselfNoSideEffect.java create mode 100644 java/java-tests/testSrc/com/siyeh/ig/controlflow/ExpressionComparedToItselfInspectionTest.java diff --git a/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties b/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties index 7d58357a6704..e897cf3a3f5d 100644 --- a/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties +++ b/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties @@ -629,3 +629,5 @@ inspection.depends.on.the.java.feature=This inspection depends on the Java featu inspection.depends.on.the.java.features=This inspection depends on the following Java features: inspection.depends.on.the.java.features.minimal.version=These features are available since Java {0}. inspection.data.flow.warn.when.reading.a.value.guaranteed.to.be.constant=Warn when constant is stored in variable +inspection.message.expression.compared.to.itself.description=Expression is compared to itself +intention.name.do.not.report.conditions.with.possible.side.effect=Do not report conditions with possible side-effect diff --git a/java/java-analysis-impl/src/com/siyeh/ig/controlflow/ExpressionComparedToItselfInspection.java b/java/java-analysis-impl/src/com/siyeh/ig/controlflow/ExpressionComparedToItselfInspection.java new file mode 100644 index 000000000000..2a2832b49cab --- /dev/null +++ b/java/java-analysis-impl/src/com/siyeh/ig/controlflow/ExpressionComparedToItselfInspection.java @@ -0,0 +1,64 @@ +// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +package com.siyeh.ig.controlflow; + +import com.intellij.codeInspection.AbstractBaseJavaLocalInspectionTool; +import com.intellij.codeInspection.ProblemsHolder; +import com.intellij.codeInspection.UpdateInspectionOptionFix; +import com.intellij.codeInspection.options.OptPane; +import com.intellij.java.analysis.JavaAnalysisBundle; +import com.intellij.modcommand.ModCommandAction; +import com.intellij.psi.JavaElementVisitor; +import com.intellij.psi.PsiBinaryExpression; +import com.intellij.psi.PsiElementVisitor; +import com.intellij.psi.PsiExpression; +import com.intellij.psi.tree.IElementType; +import com.intellij.psi.util.PsiUtil; +import com.intellij.util.ThreeState; +import com.siyeh.InspectionGadgetsBundle; +import com.siyeh.ig.psiutils.ComparisonUtils; +import com.siyeh.ig.psiutils.EquivalenceChecker; +import com.siyeh.ig.psiutils.SideEffectChecker; +import org.jetbrains.annotations.NotNull; + +import static com.intellij.codeInspection.options.OptPane.checkbox; +import static com.intellij.codeInspection.options.OptPane.pane; + +public final class ExpressionComparedToItselfInspection extends AbstractBaseJavaLocalInspectionTool { + public boolean ignoreSideEffectConditions = false; + + @Override + public @NotNull OptPane getOptionsPane() { + return pane( + checkbox("ignoreSideEffectConditions", InspectionGadgetsBundle.message("duplicate.condition.ignore.method.calls.option")) + .description(InspectionGadgetsBundle.message("duplicate.condition.ignore.method.calls.option.description"))); + } + + @Override + public @NotNull PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) { + return new JavaElementVisitor() { + @Override + public void visitBinaryExpression(@NotNull PsiBinaryExpression expression) { + IElementType tokenType = expression.getOperationTokenType(); + if (!ComparisonUtils.isComparisonOperation(tokenType)) return; + PsiExpression leftOperand = expression.getLOperand(); + PsiExpression rightOperand = expression.getROperand(); + if (rightOperand == null) return; + boolean equivalent = EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(leftOperand, rightOperand); + if (!equivalent) return; + if (PsiUtil.isConstantExpression(leftOperand)) return; + ThreeState wantedStatus = ignoreSideEffectConditions ? ThreeState.UNSURE : ThreeState.YES; + ThreeState actualStatus = SideEffectChecker.getSideEffectStatus(leftOperand); + if (actualStatus.isAtLeast(wantedStatus)) return; + ModCommandAction fix = null; + if (actualStatus == ThreeState.UNSURE) { + fix = new UpdateInspectionOptionFix( + ExpressionComparedToItselfInspection.this, "ignoreSideEffectConditions", + JavaAnalysisBundle.message("intention.name.do.not.report.conditions.with.possible.side.effect"), true); + } + holder.problem(expression.getOperationSign(), + JavaAnalysisBundle.message("inspection.message.expression.compared.to.itself.description")) + .maybeFix(fix).register(); + } + }; + } +} diff --git a/java/java-impl/src/META-INF/InspectionGadgets.xml b/java/java-impl/src/META-INF/InspectionGadgets.xml index 1215ed1f4e72..afb00ed8ad08 100644 --- a/java/java-impl/src/META-INF/InspectionGadgets.xml +++ b/java/java-impl/src/META-INF/InspectionGadgets.xml @@ -644,6 +644,9 @@ + + +Reports comparisons where left and right operand represent the identical expression. +While sometimes comparison of an expression with itself could be intended, in most cases they are the result of an oversight. +

Example:

+

+  // Probably left.getLength() == right.getLength() was intended
+  boolean result = left.getLength() == left.getLength();
+
+ +

+ To ignore comparisons that may produce side effects, use the Ignore conditions with side effects option. +Disabling this option may lead to false-positives, for example, when the same method returns different values on subsequent invocations. +

+

Example:

+

+  native int unknownMethod();
+  
+  ...
+  
+  if (unknownMethod() > unknownMethod()) {
+    System.out.println("Got it");
+  }
+
+

Due to possible side effects of unknownMethod() (on the example), the warning will only be + triggered if the Ignore conditions with side effects option is disabled.

+

New in 2024.2

+ + \ No newline at end of file diff --git a/java/java-tests/testData/ig/com/siyeh/igtest/controlflow/expression_compared_to_itself/ExpressionComparedToItself.java b/java/java-tests/testData/ig/com/siyeh/igtest/controlflow/expression_compared_to_itself/ExpressionComparedToItself.java new file mode 100644 index 000000000000..4d1ffcbcf962 --- /dev/null +++ b/java/java-tests/testData/ig/com/siyeh/igtest/controlflow/expression_compared_to_itself/ExpressionComparedToItself.java @@ -0,0 +1,10 @@ +class X { + void test(String a, String b, int start) { + if (Math.random() == Math.random()) { + + } + if (a.substring(start, 10).length() + b.length() > b.length() + a.substring(start, 0xA).length()) { + + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/ig/com/siyeh/igtest/controlflow/expression_compared_to_itself/ExpressionComparedToItselfNoSideEffect.java b/java/java-tests/testData/ig/com/siyeh/igtest/controlflow/expression_compared_to_itself/ExpressionComparedToItselfNoSideEffect.java new file mode 100644 index 000000000000..e3e2a8ac21b3 --- /dev/null +++ b/java/java-tests/testData/ig/com/siyeh/igtest/controlflow/expression_compared_to_itself/ExpressionComparedToItselfNoSideEffect.java @@ -0,0 +1,10 @@ +class X { + void test(String a, String b, int start) { + if (Math.random() == Math.random()) { + + } + if (a.substring(start, 10).length() + b.length() > b.length() + a.substring(start, 0xA).length()) { + + } + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/siyeh/ig/controlflow/ExpressionComparedToItselfInspectionTest.java b/java/java-tests/testSrc/com/siyeh/ig/controlflow/ExpressionComparedToItselfInspectionTest.java new file mode 100644 index 000000000000..e7362ddaf39f --- /dev/null +++ b/java/java-tests/testSrc/com/siyeh/ig/controlflow/ExpressionComparedToItselfInspectionTest.java @@ -0,0 +1,30 @@ +package com.siyeh.ig.controlflow; + +import com.intellij.codeInspection.InspectionProfileEntry; +import com.intellij.testFramework.LightProjectDescriptor; +import com.siyeh.ig.LightJavaInspectionTestCase; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +public class ExpressionComparedToItselfInspectionTest extends LightJavaInspectionTestCase { + @Override + protected @NotNull LightProjectDescriptor getProjectDescriptor() { + return JAVA_21_ANNOTATED; + } + + public void testExpressionComparedToItself() { + doTest(); + } + + public void testExpressionComparedToItselfNoSideEffect() { + doTest(); + } + + @Nullable + @Override + protected InspectionProfileEntry getInspection() { + ExpressionComparedToItselfInspection inspection = new ExpressionComparedToItselfInspection(); + inspection.ignoreSideEffectConditions = getTestName(false).contains("NoSideEffect"); + return inspection; + } +} \ No newline at end of file