More specific check for Kotlin boolean property naming
Based on go/android-api-guidelines#boolean-methods-kotlin. This adds additional error cases, so existing boolean properties with names that don't start with "is", "has", "can", or "should" would need to be baselined.
Bug: 276326483
Test: Added a Kotlin boolean property test to ApiLintTest
Change-Id: I677b0451ff5fe45ab6b5944cf3edcc374ff43c68
diff --git a/src/main/java/com/android/tools/metalava/ApiLint.kt b/src/main/java/com/android/tools/metalava/ApiLint.kt
index fc9f9d2..8984e2a 100644
--- a/src/main/java/com/android/tools/metalava/ApiLint.kt
+++ b/src/main/java/com/android/tools/metalava/ApiLint.kt
@@ -1308,6 +1308,55 @@
}
}
+ /**
+ * Check the Kotlin property associated with the [getter] is well-named and has correctly named accessors.
+ */
+ fun checkKotlinProperty(getter: MethodItem) {
+ val propertyItem = getter.property ?: return
+ val setter = propertyItem.setter
+
+ // The property name rules are the same as the getter name rules.
+ val pattern = goodBooleanGetterSetterPrefixes.match(propertyItem.name(), GetterSetterPattern::getter)
+ if (pattern == null) {
+ report(
+ GETTER_SETTER_NAMES, propertyItem,
+ "Invalid name for boolean property `${propertyItem.name()}`. " +
+ "Should start with one of $goodBooleanPropertyPrefixes."
+ )
+ return
+ }
+
+ // The property starts with a good prefix, but could still also start with a bad prefix (e.g. "isHas")
+ val badPrefix = badBooleanGetterPrefixes.firstOrNull { propertyItem.name().startsWith(it) }
+ if (badPrefix != null && badPrefix != pattern.getter) {
+ report(
+ GETTER_SETTER_NAMES, propertyItem,
+ "Invalid prefix `$badPrefix` for boolean property `${propertyItem.name()}`."
+ )
+ return
+ }
+
+ if (getter.name() != propertyItem.name()) {
+ // Only properties beginning with "is" have the correct autogenerated getter name.
+ // Others need to be set with @JvmName.
+ report(
+ GETTER_SETTER_NAMES, getter,
+ "Getter for boolean property `${propertyItem.name()}` is named `${getter.name()}` " +
+ "but should match the property name. Use `@get:JvmName` to rename."
+ )
+ }
+
+ val target = propertyItem.name().substring(pattern.getter.length)
+ val expectedSetter = "${pattern.setter}$target"
+ if (setter != null && setter.name() != expectedSetter) {
+ // If this happens, the setter name must have been incorrectly set with @set:JvmName
+ report(
+ GETTER_SETTER_NAMES, setter,
+ "Invalid name for boolean property setter `${setter.name()}`, should be `$expectedSetter`."
+ )
+ }
+ }
+
fun isGetter(method: MethodItem): Boolean {
val returnType = method.returnType() ?: return false
return method.parameters().isEmpty() && returnType.primitive && returnType.toTypeString() == "boolean"
@@ -1320,17 +1369,26 @@
for (method in methods) {
val name = method.name()
if (isGetter(method)) {
- val pattern = goodBooleanGetterSetterPrefixes.match(name, GetterSetterPattern::getter) ?: continue
- val target = name.substring(pattern.getter.length)
- val expectedSetter = "${pattern.setter}$target"
+ // Checks for Java and Kotlin getters are handled separately
+ if (method.isKotlinProperty()) {
+ checkKotlinProperty(method)
+ } else {
+ val pattern = goodBooleanGetterSetterPrefixes.match(name, GetterSetterPattern::getter) ?: continue
+ val target = name.substring(pattern.getter.length)
+ val expectedSetter = "${pattern.setter}$target"
- badBooleanSetterPrefixes.forEach {
- val actualSetter = "${it}$target"
- if (actualSetter != expectedSetter) {
- errorIfExists(methods, name, expectedSetter, actualSetter)
+ badBooleanSetterPrefixes.forEach {
+ val actualSetter = "${it}$target"
+ if (actualSetter != expectedSetter) {
+ errorIfExists(methods, name, expectedSetter, actualSetter)
+ }
}
}
} else if (isSetter(method)) {
+ // Handled in the getter case (if the setter is part of the API, the getter also
+ // has to be: https://youtrack.jetbrains.com/issue/KT-3110)
+ if (method.isKotlinProperty()) continue
+
val pattern = goodBooleanGetterSetterPrefixes.match(name, GetterSetterPattern::setter) ?: continue
val target = name.substring(pattern.setter.length)
val expectedGetter = "${pattern.getter}$target"
@@ -2726,6 +2784,9 @@
GetterSetterPattern("should", "setShould"),
GetterSetterPattern("is", "set")
)
+ private val goodBooleanPropertyPrefixes =
+ goodBooleanGetterSetterPrefixes.joinToString(", ") { "`${it.getter}`" }
+
private fun List<GetterSetterPattern>.match(
name: String,
prop: (GetterSetterPattern) -> String
diff --git a/src/test/java/com/android/tools/metalava/ApiLintTest.kt b/src/test/java/com/android/tools/metalava/ApiLintTest.kt
index c6b8bfb..550b40b 100644
--- a/src/test/java/com/android/tools/metalava/ApiLintTest.kt
+++ b/src/test/java/com/android/tools/metalava/ApiLintTest.kt
@@ -1465,6 +1465,75 @@
}
@Test
+ fun `Check boolean property accessor naming patterns in Kotlin`() {
+ check(
+ apiLint = "", // enabled
+ expectedIssues = """
+ src/android/pkg/MyClass.kt:19: error: Invalid name for boolean property `visibleBad`. Should start with one of `has`, `can`, `should`, `is`. [GetterSetterNames]
+ src/android/pkg/MyClass.kt:22: error: Invalid name for boolean property setter `setIsVisibleBad`, should be `setVisibleSetterBad`. [GetterSetterNames]
+ src/android/pkg/MyClass.kt:25: error: Invalid name for boolean property `transientStateBad`. Should start with one of `has`, `can`, `should`, `is`. [GetterSetterNames]
+ src/android/pkg/MyClass.kt:27: error: Invalid prefix `isHas` for boolean property `isHasTransientStateAlsoBad`. [GetterSetterNames]
+ src/android/pkg/MyClass.kt:29: error: Invalid prefix `isCan` for boolean property `isCanRecordBad`. [GetterSetterNames]
+ src/android/pkg/MyClass.kt:31: error: Invalid prefix `isShould` for boolean property `isShouldFitWidthBad`. [GetterSetterNames]
+ src/android/pkg/MyClass.kt:33: error: Invalid name for boolean property `wiFiRoamingSettingEnabledBad`. Should start with one of `has`, `can`, `should`, `is`. [GetterSetterNames]
+ src/android/pkg/MyClass.kt:35: error: Invalid name for boolean property `enabledBad`. Should start with one of `has`, `can`, `should`, `is`. [GetterSetterNames]
+ src/android/pkg/MyClass.kt:37: error: Getter for boolean property `hasTransientStateGetterBad` is named `getHasTransientStateGetterBad` but should match the property name. Use `@get:JvmName` to rename. [GetterSetterNames]
+ src/android/pkg/MyClass.kt:39: error: Getter for boolean property `canRecordGetterBad` is named `getCanRecordGetterBad` but should match the property name. Use `@get:JvmName` to rename. [GetterSetterNames]
+ src/android/pkg/MyClass.kt:41: error: Getter for boolean property `shouldFitWidthGetterBad` is named `getShouldFitWidthGetterBad` but should match the property name. Use `@get:JvmName` to rename. [GetterSetterNames]
+ """,
+ expectedFail = DefaultLintErrorMessage,
+ sourceFiles = arrayOf(
+ kotlin(
+ """
+ package android.pkg
+
+ class MyClass {
+ // Correct
+ var isVisible: Boolean = false
+
+ @get:JvmName("hasTransientState")
+ var hasTransientState: Boolean = false
+
+ @get:JvmName("canRecord")
+ var canRecord: Boolean = false
+
+ @get:JvmName("shouldFitWidth")
+ var shouldFitWidth: Boolean = false
+
+ var isWiFiRoamingSettingEnabled: Boolean = false
+
+ // Bad
+ var visibleBad: Boolean = false
+
+ @set:JvmName("setIsVisibleBad")
+ var isVisibleSetterBad: Boolean = false
+
+ @get:JvmName("hasTransientStateBad")
+ var transientStateBad: Boolean = false
+
+ var isHasTransientStateAlsoBad: Boolean = false
+
+ var isCanRecordBad: Boolean = false
+
+ var isShouldFitWidthBad: Boolean = false
+
+ var wiFiRoamingSettingEnabledBad: Boolean = false
+
+ var enabledBad: Boolean = false
+
+ var hasTransientStateGetterBad: Boolean = false
+
+ var canRecordGetterBad: Boolean = false
+
+ var shouldFitWidthGetterBad: Boolean = false
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
fun `Check banned collections`() {
check(
apiLint = "", // enabled
