Skip to content

Commit

Permalink
Set the default values of extendedIndentAfterOperators and `extende…
Browse files Browse the repository at this point in the history
…dIndentBeforeDot` to `false` (#1342)

### What's done:

 * Since Kotlin style guide mentions no continuation indent,
   and IDEA defaults to no continuation indent as well,
   all `extendedIndent*` flags default to `false` from now on.
 * Reformatted the codebase by running `mvn diktat:fix@diktat` so that it conforms to the new rules.
 * Unit tests adapted to the reformatted test resources.
 * Unit tests adapted to the new defaults.
 * Closes: #1312
  • Loading branch information
0x6675636b796f75676974687562 authored Jun 3, 2022
1 parent 92f2fef commit f5d73ed
Show file tree
Hide file tree
Showing 100 changed files with 1,442 additions and 1,407 deletions.
4 changes: 2 additions & 2 deletions diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@
# If true: if first parameter in parameter list is on the same line as opening parenthesis, then other parameters can be aligned with it
alignedParameters: true
# If true: if expression is split by newline after operator like +/-/`*`, then the next line is indented with two indentations instead of one
extendedIndentAfterOperators: true
extendedIndentAfterOperators: false
# If true: when dot qualified expression starts on a new line, this line will be indented with two indentations instead of one
extendedIndentBeforeDot: false
# The indentation size for each file
Expand Down Expand Up @@ -527,4 +527,4 @@
enabled: true
# Only properties from the primary constructor should be documented in a @property tag in class KDoc
- name: KDOC_NO_CLASS_BODY_PROPERTIES_IN_HEADER
enabled: true
enabled: true
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ abstract class JsonResourceConfigReader<T> {
* @return [BufferedReader] representing loaded resource
*/
protected open fun getConfigFile(resourceFileName: String): BufferedReader? =
classLoader.getResourceAsStream(resourceFileName)?.bufferedReader()
classLoader.getResourceAsStream(resourceFileName)?.bufferedReader()

/**
* you can specify your own parser, in example for parsing stream as a json
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,13 +202,13 @@ fun List<RulesConfig>.isRuleEnabled(rule: Rule): Boolean {
* @return true if the code block is marked with annotation that is in `ignored list` in the rule
*/
fun List<RulesConfig>.isAnnotatedWithIgnoredAnnotation(rule: Rule, annotations: Set<String>): Boolean =
getRuleConfig(rule)
?.ignoreAnnotated
?.map { it.trim() }
?.map { it.trim('"') }
?.intersect(annotations)
?.isNotEmpty()
?: false
getRuleConfig(rule)
?.ignoreAnnotated
?.map { it.trim() }
?.map { it.trim('"') }
?.intersect(annotations)
?.isNotEmpty()
?: false

/**
* Parse string into KotlinVersion
Expand Down
2 changes: 1 addition & 1 deletion diktat-common/src/test/resources/test-rules-config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@
newlineAtEnd: true
extendedIndentOfParameters: false
alignedParameters: true
extendedIndentAfterOperators: true
extendedIndentAfterOperators: false
indentationSize: 4
- name: EMPTY_BLOCK_STRUCTURE_ERROR
enabled: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ open class DiktatJavaExecTaskBase @Inject constructor(

@Suppress("MagicNumber")
private fun isMainClassPropertySupported(gradleVersionString: String) =
GradleVersion.version(gradleVersionString) >= GradleVersion.version("6.4")
GradleVersion.version(gradleVersionString) >= GradleVersion.version("6.4")

private fun MutableList<String>.addInput(file: File) {
add(file.toRelativeString(project.projectDir))
Expand Down Expand Up @@ -226,7 +226,7 @@ open class DiktatJavaExecTaskBase @Inject constructor(
}

private fun getJavaExecJvmVersion(): JavaVersion = if (GradleVersion.version(gradleVersionString) >= GradleVersion.version("6.7") &&
javaLauncher.isPresent
javaLauncher.isPresent
) {
// Java Launchers are available since 6.7, but may not always be configured
javaLauncher.map { it.metadata.jvmVersion }.map(JavaVersion::toVersion).get()
Expand All @@ -247,10 +247,10 @@ fun Project.registerDiktatCheckTask(diktatExtension: DiktatExtension,
diktatConfiguration: Configuration,
patternSet: PatternSet
): TaskProvider<DiktatJavaExecTaskBase> =
tasks.register(
DIKTAT_CHECK_TASK, DiktatJavaExecTaskBase::class.java, gradle.gradleVersion,
diktatExtension, diktatConfiguration, patternSet
)
tasks.register(
DIKTAT_CHECK_TASK, DiktatJavaExecTaskBase::class.java, gradle.gradleVersion,
diktatExtension, diktatConfiguration, patternSet
)

/**
* @param diktatExtension [DiktatExtension] with some values for task configuration
Expand All @@ -262,7 +262,7 @@ fun Project.registerDiktatFixTask(diktatExtension: DiktatExtension,
diktatConfiguration: Configuration,
patternSet: PatternSet
): TaskProvider<DiktatJavaExecTaskBase> =
tasks.register(
DIKTAT_FIX_TASK, DiktatJavaExecTaskBase::class.java, gradle.gradleVersion,
diktatExtension, diktatConfiguration, patternSet, listOf("-F ")
)
tasks.register(
DIKTAT_FIX_TASK, DiktatJavaExecTaskBase::class.java, gradle.gradleVersion,
diktatExtension, diktatConfiguration, patternSet, listOf("-F ")
)
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,4 @@ class KotlinClosure1<in T : Any?, V : Any>(
"KDOC_WITHOUT_RETURN_TAG"
)
fun <T> Any.closureOf(action: T.() -> Unit): Closure<Any?> =
KotlinClosure1(action, this, this)
KotlinClosure1(action, this, this)
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ abstract class DiktatBaseMojo : AbstractMojo() {
throw MojoExecutionException("Configuration file $diktatConfigFile doesn't exist")
}
log.info("Running diKTat plugin with configuration file $configFile and inputs $inputs" +
if (excludes.isNotEmpty()) " and excluding $excludes" else ""
if (excludes.isNotEmpty()) " and excluding $excludes" else ""
)

val ruleSets by lazy {
Expand Down Expand Up @@ -236,24 +236,24 @@ abstract class DiktatBaseMojo : AbstractMojo() {
) {
val text = file.readText()
val params =
KtLint.ExperimentalParams(
fileName = file.relativeTo(mavenProject.basedir).path,
text = text,
ruleSets = ruleSets,
userData = mapOf("file_path" to file.path),
script = file.extension.equals("kts", ignoreCase = true),
cb = { lintError, isCorrected ->
if (baselineErrors.none {
// ktlint's BaselineReporter stores only these fields
it.line == lintError.line && it.col == lintError.col &&
it.ruleId == lintError.ruleId
}) {
reporterImpl.onLintError(file.path, lintError, isCorrected)
lintErrors.add(lintError)
}
},
debug = debug
)
KtLint.ExperimentalParams(
fileName = file.relativeTo(mavenProject.basedir).path,
text = text,
ruleSets = ruleSets,
userData = mapOf("file_path" to file.path),
script = file.extension.equals("kts", ignoreCase = true),
cb = { lintError, isCorrected ->
if (baselineErrors.none {
// ktlint's BaselineReporter stores only these fields
it.line == lintError.line && it.col == lintError.col &&
it.ruleId == lintError.ruleId
}) {
reporterImpl.onLintError(file.path, lintError, isCorrected)
lintErrors.add(lintError)
}
},
debug = debug
)
runAction(params)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ fun Warnings.isRuleFromActiveChapter(configRules: List<RulesConfig>): Boolean {
fun Warnings.getChapterByWarning() = Chapters.values().find { it.number == this.ruleId.first().toString() }!!

private fun validate(chapter: String) =
require(chapter in Chapters.values().map { it.title }) {
val closestMatch = Chapters.values().minByOrNull { Levenshtein.distance(it.title, chapter) }
"Chapter name <$chapter> in configuration file is invalid, did you mean <$closestMatch>?"
}
require(chapter in Chapters.values().map { it.title }) {
val closestMatch = Chapters.values().minByOrNull { Levenshtein.distance(it.title, chapter) }
"Chapter name <$chapter> in configuration file is invalid, did you mean <$closestMatch>?"
}
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ enum class Warnings(
NO_BRACES_IN_CONDITIONALS_AND_LOOPS(true, "3.2.1", "in if, else, when, for, do, and while statements braces should be used. Exception: single line if statement."),
WRONG_ORDER_IN_CLASS_LIKE_STRUCTURES(true, "3.1.4", "the declaration part of a class-like code structures (class/interface/etc.) should be in the proper order"),
BLANK_LINE_BETWEEN_PROPERTIES(true, "3.1.4", "there should be no blank lines between properties without comments; comment, KDoc or annotation on property should have blank" +
" line before"),
" line before"),
TOP_LEVEL_ORDER(true, "3.1.5", "the declaration part of a top level elements should be in the proper order"),
BRACES_BLOCK_STRUCTURE_ERROR(true, "3.2.2", "braces should follow 1TBS style"),
WRONG_INDENTATION(true, "3.3.1", "only spaces are allowed for indentation and each indentation should equal to 4 spaces (tabs are not allowed)"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ import kotlin.io.path.writeLines
* The comment that will be added to the generated sources file.
*/
private val autoGenerationComment =
"""
| This document was auto generated, please don't modify it.
| This document contains all enum properties from Warnings.kt as Strings.
""".trimMargin()
"""
| This document was auto generated, please don't modify it.
| This document contains all enum properties from Warnings.kt as Strings.
""".trimMargin()

fun main() {
generateWarningNames()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ abstract class DiktatRule(
}

private fun areInspectionsDisabled(): Boolean =
inspections.none { configRules.isRuleEnabled(it) }
inspections.none { configRules.isRuleEnabled(it) }

/**
* Logic of the rule
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,20 +118,20 @@ class DiktatRuleSetProvider(private var diktatConfigFile: String = DIKTAT_ANALYS
)
override fun get(): RuleSet {
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)")
" (it can be placed to the run directory or the default file from resources will be used)")
val configPath = possibleConfigs
.firstOrNull { it != null && File(it).exists() }
diktatConfigFile = configPath
?: run {
val possibleConfigsList = possibleConfigs.toList()
log.warn(
"Configuration file not found in directory where diktat is run (${possibleConfigsList[0]}) " +
"or in the directory where diktat.jar is stored (${possibleConfigsList[1]}) " +
"or in system property <diktat.config.path> (${possibleConfigsList[2]}), " +
"the default file included in jar will be used. " +
"Some configuration options will be disabled or substituted with defaults. " +
"Custom configuration file should be placed in diktat working directory if run from CLI " +
"or provided as configuration options in plugins."
"or in the directory where diktat.jar is stored (${possibleConfigsList[1]}) " +
"or in system property <diktat.config.path> (${possibleConfigsList[2]}), " +
"the default file included in jar will be used. " +
"Some configuration options will be disabled or substituted with defaults. " +
"Custom configuration file should be placed in diktat working directory if run from CLI " +
"or provided as configuration options in plugins."
)
diktatConfigFile
}
Expand Down Expand Up @@ -241,23 +241,23 @@ class DiktatRuleSetProvider(private var diktatConfigFile: String = DIKTAT_ANALYS
}

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) }
"Warning name <${config.name}> in configuration file is invalid, did you mean <$closestMatch>?"
}
require(config.name == DIKTAT_COMMON || config.name in Warnings.names) {
val closestMatch = Warnings.names.minByOrNull { Levenshtein.distance(it, config.name) }
"Warning name <${config.name}> in configuration file is invalid, did you mean <$closestMatch>?"
}

private fun resolveDefaultConfig() = diktatConfigFile

private fun resolveConfigFileFromJarLocation(): String {
// for some aggregators of static analyzers we need to provide configuration for cli
// in this case diktat would take the configuration from the directory where jar file is stored
val ruleSetProviderPath =
DiktatRuleSetProvider::class
.java
.protectionDomain
.codeSource
.location
.toURI()
DiktatRuleSetProvider::class
.java
.protectionDomain
.codeSource
.location
.toURI()

val configPathWithFileName = File(ruleSetProviderPath).absolutePath

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(
}
// check if identifier of a property has a confusing name
if (confusingIdentifierNames.contains(variableName.text) && !isValidCatchIdentifier(variableName) &&
node.elementType == ElementType.PROPERTY
node.elementType == ElementType.PROPERTY
) {
warnConfusingName(variableName)
}
Expand All @@ -193,7 +193,7 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(
?.forEach { (it.node.firstChildNode as LeafPsiElement).rawReplaceWithText(correctVariableName) }
if (variableName.treeParent.psi.run {
(this is KtProperty && isMember) ||
(this is KtParameter && getParentOfType<KtPrimaryConstructor>(true)?.valueParameters?.contains(this) == true)
(this is KtParameter && getParentOfType<KtPrimaryConstructor>(true)?.valueParameters?.contains(this) == true)
}) {
// For class members also check `@property` KDoc tag.
// If we are here, then `variableName` is definitely a node from a class or an object.
Expand Down Expand Up @@ -257,7 +257,7 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(
destructingDeclaration.getAllChildrenWithType(DESTRUCTURING_DECLARATION_ENTRY)
.map { it.getIdentifierName()!! }
} else if (node.parents().count() > 1 && node.treeParent.elementType == VALUE_PARAMETER_LIST &&
node.treeParent.treeParent.elementType == FUNCTION_TYPE
node.treeParent.treeParent.elementType == FUNCTION_TYPE
) {
listOfNotNull(node.getIdentifierName())
} else {
Expand Down Expand Up @@ -434,7 +434,7 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(
nodes.forEach {
val isValidOneCharVariable = oneCharIdentifiers.contains(it.text) && isVariable
if (it.text != "_" && !it.isTextLengthInRange(MIN_IDENTIFIER_LENGTH..MAX_IDENTIFIER_LENGTH) &&
!isValidOneCharVariable && !isValidCatchIdentifier(it)
!isValidOneCharVariable && !isValidCatchIdentifier(it)
) {
IDENTIFIER_LENGTH.warn(configRules, emitWarn, isFixMode, it.text, it.startOffset, it)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ class PackageNaming(configRules: List<RulesConfig>) : DiktatRule(
}
} ?: if (visitorCounter.incrementAndGet() == 1) {
log.error("Not able to find an external configuration for domain" +
" name in the common configuration (is it missing in yml config?)")
" name in the common configuration (is it missing in yml config?)")
}
}

Expand Down Expand Up @@ -123,7 +123,7 @@ class PackageNaming(configRules: List<RulesConfig>) : DiktatRule(

return if (!filePathParts.contains(PACKAGE_PATH_ANCHOR)) {
log.error("Not able to determine a path to a scanned file or \"$PACKAGE_PATH_ANCHOR\" directory cannot be found in it's path." +
" Will not be able to determine correct package name. It can happen due to missing <$PACKAGE_PATH_ANCHOR> directory in the path")
" Will not be able to determine correct package name. It can happen due to missing <$PACKAGE_PATH_ANCHOR> directory in the path")
emptyList()
} else {
// creating a real package name:
Expand Down Expand Up @@ -198,8 +198,8 @@ class PackageNaming(configRules: List<RulesConfig>) : DiktatRule(
val wordFromPackage = word.replace("_", "")

return wordFromPackage[0].isDigit() ||
wordFromPackage.isKotlinKeyWord() ||
wordFromPackage.isJavaKeyWord()
wordFromPackage.isKotlinKeyWord() ||
wordFromPackage.isJavaKeyWord()
}

/**
Expand Down
Loading

0 comments on commit f5d73ed

Please sign in to comment.