Skip to content

Commit

Permalink
Allow backing property to be correlated to a public function
Browse files Browse the repository at this point in the history 
Closes #2332
  • Loading branch information
@paul-dingemans
paul-dingemans committed Nov 15, 2023
1 parent 2bfc5a1 commit 2d29528
Show file tree
Hide file tree
Showing 2 changed files with 179 additions and 59 deletions.
48 changes: 42 additions & 6 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 @@ -3,15 +3,19 @@ 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
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
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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()
Expand Down
190 changes: 137 additions & 53 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 @@ -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<Element>()
@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<Element>()
val elementList: List<Element>
get() = _elementList
}
""".trimIndent()
propertyNamingRuleAssertThat(code).hasNoLintViolations()
}

val elementList: List<Element>
get() = _elementList
@Test
fun `Given that the correlated property is explicitly public then do not emit`() {
val code =
"""
class Foo {
private val _elementList = mutableListOf<Element>()
public val elementList: List<Element>
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<Element>()
@Test
fun `Given that the backing and correlated property contain diacritics then do not emit`() {
val code =
"""
class Foo {
private val _elementŁîšt = mutableListOf<Element>()
public val elementList: List<Element>
get() = _elementList
val elementŁîšt: List<Element>
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<Element>()
@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<Element>()
$modifier val elementList: List<Element>
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<Element>
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<Element>()
fun getElementList(): List<Element> = _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<Element>()
@Test
fun `Given that the correlated function is explicitly public then do not emit`() {
val code =
"""
class Foo {
private val _elementList = mutableListOf<Element>()
val elementŁîšt: List<Element>
get() = _elementList
public fun getElementList(): List<Element> = _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<Element>()
fun getËlementŁîšt(): List<Element> = _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<Element>()
$modifier fun getElementList(): List<Element> = _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<Element>()
fun getElementList(bar: String): List<Element> = _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
Expand Down Expand Up @@ -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),
)
}
Expand Down

0 comments on commit 2d29528

Please sign in to comment.