-
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.
Recommendation 4.3.2 generic types explicit declaration (#314)
### What's done: * Added rule logic * Added tests * Added fixme
- Loading branch information
Showing
12 changed files
with
281 additions
and
1 deletion.
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
65 changes: 65 additions & 0 deletions
65
...rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/VariableGenericTypeDeclarationRule.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,65 @@ | ||
package org.cqfn.diktat.ruleset.rules | ||
|
||
import com.pinterest.ktlint.core.Rule | ||
import com.pinterest.ktlint.core.ast.ElementType.CALL_EXPRESSION | ||
import com.pinterest.ktlint.core.ast.ElementType.DOT_QUALIFIED_EXPRESSION | ||
import com.pinterest.ktlint.core.ast.ElementType.PROPERTY | ||
import com.pinterest.ktlint.core.ast.ElementType.TYPE_ARGUMENT_LIST | ||
import com.pinterest.ktlint.core.ast.ElementType.TYPE_REFERENCE | ||
import com.pinterest.ktlint.core.ast.ElementType.VALUE_PARAMETER | ||
import org.cqfn.diktat.common.config.rules.RulesConfig | ||
import org.cqfn.diktat.ruleset.constants.Warnings | ||
import org.cqfn.diktat.ruleset.utils.getAllChildrenWithType | ||
import org.cqfn.diktat.ruleset.utils.hasChildOfType | ||
import org.jetbrains.kotlin.com.intellij.lang.ASTNode | ||
import org.jetbrains.kotlin.psi.KtCallExpression | ||
import org.jetbrains.kotlin.psi.KtParameter | ||
import org.jetbrains.kotlin.psi.KtProperty | ||
|
||
/** | ||
* This Rule checks if declaration of a generic variable is appropriate. | ||
* Not recommended: val myVariable: Map<Int, String> = emptyMap<Int, String>() or val myVariable = emptyMap<Int, String>() | ||
* Recommended: val myVariable: Map<Int, String> = emptyMap() | ||
*/ | ||
// FIXME: we now don't have access to return types, so we can perform this check only if explicit type is present, but should be able also if it's not. | ||
class VariableGenericTypeDeclarationRule(private val configRules: List<RulesConfig>) : Rule("variable-generic-type") { | ||
private lateinit var emitWarn: ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) | ||
private var isFixMode: Boolean = false | ||
|
||
override fun visit(node: ASTNode, | ||
autoCorrect: Boolean, | ||
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) { | ||
emitWarn = emit | ||
isFixMode = autoCorrect | ||
|
||
when(node.elementType) { | ||
PROPERTY, VALUE_PARAMETER -> handleProperty(node) | ||
} | ||
} | ||
|
||
@Suppress("UnsafeCallOnNullableType") | ||
private fun handleProperty(node: ASTNode) { | ||
val callExpr = node.findChildByType(CALL_EXPRESSION) | ||
?: node.findChildByType(DOT_QUALIFIED_EXPRESSION)?.getAllChildrenWithType(CALL_EXPRESSION)?.lastOrNull() | ||
|
||
val rightSide = (callExpr?.psi as? KtCallExpression)?.typeArgumentList?.arguments | ||
val leftSide = if (node.elementType == PROPERTY) { | ||
(node.psi as? KtProperty)?.typeReference?.typeElement?.typeArgumentsAsTypes | ||
} else { | ||
(node.psi as? KtParameter)?.typeReference?.typeElement?.typeArgumentsAsTypes | ||
} | ||
|
||
if ((rightSide != null && leftSide != null) && | ||
rightSide.size == leftSide.size && | ||
rightSide.zip(leftSide).all { (first, second) -> first.text == second.text }) { | ||
Warnings.GENERIC_VARIABLE_WRONG_DECLARATION.warnAndFix(configRules, emitWarn, isFixMode, | ||
"type arguments are unnecessary in ${callExpr.text}", node.startOffset, node) { | ||
callExpr.removeChild(callExpr.findChildByType(TYPE_ARGUMENT_LIST)!!) | ||
} | ||
} | ||
|
||
if (leftSide == null && rightSide != null) { | ||
Warnings.GENERIC_VARIABLE_WRONG_DECLARATION.warn(configRules, emitWarn, isFixMode, node.text, node.startOffset, node) | ||
} | ||
} | ||
} |
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
16 changes: 16 additions & 0 deletions
16
...test/kotlin/org/cqfn/diktat/ruleset/chapter4/VariableGenericTypeDeclarationRuleFixTest.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,16 @@ | ||
package org.cqfn.diktat.ruleset.chapter4 | ||
|
||
import generated.WarningNames | ||
import org.cqfn.diktat.ruleset.rules.VariableGenericTypeDeclarationRule | ||
import org.cqfn.diktat.util.FixTestBase | ||
import org.junit.jupiter.api.Tag | ||
import org.junit.jupiter.api.Test | ||
|
||
class VariableGenericTypeDeclarationRuleFixTest : FixTestBase("test/paragraph4/generics", ::VariableGenericTypeDeclarationRule) { | ||
|
||
@Test | ||
@Tag(WarningNames.GENERIC_VARIABLE_WRONG_DECLARATION) | ||
fun `basic fix test`() { | ||
fixAndCompare("VariableGenericTypeDeclarationExpected.kt", "VariableGenericTypeDeclarationTest.kt") | ||
} | ||
} |
170 changes: 170 additions & 0 deletions
170
...est/kotlin/org/cqfn/diktat/ruleset/chapter4/VariableGenericTypeDeclarationRuleWarnTest.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,170 @@ | ||
package org.cqfn.diktat.ruleset.chapter4 | ||
|
||
import com.pinterest.ktlint.core.LintError | ||
import generated.WarningNames.GENERIC_VARIABLE_WRONG_DECLARATION | ||
import org.cqfn.diktat.ruleset.constants.Warnings | ||
import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID | ||
import org.cqfn.diktat.ruleset.rules.VariableGenericTypeDeclarationRule | ||
import org.cqfn.diktat.util.LintTestBase | ||
import org.junit.jupiter.api.Tag | ||
import org.junit.jupiter.api.Test | ||
|
||
class VariableGenericTypeDeclarationRuleWarnTest : LintTestBase(::VariableGenericTypeDeclarationRule) { | ||
|
||
private val ruleId = "$DIKTAT_RULE_SET_ID:variable-generic-type" | ||
|
||
@Test | ||
@Tag(GENERIC_VARIABLE_WRONG_DECLARATION) | ||
fun `property with generic type good`() { | ||
lintMethod( | ||
""" | ||
|class SomeClass { | ||
| val myVariable: Map<Int, String> = emptyMap() | ||
| val lazyValue: Map<Int, String> by lazy { | ||
| println("computed!") | ||
| emptyMap<Int, String>() | ||
| } | ||
| val sideRegex = Regex("<([a-zA-Z, <>?]+)>") | ||
| val str = someMethod("mapOf<String>") | ||
| val x = foo.bar<Bar>().baz() | ||
|} | ||
""".trimMargin() | ||
) | ||
} | ||
|
||
@Test | ||
@Tag(GENERIC_VARIABLE_WRONG_DECLARATION) | ||
fun `property with generic type bad`() { | ||
lintMethod( | ||
""" | ||
|class SomeClass { | ||
| val myVariable: Map<Int, String> = emptyMap<Int, String>() | ||
| val any = Array<Any>(3) { "" } | ||
| val x = foo.bar<Bar>().baz<Some>() | ||
|} | ||
""".trimMargin(), | ||
LintError(2,4, ruleId, | ||
"${Warnings.GENERIC_VARIABLE_WRONG_DECLARATION.warnText()} type arguments are unnecessary in emptyMap<Int, String>()", true), | ||
LintError(3,4, ruleId, | ||
"${Warnings.GENERIC_VARIABLE_WRONG_DECLARATION.warnText()} val any = Array<Any>(3) { \"\" }", false), | ||
LintError(4,4, ruleId, | ||
"${Warnings.GENERIC_VARIABLE_WRONG_DECLARATION.warnText()} val x = foo.bar<Bar>().baz<Some>()", false) | ||
) | ||
} | ||
|
||
@Test | ||
@Tag(GENERIC_VARIABLE_WRONG_DECLARATION) | ||
fun `property in function as parameter good`() { | ||
lintMethod( | ||
""" | ||
|class SomeClass { | ||
| private fun someFunc(myVariable: Map<Int, String> = emptyMap()) { | ||
| | ||
| } | ||
|} | ||
""".trimMargin() | ||
) | ||
} | ||
|
||
@Test | ||
@Tag(GENERIC_VARIABLE_WRONG_DECLARATION) | ||
fun `property in function as parameter bad`() { | ||
lintMethod( | ||
""" | ||
|class SomeClass { | ||
| private fun someFunc(myVariable: Map<Int, String> = emptyMap<Int, String>()) { | ||
| | ||
| } | ||
|} | ||
""".trimMargin(), | ||
LintError(2,25, ruleId, | ||
"${Warnings.GENERIC_VARIABLE_WRONG_DECLARATION.warnText()} type arguments are unnecessary in emptyMap<Int, String>()", true) | ||
) | ||
} | ||
|
||
@Test | ||
@Tag(GENERIC_VARIABLE_WRONG_DECLARATION) | ||
fun `property in function good`() { | ||
lintMethod( | ||
""" | ||
|class SomeClass { | ||
| private fun someFunc() { | ||
| val myVariable: Map<Int, String> = emptyMap() | ||
| } | ||
|} | ||
""".trimMargin() | ||
) | ||
} | ||
|
||
@Test | ||
@Tag(GENERIC_VARIABLE_WRONG_DECLARATION) | ||
fun `property in function bad`() { | ||
lintMethod( | ||
""" | ||
|class SomeClass { | ||
| private fun someFunc() { | ||
| val myVariable: Map<Int, String> = emptyMap<Int, String>() | ||
| } | ||
|} | ||
""".trimMargin(), | ||
LintError(3,8, ruleId, | ||
"${Warnings.GENERIC_VARIABLE_WRONG_DECLARATION.warnText()} type arguments are unnecessary in emptyMap<Int, String>()", true) | ||
) | ||
} | ||
|
||
@Test | ||
@Tag(GENERIC_VARIABLE_WRONG_DECLARATION) | ||
fun `property in class good`() { | ||
lintMethod( | ||
""" | ||
|class SomeClass(val myVariable: Map<Int, String> = emptyMap()) { | ||
| | ||
|} | ||
""".trimMargin() | ||
) | ||
} | ||
|
||
@Test | ||
@Tag(GENERIC_VARIABLE_WRONG_DECLARATION) | ||
fun `property in class bad`() { | ||
lintMethod( | ||
""" | ||
|class SomeClass(val myVariable: Map<Int, String> = emptyMap<Int, String>()) { | ||
| | ||
|} | ||
""".trimMargin(), | ||
LintError(1,17, ruleId, | ||
"${Warnings.GENERIC_VARIABLE_WRONG_DECLARATION.warnText()} type arguments are unnecessary in emptyMap<Int, String>()", true) | ||
) | ||
} | ||
|
||
@Test | ||
@Tag(GENERIC_VARIABLE_WRONG_DECLARATION) | ||
fun `property with multiple generics`() { | ||
lintMethod( | ||
""" | ||
|class SomeClass { | ||
| private fun someFunc() { | ||
| val myVariable: Map<Int, Map<String>> = emptyMap<Int, Map<String>>() | ||
| } | ||
|} | ||
""".trimMargin(), | ||
LintError(3,8, ruleId, | ||
"${Warnings.GENERIC_VARIABLE_WRONG_DECLARATION.warnText()} type arguments are unnecessary in emptyMap<Int, Map<String>>()", true) | ||
) | ||
} | ||
|
||
@Test | ||
@Tag(GENERIC_VARIABLE_WRONG_DECLARATION) | ||
fun `should not trigger`() { | ||
lintMethod( | ||
""" | ||
|class SomeClass { | ||
| private fun someFunc() { | ||
| var myVariable: Map<Int, Any> = emptyMap<Int, String>() | ||
| } | ||
|} | ||
""".trimMargin() | ||
) | ||
} | ||
} |
7 changes: 7 additions & 0 deletions
7
...les/src/test/resources/test/paragraph4/generics/VariableGenericTypeDeclarationExpected.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,7 @@ | ||
package test.paragraph4.generics | ||
|
||
class SomeClass(val some: Map<Int, String> = emptyMap()) { | ||
val myVariable: Map<Int, String> = emptyMap() | ||
|
||
fun someFunc(myVariable: Map<Int, String> = emptyMap()) {} | ||
} |
7 changes: 7 additions & 0 deletions
7
...t-rules/src/test/resources/test/paragraph4/generics/VariableGenericTypeDeclarationTest.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,7 @@ | ||
package test.paragraph4.generics | ||
|
||
class SomeClass(val some: Map<Int, String> = emptyMap<Int, String>()) { | ||
val myVariable: Map<Int, String> = emptyMap<Int, String>() | ||
|
||
fun someFunc(myVariable: Map<Int, String> = emptyMap<Int, String>()) {} | ||
} |
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
8738dd1
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: