Skip to content

Commit

Permalink
Improve checking on backing property
Browse files Browse the repository at this point in the history 
- 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
  • Loading branch information
@paul-dingemans
paul-dingemans committed Nov 8, 2023
1 parent 9644235 commit bf13008
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 20 deletions.
40 changes: 23 additions & 17 deletions ...tandard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/PropertyNamingRule.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 -> {
Expand All @@ -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(
Expand Down Expand Up @@ -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()
Expand Down
43 changes: 40 additions & 3 deletions ...ard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/PropertyNamingRuleTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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<Element>()
public val elementList: List<Element>
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<Element>()
$modifier val elementList: List<Element>
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 =
Expand Down Expand Up @@ -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),
)
}
}

0 comments on commit bf13008

Please sign in to comment.