From 2d295281924efbcbfd5c9e8d0e634e72e5e20f68 Mon Sep 17 00:00:00 2001 From: paul-dingemans Date: Wed, 15 Nov 2023 21:02:04 +0100 Subject: [PATCH] Allow backing property to be correlated to a public function Closes #2332 --- .../standard/rules/PropertyNamingRule.kt | 48 ++++- .../standard/rules/PropertyNamingRuleTest.kt | 190 +++++++++++++----- 2 files changed, 179 insertions(+), 59 deletions(-) diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/PropertyNamingRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/PropertyNamingRule.kt index 375329afb4..d1e76a5e95 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/PropertyNamingRule.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/PropertyNamingRule.kt @@ -3,8 +3,10 @@ package com.pinterest.ktlint.ruleset.standard.rules import com.pinterest.ktlint.rule.engine.core.api.ElementType.CLASS_BODY import com.pinterest.ktlint.rule.engine.core.api.ElementType.CONST_KEYWORD import com.pinterest.ktlint.rule.engine.core.api.ElementType.FILE +import com.pinterest.ktlint.rule.engine.core.api.ElementType.FUN import com.pinterest.ktlint.rule.engine.core.api.ElementType.GET_KEYWORD import com.pinterest.ktlint.rule.engine.core.api.ElementType.IDENTIFIER +import com.pinterest.ktlint.rule.engine.core.api.ElementType.INTERNAL_KEYWORD import com.pinterest.ktlint.rule.engine.core.api.ElementType.MODIFIER_LIST import com.pinterest.ktlint.rule.engine.core.api.ElementType.OBJECT_DECLARATION import com.pinterest.ktlint.rule.engine.core.api.ElementType.OVERRIDE_KEYWORD @@ -12,6 +14,8 @@ import com.pinterest.ktlint.rule.engine.core.api.ElementType.PRIVATE_KEYWORD import com.pinterest.ktlint.rule.engine.core.api.ElementType.PROPERTY import com.pinterest.ktlint.rule.engine.core.api.ElementType.PROPERTY_ACCESSOR import com.pinterest.ktlint.rule.engine.core.api.ElementType.PROTECTED_KEYWORD +import com.pinterest.ktlint.rule.engine.core.api.ElementType.VALUE_PARAMETER +import com.pinterest.ktlint.rule.engine.core.api.ElementType.VALUE_PARAMETER_LIST import com.pinterest.ktlint.rule.engine.core.api.ElementType.VAL_KEYWORD import com.pinterest.ktlint.rule.engine.core.api.RuleId import com.pinterest.ktlint.rule.engine.core.api.SinceKtlint @@ -79,6 +83,7 @@ public class PropertyNamingRule : StandardRule("property-naming") { emit(identifier.startOffset, "Backing property name not allowed when 'private' modifier is missing", false) } + // A backing property can only exist when a correlated public property or function exists identifier .treeParent ?.treeParent @@ -87,16 +92,42 @@ public class PropertyNamingRule : StandardRule("property-naming") { ?.mapNotNull { it.findChildByType(IDENTIFIER) } ?.firstOrNull { it.text == identifier.text.removePrefix("_") } ?.treeParent - .let { otherProperty -> - if (otherProperty == null || - otherProperty.hasModifier(PRIVATE_KEYWORD) || - otherProperty.hasModifier(PROTECTED_KEYWORD) - ) { - emit(identifier.startOffset, "Backing property not allowed when matching public property is missing", false) + ?.let { correlatedProperty -> + if (correlatedProperty.isNotPublic()) { + return } } + + val correlatedFunctionName = "get${identifier.capitalizeFirstChar()}" + identifier + .treeParent + ?.treeParent + ?.children() + ?.filter { it.elementType == FUN } + ?.filter { it.hasNonEmptyParameterList() } + ?.mapNotNull { it.findChildByType(IDENTIFIER) } + ?.firstOrNull { it.text == correlatedFunctionName } + ?.treeParent + ?.let { correlatedFunction -> + if (correlatedFunction.isNotPublic()) { + return + } + } + + emit(identifier.startOffset, "Backing property name is only allowed when a matching public property or function exists", false) } + private fun ASTNode.hasNonEmptyParameterList() = + findChildByType(VALUE_PARAMETER_LIST) + ?.children() + ?.none { it.elementType == VALUE_PARAMETER } + ?: false + + private fun ASTNode.capitalizeFirstChar() = + text + .removePrefix("_") + .replaceFirstChar { it.uppercase() } + private fun visitConstProperty( identifier: ASTNode, emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit, @@ -151,6 +182,11 @@ public class PropertyNamingRule : StandardRule("property-naming") { containsValKeyword() && !hasModifier(OVERRIDE_KEYWORD) + private fun ASTNode.isNotPublic() = + !hasModifier(PRIVATE_KEYWORD) && + !hasModifier(PROTECTED_KEYWORD) && + !hasModifier(INTERNAL_KEYWORD) + private companion object { val LOWER_CAMEL_CASE_REGEXP = "[a-z][a-zA-Z0-9]*".regExIgnoringDiacriticsAndStrokesOnLetters() val SCREAMING_SNAKE_CASE_REGEXP = "[A-Z][_A-Z0-9]*".regExIgnoringDiacriticsAndStrokesOnLetters() diff --git a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/PropertyNamingRuleTest.kt b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/PropertyNamingRuleTest.kt index 347c03fa20..204c70e116 100644 --- a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/PropertyNamingRuleTest.kt +++ b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/PropertyNamingRuleTest.kt @@ -118,69 +118,153 @@ class PropertyNamingRuleTest { propertyNamingRuleAssertThat(code).hasNoLintViolations() } - @Test - fun `Given a backing val property name, not in screaming case notation, and having another property with a custom get function then do not emit`() { - val code = - """ - class Foo { - private val _elementList = mutableListOf() + @Nested + inner class `Given a property name starting with '_', and not in screaming case notation` { + @Nested + inner class `Given that a correlated property exists` { + @Test + fun `Given that the correlated property is implicitly public then do not emit`() { + val code = + """ + class Foo { + private val _elementList = mutableListOf() + + val elementList: List + get() = _elementList + } + """.trimIndent() + propertyNamingRuleAssertThat(code).hasNoLintViolations() + } - val elementList: List - get() = _elementList + @Test + fun `Given that the correlated property is explicitly public then do not emit`() { + val code = + """ + class Foo { + private val _elementList = mutableListOf() + + public val elementList: List + get() = _elementList + } + """.trimIndent() + propertyNamingRuleAssertThat(code).hasNoLintViolations() } - """.trimIndent() - propertyNamingRuleAssertThat(code).hasNoLintViolations() - } - @Test - fun `Given a backing val property name, not in screaming case notation, and having another property with a custom get function with public modifier then do not emit`() { - val code = - """ - class Foo { - private val _elementList = mutableListOf() + @Test + fun `Given that the backing and correlated property contain diacritics then do not emit`() { + val code = + """ + class Foo { + private val _elementŁîšt = mutableListOf() - public val elementList: List - get() = _elementList + val elementŁîšt: List + get() = _elementList + } + """.trimIndent() + propertyNamingRuleAssertThat(code).hasNoLintViolations() } - """.trimIndent() - propertyNamingRuleAssertThat(code).hasNoLintViolations() - } - @ParameterizedTest(name = "Modifier: {0}") - @ValueSource( - strings = [ - "private", - "protected", - ], - ) - fun `Given a backing val property name, not in screaming case notation, and having having another property with a custom get function with non-public modifier then do emit`( - modifier: String, - ) { - val code = - """ - class Foo { - private val _elementList = mutableListOf() + @ParameterizedTest(name = "Modifier: {0}") + @ValueSource( + strings = [ + "private", + "protected", + ], + ) + fun `Given that correlated property is non-public then emit`(modifier: String) { + val code = + """ + class Foo { + private val _elementList = mutableListOf() + + $modifier val elementList: List + get() = _elementList + } + """.trimIndent() + @Suppress("ktlint:standard:argument-list-wrapping", "ktlint:standard:max-line-length") + propertyNamingRuleAssertThat(code) + .hasLintViolationWithoutAutoCorrect(2, 17, "Backing property name is only allowed when a matching public property or function exists") + } + } - $modifier val elementList: List - get() = _elementList + @Nested + inner class `Given that a correlated function exists` { + @Test + fun `Given that the correlated function is implicitly public then do not emit`() { + val code = + """ + class Foo { + private val _elementList = mutableListOf() + + fun getElementList(): List = _elementList + } + """.trimIndent() + propertyNamingRuleAssertThat(code).hasNoLintViolations() } - """.trimIndent() - propertyNamingRuleAssertThat(code) - .hasLintViolationWithoutAutoCorrect(2, 17, "Backing property not allowed when matching public property is missing") - } - @Test - fun `Given a backing val property name containing diacritics having a custom get function and not in screaming case notation then do not emit`() { - val code = - """ - class Foo { - private val _elementŁîšt = mutableListOf() + @Test + fun `Given that the correlated function is explicitly public then do not emit`() { + val code = + """ + class Foo { + private val _elementList = mutableListOf() - val elementŁîšt: List - get() = _elementList + public fun getElementList(): List = _elementList + } + """.trimIndent() + propertyNamingRuleAssertThat(code).hasNoLintViolations() } - """.trimIndent() - propertyNamingRuleAssertThat(code).hasNoLintViolations() + + @Test + fun `Given that the backing and correlated function contain diacritics then do not emit`() { + val code = + """ + class Foo { + private val _ëlementŁîšt = mutableListOf() + + fun getËlementŁîšt(): List = _elementList + } + """.trimIndent() + propertyNamingRuleAssertThat(code).hasNoLintViolations() + } + + @ParameterizedTest(name = "Modifier: {0}") + @ValueSource( + strings = [ + "private", + "protected", + "internal", + ], + ) + fun `Given that correlated function is non-public then emit`(modifier: String) { + val code = + """ + class Foo { + private val _elementList = mutableListOf() + + $modifier fun getElementList(): List = _elementList + } + """.trimIndent() + @Suppress("ktlint:standard:argument-list-wrapping", "ktlint:standard:max-line-length") + propertyNamingRuleAssertThat(code) + .hasLintViolationWithoutAutoCorrect(2, 17, "Backing property name is only allowed when a matching public property or function exists") + } + + @Test + fun `Given that the correlated function has at least 1 parameter then emit`() { + val code = + """ + class Foo { + private val _elementList = mutableListOf() + + fun getElementList(bar: String): List = _elementList + bar + } + """.trimIndent() + @Suppress("ktlint:standard:argument-list-wrapping", "ktlint:standard:max-line-length") + propertyNamingRuleAssertThat(code) + .hasLintViolationWithoutAutoCorrect(2, 17, "Backing property name is only allowed when a matching public property or function exists") + } + } } @Test @@ -303,7 +387,7 @@ class PropertyNamingRuleTest { 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, "Backing property not allowed when matching public property is missing", canBeAutoCorrected = false), + LintViolation(9, 17, "Backing property name is only allowed when a matching public property or function exists", canBeAutoCorrected = false), LintViolation(12, 9, "Backing property name not allowed when 'private' modifier is missing", canBeAutoCorrected = false), ) }