From 0083d1cae52b94ada3cb91e59335ef8d6e6058b8 Mon Sep 17 00:00:00 2001 From: Nariman Abdullin Date: Thu, 21 Sep 2023 11:53:00 +0300 Subject: [PATCH] Fixed PreviewAnnotationRule ### What's done: - moved creating private node to ASTFactory - added a new node to ANNOTATION_LIST It's part of 1737 --- .../rules/chapter3/PreviewAnnotationRule.kt | 37 +++++++++++-------- .../chapter3/PreviewAnnotationFixTest.kt | 2 - .../saveourtool/diktat/util/FixTestBase.kt | 8 ++++ .../framework/processing/FileComparator.kt | 20 ++++++---- .../processing/FileComparisonResult.kt | 4 +- .../processing/TestComparatorUnit.kt | 10 +++-- 6 files changed, 51 insertions(+), 30 deletions(-) diff --git a/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/PreviewAnnotationRule.kt b/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/PreviewAnnotationRule.kt index c756ab5d69..64cf3afa56 100644 --- a/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/PreviewAnnotationRule.kt +++ b/diktat-rules/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) : 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) : 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) : 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/diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/PreviewAnnotationFixTest.kt b/diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/PreviewAnnotationFixTest.kt index d6ad623469..838683d856 100644 --- a/diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/PreviewAnnotationFixTest.kt +++ b/diktat-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 index ff350fa2f4..a5a3c2aaf9 100644 --- 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/diktat-test-framework/src/main/kotlin/com/saveourtool/diktat/test/framework/processing/FileComparator.kt b/diktat-test-framework/src/main/kotlin/com/saveourtool/diktat/test/framework/processing/FileComparator.kt index cb7d88c936..4e229f7641 100644 --- a/diktat-test-framework/src/main/kotlin/com/saveourtool/diktat/test/framework/processing/FileComparator.kt +++ b/diktat-test-framework/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/diktat-test-framework/src/main/kotlin/com/saveourtool/diktat/test/framework/processing/FileComparisonResult.kt b/diktat-test-framework/src/main/kotlin/com/saveourtool/diktat/test/framework/processing/FileComparisonResult.kt index 75477cef66..8bcb108512 100644 --- a/diktat-test-framework/src/main/kotlin/com/saveourtool/diktat/test/framework/processing/FileComparisonResult.kt +++ b/diktat-test-framework/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/diktat-test-framework/src/main/kotlin/com/saveourtool/diktat/test/framework/processing/TestComparatorUnit.kt b/diktat-test-framework/src/main/kotlin/com/saveourtool/diktat/test/framework/processing/TestComparatorUnit.kt index 5095bcf98a..85d82fb64e 100644 --- a/diktat-test-framework/src/main/kotlin/com/saveourtool/diktat/test/framework/processing/TestComparatorUnit.kt +++ b/diktat-test-framework/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 {