Fix bug with incorrect package name fixing #877
Conversation
### What's done: * Fix bug with incorrect package name fixing when there is exist file annotation * Add tests
Codecov Report
@@ Coverage Diff @@
## master #877 +/- ##
=========================================
Coverage 83.52% 83.53%
- Complexity 2211 2215 +4
=========================================
Files 101 101
Lines 5681 5690 +9
Branches 1624 1628 +4
=========================================
+ Hits 4745 4753 +8
Misses 256 256
- Partials 680 681 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter1/PackageNaming.kt
Outdated
Show resolved
Hide resolved
diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter1/PackageNamingWarnTest.kt
Show resolved
Hide resolved
### What's done: * Add recommendations according review
| val insertPackageDirectiveBefore = if (newPackageDirective != newPackageDirective.treeParent.firstChildNode) { | ||
| newPackageDirective | ||
| } else { | ||
| newPackageDirective.treeNext |
There was a problem hiding this comment.
What if there will be EOL_COMMENT or another type of comment? I suppose, comment will be in wrong line.
There was a problem hiding this comment.
very good note, Denis, thanks
| newPackageDirective.treeParent.addChild(PsiWhiteSpaceImpl("\n\n"), newPackageDirective.treeNext) | ||
| val packageDirectiveParent = packageDirectiveNode.treeParent | ||
| packageDirectiveParent.replaceChild(packageDirectiveNode, newPackageDirective) | ||
| val insertWhitespaceBefore = if (newPackageDirective != packageDirectiveParent.firstChildNode) { |
There was a problem hiding this comment.
Maybe, check whether there is a white space node before and after package directive node? Actually, these \n\n are not responsibility of this rule, so here we should ensure that the tree is correct, and FileStructureRule will do the rest.
There was a problem hiding this comment.
But shouldn't each rule work independently? What if user disables FileStructureRule ?
There was a problem hiding this comment.
Yes, rules should work independently. My main suggestion was to check, whether there are white spaces both before and after the package directive node; I think we can make two newlines on both sides.
### What's done: * Improve algorithm of insertNewPackageName func, add more tests * Discovered bug in FileStructure rule
kgevorkyan
force-pushed
the
bugfix/incorrect_package_name_fix_with_annotations
branch
from
914c59d to
9331884
Compare
| 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().filter { it.elementType !in possibleTypesBeforePackageDirective }.first() |
There was a problem hiding this comment.
| val addBefore = packageDirectiveParent.children().filter { it.elementType !in possibleTypesBeforePackageDirective }.first() | |
| val addBefore = packageDirectiveParent.children().first { it.elementType !in possibleTypesBeforePackageDirective } |
| @@ -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: | |||
| @Disabled | |||
There was a problem hiding this comment.
Please add reason (@Disabled("reason")
### What's done: Fix bug in smoke test
### What's done: * Add recommendations
kgevorkyan
deleted the
bugfix/incorrect_package_name_fix_with_annotations
branch
What's done:
Closes #875