Refactored warn and warnAndFix in Warnings (#1751)

- removed `isFixMode` from `warn` - removed `canBeAutoCorrected` from `warnAndFix`
saveourtool · Sep 26, 2023 · aa26135 · aa26135
1 parent 3c4e688
commit aa26135
Show file tree

Hide file tree

Showing 46 changed files with 173 additions and 122 deletions.
diff --git a/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/constants/Warnings.kt b/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/constants/Warnings.kt
@@ -1,20 +1,13 @@
 package com.saveourtool.diktat.ruleset.constants
 
+import com.saveourtool.diktat.api.DiktatErrorEmitter
 import com.saveourtool.diktat.common.config.rules.Rule
 import com.saveourtool.diktat.common.config.rules.RulesConfig
 import com.saveourtool.diktat.common.config.rules.isRuleEnabled
 import com.saveourtool.diktat.ruleset.generation.EnumNames
 import com.saveourtool.diktat.ruleset.utils.isSuppressed
 import org.jetbrains.kotlin.com.intellij.lang.ASTNode
 
-typealias EmitType = ((offset: Int,
-                       errorMessage: String,
-                       canBeAutoCorrected: Boolean) -> Unit)
-
-typealias ListOfList = MutableList<MutableList<ASTNode>>
-
-typealias ListOfPairs = MutableList<Pair<ASTNode, String>>
-
 /**
  * This class represent individual inspections of diktat code style.
  * A [Warnings] entry contains rule name, warning message and is used in code check.
@@ -155,7 +148,7 @@ enum class Warnings(
 
     // FixMe: change float literal to BigDecimal? Or kotlin equivalent?
     FLOAT_IN_ACCURATE_CALCULATIONS(false, "4.1.1", "floating-point values shouldn't be used in accurate calculations"),
-    AVOID_NULL_CHECKS(false, "4.3.3", "Try to avoid explicit null-checks"),
+    AVOID_NULL_CHECKS(true, "4.3.3", "Try to avoid explicit null-checks"),
 
     // ======== chapter 5 ========
     TOO_LONG_FUNCTION(false, "5.1.1", "function is too long: split it or make more primitive"),
@@ -201,53 +194,95 @@ enum class Warnings(
      */
     fun warnText() = "[${ruleName()}] ${this.warn}:"
 
+    /**
+     * @param configRules list of [RulesConfig]
+     * @param emit function that will be called on warning
+     * @param freeText text that will be added to the warning message
+     * @param offset offset from the beginning of the file
+     * @param node the [ASTNode] on which the warning was triggered
+     * @param shouldBeAutoCorrected should be auto corrected or not
+     * @param isFixMode whether autocorrect mode is on
+     * @param autoFix function that will be called to autocorrect the warning
+     */
+    @Suppress("LongParameterList", "TOO_MANY_PARAMETERS")
+    fun warnOnlyOrWarnAndFix(
+        configRules: List<RulesConfig>,
+        emit: DiktatErrorEmitter,
+        freeText: String,
+        offset: Int,
+        node: ASTNode,
+        shouldBeAutoCorrected: Boolean,
+        isFixMode: Boolean,
+        autoFix: () -> Unit,
+    ) {
+        if (shouldBeAutoCorrected) {
+            warnAndFix(configRules, emit, isFixMode, freeText, offset, node, autoFix)
+        } else {
+            warn(configRules, emit, freeText, offset, node)
+        }
+    }
+
     /**
      * @param configRules list of [RulesConfig]
      * @param emit function that will be called on warning
      * @param isFixMode whether autocorrect mode is on
      * @param freeText text that will be added to the warning message
      * @param offset offset from the beginning of the file
      * @param node the [ASTNode] on which the warning was triggered
-     * @param canBeAutoCorrected whether this warning can be autocorrected
      * @param autoFix function that will be called to autocorrect the warning
      */
     @Suppress("LongParameterList", "TOO_MANY_PARAMETERS")
-    fun warnAndFix(configRules: List<RulesConfig>,
-                   emit: EmitType,
-                   isFixMode: Boolean,
-                   freeText: String,
-                   offset: Int,
-                   node: ASTNode,
-                   canBeAutoCorrected: Boolean = this.canBeAutoCorrected,
-                   autoFix: () -> Unit) {
-        warn(configRules, emit, canBeAutoCorrected, freeText, offset, node)
-        if (canBeAutoCorrected) {
-            fix(configRules, isFixMode, node, autoFix)
+    fun warnAndFix(
+        configRules: List<RulesConfig>,
+        emit: DiktatErrorEmitter,
+        isFixMode: Boolean,
+        freeText: String,
+        offset: Int,
+        node: ASTNode,
+        autoFix: () -> Unit,
+    ) {
+        require(canBeAutoCorrected) {
+            "warnAndFix is called, but canBeAutoCorrected is false"
         }
+        doWarn(configRules, emit, freeText, offset, node, true)
+        fix(configRules, isFixMode, node, autoFix)
     }
 
     /**
      * @param configs list of [RulesConfig]
      * @param emit function that will be called on warning
-     * @param autoCorrected whether this warning can be autocorrected
      * @param freeText text that will be added to the warning message
      * @param offset offset from the beginning of the file
      * @param node the [ASTNode] on which the warning was triggered
      */
     @Suppress("LongParameterList", "TOO_MANY_PARAMETERS")
-    fun warn(configs: List<RulesConfig>,
-             emit: EmitType,
-             autoCorrected: Boolean,
-             freeText: String,
-             offset: Int,
-             node: ASTNode) {
+    fun warn(
+        configs: List<RulesConfig>,
+        emit: DiktatErrorEmitter,
+        freeText: String,
+        offset: Int,
+        node: ASTNode,
+    ) {
+        doWarn(configs, emit, freeText, offset, node, false)
+    }
+
+    @Suppress("LongParameterList", "TOO_MANY_PARAMETERS")
+    private fun doWarn(
+        configs: List<RulesConfig>,
+        emit: DiktatErrorEmitter,
+        freeText: String,
+        offset: Int,
+        node: ASTNode,
+        canBeAutoCorrected: Boolean,
+    ) {
         if (isRuleFromActiveChapter(configs) && configs.isRuleEnabled(this) && !node.isSuppressed(name, this, configs)) {
             val trimmedFreeText = freeText
                 .lines()
                 .run { if (size > 1) "${first()}..." else first() }
-            emit(offset,
-                "${this.warnText()} $trimmedFreeText",
-                autoCorrected
+            emit(
+                offset,
+                errorMessage = "${this.warnText()} $trimmedFreeText",
+                canBeAutoCorrected = canBeAutoCorrected,
             )
         }
     }
@@ -264,7 +299,7 @@ enum class Warnings(
 
     companion object {
         val names by lazy {
-            values().map { it.name }
+            entries.map { it.name }
         }
     }
 }
diff --git a/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter1/FileNaming.kt b/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter1/FileNaming.kt
@@ -46,9 +46,8 @@ class FileNaming(configRules: List<RulesConfig>) : DiktatRule(
     private fun checkFileNaming(node: ASTNode) {
         val (name, extension) = getFileParts(filePath)
         if (!name.isPascalCase() || !validExtensions.contains(extension)) {
-            FILE_NAME_INCORRECT.warnAndFix(configRules, emitWarn, isFixMode, "$name$extension", 0, node) {
-                // FixMe: we can add an autocorrect here in future, but is there any purpose to change file or class name?
-            }
+            // FixMe: we can add an autocorrect here in future, but is there any purpose to change file or class name?
+            FILE_NAME_INCORRECT.warn(configRules, emitWarn, "$name$extension", 0, node)
         }
     }
 
@@ -59,9 +58,8 @@ class FileNaming(configRules: List<RulesConfig>) : DiktatRule(
         if (classes.size == 1) {
             val className = classes[0].getFirstChildWithType(IDENTIFIER)!!.text
             if (className != fileNameWithoutSuffix) {
-                FILE_NAME_MATCH_CLASS.warnAndFix(configRules, emitWarn, isFixMode, "$fileNameWithoutSuffix$fileNameSuffix vs $className", 0, fileLevelNode) {
-                    // FixMe: we can add an autocorrect here in future, but is there any purpose to change file name?
-                }
+                // FixMe: we can add an autocorrect here in future, but is there any purpose to change file name?
+                FILE_NAME_MATCH_CLASS.warn(configRules, emitWarn, "$fileNameWithoutSuffix$fileNameSuffix vs $className", 0, fileLevelNode)
             }
         } else {
             // FixMe: need to check that if there are several classes - at least one of them should match

diff --git a/...t-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter1/IdentifierNaming.kt b/...t-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter1/IdentifierNaming.kt
@@ -131,7 +131,7 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(
             if (identifierText.startsWith('`') && identifierText.endsWith('`')) {
                 val isTestFun = node.elementType == KtNodeTypes.FUN && node.hasTestAnnotation()
                 if (!isTestFun) {
-                    BACKTICKS_PROHIBITED.warn(configRules, emitWarn, isFixMode, identifierText, identifier.startOffset, identifier)
+                    BACKTICKS_PROHIBITED.warn(configRules, emitWarn, identifierText, identifier.startOffset, identifier)
                 }
                 return true
             }
@@ -146,6 +146,7 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(
     @Suppress(
         "SAY_NO_TO_VAR",
         "TOO_LONG_FUNCTION",
+        "LongMethod",
         "ComplexMethod",
         "UnsafeCallOnNullableType",
     )
@@ -158,13 +159,13 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(
         val isNonPrivatePrimaryConstructorParameter = (node.psi as? KtParameter)?.run {
             hasValOrVar() && getParentOfType<KtPrimaryConstructor>(true)?.valueParameters?.contains(this) == true && !isPrivate()
         } ?: false
-        val canBeAutoCorrected = !(isPublicOrNonLocalProperty || isNonPrivatePrimaryConstructorParameter)
+        val shouldBeAutoCorrected = !(isPublicOrNonLocalProperty || isNonPrivatePrimaryConstructorParameter)
         namesOfVariables
             .forEach { variableName ->
                 // variable should not contain only one letter in it's name. This is a bad example: b512
                 // but no need to raise a warning here if length of a variable. In this case we will raise IDENTIFIER_LENGTH
                 if (variableName.text.containsOneLetterOrZero() && variableName.text.length > 1) {
-                    VARIABLE_NAME_INCORRECT.warn(configRules, emitWarn, false, variableName.text, variableName.startOffset, node)
+                    VARIABLE_NAME_INCORRECT.warn(configRules, emitWarn, variableName.text, variableName.startOffset, node)
                 }
                 // check if identifier of a property has a confusing name
                 if (confusingIdentifierNames.contains(variableName.text) && !isValidCatchIdentifier(variableName) &&
@@ -176,13 +177,29 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(
                 // it should be in UPPER_CASE, no need to raise this warning if it is one-letter variable
                 if (node.isConstant()) {
                     if (!variableName.text.isUpperSnakeCase() && variableName.text.length > 1) {
-                        CONSTANT_UPPERCASE.warnAndFix(configRules, emitWarn, isFixMode, variableName.text, variableName.startOffset, node, canBeAutoCorrected) {
+                        CONSTANT_UPPERCASE.warnOnlyOrWarnAndFix(
+                            configRules = configRules,
+                            emit = emitWarn,
+                            freeText = variableName.text,
+                            offset = variableName.startOffset,
+                            node = node,
+                            shouldBeAutoCorrected = shouldBeAutoCorrected,
+                            isFixMode = isFixMode,
+                        ) {
                             (variableName as LeafPsiElement).rawReplaceWithText(variableName.text.toDeterministic { toUpperSnakeCase() })
                         }
                     }
                 } else if (variableName.text != "_" && !variableName.text.isLowerCamelCase()) {
                     // variable name should be in camel case. The only exception is a list of industry standard variables like i, j, k.
-                    VARIABLE_NAME_INCORRECT_FORMAT.warnAndFix(configRules, emitWarn, isFixMode, variableName.text, variableName.startOffset, node, canBeAutoCorrected) {
+                    VARIABLE_NAME_INCORRECT_FORMAT.warnOnlyOrWarnAndFix(
+                        configRules = configRules,
+                        emit = emitWarn,
+                        freeText = variableName.text,
+                        offset = variableName.startOffset,
+                        node = node,
+                        shouldBeAutoCorrected = shouldBeAutoCorrected,
+                        isFixMode = isFixMode,
+                    ) {
                         // FixMe: cover fixes with tests
                         val correctVariableName = variableName.text.toDeterministic { toLowerCamelCase() }
                         variableName
@@ -240,7 +257,7 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(
             else -> ""
 
         }
-        CONFUSING_IDENTIFIER_NAMING.warn(configRules, emitWarn, false, warnText, variableName.startOffset, variableName)
+        CONFUSING_IDENTIFIER_NAMING.warn(configRules, emitWarn, warnText, variableName.startOffset, variableName)
     }
 
     /**
@@ -274,9 +291,8 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(
     private fun checkClassNamings(node: ASTNode): List<ASTNode> {
         val genericType: ASTNode? = node.getTypeParameterList()
         if (genericType != null && !validGenericTypeName(genericType)) {
-            GENERIC_NAME.warnAndFix(configRules, emitWarn, isFixMode, genericType.text, genericType.startOffset, genericType) {
-                // FixMe: should fix generic name here
-            }
+            // FixMe: should fix generic name here
+            GENERIC_NAME.warn(configRules, emitWarn, genericType.text, genericType.startOffset, genericType)
         }
 
         val className: ASTNode = node.getIdentifierName() ?: return emptyList()
@@ -397,9 +413,8 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(
             if (functionReturnType != null && functionReturnType == PrimitiveType.BOOLEAN.typeName.asString()) {
                 @Suppress("COLLAPSE_IF_STATEMENTS")
                 if (allMethodPrefixes.none { functionName.text.startsWith(it) }) {
-                    FUNCTION_BOOLEAN_PREFIX.warnAndFix(configRules, emitWarn, isFixMode, functionName.text, functionName.startOffset, functionName) {
-                        // FixMe: add agressive autofix for this
-                    }
+                    // FixMe: add agressive autofix for this
+                    FUNCTION_BOOLEAN_PREFIX.warn(configRules, emitWarn, functionName.text, functionName.startOffset, functionName)
                 }
             }
         }
@@ -441,7 +456,7 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(
             if (it.text != "_" && !it.isTextLengthInRange(MIN_IDENTIFIER_LENGTH..MAX_IDENTIFIER_LENGTH) &&
                     !isValidOneCharVariable && !isValidCatchIdentifier(it)
             ) {
-                IDENTIFIER_LENGTH.warn(configRules, emitWarn, isFixMode, it.text, it.startOffset, it)
+                IDENTIFIER_LENGTH.warn(configRules, emitWarn, it.text, it.startOffset, it)
             }
         }
     }

diff --git a/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter1/PackageNaming.kt b/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter1/PackageNaming.kt
@@ -168,7 +168,7 @@ class PackageNaming(configRules: List<RulesConfig>) : DiktatRule(
         // all words should contain only ASCII letters or digits
         wordsInPackageName
             .filter { word -> !areCorrectSymbolsUsed(word.text) }
-            .forEach { PACKAGE_NAME_INCORRECT_SYMBOLS.warn(configRules, emitWarn, isFixMode, it.text, it.startOffset, it) }
+            .forEach { PACKAGE_NAME_INCORRECT_SYMBOLS.warn(configRules, emitWarn, it.text, it.startOffset, it) }
 
         // all words should contain only ASCII letters or digits
         wordsInPackageName.forEach { correctPackageWordSeparatorsUsed(it) }

diff --git a/...es/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter2/comments/CommentsRule.kt b/...es/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter2/comments/CommentsRule.kt
@@ -1,7 +1,6 @@
 package com.saveourtool.diktat.ruleset.rules.chapter2.comments
 
 import com.saveourtool.diktat.common.config.rules.RulesConfig
-import com.saveourtool.diktat.ruleset.constants.ListOfPairs
 import com.saveourtool.diktat.ruleset.constants.Warnings.COMMENTED_OUT_CODE
 import com.saveourtool.diktat.ruleset.rules.DiktatRule
 import com.saveourtool.diktat.ruleset.utils.findAllDescendantsWithSpecificType
@@ -20,6 +19,8 @@ import org.jetbrains.kotlin.psi.KtPsiFactory
 import org.jetbrains.kotlin.psi.stubs.elements.KtFileElementType
 import org.jetbrains.kotlin.resolve.ImportPath
 
+private typealias ListOfPairs = MutableList<Pair<ASTNode, String>>
+
 /**
  * This rule performs checks if there is any commented code.
  * No commented out code is allowed, including imports.
@@ -105,7 +106,6 @@ class CommentsRule(configRules: List<RulesConfig>) : DiktatRule(
                     COMMENTED_OUT_CODE.warn(
                         configRules,
                         emitWarn,
-                        isFixMode,
                         parsedNode.text.substringBefore("\n").trim(),
                         offset,
                         invalidNode

diff --git a/...c/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter2/comments/HeaderCommentRule.kt b/...c/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter2/comments/HeaderCommentRule.kt
@@ -72,7 +72,7 @@ class HeaderCommentRule(configRules: List<RulesConfig>) : DiktatRule(
                 val numDeclaredClassesAndObjects = node.getAllChildrenWithType(KtNodeTypes.CLASS).size +
                         node.getAllChildrenWithType(KtNodeTypes.OBJECT_DECLARATION).size
                 if (numDeclaredClassesAndObjects != 1) {
-                    HEADER_MISSING_IN_NON_SINGLE_CLASS_FILE.warn(configRules, emitWarn, isFixMode,
+                    HEADER_MISSING_IN_NON_SINGLE_CLASS_FILE.warn(configRules, emitWarn,
                         "there are $numDeclaredClassesAndObjects declared classes and/or objects", node.startOffset, node)
                 }
             }

diff --git a/...-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter2/kdoc/KdocComments.kt b/...-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter2/kdoc/KdocComments.kt
@@ -106,7 +106,7 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
             .mapNotNull { it.nameIdentifier?.text }
         propertiesInKdoc
             .filterNot { it.getSubjectName() == null || it.getSubjectName() in propertyNames }
-            .forEach { KDOC_EXTRA_PROPERTY.warn(configRules, emitWarn, isFixMode, it.text, it.node.startOffset, node) }
+            .forEach { KDOC_EXTRA_PROPERTY.warn(configRules, emitWarn, it.text, it.node.startOffset, node) }
     }
 
     @Suppress("UnsafeCallOnNullableType", "ComplexMethod")
@@ -236,7 +236,7 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
         // if property is documented with KDoc, which has a property tag inside, then it can contain some additional more complicated
         // structure, that will be hard to move automatically
         val isFixable = propertyInLocalKdoc == null
-        KDOC_NO_CONSTRUCTOR_PROPERTY.warnAndFix(configRules, emitWarn, isFixMode, prevComment.text, prevComment.startOffset, node, isFixable) {
+        KDOC_NO_CONSTRUCTOR_PROPERTY.warnOnlyOrWarnAndFix(configRules, emitWarn, prevComment.text, prevComment.startOffset, node, shouldBeAutoCorrected = isFixable, isFixMode) {
             propertyInClassKdoc?.let {
                 // local docs should be appended to docs in class
                 appendKdocTagContent(propertyInClassKdoc, "\n$kdocText")
@@ -274,7 +274,7 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
         val traversedNodes: MutableSet<String?> = mutableSetOf()
         propertiesAndParams.forEach { property ->
             if (!traversedNodes.add(property.getSubjectName())) {
-                KDOC_DUPLICATE_PROPERTY.warn(configRules, emitWarn, isFixMode, property.text, property.node.startOffset, kdoc)
+                KDOC_DUPLICATE_PROPERTY.warn(configRules, emitWarn, property.text, property.node.startOffset, kdoc)
             }
         }
     }
@@ -326,7 +326,7 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
                         }
                         propertyInClassKdoc?.let {
                             // if property is documented as `@property`, then we suggest to move docs to the declaration inside the class body
-                            KDOC_NO_CLASS_BODY_PROPERTIES_IN_HEADER.warn(configRules, emitWarn, isFixMode, classElement.text, classElement.startOffset, classElement)
+                            KDOC_NO_CLASS_BODY_PROPERTIES_IN_HEADER.warn(configRules, emitWarn, classElement.text, classElement.startOffset, classElement)
                             return
                         }
                     }
@@ -359,7 +359,7 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
         }
 
         if (isModifierAccessibleOutsideOrActual && kdoc == null && !isTopLevelFunctionStandard(node)) {
-            warning.warn(configRules, emitWarn, isFixMode, name!!.text, node.startOffset, node)
+            warning.warn(configRules, emitWarn, name!!.text, node.startOffset, node)
         }
     }