### What's done:

- fixed warning `VARIABLE_NAME_INCORRECT_FORMAT` on `backing field`. - added warning test. Closes #1711
saveourtool · Nov 14, 2023 · 4dce6b6 · 4dce6b6
1 parent 7b7d2b6
commit 4dce6b6
Show file tree

Hide file tree

Showing 3 changed files with 113 additions and 30 deletions.
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
@@ -30,10 +30,15 @@ import org.jetbrains.kotlin.KtNodeTypes.CLASS
 import org.jetbrains.kotlin.KtNodeTypes.DESTRUCTURING_DECLARATION
 import org.jetbrains.kotlin.KtNodeTypes.DESTRUCTURING_DECLARATION_ENTRY
 import org.jetbrains.kotlin.KtNodeTypes.FUNCTION_TYPE
+import org.jetbrains.kotlin.KtNodeTypes.MODIFIER_LIST
+import org.jetbrains.kotlin.KtNodeTypes.NULLABLE_TYPE
 import org.jetbrains.kotlin.KtNodeTypes.OBJECT_DECLARATION
+import org.jetbrains.kotlin.KtNodeTypes.PROPERTY
+import org.jetbrains.kotlin.KtNodeTypes.PROPERTY_ACCESSOR
 import org.jetbrains.kotlin.KtNodeTypes.REFERENCE_EXPRESSION
 import org.jetbrains.kotlin.KtNodeTypes.TYPE_PARAMETER
 import org.jetbrains.kotlin.KtNodeTypes.TYPE_REFERENCE
+import org.jetbrains.kotlin.KtNodeTypes.USER_TYPE
 import org.jetbrains.kotlin.KtNodeTypes.VALUE_PARAMETER_LIST
 import org.jetbrains.kotlin.com.intellij.lang.ASTNode
 import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement
@@ -43,6 +48,7 @@ import org.jetbrains.kotlin.kdoc.parser.KDocKnownTag
 import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.lexer.KtTokens.CATCH_KEYWORD
 import org.jetbrains.kotlin.lexer.KtTokens.IDENTIFIER
+import org.jetbrains.kotlin.lexer.KtTokens.PRIVATE_KEYWORD
 import org.jetbrains.kotlin.psi.KtParameter
 import org.jetbrains.kotlin.psi.KtPrimaryConstructor
 import org.jetbrains.kotlin.psi.KtProperty
@@ -149,9 +155,12 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(
         "ComplexMethod",
         "UnsafeCallOnNullableType",
     )
+
     private fun checkVariableName(node: ASTNode): List<ASTNode> {
         // special case for Destructuring declarations that can be treated as parameters in lambda:
         var namesOfVariables = extractVariableIdentifiers(node)
+        val propertyNodes = node.treeParent.getAllChildrenWithType(KtNodeTypes.PROPERTY)
+
         // Only local private properties will be autofix in order not to break code if there are usages in other files.
         // Destructuring declarations are only allowed for local variables/values, so we don't need to calculate `isFix` for every node in `namesOfVariables`
         val isPublicOrNonLocalProperty = if (node.elementType == KtNodeTypes.PROPERTY) (node.psi as KtProperty).run { !isLocal && !isPrivate() } else false
@@ -189,38 +198,55 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(
                         }
                     }
                 } else if (variableName.text != "_" && !variableName.text.isLowerCamelCase()) {
+                    var isCorrectBackingField = false
+                    if (variableName.text.commonPrefixWith("_").length == 1 && variableName.text.drop(1).isLowerCamelCase()) {
+                        val matchingNode = propertyNodes.find { propertyNode ->
+                            val nodeType = node.getFirstChildWithType(TYPE_REFERENCE)
+                            val propertyType = propertyNode.getFirstChildWithType(TYPE_REFERENCE)
+                            propertyNode.getFirstChildWithType(IDENTIFIER)?.text == variableName.text.drop(1) &&
+                                    (propertyType?.text == nodeType?.text ||
+                                            propertyType?.getFirstChildWithType(USER_TYPE)?.text ==
+                                                    nodeType?.getFirstChildWithType(NULLABLE_TYPE)?.getFirstChildWithType(USER_TYPE)?.text) &&
+                                    node.getFirstChildWithType(MODIFIER_LIST)?.getFirstChildWithType(PRIVATE_KEYWORD) != null &&
+                                    node.getFirstChildWithType(PROPERTY_ACCESSOR) == null &&
+                                    propertyNode.getFirstChildWithType(PROPERTY_ACCESSOR) != null
+                        }
+                        matchingNode.let { isCorrectBackingField = true }
+                    }
                     // 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.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
-                            .parent { it.elementType == KtFileElementType.INSTANCE }
-                            ?.findAllVariablesWithUsages { it.name == variableName.text }
-                            ?.flatMap { it.value.toList() }
-                            ?.forEach { (it.node.firstChildNode as LeafPsiElement).rawReplaceWithText(correctVariableName) }
-                        if (variableName.treeParent.psi.run {
-                            (this is KtProperty && isMember) ||
-                                    (this is KtParameter && getParentOfType<KtPrimaryConstructor>(true)?.valueParameters?.contains(this) == true)
-                        }) {
-                            // For class members also check `@property` KDoc tag.
-                            // If we are here, then `variableName` is definitely a node from a class or an object.
-                            (variableName.parent(CLASS) ?: variableName.parent(OBJECT_DECLARATION))?.findChildByType(KDOC)?.kDocTags()
-                                ?.firstOrNull {
-                                    it.knownTag == KDocKnownTag.PROPERTY && it.getSubjectName() == variableName.text
-                                }
-                                ?.run {
-                                    (getSubjectLink()!!.node.findAllDescendantsWithSpecificType(IDENTIFIER).single() as LeafPsiElement).rawReplaceWithText(correctVariableName)
-                                }
+                    if (!isCorrectBackingField) {
+                        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
+                                .parent { it.elementType == KtFileElementType.INSTANCE }
+                                ?.findAllVariablesWithUsages { it.name == variableName.text }
+                                ?.flatMap { it.value.toList() }
+                                ?.forEach { (it.node.firstChildNode as LeafPsiElement).rawReplaceWithText(correctVariableName) }
+                            if (variableName.treeParent.psi.run {
+                                (this is KtProperty && isMember) ||
+                                        (this is KtParameter && getParentOfType<KtPrimaryConstructor>(true)?.valueParameters?.contains(this) == true)
+                            }) {
+                                // For class members also check `@property` KDoc tag.
+                                // If we are here, then `variableName` is definitely a node from a class or an object.
+                                (variableName.parent(CLASS) ?: variableName.parent(OBJECT_DECLARATION))?.findChildByType(KDOC)?.kDocTags()
+                                    ?.firstOrNull {
+                                        it.knownTag == KDocKnownTag.PROPERTY && it.getSubjectName() == variableName.text
+                                    }
+                                    ?.run {
+                                        (getSubjectLink()!!.node.findAllDescendantsWithSpecificType(IDENTIFIER).single() as LeafPsiElement).rawReplaceWithText(correctVariableName)
+                                    }
+                            }
+                            (variableName as LeafPsiElement).rawReplaceWithText(correctVariableName)
                         }
-                        (variableName as LeafPsiElement).rawReplaceWithText(correctVariableName)
                     }
                 }
             }

diff --git a/...rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter1/IdentifierNamingWarnTest.kt b/...rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter1/IdentifierNamingWarnTest.kt
@@ -23,7 +23,14 @@ import com.saveourtool.diktat.ruleset.rules.chapter1.IdentifierNaming
 import com.saveourtool.diktat.util.LintTestBase
 
 import com.saveourtool.diktat.api.DiktatError
+import com.saveourtool.diktat.ruleset.constants.Warnings
+import com.saveourtool.diktat.ruleset.utils.getFirstChildWithType
+import com.saveourtool.diktat.ruleset.utils.hasAnyChildOfTypes
+import com.saveourtool.diktat.ruleset.utils.prettyPrint
 import generated.WarningNames
+import org.jetbrains.kotlin.KtNodeTypes
+import org.jetbrains.kotlin.com.intellij.lang.ASTNode
+import org.jetbrains.kotlin.lexer.KtTokens
 import org.junit.jupiter.api.Tag
 import org.junit.jupiter.api.Tags
 import org.junit.jupiter.api.Test
@@ -629,6 +636,33 @@ class IdentifierNamingWarnTest : LintTestBase(::IdentifierNaming) {
         lintMethod(code)
     }
 
+    @Test
+    @Tag(WarningNames.VARIABLE_NAME_INCORRECT_FORMAT)
+    fun `should not trigger on backing field`() {
+        //language=kotlin
+        lintMethod(
+            """
+                    |package com.example
+                    |
+                    |class MutableTableContainer {
+                    |   private var _table: Map<String, Int>? = null
+                    |
+                    |   val table: Map<String, Int>
+                    |       get() {
+                    |           if (_table == null) {
+                    |               _table = hashMapOf()
+                    |           }
+                    |           return _table ?: throw AssertionError("Set to null by another thread")
+                    |       }
+                    |       set(value) {
+                    |           field = value
+                    |       }
+                    |
+                    |}
+            """.trimMargin(),
+        )
+    }
+
     @Test
     @Tag(WarningNames.CONFUSING_IDENTIFIER_NAMING)
     fun `confusing identifier naming`() {

diff --git a/...les/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter6/CustomGetterSetterWarnTest.kt b/...les/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter6/CustomGetterSetterWarnTest.kt
@@ -102,4 +102,27 @@ class CustomGetterSetterWarnTest : LintTestBase(::CustomGetterSetterRule) {
             DiktatError(7, 9, ruleId, "${Warnings.CUSTOM_GETTERS_SETTERS.warnText()} get"),
         )
     }
+
+    @Test
+    @Tag(CUSTOM_GETTERS_SETTERS)
+    fun `exception for accessors or modifiers of property`() {
+        lintMethod(
+            """
+                    |package com.example
+                    |
+                    |class MutableTableContainer {
+                    |   private var _table: Map<String, Int>? = null
+                    |
+                    |   val table: Map<String, Int>
+                    |       get() {
+                    |           if (_table == null) {
+                    |               _table = hashMapOf()
+                    |           }
+                    |           return _table ?: throw AssertionError("Set to null by another thread")
+                    |       }
+                    |}
+            """.trimMargin(),
+        )
+    }
 }
+