Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed PreviewAnnotationRule #1740

Merged
merged 1 commit into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
)
Expand All @@ -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 = "@"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,14 @@ open class FixTestBase(
val testComparatorUnit = testComparatorUnitSupplier(overrideRulesConfigList)
val result = testComparatorUnit
.compareFilesFromResources(expectedPath, testPath, resourceReader)
if (!result.isSuccessful) {
nulls marked this conversation as resolved.
Show resolved Hide resolved
Assertions.assertEquals(
result.expectedContentWithoutWarns,
result.actualContent,
) {
"Content are different"
}
}
Assertions.assertTrue(
result.isSuccessful
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down