Skip to content

Commit

Permalink
Ignore modifier of backing property in android_studio code style (#…
Browse files Browse the repository at this point in the history
…2552)

Extract `backing-property-name` from `property-naming` rule. This allows users to disable this rule, but keep the `property-naming` rule in place.

 For `android_studio` code style the restrictions regarding the modifier of the correlated property or function is ignored entirely as the Android Kotlin Styleguide does not require it to be public.

 Closes #2528
  • Loading branch information
paul-dingemans authored Feb 14, 2024
1 parent 33396eb commit a88a2e5
Show file tree
Hide file tree
Showing 7 changed files with 538 additions and 244 deletions.
30 changes: 30 additions & 0 deletions documentation/snapshot/docs/rules/experimental.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,36 @@ ktlint_experimental=enabled
```
Also see [enable/disable specific rules](../configuration-ktlint/#disabled-rules).

### Backing property naming

Allows property names to start with `_` in case the property is a backing property. `ktlint_official` and `android_studio` code styles require the correlated property/function to be defined as `public`.

=== "[:material-heart:](#) Ktlint"

```kotlin
class Bar {
// Backing property
private val _elementList = mutableListOf<Element>()
val elementList: List<Element>
get() = _elementList
}
```
=== "[:material-heart-off-outline:](#) Disallowed"

```kotlin
class Bar {
// 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
}
```

Rule id: `backing-property-naming` (`standard` rule set)

## Binary expression wrapping

Wraps binary expression at the operator reference whenever the binary expression does not fit on the line. In case the binary expression is nested, the expression is evaluated from outside to inside. If the left and right hand sides of the binary expression, after wrapping, fit on a single line then the inner binary expressions will not be wrapped. If one or both inner binary expression still do not fit on a single after wrapping of the outer binary expression, then each of those inner binary expressions will be wrapped.
Expand Down
10 changes: 10 additions & 0 deletions ktlint-ruleset-standard/api/ktlint-ruleset-standard.api
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,16 @@ public final class com/pinterest/ktlint/ruleset/standard/rules/ArgumentListWrapp
public static final fun getARGUMENT_LIST_WRAPPING_RULE_ID ()Lcom/pinterest/ktlint/rule/engine/core/api/RuleId;
}

public final class com/pinterest/ktlint/ruleset/standard/rules/BackingPropertyNamingRule : com/pinterest/ktlint/ruleset/standard/StandardRule {
public fun <init> ()V
public fun beforeFirstNode (Lcom/pinterest/ktlint/rule/engine/core/api/editorconfig/EditorConfig;)V
public fun beforeVisitChildNodes (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;ZLkotlin/jvm/functions/Function3;)V
}

public final class com/pinterest/ktlint/ruleset/standard/rules/BackingPropertyNamingRuleKt {
public static final fun getBACKING_PROPERTY_NAMING_RULE_ID ()Lcom/pinterest/ktlint/rule/engine/core/api/RuleId;
}

public final class com/pinterest/ktlint/ruleset/standard/rules/BinaryExpressionWrappingRule : com/pinterest/ktlint/ruleset/standard/StandardRule, com/pinterest/ktlint/rule/engine/core/api/Rule$Experimental {
public fun <init> ()V
public fun beforeFirstNode (Lcom/pinterest/ktlint/rule/engine/core/api/editorconfig/EditorConfig;)V
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import com.pinterest.ktlint.rule.engine.core.api.RuleSetId
import com.pinterest.ktlint.ruleset.standard.rules.AnnotationRule
import com.pinterest.ktlint.ruleset.standard.rules.AnnotationSpacingRule
import com.pinterest.ktlint.ruleset.standard.rules.ArgumentListWrappingRule
import com.pinterest.ktlint.ruleset.standard.rules.BackingPropertyNamingRule
import com.pinterest.ktlint.ruleset.standard.rules.BinaryExpressionWrappingRule
import com.pinterest.ktlint.ruleset.standard.rules.BlankLineBeforeDeclarationRule
import com.pinterest.ktlint.ruleset.standard.rules.BlockCommentInitialStarAlignmentRule
Expand Down Expand Up @@ -104,6 +105,7 @@ public class StandardRuleSetProvider : RuleSetProviderV3(RuleSetId.STANDARD) {
RuleProvider { AnnotationRule() },
RuleProvider { AnnotationSpacingRule() },
RuleProvider { ArgumentListWrappingRule() },
RuleProvider { BackingPropertyNamingRule() },
RuleProvider { BinaryExpressionWrappingRule() },
RuleProvider { BlankLineBeforeDeclarationRule() },
RuleProvider { BlockCommentInitialStarAlignmentRule() },
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
package com.pinterest.ktlint.ruleset.standard.rules

import com.pinterest.ktlint.rule.engine.core.api.ElementType.FUN
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.PRIVATE_KEYWORD
import com.pinterest.ktlint.rule.engine.core.api.ElementType.PROPERTY
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.RuleId
import com.pinterest.ktlint.rule.engine.core.api.SinceKtlint
import com.pinterest.ktlint.rule.engine.core.api.SinceKtlint.Status.EXPERIMENTAL
import com.pinterest.ktlint.rule.engine.core.api.children
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.CODE_STYLE_PROPERTY
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.CodeStyleValue.android_studio
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.EditorConfig
import com.pinterest.ktlint.rule.engine.core.api.hasModifier
import com.pinterest.ktlint.ruleset.standard.StandardRule
import com.pinterest.ktlint.ruleset.standard.rules.internal.regExIgnoringDiacriticsAndStrokesOnLetters
import org.jetbrains.kotlin.com.intellij.lang.ASTNode

/**
* https://kotlinlang.org/docs/coding-conventions.html#property-names
* https://developer.android.com/kotlin/style-guide#backing_properties
*/
@SinceKtlint("1.2.0", EXPERIMENTAL)
public class BackingPropertyNamingRule :
StandardRule(
id = "backing-property-naming",
usesEditorConfigProperties = setOf(CODE_STYLE_PROPERTY),
) {
private var codeStyle = CODE_STYLE_PROPERTY.defaultValue

override fun beforeFirstNode(editorConfig: EditorConfig) {
codeStyle = editorConfig[CODE_STYLE_PROPERTY]
}

override fun beforeVisitChildNodes(
node: ASTNode,
autoCorrect: Boolean,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
) {
node
.takeIf { node.elementType == PROPERTY }
?.let { property -> visitProperty(property, emit) }
}

private fun visitProperty(
property: ASTNode,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
) {
property
.findChildByType(IDENTIFIER)
?.takeIf { it.text.startsWith("_") }
?.let { identifier ->
visitBackingProperty(identifier, emit)
}
}

private fun visitBackingProperty(
identifier: ASTNode,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
) {
identifier
.text
.takeUnless { it.matches(BACKING_PROPERTY_LOWER_CAMEL_CASE_REGEXP) }
?.let {
emit(identifier.startOffset, "Backing property should start with underscore followed by lower camel case", false)
}

if (!identifier.treeParent.hasModifier(PRIVATE_KEYWORD)) {
emit(identifier.startOffset, "Backing property not allowed when 'private' modifier is missing", false)
}

// A backing property can only exist when a correlated public property or function exists
val correlatedPropertyOrFunction = identifier.findCorrelatedPropertyOrFunction()
if (correlatedPropertyOrFunction == null) {
emit(identifier.startOffset, "Backing property is only allowed when a matching property or function exists", false)
} else {
if (codeStyle == android_studio || correlatedPropertyOrFunction.isPublic()) {
return
} else {
emit(identifier.startOffset, "Backing property is only allowed when the matching property or function is public", false)
}
}
}

private fun ASTNode.findCorrelatedPropertyOrFunction() = findCorrelatedProperty() ?: findCorrelatedFunction()

private fun ASTNode.findCorrelatedProperty() =
treeParent
?.treeParent
?.children()
?.filter { it.elementType == PROPERTY }
?.mapNotNull { it.findChildByType(IDENTIFIER) }
?.firstOrNull { it.text == text.removePrefix("_") }
?.treeParent

private fun ASTNode.findCorrelatedFunction(): ASTNode? {
val correlatedFunctionName = "get${capitalizeFirstChar()}"
return treeParent
?.treeParent
?.children()
?.filter { it.elementType == FUN }
?.filter { it.hasNonEmptyParameterList() }
?.mapNotNull { it.findChildByType(IDENTIFIER) }
?.firstOrNull { it.text == correlatedFunctionName }
?.treeParent
}

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 ASTNode.isPublic() =
!hasModifier(PRIVATE_KEYWORD) &&
!hasModifier(PROTECTED_KEYWORD) &&
!hasModifier(INTERNAL_KEYWORD)

private companion object {
val BACKING_PROPERTY_LOWER_CAMEL_CASE_REGEXP = "_[a-z][a-zA-Z0-9]*".regExIgnoringDiacriticsAndStrokesOnLetters()
}
}

public val BACKING_PROPERTY_NAMING_RULE_ID: RuleId = BackingPropertyNamingRule().ruleId
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,12 @@ 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.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 @@ -59,76 +53,13 @@ public class PropertyNamingRule : StandardRule("property-naming") {
property.hasCustomGetter() || property.isTopLevelValue() || property.isObjectValue() -> {
// Can not reliably determine whether the value is immutable or not
}
identifier.text.startsWith("_") -> {
visitBackingProperty(identifier, emit)
}
else -> {
visitNonConstProperty(identifier, emit)
}
}
}
}

private fun visitBackingProperty(
identifier: ASTNode,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
) {
identifier
.text
.takeUnless { it.matches(BACKING_PROPERTY_LOWER_CAMEL_CASE_REGEXP) }
?.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)
}

// A backing property can only exist when a correlated public property or function exists
identifier
.treeParent
?.treeParent
?.children()
?.filter { it.elementType == PROPERTY }
?.mapNotNull { it.findChildByType(IDENTIFIER) }
?.firstOrNull { it.text == identifier.text.removePrefix("_") }
?.treeParent
?.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 @@ -158,7 +89,10 @@ public class PropertyNamingRule : StandardRule("property-naming") {
identifier
.text
.takeUnless { it.matches(LOWER_CAMEL_CASE_REGEXP) }
?.let {
?.takeUnless {
// Ignore backing properties
it.startsWith("_")
}?.let {
emit(identifier.startOffset, "Property name should start with a lowercase letter and use camel case", false)
}
}
Expand All @@ -177,11 +111,6 @@ public class PropertyNamingRule : StandardRule("property-naming") {
containsValKeyword() &&
!hasModifier(OVERRIDE_KEYWORD)

private fun ASTNode.isNotPublic() =
!hasModifier(PRIVATE_KEYWORD) &&
!hasModifier(PROTECTED_KEYWORD) &&
!hasModifier(INTERNAL_KEYWORD)

private fun ASTNode.isTokenKeywordBetweenBackticks() =
this
.takeIf { it.elementType == IDENTIFIER }
Expand All @@ -193,7 +122,6 @@ public class PropertyNamingRule : StandardRule("property-naming") {
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()
val BACKING_PROPERTY_LOWER_CAMEL_CASE_REGEXP = "_[a-z][a-zA-Z0-9]*".regExIgnoringDiacriticsAndStrokesOnLetters()
const val SERIAL_VERSION_UID_PROPERTY_NAME = "serialVersionUID"
val KEYWORDS =
setOf(KtTokens.KEYWORDS, KtTokens.SOFT_KEYWORDS)
Expand Down
Loading

0 comments on commit a88a2e5

Please sign in to comment.