diff --git a/README.md b/README.md index 698e749..8ba4428 100644 --- a/README.md +++ b/README.md @@ -34,6 +34,10 @@ Then AssertJ would tell you the _actual contents_ of the collection on failure. The plugin also supports the conversion of the most common JUnit 4 assertions to AssertJ. +## Wrong use of AssertJ + +Cajon also warns about bogus or incorrect uses of AssertJ. + ## Lookup and refactoring of string-based extracting() AssertJ allows [extracting POJO fields/properties on iterables/arrays](http://joel-costigliola.github.io/assertj/assertj-core-features-highlight.html#extracted-properties-assertion). @@ -484,6 +488,67 @@ You can toggle the various inspections in the Settings/Editor/Inspections in the ``` ...and many, many more combinations (more than 150). +- TwistedAssertion + + Examines the actual expression for common mistakes such as mixing expected and actual expression. + For simple cases, a quick fix is offered to swap them. Otherwise, only a warning is issued. + + ``` + from: assertThat(5).isEqualTo(variable); + to: assertThat(variable).isEqualTo(5); + + from: assertThat(8.0).isGreaterThan(variable); + to: assertThat(variable).isLessOrEqualTo(8.0); + ``` + + There are, of course, more variations of the theme. + +- BogusAssertion + + Sometimes programmers make copy and paste or logical errors writing down assertions + that will never fail due to the same actual and expected assertions. + This inspection will warn about obvious cases such as the following ones. + + ``` + assertThat(object).isEqualTo(object); + assertThat(object).isSameAs(object); + assertThat(object).hasSameClassAs(object); + assertThat(object).hasSameHashCodeAs(object); + + assertThat(array).hasSameSizeAs(array); + assertThat(array).contains(array); + assertThat(array).containsAnyOf(array); + assertThat(array).containsExactly(array); + assertThat(array).containsExactlyInAnyOrder(array); + assertThat(array).containsExactlyInAnyOrder(array); + assertThat(array).containsOnly(array); + assertThat(array).containsSequence(array); + assertThat(array).containsSubsequence(array); + assertThat(array).startsWith(array); + assertThat(array).endsWith(array); + + assertThat(enumerable).hasSameSizeAs(enumerable); + + assertThat(iterable).hasSameElementsAs(iterable); + assertThat(iterable).containsAll(iterable); + assertThat(iterable).containsAnyElementOf(iterable); + assertThat(iterable).containsOnlyElementsOf(iterable); + assertThat(iterable).containsExactlyElementsOf(iterable); + assertThat(iterable).containsSequence(iterable); + assertThat(iterable).containsSubsequence(iterable); + + assertThat(charSeq).isEqualToIgnoringCase(charSeq); + assertThat(charSeq).startsWith(charSeq); + assertThat(charSeq).endsWith(charSeq); + assertThat(charSeq).containsSequence(charSeq); + assertThat(charSeq).containsSubsequence(charSeq); + + assertThat(map).containsAllEntriesOf(map); + assertThat(map).containsExactlyEntriesOf(map); + assertThat(map).containsExactlyInAnyOrderEntriesOf(map); + assertThat(map).hasSameSizeAs(map); + ``` + - ImplicitAssertion Detects and removes implicit use of ```isNotNull()```, ```isNotEmpty()``` and @@ -711,7 +776,11 @@ Feel free to use the code (in package ```de.platon42.intellij.jupiter```) for yo ## Changelog -#### V1.8 (unreleased) +#### 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 + swapped use of actual and expected expressions. +- Added new BogusAssertion inspection that showing typical copy and paste errors where actual and expected expressions are the same. #### V1.7 (19-Nov-19) - Fixed a lapsuus in AssertThatFileExpression also transforming ```.listFiles()``` with a filter argument. diff --git a/build.gradle b/build.gradle index 94be48f..21974c9 100644 --- a/build.gradle +++ b/build.gradle @@ -35,7 +35,7 @@ compileTestKotlin { kotlinOptions.jvmTarget = "1.8" } intellij { - version '2019.3.2' + version '2019.3.3' // pluginName 'Concise AssertJ Optimizing Nitpicker (Cajon)' updateSinceUntilBuild false plugins = ['java'] @@ -43,9 +43,12 @@ intellij { patchPluginXml { changeNotes """ -

V1.8 (unreleased)

+

V1.8 (14-Feb-20) Valentine Edition

Full changelog available at Github project site.

""" diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/AssertJClassNames.kt b/src/main/java/de/platon42/intellij/plugins/cajon/AssertJClassNames.kt index c780987..6605812 100644 --- a/src/main/java/de/platon42/intellij/plugins/cajon/AssertJClassNames.kt +++ b/src/main/java/de/platon42/intellij/plugins/cajon/AssertJClassNames.kt @@ -44,6 +44,22 @@ class AssertJClassNames { @NonNls const val ABSTRACT_MAP_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractMapAssert" @NonNls + const val ABSTRACT_BOOLEAN_ARRAY_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractBooleanArrayAssert" + @NonNls + const val ABSTRACT_BYTE_ARRAY_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractByteArrayAssert" + @NonNls + const val ABSTRACT_SHORT_ARRAY_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractShortArrayAssert" + @NonNls + const val ABSTRACT_INT_ARRAY_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractIntArrayAssert" + @NonNls + const val ABSTRACT_LONG_ARRAY_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractLongArrayAssert" + @NonNls + const val ABSTRACT_FLOAT_ARRAY_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractFloatArrayAssert" + @NonNls + const val ABSTRACT_DOUBLE_ARRAY_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractDoubleArrayAssert" + @NonNls + const val ABSTRACT_CHAR_ARRAY_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractCharArrayAssert" + @NonNls const val ABSTRACT_OBJECT_ARRAY_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractObjectArrayAssert" @NonNls const val ABSTRACT_ITERABLE_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractIterableAssert" diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/Extensions.kt b/src/main/java/de/platon42/intellij/plugins/cajon/Extensions.kt index 1f3b975..d303f9d 100644 --- a/src/main/java/de/platon42/intellij/plugins/cajon/Extensions.kt +++ b/src/main/java/de/platon42/intellij/plugins/cajon/Extensions.kt @@ -43,7 +43,7 @@ fun PsiElement.findStaticMethodCall(): PsiMethodCallExpression? { fun PsiElement.gatherAssertionCalls(): List { val assertThatMethodCall = findStaticMethodCall() ?: return emptyList() return assertThatMethodCall.collectMethodCallsUpToStatement() - .filterNot { NOT_ACTUAL_ASSERTIONS.test(it) } + .filterNot(NOT_ACTUAL_ASSERTIONS::test) .toList() } 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 new file mode 100644 index 0000000..7a332f1 --- /dev/null +++ b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/BogusAssertionInspection.kt @@ -0,0 +1,138 @@ +package de.platon42.intellij.plugins.cajon.inspections + +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.util.PsiTreeUtil +import com.siyeh.ig.callMatcher.CallMatcher +import com.siyeh.ig.psiutils.EquivalenceChecker +import de.platon42.intellij.plugins.cajon.* + +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 val SAME_OBJECT = + CallMatcher.instanceCall( + AssertJClassNames.ASSERT_INTERFACE, + MethodNames.IS_EQUAL_TO, + MethodNames.IS_SAME_AS, + "hasSameClassAs", + "hasSameHashCodeAs" + ).parameterCount(1) + + private val ARRAY_METHODS = arrayOf( + "hasSameSizeAs", + MethodNames.CONTAINS, + "containsAnyOf", + "containsExactly", + "containsExactlyInAnyOrder", + "containsOnly", + "containsSequence", + "containsSubsequence", + "startsWith", + "endsWith" + ) + + private val SAME_BOOLEAN_ARRAY_CONTENTS = + CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_BOOLEAN_ARRAY_ASSERT_CLASSNAME, *ARRAY_METHODS).parameterCount(1) + private val SAME_BYTE_ARRAY_CONTENTS = + CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_BYTE_ARRAY_ASSERT_CLASSNAME, *ARRAY_METHODS).parameterCount(1) + private val SAME_SHORT_ARRAY_CONTENTS = + CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_SHORT_ARRAY_ASSERT_CLASSNAME, *ARRAY_METHODS).parameterCount(1) + private val SAME_INT_ARRAY_CONTENTS = + CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_INT_ARRAY_ASSERT_CLASSNAME, *ARRAY_METHODS).parameterCount(1) + private val SAME_LONG_ARRAY_CONTENTS = + CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_LONG_ARRAY_ASSERT_CLASSNAME, *ARRAY_METHODS).parameterCount(1) + private val SAME_FLOAT_ARRAY_CONTENTS = + CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_FLOAT_ARRAY_ASSERT_CLASSNAME, *ARRAY_METHODS).parameterCount(1) + private val SAME_DOUBLE_ARRAY_CONTENTS = + CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_DOUBLE_ARRAY_ASSERT_CLASSNAME, *ARRAY_METHODS).parameterCount(1) + private val SAME_CHAR_ARRAY_CONTENTS = + CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_CHAR_ARRAY_ASSERT_CLASSNAME, *ARRAY_METHODS).parameterCount(1) + private val SAME_OBJECT_ARRAY_CONTENTS = + CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_OBJECT_ARRAY_ASSERT_CLASSNAME, *ARRAY_METHODS).parameterCount(1) + + private val SAME_ENUMERABLE_CONTENTS = + CallMatcher.instanceCall( + AssertJClassNames.ENUMERABLE_ASSERT_INTERFACE, + MethodNames.HAS_SAME_SIZE_AS + ).parameterCount(1) + + private val SAME_ITERABLE_CONTENTS = + CallMatcher.instanceCall( + AssertJClassNames.ABSTRACT_ITERABLE_ASSERT_CLASSNAME, + "hasSameElementsAs", + MethodNames.CONTAINS_ALL, + "containsAnyElementsOf", + "containsOnlyElementsOf", + "containsExactlyElementsOf", + "containsSequence", + "containsSubsequence" + ).parameterCount(1) + + private val SAME_MAP_CONTENTS = + CallMatcher.instanceCall( + AssertJClassNames.ABSTRACT_MAP_ASSERT_CLASSNAME, + "containsAllEntriesOf", + "containsExactlyEntriesOf", + "containsExactlyInAnyOrderEntriesOf", + MethodNames.HAS_SAME_SIZE_AS + ).parameterCount(1) + + private val SAME_CHAR_SEQUENCE_CONTENTS = + CallMatcher.instanceCall( + AssertJClassNames.ABSTRACT_CHAR_SEQUENCE_ASSERT_CLASSNAME, + MethodNames.IS_EQUAL_TO, + MethodNames.IS_EQUAL_TO_IC, + MethodNames.STARTS_WITH, + MethodNames.ENDS_WITH, + "containsSequence", + "containsSubsequence" + ).parameterCount(1) + + private val SAME_ACTUAL_AND_EXPECTED_MATCHERS = CallMatcher.anyOf( + SAME_OBJECT, + SAME_ENUMERABLE_CONTENTS, + SAME_ITERABLE_CONTENTS, + SAME_MAP_CONTENTS, + SAME_CHAR_SEQUENCE_CONTENTS, + + SAME_BOOLEAN_ARRAY_CONTENTS, + SAME_BYTE_ARRAY_CONTENTS, + SAME_SHORT_ARRAY_CONTENTS, + SAME_INT_ARRAY_CONTENTS, + SAME_LONG_ARRAY_CONTENTS, + SAME_FLOAT_ARRAY_CONTENTS, + SAME_DOUBLE_ARRAY_CONTENTS, + SAME_CHAR_ARRAY_CONTENTS, + SAME_OBJECT_ARRAY_CONTENTS + ) + } + + override fun getDisplayName() = DISPLAY_NAME + + override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor { + return object : JavaElementVisitor() { + override fun visitExpressionStatement(statement: PsiExpressionStatement) { + super.visitExpressionStatement(statement) + if (!statement.hasAssertThat()) return + val assertThatCall = PsiTreeUtil.findChildrenOfType(statement, PsiMethodCallExpression::class.java).find { ALL_ASSERT_THAT_MATCHERS.test(it) } ?: return + val actualExpression = assertThatCall.firstArg + val allCalls = assertThatCall.collectMethodCallsUpToStatement().toList() + // Note: replace with TrackingEquivalenceChecker() for IDEA >= 2019.1 + val equivalenceChecker = EquivalenceChecker.getCanonicalPsiEquivalence()!! + val isSameExpression = allCalls + .filter(SAME_ACTUAL_AND_EXPECTED_MATCHERS::test) + .any { equivalenceChecker.expressionsAreEquivalent(actualExpression, it.firstArg) } + if (isSameExpression) { + holder.registerProblem(statement, ACTUAL_IS_EQUAL_TO_EXPECTED_MESSAGE) + } + } + } + } +} \ No newline at end of file diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/JoinVarArgsContainsInspection.kt b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/JoinVarArgsContainsInspection.kt index d569699..1befd4d 100644 --- a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/JoinVarArgsContainsInspection.kt +++ b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/JoinVarArgsContainsInspection.kt @@ -26,7 +26,7 @@ class JoinVarArgsContainsInspection : AbstractAssertJInspection() { override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor { return object : JavaElementVisitor() { override fun visitExpressionStatement(statement: PsiExpressionStatement) { - super.visitStatement(statement) + super.visitExpressionStatement(statement) if (!statement.hasAssertThat()) return val assertThatCall = PsiTreeUtil.findChildrenOfType(statement, PsiMethodCallExpression::class.java).find { ALL_ASSERT_THAT_MATCHERS.test(it) } ?: return @@ -35,7 +35,7 @@ class JoinVarArgsContainsInspection : AbstractAssertJInspection() { if (allCalls.find(COMPLEX_CALLS_THAT_MAKES_STUFF_TRICKY::test) != null) return val onlyAssertionCalls = allCalls - .filterNot { NOT_ACTUAL_ASSERTIONS.test(it) } + .filterNot(NOT_ACTUAL_ASSERTIONS::test) .toList() for (methodMatcher in MATCHERS) { diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/TwistedAssertionInspection.kt b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/TwistedAssertionInspection.kt new file mode 100644 index 0000000..15f6cc2 --- /dev/null +++ b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/TwistedAssertionInspection.kt @@ -0,0 +1,91 @@ +package de.platon42.intellij.plugins.cajon.inspections + +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.util.PsiTreeUtil +import com.siyeh.ig.callMatcher.CallMatcher +import de.platon42.intellij.plugins.cajon.* +import de.platon42.intellij.plugins.cajon.quickfixes.SwapActualAndExpectedExpressionMethodCallQuickFix + +class TwistedAssertionInspection : AbstractAssertJInspection() { + + companion object { + private const val DISPLAY_NAME = "Twisted or suspicious actual and expected expressions" + private const val TWISTED_ACTUAL_AND_EXPECTED_MESSAGE = "Twisted actual and expected expressions in assertion" + private const val SWAP_ACTUAL_AND_EXPECTED_DESCRIPTION = "Swap actual and expected expressions in assertion" + private const val SWAP_ACTUAL_AND_EXPECTED_AND_REPLACE_DESCRIPTION_TEMPLATE = "Replace %s() by %s() and swap actual and expected expressions" + private const val ACTUAL_IS_A_CONSTANT_MESSAGE = "Actual expression in assertThat() is a constant" + + private val GENERIC_IS_EQUAL_TO = CallMatcher.instanceCall(AssertJClassNames.ASSERT_INTERFACE, MethodNames.IS_EQUAL_TO).parameterCount(1) + private val GENERIC_IS_NOT_EQUAL_TO = CallMatcher.instanceCall(AssertJClassNames.ASSERT_INTERFACE, MethodNames.IS_NOT_EQUAL_TO).parameterCount(1) + private val GENERIC_IS_SAME_AS = CallMatcher.instanceCall(AssertJClassNames.ASSERT_INTERFACE, MethodNames.IS_SAME_AS).parameterCount(1) + private val GENERIC_IS_NOT_SAME_AS = CallMatcher.instanceCall(AssertJClassNames.ASSERT_INTERFACE, MethodNames.IS_NOT_SAME_AS).parameterCount(1) + private val GENERIC_IS_GREATER_THAN = CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_COMPARABLE_ASSERT_CLASSNAME, MethodNames.IS_GREATER_THAN).parameterCount(1) + private val GENERIC_IS_GREATER_THAN_OR_EQUAL_TO = + CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_COMPARABLE_ASSERT_CLASSNAME, MethodNames.IS_GREATER_THAN_OR_EQUAL_TO).parameterCount(1) + private val GENERIC_IS_LESS_THAN = CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_COMPARABLE_ASSERT_CLASSNAME, MethodNames.IS_LESS_THAN).parameterCount(1) + private val GENERIC_IS_LESS_THAN_OR_EQUAL_TO = + CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_COMPARABLE_ASSERT_CLASSNAME, MethodNames.IS_LESS_THAN_OR_EQUAL_TO).parameterCount(1) + + private val STRING_IS_EQUAL_TO_IC = CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_CHAR_SEQUENCE_ASSERT_CLASSNAME, MethodNames.IS_EQUAL_TO_IC).parameterCount(1) + + private val CALL_MATCHER_TO_REPLACEMENT_MAP = mapOf( + GENERIC_IS_EQUAL_TO to MethodNames.IS_EQUAL_TO, + GENERIC_IS_NOT_EQUAL_TO to MethodNames.IS_NOT_EQUAL_TO, + GENERIC_IS_SAME_AS to MethodNames.IS_SAME_AS, + GENERIC_IS_NOT_SAME_AS to MethodNames.IS_NOT_SAME_AS, + GENERIC_IS_GREATER_THAN to MethodNames.IS_LESS_THAN_OR_EQUAL_TO, + GENERIC_IS_GREATER_THAN_OR_EQUAL_TO to MethodNames.IS_LESS_THAN, + GENERIC_IS_LESS_THAN to MethodNames.IS_GREATER_THAN_OR_EQUAL_TO, + GENERIC_IS_LESS_THAN_OR_EQUAL_TO to MethodNames.IS_GREATER_THAN, + + STRING_IS_EQUAL_TO_IC to MethodNames.IS_EQUAL_TO_IC, + CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_CHAR_SEQUENCE_ASSERT_CLASSNAME, MethodNames.IS_NOT_EQUAL_TO_IC).parameterCount(1) + to MethodNames.IS_NOT_EQUAL_TO_IC + ) + } + + override fun getDisplayName() = DISPLAY_NAME + + override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor { + return object : JavaElementVisitor() { + override fun visitExpressionStatement(statement: PsiExpressionStatement) { + super.visitExpressionStatement(statement) + if (!statement.hasAssertThat()) return + val assertThatCall = PsiTreeUtil.findChildrenOfType(statement, PsiMethodCallExpression::class.java).find { ALL_ASSERT_THAT_MATCHERS.test(it) } ?: return + val actualExpression = assertThatCall.firstArg + actualExpression.calculateConstantValue() ?: return + val allCalls = assertThatCall.collectMethodCallsUpToStatement().toList() + val tooComplex = allCalls.find(USING_COMPARATOR::test) != null + if (!tooComplex) { + val onlyAssertionCalls = allCalls + .filterNot(NOT_ACTUAL_ASSERTIONS::test) + .toList() + if (onlyAssertionCalls.size == 1) { + val originalMethodCall = onlyAssertionCalls.first() + val matchedMethod = CALL_MATCHER_TO_REPLACEMENT_MAP.asSequence().firstOrNull { it.key.test(originalMethodCall) } + if (matchedMethod != null) { + val originalMethodName = getOriginalMethodName(originalMethodCall) + val replacementMethod = matchedMethod.value + val description = if (originalMethodName == replacementMethod) { + SWAP_ACTUAL_AND_EXPECTED_DESCRIPTION + } else { + SWAP_ACTUAL_AND_EXPECTED_AND_REPLACE_DESCRIPTION_TEMPLATE.format(originalMethodName, replacementMethod) + } + holder.registerProblem( + statement, + TWISTED_ACTUAL_AND_EXPECTED_MESSAGE, + SwapActualAndExpectedExpressionMethodCallQuickFix(description, replacementMethod) + ) + return + } + } + } + holder.registerProblem(statement, ACTUAL_IS_A_CONSTANT_MESSAGE) + } + } + } +} \ No newline at end of file diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/quickfixes/SwapActualAndExpectedExpressionMethodCallQuickFix.kt b/src/main/java/de/platon42/intellij/plugins/cajon/quickfixes/SwapActualAndExpectedExpressionMethodCallQuickFix.kt new file mode 100644 index 0000000..501c2dd --- /dev/null +++ b/src/main/java/de/platon42/intellij/plugins/cajon/quickfixes/SwapActualAndExpectedExpressionMethodCallQuickFix.kt @@ -0,0 +1,34 @@ +package de.platon42.intellij.plugins.cajon.quickfixes + +import com.intellij.codeInspection.ProblemDescriptor +import com.intellij.openapi.project.Project +import de.platon42.intellij.plugins.cajon.* + +class SwapActualAndExpectedExpressionMethodCallQuickFix( + description: String, + private val replacementMethod: String +) : AbstractCommonQuickFix(description) { + + companion object { + private const val SPLIT_EXPRESSION_DESCRIPTION = "Swap actual and expected expressions of assertions" + } + + override fun getFamilyName(): String { + return SPLIT_EXPRESSION_DESCRIPTION + } + + override fun applyFix(project: Project, descriptor: ProblemDescriptor) { + val assertThatMethodCall = descriptor.startElement.findStaticMethodCall() ?: return + + val methodToFix = assertThatMethodCall.collectMethodCallsUpToStatement() + .filterNot(NOT_ACTUAL_ASSERTIONS::test) + .first() + + val oldActualExpression = assertThatMethodCall.firstArg.copy()!! + assertThatMethodCall.firstArg.replace(methodToFix.firstArg) + + val expectedExpression = createExpectedMethodCall(methodToFix, replacementMethod, oldActualExpression) + expectedExpression.replaceQualifierFromMethodCall(methodToFix) + methodToFix.replace(expectedExpression) + } +} \ No newline at end of file diff --git a/src/main/resources/META-INF/plugin.xml b/src/main/resources/META-INF/plugin.xml index 8a200f8..25e1227 100644 --- a/src/main/resources/META-INF/plugin.xml +++ b/src/main/resources/META-INF/plugin.xml @@ -5,10 +5,11 @@ inspections and quick fixes to fully use the fluent assertion methods and thus makes the intention clear and concise, also generating better messages on test failures. - It can also be used to convert JUnit 4 assertions and assumptions to AssertJ. - It supports referencing inside extracting()-methods with strings, adding refactoring safety. + It can also be used to convert JUnit 4 assertions and assumptions to AssertJ. + It supports referencing inside extracting()-methods with strings, adding refactoring safety. + Bogus or twisted assertions are also reported.

Full documentation here... ]]> @@ -67,6 +68,11 @@ + + + diff --git a/src/main/resources/inspectionDescriptions/BogusAssertion.html b/src/main/resources/inspectionDescriptions/BogusAssertion.html new file mode 100644 index 0000000..3f26fbb --- /dev/null +++ b/src/main/resources/inspectionDescriptions/BogusAssertion.html @@ -0,0 +1,6 @@ + + +Finds typical copy and paste errors where the assertion will never fail, such as assertThat(foo).isEqualTo(foo), because actual +and expected expressions are the same. + + \ No newline at end of file diff --git a/src/main/resources/inspectionDescriptions/TwistedAssertion.html b/src/main/resources/inspectionDescriptions/TwistedAssertion.html new file mode 100644 index 0000000..b17802b --- /dev/null +++ b/src/main/resources/inspectionDescriptions/TwistedAssertion.html @@ -0,0 +1,7 @@ + + +Finds assertion method calls that have the expected and actual expressions twisted, such as assertThat(5).isEqualTo(foo). + +For some obvious cases, a quickfix to swap the actual and expected expressions is provided. + + \ No newline at end of file diff --git a/src/test/java/de/platon42/intellij/plugins/cajon/AbstractCajonTest.kt b/src/test/java/de/platon42/intellij/plugins/cajon/AbstractCajonTest.kt index d25561b..1a2eb50 100644 --- a/src/test/java/de/platon42/intellij/plugins/cajon/AbstractCajonTest.kt +++ b/src/test/java/de/platon42/intellij/plugins/cajon/AbstractCajonTest.kt @@ -38,6 +38,14 @@ abstract class AbstractCajonTest { return quickfixes } + protected fun assertHighlightings(myFixture: JavaCodeInsightTestFixture, count: Int, snippet: String) { + val highlights = myFixture.doHighlighting() + .asSequence() + .filter { it.description?.contains(snippet) ?: false } + .toList() + assertThat(highlights).hasSize(count); + } + class CutOffFixtureDisplayNameGenerator : DisplayNameGenerator.ReplaceUnderscores() { override fun generateDisplayNameForMethod(testClass: Class<*>?, testMethod: Method?): String { val nameForMethod = super.generateDisplayNameForMethod(testClass, testMethod) diff --git a/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatSizeInspectionTest.kt b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatSizeInspectionTest.kt index c730d3a..8fc0c29 100644 --- a/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatSizeInspectionTest.kt +++ b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatSizeInspectionTest.kt @@ -4,8 +4,6 @@ import com.intellij.testFramework.fixtures.JavaCodeInsightTestFixture import de.platon42.intellij.jupiter.MyFixture import de.platon42.intellij.jupiter.TestDataSubPath import de.platon42.intellij.plugins.cajon.AbstractCajonTest -import org.assertj.core.api.Assertions.assertThat -import org.assertj.core.api.extrakting import org.junit.jupiter.api.Test internal class AssertThatSizeInspectionTest : AbstractCajonTest() { @@ -15,7 +13,7 @@ internal class AssertThatSizeInspectionTest : AbstractCajonTest() { internal fun assertThat_size_of_array_collection_or_map_can_be_simplified(@MyFixture myFixture: JavaCodeInsightTestFixture) { myFixture.enableInspections(AssertThatSizeInspection::class.java) myFixture.configureByFile("SizeBefore.java") - assertThat(myFixture.doHighlighting()).extrakting { it.description }.containsOnlyOnce("Try to operate on the iterable itself rather than its size") + assertHighlightings(myFixture, 1, "Try to operate on the iterable itself rather than its size") executeQuickFixes(myFixture, Regex.fromLiteral("Replace isEqualTo() with isEmpty()"), 5) executeQuickFixes(myFixture, Regex.fromLiteral("Replace isZero() with isEmpty()"), 5) 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 new file mode 100644 index 0000000..d099dd5 --- /dev/null +++ b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/BogusAssertionInspectionTest.kt @@ -0,0 +1,18 @@ +package de.platon42.intellij.plugins.cajon.inspections + +import com.intellij.testFramework.fixtures.JavaCodeInsightTestFixture +import de.platon42.intellij.jupiter.MyFixture +import de.platon42.intellij.jupiter.TestDataSubPath +import de.platon42.intellij.plugins.cajon.AbstractCajonTest +import org.junit.jupiter.api.Test + +internal class BogusAssertionInspectionTest : AbstractCajonTest() { + + @Test + @TestDataSubPath("inspections/BogusAssertion") + internal fun reports_bogus_assertions(@MyFixture myFixture: JavaCodeInsightTestFixture) { + 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") + } +} \ No newline at end of file diff --git a/src/test/java/de/platon42/intellij/plugins/cajon/inspections/TwistedAssertionInspectionTest.kt b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/TwistedAssertionInspectionTest.kt new file mode 100644 index 0000000..c60628a --- /dev/null +++ b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/TwistedAssertionInspectionTest.kt @@ -0,0 +1,23 @@ +package de.platon42.intellij.plugins.cajon.inspections + +import com.intellij.testFramework.fixtures.JavaCodeInsightTestFixture +import de.platon42.intellij.jupiter.MyFixture +import de.platon42.intellij.jupiter.TestDataSubPath +import de.platon42.intellij.plugins.cajon.AbstractCajonTest +import org.junit.jupiter.api.Test + +internal class TwistedAssertionInspectionTest : AbstractCajonTest() { + + @Test + @TestDataSubPath("inspections/TwistedAssertion") + internal fun hint_twisted_actual_and_expected_and_provide_quickfix_where_possible(@MyFixture myFixture: JavaCodeInsightTestFixture) { + myFixture.enableInspections(TwistedAssertionInspection::class.java) + myFixture.configureByFile("TwistedAssertionBefore.java") + assertHighlightings(myFixture, 4, "Actual expression in assertThat() is a constant") + assertHighlightings(myFixture, 10, "Twisted actual and expected expressions") + + executeQuickFixes(myFixture, Regex.fromLiteral("Swap actual and expected expressions in assertion"), 6) + executeQuickFixesNoFamilyNameCheck(myFixture, Regex("Replace .* by .* and swap actual and expected expressions"), 4) + myFixture.checkResultByFile("TwistedAssertionAfter.java") + } +} \ No newline at end of file diff --git a/src/test/resources/inspections/BogusAssertion/BogusAssertionBefore.java b/src/test/resources/inspections/BogusAssertion/BogusAssertionBefore.java new file mode 100644 index 0000000..6980b31 --- /dev/null +++ b/src/test/resources/inspections/BogusAssertion/BogusAssertionBefore.java @@ -0,0 +1,196 @@ +import java.util.*; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.fail; + +public class BogusAssertions { + + private void bogusAssertions() { + boolean[] boolarray = new boolean[10]; + byte[] bytearray = new byte[10]; + short[] shortarray = new short[10]; + int[] intarray = new int[10]; + long[] longarray = new long[10]; + float[] floatarray = new float[10]; + double[] doublearray = new double[10]; + char[] chararray = new char[10]; + Object[] objarray = new Object[10]; + String string = "foo"; + List list = new ArrayList<>(); + Map map = new HashMap<>(); + String bar = "bar"; + + assertThat(boolarray).isEqualTo(boolarray); + assertThat(boolarray).isSameAs(boolarray); + assertThat(boolarray).hasSameClassAs(boolarray); + assertThat(boolarray).hasSameHashCodeAs(boolarray); + assertThat(boolarray).hasSameSizeAs(boolarray); + assertThat(boolarray).contains(boolarray); + assertThat(boolarray).containsAnyOf(boolarray); + assertThat(boolarray).containsExactly(boolarray); + assertThat(boolarray).containsExactlyInAnyOrder(boolarray); + assertThat(boolarray).containsOnly(boolarray); + assertThat(boolarray).containsSequence(boolarray); + assertThat(boolarray).containsSubsequence(boolarray); + assertThat(boolarray).startsWith(boolarray); + assertThat(boolarray).endsWith(boolarray); + + assertThat(bytearray).isEqualTo(bytearray); + assertThat(bytearray).isSameAs(bytearray); + assertThat(bytearray).hasSameClassAs(bytearray); + assertThat(bytearray).hasSameHashCodeAs(bytearray); + assertThat(bytearray).hasSameSizeAs(bytearray); + assertThat(bytearray).contains(bytearray); + assertThat(bytearray).containsAnyOf(bytearray); + assertThat(bytearray).containsExactly(bytearray); + assertThat(bytearray).containsExactlyInAnyOrder(bytearray); + assertThat(bytearray).containsOnly(bytearray); + assertThat(bytearray).containsSequence(bytearray); + assertThat(bytearray).containsSubsequence(bytearray); + assertThat(bytearray).startsWith(bytearray); + assertThat(bytearray).endsWith(bytearray); + + assertThat(shortarray).isEqualTo(shortarray); + assertThat(shortarray).isSameAs(shortarray); + assertThat(shortarray).hasSameClassAs(shortarray); + assertThat(shortarray).hasSameHashCodeAs(shortarray); + assertThat(shortarray).hasSameSizeAs(shortarray); + assertThat(shortarray).contains(shortarray); + assertThat(shortarray).containsAnyOf(shortarray); + assertThat(shortarray).containsExactly(shortarray); + assertThat(shortarray).containsExactlyInAnyOrder(shortarray); + assertThat(shortarray).containsOnly(shortarray); + assertThat(shortarray).containsSequence(shortarray); + assertThat(shortarray).containsSubsequence(shortarray); + assertThat(shortarray).startsWith(shortarray); + assertThat(shortarray).endsWith(shortarray); + + assertThat(intarray).isEqualTo(intarray); + assertThat(intarray).isSameAs(intarray); + assertThat(intarray).hasSameClassAs(intarray); + assertThat(intarray).hasSameHashCodeAs(intarray); + assertThat(intarray).hasSameSizeAs(intarray); + assertThat(intarray).contains(intarray); + assertThat(intarray).containsAnyOf(intarray); + assertThat(intarray).containsExactly(intarray); + assertThat(intarray).containsExactlyInAnyOrder(intarray); + assertThat(intarray).containsOnly(intarray); + assertThat(intarray).containsSequence(intarray); + assertThat(intarray).containsSubsequence(intarray); + assertThat(intarray).startsWith(intarray); + assertThat(intarray).endsWith(intarray); + + assertThat(longarray).isEqualTo(longarray); + assertThat(longarray).isSameAs(longarray); + assertThat(longarray).hasSameClassAs(longarray); + assertThat(longarray).hasSameHashCodeAs(longarray); + assertThat(longarray).hasSameSizeAs(longarray); + assertThat(longarray).contains(longarray); + assertThat(longarray).containsAnyOf(longarray); + assertThat(longarray).containsExactly(longarray); + assertThat(longarray).containsExactlyInAnyOrder(longarray); + assertThat(longarray).containsOnly(longarray); + assertThat(longarray).containsSequence(longarray); + assertThat(longarray).containsSubsequence(longarray); + assertThat(longarray).startsWith(longarray); + assertThat(longarray).endsWith(longarray); + + assertThat(floatarray).isEqualTo(floatarray); + assertThat(floatarray).isSameAs(floatarray); + assertThat(floatarray).hasSameClassAs(floatarray); + assertThat(floatarray).hasSameHashCodeAs(floatarray); + assertThat(floatarray).hasSameSizeAs(floatarray); + assertThat(floatarray).contains(floatarray); + assertThat(floatarray).containsAnyOf(floatarray); + assertThat(floatarray).containsExactly(floatarray); + assertThat(floatarray).containsExactlyInAnyOrder(floatarray); + assertThat(floatarray).containsOnly(floatarray); + assertThat(floatarray).containsSequence(floatarray); + assertThat(floatarray).containsSubsequence(floatarray); + assertThat(floatarray).startsWith(floatarray); + assertThat(floatarray).endsWith(floatarray); + + assertThat(doublearray).isEqualTo(doublearray); + assertThat(doublearray).isSameAs(doublearray); + assertThat(doublearray).hasSameClassAs(doublearray); + assertThat(doublearray).hasSameHashCodeAs(doublearray); + assertThat(doublearray).hasSameSizeAs(doublearray); + assertThat(doublearray).contains(doublearray); + assertThat(doublearray).containsAnyOf(doublearray); + assertThat(doublearray).containsExactly(doublearray); + assertThat(doublearray).containsExactlyInAnyOrder(doublearray); + assertThat(doublearray).containsOnly(doublearray); + assertThat(doublearray).containsSequence(doublearray); + assertThat(doublearray).containsSubsequence(doublearray); + assertThat(doublearray).startsWith(doublearray); + assertThat(doublearray).endsWith(doublearray); + + assertThat(chararray).isEqualTo(chararray); + assertThat(chararray).isSameAs(chararray); + assertThat(chararray).hasSameClassAs(chararray); + assertThat(chararray).hasSameHashCodeAs(chararray); + assertThat(chararray).hasSameSizeAs(chararray); + assertThat(chararray).contains(chararray); + assertThat(chararray).containsAnyOf(chararray); + assertThat(chararray).containsExactly(chararray); + assertThat(chararray).containsExactlyInAnyOrder(chararray); + assertThat(chararray).containsOnly(chararray); + assertThat(chararray).containsSequence(chararray); + assertThat(chararray).containsSubsequence(chararray); + assertThat(chararray).startsWith(chararray); + assertThat(chararray).endsWith(chararray); + + assertThat(objarray).isEqualTo(objarray); + assertThat(objarray).isSameAs(objarray); + assertThat(objarray).hasSameClassAs(objarray); + assertThat(objarray).hasSameHashCodeAs(objarray); + assertThat(objarray).hasSameSizeAs(objarray); + assertThat(objarray).contains(objarray); + assertThat(objarray).containsAnyOf(objarray); + assertThat(objarray).containsExactly(objarray); + assertThat(objarray).containsExactlyInAnyOrder(objarray); + assertThat(objarray).containsOnly(objarray); + assertThat(objarray).containsSequence(objarray); + assertThat(objarray).containsSubsequence(objarray); + assertThat(objarray).startsWith(objarray); + assertThat(objarray).endsWith(objarray); + + assertThat(string).as("foo").isEqualTo(string); + assertThat(string).as("foo").isSameAs(string); + assertThat(string).as("foo").hasSameClassAs(string); + assertThat(string).as("foo").hasSameHashCodeAs(string); + assertThat(string).as("foo").hasSameSizeAs(string); + assertThat(string).as("foo").isEqualToIgnoringCase(string); + assertThat(string).as("foo").containsSequence(string); + assertThat(string).as("foo").containsSubsequence(string); + assertThat(string).as("foo").startsWith(string); + assertThat(string).as("foo").endsWith(string); + + assertThat(list).as("foo").isEqualTo(list); + assertThat(list).as("foo").isSameAs(list); + assertThat(list).as("foo").hasSameClassAs(list); + assertThat(list).as("foo").hasSameHashCodeAs(list); + assertThat(list).as("foo").hasSameSizeAs(list); + assertThat(list).as("foo").containsAll(list); + assertThat(list).as("foo").containsAnyElementsOf(list); + assertThat(list).as("foo").containsOnlyElementsOf(list); + assertThat(list).as("foo").containsExactlyElementsOf(list); + assertThat(list).as("foo").hasSameElementsAs(list); + assertThat(list).as("foo").containsSequence(list); + assertThat(list).as("foo").containsSubsequence(list); + + assertThat(map).as("foo").isEqualTo(map); + assertThat(map).as("foo").isSameAs(map); + assertThat(map).as("foo").hasSameClassAs(map); + assertThat(map).as("foo").hasSameHashCodeAs(map); + assertThat(map).as("foo").hasSameSizeAs(map); + assertThat(map).as("foo").containsAllEntriesOf(map); + assertThat(map).as("foo").containsExactlyEntriesOf(map); + assertThat(map).as("foo").containsExactlyInAnyOrderEntriesOf(map); + + assertThat(bar).isEqualTo(string); + + org.junit.Assert.assertThat(list, null); + fail("oh no!"); + } +} diff --git a/src/test/resources/inspections/TwistedAssertion/TwistedAssertionAfter.java b/src/test/resources/inspections/TwistedAssertion/TwistedAssertionAfter.java new file mode 100644 index 0000000..21c7b0d --- /dev/null +++ b/src/test/resources/inspections/TwistedAssertion/TwistedAssertionAfter.java @@ -0,0 +1,38 @@ +import java.util.*; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.fail; + +public class TwistedAssertions { + + private static final int SOME_CONST = 10; + private static final String SOME_CONST_STRING = "bar"; + + private void twistedAssertions() { + List list = new ArrayList<>(); + String foo = "foo"; + String bar = "bar"; + int number = 4; + + assertThat(number).as("foo").isEqualTo(5 + 2); + assertThat(number).as("foo").isNotEqualTo(8); + assertThat(number).as("foo").isLessThanOrEqualTo(5 * 2); + assertThat(number + 1).as("foo").isLessThan(4 + (1 - 2)); + assertThat(number * 2).as("foo").isGreaterThanOrEqualTo(3); + assertThat(number / 2).as("foo").isGreaterThan(2 + SOME_CONST); + assertThat(foo).as("foo").isEqualTo("foo"); + assertThat(foo).as("foo").isSameAs(SOME_CONST_STRING); + assertThat(foo).as("foo").isNotEqualTo("bar"); + assertThat(foo).as("foo").isNotSameAs("bar"); + assertThat("bar").as("foo").startsWith(foo); + assertThat("foo").as("foo").endsWith(foo); + + assertThat(bar).isEqualTo(foo); + + assertThat(4).isEqualTo(number).isNotEqualTo(number * 2); + assertThat(4).usingComparator(Comparator.reverseOrder()).isGreaterThanOrEqualTo(number); + + org.junit.Assert.assertThat(list, null); + fail("oh no!"); + } +} diff --git a/src/test/resources/inspections/TwistedAssertion/TwistedAssertionBefore.java b/src/test/resources/inspections/TwistedAssertion/TwistedAssertionBefore.java new file mode 100644 index 0000000..3788558 --- /dev/null +++ b/src/test/resources/inspections/TwistedAssertion/TwistedAssertionBefore.java @@ -0,0 +1,38 @@ +import java.util.*; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.fail; + +public class TwistedAssertions { + + private static final int SOME_CONST = 10; + private static final String SOME_CONST_STRING = "bar"; + + private void twistedAssertions() { + List list = new ArrayList<>(); + String foo = "foo"; + String bar = "bar"; + int number = 4; + + assertThat(5 + 2).as("foo").isEqualTo(number); + assertThat(8).as("foo").isNotEqualTo(number); + assertThat(5 * 2).as("foo").isGreaterThan(number); + assertThat(4 + (1 - 2)).as("foo").isGreaterThanOrEqualTo(number + 1); + assertThat(3).as("foo").isLessThan(number * 2); + assertThat(2 + SOME_CONST).as("foo").isLessThanOrEqualTo(number / 2); + assertThat("foo").as("foo").isEqualTo(foo); + assertThat(SOME_CONST_STRING).as("foo").isSameAs(foo); + assertThat("bar").as("foo").isNotEqualTo(foo); + assertThat("bar").as("foo").isNotSameAs(foo); + assertThat("bar").as("foo").startsWith(foo); + assertThat("foo").as("foo").endsWith(foo); + + assertThat(bar).isEqualTo(foo); + + assertThat(4).isEqualTo(number).isNotEqualTo(number * 2); + assertThat(4).usingComparator(Comparator.reverseOrder()).isGreaterThanOrEqualTo(number); + + org.junit.Assert.assertThat(list, null); + fail("oh no!"); + } +}