Fix false positive in property-naming (#2141)

Fix false positive regarding property that potentially can contain data which is mutable

Closes #2140

CHANGELOG.md

@@ -21,6 +21,7 @@ This project adheres to [Semantic Versioning](https://semver.org/).
 * Fix spacing around colon in annotations `spacing-around-colon` ([#2093](https://github.com/pinterest/ktlint/issues/2093))
 * Do not wrap a binary expression after an elvis operator in case the max line length is exceeded ([#2128](https://github.com/pinterest/ktlint/issues/2128))
 * Fix indent of IS_EXPRESSION, PREFIX_EXPRESSION and POSTFIX_EXPRESSION in case it contains a linebreak `indent` [#2094](https://github.com/pinterest/ktlint/issues/2094)
+* Fix false positive regarding property that potentially can contain data which is mutable. `property-naming` [#2140](https://github.com/pinterest/ktlint/issues/2140)
 * Add new experimental rule `function-literal`. This rule enforces the parameter list of a function literal to be formatted consistently. `function-literal` [#2121](https://github.com/pinterest/ktlint/issues/2121)
 * Fix null pointer exception for if-else statement with empty THEN block `if-else-bracing` [#2135](https://github.com/pinterest/ktlint/issues/2135)
 

documentation/snapshot/docs/rules/experimental.md

@@ -637,20 +637,26 @@ Rule id: `package-naming` (`standard` rule set)
 
 Enforce naming of property.
 
+!!! note
+    This rule can not reliably detect all situations in which incorrect property naming is used. So it only detects in which it is certain that naming is incorrect.
+
 === "[:material-heart:](#) Ktlint"
 
     ```kotlin
-    val FOO_1 = Foo()
-    val FOO_BAR_1 = "FOO-BAR"
+    val foo1 = Foo() // In case developer want to communicate that Foo is mutable
+    val FOO1 = Foo() // In case developer want to communicate that Foo is deeply immutable
 
-    var foo1: Foo = Foo()
+    const val FOO_BAR = "FOO-BAR" // By definition deeply immutable
+
+    var foo2: Foo = Foo() // By definition not immutable
 
     class Bar {
-        const val FOO_2 = "foo"
-        const val FOO_BAR_2 = "FOO-BAR"
+        val foo1 = Foo() // In case developer want to communicate that Foo is mutable
+        val FOO1 = Foo() // In case developer want to communicate that Foo is deeply immutable
+
+        const val FOO_BAR = "FOO-BAR" // By definition deeply immutable
 
-        val foo2 = "foo"
-        val fooBar2 = "foo-bar"
+        var foo2: Foo = Foo() // By definition not immutable
 
         // Backing property
         private val _elementList = mutableListOf<Element>()
@@ -661,17 +667,12 @@ Enforce naming of property.
 === "[:material-heart-off-outline:](#) Disallowed"
 
     ```kotlin
-    val Foo1 = Foo()
-    val FooBar1 = "FOO-BAR"
+    const val fooBar = "FOO-BAR" // By definition deeply immutable
 
-    var FOO_1: Foo = Foo()
+    var FOO2: Foo = Foo() // By definition not immutable
 
     class Bar {
-        const val foo2 = "foo"
-        const val fooBar2 = "FOO-BAR"
-
-        val FOO2 = "foo"
-        val FOO_BAR_2 = "foo-bar"
+        val FOO_BAR = "FOO-BAR" // Class properties always start with lowercase, const is not allowed
 
         // Incomplete backing property as public property 'elementList1' is missing
         private val _elementList1 = mutableListOf<Element>()
@@ -683,9 +684,6 @@ Enforce naming of property.
     }
     ```
 
-!!! note
-    Top level `val` properties and `const val` properties have to be written in screaming snake notation. Local `val` and `const val` are written in lower camel case.
-
 This rule can also be suppressed with the IntelliJ IDEA inspection suppression `PropertyName`.
 
 Rule id: `property-naming` (`standard` rule set)

ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/PropertyNamingRule.kt

@@ -45,17 +45,15 @@ public class PropertyNamingRule :
             .findChildByType(IDENTIFIER)
             ?.let { identifier ->
                 when {
-                    property.hasCustomGetter() -> {
+                    property.hasConstModifier() -> {
+                        visitConstProperty(identifier, emit)
+                    }
+                    property.hasCustomGetter() || property.isTopLevelValue() || property.isObjectValue() -> {
                         // Can not reliably determine whether the value is immutable or not
                     }
                     property.isBackingProperty() -> {
                         visitBackingProperty(identifier, emit)
                     }
-                    property.hasConstModifier() ||
-                        property.isTopLevelValue() ||
-                        property.isObjectValue() -> {
-                        visitConstProperty(identifier, emit)
-                    }
                     else -> {
                         visitNonConstProperty(identifier, emit)
                     }

ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/PropertyNamingRuleTest.kt

@@ -1,14 +1,15 @@
 package com.pinterest.ktlint.ruleset.standard.rules
 
-import com.pinterest.ktlint.test.KtLintAssertThat
+import com.pinterest.ktlint.test.KtLintAssertThat.Companion.assertThatRule
+import com.pinterest.ktlint.test.KtlintDocumentationTest
+import com.pinterest.ktlint.test.LintViolation
 import org.junit.jupiter.api.Nested
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.params.ParameterizedTest
 import org.junit.jupiter.params.provider.ValueSource
 
 class PropertyNamingRuleTest {
-    private val propertyNamingRuleAssertThat =
-        KtLintAssertThat.assertThatRule { PropertyNamingRule() }
+    private val propertyNamingRuleAssertThat = assertThatRule { PropertyNamingRule() }
 
     @Test
     fun `Given a valid property name then do not emit`() {
@@ -49,19 +50,18 @@ class PropertyNamingRuleTest {
     }
 
     @Test
-    fun `Given a top level val property name not in screaming case notation then do emit`() {
+    fun `Issue 2140 - Given a top level val property name not in screaming case notation then do not emit as it can not be reliably be determined whether the value is (deeply) immutable`() {
         val code =
             """
             val foo = Foo()
             val FOO_BAR = FooBar()
             """.trimIndent()
         @Suppress("ktlint:standard:argument-list-wrapping", "ktlint:standard:max-line-length")
-        propertyNamingRuleAssertThat(code)
-            .hasLintViolationWithoutAutoCorrect(1, 5, "Property name should use the screaming snake case notation when the value can not be changed")
+        propertyNamingRuleAssertThat(code).hasNoLintViolations()
     }
 
     @Test
-    fun `Given an object val property name not having a custom get function and not in screaming case notation then do emit`() {
+    fun `Issue 2140 - Given an object val property name not having a custom get function and not in screaming case notation then do not emit as it can not be reliably be determined whether the value is (deeply) immutable`() {
         val code =
             """
             class Foo {
@@ -72,8 +72,7 @@ class PropertyNamingRuleTest {
             }
             """.trimIndent()
         @Suppress("ktlint:standard:argument-list-wrapping", "ktlint:standard:max-line-length")
-        propertyNamingRuleAssertThat(code)
-            .hasLintViolationWithoutAutoCorrect(3, 13, "Property name should use the screaming snake case notation when the value can not be changed")
+        propertyNamingRuleAssertThat(code).hasNoLintViolations()
     }
 
     @Test
@@ -208,4 +207,67 @@ class PropertyNamingRuleTest {
             propertyNamingRuleAssertThat(code).hasNoLintViolations()
         }
     }
+
+    @KtlintDocumentationTest
+    fun `Ktlint allowed examples`() {
+        val code =
+            """
+            val foo1 = Foo() // In case developer want to communicate that Foo is mutable
+            val FOO1 = Foo() // In case developer want to communicate that Foo is deeply immutable
+
+            const val FOO_BAR = "FOO-BAR" // By definition deeply immutable
+
+            var foo2: Foo = Foo() // By definition not immutable
+
+            class Bar {
+                val foo1 = "foo1" // Class properties always start with lowercase, const is not allowed
+
+                const val FOO_BAR = "FOO-BAR" // By definition deeply immutable
+
+                var foo2: Foo = Foo() // By definition not immutable
+
+                // Backing property
+                private val _elementList = mutableListOf<Element>()
+                val elementList: List<Element>
+                    get() = _elementList
+
+                companion object {
+                    val foo1 = Foo() // In case developer want to communicate that Foo is mutable
+                    val FOO1 = Foo() // In case developer want to communicate that Foo is deeply immutable
+                }
+            }
+            """.trimIndent()
+        propertyNamingRuleAssertThat(code).hasNoLintViolations()
+    }
+
+    @KtlintDocumentationTest
+    fun `Ktlint disallowed examples`() {
+        val code =
+            """
+            const val fooBar = "FOO-BAR" // By definition deeply immutable
+
+            var FOO2: Foo = Foo() // By definition not immutable
+
+            class Bar {
+                val FOO_BAR = "FOO-BAR" // Class properties always start with lowercase, const is not allowed
+
+                // Incomplete backing property as public property 'elementList1' is missing
+                private val _elementList1 = mutableListOf<Element>()
+
+                // Invalid backing property as '_elementList2' is not a private property
+                val _elementList2 = mutableListOf<Element>()
+                val elementList2: List<Element>
+                    get() = _elementList2
+            }
+            """.trimIndent()
+        @Suppress("ktlint:standard:argument-list-wrapping", "ktlint:standard:max-line-length")
+        propertyNamingRuleAssertThat(code)
+            .hasLintViolations(
+                LintViolation(1, 11, "Property name should use the screaming snake case notation when the value can not be changed", canBeAutoCorrected = false),
+                LintViolation(3, 5, "Property name should start with a lowercase letter and use camel case", canBeAutoCorrected = false),
+                LintViolation(6, 9, "Property name should start with a lowercase letter and use camel case", canBeAutoCorrected = false),
+                LintViolation(9, 17, "Property name should start with a lowercase letter and use camel case", canBeAutoCorrected = false),
+                LintViolation(12, 9, "Property name should start with a lowercase letter and use camel case", canBeAutoCorrected = false),
+            )
+    }
 }