Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug with incorrect package name fixing #877

Merged
merged 6 commits into from
May 12, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ Main features of diktat are the following:
# another option is "brew install ktlint"
```

2. Load diKTat manually: [here](https://github.com/cqfn/diKTat/releases/download/v0.5.2/diktat.jar)
2. Load diKTat manually: [here](https://github.com/cqfn/diKTat/releases/download/v0.5.2/diktat-0.5.2.jar)

**OR** use curl:
```bash
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,16 @@ import org.cqfn.diktat.ruleset.constants.Warnings.PACKAGE_NAME_MISSING
import org.cqfn.diktat.ruleset.rules.DiktatRule
import org.cqfn.diktat.ruleset.utils.*

import com.pinterest.ktlint.core.ast.ElementType.BLOCK_COMMENT
import com.pinterest.ktlint.core.ast.ElementType.DOT_QUALIFIED_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.EOL_COMMENT
import com.pinterest.ktlint.core.ast.ElementType.FILE_ANNOTATION_LIST
import com.pinterest.ktlint.core.ast.ElementType.IDENTIFIER
import com.pinterest.ktlint.core.ast.ElementType.KDOC
import com.pinterest.ktlint.core.ast.ElementType.PACKAGE_DIRECTIVE
import com.pinterest.ktlint.core.ast.ElementType.REFERENCE_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.WHITE_SPACE
import com.pinterest.ktlint.core.ast.children
import com.pinterest.ktlint.core.ast.isLeaf
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement
Expand Down Expand Up @@ -218,8 +224,27 @@ class PackageNaming(configRules: List<RulesConfig>) : DiktatRule(
?: run {
// there is missing package statement in a file, so it will be created and inserted
val newPackageDirective = generatedPackageDirective.findLeafWithSpecificType(PACKAGE_DIRECTIVE)!!
packageDirectiveNode.treeParent.replaceChild(packageDirectiveNode, newPackageDirective)
newPackageDirective.treeParent.addChild(PsiWhiteSpaceImpl("\n\n"), newPackageDirective.treeNext)
val packageDirectiveParent = packageDirectiveNode.treeParent
// When package directive is missing in .kt file,
// the node is still present in the AST, and not always in a convenient place.
// E.g. `@file:Suppress("...") // comments`
// AST will be: FILE_ANNOTATION_LIST, PACKAGE_DIRECTIVE, WHITE_SPACE, EOL_COMMENT
// So, we can't just put new package directive in it's old place and rely on FileStructure rule
if (packageDirectiveNode != packageDirectiveParent.firstChildNode) {
// We will insert new package directive node before first node, which is not in the following list
val possibleTypesBeforePackageDirective = listOf(WHITE_SPACE, EOL_COMMENT, BLOCK_COMMENT, KDOC, PACKAGE_DIRECTIVE, FILE_ANNOTATION_LIST)
val addBefore = packageDirectiveParent.children().first { it.elementType !in possibleTypesBeforePackageDirective }
packageDirectiveParent.removeChild(packageDirectiveNode)
packageDirectiveParent.addChild(newPackageDirective, addBefore)
if (newPackageDirective.treePrev.elementType != WHITE_SPACE) {
packageDirectiveParent.addChild(PsiWhiteSpaceImpl("\n"), newPackageDirective)
}
} else {
packageDirectiveParent.replaceChild(packageDirectiveNode, newPackageDirective)
}
if (newPackageDirective.treeNext.elementType != WHITE_SPACE) {
packageDirectiveParent.addChild(PsiWhiteSpaceImpl("\n"), newPackageDirective.treeNext)
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ class FileStructureRule(configRules: List<RulesConfig>) : DiktatRule(
if (elementType == WHITE_SPACE && text.count { it == '\n' } != 2) {
FILE_NO_BLANK_LINE_BETWEEN_BLOCKS.warnAndFix(configRules, emitWarn, isFixMode, astNode.text.lines().first(),
astNode.startOffset, astNode) {
(this as LeafPsiElement).replaceWithText("\n\n${text.replace("\n", "")}")
(this as LeafPsiElement).parent.node.replaceChild(this, PsiWhiteSpaceImpl("\n\n${text.replace("\n", "")}"))
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,26 @@ class PackageNamingWarnTest : LintTestBase(::PackageNaming) {
)
}

@Test
@Tag(WarningNames.PACKAGE_NAME_MISSING)
fun `missing package name with annotation (check)`() {
kgevorkyan marked this conversation as resolved.
Show resolved Hide resolved
lintMethod(
"""
@file:Suppress("CONSTANT_UPPERCASE")

import org.cqfn.diktat.a.b.c

/**
* testComment
*/
class TestPackageName { }

""".trimIndent(),
LintError(1, 37, ruleId, "${PACKAGE_NAME_MISSING.warnText()} $TEST_FILE_NAME", true),
rulesConfigList = rulesConfigList
)
}

@Test
@Tag(WarningNames.PACKAGE_NAME_INCORRECT_CASE)
fun `package name should be in a lower case (check)`() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,32 @@ class PackagePathFixTest : FixTestBase(
fixAndCompare("some/FixIncorrectExpected.kt", "some/FixIncorrectTest.kt")
}

@Test
@Tag(WarningNames.PACKAGE_NAME_MISSING)
fun `fix missing package name with file annotation`() {
fixAndCompare("org/cqfn/diktat/some/name/FixMissingWithAnnotationExpected.kt", "org/cqfn/diktat/some/name/FixMissingWithAnnotationTest.kt")
}

@Test
@Tag(WarningNames.PACKAGE_NAME_MISSING)
fun `fix missing package name with file annotation and comments`() {
fixAndCompare("org/cqfn/diktat/some/name/FixMissingWithAnnotationExpected2.kt", "org/cqfn/diktat/some/name/FixMissingWithAnnotationTest2.kt")
}

@Test
@Tag(WarningNames.PACKAGE_NAME_MISSING)
fun `fix missing package name with file annotation and comments 2`() {
fixAndCompare("org/cqfn/diktat/some/name/FixMissingWithAnnotationExpected3.kt", "org/cqfn/diktat/some/name/FixMissingWithAnnotationTest3.kt")
}

// If there is no import list in code, the node is still present in the AST, but without any whitespaces around
// So, this check covered case, when we manually add whitespace before package directive
@Test
@Tag(WarningNames.PACKAGE_NAME_MISSING)
fun `fix missing package name without import list`() {
fixAndCompare("org/cqfn/diktat/some/name/FixMissingWithoutImportExpected.kt", "org/cqfn/diktat/some/name/FixMissingWithoutImportTest.kt")
}

@Test
@Tag(WarningNames.PACKAGE_NAME_MISSING)
fun `fix missing package name with a proper location without domain`() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import org.cqfn.diktat.ruleset.rules.chapter3.files.TopLevelOrderRule
import org.cqfn.diktat.util.FixTestBase

import generated.WarningNames
import org.junit.jupiter.api.Disabled
import org.junit.jupiter.api.Tag
import org.junit.jupiter.api.Test

Expand All @@ -19,4 +20,12 @@ class TopLevelOrderRuleFixTest : FixTestBase("test/paragraph3/top_level", ::TopL
fun `should fix top level order with comment`() {
fixAndCompare("TopLevelWithCommentExpected.kt", "TopLevelWithCommentTest.kt")
}

// FixMe: should be considered this case (swapped order of kdoc and package directive)
@Disabled("Isn't working yet")
@Test
@Tag(WarningNames.TOP_LEVEL_ORDER)
fun `should fix top level order with header kdoc`() {
fixAndCompare("TopLevelWithHeaderKdocExpected.kt", "TopLevelWithHeaderKdocTest.kt")
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
package org.cqfn.diktat.some.name

import org.cqfn.diktat.ktlint.core.Rule

class TestPackageName {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
@file:Suppress("CONSTANT_UPPERCASE")

package org.cqfn.diktat.some.name
import org.cqfn.diktat.ktlint.core.Rule

class TestPackageName {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
@file:Suppress("CONSTANT_UPPERCASE") // comment
package org.cqfn.diktat.some.name
import org.cqfn.diktat.ktlint.core.Rule

class TestPackageName {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/**
* comment
*/
@file:Suppress("CONSTANT_UPPERCASE")
package org.cqfn.diktat.some.name
import org.cqfn.diktat.ktlint.core.Rule

class TestPackageName {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
@file:Suppress("CONSTANT_UPPERCASE")

import org.cqfn.diktat.ktlint.core.Rule

class TestPackageName {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
@file:Suppress("CONSTANT_UPPERCASE") // comment
import org.cqfn.diktat.ktlint.core.Rule

class TestPackageName {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/**
* comment
*/
@file:Suppress("CONSTANT_UPPERCASE")
import org.cqfn.diktat.ktlint.core.Rule

class TestPackageName {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
@file:Suppress("CONSTANT_UPPERCASE")
package org.cqfn.diktat.some.name

val a = 5

class TestPackageName {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
@file:Suppress("CONSTANT_UPPERCASE")
val a = 5

class TestPackageName {
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
package org.cqfn.diktat.some

import org.cqfn.diktat.ktlint.core.Rule

class TestPackageName {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/**
* Header Kdoc
*/

package test.paragraph3.top_level

import org.cqfn.diktat.bar

class A {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package test.paragraph3.top_level

/**
* Header Kdoc
*/

import org.cqfn.diktat.bar

class A {}