Skip to content

Commit

Permalink
Fix the race condition introduced with 3470888
Browse files Browse the repository at this point in the history
### What's done:

 - Original issue: #1548.
 - Original PR: #1553.
 - This change reverts 3470888.
 - Enhances the KDoc of `DiktatRule`.
 - Fixes #1548.
  • Loading branch information
0x6675636b796f75676974687562 committed Nov 9, 2022
1 parent 0cb47bb commit 4f01cdd
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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<RulesConfig> = fileStream.use { stream ->
yamlSerializer.decodeFromString<List<RulesConfig>>(stream.readLines().joinToString(separator = "\n")).reversed().distinctBy { it.name }
}
Expand All @@ -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)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<RulesConfig> 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
Expand All @@ -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).
Expand Down Expand Up @@ -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) }
Expand Down

0 comments on commit 4f01cdd

Please sign in to comment.