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

Recommendation 4.3.2 generic types explicit declaration #314

Merged
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
d32c237
feature/recommendation-4.3.2-generic-types-explicit-declaration
aktsay6 Sep 22, 2020
ee75898
feature/recommendation-4.3.2-generics-types-explicit-declaration
aktsay6 Sep 22, 2020
b28a8bc
Merge branch 'master' into feature/recommendation-4.3.2-generic-types…
aktsay6 Sep 22, 2020
dd6a586
feature/recommendation-4.3.2-generics-types-explicit-declaration
aktsay6 Sep 22, 2020
09a214a
Merge branch 'master' into feature/recommendation-4.3.2-generic-types…
aktsay6 Sep 22, 2020
46b11be
Merge branch 'master' into feature/recommendation-4.3.2-generic-types…
aktsay6 Sep 22, 2020
c85695e
feature/recommendation-4.3.2-generic-types-explicit-declaration
aktsay6 Sep 24, 2020
69b6f80
feature/recommendation-4.3.2-generic-explicit-declaration
aktsay6 Sep 24, 2020
92e12b4
Merge branch 'master' into feature/recommendation-4.3.2-generic-types…
aktsay6 Sep 25, 2020
d5a30e0
feature/recommendation-4.3.2-generic-types-explicit-declaration
aktsay6 Sep 25, 2020
4d00399
feature/recommendation-4.3.2-generic-types-explicit-declaration
aktsay6 Sep 25, 2020
75b85a7
Merge branch 'master' into feature/recommendation-4.3.2-generic-types…
aktsay6 Sep 25, 2020
ef3d993
feature/recommendation-4.3.2
aktsay6 Sep 25, 2020
d76b501
Recommendation-4.3.2-generic-types-explicit-type-declaration
aktsay6 Sep 28, 2020
cd2f8e5
Merge branch 'master' into feature/recommendation-4.3.2-generic-types…
aktsay6 Sep 28, 2020
2d93eee
Recommendation-4.3.2-generic-types-explicit-declaration
aktsay6 Sep 28, 2020
58ed2e0
Recommendation-4.3.2-generic-types-explicit-declaration
aktsay6 Sep 28, 2020
312052f
Merge branch 'master' into feature/recommendation-4.3.2-generic-types…
aktsay6 Sep 28, 2020
e43bf01
Recommendation-4.3.2-generic-types-explicit-declaration
aktsay6 Sep 28, 2020
c2ba355
Merge remote-tracking branch 'origin/feature/recommendation-4.3.2-gen…
aktsay6 Sep 28, 2020
1e0b916
Merge branch 'feature/recommendation-4.3.2-generic-types-explicit-dec…
aktsay6 Sep 29, 2020
8428852
Recommendation-4.3.2-generic-types-explicit-declaration
aktsay6 Sep 29, 2020
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
3 changes: 3 additions & 0 deletions diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,9 @@
enabled: true
configuration:
typeReferenceLength: '25' # max length of type reference
- name: GENERIC_VARIABLE_WRONG_DECLARATION
enabled: true
configuration: {}
# Inspection that checks if string template has redundant curly braces
- name: STRING_TEMPLATE_CURLY_BRACES
enabled: true
Expand Down
2 changes: 2 additions & 0 deletions diktat-rules/src/main/kotlin/generated/WarningNames.kt
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@ object WarningNames {

const val TYPE_ALIAS: String = "TYPE_ALIAS"

const val GENERIC_VARIABLE_WRONG_DECLARATION: String = "GENERIC_VARIABLE_WRONG_DECLARATION"

const val STRING_TEMPLATE_CURLY_BRACES: String = "STRING_TEMPLATE_CURLY_BRACES"

const val STRING_TEMPLATE_QUOTES: String = "STRING_TEMPLATE_QUOTES"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ enum class Warnings(private val canBeAutoCorrected: Boolean, private val warn: S

// ======== chapter 4 ========
TYPE_ALIAS(false, "variable's type is too complex and should be replaced with typealias"),
GENERIC_VARIABLE_WRONG_DECLARATION(true, "variable should have explicit type declaration"),
STRING_TEMPLATE_CURLY_BRACES(true, "string template has redundant curly braces"),
STRING_TEMPLATE_QUOTES(true, "string template has redundant quotes"),
// FixMe: change float literal to BigDecimal? Or kotlin equivalent?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class DiktatRuleSetProvider(private val diktatConfigFile: String = "diktat-analy
::BlockStructureBraces,
::EmptyBlock,
::EnumsSeparated,
::VariableGenericTypeDeclarationRule,
::SingleLineStatementsRule,
::CommentsFormatting,
::ConsecutiveSpacesRule,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
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

/**
* 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") {
aktsay6 marked this conversation as resolved.
Show resolved Hide resolved
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) {
aktsay6 marked this conversation as resolved.
Show resolved Hide resolved
val callExpr = node.findChildByType(CALL_EXPRESSION)
?: node.findChildByType(DOT_QUALIFIED_EXPRESSION)?.getAllChildrenWithType(CALL_EXPRESSION)?.lastOrNull()

val rightSide = sideRegex.find(callExpr?.text ?: "")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it will be more reliable to use something like (callExpr.psi as KtCallExpression).typeArgumentList? To explicitly retrieve type parameters and compare them instead of text.

val leftSide = sideRegex.find(node.findChildByType(TYPE_REFERENCE)?.text ?: "")

if ((rightSide != null && leftSide != null) && rightSide.groupValues.first() == leftSide.groupValues.first()) {
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)
}
}

companion object VariableSide {
val sideRegex = Regex("<([a-zA-Z, ?<>]+)>(?=([^\"]*\"[^\"]*\")*[^\"]*\$)")
}
}
3 changes: 3 additions & 0 deletions diktat-rules/src/main/resources/diktat-analysis-huawei.yml
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,9 @@
enabled: true
configuration:
typeReferenceLength: '25' # max length of type reference
- name: GENERIC_VARIABLE_WRONG_DECLARATION
enabled: true
configuration: {}
# Inspection that checks if string template has redundant curly braces
- name: STRING_TEMPLATE_CURLY_BRACES
enabled: true
Expand Down
3 changes: 3 additions & 0 deletions diktat-rules/src/main/resources/diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,9 @@
enabled: true
configuration:
typeReferenceLength: '25' # max length of type reference
- name: GENERIC_VARIABLE_WRONG_DECLARATION
enabled: true
configuration: {}
# Inspection that checks if string template has redundant curly braces
- name: STRING_TEMPLATE_CURLY_BRACES
enabled: true
Expand Down
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")
}
}
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) {
aktsay6 marked this conversation as resolved.
Show resolved Hide resolved

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()
)
}
}
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()) {}
}
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>()) {}
}
4 changes: 3 additions & 1 deletion info/available-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,6 @@
| 3 | 3.14.2 | STRING_TEMPLATE_CURLY_BRACES | Check: warns if there is redundant curly braces in string template<br> Fix: deletes curly braces | yes | - | + |
| 3 | 3.14.2 | STRING_TEMPLATE_QUOTES | Check: warns if there are redundant quotes in string template<br> Fix: deletes quotes and $ symbol | yes | - | + |
| 4 | 4.2.2| TYPE_ALIAS | Check: if type reference of property is longer than expected | yes | typeReferenceLength | -|
| 4 | 4.1.1 | FLOAT_IN_ACCURATE_CALCULATIONS | Checks that floating-point values are not used in arithmetic expressions<br>Fix: no | no | - | Current implementation detects only floating-point constants |
| 4 | 4.1.1 | FLOAT_IN_ACCURATE_CALCULATIONS | Checks that floating-point values are not used in arithmetic expressions<br>Fix: no | no | - | Current implementation detects only floating-point constants |
| 4 | 4.2.2 | TYPE_ALIAS | Check: if type reference of property is longer than expected | yes | typeReferenceLength | -|
| 4 | 4.3.2 | GENERIC_VARIABLE_WRONG_DECLARATION | Check: warns if variables of generic types don't have explicit type declaration<br>Fix: fixes only variables that have generic declaration on both sides | yes | - | + |