From 8133f3850f060225ea7af3eb8de2462ddbcc9eac Mon Sep 17 00:00:00 2001 From: chrisly42 Date: Sun, 17 Nov 2019 19:10:41 +0100 Subject: [PATCH] Added first version of AssertThatPathExpression for a limited number transformations (more stuff is possible, but requires detection and transformation of static Files-methods). --- README.md | 28 ++++ build.gradle | 2 + .../AssertThatPathExpressionInspection.kt | 51 +++++++ src/main/resources/META-INF/plugin.xml | 2 + .../AssertThatPathExpression.html | 7 + .../AssertThatPathExpressionInspectionTest.kt | 24 ++++ .../PathExpression/PathExpressionAfter.java | 126 ++++++++++++++++++ .../PathExpression/PathExpressionBefore.java | 126 ++++++++++++++++++ 8 files changed, 366 insertions(+) create mode 100644 src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatPathExpressionInspection.kt create mode 100644 src/main/resources/inspectionDescriptions/AssertThatPathExpression.html create mode 100644 src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatPathExpressionInspectionTest.kt create mode 100644 src/test/resources/inspections/PathExpression/PathExpressionAfter.java create mode 100644 src/test/resources/inspections/PathExpression/PathExpressionBefore.java diff --git a/README.md b/README.md index 0865711..f42a673 100644 --- a/README.md +++ b/README.md @@ -333,6 +333,32 @@ You can toggle the various inspections in the Settings/Editor/Inspections in the to: assertThat(file).isNotEmptyDirectory(); ``` +- AssertThatPathExpression + + Moves ```Path``` method calls inside ```assertThat()``` out. + Note: Uses hasParentRaw() instead of hasParent() for quickfixes, because it is semantically + equivalent. For most cases though, hasParent() will show identical behavior. + + ``` + from: assertThat(path.isAbsolute()).isTrue(); + to: assertThat(path).isAbsolute(); + + from: assertThat(path.isAbsolute()).isFalse(); + to: assertThat(path).isRelative(); + + from: assertThat(path.getParent()).isEqualTo(pathname); + to: assertThat(path).hasParentRaw(pathname); + + from: assertThat(path.getParent()).isNull(); + to: assertThat(path).hasNoParentRaw(); + + from: assertThat(path.startsWith(otherPath)).isTrue(); + to: assertThat(path).startsWithRaw(otherPath); + + from: assertThat(path.endsWith(otherPath)).isTrue(); + to: assertThat(path).endsWithRaw(otherPath); + ``` + - AssertThatEnumerableIsEmpty Uses ```isEmpty()``` for ```hasSize(0)``` iterable assertions instead. @@ -644,6 +670,8 @@ Feel free to use the code (in package ```de.platon42.intellij.jupiter```) for yo #### V1.7 (unreleased) - Fixed a lapsuus in AssertThatFileExpression also transforming ```.listFiles()``` with a filter argument. +- Added first version of AssertThatPathExpression for a limited number transformations (more stuff is possible, + but requires detection and transformation of static ```Files```-methods). #### V1.6 (30-Sep-19) - Really fixed AssertThatGuavaOptional inspections to avoid conversions from ```.get()``` to ```.contains()``` diff --git a/build.gradle b/build.gradle index 1cb6205..7e28525 100644 --- a/build.gradle +++ b/build.gradle @@ -46,6 +46,8 @@ patchPluginXml {

V1.7 (unreleased)

Full changelog available at Github project site.

""" diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatPathExpressionInspection.kt b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatPathExpressionInspection.kt new file mode 100644 index 0000000..24aabbd --- /dev/null +++ b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatPathExpressionInspection.kt @@ -0,0 +1,51 @@ +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.siyeh.ig.callMatcher.CallMatcher + + +class AssertThatPathExpressionInspection : AbstractMoveOutInspection() { + + companion object { + private const val DISPLAY_NAME = "Asserting a path specific expression" + private const val JAVA_NIO_PATH = "java.nio.file.Path" + + private val MAPPINGS = listOf( + MoveOutMapping( + CallMatcher.instanceCall(JAVA_NIO_PATH, "isAbsolute").parameterCount(0), + "isAbsolute", "isRelative", expectBoolean = true + ), + MoveOutMapping( + CallMatcher.instanceCall(JAVA_NIO_PATH, "startsWith").parameterTypes(JAVA_NIO_PATH), + "startsWithRaw", expectBoolean = true + ), + MoveOutMapping( + CallMatcher.instanceCall(JAVA_NIO_PATH, "endsWith").parameterTypes(JAVA_NIO_PATH), + "endsWithRaw", expectBoolean = true + ), + MoveOutMapping( + CallMatcher.instanceCall(JAVA_NIO_PATH, "getParent").parameterCount(0), + "hasParentRaw", + expectedMatcher = IS_EQUAL_TO_OBJECT + ), + MoveOutMapping( + CallMatcher.instanceCall(JAVA_NIO_PATH, "getParent").parameterCount(0), + "hasNoParentRaw", expectNullNonNull = true + ) + ) + } + + override fun getDisplayName() = DISPLAY_NAME + + override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor { + return object : JavaElementVisitor() { + override fun visitExpressionStatement(statement: PsiExpressionStatement) { + super.visitExpressionStatement(statement) + createInspectionsForMappings(statement, holder, MAPPINGS) + } + } + } +} \ 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 f53d42a..76d686c 100644 --- a/src/main/resources/META-INF/plugin.xml +++ b/src/main/resources/META-INF/plugin.xml @@ -47,6 +47,8 @@ implementationClass="de.platon42.intellij.plugins.cajon.inspections.AssertThatCollectionOrMapExpressionInspection"/> + diff --git a/src/main/resources/inspectionDescriptions/AssertThatPathExpression.html b/src/main/resources/inspectionDescriptions/AssertThatPathExpression.html new file mode 100644 index 0000000..4db3e80 --- /dev/null +++ b/src/main/resources/inspectionDescriptions/AssertThatPathExpression.html @@ -0,0 +1,7 @@ + + +Operates on assertions on objects of type Path. Turns assertThat(file.someMethod(arg)).someAssertion() into assertThat(path).someMethod(arg). + +
someMethod() can be isAbsolute(), getParent(), startsWith() and endsWith(). + + \ No newline at end of file diff --git a/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatPathExpressionInspectionTest.kt b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatPathExpressionInspectionTest.kt new file mode 100644 index 0000000..2a9e295 --- /dev/null +++ b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatPathExpressionInspectionTest.kt @@ -0,0 +1,24 @@ +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 AssertThatPathExpressionInspectionTest : AbstractCajonTest() { + + @Test + @TestDataSubPath("inspections/PathExpression") + internal fun assertThat_with_certain_Path_methods(@MyFixture myFixture: JavaCodeInsightTestFixture) { + myFixture.enableInspections(AssertThatPathExpressionInspection::class.java) + myFixture.configureByFile("PathExpressionBefore.java") + executeQuickFixes(myFixture, Regex.fromLiteral("Remove isAbsolute() of actual expression and use assertThat().isAbsolute() instead"), 3) + executeQuickFixes(myFixture, Regex.fromLiteral("Remove isAbsolute() of actual expression and use assertThat().isRelative() instead"), 3) + executeQuickFixes(myFixture, Regex.fromLiteral("Remove startsWith() of actual expression and use assertThat().startsWithRaw() instead"), 3) + executeQuickFixes(myFixture, Regex.fromLiteral("Remove endsWith() of actual expression and use assertThat().endsWithRaw() instead"), 3) + executeQuickFixes(myFixture, Regex.fromLiteral("Remove getParent() of actual expression and use assertThat().hasNoParentRaw() instead"), 2) + executeQuickFixes(myFixture, Regex.fromLiteral("Remove getParent() of actual expression and use assertThat().hasParentRaw() instead"), 1) + myFixture.checkResultByFile("PathExpressionAfter.java") + } +} \ No newline at end of file diff --git a/src/test/resources/inspections/PathExpression/PathExpressionAfter.java b/src/test/resources/inspections/PathExpression/PathExpressionAfter.java new file mode 100644 index 0000000..2b68857 --- /dev/null +++ b/src/test/resources/inspections/PathExpression/PathExpressionAfter.java @@ -0,0 +1,126 @@ +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.fail; + +public class PathExpression { + + private void pathExpression() { + Path path = Paths.get("foo"); + Path otherPath = Paths.get("bar"); + + assertThat(path).as("foo").isAbsolute(); + assertThat(path).isAbsolute(); + assertThat(path).isAbsolute(); + assertThat(path).as("foo").isRelative(); + assertThat(path).isRelative(); + assertThat(path).isRelative(); + + assertThat(path).hasParentRaw(otherPath); + assertThat(path.getParent()).isNotEqualTo(otherPath); + assertThat(path).hasNoParentRaw(); + assertThat(path).hasNoParentRaw(); + assertThat(path.getParent()).isNotEqualTo(null); + assertThat(path.getParent()).isNotNull(); + + assertThat(path).as("foo").startsWithRaw(otherPath); + assertThat(path).startsWithRaw(otherPath); + assertThat(path).startsWithRaw(otherPath); + assertThat(path.startsWith(otherPath)).as("foo").isEqualTo(false); + assertThat(path.startsWith(otherPath)).isNotEqualTo(true); + assertThat(path.startsWith(otherPath)).isFalse(); + + assertThat(path.startsWith("otherPath")).as("foo").isEqualTo(true); + assertThat(path.startsWith("otherPath")).isNotEqualTo(false); + assertThat(path.startsWith("otherPath")).isTrue(); + assertThat(path.startsWith("otherPath")).as("foo").isEqualTo(false); + assertThat(path.startsWith("otherPath")).isNotEqualTo(true); + assertThat(path.startsWith("otherPath")).isFalse(); + + assertThat(path).as("foo").endsWithRaw(otherPath); + assertThat(path).endsWithRaw(otherPath); + assertThat(path).endsWithRaw(otherPath); + assertThat(path.endsWith(otherPath)).as("foo").isEqualTo(false); + assertThat(path.endsWith(otherPath)).isNotEqualTo(true); + assertThat(path.endsWith(otherPath)).isFalse(); + + assertThat(path.endsWith("otherPath")).as("foo").isEqualTo(true); + assertThat(path.endsWith("otherPath")).isNotEqualTo(false); + assertThat(path.endsWith("otherPath")).isTrue(); + assertThat(path.endsWith("otherPath")).as("foo").isEqualTo(false); + assertThat(path.endsWith("otherPath")).isNotEqualTo(true); + assertThat(path.endsWith("otherPath")).isFalse(); + + assertThat(Files.isReadable(path)).as("foo").isEqualTo(true); + assertThat(Files.isReadable(path)).isNotEqualTo(false); + assertThat(Files.isReadable(path)).isTrue(); + assertThat(Files.isReadable(path)).as("foo").isEqualTo(false); + assertThat(Files.isReadable(path)).isNotEqualTo(true); + assertThat(Files.isReadable(path)).isFalse(); + + assertThat(Files.isWritable(path)).as("foo").isEqualTo(true); + assertThat(Files.isWritable(path)).isNotEqualTo(false); + assertThat(Files.isWritable(path)).isTrue(); + assertThat(Files.isWritable(path)).as("foo").isEqualTo(false); + assertThat(Files.isWritable(path)).isNotEqualTo(true); + assertThat(Files.isWritable(path)).isFalse(); + + assertThat(Files.isExecutable(path)).as("foo").isEqualTo(true); + assertThat(Files.isExecutable(path)).isNotEqualTo(false); + assertThat(Files.isExecutable(path)).isTrue(); + assertThat(Files.isExecutable(path)).as("foo").isEqualTo(false); + assertThat(Files.isExecutable(path)).isNotEqualTo(true); + assertThat(Files.isExecutable(path)).isFalse(); + + assertThat(Files.isDirectory(path)).as("foo").isEqualTo(true); + assertThat(Files.isDirectory(path)).isNotEqualTo(false); + assertThat(Files.isDirectory(path)).isTrue(); + assertThat(Files.isDirectory(path)).as("foo").isEqualTo(false); + assertThat(Files.isDirectory(path)).isNotEqualTo(true); + assertThat(Files.isDirectory(path)).isFalse(); + + assertThat(Files.isRegularFile(path)).as("foo").isEqualTo(true); + assertThat(Files.isRegularFile(path)).isNotEqualTo(false); + assertThat(Files.isRegularFile(path)).isTrue(); + assertThat(Files.isRegularFile(path)).as("foo").isEqualTo(false); + assertThat(Files.isRegularFile(path)).isNotEqualTo(true); + assertThat(Files.isRegularFile(path)).isFalse(); + + assertThat(Files.isSymbolicLink(path)).as("foo").isEqualTo(true); + assertThat(Files.isSymbolicLink(path)).isNotEqualTo(false); + assertThat(Files.isSymbolicLink(path)).isTrue(); + assertThat(Files.isSymbolicLink(path)).as("foo").isEqualTo(false); + assertThat(Files.isSymbolicLink(path)).isNotEqualTo(true); + assertThat(Files.isSymbolicLink(path)).isFalse(); + + assertThat(Files.exists(path)).as("foo").isEqualTo(true); + assertThat(Files.exists(path)).isNotEqualTo(false); + assertThat(Files.exists(path)).isTrue(); + assertThat(Files.exists(path)).as("foo").isEqualTo(false); + assertThat(Files.exists(path)).isNotEqualTo(true); + assertThat(Files.exists(path)).isFalse(); + + assertThat(Files.notExists(path)).as("foo").isEqualTo(true); + assertThat(Files.notExists(path)).isNotEqualTo(false); + assertThat(Files.notExists(path)).isTrue(); + assertThat(Files.notExists(path)).as("foo").isEqualTo(false); + assertThat(Files.notExists(path)).isNotEqualTo(true); + assertThat(Files.notExists(path)).isFalse(); + + assertThat(Files.list(path)).isEmpty(); + assertThat(Files.list(path)).isNotEmpty(); + + assertThat(Files.readAllBytes(path)).isEqualTo(new byte[1]); + assertThat(Files.readAllLines(path)).containsExactly("foo"); + assertThat(Files.lines(path)).containsExactly("foo"); + + assertThat(path.getName()).endsWith(".foo"); // could be turned into .hasExtension("foo"), but not always. + + assertThat(path.getName()).as("foo").isEqualTo("foo").as("bar").isEqualTo("bar"); + + org.junit.Assert.assertThat(path, null); + fail("oh no!"); + } +} diff --git a/src/test/resources/inspections/PathExpression/PathExpressionBefore.java b/src/test/resources/inspections/PathExpression/PathExpressionBefore.java new file mode 100644 index 0000000..b5fe378 --- /dev/null +++ b/src/test/resources/inspections/PathExpression/PathExpressionBefore.java @@ -0,0 +1,126 @@ +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.fail; + +public class PathExpression { + + private void pathExpression() { + Path path = Paths.get("foo"); + Path otherPath = Paths.get("bar"); + + assertThat(path.isAbsolute()).as("foo").isEqualTo(true); + assertThat(path.isAbsolute()).isNotEqualTo(false); + assertThat(path.isAbsolute()).isTrue(); + assertThat(path.isAbsolute()).as("foo").isEqualTo(false); + assertThat(path.isAbsolute()).isNotEqualTo(true); + assertThat(path.isAbsolute()).isFalse(); + + assertThat(path.getParent()).isEqualTo(otherPath); + assertThat(path.getParent()).isNotEqualTo(otherPath); + assertThat(path.getParent()).isEqualTo(null); + assertThat(path.getParent()).isNull(); + assertThat(path.getParent()).isNotEqualTo(null); + assertThat(path.getParent()).isNotNull(); + + assertThat(path.startsWith(otherPath)).as("foo").isEqualTo(true); + assertThat(path.startsWith(otherPath)).isNotEqualTo(false); + assertThat(path.startsWith(otherPath)).isTrue(); + assertThat(path.startsWith(otherPath)).as("foo").isEqualTo(false); + assertThat(path.startsWith(otherPath)).isNotEqualTo(true); + assertThat(path.startsWith(otherPath)).isFalse(); + + assertThat(path.startsWith("otherPath")).as("foo").isEqualTo(true); + assertThat(path.startsWith("otherPath")).isNotEqualTo(false); + assertThat(path.startsWith("otherPath")).isTrue(); + assertThat(path.startsWith("otherPath")).as("foo").isEqualTo(false); + assertThat(path.startsWith("otherPath")).isNotEqualTo(true); + assertThat(path.startsWith("otherPath")).isFalse(); + + assertThat(path.endsWith(otherPath)).as("foo").isEqualTo(true); + assertThat(path.endsWith(otherPath)).isNotEqualTo(false); + assertThat(path.endsWith(otherPath)).isTrue(); + assertThat(path.endsWith(otherPath)).as("foo").isEqualTo(false); + assertThat(path.endsWith(otherPath)).isNotEqualTo(true); + assertThat(path.endsWith(otherPath)).isFalse(); + + assertThat(path.endsWith("otherPath")).as("foo").isEqualTo(true); + assertThat(path.endsWith("otherPath")).isNotEqualTo(false); + assertThat(path.endsWith("otherPath")).isTrue(); + assertThat(path.endsWith("otherPath")).as("foo").isEqualTo(false); + assertThat(path.endsWith("otherPath")).isNotEqualTo(true); + assertThat(path.endsWith("otherPath")).isFalse(); + + assertThat(Files.isReadable(path)).as("foo").isEqualTo(true); + assertThat(Files.isReadable(path)).isNotEqualTo(false); + assertThat(Files.isReadable(path)).isTrue(); + assertThat(Files.isReadable(path)).as("foo").isEqualTo(false); + assertThat(Files.isReadable(path)).isNotEqualTo(true); + assertThat(Files.isReadable(path)).isFalse(); + + assertThat(Files.isWritable(path)).as("foo").isEqualTo(true); + assertThat(Files.isWritable(path)).isNotEqualTo(false); + assertThat(Files.isWritable(path)).isTrue(); + assertThat(Files.isWritable(path)).as("foo").isEqualTo(false); + assertThat(Files.isWritable(path)).isNotEqualTo(true); + assertThat(Files.isWritable(path)).isFalse(); + + assertThat(Files.isExecutable(path)).as("foo").isEqualTo(true); + assertThat(Files.isExecutable(path)).isNotEqualTo(false); + assertThat(Files.isExecutable(path)).isTrue(); + assertThat(Files.isExecutable(path)).as("foo").isEqualTo(false); + assertThat(Files.isExecutable(path)).isNotEqualTo(true); + assertThat(Files.isExecutable(path)).isFalse(); + + assertThat(Files.isDirectory(path)).as("foo").isEqualTo(true); + assertThat(Files.isDirectory(path)).isNotEqualTo(false); + assertThat(Files.isDirectory(path)).isTrue(); + assertThat(Files.isDirectory(path)).as("foo").isEqualTo(false); + assertThat(Files.isDirectory(path)).isNotEqualTo(true); + assertThat(Files.isDirectory(path)).isFalse(); + + assertThat(Files.isRegularFile(path)).as("foo").isEqualTo(true); + assertThat(Files.isRegularFile(path)).isNotEqualTo(false); + assertThat(Files.isRegularFile(path)).isTrue(); + assertThat(Files.isRegularFile(path)).as("foo").isEqualTo(false); + assertThat(Files.isRegularFile(path)).isNotEqualTo(true); + assertThat(Files.isRegularFile(path)).isFalse(); + + assertThat(Files.isSymbolicLink(path)).as("foo").isEqualTo(true); + assertThat(Files.isSymbolicLink(path)).isNotEqualTo(false); + assertThat(Files.isSymbolicLink(path)).isTrue(); + assertThat(Files.isSymbolicLink(path)).as("foo").isEqualTo(false); + assertThat(Files.isSymbolicLink(path)).isNotEqualTo(true); + assertThat(Files.isSymbolicLink(path)).isFalse(); + + assertThat(Files.exists(path)).as("foo").isEqualTo(true); + assertThat(Files.exists(path)).isNotEqualTo(false); + assertThat(Files.exists(path)).isTrue(); + assertThat(Files.exists(path)).as("foo").isEqualTo(false); + assertThat(Files.exists(path)).isNotEqualTo(true); + assertThat(Files.exists(path)).isFalse(); + + assertThat(Files.notExists(path)).as("foo").isEqualTo(true); + assertThat(Files.notExists(path)).isNotEqualTo(false); + assertThat(Files.notExists(path)).isTrue(); + assertThat(Files.notExists(path)).as("foo").isEqualTo(false); + assertThat(Files.notExists(path)).isNotEqualTo(true); + assertThat(Files.notExists(path)).isFalse(); + + assertThat(Files.list(path)).isEmpty(); + assertThat(Files.list(path)).isNotEmpty(); + + assertThat(Files.readAllBytes(path)).isEqualTo(new byte[1]); + assertThat(Files.readAllLines(path)).containsExactly("foo"); + assertThat(Files.lines(path)).containsExactly("foo"); + + assertThat(path.getName()).endsWith(".foo"); // could be turned into .hasExtension("foo"), but not always. + + assertThat(path.getName()).as("foo").isEqualTo("foo").as("bar").isEqualTo("bar"); + + org.junit.Assert.assertThat(path, null); + fail("oh no!"); + } +}