-
Notifications
You must be signed in to change notification settings - Fork 39
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add warning to suggest to use inline classes when a class has single …
- Loading branch information
Showing
12 changed files
with
277 additions
and
5 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
77 changes: 77 additions & 0 deletions
77
...rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/InlineClassesRule.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
package org.cqfn.diktat.ruleset.rules.chapter6.classes | ||
|
||
import org.cqfn.diktat.common.config.rules.RulesConfig | ||
import org.cqfn.diktat.ruleset.constants.EmitType | ||
import org.cqfn.diktat.ruleset.constants.Warnings.INLINE_CLASS_CAN_BE_USED | ||
import org.cqfn.diktat.ruleset.utils.getFirstChildWithType | ||
import org.cqfn.diktat.ruleset.utils.hasChildOfType | ||
|
||
import com.pinterest.ktlint.core.Rule | ||
import com.pinterest.ktlint.core.ast.ElementType.CLASS | ||
import com.pinterest.ktlint.core.ast.ElementType.CONSTRUCTOR_CALLEE | ||
import com.pinterest.ktlint.core.ast.ElementType.FINAL_KEYWORD | ||
import com.pinterest.ktlint.core.ast.ElementType.INTERNAL_KEYWORD | ||
import com.pinterest.ktlint.core.ast.ElementType.MODIFIER_LIST | ||
import com.pinterest.ktlint.core.ast.ElementType.PRIVATE_KEYWORD | ||
import com.pinterest.ktlint.core.ast.ElementType.PROTECTED_KEYWORD | ||
import com.pinterest.ktlint.core.ast.ElementType.PUBLIC_KEYWORD | ||
import com.pinterest.ktlint.core.ast.ElementType.SUPER_TYPE_LIST | ||
import com.pinterest.ktlint.core.ast.ElementType.VAR_KEYWORD | ||
import com.pinterest.ktlint.core.ast.children | ||
import org.jetbrains.kotlin.com.intellij.lang.ASTNode | ||
import org.jetbrains.kotlin.psi.KtClass | ||
import org.jetbrains.kotlin.psi.psiUtil.visibilityModifierType | ||
|
||
/** | ||
* This rule checks if inline class can be used. | ||
*/ | ||
class InlineClassesRule(private val configRule: List<RulesConfig>) : Rule("inline-classes") { | ||
private var isFixMode: Boolean = false | ||
private lateinit var emitWarn: EmitType | ||
|
||
override fun visit( | ||
node: ASTNode, | ||
autoCorrect: Boolean, | ||
emit: EmitType | ||
) { | ||
emitWarn = emit | ||
isFixMode = autoCorrect | ||
|
||
if (node.elementType == CLASS) { | ||
handleClasses(node.psi as KtClass) | ||
} | ||
} | ||
|
||
private fun handleClasses(classPsi: KtClass) { | ||
// Fixme: In Kotlin 1.4.30 inline classes may be used with internal constructors. When it will be released need to check it | ||
if (hasValidProperties(classPsi) && | ||
!isExtendingClass(classPsi.node) && | ||
classPsi.node.getFirstChildWithType(MODIFIER_LIST)?.getChildren(null)?.all { it.elementType in goodModifiers } != false) { | ||
INLINE_CLASS_CAN_BE_USED.warnAndFix(configRule, emitWarn, isFixMode, "class ${classPsi.name}", classPsi.node.startOffset, classPsi.node) { | ||
// Fixme: since it's an experimental feature we shouldn't do fixer | ||
} | ||
} | ||
} | ||
|
||
private fun hasValidProperties(classPsi: KtClass): Boolean { | ||
if (classPsi.getProperties().size == 1 && !classPsi.hasExplicitPrimaryConstructor()) { | ||
return !classPsi.getProperties().single().isVar | ||
} else if (classPsi.getProperties().isEmpty() && classPsi.hasExplicitPrimaryConstructor()) { | ||
return classPsi.primaryConstructorParameters.size == 1 && | ||
!classPsi.primaryConstructorParameters.first().node.hasChildOfType(VAR_KEYWORD) && | ||
classPsi.primaryConstructor?.visibilityModifierType()?.value?.let { it == "public" } ?: true | ||
} | ||
return false | ||
} | ||
|
||
private fun isExtendingClass(node: ASTNode): Boolean = | ||
node | ||
.getFirstChildWithType(SUPER_TYPE_LIST) | ||
?.children() | ||
?.any { it.hasChildOfType(CONSTRUCTOR_CALLEE) } | ||
?: false | ||
|
||
companion object { | ||
val goodModifiers = listOf(PUBLIC_KEYWORD, PRIVATE_KEYWORD, FINAL_KEYWORD, PROTECTED_KEYWORD, INTERNAL_KEYWORD) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
163 changes: 163 additions & 0 deletions
163
diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/InlineClassesWarnTest.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,163 @@ | ||
package org.cqfn.diktat.ruleset.chapter6 | ||
|
||
import org.cqfn.diktat.ruleset.constants.Warnings.INLINE_CLASS_CAN_BE_USED | ||
import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID | ||
import org.cqfn.diktat.ruleset.rules.chapter6.classes.InlineClassesRule | ||
import org.cqfn.diktat.util.LintTestBase | ||
|
||
import com.pinterest.ktlint.core.LintError | ||
import generated.WarningNames | ||
import org.junit.jupiter.api.Tag | ||
import org.junit.jupiter.api.Test | ||
|
||
class InlineClassesWarnTest : LintTestBase(::InlineClassesRule) { | ||
private val ruleId = "$DIKTAT_RULE_SET_ID:inline-classes" | ||
|
||
@Test | ||
@Tag(WarningNames.INLINE_CLASS_CAN_BE_USED) | ||
fun `should not trigger on inline class`() { | ||
lintMethod( | ||
""" | ||
|inline class Name(val s: String) {} | ||
""".trimMargin() | ||
) | ||
} | ||
|
||
@Test | ||
@Tag(WarningNames.INLINE_CLASS_CAN_BE_USED) | ||
fun `should trigger on regular class`() { | ||
lintMethod( | ||
""" | ||
|class Some { | ||
| val config = Config() | ||
|} | ||
""".trimMargin(), | ||
LintError(1, 1, ruleId, "${INLINE_CLASS_CAN_BE_USED.warnText()} class Some", true) | ||
) | ||
} | ||
|
||
@Test | ||
@Tag(WarningNames.INLINE_CLASS_CAN_BE_USED) | ||
fun `should trigger on class with appropriate modifiers`() { | ||
lintMethod( | ||
""" | ||
|final class Some { | ||
| val config = Config() | ||
|} | ||
""".trimMargin(), | ||
LintError(1, 1, ruleId, "${INLINE_CLASS_CAN_BE_USED.warnText()} class Some", true) | ||
) | ||
} | ||
|
||
@Test | ||
@Tag(WarningNames.INLINE_CLASS_CAN_BE_USED) | ||
fun `should not trigger on class with inappropriate modifiers`() { | ||
lintMethod( | ||
""" | ||
|abstract class Some { | ||
| val config = Config() | ||
|} | ||
""".trimMargin() | ||
) | ||
} | ||
|
||
@Test | ||
@Tag(WarningNames.INLINE_CLASS_CAN_BE_USED) | ||
fun `should trigger on class with val prop in constructor`() { | ||
lintMethod( | ||
""" | ||
|class Some(val anything: Int) { | ||
| | ||
|} | ||
""".trimMargin(), | ||
LintError(1, 1, ruleId, "${INLINE_CLASS_CAN_BE_USED.warnText()} class Some", true) | ||
) | ||
} | ||
|
||
@Test | ||
@Tag(WarningNames.INLINE_CLASS_CAN_BE_USED) | ||
fun `should not trigger on class with var prop #1`() { | ||
lintMethod( | ||
""" | ||
|class Some(var anything: Int) { | ||
| | ||
|} | ||
""".trimMargin() | ||
) | ||
} | ||
|
||
@Test | ||
@Tag(WarningNames.INLINE_CLASS_CAN_BE_USED) | ||
fun `should not trigger on class with var prop #2`() { | ||
lintMethod( | ||
""" | ||
|class Some { | ||
| var some = 3 | ||
|} | ||
""".trimMargin() | ||
) | ||
} | ||
|
||
@Test | ||
@Tag(WarningNames.INLINE_CLASS_CAN_BE_USED) | ||
fun `should not trigger on class that extends class`() { | ||
lintMethod( | ||
""" | ||
|class Some : Any() { | ||
| val some = 3 | ||
|} | ||
""".trimMargin() | ||
) | ||
} | ||
|
||
@Test | ||
@Tag(WarningNames.INLINE_CLASS_CAN_BE_USED) | ||
fun `should trigger on class that extends interface`() { | ||
lintMethod( | ||
""" | ||
|class Some : Any { | ||
| val some = 3 | ||
|} | ||
""".trimMargin(), | ||
LintError(1, 1, ruleId, "${INLINE_CLASS_CAN_BE_USED.warnText()} class Some", true) | ||
) | ||
} | ||
|
||
@Test | ||
@Tag(WarningNames.INLINE_CLASS_CAN_BE_USED) | ||
fun `should not trigger on class with internal constructor`() { | ||
lintMethod( | ||
""" | ||
|class LocalCommandExecutor internal constructor(private val command: String) { | ||
| | ||
|} | ||
""".trimMargin() | ||
) | ||
} | ||
|
||
@Test | ||
@Tag(WarningNames.INLINE_CLASS_CAN_BE_USED) | ||
fun `should trigger on class with public constructor`() { | ||
lintMethod( | ||
""" | ||
|class LocalCommandExecutor public constructor(private val command: String) { | ||
| | ||
|} | ||
""".trimMargin(), | ||
LintError(1, 1, ruleId, "${INLINE_CLASS_CAN_BE_USED.warnText()} class LocalCommandExecutor", true) | ||
) | ||
} | ||
|
||
@Test | ||
@Tag(WarningNames.INLINE_CLASS_CAN_BE_USED) | ||
fun `should trigger on class with annotation before the constructor`() { | ||
lintMethod( | ||
""" | ||
|class LocalCommandExecutor @Inject constructor(private val command: String) { | ||
| | ||
|} | ||
""".trimMargin(), | ||
LintError(1, 1, ruleId, "${INLINE_CLASS_CAN_BE_USED.warnText()} class LocalCommandExecutor", true) | ||
) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
05c3ebb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't able to retrieve PDD puzzles from the code base and submit them to GitHub. If you think that it's a bug on our side, please submit it to yegor256/0pdd:
Please, copy and paste this stack trace to GitHub: