Magic Number

diktat-analysis.yml

@@ -297,6 +297,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

@@ -169,6 +169,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

@@ -123,6 +123,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", "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"),

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

@@ -23,6 +23,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
@@ -193,6 +194,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

@@ -384,6 +384,7 @@ class LineLength(configRules: List<RulesConfig>) : DiktatRule(
     }
 
     /**
+     *
      * [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,107 @@
+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.*
+
+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)
+) {
+    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)
+        }
+    }
+
+    @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
+        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 ignored numbers
+         */
+        val ignoreNumbers = config["ignoreNumbers"]?.split(",")?.map { it.trim() }?.filter { it.isNumber() } ?: ignoreNumbersList
+
+        /**
+         * @param param parameter from config
+         * @return value of parameter
+         */
+        @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 {
+        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

@@ -297,6 +297,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

@@ -293,6 +293,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
+    }
 }