From 4f01cddfaf5c731c84a9402765bd28ecc049e352 Mon Sep 17 00:00:00 2001 From: Andrey Shcheglov Date: Wed, 9 Nov 2022 14:32:08 +0300 Subject: [PATCH] Fix the race condition introduced with 34708889da981dd84b1539424fb2398643ac321f ### What's done: - Original issue: #1548. - Original PR: #1553. - This change reverts 34708889da981dd84b1539424fb2398643ac321f. - Enhances the KDoc of `DiktatRule`. - Fixes #1548. --- .../common/config/rules/RulesConfigReader.kt | 11 ++----- .../cqfn/diktat/ruleset/rules/DiktatRule.kt | 14 +++++++-- .../ruleset/rules/DiktatRuleSetProvider.kt | 31 ++++++++----------- 3 files changed, 28 insertions(+), 28 deletions(-) diff --git a/diktat-common/src/main/kotlin/org/cqfn/diktat/common/config/rules/RulesConfigReader.kt b/diktat-common/src/main/kotlin/org/cqfn/diktat/common/config/rules/RulesConfigReader.kt index 5605f89600..c063e840ce 100644 --- a/diktat-common/src/main/kotlin/org/cqfn/diktat/common/config/rules/RulesConfigReader.kt +++ b/diktat-common/src/main/kotlin/org/cqfn/diktat/common/config/rules/RulesConfigReader.kt @@ -18,7 +18,6 @@ import java.io.File import java.util.Locale import java.util.concurrent.atomic.AtomicInteger -import kotlinx.serialization.ExperimentalSerializationApi import kotlinx.serialization.Serializable import kotlinx.serialization.decodeFromString @@ -84,7 +83,6 @@ open class RulesConfigReader(override val classLoader: ClassLoader) : JsonResour * @param fileStream a [BufferedReader] representing loaded rules config file * @return list of [RulesConfig] */ - @OptIn(ExperimentalSerializationApi::class) override fun parseResource(fileStream: BufferedReader): List = fileStream.use { stream -> yamlSerializer.decodeFromString>(stream.readLines().joinToString(separator = "\n")).reversed().distinctBy { it.name } } @@ -99,19 +97,16 @@ open class RulesConfigReader(override val classLoader: ClassLoader) : JsonResour override fun getConfigFile(resourceFileName: String): BufferedReader? { val resourceFile = File(resourceFileName) return if (resourceFile.exists()) { - log.debug("Using diktat-analysis.yml file from the following path: ${resourceFile.absolutePath}") + log.debug("Using $DIKTAT_ANALYSIS_CONF file from the following path: ${resourceFile.absolutePath}") File(resourceFileName).bufferedReader() } else { - log.debug("Using the default diktat-analysis.yml file from the class path") + log.debug("Using the default $DIKTAT_ANALYSIS_CONF file from the class path") classLoader.getResourceAsStream(resourceFileName)?.bufferedReader() } } companion object { - /** - * A [Logger] that can be used - */ - val log: KLogger = KotlinLogging.loggerWithKtlintConfig(RulesConfigReader::class) + internal val log: KLogger = KotlinLogging.loggerWithKtlintConfig(RulesConfigReader::class) } } diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRule.kt index 1d5bd70901..b37b46f15b 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRule.kt @@ -14,7 +14,7 @@ import org.jetbrains.kotlin.com.intellij.lang.ASTNode private typealias DiktatConfigRule = org.cqfn.diktat.common.config.rules.Rule /** - * This is a wrapper around Ktlint Rule + * This is a wrapper around _KtLint_ `Rule`. * * @param id id of the rule * @property configRules all rules from configuration @@ -33,7 +33,17 @@ abstract class DiktatRule( var isFixMode: Boolean = false /** - * Will be initialized in visit + * The **file-specific** error emitter, initialized in + * [beforeVisitChildNodes] and used in [logic] implementations. + * + * Since the file is indirectly a part of the state of a `Rule`, the same + * `Rule` instance should **never be re-used** to check more than a single + * file, or confusing effects (incl. race conditions) will occur. + * See the documentation of the [Rule] class for more details. + * + * @see Rule + * @see beforeVisitChildNodes + * @see logic */ lateinit var emitWarn: EmitType diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt index 0ac5cc067d..661056c931 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt @@ -108,16 +108,7 @@ class DiktatRuleSetProvider(private var diktatConfigFile: String = DIKTAT_ANALYS yield(resolveConfigFileFromJarLocation()) yield(resolveConfigFileFromSystemProperty()) } - - /** - * As of _KtLint_ **0.47**, each rule is expected to have a state and is executed - * twice per file, and a new `Rule` instance is created per each file checked. - * - * Diktat rules have no mutable state yet and use the deprecated _KtLint_ - * API, so we initialize them only _once_ for performance reasons and also - * to avoid redundant logging. - */ - private val ruleSet: RuleSet by lazy { + private val configRules: List by lazy { log.debug("Will run $DIKTAT_RULE_SET_ID with $diktatConfigFile" + " (it can be placed to the run directory or the default file from resources will be used)") val configPath = possibleConfigs @@ -137,10 +128,20 @@ class DiktatRuleSetProvider(private var diktatConfigFile: String = DIKTAT_ANALYS diktatConfigFile } - val configRules = RulesConfigReader(javaClass.classLoader) + RulesConfigReader(javaClass.classLoader) .readResource(diktatConfigFile) ?.onEach(::validate) ?: emptyList() + } + + @Suppress( + "LongMethod", + "TOO_LONG_FUNCTION", + ) + @Deprecated( + "Marked for removal in KtLint 0.48. See changelog or KDoc for more information.", + ) + override fun get(): RuleSet { // Note: the order of rules is important in autocorrect mode. For example, all rules that add new code should be invoked before rules that fix formatting. // We don't have a way to enforce a specific order, so we should just be careful when adding new rules to this list and, when possible, // cover new rules in smoke test as well. If a rule needs to be at a specific position in a list, please add comment explaining it (like for NewlinesRule). @@ -229,18 +230,12 @@ class DiktatRuleSetProvider(private var diktatConfigFile: String = DIKTAT_ANALYS .map { it.invoke(configRules) } - RuleSet( + return RuleSet( DIKTAT_RULE_SET_ID, rules = rules.toTypedArray() ).ordered() } - @Deprecated( - "Marked for removal in KtLint 0.48. See changelog or KDoc for more information.", - ) - override fun get(): RuleSet = - ruleSet - private fun validate(config: RulesConfig) = require(config.name == DIKTAT_COMMON || config.name in Warnings.names) { val closestMatch = Warnings.names.minByOrNull { Levenshtein.distance(it, config.name) }