From 94c695617ac772500cba4aed2d20a54cb4515546 Mon Sep 17 00:00:00 2001 From: chrisly42 Date: Sat, 11 May 2019 13:40:37 +0200 Subject: [PATCH] Improved JoinAssertThatStatements detection of expressions with side-effects and added pre/post-increment/decrement detection. --- README.md | 24 ++++++++------ build.gradle | 17 +++------- .../JoinAssertThatStatementsInspection.kt | 32 +++++++++++++------ .../JoinStatements/JoinStatementsAfter.java | 9 ++++++ .../JoinStatements/JoinStatementsBefore.java | 9 ++++++ 5 files changed, 60 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index 6053a4b..883b058 100644 --- a/README.md +++ b/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, - except for method calls with known side-effect methods such as ```Iterator.next()``` -- please notify me about others. + 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()``` 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). diff --git a/build.gradle b/build.gradle index e22698c..fc506a3 100644 --- a/build.gradle +++ b/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 """ +

V1.1 (unreleased)

+

V1.0 (06-May-19)

-

V0.8 (05-May-19)

-

Full changelog available at Github project site.

""" } diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/JoinAssertThatStatementsInspection.kt b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/JoinAssertThatStatementsInspection.kt index f8b16a5..7468c88 100644 --- a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/JoinAssertThatStatementsInspection.kt +++ b/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) { - is PsiMethodCallExpression -> equivalenceChecker.expressionsAreEquivalent(actualExpression, lastActualExpression) - // 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) - } + val isSame = equivalenceChecker.expressionsAreEquivalent(actualExpression, lastActualExpression) + && !hasExpressionWithSideEffects(actualExpression) + 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 + } } } diff --git a/src/test/resources/inspections/JoinStatements/JoinStatementsAfter.java b/src/test/resources/inspections/JoinStatements/JoinStatementsAfter.java index 21ab11f..e6ffe1e 100644 --- a/src/test/resources/inspections/JoinStatements/JoinStatementsAfter.java +++ b/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"); } } diff --git a/src/test/resources/inspections/JoinStatements/JoinStatementsBefore.java b/src/test/resources/inspections/JoinStatements/JoinStatementsBefore.java index 737d781..6fea244 100644 --- a/src/test/resources/inspections/JoinStatements/JoinStatementsBefore.java +++ b/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"); } }