Improved JoinAssertThatStatements detection of expressions with side-effects and added pre/post-increment/decrement detection.

README.md

@@ -81,8 +81,9 @@ You can toggle the various inspections in the Settings/Editor/Inspections in the
         assertThat(expected).anotherCondition();
     to: assertThat(expected).someCondition().anotherCondition();
   ```
-  Joining will work on actual expressions inside assertThat() that are equivalent expressions,
+  Joining will work on actual expressions inside ```assertThat()``` that are equivalent expressions,
-  except for method calls with known side-effect methods such as ```Iterator.next()``` -- please notify me about others.
+  except for method calls with known side-effect methods such as ```Iterator.next()``` and
+  pre/post-increment/decrement operations -- please notify me about others.
 
   The comments of the statements will be preserved. When using ```.extracting()``` or similar, the statements will not be merged.
 
@@ -145,7 +146,7 @@ You can toggle the various inspections in the Settings/Editor/Inspections in the
 
 - AssertThatStringExpression
   
-  Moves string operations inside assertThat() out.
+  Moves string operations inside ```assertThat()``` out.
   
   ```
   from: assertThat(stringActual.isEmpty()).isTrue();
@@ -171,7 +172,7 @@ You can toggle the various inspections in the Settings/Editor/Inspections in the
 
 - AssertThatCollectionOrMapExpression
 
-  Moves collection and map operations inside assertThat() out.
+  Moves collection and map operations inside ```assertThat()``` out.
 
   ```
   from: assertThat(collection.isEmpty()).isTrue();
@@ -192,7 +193,7 @@ You can toggle the various inspections in the Settings/Editor/Inspections in the
   from: assertThat(map.containsValue(value)).isTrue();
     to: assertThat(map).containsValue(value);
   ```
-  Analogously with ```isFalse()``` (except for containsAll()).
+  Analogously with ```isFalse()``` (except for ```containsAll()```).
 
 - AssertThatEnumerableIsEmpty
 
@@ -279,7 +280,7 @@ You can toggle the various inspections in the Settings/Editor/Inspections in the
 
 - AssertThatJava8Optional
 
-  Examines the statement for Java 8 Optional type and whether the statement
+  Examines the statement for Java 8 ```Optional``` type and whether the statement
   effectively tries to assert the presence, absence or content and then 
   replaces the statement by better assertions.
   
@@ -313,7 +314,7 @@ You can toggle the various inspections in the Settings/Editor/Inspections in the
 
 - AssertThatGuavaOptional
 
-  Examines the statement for Google Guava Optional type and whether the statement
+  Examines the statement for Google Guava ```Optional``` type and whether the statement
   effectively tries to assert the presence, absence or content and then 
   replaces the statement by better assertions.
 
@@ -444,9 +445,11 @@ Cajon is written in Kotlin 1.3.
 
 Cajon is probably the only plugin that uses JUnit 5 Jupiter for unit testing so far (or at least the only one that I'm aware of ;) ).
 The IntelliJ framework actually uses the JUnit 3 TestCase for plugin testing and it took me quite a while to make it work with JUnit 5.
-Feel free to use the code (in package de.platon42.intellij.jupiter) for your projects (with attribution).
+Feel free to use the code (in package ```de.platon42.intellij.jupiter```) for your projects (with attribution).
 
 ## Planned features
+- Joining .contains() expressions
+- Removing .isPresent().contains() combinations for Optionals
 - Extraction with property names to lambda with Java 8
   ```
   from: assertThat(object).extracting("propOne", "propNoGetter", "propTwo.innerProp")...
@@ -455,11 +458,14 @@ Feel free to use the code (in package de.platon42.intellij.jupiter) for your pro
 
 ## Changelog
 
+#### V1.1 (unreleased)
+- Improved JoinAssertThatStatements detection of expressions with side-effects and added pre/post-increment/decrement detection.
+
 #### V1.0 (06-May-19)
 - First release to be considered stable enough for production use.
 - Fixed a NPE in AssumeThatInsteadOfReturn inspection quickfix for empty else branches.
 - Fixed missing description for AssumeThatInsteadOfReturn inspection.
-- Added new AssertThatCollectionOrMapExpression inspection that tries to pull out methods such as ```isEmpty()``` or ```contains()``` out of an actual assertThat() expression.
+- Added new AssertThatCollectionOrMapExpression inspection that tries to pull out methods such as ```isEmpty()``` or ```contains()``` out of an actual ```assertThat()``` expression.
 
 #### V0.8 (05-May-19)
 - Fixed missing description for JoinAssertThatStatements and detection of equivalent expressions (sorry, released it too hastily).

build.gradle

@@ -7,7 +7,7 @@ plugins {
 }
 
 group 'de.platon42'
-version '1.0'
+version '1.1'
 
 repositories {
     mavenCentral()
@@ -42,6 +42,10 @@ intellij {
 
 patchPluginXml {
     changeNotes """
+  <h4>V1.1 (unreleased)</h4>
+    <ul>
+      <li>Improved JoinAssertThatStatements detection of expressions with side-effects and added pre/post-increment/decrement detection.
+    </ul>
   <h4>V1.0 (06-May-19)</h4>
     <ul>
       <li>First release to be considered stable enough for production use.
@@ -49,17 +53,6 @@ patchPluginXml {
       <li>Fixed missing description for AssumeThatInsteadOfReturn inspection.
       <li>Added new AssertThatCollectionOrMapExpression inspection that tries to pull out methods such as isEmpty() or contains() out of an actual assertThat() expression.
     </ul>
-  <h4>V0.8 (05-May-19)</h4>
-    <ul>
-      <li>Fixed missing description for JoinAssertThatStatements and detection of equivalent expressions (sorry, released it too hastily).
-      <li>Fixed isEmpty() for enumerables and strings and isNull() for object conversions to be applied only if it is the terminal method call as isEmpty() and isNull() return void.
-      <li>Heavily reworked inspections for edge cases, such as multiple isEqualTo() calls inside a single statement.
-      <li>Some inspections could generate bogus code for weird situations, this has been made more fool-proof.
-      <li>Corrected highlighting for many inspections.
-      <li>Fixed family names for inspections in batch mode.
-      <li>Reworded many inspection messages for better understanding.
-      <li>Added a first version of a new inspection that tries to detect bogus uses of return statements in test methods and replaces them by assumeThat() calls.
-    </ul>
   <p>Full changelog available at <a href="https://github.com/chrisly42/cajon-plugin#changelog">Github project site</a>.</p>
 """
 }

src/main/java/de/platon42/intellij/plugins/cajon/inspections/JoinAssertThatStatementsInspection.kt

@@ -35,16 +35,9 @@ class JoinAssertThatStatementsInspection : AbstractAssertJInspection() {
                         reset = (lastActualExpression == null)
                         actualExpression = assertThatCall.firstArg
                         if (!reset) {
-                            val isSame = when (actualExpression) {
+                            val isSame = equivalenceChecker.expressionsAreEquivalent(actualExpression, lastActualExpression)
-                                is PsiMethodCallExpression -> equivalenceChecker.expressionsAreEquivalent(actualExpression, lastActualExpression)
+                                    && !hasExpressionWithSideEffects(actualExpression)
-                                        // Note: replace with PsiTreeUtil.findChildrenOfAnyType(strict = false) for IDEA >= 2018.1
+
-                                        && !KNOWN_METHODS_WITH_SIDE_EFFECTS.test(actualExpression)
-                                        && PsiTreeUtil.findChildrenOfAnyType(
-                                    actualExpression,
-                                    PsiMethodCallExpression::class.java
-                                ).none { KNOWN_METHODS_WITH_SIDE_EFFECTS.test(it) }
-                                else -> equivalenceChecker.expressionsAreEquivalent(actualExpression, lastActualExpression)
-                            }
                             if (isSame) {
                                 sameCount++
                                 lastStatement = statement
@@ -78,6 +71,25 @@ class JoinAssertThatStatementsInspection : AbstractAssertJInspection() {
                 }
                 return null
             }
+
+            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 -> KNOWN_METHODS_WITH_SIDE_EFFECTS.test(element)
+                        else -> false
+                    }
+                    if (matched) {
+                        result = true
+                        false
+                    } else {
+                        true
+                    }
+                }
+                return result
+            }
         }
     }
 

src/test/resources/inspections/JoinStatements/JoinStatementsAfter.java

@@ -48,5 +48,14 @@ public class JoinStatements {
         assertThat(iterator.next()).isEqualTo("bar");
         assertThat(iterator.next().toLowerCase()).isEqualTo("foo");
         assertThat(iterator.next().toLowerCase()).isEqualTo("bar");
+        assertThat(iterator.next() + "bar").isEqualTo("foobar");
+        assertThat(iterator.next() + "bar").isEqualTo("barbar");
+        int i = 0;
+        assertThat(++i).isEqualTo(1);
+        assertThat(++i).isEqualTo(2);
+        assertThat(list.get(i++).toLowerCase()).isEqualTo("foo");
+        assertThat(list.get(i++).toLowerCase()).isEqualTo("foo");
+        assertThat(list.get(--i)).isEqualTo("foo");
+        assertThat(list.get(--i)).isEqualTo("foo");
     }
 }

src/test/resources/inspections/JoinStatements/JoinStatementsBefore.java

@@ -50,5 +50,14 @@ public class JoinStatements {
         assertThat(iterator.next()).isEqualTo("bar");
         assertThat(iterator.next().toLowerCase()).isEqualTo("foo");
         assertThat(iterator.next().toLowerCase()).isEqualTo("bar");
+        assertThat(iterator.next() + "bar").isEqualTo("foobar");
+        assertThat(iterator.next() + "bar").isEqualTo("barbar");
+        int i = 0;
+        assertThat(++i).isEqualTo(1);
+        assertThat(++i).isEqualTo(2);
+        assertThat(list.get(i++).toLowerCase()).isEqualTo("foo");
+        assertThat(list.get(i++).toLowerCase()).isEqualTo("foo");
+        assertThat(list.get(--i)).isEqualTo("foo");
+        assertThat(list.get(--i)).isEqualTo("foo");
     }
 }