diff --git a/detekt-api/api/detekt-api.api b/detekt-api/api/detekt-api.api index 15b3cc03e..301c25f51 100644 --- a/detekt-api/api/detekt-api.api +++ b/detekt-api/api/detekt-api.api @@ -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 { diff --git a/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/ConfigProperty.kt b/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/ConfigProperty.kt index 5c98aba46..b519b819e 100644 --- a/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/ConfigProperty.kt +++ b/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/ConfigProperty.kt @@ -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 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] are supported. */ @UnstableApi("fallback property handling is still under discussion") fun configWithFallback( - fallbackPropertyName: String, + fallbackProperty: KProperty0, defaultValue: T -): ReadOnlyProperty = configWithFallback(fallbackPropertyName, defaultValue) { it } +): ReadOnlyProperty = 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] are supported. * @param transformer a function that transforms the value from the configuration (or the default) into its final @@ -67,10 +68,11 @@ fun configWithFallback( */ @UnstableApi("fallback property handling is still under discussion") fun configWithFallback( - fallbackPropertyName: String, + fallbackProperty: KProperty0, defaultValue: T, transformer: (T) -> U -): ReadOnlyProperty = FallbackConfigProperty(fallbackPropertyName, defaultValue, transformer) +): ReadOnlyProperty = + 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( } private class FallbackConfigProperty( - private val fallbackPropertyName: String, + private val fallbackProperty: KProperty0, private val defaultValue: T, private val transform: (T) -> U ) : MemoizedConfigProperty() { 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(propertyName) != null } diff --git a/detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/ConfigPropertySpec.kt b/detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/ConfigPropertySpec.kt index 7a75e2e06..9ee51433d 100644 --- a/detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/ConfigPropertySpec.kt +++ b/detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/ConfigPropertySpec.kt @@ -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") diff --git a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/ConfigurationCollector.kt b/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/ConfigurationCollector.kt index 74d6a45fa..1f4fa5edc 100644 --- a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/ConfigurationCollector.kt +++ b/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/ConfigurationCollector.kt @@ -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) { - 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 { diff --git a/detekt-generator/src/test/kotlin/io/gitlab/arturbosch/detekt/generator/collection/RuleCollectorSpec.kt b/detekt-generator/src/test/kotlin/io/gitlab/arturbosch/detekt/generator/collection/RuleCollectorSpec.kt index a7efc5eac..ca2725d0c 100644 --- a/detekt-generator/src/test/kotlin/io/gitlab/arturbosch/detekt/generator/collection/RuleCollectorSpec.kt +++ b/detekt-generator/src/test/kotlin/io/gitlab/arturbosch/detekt/generator/collection/RuleCollectorSpec.kt @@ -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 - ) - } - } - } } } diff --git a/detekt-rules-complexity/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/LongParameterList.kt b/detekt-rules-complexity/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/LongParameterList.kt index a9b9a8958..ca0e5f4ae 100644 --- a/detekt-rules-complexity/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/LongParameterList.kt +++ b/detekt-rules-complexity/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/LongParameterList.kt @@ -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 " + diff --git a/detekt-rules-empty/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/empty/EmptyFunctionBlock.kt b/detekt-rules-empty/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/empty/EmptyFunctionBlock.kt index 0bfda6341..786c8b56e 100644 --- a/detekt-rules-empty/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/empty/EmptyFunctionBlock.kt +++ b/detekt-rules-empty/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/empty/EmptyFunctionBlock.kt @@ -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) diff --git a/detekt-rules-naming/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/naming/FunctionParameterNaming.kt b/detekt-rules-naming/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/naming/FunctionParameterNaming.kt index be5eef9de..619121991 100644 --- a/detekt-rules-naming/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/naming/FunctionParameterNaming.kt +++ b/detekt-rules-naming/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/naming/FunctionParameterNaming.kt @@ -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)) { diff --git a/detekt-rules-naming/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/naming/MemberNameEqualsClassName.kt b/detekt-rules-naming/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/naming/MemberNameEqualsClassName.kt index 374612196..b62b56750 100644 --- a/detekt-rules-naming/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/naming/MemberNameEqualsClassName.kt +++ b/detekt-rules-naming/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/naming/MemberNameEqualsClassName.kt @@ -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()) {