From bf130086f4bc54b605b50324b51ccb3b9358bd62 Mon Sep 17 00:00:00 2001 From: paul-dingemans Date: Wed, 8 Nov 2023 20:05:07 +0100 Subject: [PATCH] Improve checking on backing property - Provide better message when the backing property does not have a private modifier - Check that property correlated with the backing property is public Closes #2345 --- .../standard/rules/PropertyNamingRule.kt | 40 +++++++++-------- .../standard/rules/PropertyNamingRuleTest.kt | 43 +++++++++++++++++-- 2 files changed, 63 insertions(+), 20 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 b604267bb4..375329afb4 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 @@ -11,6 +11,7 @@ import com.pinterest.ktlint.rule.engine.core.api.ElementType.OVERRIDE_KEYWORD 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.VAL_KEYWORD import com.pinterest.ktlint.rule.engine.core.api.RuleId import com.pinterest.ktlint.rule.engine.core.api.SinceKtlint @@ -53,7 +54,7 @@ public class PropertyNamingRule : StandardRule("property-naming") { property.hasCustomGetter() || property.isTopLevelValue() || property.isObjectValue() -> { // Can not reliably determine whether the value is immutable or not } - property.isBackingProperty() -> { + identifier.text.startsWith("_") -> { visitBackingProperty(identifier, emit) } else -> { @@ -73,6 +74,27 @@ public class PropertyNamingRule : StandardRule("property-naming") { ?.let { emit(identifier.startOffset, "Backing property name should start with underscore followed by lower camel case", false) } + + if (!identifier.treeParent.hasModifier(PRIVATE_KEYWORD)) { + emit(identifier.startOffset, "Backing property name not allowed when 'private' modifier is missing", false) + } + + identifier + .treeParent + ?.treeParent + ?.children() + ?.filter { it.elementType == PROPERTY } + ?.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) + } + } } private fun visitConstProperty( @@ -129,22 +151,6 @@ public class PropertyNamingRule : StandardRule("property-naming") { containsValKeyword() && !hasModifier(OVERRIDE_KEYWORD) - private fun ASTNode.isBackingProperty() = - takeIf { hasModifier(PRIVATE_KEYWORD) } - ?.findChildByType(IDENTIFIER) - ?.takeIf { it.text.startsWith("_") } - ?.let { identifier -> - this.hasPublicProperty(identifier.text.removePrefix("_")) - } - ?: false - - private fun ASTNode.hasPublicProperty(identifier: String) = - treeParent - .children() - .filter { it.elementType == PROPERTY } - .mapNotNull { it.findChildByType(IDENTIFIER) } - .any { it.text == identifier } - 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 2963208bbd..347c03fa20 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 @@ -119,7 +119,7 @@ class PropertyNamingRuleTest { } @Test - fun `Given a backing val property name having a custom get function and not in screaming case notation then do not emit`() { + 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 { @@ -132,6 +132,43 @@ 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 with public modifier then do not emit`() { + val code = + """ + class Foo { + private val _elementList = mutableListOf() + + public val elementList: List + get() = _elementList + } + """.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() + + $modifier val elementList: List + get() = _elementList + } + """.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 = @@ -266,8 +303,8 @@ 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, "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), + LintViolation(9, 17, "Backing property not allowed when matching public property is missing", canBeAutoCorrected = false), + LintViolation(12, 9, "Backing property name not allowed when 'private' modifier is missing", canBeAutoCorrected = false), ) } }