Repository: analysis-dev/diktat (kotlin)

Issue:
FileStructureRule follow-ups
There are some issues stated in comments in code.
* handle other elements that could be present before package (other comments)
* ~handle cases when PACKAGE_DIRECTIVE node is not present~ already supports nullable package directive for default package
* ~find cases when node.psi.importLists.size > 1~ - shouldn't happen in valid kotlin code, handle cases when it's not present at all

Pull request:
### What's done:
* Fixed logic
* Added tests

This pull request closes #267

Patch:
diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/comments/HeaderCommentRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/comments/HeaderCommentRule.kt
index 9f9a96a39f..5fabfe31e4 100644
--- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/comments/HeaderCommentRule.kt
+++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/comments/HeaderCommentRule.kt
@@ -10,6 +10,7 @@ import org.cqfn.diktat.ruleset.constants.Warnings.HEADER_MISSING_OR_WRONG_COPYRI
 import org.cqfn.diktat.ruleset.constants.Warnings.HEADER_NOT_BEFORE_PACKAGE
 import org.cqfn.diktat.ruleset.constants.Warnings.HEADER_WRONG_FORMAT
 import org.cqfn.diktat.ruleset.constants.Warnings.WRONG_COPYRIGHT_YEAR
+import org.cqfn.diktat.ruleset.utils.copyrightWords
 import org.cqfn.diktat.ruleset.utils.findChildAfter
 import org.cqfn.diktat.ruleset.utils.findChildBefore
 import org.cqfn.diktat.ruleset.utils.getAllChildrenWithType
@@ -40,7 +41,6 @@ import java.time.LocalDate
  */
 @Suppress("ForbiddenComment")
 class HeaderCommentRule(private val configRules: List<RulesConfig>) : Rule("header-comment") {
-    private val copyrightWords = setOf("copyright", "版权")
     private var isFixMode: Boolean = false
     private lateinit var emitWarn: EmitType
 
diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/FileStructureRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/FileStructureRule.kt
index a1c092e995..888eb7a7b0 100644
--- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/FileStructureRule.kt
+++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/FileStructureRule.kt
@@ -12,6 +12,7 @@ import org.cqfn.diktat.ruleset.constants.Warnings.FILE_UNORDERED_IMPORTS
 import org.cqfn.diktat.ruleset.constants.Warnings.FILE_WILDCARD_IMPORTS
 import org.cqfn.diktat.ruleset.rules.PackageNaming.Companion.PACKAGE_SEPARATOR
 import org.cqfn.diktat.ruleset.utils.StandardPlatforms
+import org.cqfn.diktat.ruleset.utils.copyrightWords
 import org.cqfn.diktat.ruleset.utils.handleIncorrectOrder
 import org.cqfn.diktat.ruleset.utils.moveChildBefore
 
@@ -37,6 +38,7 @@ import org.jetbrains.kotlin.com.intellij.psi.tree.TokenSet
 import org.jetbrains.kotlin.name.Name
 import org.jetbrains.kotlin.psi.KtFile
 import org.jetbrains.kotlin.psi.KtImportDirective
+import org.jetbrains.kotlin.psi.psiUtil.siblings
 
 /**
  * Visitor for checking internal file structure.
@@ -100,17 +102,17 @@ class FileStructureRule(private val configRules: List<RulesConfig>) : Rule("file
         return hasCode
     }
 
-    @Suppress("TOO_LONG_FUNCTION")
+    @Suppress("ComplexMethod", "TOO_LONG_FUNCTION")
     private fun checkCodeBlocksOrderAndEmptyLines(node: ASTNode) {
-        // PACKAGE_DIRECTIVE node is always present in regular kt files and might be absent in kts.
+        // From KtFile.kt: 'scripts have no package directive, all other files must have package directives'.
         // Kotlin compiler itself enforces it's position in the file if it is present.
         // If package directive is missing in .kt file (default package), the node is still present in the AST.
-        // fixme: find and handle cases when this node is not present (.kts files?)
         val packageDirectiveNode = (node.psi as KtFile)
             .packageDirective
             ?.takeUnless { it.isRoot }
             ?.node
-        // fixme: find cases when node.psi.importLists.size > 1, handle cases when it's not present (null)
+        // There is a private property node.psi.importLists, but it's size can't be > 1 in valid kotlin code. It exists to help in situations
+        // when, e.g. merge conflict marker breaks the imports list. We shouldn't handle this situation here.
         val importsList = (node.psi as KtFile)
             .importList
             ?.takeIf { it.imports.isNotEmpty() }
@@ -123,19 +125,34 @@ class FileStructureRule(private val configRules: List<RulesConfig>) : Rule("file
                 // taking nodes with actual code
                 !it.isWhiteSpace() && !it.isPartOfComment() &&
                         // but not the ones we are going to move
-                        it.elementType != FILE_ANNOTATION_LIST && it.elementType != IMPORT_LIST &&
-                        // if we are here, then package is default and we don't need to select the empty PACKAGE_DIRECTIVE node
+                        it.elementType != FILE_ANNOTATION_LIST &&
+                        // if we are here, then IMPORT_LIST either is not present in the AST, or is empty. Either way, we don't need to select it.
+                        it.elementType != IMPORT_LIST &&
+                        // if we are here, then package is default and we don't need to select the empty PACKAGE_DIRECTIVE node.
                         it.elementType != PACKAGE_DIRECTIVE
             }
             ?: return  // at this point it means the file contains only comments
-        // fixme: handle other elements that could be present before package (other comments)
+        // We consider the first block comment of the file to be the one that possibly contains copyright information.
         var copyrightComment = firstCodeNode.prevSibling { it.elementType == BLOCK_COMMENT }
+            ?.takeIf { blockCommentNode ->
+                copyrightWords.any { blockCommentNode.text.contains(it, ignoreCase = true) }
+            }
         var headerKdoc = firstCodeNode.prevSibling { it.elementType == KDOC }
+        // Annotations with target`file` can only be placed before `package` directive.
         var fileAnnotations = node.findChildByType(FILE_ANNOTATION_LIST)
+        // We also collect all other elements that are placed on top of the file.
+        // These may be other comments, so we just place them before the code starts.
+        val otherNodesBeforeCode = firstCodeNode.siblings(forward = false)
+            .filterNot {
+                it.isWhiteSpace() ||
+                        it == copyrightComment || it == headerKdoc || it == fileAnnotations
+            }
+            .toList()
+            .reversed()
 
         // checking order
-        listOfNotNull(copyrightComment, headerKdoc, fileAnnotations).handleIncorrectOrder({
-            getSiblingBlocks(copyrightComment, headerKdoc, fileAnnotations, firstCodeNode)
+        listOfNotNull(copyrightComment, headerKdoc, fileAnnotations, *otherNodesBeforeCode.toTypedArray()).handleIncorrectOrder({
+            getSiblingBlocks(copyrightComment, headerKdoc, fileAnnotations, firstCodeNode, otherNodesBeforeCode)
         }) { astNode, beforeThisNode ->
             FILE_INCORRECT_BLOCKS_ORDER.warnAndFix(configRules, emitWarn, isFixMode, astNode.text.lines().first(), astNode.startOffset, astNode) {
                 val result = node.moveChildBefore(astNode, beforeThisNode, true)
@@ -237,12 +254,13 @@ class FileStructureRule(private val configRules: List<RulesConfig>) : Rule("file
         copyrightComment: ASTNode?,
         headerKdoc: ASTNode?,
         fileAnnotations: ASTNode?,
-        packageDirectiveNode: ASTNode
-    ): Pair<ASTNode?, ASTNode> = when (elementType) {
-        BLOCK_COMMENT -> null to listOfNotNull(headerKdoc, fileAnnotations, packageDirectiveNode).first()
-        KDOC -> copyrightComment to (fileAnnotations ?: packageDirectiveNode)
-        FILE_ANNOTATION_LIST -> (headerKdoc ?: copyrightComment) to packageDirectiveNode
-        else -> error("Only BLOCK_COMMENT, KDOC and FILE_ANNOTATION_LIST are valid inputs.")
+        firstCodeNode: ASTNode,
+        otherNodesBeforeFirst: List<ASTNode>
+    ): Pair<ASTNode?, ASTNode> = when (this) {
+        copyrightComment -> null to listOfNotNull(headerKdoc, fileAnnotations, otherNodesBeforeFirst.firstOrNull(), firstCodeNode).first()
+        headerKdoc -> copyrightComment to (fileAnnotations ?: otherNodesBeforeFirst.firstOrNull() ?: firstCodeNode)
+        fileAnnotations -> (headerKdoc ?: copyrightComment) to (otherNodesBeforeFirst.firstOrNull() ?: firstCodeNode)
+        else -> (headerKdoc ?: copyrightComment) to firstCodeNode
     }
 
     @Suppress("TYPE_ALIAS")
diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstConstants.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstConstants.kt
index 6faefd97e3..17601a595b 100644
--- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstConstants.kt
+++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstConstants.kt
@@ -24,6 +24,7 @@ internal const val SET_PREFIX = "set"
 val emptyBlockList = listOf(LBRACE, WHITE_SPACE, SEMICOLON, RBRACE)
 
 val commentType = listOf(BLOCK_COMMENT, EOL_COMMENT, KDOC)
+val copyrightWords = setOf("copyright", "版权")
 
 internal const val EMPTY_BLOCK_TEXT = "{}"
 
diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt
index 999632ddd4..eae3ab814b 100644
--- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt
+++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt
@@ -1,8 +1,8 @@
 /**
  * Various utility methods to work with kotlin AST
+ * FixMe: fix suppressed inspections on KDocs
  */
 
-// todo fix inspections on KDocs
 @file:Suppress("FILE_NAME_MATCH_CLASS", "KDOC_WITHOUT_RETURN_TAG", "KDOC_WITHOUT_PARAM_TAG")
 
 package org.cqfn.diktat.ruleset.utils


Test patch:
diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/FileStructureRuleFixTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/FileStructureRuleFixTest.kt
index f2f544ebb2..59e4074658 100644
--- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/FileStructureRuleFixTest.kt
+++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/FileStructureRuleFixTest.kt
@@ -78,4 +78,10 @@ class FileStructureRuleFixTest : FixTestBase("test/paragraph3/file_structure", :
     fun `should still work with default package and no imports`() {
         fixAndCompare("NoImportNoPackageExpected.kt", "NoImportNoPackageTest.kt")
     }
+
+    @Test
+    @Tag(WarningNames.FILE_UNORDERED_IMPORTS)
+    fun `should move other comments before package node`() {
+        fixAndCompare("OtherCommentsExpected.kt", "OtherCommentsTest.kt")
+    }
 }
diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/FileStructureRuleTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/FileStructureRuleTest.kt
index c1fe23eecc..480c269d8e 100644
--- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/FileStructureRuleTest.kt
+++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/FileStructureRuleTest.kt
@@ -146,4 +146,84 @@ class FileStructureRuleTest : LintTestBase(::FileStructureRule) {
             """.trimMargin(), rulesConfigList = rulesConfigListWildCardImports
         )
     }
+
+    @Test
+    @Tag(WarningNames.FILE_INCORRECT_BLOCKS_ORDER)
+    fun `should warn if there are other misplaced comments before package - positive example`() {
+        lintMethod(
+            """
+                |/**
+                | * This is an example
+                | */
+                |
+                |// some notes on this file
+                |package org.cqfn.diktat.example
+                |
+                |import org.cqfn.diktat.example.Foo
+                |
+                |class Example
+            """.trimMargin()
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.FILE_INCORRECT_BLOCKS_ORDER)
+    fun `should warn if there are other misplaced comments before package`() {
+        lintMethod(
+            """
+                |// some notes on this file
+                |/**
+                | * This is an example
+                | */
+                |
+                |package org.cqfn.diktat.example
+                |
+                |import org.cqfn.diktat.example.Foo
+                |
+                |class Example
+            """.trimMargin(),
+            LintError(1, 1, ruleId, "${FILE_INCORRECT_BLOCKS_ORDER.warnText()} // some notes on this file", true),
+            LintError(2, 1, ruleId, "${FILE_INCORRECT_BLOCKS_ORDER.warnText()} /**", true),
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.FILE_INCORRECT_BLOCKS_ORDER)
+    fun `block comment should be detected as copyright - positive example`() {
+        lintMethod(
+            """
+                |/*
+                | * Copyright Example Inc. (c)
+                | */
+                |
+                |@file:Annotation
+                |
+                |package org.cqfn.diktat.example
+                |
+                |import org.cqfn.diktat.example.Foo
+                |
+                |class Example
+            """.trimMargin()
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.FILE_INCORRECT_BLOCKS_ORDER)
+    fun `block comment shouldn't be detected as copyright without keywords`() {
+        lintMethod(
+            """
+                |/*
+                | * Just a regular block comment
+                | */
+                |@file:Annotation
+                |
+                |package org.cqfn.diktat.example
+                |
+                |import org.cqfn.diktat.example.Foo
+                |
+                |class Example
+            """.trimMargin(),
+            LintError(4, 1, ruleId, "${FILE_INCORRECT_BLOCKS_ORDER.warnText()} @file:Annotation", true)
+        )
+    }
 }
diff --git a/diktat-rules/src/test/resources/test/paragraph3/file_structure/OtherCommentsExpected.kt b/diktat-rules/src/test/resources/test/paragraph3/file_structure/OtherCommentsExpected.kt
new file mode 100644
index 0000000000..7573a053d1
--- /dev/null
+++ b/diktat-rules/src/test/resources/test/paragraph3/file_structure/OtherCommentsExpected.kt
@@ -0,0 +1,11 @@
+/**
+ * This is an example
+ */
+
+// some notes on this file
+// and some more
+package org.cqfn.diktat.example
+
+import org.cqfn.diktat.example.Foo
+
+class Example
diff --git a/diktat-rules/src/test/resources/test/paragraph3/file_structure/OtherCommentsTest.kt b/diktat-rules/src/test/resources/test/paragraph3/file_structure/OtherCommentsTest.kt
new file mode 100644
index 0000000000..fca05021a6
--- /dev/null
+++ b/diktat-rules/src/test/resources/test/paragraph3/file_structure/OtherCommentsTest.kt
@@ -0,0 +1,11 @@
+// some notes on this file
+// and some more
+/**
+ * This is an example
+ */
+
+package org.cqfn.diktat.example
+
+import org.cqfn.diktat.example.Foo
+
+class Example