diff --git a/README.md b/README.md
index 8ba4428..97ca2c5 100644
--- a/README.md
+++ b/README.md
@@ -549,6 +549,14 @@ You can toggle the various inspections in the Settings/Editor/Inspections in the
assertThat(map).hasSameSizeAs(map);
```
+ Note that expressions with method calls will not cause a warning as the method call might have side effects
+ that result in the assertion not being bogus at all.
+
+ If the assertions is either ```isEqualTo()``` or ```hasSameHashCodeAs()``` it may be checking custom
+ ```equals()``` or ```hashCode()``` behavior. If the test method name containing the statement has a
+ name that contains 'equal' or 'hashcode' (case insensitive), the warning will be weakened to information
+ level.
+
- ImplicitAssertion
Detects and removes implicit use of ```isNotNull()```, ```isNotEmpty()``` and
@@ -776,6 +784,11 @@ Feel free to use the code (in package ```de.platon42.intellij.jupiter```) for yo
## Changelog
+#### V1.9 (unreleased)
+- BogusAssertion inspection will no longer warn if the expression contains method calls.
+ Moreover, for assertions of ```isEqualTo()``` and ```hasSameHashCodeAs()```, AND if the containing method name contains 'equal' or 'hashcode',
+ the warning will be reduced to information level as the assertion may be testing ```equals()``` or ```hashCode()``` for validity.
+
#### V1.8 (14-Feb-20) Valentine Edition
- Maintenance. Removed experimental API use. Updated dependencies. Fixed testing problems introduced with IntelliJ IDEA 2019.3
- Added new TwistedAssertion inspection that will warn about assertions with the actual expression being a constant indicating
diff --git a/build.gradle b/build.gradle
index 21974c9..1b44ca2 100644
--- a/build.gradle
+++ b/build.gradle
@@ -7,7 +7,7 @@ plugins {
}
group 'de.platon42'
-version '1.8'
+version '1.9'
repositories {
mavenCentral()
@@ -43,6 +43,12 @@ intellij {
patchPluginXml {
changeNotes """
+ V1.9 (unreleased)
+
+ - BogusAssertion inspection will no longer warn if the expression contains method calls.
+ Moreover, for assertions of isEqualTo() and hasSameHashCodeAs(), AND if the containing method name contains 'equal' or 'hashcode',
+ the warning will be reduced to information level as the assertion may be testing equals() or hashCode() for validity.
+
V1.8 (14-Feb-20) Valentine Edition
- Maintenance. Removed experimental API use. Updated dependencies. Fixed testing problems introduced with IntelliJ IDEA 2019.3.x
diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/BogusAssertionInspection.kt b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/BogusAssertionInspection.kt
index 7a332f1..3b4c59b 100644
--- a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/BogusAssertionInspection.kt
+++ b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/BogusAssertionInspection.kt
@@ -1,10 +1,8 @@
package de.platon42.intellij.plugins.cajon.inspections
+import com.intellij.codeInspection.ProblemHighlightType
import com.intellij.codeInspection.ProblemsHolder
-import com.intellij.psi.JavaElementVisitor
-import com.intellij.psi.PsiElementVisitor
-import com.intellij.psi.PsiExpressionStatement
-import com.intellij.psi.PsiMethodCallExpression
+import com.intellij.psi.*
import com.intellij.psi.util.PsiTreeUtil
import com.siyeh.ig.callMatcher.CallMatcher
import com.siyeh.ig.psiutils.EquivalenceChecker
@@ -15,6 +13,7 @@ class BogusAssertionInspection : AbstractAssertJInspection() {
companion object {
private const val DISPLAY_NAME = "Bogus assertion due to same actual and expected expressions"
private const val ACTUAL_IS_EQUAL_TO_EXPECTED_MESSAGE = "Actual expression in assertThat() is the same as expected"
+ private const val WEAK_ACTUAL_IS_EQUAL_TO_EXPECTED_MESSAGE = "Same actual and expected expression, but may be testing equals() or hashCode()"
private val SAME_OBJECT =
CallMatcher.instanceCall(
@@ -57,6 +56,12 @@ class BogusAssertionInspection : AbstractAssertJInspection() {
private val SAME_OBJECT_ARRAY_CONTENTS =
CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_OBJECT_ARRAY_ASSERT_CLASSNAME, *ARRAY_METHODS).parameterCount(1)
+ private val HASHCODE_OR_IS_EQUAL_TO =
+ CallMatcher.instanceCall(
+ AssertJClassNames.ASSERT_INTERFACE,
+ MethodNames.IS_EQUAL_TO, "hasSameHashCodeAs"
+ ).parameterCount(1)
+
private val SAME_ENUMERABLE_CONTENTS =
CallMatcher.instanceCall(
AssertJClassNames.ENUMERABLE_ASSERT_INTERFACE,
@@ -130,9 +135,42 @@ class BogusAssertionInspection : AbstractAssertJInspection() {
.filter(SAME_ACTUAL_AND_EXPECTED_MATCHERS::test)
.any { equivalenceChecker.expressionsAreEquivalent(actualExpression, it.firstArg) }
if (isSameExpression) {
- holder.registerProblem(statement, ACTUAL_IS_EQUAL_TO_EXPECTED_MESSAGE)
+ if (!hasExpressionWithSideEffects(actualExpression)) {
+ if (allCalls.any(HASHCODE_OR_IS_EQUAL_TO::test)) {
+ val method = PsiTreeUtil.getParentOfType(statement, PsiMethod::class.java, true)
+ val methodName = method?.name
+ if ((methodName != null)
+ && ((methodName.contains("equal", ignoreCase = true) || methodName.contains("hashcode", ignoreCase = true)))
+ ) {
+ if (isOnTheFly) {
+ holder.registerProblem(statement, WEAK_ACTUAL_IS_EQUAL_TO_EXPECTED_MESSAGE, ProblemHighlightType.INFORMATION)
+ }
+ return
+ }
+ }
+ holder.registerProblem(statement, ACTUAL_IS_EQUAL_TO_EXPECTED_MESSAGE)
+ }
}
}
+
+ private fun hasExpressionWithSideEffects(actualExpression: PsiExpression): Boolean {
+ var result = false
+ PsiTreeUtil.processElements(actualExpression) { element ->
+ val matched = when (element) {
+ is PsiUnaryExpression -> (element.operationTokenType == JavaTokenType.PLUSPLUS)
+ || (element.operationTokenType == JavaTokenType.MINUSMINUS)
+ is PsiMethodCallExpression -> true
+ else -> false
+ }
+ if (matched) {
+ result = true
+ false
+ } else {
+ true
+ }
+ }
+ return result
+ }
}
}
}
\ No newline at end of file
diff --git a/src/test/java/de/platon42/intellij/plugins/cajon/inspections/BogusAssertionInspectionTest.kt b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/BogusAssertionInspectionTest.kt
index d099dd5..5c3cc75 100644
--- a/src/test/java/de/platon42/intellij/plugins/cajon/inspections/BogusAssertionInspectionTest.kt
+++ b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/BogusAssertionInspectionTest.kt
@@ -14,5 +14,6 @@ internal class BogusAssertionInspectionTest : AbstractCajonTest() {
myFixture.enableInspections(BogusAssertionInspection::class.java)
myFixture.configureByFile("BogusAssertionBefore.java")
assertHighlightings(myFixture, 14 * 9 + 10 + 12 + 8, "Actual expression in assertThat() is the same as expected")
+ assertHighlightings(myFixture, 3, "Same actual and expected expression, but may be testing equals() or hashCode()")
}
}
\ No newline at end of file
diff --git a/src/test/resources/inspections/BogusAssertion/BogusAssertionBefore.java b/src/test/resources/inspections/BogusAssertion/BogusAssertionBefore.java
index 6980b31..f3dc502 100644
--- a/src/test/resources/inspections/BogusAssertion/BogusAssertionBefore.java
+++ b/src/test/resources/inspections/BogusAssertion/BogusAssertionBefore.java
@@ -1,3 +1,4 @@
+import java.io.File;
import java.util.*;
import static org.assertj.core.api.Assertions.assertThat;
@@ -190,7 +191,28 @@ public class BogusAssertions {
assertThat(bar).isEqualTo(string);
+ assertThat(new Random().nextBoolean()).isEqualTo(new Random().nextBoolean());
+ assertThat(generateString()).isEqualTo(generateString());
+
+ int number = 4;
+ assertThat(number++).isEqualTo(number++);
+ assertThat(number++).isEqualTo(number++);
+
org.junit.Assert.assertThat(list, null);
fail("oh no!");
}
+
+ private void test_equals() {
+ assertThat("foo").isEqualTo("foo");
+ assertThat(new File("foo")).isEqualTo(new File("foo"));
+ }
+
+ private void test_HasHCode() {
+ assertThat("foo").hasSameHashCodeAs("foo");
+ }
+
+ private String generateString()
+ {
+ return "foo";
+ }
}