Use reference in fallback property delegate (#3982)

Co-authored-by: Markus Schwarz <post@markus-schwarz.net>
Co-authored-by: Brais <braisgabin@gmail.com>
This commit is contained in:
marschwar
2021-09-14 14:12:49 +02:00
committed by GitHub
parent 19b06c96d0
commit 7962979fb3
9 changed files with 74 additions and 137 deletions

View File

@@ -97,8 +97,8 @@ public final class io/gitlab/arturbosch/detekt/api/ConfigPropertyKt {
public static final fun config (Ljava/lang/Object;Lkotlin/jvm/functions/Function1;)Lkotlin/properties/ReadOnlyProperty;
public static final fun configWithAndroidVariants (Ljava/lang/Object;Ljava/lang/Object;)Lkotlin/properties/ReadOnlyProperty;
public static final fun configWithAndroidVariants (Ljava/lang/Object;Ljava/lang/Object;Lkotlin/jvm/functions/Function1;)Lkotlin/properties/ReadOnlyProperty;
public static final fun configWithFallback (Ljava/lang/String;Ljava/lang/Object;)Lkotlin/properties/ReadOnlyProperty;
public static final fun configWithFallback (Ljava/lang/String;Ljava/lang/Object;Lkotlin/jvm/functions/Function1;)Lkotlin/properties/ReadOnlyProperty;
public static final fun configWithFallback (Lkotlin/reflect/KProperty0;Ljava/lang/Object;)Lkotlin/properties/ReadOnlyProperty;
public static final fun configWithFallback (Lkotlin/reflect/KProperty0;Ljava/lang/Object;Lkotlin/jvm/functions/Function1;)Lkotlin/properties/ReadOnlyProperty;
}
public abstract interface class io/gitlab/arturbosch/detekt/api/ConfigValidator : io/gitlab/arturbosch/detekt/api/Extension {

View File

@@ -3,6 +3,7 @@ package io.gitlab.arturbosch.detekt.api
import io.gitlab.arturbosch.detekt.api.internal.valueOrDefaultCommaSeparated
import kotlin.properties.ReadOnlyProperty
import kotlin.reflect.KProperty
import kotlin.reflect.KProperty0
/**
* Creates a delegated read-only property that can be used in [ConfigAware] objects. The name of the property is the
@@ -32,34 +33,34 @@ fun <T : Any, U : Any> config(
/**
* Creates a delegated read-only property that can be used in [ConfigAware] objects. The name of the property is the
* key that is used during configuration lookup. If there is no such property, the value of the
* supplied [fallbackPropertyName] is also considered before using the [defaultValue].
* supplied [fallbackProperty] is also considered before using the [defaultValue].
* The value of the property is evaluated only once.
*
* This method is only intended to be used in migration scenarios where there is no way to update all configuration
* files immediately.
*
* @param fallbackPropertyName the configuration key that is checked when there is no key that matches the property
* name before falling back to the default value.
* @param fallbackProperty The reference to the configuration key to fall back to. This property must be defined as a
* configuration delegate.
* @param defaultValue the value that the property evaluates to when there is no key with the name of the property in
* the config. Although [T] is defined as [Any], only [String], [Int], [Boolean] and [List<String>] are supported.
*/
@UnstableApi("fallback property handling is still under discussion")
fun <T : Any> configWithFallback(
fallbackPropertyName: String,
fallbackProperty: KProperty0<T>,
defaultValue: T
): ReadOnlyProperty<ConfigAware, T> = configWithFallback(fallbackPropertyName, defaultValue) { it }
): ReadOnlyProperty<ConfigAware, T> = configWithFallback(fallbackProperty, defaultValue) { it }
/**
* Creates a delegated read-only property that can be used in [ConfigAware] objects. The name of the property is the
* key that is used during configuration lookup. If there is no such property, the value of the
* supplied [fallbackPropertyName] is also considered before using the [defaultValue].
* The value of the property is evaluated and transfored only once.
* supplied [fallbackProperty] is also considered before using the [defaultValue].
* The value of the property is evaluated and transformed only once.
*
* This method is only intended to be used in migration scenarios where there is no way to update all configuration
* files immediately.
*
* @param fallbackPropertyName the configuration key that is checked when there is no key that matches the property
* name before falling back to the default value.
* @param fallbackProperty The reference to the configuration key to fall back to. This property must be defined as a
* configuration delegate.
* @param defaultValue the value that the property evaluates to when there is no key with the name of the property in
* the config. Although [T] is defined as [Any], only [String], [Int], [Boolean] and [List<String>] are supported.
* @param transformer a function that transforms the value from the configuration (or the default) into its final
@@ -67,10 +68,11 @@ fun <T : Any> configWithFallback(
*/
@UnstableApi("fallback property handling is still under discussion")
fun <T : Any, U : Any> configWithFallback(
fallbackPropertyName: String,
fallbackProperty: KProperty0<U>,
defaultValue: T,
transformer: (T) -> U
): ReadOnlyProperty<ConfigAware, U> = FallbackConfigProperty(fallbackPropertyName, defaultValue, transformer)
): ReadOnlyProperty<ConfigAware, U> =
FallbackConfigProperty(fallbackProperty, defaultValue, transformer)
/**
* Creates a delegated read-only property that can be used in [ConfigAware] objects. The name of the property is the
@@ -159,12 +161,19 @@ private class TransformedConfigProperty<T : Any, U : Any>(
}
private class FallbackConfigProperty<T : Any, U : Any>(
private val fallbackPropertyName: String,
private val fallbackProperty: KProperty0<U>,
private val defaultValue: T,
private val transform: (T) -> U
) : MemoizedConfigProperty<U>() {
override fun doGetValue(thisRef: ConfigAware, property: KProperty<*>): U {
val fallbackValue = getValueOrDefault(thisRef, fallbackPropertyName, defaultValue)
return transform(getValueOrDefault(thisRef, property.name, fallbackValue))
if (thisRef.isConfigured(property.name)) {
return transform(getValueOrDefault(thisRef, property.name, defaultValue))
}
if (thisRef.isConfigured(fallbackProperty.name)) {
return fallbackProperty.get()
}
return transform(defaultValue)
}
private fun ConfigAware.isConfigured(propertyName: String) = valueOrNull<Any>(propertyName) != null
}

View File

@@ -298,9 +298,11 @@ class ConfigPropertySpec : Spek({
val fallbackValue = -1
val subject by memoized {
object : TestConfigAware("present" to "$configValue", "fallback" to fallbackValue) {
val present: Int by configWithFallback("fallback", defaultValue)
val notPresentWithFallback: Int by configWithFallback("fallback", defaultValue)
val notPresentFallbackMissing: Int by configWithFallback("missing", defaultValue)
private val fallback: Int by config(42)
private val missing: Int by config(42)
val present: Int by configWithFallback(::fallback, defaultValue)
val notPresentWithFallback: Int by configWithFallback(::fallback, defaultValue)
val notPresentFallbackMissing: Int by configWithFallback(::missing, defaultValue)
}
}
it("uses the value provided in config if present") {
@@ -317,15 +319,18 @@ class ConfigPropertySpec : Spek({
val configValue = 99
val defaultValue = 0
val fallbackValue = -1
val fallbackOffset = 10
val subject by memoized {
object : TestConfigAware("present" to configValue, "fallback" to fallbackValue) {
val present: String by configWithFallback("fallback", defaultValue) { v ->
private val fallback: String by config(42) { (it + fallbackOffset).toString() }
private val missing: String by config(42) { (it + fallbackOffset).toString() }
val present: String by configWithFallback(::fallback, defaultValue) { v ->
v.toString()
}
val notPresentWithFallback: String by configWithFallback("fallback", defaultValue) { v ->
val notPresentWithFallback: String by configWithFallback(::fallback, defaultValue) { v ->
v.toString()
}
val notPresentFallbackMissing: String by configWithFallback("missing", defaultValue) { v ->
val notPresentFallbackMissing: String by configWithFallback(::missing, defaultValue) { v ->
v.toString()
}
}
@@ -333,8 +338,8 @@ class ConfigPropertySpec : Spek({
it("uses the value provided in config if present") {
assertThat(subject.present).isEqualTo("$configValue")
}
it("uses the value from fallback property if value is missing and fallback exists") {
assertThat(subject.notPresentWithFallback).isEqualTo("$fallbackValue")
it("transforms the value from fallback property if value is missing and fallback exists") {
assertThat(subject.notPresentWithFallback).isEqualTo("${fallbackValue + fallbackOffset}")
}
it("uses the default value if not present") {
assertThat(subject.notPresentFallbackMissing).isEqualTo("$defaultValue")

View File

@@ -8,6 +8,7 @@ import io.gitlab.arturbosch.detekt.generator.collection.ConfigurationCollector.C
import io.gitlab.arturbosch.detekt.generator.collection.ConfigurationCollector.ConfigWithFallbackSupport.isFallbackConfigDelegate
import io.gitlab.arturbosch.detekt.generator.collection.exception.InvalidDocumentationException
import org.jetbrains.kotlin.psi.KtCallExpression
import org.jetbrains.kotlin.psi.KtCallableReferenceExpression
import org.jetbrains.kotlin.psi.KtConstantExpression
import org.jetbrains.kotlin.psi.KtElement
import org.jetbrains.kotlin.psi.KtExpression
@@ -140,25 +141,29 @@ class ConfigurationCollector {
private object ConfigWithFallbackSupport {
const val FALLBACK_DELEGATE_NAME = "configWithFallback"
private const val FALLBACK_ARGUMENT_NAME = "fallbackPropertyName"
private const val FALLBACK_ARGUMENT_NAME = "fallbackProperty"
fun KtProperty.isFallbackConfigDelegate(): Boolean =
delegate?.expression?.referenceExpression()?.text == FALLBACK_DELEGATE_NAME
fun KtProperty.checkUsingInvalidFallbackReference(properties: List<KtProperty>) {
val fallbackPropertyName = getValueArgument(
val fallbackPropertyReference = getValueArgument(
name = FALLBACK_ARGUMENT_NAME,
actionForPositionalMatch = { it.first() }
)?.getArgumentExpression()?.text?.withoutQuotes()
val hasInvalidFallbackReference = properties
.filter { it.isInitializedWithConfigDelegate() }
.none { it.name == fallbackPropertyName }
if (hasInvalidFallbackReference) {
)?.getReferenceIdentifierOrNull()
val fallbackProperty = properties.find { it.name == fallbackPropertyReference }
if (fallbackProperty == null || !fallbackProperty.isInitializedWithConfigDelegate()) {
invalidDocumentation {
"The fallback property '$fallbackPropertyName' is missing for property '$name'"
"The fallback property '$fallbackPropertyReference' of property '$name' " +
"must also be defined using a config property delegate "
}
}
}
private fun KtValueArgument.getReferenceIdentifierOrNull(): String? =
(getArgumentExpression() as? KtCallableReferenceExpression)
?.callableReference?.getIdentifier()?.text
}
private object ConfigWithAndroidVariantsSupport {

View File

@@ -7,6 +7,7 @@ import io.gitlab.arturbosch.detekt.generator.collection.exception.InvalidIssueDe
import io.gitlab.arturbosch.detekt.generator.util.run
import org.assertj.core.api.Assertions.assertThat
import org.assertj.core.api.Assertions.assertThatExceptionOfType
import org.assertj.core.api.Assertions.assertThatThrownBy
import org.spekframework.spek2.Spek
import org.spekframework.spek2.style.specification.describe
@@ -520,13 +521,13 @@ object RuleCollectorSpec : Spek({
*/
class SomeRandomClass() : Rule {
@Configuration("description")
private val prop: Int by config(1)
val prop: Int by config(1)
@Configuration("description")
private val config1: Int by configWithFallback("prop", 99)
private val config1: Int by configWithFallback(this::prop, 99)
@Configuration("description")
private val config2: Int by configWithFallback(fallbackPropertyName = "prop", defaultValue = 99)
private val config2: Int by configWithFallback(fallbackProperty = ::prop, defaultValue = 99)
@Configuration("description")
private val config3: Int by configWithFallback(defaultValue = 99, fallbackPropertyName = "prop")
private val config3: Int by configWithFallback(defaultValue = 99, fallbackProperty = ::prop)
}
"""
val items = subject.run(code)
@@ -535,41 +536,23 @@ object RuleCollectorSpec : Spek({
assertThat(fallbackProperties.map { it.defaultValue }).containsOnly("99")
}
it("reports an error if the property to fallback on does not exist") {
val code = """
/**
* description
*/
class SomeRandomClass() : Rule {
@Configuration("description")
private val config: Int by configWithFallback("prop", 99)
}
"""
assertThatExceptionOfType(InvalidDocumentationException::class.java).isThrownBy {
subject.run(
code
)
}
}
it("reports an error if the property to fallback on exists but is not a config property") {
val code = """
/**
* description
*/
class SomeRandomClass() : Rule {
private val prop: Int = 1
val prop: Int = 1
@Configuration("description")
private val config: Int by configWithFallback("prop", 99)
private val config: Int by configWithFallback(::prop, 99)
}
"""
assertThatExceptionOfType(InvalidDocumentationException::class.java).isThrownBy {
subject.run(
code
)
}
assertThatThrownBy { subject.run(code) }
.isInstanceOf(InvalidDocumentationException::class.java)
.hasMessageContaining("delegate")
}
}
context("transformed property") {
val code = """
/**
@@ -591,66 +574,6 @@ object RuleCollectorSpec : Spek({
assertThat(items[0].configuration[1].defaultValue).isEqualTo("'false'")
}
}
context("fallback property") {
it("extracts default value") {
val code = """
/**
* description
*/
class SomeRandomClass() : Rule {
@Configuration("description")
private val prop: Int by config(1)
@Configuration("description")
private val config1: Int by configWithFallback("prop", 99)
@Configuration("description")
private val config2: Int by configWithFallback(fallbackPropertyName = "prop", defaultValue = 99)
@Configuration("description")
private val config3: Int by configWithFallback(defaultValue = 99, fallbackPropertyName = "prop")
@Configuration("description")
private val config4: Long by configWithFallback("prop", 99, Int::toLong)
}
"""
val items = subject.run(code)
val fallbackProperties = items[0].configuration.filter { it.name.startsWith("config") }
assertThat(fallbackProperties).hasSize(4)
assertThat(fallbackProperties.map { it.defaultValue }).containsOnly("99")
}
it("reports an error if the property to fallback on does not exist") {
val code = """
/**
* description
*/
class SomeRandomClass() : Rule {
@Configuration("description")
private val config: Int by configWithFallback("prop", 99)
}
"""
assertThatExceptionOfType(InvalidDocumentationException::class.java).isThrownBy {
subject.run(
code
)
}
}
it("reports an error if the property to fallback on exists but is not a config property") {
val code = """
/**
* description
*/
class SomeRandomClass() : Rule {
private val prop: Int = 1
@Configuration("description")
private val config: Int by configWithFallback("prop", 99)
}
"""
assertThatExceptionOfType(InvalidDocumentationException::class.java).isThrownBy {
subject.run(
code
)
}
}
}
}
}

View File

@@ -40,30 +40,25 @@ class LongParameterList(config: Config = Config.empty) : Rule(config) {
Debt.TWENTY_MINS
)
@Suppress("unused")
@Deprecated("Use `functionThreshold` and `constructorThreshold` instead")
@Configuration("number of parameters required to trigger the rule")
private val threshold: Int by config(DEFAULT_FUNCTION_THRESHOLD)
@Suppress("DEPRECATION")
@OptIn(UnstableApi::class)
@Configuration("number of function parameters required to trigger the rule")
private val functionThreshold: Int by configWithFallback(
fallbackPropertyName = "threshold",
defaultValue = DEFAULT_FUNCTION_THRESHOLD
)
private val functionThreshold: Int by configWithFallback(::threshold, DEFAULT_FUNCTION_THRESHOLD)
@Suppress("DEPRECATION")
@OptIn(UnstableApi::class)
@Configuration("number of constructor parameters required to trigger the rule")
private val constructorThreshold: Int by configWithFallback(
fallbackPropertyName = "threshold",
defaultValue = DEFAULT_CONSTRUCTOR_THRESHOLD
)
private val constructorThreshold: Int by configWithFallback(::threshold, DEFAULT_CONSTRUCTOR_THRESHOLD)
@Configuration("ignore parameters that have a default value")
private val ignoreDefaultParameters: Boolean by config(defaultValue = false)
private val ignoreDefaultParameters: Boolean by config(false)
@Configuration("ignore long constructor parameters list for data classes")
private val ignoreDataClasses: Boolean by config(defaultValue = true)
private val ignoreDataClasses: Boolean by config(true)
@Configuration(
"ignore long parameters list for constructors, functions or their parameters in the " +

View File

@@ -23,14 +23,14 @@ import org.jetbrains.kotlin.psi.psiUtil.getParentOfType
@ActiveByDefault(since = "1.0.0")
class EmptyFunctionBlock(config: Config) : EmptyRule(config) {
@Suppress("unused")
@Configuration("Excludes all the overridden functions")
@Deprecated("Use `ignoreOverridden` instead")
private val ignoreOverriddenFunctions: Boolean by config(false)
@Suppress("DEPRECATION")
@OptIn(UnstableApi::class)
@Configuration("Excludes all the overridden functions")
private val ignoreOverridden: Boolean by configWithFallback("ignoreOverriddenFunctions", false)
private val ignoreOverridden: Boolean by configWithFallback(::ignoreOverriddenFunctions, false)
override fun visitNamedFunction(function: KtNamedFunction) {
super.visitNamedFunction(function)

View File

@@ -36,14 +36,14 @@ class FunctionParameterNaming(config: Config = Config.empty) : Rule(config) {
@Configuration("ignores variables in classes which match this regex")
private val excludeClassPattern: Regex by config("$^", String::toRegex)
@Suppress("unused")
@Configuration("ignores overridden functions with parameters not matching the pattern")
@Deprecated("Use `ignoreOverridden` instead")
private val ignoreOverriddenFunctions: Boolean by config(true)
@Suppress("DEPRECATION")
@OptIn(UnstableApi::class)
@Configuration("ignores overridden functions with parameters not matching the pattern")
private val ignoreOverridden: Boolean by configWithFallback("ignoreOverriddenFunctions", true)
private val ignoreOverridden: Boolean by configWithFallback(::ignoreOverriddenFunctions, true)
override fun visitParameter(parameter: KtParameter) {
if (parameter.isContainingExcludedClass(excludeClassPattern)) {

View File

@@ -68,14 +68,14 @@ class MemberNameEqualsClassName(config: Config = Config.empty) : Rule(config) {
private val objectMessage = "A member is named after the object. " +
"This might result in confusion. Please rename the member."
@Suppress("unused")
@Configuration("if overridden functions and properties should be ignored")
@Deprecated("Use `ignoreOverridden` instead")
private val ignoreOverriddenFunction: Boolean by config(true)
@Suppress("DEPRECATION")
@OptIn(UnstableApi::class)
@Configuration("if overridden functions and properties should be ignored")
private val ignoreOverridden: Boolean by configWithFallback("ignoreOverriddenFunction", true)
private val ignoreOverridden: Boolean by configWithFallback(::ignoreOverriddenFunction, true)
override fun visitClass(klass: KtClass) {
if (!klass.isInterface()) {