Magic Number

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

diktat-common/src/test/kotlin/org/cqfn/diktat/test/ConfigReaderTest.kt

@@ -23,12 +23,15 @@ class ConfigReaderTest {
     fun `testing kotlin version`() {
         val rulesConfigList: List<RulesConfig>? = 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)
     }
 }

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"

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.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"),

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,

diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/LineLength.kt

@@ -381,6 +381,7 @@ class LineLength(configRules: List<RulesConfig>) : DiktatRule("line-length", con
     }
 
     /**
+     *
      * [RuleConfiguration] for maximum line length
      */
     class LineLengthConfiguration(config: Map<String, String>) : RuleConfiguration(config) {

diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/MagicNumberRule.kt

@@ -0,0 +1,100 @@
+package org.cqfn.diktat.ruleset.rules.chapter3
+
+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 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.parents
+
+/**
+ * Rule for magic number
+ */
+class MagicNumberRule(configRules: List<RulesConfig>) : 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)
+        }
+    }
+
+    @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 && !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) {
+            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<String, String>) : RuleConfiguration(config) {
+        /**
+         * List of ignore numbers
+         */
+        val ignoreNumbers = config["ignoreNumbers"]?.split(",")?.map { it.trim() } ?: ignoreNumbersList
+
+        /**
+         * @param param parameter from config
+         * @return value of parameter
+         */
+        @Suppress("UnsafeCallOnNullableType")
+        fun getParameter(param: String) = 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,
+            "ignoreRanges" to false,
+            "ignoreExtensionFunctions" to false)
+    }
+}

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

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

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
+    }
 }

diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/MagicNumberRuleWarnTest.kt

@@ -0,0 +1,127 @@
+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<RulesConfig> = 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<RulesConfig> = 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 e = 3.4f
+                |   val g = 4/3
+                |   val f = "qwe\$\{12\}hhe"
+                |}
+                |
+                |@Override                
+                |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),
+            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
+        )
+    }
+}