Fix for issue #407 - no newline between consecutive braces in autocor…

…rect mode (#463) ### What's done: * Rearranged rules in DiktatRuleSetProvider * Code style
saveourtool · Oct 29, 2020 · 6852628 · 6852628 · 0pdd · Oct 29, 2020
1 parent 2d9e88e
commit 6852628
Show file tree

Hide file tree

Showing 18 changed files with 115 additions and 54 deletions.
diff --git a/diktat-common/src/main/kotlin/org/cqfn/diktat/common/cli/CliArgument.kt b/diktat-common/src/main/kotlin/org/cqfn/diktat/common/cli/CliArgument.kt
@@ -1,8 +1,9 @@
 package org.cqfn.diktat.common.cli
 
-import kotlinx.serialization.*
 import org.apache.commons.cli.Option
 
+import kotlinx.serialization.*
+
 /**
  * This class is used to serialize/deserialize json representation
  * that is used to store command line arguments

diff --git a/diktat-common/src/main/kotlin/org/cqfn/diktat/common/config/reader/ApplicationProperties.kt b/diktat-common/src/main/kotlin/org/cqfn/diktat/common/config/reader/ApplicationProperties.kt
@@ -1,9 +1,11 @@
 package org.cqfn.diktat.common.config.reader
 
+import org.slf4j.LoggerFactory
+
 import java.io.IOException
 import java.util.Properties
+
 import kotlin.system.exitProcess
-import org.slf4j.LoggerFactory
 
 /**
  * Base class for working with properties files.

diff --git a/...t-common/src/main/kotlin/org/cqfn/diktat/common/config/reader/JsonResourceConfigReader.kt b/...t-common/src/main/kotlin/org/cqfn/diktat/common/config/reader/JsonResourceConfigReader.kt
@@ -1,10 +1,11 @@
 package org.cqfn.diktat.common.config.reader
 
-import java.io.BufferedReader
-import java.io.IOException
 import org.slf4j.Logger
 import org.slf4j.LoggerFactory
 
+import java.io.BufferedReader
+import java.io.IOException
+
 /**
  * This class is used to read some resource in any format that you will specify.
  * Usage:

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
@@ -4,15 +4,18 @@
 
 package org.cqfn.diktat.common.config.rules
 
+import org.cqfn.diktat.common.config.reader.JsonResourceConfigReader
+
 import com.charleskorn.kaml.Yaml
 import com.charleskorn.kaml.YamlConfiguration
+import org.slf4j.Logger
+import org.slf4j.LoggerFactory
+
 import java.io.BufferedReader
 import java.io.File
+
 import kotlinx.serialization.Serializable
 import kotlinx.serialization.decodeFromString
-import org.cqfn.diktat.common.config.reader.JsonResourceConfigReader
-import org.slf4j.Logger
-import org.slf4j.LoggerFactory
 
 const val DIKTAT_COMMON = "DIKTAT_COMMON"
 
@@ -98,7 +101,7 @@ fun List<RulesConfig>.getCommonConfiguration() = lazy {
  *
  * @param configuration map of common configuration
  */
-class CommonConfiguration(configuration: Map<String, String>?) {
+data class CommonConfiguration(private val configuration: Map<String, String>?) {
     /**
      * List of directory names which will be used to detect test sources
      */

diff --git a/diktat-maven-plugin/src/main/kotlin/org/cqfn/diktat/plugin/maven/DiktatBaseMojo.kt b/diktat-maven-plugin/src/main/kotlin/org/cqfn/diktat/plugin/maven/DiktatBaseMojo.kt
@@ -1,18 +1,20 @@
 package org.cqfn.diktat.plugin.maven
 
+import org.cqfn.diktat.ruleset.rules.DiktatRuleSetProvider
+
 import com.pinterest.ktlint.core.KtLint
 import com.pinterest.ktlint.core.LintError
 import com.pinterest.ktlint.core.RuleExecutionException
 import com.pinterest.ktlint.core.RuleSet
 import com.pinterest.ktlint.reporter.plain.PlainReporter
-import java.io.File
 import org.apache.maven.plugin.AbstractMojo
 import org.apache.maven.plugin.MojoExecutionException
 import org.apache.maven.plugin.MojoFailureException
 import org.apache.maven.plugins.annotations.Mojo
 import org.apache.maven.plugins.annotations.Parameter
 import org.apache.maven.project.MavenProject
-import org.cqfn.diktat.ruleset.rules.DiktatRuleSetProvider
+
+import java.io.File
 
 /**
  * Base [Mojo] for checking and fixing code using diktat

diff --git a/diktat-maven-plugin/src/main/kotlin/org/cqfn/diktat/plugin/maven/DiktatMojo.kt b/diktat-maven-plugin/src/main/kotlin/org/cqfn/diktat/plugin/maven/DiktatMojo.kt
@@ -4,10 +4,12 @@
 
 package org.cqfn.diktat.plugin.maven
 
+import org.cqfn.diktat.ruleset.rules.DiktatRuleSetProvider
+
 import com.pinterest.ktlint.core.KtLint
-import java.io.File
 import org.apache.maven.plugins.annotations.Mojo
-import org.cqfn.diktat.ruleset.rules.DiktatRuleSetProvider
+
+import java.io.File
 
 /**
  * Main [Mojo] that call [DiktatRuleSetProvider]'s rules on [inputs] files

diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/BlockStructureBraces.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/BlockStructureBraces.kt
@@ -46,6 +46,16 @@ import org.jetbrains.kotlin.com.intellij.psi.tree.IElementType
 import org.jetbrains.kotlin.psi.KtIfExpression
 import org.jetbrains.kotlin.psi.KtTryExpression
 
+/**
+ * This rule checks that *non-empty* code blocks with braces follow the K&R style (1TBS or OTBS style):
+ * - The opening brace is on the same same line with the first line of the code block
+ * - The closing brace is on it's new line
+ * - The closing brace can be followed by a new line. Only exceptions are: `else`, `finally`, `while` (from do-while statement) or `catch` keywords.
+ *   These keywords should not be split from the closing brace by a newline.
+ * Exceptions:
+ * - opening brace of lambda
+ * - braces around `else`/`catch`/`finally`/`while` (in `do-while` loop)
+ */
 class BlockStructureBraces(private val configRules: List<RulesConfig>) : Rule("block-structure") {
     private var isFixMode: Boolean = false
     private lateinit var emitWarn: ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit)

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
@@ -45,35 +45,40 @@ class DiktatRuleSetProvider(private val diktatConfigFile: String = "diktat-analy
             .readResource(diktatConfigFile)
             ?.onEach(::validate)
             ?: emptyList()
+        // 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).
         val rules = listOf(
+                // comments & documentation
                 ::CommentsRule,
                 ::KdocComments,
                 ::KdocMethods,
                 ::KdocFormatting,
+                ::CommentsFormatting,
+                // naming
                 ::FileNaming,
                 ::PackageNaming,
-                ::StringTemplateFormatRule,
-                ::FileSize,
-                ::DataClassesRule,
                 ::IdentifierNaming,
-                ::LocalVariablesRule,
+                // code structure
                 ::ClassLikeStructuresOrderRule,
+                ::WhenMustHaveElseRule,
                 ::BracesInConditionalsAndLoopsRule,
                 ::BlockStructureBraces,
                 ::EmptyBlock,
+                ::EnumsSeparated,
+                ::SingleLineStatementsRule,
+                ::MultipleModifiersSequence,
+                // other rules
+                ::StringTemplateFormatRule,
+                ::DataClassesRule,
+                ::LocalVariablesRule,
                 ::SmartCastRule,
                 ::PropertyAccessorFields,
-                ::EnumsSeparated,
                 ::AbstractClassesRule,
                 ::VariableGenericTypeDeclarationRule,
-                ::SingleLineStatementsRule,
-                ::CommentsFormatting,
-                ::ConsecutiveSpacesRule,
                 ::LongNumericalValuesSeparatedRule,
                 ::NestedFunctionBlock,
-                ::MultipleModifiersSequence,
                 ::AnnotationNewLineRule,
-                ::HeaderCommentRule,
                 ::SortRule,
                 ::StringConcatenationRule,
                 ::AccurateCalculationsRule,
@@ -84,11 +89,14 @@ class DiktatRuleSetProvider(private val diktatConfigFile: String = "diktat-analy
                 ::LambdaParameterOrder,
                 ::FunctionArgumentsSize,
                 ::BlankLinesRule,
+                ::FileSize,
                 ::NullableTypeRule,
-                ::WhiteSpaceRule,
-                ::WhenMustHaveElseRule,
                 ::ImmutableValNoVarRule,
                 ::AvoidNestedFunctionsRule,
+                // formatting: moving blocks, adding line breaks, indentations etc.
+                ::ConsecutiveSpacesRule,
+                ::WhiteSpaceRule,
+                ::HeaderCommentRule,
                 ::FileStructureRule,  // this rule should be right before indentation because it should operate on already valid code
                 ::NewlinesRule,  // newlines need to be inserted right before fixing indentation
                 ::IndentationRule  // indentation rule should be the last because it fixes formatting after all the changes done by previous rules

diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/WhenMustHaveElseRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/WhenMustHaveElseRule.kt
@@ -51,14 +51,13 @@ class WhenMustHaveElseRule(private val configRules: List<RulesConfig>) : Rule("n
         }
     }
 
-
-    //FixMe: If a when statement of type enum or sealed contains all values of a enum - there is no need to have "else" branch.
-
     private fun checkEntries(node: ASTNode) {
         if (!hasElse(node)) {
             Warnings.WHEN_WITHOUT_ELSE.warnAndFix(configRules, emitWarn, isFixMode, "else was not found", node.startOffset, node) {
                 val whenEntryElse = CompositeElement(WHEN_ENTRY)
-                node.appendNewlineMergingWhiteSpace(node.lastChildNode.treePrev, node.lastChildNode.treePrev)
+                if (!node.lastChildNode.isBeginByNewline()) {
+                    node.appendNewlineMergingWhiteSpace(node.lastChildNode.treePrev, node.lastChildNode)
+                }
                 node.addChild(whenEntryElse, node.lastChildNode)
                 addChildren(whenEntryElse)
                 if(!whenEntryElse.isBeginByNewline()) {
@@ -69,7 +68,6 @@ class WhenMustHaveElseRule(private val configRules: List<RulesConfig>) : Rule("n
     }
 
     private fun isStatement(node: ASTNode) : Boolean {
-
         // Checks if there is return before when
         if (node.hasParent(RETURN)) {
             return false
@@ -88,7 +86,11 @@ class WhenMustHaveElseRule(private val configRules: List<RulesConfig>) : Rule("n
         }
     }
 
-    private fun hasElse(node: ASTNode): Boolean = (node.psi as KtWhenExpression).elseExpression != null
+    /**
+     * Check if this `when` has `else` branch. If `else` branch is empty, `(node.psi as KtWhenExpression).elseExpression` returns `null`,
+     * so we need to manually check if any entry contains `else` keyword.
+     */
+    private fun hasElse(node: ASTNode): Boolean = (node.psi as KtWhenExpression).entries.any { it.isElse }
 
     private fun addChildren(node: ASTNode) {
         val block = PsiBlockStatementImpl()
@@ -101,14 +103,12 @@ class WhenMustHaveElseRule(private val configRules: List<RulesConfig>) : Rule("n
             addChild(block, null)
         }
 
-
         block.apply{
             addChild(LeafPsiElement(LBRACE, "{"), null)
             addChild(PsiWhiteSpaceImpl("\n"),null)
             addChild(LeafPsiElement(EOL_COMMENT, "// this is a generated else block"),null)
             addChild(PsiWhiteSpaceImpl("\n"),null)
             addChild(LeafPsiElement(RBRACE, "}"), null)
         }
-
     }
 }
diff --git a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example2Expected.kt b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example2Expected.kt
@@ -14,4 +14,15 @@ import kotlin.system.exitProcess
 @ExperimentalStdlibApi public data class Example(val foo: Int, val bar: Double) : SuperExample("lorem ipsum")
 
 private class TestException : Exception()
+/* this class is unused */
+// private class Test : RuntimeException()
+
+private fun foo(node: ASTNode) {
+    when (node.elementType) {
+        CLASS, FUN, PRIMARY_CONSTRUCTOR, SECONDARY_CONSTRUCTOR -> checkAnnotation(node)
+        else -> {
+            // this is a generated else block
+        }
+    }
+}
 
diff --git a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example2Test.kt b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example2Test.kt
@@ -7,4 +7,12 @@ import org.slf4j.LoggerFactory
 
 data @ExperimentalStdlibApi public class Example(val foo:Int, val bar:Double):SuperExample("lorem ipsum")
 
-private class Test : Exception()
+private class Test : Exception()
+/*    this class is unused */
+//private class Test : RuntimeException()
+
+private fun foo (node: ASTNode) {
+    when (node.elementType) {
+        CLASS, FUN, PRIMARY_CONSTRUCTOR, SECONDARY_CONSTRUCTOR -> checkAnnotation(node)
+    }
+}
diff --git a/...t-framework/src/main/kotlin/org/cqfn/diktat/test/framework/common/LocalCommandExecutor.kt b/...t-framework/src/main/kotlin/org/cqfn/diktat/test/framework/common/LocalCommandExecutor.kt
@@ -1,8 +1,9 @@
 package org.cqfn.diktat.test.framework.common
 
-import java.io.IOException
 import org.slf4j.LoggerFactory
 
+import java.io.IOException
+
 /**
  * Class that wraps shell [command] and can execute it
  */

diff --git a/...st-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestArgumentsReader.kt b/...st-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestArgumentsReader.kt
@@ -1,21 +1,24 @@
 package org.cqfn.diktat.test.framework.config
 
-import java.io.BufferedReader
-import java.io.IOException
-import java.util.stream.Collectors
-import kotlin.system.exitProcess
-import kotlinx.serialization.decodeFromString
-import kotlinx.serialization.json.Json
+import org.cqfn.diktat.common.cli.CliArgument
+import org.cqfn.diktat.common.config.reader.JsonResourceConfigReader
+
 import org.apache.commons.cli.CommandLine
 import org.apache.commons.cli.CommandLineParser
 import org.apache.commons.cli.DefaultParser
 import org.apache.commons.cli.HelpFormatter
 import org.apache.commons.cli.Options
 import org.apache.commons.cli.ParseException
-import org.cqfn.diktat.common.cli.CliArgument
-import org.cqfn.diktat.common.config.reader.JsonResourceConfigReader
 import org.slf4j.LoggerFactory
 
+import java.io.BufferedReader
+import java.io.IOException
+import java.util.stream.Collectors
+
+import kotlin.system.exitProcess
+import kotlinx.serialization.decodeFromString
+import kotlinx.serialization.json.Json
+
 /**
  * Class that gives access to properties of a test
  *

diff --git a/...-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestConfigReader.kt b/...-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestConfigReader.kt
@@ -1,11 +1,13 @@
 package org.cqfn.diktat.test.framework.config
 
+import org.cqfn.diktat.common.config.reader.JsonResourceConfigReader
+
 import java.io.BufferedReader
 import java.io.IOException
 import java.util.stream.Collectors
+
 import kotlinx.serialization.decodeFromString
 import kotlinx.serialization.json.Json
-import org.cqfn.diktat.common.config.reader.JsonResourceConfigReader
 
 /**
  * A [JsonResourceConfigReader] to read tests configuration as [TestConfig]

diff --git a/...est-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/FileComparator.kt b/...est-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/FileComparator.kt
@@ -1,14 +1,15 @@
 package org.cqfn.diktat.test.framework.processing
 
 import com.github.difflib.DiffUtils
+import org.slf4j.LoggerFactory
+
 import java.io.File
 import java.io.IOException
 import java.nio.file.Files
 import java.nio.file.Paths
 import java.util.ArrayList
 import java.util.StringJoiner
 import java.util.stream.Collectors
-import org.slf4j.LoggerFactory
 
 /**
  * A class that is capable of comparing files content

diff --git a/...framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestComparatorUnit.kt b/...framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestComparatorUnit.kt
@@ -1,14 +1,15 @@
 package org.cqfn.diktat.test.framework.processing
 
+import org.apache.commons.io.FileUtils
+import org.slf4j.Logger
+import org.slf4j.LoggerFactory
+
 import java.io.File
 import java.io.IOException
 import java.nio.file.Files
 import java.nio.file.Paths
 import java.util.ArrayList
 import java.util.stream.Collectors
-import org.apache.commons.io.FileUtils
-import org.slf4j.Logger
-import org.slf4j.LoggerFactory
 
 /**
  * Class that can apply transformation to an input file and then compare with expected result and output difference.

diff --git a/...t-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestCompare.kt b/...t-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestCompare.kt
@@ -1,14 +1,16 @@
 package org.cqfn.diktat.test.framework.processing
 
-import java.io.File
-import org.apache.commons.io.FileUtils
 import org.cqfn.diktat.test.framework.common.ExecutionResult
 import org.cqfn.diktat.test.framework.common.TestBase
 import org.cqfn.diktat.test.framework.config.TestConfig
 import org.cqfn.diktat.test.framework.config.TestFrameworkProperties
+
+import org.apache.commons.io.FileUtils
 import org.slf4j.Logger
 import org.slf4j.LoggerFactory
 
+import java.io.File
+
 /**
  * A class that runs tests and compares output with expected result
  */