From 0b50546fc4bb9bc0172cfc1b026c555c432c0436 Mon Sep 17 00:00:00 2001 From: kentr0w Date: Fri, 12 Feb 2021 17:27:52 +0300 Subject: [PATCH 01/11] Magic Number ### What's done: Implemented rule, added test and documentation --- .../cqfn/diktat/ruleset/constants/Warnings.kt | 1 + .../ruleset/rules/chapter3/LineLength.kt | 1 + .../ruleset/rules/chapter3/MagicNumberRule.kt | 83 +++++++++++++++++++ .../chapter3/MagicNumberRuleWarnTest.kt | 28 +++++++ 4 files changed, 113 insertions(+) create mode 100644 diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt create mode 100644 diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/MagicNumberRuleWarnTest.kt diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt index 92d00419a9..60163ae7ce 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt @@ -112,6 +112,7 @@ enum class Warnings( ENUMS_SEPARATED(true, "3.9.1", "enum is incorrectly formatted"), WHEN_WITHOUT_ELSE(true, "3.11.1", "each 'when' statement must have else at the end"), LONG_NUMERICAL_VALUES_SEPARATED(true, "3.14.2", "long numerical values should be separated with underscore"), + MAGIC_NUMBER(false, "3.14.2", "defining constants with clear names describing what the magic number means"), WRONG_DECLARATIONS_ORDER(true, "3.1.4", "declarations of constants and enum members should be sorted alphabetically"), WRONG_MULTIPLE_MODIFIERS_ORDER(true, "3.14.1", "sequence of modifier-keywords is incorrect"), LOCAL_VARIABLE_EARLY_DECLARATION(false, "3.10.2", "local variables should be declared close to the line where they are first used"), diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/LineLength.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/LineLength.kt index 92612f05be..777ab27ba4 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/LineLength.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/LineLength.kt @@ -381,6 +381,7 @@ class LineLength(configRules: List) : DiktatRule("line-length", con } /** + * * [RuleConfiguration] for maximum line length */ class LineLengthConfiguration(config: Map) : RuleConfiguration(config) { diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt new file mode 100644 index 0000000000..930930396d --- /dev/null +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt @@ -0,0 +1,83 @@ +package org.cqfn.diktat.ruleset.rules.chapter3 + +import com.pinterest.ktlint.core.ast.ElementType +import com.pinterest.ktlint.core.ast.ElementType.ENUM_ENTRY +import com.pinterest.ktlint.core.ast.ElementType.FILE +import com.pinterest.ktlint.core.ast.ElementType.FLOAT_CONSTANT +import com.pinterest.ktlint.core.ast.ElementType.FUN +import com.pinterest.ktlint.core.ast.ElementType.INTEGER_CONSTANT +import com.pinterest.ktlint.core.ast.ElementType.PROPERTY +import com.pinterest.ktlint.core.ast.parent +import org.cqfn.diktat.common.config.rules.RuleConfiguration +import org.cqfn.diktat.common.config.rules.RulesConfig +import org.cqfn.diktat.common.config.rules.getRuleConfig +import org.cqfn.diktat.ruleset.constants.Warnings.MAGIC_NUMBER +import org.cqfn.diktat.ruleset.rules.DiktatRule +import org.cqfn.diktat.ruleset.utils.isConstant +import org.cqfn.diktat.ruleset.utils.isNodeFromCompanionObject +import org.cqfn.diktat.ruleset.utils.isVarProperty +import org.cqfn.diktat.ruleset.utils.prettyPrint +import org.jetbrains.kotlin.com.intellij.lang.ASTNode +import org.jetbrains.kotlin.psi.KtFunction +import org.jetbrains.kotlin.psi.KtProperty +import org.jetbrains.kotlin.psi.psiUtil.isExtensionDeclaration +import org.jetbrains.kotlin.psi.psiUtil.startOffset + +/** + * Litaral and constant + */ +class MagicNumberRule(configRules: List) : DiktatRule("magic-number", configRules, listOf(MAGIC_NUMBER)) { + override fun logic(node: ASTNode) { + val configuration = MagicNumberConfiguration( + configRules.getRuleConfig(MAGIC_NUMBER)?.configuration ?: emptyMap() + ) + if (node.elementType == INTEGER_CONSTANT || node.elementType == FLOAT_CONSTANT){ + checkNumber(node, configuration) + } + } + + private fun checkNumber(node: ASTNode, configuration: MagicNumberConfiguration) { + val isIgnoreNumber = configuration.ignoreNumbers.contains(node.text.toInt()) + val isPropertyDeclaration = node.parent({it.elementType == PROPERTY}) != null + val isLocalVariable = node.parent({it.isVarProperty() && (it.psi as KtProperty).isLocal}) != null + val isConstant = node.parent({it.isConstant()}) != null + val isCompanionObjectProperty = node.parent({it.elementType == PROPERTY && it.isNodeFromCompanionObject()}) != null + val isEnums = node.parent({it.elementType == ENUM_ENTRY}) != null + val isExtensionFunctions = node.parent({it.elementType == FUN && (it.psi as KtFunction).isExtensionDeclaration()}) != null + + val allList = listOf(isIgnoreNumber, isPropertyDeclaration, isLocalVariable, isConstant, + isCompanionObjectProperty, isEnums, isExtensionFunctions) + + val result = allList.zip(mapConfiguration.map {configuration.getParameter(it.key)}) + + if(result.all {it.first != it.second}) { + MAGIC_NUMBER.warn(configRules, emitWarn, isFixMode, node.text, node.startOffset, node) + } + } + + /** + * [RuleConfiguration] for configuration + */ + class MagicNumberConfiguration(config: Map) : RuleConfiguration(config) { + /** + * Ignore numbers + */ + val ignoreNumbers = config["ignoreNumbers"]?.split(",")?.map { it.trim().toInt() } ?: ignoreNumbersList + + //val ignoreRange = config["ignoreRange"] ?: LongRange.EMPTY + + fun getParameter(param: String): Boolean = config[param]?.toBoolean() ?: mapConfiguration[param]!! + + } + + companion object { + val ignoreNumbersList = listOf(-1, 1, 0, 2) + val mapConfiguration = mapOf("ignoreHashCodeFunction" to true, + "ignorePropertyDeclaration" to false, + "ignoreLocalVariableDeclaration" to false, + "ignoreConstantDeclaration" to true, + "ignoreCompanionObjectPropertyDeclaration" to true, + "ignoreEnums" to false, + "ignoreExtensionFunctions" to false,) + } +} diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/MagicNumberRuleWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/MagicNumberRuleWarnTest.kt new file mode 100644 index 0000000000..0f43086af4 --- /dev/null +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/MagicNumberRuleWarnTest.kt @@ -0,0 +1,28 @@ +package org.cqfn.diktat.ruleset.chapter3 + +import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID +import org.cqfn.diktat.ruleset.rules.chapter3.MagicNumberRule +import org.cqfn.diktat.util.LintTestBase +import org.junit.jupiter.api.Test + +class MagicNumberRuleWarnTest : LintTestBase(::MagicNumberRule) { + + private val ruleId = "$DIKTAT_RULE_SET_ID:magic-number" + + @Test + fun `simple check`() { + lintMethod( + """ + |fun f1oo() { + | val a: Byte = 4 + | val b = 0xff + | val c = 2 + | val d = 3.4 + | val e = 3.4f + | val g = 1/2 + | val f = "qwe\$\{12\}hhe" + |} + """.trimMargin() + ) + } +} \ No newline at end of file From aafa792133118fe65741c8c43af66d165d67fb81 Mon Sep 17 00:00:00 2001 From: kentr0w Date: Sat, 13 Feb 2021 19:07:26 +0300 Subject: [PATCH 02/11] Magic Number ### What's done: Implemented rule, added test and documentation --- diktat-analysis.yml | 22 ++++ .../src/main/kotlin/generated/WarningNames.kt | 2 + .../cqfn/diktat/ruleset/constants/Warnings.kt | 2 +- .../ruleset/rules/DiktatRuleSetProvider.kt | 2 + .../ruleset/rules/chapter3/MagicNumberRule.kt | 69 +++++++----- .../main/resources/diktat-analysis-huawei.yml | 22 ++++ .../src/main/resources/diktat-analysis.yml | 22 ++++ .../chapter3/MagicNumberRuleWarnTest.kt | 105 +++++++++++++++++- info/available-rules.md | 1 + info/guide/guide-chapter-3.md | 13 +++ 10 files changed, 223 insertions(+), 37 deletions(-) diff --git a/diktat-analysis.yml b/diktat-analysis.yml index 0f0be5e505..3c93bfdfff 100644 --- a/diktat-analysis.yml +++ b/diktat-analysis.yml @@ -296,6 +296,28 @@ maxNumberLength: '5' # Maximum number of digits between separators maxBlockLength: '3' +# Checks magic number +- name: MAGIC_NUMBER + enabled: true + configuration: + # Ignore numbers + ignoreNumbers: "-1, 1, 0, 2" + # Is ignore override hashCode function + ignoreHashCodeFunction: "true" + # Is ignore property + ignorePropertyDeclaration: "false" + # Is ignore local variable + ignoreLocalVariableDeclaration: "false" + # Is ignore constant + ignoreConstantDeclaration: "true" + # Is ignore property in companion object + ignoreCompanionObjectPropertyDeclaration: "true" + # Is ignore numbers in enum + ignoreEnums: "false" + # Is ignore number in ranges + ignoreRanges: "false" + # Is ignore number in extension function + ignoreExtensionFunctions: "false" # Checks that order of enum values or constant property inside companion is correct - name: WRONG_DECLARATIONS_ORDER enabled: true diff --git a/diktat-rules/src/main/kotlin/generated/WarningNames.kt b/diktat-rules/src/main/kotlin/generated/WarningNames.kt index c762fbf767..fdfc9db868 100644 --- a/diktat-rules/src/main/kotlin/generated/WarningNames.kt +++ b/diktat-rules/src/main/kotlin/generated/WarningNames.kt @@ -167,6 +167,8 @@ public object WarningNames { public const val LONG_NUMERICAL_VALUES_SEPARATED: String = "LONG_NUMERICAL_VALUES_SEPARATED" + public const val MAGIC_NUMBER: String = "MAGIC_NUMBER" + public const val WRONG_DECLARATIONS_ORDER: String = "WRONG_DECLARATIONS_ORDER" public const val WRONG_MULTIPLE_MODIFIERS_ORDER: String = "WRONG_MULTIPLE_MODIFIERS_ORDER" diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt index 60163ae7ce..34abc942e5 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt @@ -112,7 +112,7 @@ enum class Warnings( ENUMS_SEPARATED(true, "3.9.1", "enum is incorrectly formatted"), WHEN_WITHOUT_ELSE(true, "3.11.1", "each 'when' statement must have else at the end"), LONG_NUMERICAL_VALUES_SEPARATED(true, "3.14.2", "long numerical values should be separated with underscore"), - MAGIC_NUMBER(false, "3.14.2", "defining constants with clear names describing what the magic number means"), + MAGIC_NUMBER(false, "3.14.3", "defining constants with clear names describing what the magic number means"), WRONG_DECLARATIONS_ORDER(true, "3.1.4", "declarations of constants and enum members should be sorted alphabetically"), WRONG_MULTIPLE_MODIFIERS_ORDER(true, "3.14.1", "sequence of modifier-keywords is incorrect"), LOCAL_VARIABLE_EARLY_DECLARATION(false, "3.10.2", "local variables should be declared close to the line where they are first used"), diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt index 86331ed1bc..56bd730e75 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt @@ -22,6 +22,7 @@ import org.cqfn.diktat.ruleset.rules.chapter3.EmptyBlock import org.cqfn.diktat.ruleset.rules.chapter3.EnumsSeparated import org.cqfn.diktat.ruleset.rules.chapter3.LineLength import org.cqfn.diktat.ruleset.rules.chapter3.LongNumericalValuesSeparatedRule +import org.cqfn.diktat.ruleset.rules.chapter3.MagicNumberRule import org.cqfn.diktat.ruleset.rules.chapter3.MultipleModifiersSequence import org.cqfn.diktat.ruleset.rules.chapter3.NullableTypeRule import org.cqfn.diktat.ruleset.rules.chapter3.SingleLineStatementsRule @@ -184,6 +185,7 @@ class DiktatRuleSetProvider(private var diktatConfigFile: String = DIKTAT_ANALYS ::TypeAliasRule, ::OverloadingArgumentsFunction, ::FunctionLength, + ::MagicNumberRule, ::LambdaParameterOrder, ::FunctionArgumentsSize, ::BlankLinesRule, diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt index 930930396d..c63b45a211 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt @@ -1,30 +1,33 @@ package org.cqfn.diktat.ruleset.rules.chapter3 -import com.pinterest.ktlint.core.ast.ElementType -import com.pinterest.ktlint.core.ast.ElementType.ENUM_ENTRY -import com.pinterest.ktlint.core.ast.ElementType.FILE -import com.pinterest.ktlint.core.ast.ElementType.FLOAT_CONSTANT -import com.pinterest.ktlint.core.ast.ElementType.FUN -import com.pinterest.ktlint.core.ast.ElementType.INTEGER_CONSTANT -import com.pinterest.ktlint.core.ast.ElementType.PROPERTY -import com.pinterest.ktlint.core.ast.parent import org.cqfn.diktat.common.config.rules.RuleConfiguration import org.cqfn.diktat.common.config.rules.RulesConfig import org.cqfn.diktat.common.config.rules.getRuleConfig import org.cqfn.diktat.ruleset.constants.Warnings.MAGIC_NUMBER import org.cqfn.diktat.ruleset.rules.DiktatRule +import org.cqfn.diktat.ruleset.utils.hasChildOfType import org.cqfn.diktat.ruleset.utils.isConstant import org.cqfn.diktat.ruleset.utils.isNodeFromCompanionObject import org.cqfn.diktat.ruleset.utils.isVarProperty -import org.cqfn.diktat.ruleset.utils.prettyPrint + +import com.pinterest.ktlint.core.ast.ElementType.BINARY_EXPRESSION +import com.pinterest.ktlint.core.ast.ElementType.ENUM_ENTRY +import com.pinterest.ktlint.core.ast.ElementType.FLOAT_CONSTANT +import com.pinterest.ktlint.core.ast.ElementType.FUN +import com.pinterest.ktlint.core.ast.ElementType.INTEGER_CONSTANT +import com.pinterest.ktlint.core.ast.ElementType.MINUS +import com.pinterest.ktlint.core.ast.ElementType.OPERATION_REFERENCE +import com.pinterest.ktlint.core.ast.ElementType.PROPERTY +import com.pinterest.ktlint.core.ast.ElementType.RANGE +import com.pinterest.ktlint.core.ast.parent import org.jetbrains.kotlin.com.intellij.lang.ASTNode import org.jetbrains.kotlin.psi.KtFunction import org.jetbrains.kotlin.psi.KtProperty import org.jetbrains.kotlin.psi.psiUtil.isExtensionDeclaration -import org.jetbrains.kotlin.psi.psiUtil.startOffset +import org.jetbrains.kotlin.psi.psiUtil.parents /** - * Litaral and constant + * Rule for magic number */ class MagicNumberRule(configRules: List) : DiktatRule("magic-number", configRules, listOf(MAGIC_NUMBER)) { override fun logic(node: ASTNode) { @@ -37,47 +40,53 @@ class MagicNumberRule(configRules: List) : DiktatRule("magic-number } private fun checkNumber(node: ASTNode, configuration: MagicNumberConfiguration) { - val isIgnoreNumber = configuration.ignoreNumbers.contains(node.text.toInt()) + val nodeText = node.treePrev?.let { if(it.elementType == OPERATION_REFERENCE && it.hasChildOfType(MINUS)) "-${node.text}" else node.text} ?: node.text + val isIgnoreNumber = configuration.ignoreNumbers.contains(nodeText) + val isHashFunction = node.parent({it.elementType == FUN && it.isHashFun()}) != null && + node.parents().find {it.elementType == PROPERTY} == null val isPropertyDeclaration = node.parent({it.elementType == PROPERTY}) != null val isLocalVariable = node.parent({it.isVarProperty() && (it.psi as KtProperty).isLocal}) != null - val isConstant = node.parent({it.isConstant()}) != null + val isConstant = node.parent({it.elementType == PROPERTY && it.isConstant()}) != null val isCompanionObjectProperty = node.parent({it.elementType == PROPERTY && it.isNodeFromCompanionObject()}) != null val isEnums = node.parent({it.elementType == ENUM_ENTRY}) != null - val isExtensionFunctions = node.parent({it.elementType == FUN && (it.psi as KtFunction).isExtensionDeclaration()}) != null - - val allList = listOf(isIgnoreNumber, isPropertyDeclaration, isLocalVariable, isConstant, - isCompanionObjectProperty, isEnums, isExtensionFunctions) - - val result = allList.zip(mapConfiguration.map {configuration.getParameter(it.key)}) - - if(result.all {it.first != it.second}) { - MAGIC_NUMBER.warn(configRules, emitWarn, isFixMode, node.text, node.startOffset, node) + val isRanges = node.treeParent.run { this.elementType == BINARY_EXPRESSION && + this.findChildByType(OPERATION_REFERENCE)?.hasChildOfType(RANGE) ?: false } + val isExtensionFunctions = node.parent({it.elementType == FUN && (it.psi as KtFunction).isExtensionDeclaration()}) != null && + node.parents().find {it.elementType == PROPERTY} == null + val result = listOf(isHashFunction, isPropertyDeclaration, isLocalVariable, isConstant, + isCompanionObjectProperty, isEnums, isRanges, isExtensionFunctions).zip(mapConfiguration.map {configuration.getParameter(it.key)}) + if(result.any {it.first && it.first != it.second} && !isIgnoreNumber) { + MAGIC_NUMBER.warn(configRules, emitWarn, isFixMode, nodeText, node.startOffset, node) } } + private fun ASTNode.isHashFun() = + (this.psi as KtFunction).run { + this.nameIdentifier?.text == "hashCode" && this.annotationEntries.map { it.text }.contains("@Override") + } + /** * [RuleConfiguration] for configuration */ class MagicNumberConfiguration(config: Map) : RuleConfiguration(config) { /** - * Ignore numbers + * List of ignore numbers */ - val ignoreNumbers = config["ignoreNumbers"]?.split(",")?.map { it.trim().toInt() } ?: ignoreNumbersList - - //val ignoreRange = config["ignoreRange"] ?: LongRange.EMPTY + val ignoreNumbers = config["ignoreNumbers"]?.split(",")?.map { it.trim() } ?: ignoreNumbersList fun getParameter(param: String): Boolean = config[param]?.toBoolean() ?: mapConfiguration[param]!! - } companion object { - val ignoreNumbersList = listOf(-1, 1, 0, 2) - val mapConfiguration = mapOf("ignoreHashCodeFunction" to true, + val ignoreNumbersList = listOf("-1", "1", "0", "2") + val mapConfiguration = mapOf( + "ignoreHashCodeFunction" to true, "ignorePropertyDeclaration" to false, "ignoreLocalVariableDeclaration" to false, "ignoreConstantDeclaration" to true, "ignoreCompanionObjectPropertyDeclaration" to true, "ignoreEnums" to false, - "ignoreExtensionFunctions" to false,) + "ignoreRanges" to false, + "ignoreExtensionFunctions" to false) } } diff --git a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml index 02aadccb5a..d63ca15da0 100644 --- a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml +++ b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml @@ -296,6 +296,28 @@ maxNumberLength: '5' # Maximum number of digits between separators maxBlockLength: '3' +# Checks magic number +- name: MAGIC_NUMBER + enabled: true + configuration: + # Ignore numbers + ignoreNumbers: "-1, 1, 0, 2" + # Is ignore override hashCode function + ignoreHashCodeFunction: "true" + # Is ignore property + ignorePropertyDeclaration: "false" + # Is ignore local variable + ignoreLocalVariableDeclaration: "false" + # Is ignore constant + ignoreConstantDeclaration: "true" + # Is ignore property in companion object + ignoreCompanionObjectPropertyDeclaration: "true" + # Is ignore numbers in enum + ignoreEnums: "false" + # Is ignore number in ranges + ignoreRanges: "false" + # Is ignore number in extension function + ignoreExtensionFunctions: "false" # Checks that order of enum values or constant property inside companion is correct - name: WRONG_DECLARATIONS_ORDER enabled: true diff --git a/diktat-rules/src/main/resources/diktat-analysis.yml b/diktat-rules/src/main/resources/diktat-analysis.yml index 774694e8ab..c5afadd942 100644 --- a/diktat-rules/src/main/resources/diktat-analysis.yml +++ b/diktat-rules/src/main/resources/diktat-analysis.yml @@ -292,6 +292,28 @@ maxNumberLength: '5' # Maximum number of digits between separators maxBlockLength: '3' +# Checks magic number +- name: MAGIC_NUMBER + enabled: true + configuration: + # Ignore numbers + ignoreNumbers: "-1, 1, 0, 2" + # Is ignore override hashCode function + ignoreHashCodeFunction: "true" + # Is ignore property + ignorePropertyDeclaration: "false" + # Is ignore local variable + ignoreLocalVariableDeclaration: "false" + # Is ignore constant + ignoreConstantDeclaration: "true" + # Is ignore property in companion object + ignoreCompanionObjectPropertyDeclaration: "true" + # Is ignore numbers in enum + ignoreEnums: "false" + # Is ignore number in ranges + ignoreRanges: "false" + # Is ignore number in extension function + ignoreExtensionFunctions: "false" # Checks that order of enum values or constant property inside companion is correct - name: WRONG_DECLARATIONS_ORDER enabled: true diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/MagicNumberRuleWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/MagicNumberRuleWarnTest.kt index 0f43086af4..0ab0885788 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/MagicNumberRuleWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/MagicNumberRuleWarnTest.kt @@ -1,28 +1,121 @@ package org.cqfn.diktat.ruleset.chapter3 +import org.cqfn.diktat.common.config.rules.RulesConfig +import org.cqfn.diktat.ruleset.constants.Warnings.MAGIC_NUMBER import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID import org.cqfn.diktat.ruleset.rules.chapter3.MagicNumberRule 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 MagicNumberRuleWarnTest : LintTestBase(::MagicNumberRule) { - private val ruleId = "$DIKTAT_RULE_SET_ID:magic-number" + private val rulesConfigMagicNumber: List = listOf( + RulesConfig( + MAGIC_NUMBER.name, true, + mapOf( + "ignoreHashCodeFunction" to "true", + "ignorePropertyDeclaration" to "true", + "ignoreLocalVariableDeclaration" to "true", + "ignoreConstantDeclaration" to "true", + "ignoreCompanionObjectPropertyDeclaration" to "true", + "ignoreEnums" to "true", + "ignoreRanges" to "true", + "ignoreExtensionFunctions" to "true")) + ) + private val rulesConfigIgnoreNumbersMagicNumber: List = listOf( + RulesConfig( + MAGIC_NUMBER.name, true, + mapOf( + "ignoreNumbers" to "50,-240, 0xFF, -3.5f")) + ) @Test + @Tag(WarningNames.MAGIC_NUMBER) fun `simple check`() { lintMethod( """ |fun f1oo() { | val a: Byte = 4 | val b = 0xff - | val c = 2 - | val d = 3.4 | val e = 3.4f - | val g = 1/2 + | val g = 4/3 | val f = "qwe\$\{12\}hhe" |} - """.trimMargin() + | + |@Override + |fun hashCode(): Int { + | return 32 + |} + """.trimMargin(), + LintError(2, 18, ruleId, "${MAGIC_NUMBER.warnText()} 4", false), + LintError(3, 12, ruleId, "${MAGIC_NUMBER.warnText()} 0xff", false), + LintError(4, 12, ruleId, "${MAGIC_NUMBER.warnText()} 3.4f", false), + LintError(5, 12, ruleId, "${MAGIC_NUMBER.warnText()} 4", false), + LintError(5, 14, ruleId, "${MAGIC_NUMBER.warnText()} 3", false) + ) + } + + @Test + @Tag(WarningNames.MAGIC_NUMBER) + fun `check ignore numbers`() { + lintMethod( + """ + |fun f1oo() { + | val m = -1 + | val a: Byte = 4 + | val b = 0xFF + |} + | + |enum class A(b:Int) { + | A(-240), + | B(50), + | C(3), + |} + |@Override + |fun hashCode(): Int { + | return 32 + |} + """.trimMargin(), + LintError(2, 13, ruleId, "${MAGIC_NUMBER.warnText()} -1", false), + LintError(3, 18, ruleId, "${MAGIC_NUMBER.warnText()} 4", false), + LintError(10, 6, ruleId, "${MAGIC_NUMBER.warnText()} 3", false), + rulesConfigList = rulesConfigIgnoreNumbersMagicNumber + ) + } + + @Test + @Tag(WarningNames.MAGIC_NUMBER) + fun `check all param in config true`() { + lintMethod( + """ + |fun foo() { + | var a = 3 + |} + | + |const val aa = 21.5f + | + |class A { + | companion object { + | val b = 2 + | } + |} + | + |enum class A(b:Int) { + | A(1), + | B(2), + | C(3), + |} + | + |fun goo() { + | val q = 100.1000 + |} + | + |fun Int.foo() = 2 + """.trimMargin(), rulesConfigList = rulesConfigMagicNumber ) } -} \ No newline at end of file +} diff --git a/info/available-rules.md b/info/available-rules.md index 0d633d17e2..87eb9c3938 100644 --- a/info/available-rules.md +++ b/info/available-rules.md @@ -79,6 +79,7 @@ | 3 | 3.11.1 | WHEN_WITHOUT_ELSE | Check: warns if when statement don't have else in the end.
Fix: adds else if when doesn't have it. | yes | no | - | If a when statement of type enum or sealed contains all values of a enum - there is no need to have "else" branch. | | 3 | 3.10.2 | LOCAL_VARIABLE_EARLY_DECLARATION | Check: warns if local variable is declared not immediately before it's usage
Fix (not implemented yet): moves variable declaration | no | no | add auto fix | | 3 | 3.14.2 | LONG_NUMERICAL_VALUES_SEPARATED | Check: warns if value on integer or float constant is too big | no | maxNumberLength maxBlockLength | - | +| 3 | 3.14.3 | MAGIC_NUMBER | Check: warns if there is magic numbers in the code | no | ignoreNumbers, ignoreHashCodeFunction, ignorePropertyDeclaration, ignoreLocalVariableDeclaration, ignoreConstantDeclaration, ignoreCompanionObjectPropertyDeclaration, ignoreEnums, ignoreRanges, ignoreExtensionFunctions | no | 3 | 3.1.4 | WRONG_DECLARATIONS_ORDER | Check: if order of enum values or constant property inside companion isn't correct | yes | no | - | | 3 | 3.12.1 | ANNOTATION_NEW_LINE | Check: warns if annotation not on a new single line | yes | no | - | | 3 | 3.14.1 | WRONG_MULTIPLE_MODIFIERS_ORDER | Check: if multiple modifiers sequence is in the wrong order | yes | no | - | diff --git a/info/guide/guide-chapter-3.md b/info/guide/guide-chapter-3.md index e764e325af..3049f63487 100644 --- a/info/guide/guide-chapter-3.md +++ b/info/guide/guide-chapter-3.md @@ -817,6 +817,19 @@ val socialSecurityNumber = 999_99_9999L val hexBytes = 0xFF_EC_DE_5E val bytes = 0b11010010_01101001_10010100_10010010 ``` +#### 3.14.3: Magic number +Prefer defining constants with clear names describing what the magic number means. +**Valid example**: +```kotlin +class Person() { + + fun isAdult(age: Int): Boolean = age >= majority + + companion object { + const val majority = 18 + } +} +``` ### 3.15 Strings From 05ff05730486b42e71796bd4c7371f1032ba13ab Mon Sep 17 00:00:00 2001 From: kentr0w Date: Sat, 13 Feb 2021 22:10:16 +0300 Subject: [PATCH 03/11] Magic Number ### What's done: Fixed according to our code style --- .../cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt | 4 +++- .../cqfn/diktat/ruleset/chapter3/MagicNumberRuleWarnTest.kt | 6 ++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt index c63b45a211..a03164e34b 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt @@ -39,12 +39,13 @@ class MagicNumberRule(configRules: List) : DiktatRule("magic-number } } + @Suppress("ComplexMethod") private fun checkNumber(node: ASTNode, configuration: MagicNumberConfiguration) { val nodeText = node.treePrev?.let { if(it.elementType == OPERATION_REFERENCE && it.hasChildOfType(MINUS)) "-${node.text}" else node.text} ?: node.text val isIgnoreNumber = configuration.ignoreNumbers.contains(nodeText) val isHashFunction = node.parent({it.elementType == FUN && it.isHashFun()}) != null && node.parents().find {it.elementType == PROPERTY} == null - val isPropertyDeclaration = node.parent({it.elementType == PROPERTY}) != null + val isPropertyDeclaration = node.parent({it.elementType == PROPERTY && !it.isNodeFromCompanionObject()}) != null val isLocalVariable = node.parent({it.isVarProperty() && (it.psi as KtProperty).isLocal}) != null val isConstant = node.parent({it.elementType == PROPERTY && it.isConstant()}) != null val isCompanionObjectProperty = node.parent({it.elementType == PROPERTY && it.isNodeFromCompanionObject()}) != null @@ -74,6 +75,7 @@ class MagicNumberRule(configRules: List) : DiktatRule("magic-number */ val ignoreNumbers = config["ignoreNumbers"]?.split(",")?.map { it.trim() } ?: ignoreNumbersList + @Suppress("UnsafeCallOnNullableType") fun getParameter(param: String): Boolean = config[param]?.toBoolean() ?: mapConfiguration[param]!! } diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/MagicNumberRuleWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/MagicNumberRuleWarnTest.kt index 0ab0885788..5ae8daaf2c 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/MagicNumberRuleWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/MagicNumberRuleWarnTest.kt @@ -50,6 +50,12 @@ class MagicNumberRuleWarnTest : LintTestBase(::MagicNumberRule) { |fun hashCode(): Int { | return 32 |} + | + |class Cl{ + | companion object { + | private const val AA = 4 + | } + |} """.trimMargin(), LintError(2, 18, ruleId, "${MAGIC_NUMBER.warnText()} 4", false), LintError(3, 12, ruleId, "${MAGIC_NUMBER.warnText()} 0xff", false), From 359222d9fcea8c6a3e557814e50a7253ffa43eb7 Mon Sep 17 00:00:00 2001 From: kentr0w Date: Sat, 13 Feb 2021 22:16:31 +0300 Subject: [PATCH 04/11] Magic Number ### What's done: Fixed according to our code style --- .../test/kotlin/org/cqfn/diktat/test/ConfigReaderTest.kt | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/diktat-common/src/test/kotlin/org/cqfn/diktat/test/ConfigReaderTest.kt b/diktat-common/src/test/kotlin/org/cqfn/diktat/test/ConfigReaderTest.kt index ca7e2829e8..8d3287aa42 100644 --- a/diktat-common/src/test/kotlin/org/cqfn/diktat/test/ConfigReaderTest.kt +++ b/diktat-common/src/test/kotlin/org/cqfn/diktat/test/ConfigReaderTest.kt @@ -23,12 +23,15 @@ class ConfigReaderTest { fun `testing kotlin version`() { val rulesConfigList: List? = RulesConfigReader(javaClass.classLoader) .readResource("src/test/resources/test-rules-config.yml") - val kotlinVersionForTest = KotlinVersion(1, 4, 21) requireNotNull(rulesConfigList) - assert(rulesConfigList.getCommonConfiguration().kotlinVersion == kotlinVersionForTest) + assert(rulesConfigList.getCommonConfiguration().kotlinVersion == kotlinVersion) assert(rulesConfigList.find { it.name == DIKTAT_COMMON } ?.configuration ?.get("kotlinVersion") - ?.kotlinVersion() == kotlinVersionForTest) + ?.kotlinVersion() == kotlinVersion) + } + + companion object { + private val kotlinVersion = KotlinVersion(1, 4, 21) } } From 2aa9febc5e0be4930c63ea04eb97e75cfa3df59a Mon Sep 17 00:00:00 2001 From: kentr0w Date: Sat, 13 Feb 2021 22:24:26 +0300 Subject: [PATCH 05/11] Magic Number ### What's done: Fixed according to our code style --- .../test/framework/processing/TestProcessingFactory.kt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestProcessingFactory.kt b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestProcessingFactory.kt index 21c5ee09fa..e64b43ada4 100644 --- a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestProcessingFactory.kt +++ b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestProcessingFactory.kt @@ -28,7 +28,7 @@ class TestProcessingFactory(private val argReader: TestArgumentsReader) { ?: run { log.error("Not able to get directory with test configuration files: " + argReader.properties.testConfigsRelativePath) - exitProcess(5) + exitProcess(statusFive) } try { resource @@ -38,7 +38,7 @@ class TestProcessingFactory(private val argReader: TestArgumentsReader) { .toList() } catch (e: IOException) { log.error("Got -all option, but cannot read config files ", e) - exitProcess(3) + exitProcess(statusTree) } } @@ -91,5 +91,7 @@ class TestProcessingFactory(private val argReader: TestArgumentsReader) { companion object { private val log = LoggerFactory.getLogger(TestProcessingFactory::class.java) + private const val statusTree = 3 + private const val statusFive = 5 } } From 34d45eb618d5a3f0918c5ec0a00282ec0aa847ba Mon Sep 17 00:00:00 2001 From: kentr0w Date: Sat, 13 Feb 2021 22:28:41 +0300 Subject: [PATCH 06/11] Magic Number ### What's done: Fixed according to our code style --- .../test/framework/processing/TestProcessingFactory.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestProcessingFactory.kt b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestProcessingFactory.kt index e64b43ada4..30740e676d 100644 --- a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestProcessingFactory.kt +++ b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestProcessingFactory.kt @@ -28,7 +28,7 @@ class TestProcessingFactory(private val argReader: TestArgumentsReader) { ?: run { log.error("Not able to get directory with test configuration files: " + argReader.properties.testConfigsRelativePath) - exitProcess(statusFive) + exitProcess(STATUS_FIVE) } try { resource @@ -38,7 +38,7 @@ class TestProcessingFactory(private val argReader: TestArgumentsReader) { .toList() } catch (e: IOException) { log.error("Got -all option, but cannot read config files ", e) - exitProcess(statusTree) + exitProcess(STATUS_THREE) } } @@ -91,7 +91,7 @@ class TestProcessingFactory(private val argReader: TestArgumentsReader) { companion object { private val log = LoggerFactory.getLogger(TestProcessingFactory::class.java) - private const val statusTree = 3 - private const val statusFive = 5 + private const val STATUS_FIVE = 5 + private const val STATUS_THREE = 3 } } From 4bf0edd83a8b57c58f0fa3aeb1c289023bbffa0f Mon Sep 17 00:00:00 2001 From: kentr0w Date: Sat, 13 Feb 2021 22:42:39 +0300 Subject: [PATCH 07/11] Magic Number ### What's done: Fixed according to our code style --- .../ruleset/rules/chapter3/MagicNumberRule.kt | 46 +++++++++++-------- .../ruleset/chapter3/FileSizeWarnTest.kt | 6 ++- .../ruleset/utils/AvailableRulesDocTest.kt | 3 +- 3 files changed, 33 insertions(+), 22 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt index a03164e34b..7b5ce0a88b 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt @@ -34,37 +34,39 @@ class MagicNumberRule(configRules: List) : DiktatRule("magic-number val configuration = MagicNumberConfiguration( configRules.getRuleConfig(MAGIC_NUMBER)?.configuration ?: emptyMap() ) - if (node.elementType == INTEGER_CONSTANT || node.elementType == FLOAT_CONSTANT){ + if (node.elementType == INTEGER_CONSTANT || node.elementType == FLOAT_CONSTANT) { checkNumber(node, configuration) } } @Suppress("ComplexMethod") private fun checkNumber(node: ASTNode, configuration: MagicNumberConfiguration) { - val nodeText = node.treePrev?.let { if(it.elementType == OPERATION_REFERENCE && it.hasChildOfType(MINUS)) "-${node.text}" else node.text} ?: node.text + val nodeText = node.treePrev?.let { if (it.elementType == OPERATION_REFERENCE && it.hasChildOfType(MINUS)) "-${node.text}" else node.text } ?: node.text val isIgnoreNumber = configuration.ignoreNumbers.contains(nodeText) - val isHashFunction = node.parent({it.elementType == FUN && it.isHashFun()}) != null && - node.parents().find {it.elementType == PROPERTY} == null - val isPropertyDeclaration = node.parent({it.elementType == PROPERTY && !it.isNodeFromCompanionObject()}) != null - val isLocalVariable = node.parent({it.isVarProperty() && (it.psi as KtProperty).isLocal}) != null - val isConstant = node.parent({it.elementType == PROPERTY && it.isConstant()}) != null - val isCompanionObjectProperty = node.parent({it.elementType == PROPERTY && it.isNodeFromCompanionObject()}) != null - val isEnums = node.parent({it.elementType == ENUM_ENTRY}) != null - val isRanges = node.treeParent.run { this.elementType == BINARY_EXPRESSION && - this.findChildByType(OPERATION_REFERENCE)?.hasChildOfType(RANGE) ?: false } - val isExtensionFunctions = node.parent({it.elementType == FUN && (it.psi as KtFunction).isExtensionDeclaration()}) != null && - node.parents().find {it.elementType == PROPERTY} == null + val isHashFunction = node.parent({ it.elementType == FUN && it.isHashFun() }) != null && + node.parents().find { it.elementType == PROPERTY } == null + val isPropertyDeclaration = node.parent({ it.elementType == PROPERTY && !it.isNodeFromCompanionObject() }) != null + val isLocalVariable = node.parent({ it.isVarProperty() && (it.psi as KtProperty).isLocal }) != null + val isConstant = node.parent({ it.elementType == PROPERTY && it.isConstant() }) != null + val isCompanionObjectProperty = node.parent({ it.elementType == PROPERTY && it.isNodeFromCompanionObject() }) != null + val isEnums = node.parent({ it.elementType == ENUM_ENTRY }) != null + val isRanges = node.treeParent.run { + this.elementType == BINARY_EXPRESSION && + this.findChildByType(OPERATION_REFERENCE)?.hasChildOfType(RANGE) ?: false + } + val isExtensionFunctions = node.parent({ it.elementType == FUN && (it.psi as KtFunction).isExtensionDeclaration() }) != null && + node.parents().find { it.elementType == PROPERTY } == null val result = listOf(isHashFunction, isPropertyDeclaration, isLocalVariable, isConstant, - isCompanionObjectProperty, isEnums, isRanges, isExtensionFunctions).zip(mapConfiguration.map {configuration.getParameter(it.key)}) - if(result.any {it.first && it.first != it.second} && !isIgnoreNumber) { + isCompanionObjectProperty, isEnums, isRanges, isExtensionFunctions).zip(mapConfiguration.map { configuration.getParameter(it.key) }) + if (result.any { it.first && it.first != it.second } && !isIgnoreNumber) { MAGIC_NUMBER.warn(configRules, emitWarn, isFixMode, nodeText, node.startOffset, node) } } private fun ASTNode.isHashFun() = - (this.psi as KtFunction).run { - this.nameIdentifier?.text == "hashCode" && this.annotationEntries.map { it.text }.contains("@Override") - } + (this.psi as KtFunction).run { + this.nameIdentifier?.text == "hashCode" && this.annotationEntries.map { it.text }.contains("@Override") + } /** * [RuleConfiguration] for configuration @@ -75,13 +77,17 @@ class MagicNumberRule(configRules: List) : DiktatRule("magic-number */ val ignoreNumbers = config["ignoreNumbers"]?.split(",")?.map { it.trim() } ?: ignoreNumbersList + /** + * @param param parameter from config + * @return value of parameter + */ @Suppress("UnsafeCallOnNullableType") - fun getParameter(param: String): Boolean = config[param]?.toBoolean() ?: mapConfiguration[param]!! + fun getParameter(param: String) = config[param]?.toBoolean() ?: mapConfiguration[param]!! } companion object { val ignoreNumbersList = listOf("-1", "1", "0", "2") - val mapConfiguration = mapOf( + val mapConfiguration = mapOf( "ignoreHashCodeFunction" to true, "ignorePropertyDeclaration" to false, "ignoreLocalVariableDeclaration" to false, diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/FileSizeWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/FileSizeWarnTest.kt index 17b5ddadb1..c94418d858 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/FileSizeWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/FileSizeWarnTest.kt @@ -118,11 +118,15 @@ class FileSizeWarnTest : LintTestBase(::FileSize) { lintMethod(file.readText(), fileName = file.absolutePath, rulesConfigList = rulesConfigListTwoIgnoreFolders) path = javaClass.classLoader.getResource("test/paragraph3/src/main/C/FileSizeC.kt") file = File(path!!.file) - val size = 10 + val size = SIZE lintMethod(file.readText(), LintError(1, 1, "$DIKTAT_RULE_SET_ID:file-size", "${Warnings.FILE_IS_TOO_LONG.warnText()} $size", false), fileName = file.absolutePath, rulesConfigList = rulesConfigListTwoIgnoreFolders) } + + companion object { + private const val SIZE = 10 + } } diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/AvailableRulesDocTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/AvailableRulesDocTest.kt index 018d5f5218..8f03e57175 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/AvailableRulesDocTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/AvailableRulesDocTest.kt @@ -16,7 +16,7 @@ class AvailableRulesDocTest { val splitMarkDown = line .split("|") - val ruleName = splitMarkDown.get(3).trim() + val ruleName = splitMarkDown.get(SPLIT_MARK).trim() if (!ruleName.startsWith(TABLE_DELIMITER) && !ruleName.startsWith(RULE_NAME_HEADER)) { @@ -59,6 +59,7 @@ class AvailableRulesDocTest { companion object { const val AVAILABLE_RULES_FILE = "../info/available-rules.md" const val RULE_NAME_HEADER = "Rule name" + const val SPLIT_MARK = 3 const val TABLE_DELIMITER = "-----" } } From 0050d9176124e391856a0c5e05198a687dd1d46c Mon Sep 17 00:00:00 2001 From: kentr0w Date: Sat, 13 Feb 2021 22:47:09 +0300 Subject: [PATCH 08/11] Magic Number ### What's done: Fixed according to our code style --- .../org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt index 7b5ce0a88b..44576305b9 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt @@ -52,7 +52,7 @@ class MagicNumberRule(configRules: List) : DiktatRule("magic-number val isEnums = node.parent({ it.elementType == ENUM_ENTRY }) != null val isRanges = node.treeParent.run { this.elementType == BINARY_EXPRESSION && - this.findChildByType(OPERATION_REFERENCE)?.hasChildOfType(RANGE) ?: false + this.findChildByType(OPERATION_REFERENCE)?.hasChildOfType(RANGE) ?: false } val isExtensionFunctions = node.parent({ it.elementType == FUN && (it.psi as KtFunction).isExtensionDeclaration() }) != null && node.parents().find { it.elementType == PROPERTY } == null From 0f2eb4f5a130345c82319092cd402763eb16596b Mon Sep 17 00:00:00 2001 From: kentr0w Date: Mon, 22 Mar 2021 13:00:22 +0300 Subject: [PATCH 09/11] Magic Number ### What's done: Fixed after review --- .../cqfn/diktat/ruleset/constants/Warnings.kt | 2 +- .../ruleset/rules/chapter3/MagicNumberRule.kt | 24 +++++++++++-------- .../chapter3/MagicNumberRuleWarnTest.kt | 9 +++---- info/guide/guide-chapter-3.md | 7 ++++++ 4 files changed, 27 insertions(+), 15 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt index 34abc942e5..dd3fb7a7ac 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt @@ -112,7 +112,7 @@ enum class Warnings( ENUMS_SEPARATED(true, "3.9.1", "enum is incorrectly formatted"), WHEN_WITHOUT_ELSE(true, "3.11.1", "each 'when' statement must have else at the end"), LONG_NUMERICAL_VALUES_SEPARATED(true, "3.14.2", "long numerical values should be separated with underscore"), - MAGIC_NUMBER(false, "3.14.3", "defining constants with clear names describing what the magic number means"), + MAGIC_NUMBER(false, "3.14.3", "avoid using magic numbers, instead define constants with clear names describing what the magic number means"), WRONG_DECLARATIONS_ORDER(true, "3.1.4", "declarations of constants and enum members should be sorted alphabetically"), WRONG_MULTIPLE_MODIFIERS_ORDER(true, "3.14.1", "sequence of modifier-keywords is incorrect"), LOCAL_VARIABLE_EARLY_DECLARATION(false, "3.10.2", "local variables should be declared close to the line where they are first used"), diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt index 44576305b9..c9ceb8e0c5 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt @@ -5,10 +5,6 @@ import org.cqfn.diktat.common.config.rules.RulesConfig import org.cqfn.diktat.common.config.rules.getRuleConfig import org.cqfn.diktat.ruleset.constants.Warnings.MAGIC_NUMBER import org.cqfn.diktat.ruleset.rules.DiktatRule -import org.cqfn.diktat.ruleset.utils.hasChildOfType -import org.cqfn.diktat.ruleset.utils.isConstant -import org.cqfn.diktat.ruleset.utils.isNodeFromCompanionObject -import org.cqfn.diktat.ruleset.utils.isVarProperty import com.pinterest.ktlint.core.ast.ElementType.BINARY_EXPRESSION import com.pinterest.ktlint.core.ast.ElementType.ENUM_ENTRY @@ -20,6 +16,7 @@ import com.pinterest.ktlint.core.ast.ElementType.OPERATION_REFERENCE import com.pinterest.ktlint.core.ast.ElementType.PROPERTY import com.pinterest.ktlint.core.ast.ElementType.RANGE import com.pinterest.ktlint.core.ast.parent +import org.cqfn.diktat.ruleset.utils.* import org.jetbrains.kotlin.com.intellij.lang.ASTNode import org.jetbrains.kotlin.psi.KtFunction import org.jetbrains.kotlin.psi.KtProperty @@ -30,10 +27,12 @@ import org.jetbrains.kotlin.psi.psiUtil.parents * Rule for magic number */ class MagicNumberRule(configRules: List) : DiktatRule("magic-number", configRules, listOf(MAGIC_NUMBER)) { - override fun logic(node: ASTNode) { - val configuration = MagicNumberConfiguration( + private val configuration by lazy { + MagicNumberConfiguration( configRules.getRuleConfig(MAGIC_NUMBER)?.configuration ?: emptyMap() ) + } + override fun logic(node: ASTNode) { if (node.elementType == INTEGER_CONSTANT || node.elementType == FLOAT_CONSTANT) { checkNumber(node, configuration) } @@ -43,8 +42,7 @@ class MagicNumberRule(configRules: List) : DiktatRule("magic-number private fun checkNumber(node: ASTNode, configuration: MagicNumberConfiguration) { val nodeText = node.treePrev?.let { if (it.elementType == OPERATION_REFERENCE && it.hasChildOfType(MINUS)) "-${node.text}" else node.text } ?: node.text val isIgnoreNumber = configuration.ignoreNumbers.contains(nodeText) - val isHashFunction = node.parent({ it.elementType == FUN && it.isHashFun() }) != null && - node.parents().find { it.elementType == PROPERTY } == null + val isHashFunction = node.parent({ it.elementType == FUN && it.isHashFun() }) != null val isPropertyDeclaration = node.parent({ it.elementType == PROPERTY && !it.isNodeFromCompanionObject() }) != null val isLocalVariable = node.parent({ it.isVarProperty() && (it.psi as KtProperty).isLocal }) != null val isConstant = node.parent({ it.elementType == PROPERTY && it.isConstant() }) != null @@ -73,9 +71,9 @@ class MagicNumberRule(configRules: List) : DiktatRule("magic-number */ class MagicNumberConfiguration(config: Map) : RuleConfiguration(config) { /** - * List of ignore numbers + * List of ignored numbers */ - val ignoreNumbers = config["ignoreNumbers"]?.split(",")?.map { it.trim() } ?: ignoreNumbersList + val ignoreNumbers = config["ignoreNumbers"]?.split(",")?.map { it.trim() }?.filter { it.isNumber() } ?: ignoreNumbersList /** * @param param parameter from config @@ -83,6 +81,11 @@ class MagicNumberRule(configRules: List) : DiktatRule("magic-number */ @Suppress("UnsafeCallOnNullableType") fun getParameter(param: String) = config[param]?.toBoolean() ?: mapConfiguration[param]!! + + /** + * Check if string is number + */ + private fun String.isNumber() = (this.toLongOrNull() ?: this.toFloatOrNull()) != null } companion object { @@ -97,4 +100,5 @@ class MagicNumberRule(configRules: List) : DiktatRule("magic-number "ignoreRanges" to false, "ignoreExtensionFunctions" to false) } + } diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/MagicNumberRuleWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/MagicNumberRuleWarnTest.kt index 5ae8daaf2c..731a4a4250 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/MagicNumberRuleWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/MagicNumberRuleWarnTest.kt @@ -30,7 +30,7 @@ class MagicNumberRuleWarnTest : LintTestBase(::MagicNumberRule) { RulesConfig( MAGIC_NUMBER.name, true, mapOf( - "ignoreNumbers" to "50,-240, 0xFF, -3.5f")) + "ignoreNumbers" to "50,-240, 128L, -3.5f")) ) @Test @@ -40,7 +40,7 @@ class MagicNumberRuleWarnTest : LintTestBase(::MagicNumberRule) { """ |fun f1oo() { | val a: Byte = 4 - | val b = 0xff + | val b = 128L | val e = 3.4f | val g = 4/3 | val f = "qwe\$\{12\}hhe" @@ -58,7 +58,7 @@ class MagicNumberRuleWarnTest : LintTestBase(::MagicNumberRule) { |} """.trimMargin(), LintError(2, 18, ruleId, "${MAGIC_NUMBER.warnText()} 4", false), - LintError(3, 12, ruleId, "${MAGIC_NUMBER.warnText()} 0xff", false), + LintError(3, 12, ruleId, "${MAGIC_NUMBER.warnText()} 128L", false), LintError(4, 12, ruleId, "${MAGIC_NUMBER.warnText()} 3.4f", false), LintError(5, 12, ruleId, "${MAGIC_NUMBER.warnText()} 4", false), LintError(5, 14, ruleId, "${MAGIC_NUMBER.warnText()} 3", false) @@ -73,7 +73,7 @@ class MagicNumberRuleWarnTest : LintTestBase(::MagicNumberRule) { |fun f1oo() { | val m = -1 | val a: Byte = 4 - | val b = 0xFF + | val b = 0xff |} | |enum class A(b:Int) { @@ -88,6 +88,7 @@ class MagicNumberRuleWarnTest : LintTestBase(::MagicNumberRule) { """.trimMargin(), LintError(2, 13, ruleId, "${MAGIC_NUMBER.warnText()} -1", false), LintError(3, 18, ruleId, "${MAGIC_NUMBER.warnText()} 4", false), + LintError(4, 12, ruleId, "${MAGIC_NUMBER.warnText()} 0xff", false), LintError(10, 6, ruleId, "${MAGIC_NUMBER.warnText()} 3", false), rulesConfigList = rulesConfigIgnoreNumbersMagicNumber ) diff --git a/info/guide/guide-chapter-3.md b/info/guide/guide-chapter-3.md index 3049f63487..fd8d69543a 100644 --- a/info/guide/guide-chapter-3.md +++ b/info/guide/guide-chapter-3.md @@ -830,6 +830,13 @@ class Person() { } } ``` +**InValid example**: +```kotlin +class Person() { + val oldAge = 65 + fun isAdult(age: Int): Boolean = age >= 18 && age <= oldAge +} +``` ### 3.15 Strings From deb05c339ee8192f75ec99747ece3ee88cd727e2 Mon Sep 17 00:00:00 2001 From: kentr0w Date: Mon, 22 Mar 2021 13:15:29 +0300 Subject: [PATCH 10/11] Magic Number ### What's done: Fixed according to our code-style --- .../diktat/ruleset/rules/chapter3/MagicNumberRule.kt | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt index c9ceb8e0c5..828016e2c2 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt @@ -5,6 +5,7 @@ import org.cqfn.diktat.common.config.rules.RulesConfig import org.cqfn.diktat.common.config.rules.getRuleConfig import org.cqfn.diktat.ruleset.constants.Warnings.MAGIC_NUMBER import org.cqfn.diktat.ruleset.rules.DiktatRule +import org.cqfn.diktat.ruleset.utils.* import com.pinterest.ktlint.core.ast.ElementType.BINARY_EXPRESSION import com.pinterest.ktlint.core.ast.ElementType.ENUM_ENTRY @@ -16,7 +17,6 @@ import com.pinterest.ktlint.core.ast.ElementType.OPERATION_REFERENCE import com.pinterest.ktlint.core.ast.ElementType.PROPERTY import com.pinterest.ktlint.core.ast.ElementType.RANGE import com.pinterest.ktlint.core.ast.parent -import org.cqfn.diktat.ruleset.utils.* import org.jetbrains.kotlin.com.intellij.lang.ASTNode import org.jetbrains.kotlin.psi.KtFunction import org.jetbrains.kotlin.psi.KtProperty @@ -26,7 +26,11 @@ import org.jetbrains.kotlin.psi.psiUtil.parents /** * Rule for magic number */ -class MagicNumberRule(configRules: List) : DiktatRule("magic-number", configRules, listOf(MAGIC_NUMBER)) { +class MagicNumberRule(configRules: List) : DiktatRule( + "magic-number", + configRules, + listOf(MAGIC_NUMBER) +) { private val configuration by lazy { MagicNumberConfiguration( configRules.getRuleConfig(MAGIC_NUMBER)?.configuration ?: emptyMap() @@ -100,5 +104,4 @@ class MagicNumberRule(configRules: List) : DiktatRule("magic-number "ignoreRanges" to false, "ignoreExtensionFunctions" to false) } - } From f29080200ca9a10f428397c03f0dd3dfe142997c Mon Sep 17 00:00:00 2001 From: kentr0w Date: Mon, 22 Mar 2021 17:04:19 +0300 Subject: [PATCH 11/11] Magic Number ### What's done: Fixed after review --- info/guide/guide-chapter-3.md | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/info/guide/guide-chapter-3.md b/info/guide/guide-chapter-3.md index fa885c77f0..41455eca45 100644 --- a/info/guide/guide-chapter-3.md +++ b/info/guide/guide-chapter-3.md @@ -824,19 +824,17 @@ Prefer defining constants with clear names describing what the magic number mean **Valid example**: ```kotlin class Person() { - fun isAdult(age: Int): Boolean = age >= majority companion object { - const val majority = 18 + private const val majority = 18 } } ``` -**InValid example**: +**Invalid example**: ```kotlin class Person() { - val oldAge = 65 - fun isAdult(age: Int): Boolean = age >= 18 && age <= oldAge + fun isAdult(age: Int): Boolean = age >= 18 } ```