Fixed PreviewAnnotationRule (#1740)

- moved creating private node to ASTFactory - added a new node to ANNOTATION_LIST
saveourtool · Sep 21, 2023 · dd5542b · dd5542b
1 parent 11ea0aa
commit dd5542b
Show file tree

Hide file tree

Showing 6 changed files with 51 additions and 30 deletions.
diff --git a/...es/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/PreviewAnnotationRule.kt b/...es/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/PreviewAnnotationRule.kt
@@ -11,14 +11,16 @@ import com.saveourtool.diktat.ruleset.utils.getIdentifierName
 import org.jetbrains.kotlin.KtNodeTypes.ANNOTATION_ENTRY
 import org.jetbrains.kotlin.KtNodeTypes.FUN
 import org.jetbrains.kotlin.KtNodeTypes.MODIFIER_LIST
+import org.jetbrains.kotlin.com.intellij.lang.ASTFactory
 import org.jetbrains.kotlin.com.intellij.lang.ASTNode
-import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.PsiWhiteSpaceImpl
 import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.lexer.KtTokens.ABSTRACT_KEYWORD
 import org.jetbrains.kotlin.lexer.KtTokens.INTERNAL_KEYWORD
 import org.jetbrains.kotlin.lexer.KtTokens.OPEN_KEYWORD
+import org.jetbrains.kotlin.lexer.KtTokens.PRIVATE_KEYWORD
 import org.jetbrains.kotlin.lexer.KtTokens.PROTECTED_KEYWORD
 import org.jetbrains.kotlin.lexer.KtTokens.PUBLIC_KEYWORD
+import org.jetbrains.kotlin.lexer.KtTokens.WHITE_SPACE
 import org.jetbrains.kotlin.psi.KtNamedFunction
 import org.jetbrains.kotlin.psi.psiUtil.isPrivate
 
@@ -58,7 +60,7 @@ class PreviewAnnotationRule(configRules: List<RulesConfig>) : DiktatRule(
             val functionName = functionNameNode?.text ?: return
 
             // warn if function is not private
-            if (!((functionNode.psi as KtNamedFunction).isPrivate())) {
+            if (!(functionNode.psi as KtNamedFunction).isPrivate()) {
                 PREVIEW_ANNOTATION.warnAndFix(
                     configRules,
                     emitWarn,
@@ -94,22 +96,23 @@ class PreviewAnnotationRule(configRules: List<RulesConfig>) : DiktatRule(
         functionName.contains(PREVIEW_ANNOTATION_TEXT)
 
     private fun addPrivateModifier(functionNode: ASTNode) {
-        val modifiersList = functionNode
-            .findChildByType(MODIFIER_LIST)
-            ?.getChildren(KtTokens.MODIFIER_KEYWORDS)
-            ?.toList()
+        // MODIFIER_LIST should be present since ANNOTATION_ENTRY is there
+        val modifierListNode = functionNode.findChildByType(MODIFIER_LIST) ?: return
+        val modifiersList = modifierListNode
+            .getChildren(KtTokens.MODIFIER_KEYWORDS)
+            .toList()
 
-        val isMethodAbstract = modifiersList?.any {
+        val isMethodAbstract = modifiersList.any {
             it.elementType == ABSTRACT_KEYWORD
         }
 
         // private modifier is not applicable for abstract methods
-        if (isMethodAbstract == true) {
+        if (isMethodAbstract) {
             return
         }
 
         // these modifiers could be safely replaced via `private`
-        val modifierForReplacement = modifiersList?.firstOrNull {
+        val modifierForReplacement = modifiersList.firstOrNull {
             it.elementType in listOf(
                 PUBLIC_KEYWORD, PROTECTED_KEYWORD, INTERNAL_KEYWORD, OPEN_KEYWORD
             )
@@ -118,19 +121,21 @@ class PreviewAnnotationRule(configRules: List<RulesConfig>) : DiktatRule(
         modifierForReplacement?.let {
             // replace current modifier with `private`
             val parent = it.treeParent
-            parent.replaceChild(it, createPrivateModifierNode()
-            )
+            parent.replaceChild(it, createPrivateModifierNode())
         } ?: run {
             // the case, when there is no explicit modifier, i.e. `fun foo`
-            // just add `private` before function identifier `fun`
+            // just add `private` in MODIFIER_LIST at the end
+            // and move WHITE_SPACE before function identifier `fun` to MODIFIER_LIST
             val funNode = functionNode.findAllNodesWithCondition { it.text == "fun" }.single()
-            // add `private ` nodes before `fun`
-            funNode.treeParent?.addChild(PsiWhiteSpaceImpl(" "), funNode)
-            funNode.treeParent?.addChild(createPrivateModifierNode(), funNode.treePrev)
+            val whiteSpaceAfterAnnotation = modifierListNode.treeNext
+            modifierListNode.addChild(whiteSpaceAfterAnnotation, null)
+            modifierListNode.addChild(createPrivateModifierNode(), null)
+            // add ` ` node before `fun`
+            functionNode.addChild(ASTFactory.leaf(WHITE_SPACE, " "), funNode)
         }
     }
 
-    private fun createPrivateModifierNode() = KotlinParser().createNode("private")
+    private fun createPrivateModifierNode() = ASTFactory.leaf(PRIVATE_KEYWORD, "private")
 
     companion object {
         const val ANNOTATION_SYMBOL = "@"

diff --git a/...rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/PreviewAnnotationFixTest.kt b/...rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/PreviewAnnotationFixTest.kt
@@ -4,14 +4,12 @@ import com.saveourtool.diktat.ruleset.rules.chapter3.PreviewAnnotationRule
 import com.saveourtool.diktat.util.FixTestBase
 
 import generated.WarningNames
-import org.junit.jupiter.api.Disabled
 import org.junit.jupiter.api.Tag
 import org.junit.jupiter.api.Test
 
 class PreviewAnnotationFixTest : FixTestBase("test/paragraph3/preview_annotation", ::PreviewAnnotationRule) {
     @Test
     @Tag(WarningNames.PREVIEW_ANNOTATION)
-    @Disabled("https://github.com/saveourtool/diktat/issues/1737")
     fun `should add private modifier`() {
         fixAndCompare("PreviewAnnotationPrivateModifierExpected.kt", "PreviewAnnotationPrivateModifierTest.kt")
     }

diff --git a/diktat-rules/src/test/kotlin/com/saveourtool/diktat/util/FixTestBase.kt b/diktat-rules/src/test/kotlin/com/saveourtool/diktat/util/FixTestBase.kt
@@ -56,6 +56,14 @@ open class FixTestBase(
         val testComparatorUnit = testComparatorUnitSupplier(overrideRulesConfigList)
         val result = testComparatorUnit
             .compareFilesFromResources(expectedPath, testPath, resourceReader)
+        if (!result.isSuccessful) {
+            Assertions.assertEquals(
+                result.expectedContentWithoutWarns,
+                result.actualContent,
+            ) {
+                "Content are different"
+            }
+        }
         Assertions.assertTrue(
             result.isSuccessful
         ) {

diff --git a/...mework/src/main/kotlin/com/saveourtool/diktat/test/framework/processing/FileComparator.kt b/...mework/src/main/kotlin/com/saveourtool/diktat/test/framework/processing/FileComparator.kt
@@ -27,20 +27,26 @@ class FileComparator(
     )
 
     /**
-     * delta in files
+     * expected result without lines with warns
      */
-    val delta: String? by lazy {
-        if (expectedResult.isEmpty()) {
-            return@lazy null
-        }
+    val expectedResultWithoutWarns: String by lazy {
         val regex = (".*// ;warn:?(.*):(\\d*): (.+)").toRegex()
-        val expectWithoutWarn = expectedResult
+        expectedResult
             .split("\n")
             .filterNot { line ->
                 line.contains(regex)
             }
             .joinToString("\n")
-        val patch = diff(expectWithoutWarn, actualResult, null)
+    }
+
+    /**
+     * delta in files
+     */
+    val delta: String? by lazy {
+        if (expectedResult.isEmpty()) {
+            return@lazy null
+        }
+        val patch = diff(expectedResultWithoutWarns, actualResult, null)
 
         if (patch.deltas.isEmpty()) {
             return@lazy null

diff --git a/.../src/main/kotlin/com/saveourtool/diktat/test/framework/processing/FileComparisonResult.kt b/.../src/main/kotlin/com/saveourtool/diktat/test/framework/processing/FileComparisonResult.kt
@@ -16,10 +16,12 @@ import org.intellij.lang.annotations.Language
  * @property actualContent the actual file content (possibly slightly different
  *   from the original after `diktat:check` is run).
  * @property expectedContent the expected file content.
+ * @property expectedContentWithoutWarns the expected file content without warns.
  */
 data class FileComparisonResult(
     val isSuccessful: Boolean,
     val delta: String?,
     @Language("kotlin") val actualContent: String,
-    @Language("kotlin") val expectedContent: String
+    @Language("kotlin") val expectedContent: String,
+    @Language("kotlin") val expectedContentWithoutWarns: String = expectedContent,
 )
diff --git a/...rk/src/main/kotlin/com/saveourtool/diktat/test/framework/processing/TestComparatorUnit.kt b/...rk/src/main/kotlin/com/saveourtool/diktat/test/framework/processing/TestComparatorUnit.kt
@@ -88,10 +88,12 @@ class TestComparatorUnit(
         )
 
         return FileComparisonResult(
-            comparator.compareFilesEqual(),
-            comparator.delta,
-            actualFileContent,
-            expectedFileContent)
+            isSuccessful = comparator.compareFilesEqual(),
+            delta = comparator.delta,
+            actualContent = actualFileContent,
+            expectedContent = expectedFileContent,
+            expectedContentWithoutWarns = comparator.expectedResultWithoutWarns,
+        )
     }
 
     private companion object {