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) : 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) : 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) : 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) : Rule("file copyrightComment: ASTNode?, headerKdoc: ASTNode?, fileAnnotations: ASTNode?, - packageDirectiveNode: ASTNode - ): Pair = 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 + ): Pair = 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 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