Skip to content

Commit

Permalink
Fix backward incompatibility issues released via KtLint 0.45.0 and 0.…
Browse files Browse the repository at this point in the history
…45.1.

* Deprecate data class 'Params' as it only provides the userData parameter
  which prohibits API Consumers to split the '.editorconfig' properties
  from the other user data.
* Deprecate function 'toExperimentalParams' to force API Consumers to
  start using the 'ExperimentalParams'.
* In ExperimentalParams clarify that 'userData' should not contain any
  '.editorconfig' properties. Now also a runtime error is thrown
  whenever the lint/format methods are called and 'userData' contains
  '.editorconfig' properties.
* Remove confusing TODO's about moving methods 'lint' and 'format' to
  the 'ktlint-test' module.
* Update deprecation messages to clarify intention.
* The 'lint' and 'format' methods in module 'ktlint-core' no longer
  call the 'VisitorProvider' with parameter 'isUnitTestContext'
  enabled. API Consumers call these methods with production code
  for which this parameter should be disabled.
* The RuleExtension calls the 'lint' and 'format' methods but do
  provide a VisitorProvider for which the parameter
  'isUnitTestContext' is enabled.
* Deprecate the 'format' which accepted the 'ExperimentalParams'
  and 'Iterable<RuleSet>' while the latter is already included
  in the first.
* Add new 'format' method which accepts 'ExperimentalParams' and
  'VisitorProvider' only.
* Move 'EditorConfigOverride' from 'ktlint-test' to 'ktlint-core'

Closes pinterest#1434
  • Loading branch information
paul-dingemans committed Apr 2, 2022
1 parent 7487b47 commit 8b9fb7f
Show file tree
Hide file tree
Showing 18 changed files with 839 additions and 79 deletions.
82 changes: 66 additions & 16 deletions ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/KtLint.kt
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
package com.pinterest.ktlint.core

import com.pinterest.ktlint.core.api.DefaultEditorConfigProperties
import com.pinterest.ktlint.core.api.EditorConfigOverride
import com.pinterest.ktlint.core.api.EditorConfigOverride.Companion.emptyEditorConfigOverride
import com.pinterest.ktlint.core.api.EditorConfigProperties
import com.pinterest.ktlint.core.api.FeatureInAlphaState
import com.pinterest.ktlint.core.api.UsesEditorConfigProperties
import com.pinterest.ktlint.core.internal.EditorConfigGenerator
import com.pinterest.ktlint.core.internal.EditorConfigLoader
import com.pinterest.ktlint.core.internal.EditorConfigLoader.Companion.convertToRawValues
import com.pinterest.ktlint.core.internal.EditorConfigOverridesMap
import com.pinterest.ktlint.core.internal.KotlinPsiFileFactoryProvider
import com.pinterest.ktlint.core.internal.LineAndColumn
import com.pinterest.ktlint.core.internal.SuppressionLocator
Expand Down Expand Up @@ -59,6 +61,10 @@ public object KtLint {
* @param editorConfigPath optional path of the .editorconfig file (otherwise will use working directory)
* @param debug True if invoked with the --debug flag
*/
@Deprecated(
message = "Marked for removal in Ktlint 0.46.",
replaceWith = ReplaceWith("ExperimentalParams")
)
public data class Params(
val fileName: String? = null,
val text: String,
Expand All @@ -74,7 +80,7 @@ public object KtLint {
* @param fileName path of file to lint/format
* @param text Contents of file to lint/format
* @param ruleSets a collection of "RuleSet"s used to validate source
* @param userData Map of user options
* @param userData Map of user options. Should not be used for overrides of properties set in '.editorconfig'.
* @param cb callback invoked for each lint error
* @param script true if this is a Kotlin script file
* @param editorConfigPath optional path of the .editorconfig file (otherwise will use working directory)
Expand All @@ -100,9 +106,37 @@ public object KtLint {
val script: Boolean = false,
val editorConfigPath: String? = null,
val debug: Boolean = false,
val editorConfigOverride: EditorConfigOverridesMap = emptyMap(),
val editorConfigOverride: EditorConfigOverride = emptyEditorConfigOverride,
val isInvokedFromCli: Boolean = false
) {
init {
// Extract all default and custom ".editorconfig" properties which are defined using the
// [UsesEditorConfigProperties] interface of the rule
val editorConfigProperties =
ruleSets
.asSequence()
.flatten()
.filterIsInstance<UsesEditorConfigProperties>()
.map { it.editorConfigProperties }
.flatten()
.plus(DefaultEditorConfigProperties.defaultEditorConfigProperties)
.map { it.type.name }
.distinct()
.toSet()

userData
.keys
.intersect(editorConfigProperties)
.let {
check(it.isEmpty()) {
"UserData should not contain '.editorconfig' properties ${it.sorted()}. Such properties " +
"should be passed via the 'ExperimentalParams.editorConfigOverride' field. Note that this is " +
"only required for properties that (potentially) contain a value that differs from the " +
"actual value in the '.editorconfig' file."
}
}
}

internal val normalizedFilePath: Path? get() = if (fileName == STDIN_FILE || fileName == null) {
null
} else {
Expand All @@ -118,6 +152,10 @@ public object KtLint {
.toSet()
}

@Deprecated(
message = "Marked for removal in Ktlint 0.46.",
replaceWith = ReplaceWith("ExperimentalParams")
)
@OptIn(FeatureInAlphaState::class)
private fun toExperimentalParams(params: Params): ExperimentalParams =
ExperimentalParams(
Expand All @@ -137,10 +175,8 @@ public object KtLint {
* @throws ParseException if text is not a valid Kotlin code
* @throws RuleExecutionException in case of internal failure caused by a bug in rule implementation
*/
// TODO: Shouldn't this method be moved to module ktlint-test as it is called from unit tests only? It will be a
// breaking change for custom rule set implementations.
@Deprecated(
message = "Marked for removal in Ktlint 0.46. Convert userData to EditorConfigOverride.",
message = "Marked for removal in Ktlint 0.46. Move '.editorconfig' properties from 'Params.userData' to 'ExperimentalParam.editorConfigOverride'.",
replaceWith = ReplaceWith("lint(toExperimentalParams(params))")
)
@OptIn(FeatureInAlphaState::class)
Expand All @@ -153,8 +189,7 @@ public object KtLint {
experimentalParams,
VisitorProvider(
ruleSets = experimentalParams.ruleSets,
debug = experimentalParams.debug,
isUnitTestContext = true
debug = experimentalParams.debug
)
)
}
Expand All @@ -165,8 +200,6 @@ public object KtLint {
* @throws ParseException if text is not a valid Kotlin code
* @throws RuleExecutionException in case of internal failure caused by a bug in rule implementation
*/
// TODO: Shouldn't this method be moved to module ktlint-test as it is called from unit tests only? It will be a
// breaking change for custom rule set implementations.
@FeatureInAlphaState
public fun lint(
params: ExperimentalParams,
Expand Down Expand Up @@ -308,8 +341,10 @@ public object KtLint {
* @throws ParseException if text is not a valid Kotlin code
* @throws RuleExecutionException in case of internal failure caused by a bug in rule implementation
*/
// TODO: Shouldn't this method be moved to module ktlint-test as it is called from unit tests only? It will be a
// breaking change for custom rule set implementations.
@Deprecated(
message = "Marked for removal in Ktlint 0.46. Move '.editorconfig' properties from 'Params.userData' to 'ExperimentalParam.editorConfigOverride'.",
replaceWith = ReplaceWith("lint(toExperimentalParams(params))")
)
@OptIn(FeatureInAlphaState::class)
public fun format(params: Params): String =
format(toExperimentalParams(params))
Expand All @@ -321,8 +356,7 @@ public object KtLint {
experimentalParams.ruleSets,
VisitorProvider(
ruleSets = experimentalParams.ruleSets,
debug = experimentalParams.debug,
isUnitTestContext = true
debug = experimentalParams.debug
)
)
}
Expand All @@ -333,11 +367,27 @@ public object KtLint {
* @throws ParseException if text is not a valid Kotlin code
* @throws RuleExecutionException in case of internal failure caused by a bug in rule implementation
*/
@Deprecated(
message = "Marked for removal in Ktlint 0.46. Overrides of '.editorconfig' properties no longer should be " +
"passed via the 'Params.userData' but via the 'ExperimentalParam.editorConfigOverride' parameter. The " +
"ruleSets have to be provided via the 'ExperimentalParams.ruleSets' parameter.",
replaceWith = ReplaceWith("format(params, visitorProvider)")
)
@FeatureInAlphaState
public fun format(
params: ExperimentalParams,
ruleSets: Iterable<RuleSet>,
visitorProvider: VisitorProvider
): String =
format(
params.copy(ruleSets = ruleSets),
visitorProvider
)

@FeatureInAlphaState
public fun format(
params: ExperimentalParams,
visitorProvider: VisitorProvider
): String {
val hasUTF8BOM = params.text.startsWith(UTF8_BOM)
val psiFileFactory = kotlinPsiFileFactoryProvider.getKotlinPsiFileFactory(params.isInvokedFromCli)
Expand All @@ -347,7 +397,7 @@ public object KtLint {
var mutated = false
visitorProvider
.visitor(
ruleSets,
params.ruleSets,
preparedCode.rootNode,
concurrent = false
).invoke { node, rule, fqRuleId ->
Expand Down Expand Up @@ -378,7 +428,7 @@ public object KtLint {
if (tripped) {
val errors = mutableListOf<Pair<LintError, Boolean>>()
visitorProvider
.visitor(ruleSets, preparedCode.rootNode)
.visitor(params.ruleSets, preparedCode.rootNode)
.invoke { node, rule, fqRuleId ->
// fixme: enforcing suppression based on node.startOffset is wrong
// (not just because not all nodes are leaves but because rules are free to emit (and fix!) errors at any position)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
package com.pinterest.ktlint.test
package com.pinterest.ktlint.core.api

import com.pinterest.ktlint.core.api.FeatureInAlphaState
import com.pinterest.ktlint.core.api.UsesEditorConfigProperties.EditorConfigProperty
import org.ec4j.core.model.PropertyType

/**
* Properties set in [EditorConfigOverride] override values which are loaded from the ".EditorConfig" file. This
* [EditorConfigOverride] is only to be used in unit tests. From the perspective of the unit test as well as from the
* perspective of the rule, it is transparent whether an editor config property was loaded from an ".editorconfig" file
* or from an override.
* The [EditorConfigOverride] allows to add or replace properties which are loaded from the ".editorconfig" file. It
* serves two purposes.
*
* Firstly, the [EditorConfigOverride] can be used by API consumers to run a rule with values which are not actually
* save to the ".editorconfig" file. When doing so, this should be clearly communicated to their consumers who will
* expect the settings in that file to be respected.
*
* Secondly, the [EditorConfigOverride] is used in unit tests, to test a rule with distinct values of a property without
* having to access an ".editorconfig" file from physical storage. This also improves readability of the tests.
*/
@FeatureInAlphaState
public class EditorConfigOverride {
Expand All @@ -22,8 +26,7 @@ public class EditorConfigOverride {

public companion object {
/**
* Creates EditorConfig properties with value to be used in unit test so that the unit test does not need to
* write an .editorconfig file.
* Creates the [EditorConfigOverride] based on one or more property-value mappings.
*/
public fun from(
vararg properties: Pair<EditorConfigProperty<*>, *>
Expand All @@ -40,8 +43,8 @@ public class EditorConfigOverride {
}

/**
* Get the empty EditorConfig properties. As it does not contain any properties, all properties fall back on
* their respective default value.
* Get the empty [EditorConfigOverride]. As it does not contain any properties, all properties fall back on
* their respective default values.
*/
public val emptyEditorConfigOverride: EditorConfigOverride = EditorConfigOverride()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,13 @@ public object DefaultEditorConfigProperties {
}
}
)

public val defaultEditorConfigProperties: List<UsesEditorConfigProperties.EditorConfigProperty<out Any>> = listOf(
indentStyleProperty,
indentSizeProperty,
insertNewLineProperty,
maxLineLengthProperty
)
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.pinterest.ktlint.core.internal

import com.pinterest.ktlint.core.Rule
import com.pinterest.ktlint.core.api.EditorConfigOverride
import com.pinterest.ktlint.core.api.EditorConfigOverride.Companion.emptyEditorConfigOverride
import com.pinterest.ktlint.core.api.EditorConfigProperties
import com.pinterest.ktlint.core.api.FeatureInAlphaState
import com.pinterest.ktlint.core.api.UsesEditorConfigProperties
Expand All @@ -23,6 +25,10 @@ private val logger = KotlinLogging.logger {}.initKtLintKLogger()
* Map contains [UsesEditorConfigProperties.EditorConfigProperty] and related
* [PropertyType.PropertyValue] entries to add/replace loaded from `.editorconfig` files values.
*/
@Deprecated(
message = "Marked for removal in KtLint 0.46.",
replaceWith = ReplaceWith("EditorConfigOverride")
)
@FeatureInAlphaState
public typealias EditorConfigOverridesMap =
Map<UsesEditorConfigProperties.EditorConfigProperty<*>, PropertyType.PropertyValue<*>>
Expand All @@ -48,7 +54,7 @@ public class EditorConfigLoader(
* @param alternativeEditorConfig alternative to current [filePath] location where `.editorconfig` files should be
* looked up
* @param rules set of [Rule]s linting the file
* @param loadedValuesOverride map of values to add/replace values that were loaded from `.editorconfig` files
* @param editorConfigOverride map of values to add/replace values that were loaded from `.editorconfig` files
* @param debug pass `true` to enable some additional debug output
*
* @return all possible loaded properties applicable to given file.
Expand All @@ -60,13 +66,14 @@ public class EditorConfigLoader(
isStdIn: Boolean = false,
alternativeEditorConfig: Path? = null,
rules: Set<Rule>,
loadedValuesOverride: EditorConfigOverridesMap = emptyMap(),
editorConfigOverride: EditorConfigOverride = emptyEditorConfigOverride,
debug: Boolean = false
): EditorConfigProperties {
if (!isStdIn &&
(filePath == null || SUPPORTED_FILES.none { filePath.toString().endsWith(it) })
) {
return loadedValuesOverride
return editorConfigOverride
.properties
.map { (property, value) ->
property.type.name to Property.builder()
.name(property.type.name)
Expand Down Expand Up @@ -99,13 +106,15 @@ public class EditorConfigLoader(
)
.properties
.also { loaded ->
loadedValuesOverride.forEach {
loaded[it.key.type.name] = Property.builder()
.name(it.key.type.name)
.type(it.key.type)
.value(it.value)
.build()
}
editorConfigOverride
.properties
.forEach {
loaded[it.key.type.name] = Property.builder()
.name(it.key.type.name)
.type(it.key.type)
.value(it.value)
.build()
}
}
.also {
logger.trace {
Expand Down

0 comments on commit 8b9fb7f

Please sign in to comment.