PackageDirectoryMismatchInspection should report when a package statement is missing

#KT-13549 Fixed
2019-04-24 15:46:54 +07:00
parent 2ae30de980
commit 0198df4935
18 changed files with 136 additions and 40 deletions
@@ -6,10 +6,14 @@
 package org.jetbrains.kotlin.idea.refactoring.move.changePackage

 import com.intellij.CommonBundle
-import com.intellij.codeInspection.*
+import com.intellij.codeInspection.LocalQuickFix
+import com.intellij.codeInspection.ProblemDescriptor
+import com.intellij.codeInspection.ProblemHighlightType
+import com.intellij.codeInspection.ProblemsHolder
 import com.intellij.openapi.project.Project
 import com.intellij.openapi.roots.JavaProjectRootsUtil
 import com.intellij.openapi.ui.Messages
+import com.intellij.openapi.util.TextRange
 import com.intellij.psi.PsiManager
 import com.intellij.refactoring.PackageWrapper
 import com.intellij.refactoring.move.moveClassesOrPackages.AutocreatingSingleSourceRootMoveDestination
@@ -24,44 +28,38 @@ import org.jetbrains.kotlin.idea.refactoring.hasIdentifiersOnly
 import org.jetbrains.kotlin.idea.refactoring.isInjectedFragment
 import org.jetbrains.kotlin.idea.util.application.runWriteAction
 import org.jetbrains.kotlin.name.FqName
+import org.jetbrains.kotlin.psi.KtFile
 import org.jetbrains.kotlin.psi.KtPackageDirective
-import org.jetbrains.kotlin.psi.KtVisitorVoid
+import org.jetbrains.kotlin.psi.packageDirectiveVisitor

 class PackageDirectoryMismatchInspection : AbstractKotlinInspection() {
+    override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean) = packageDirectiveVisitor(fun(directive: KtPackageDirective) {
+        val file = directive.containingKtFile
+        if (file.isInjectedFragment || file.packageMatchesDirectoryOrImplicit()) return

-    override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession) =
-        object : KtVisitorVoid() {
-            override fun visitPackageDirective(directive: KtPackageDirective) {
-                super.visitPackageDirective(directive)
-                if (directive.text.isEmpty()) return
-
-                val file = directive.containingKtFile
-                if (file.isInjectedFragment || file.packageMatchesDirectoryOrImplicit()) return
-
-                val fixes = mutableListOf<LocalQuickFix>()
-                val qualifiedName = directive.qualifiedName
-                val dirName = if (qualifiedName.isEmpty()) "source root" else "'${qualifiedName.replace('.', '/')}'"
-                fixes += MoveFileToPackageFix(dirName)
-                val fqNameByDirectory = file.getFqNameByDirectory()
-                when {
-                    fqNameByDirectory.isRoot ->
-                        fixes += ChangePackageFix("source root", fqNameByDirectory)
-                    fqNameByDirectory.hasIdentifiersOnly() ->
-                        fixes += ChangePackageFix("'${fqNameByDirectory.asString()}'", fqNameByDirectory)
-                }
-                val fqNameWithImplicitPrefix = file.parent?.getFqNameWithImplicitPrefix()
-                if (fqNameWithImplicitPrefix != null && fqNameWithImplicitPrefix != fqNameByDirectory) {
-                    fixes += ChangePackageFix("'${fqNameWithImplicitPrefix.asString()}'", fqNameWithImplicitPrefix)
-                }
-
-                holder.registerProblem(
-                    directive,
-                    "Package directive doesn't match file location",
-                    ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
-                    *fixes.toTypedArray()
-                )
-            }
+        val fixes = mutableListOf<LocalQuickFix>()
+        val qualifiedName = directive.qualifiedName
+        val dirName = if (qualifiedName.isEmpty()) "source root" else "'${qualifiedName.replace('.', '/')}'"
+        fixes += MoveFileToPackageFix(dirName)
+        val fqNameByDirectory = file.getFqNameByDirectory()
+        when {
+            fqNameByDirectory.isRoot ->
+                fixes += ChangePackageFix("source root", fqNameByDirectory)
+            fqNameByDirectory.hasIdentifiersOnly() ->
+                fixes += ChangePackageFix("'${fqNameByDirectory.asString()}'", fqNameByDirectory)
        }
+        val fqNameWithImplicitPrefix = file.parent?.getFqNameWithImplicitPrefix()
+        if (fqNameWithImplicitPrefix != null && fqNameWithImplicitPrefix != fqNameByDirectory) {
+            fixes += ChangePackageFix("'${fqNameWithImplicitPrefix.asString()}'", fqNameWithImplicitPrefix)
+        }
+
+        holder.registerProblem(
+            file,
+            directive.textRange,
+            "Package directive doesn't match file location",
+            *fixes.toTypedArray()
+        )
+    })

    private class MoveFileToPackageFix(val dirName: String) : LocalQuickFix {
        override fun getFamilyName() = "Move file to package-matching directory"
@@ -71,16 +69,16 @@ class PackageDirectoryMismatchInspection : AbstractKotlinInspection() {
        override fun startInWriteAction() = false

        override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
-            val directive = descriptor.psiElement as? KtPackageDirective ?: return
-            val file = directive.containingKtFile
+            val file = descriptor.psiElement as? KtFile ?: return
+            val directive = file.packageDirective ?: return

            val sourceRoots = JavaProjectRootsUtil.getSuitableDestinationSourceRoots(project)
            val packageWrapper = PackageWrapper(PsiManager.getInstance(project), directive.qualifiedName)
            val fileToMove = directive.containingFile
            val chosenRoot =
                sourceRoots.singleOrNull()
-                        ?: MoveClassesOrPackagesUtil.chooseSourceRoot(packageWrapper, sourceRoots, fileToMove.containingDirectory)
-                        ?: return
+                    ?: MoveClassesOrPackagesUtil.chooseSourceRoot(packageWrapper, sourceRoots, fileToMove.containingDirectory)
+                    ?: return
            val targetDirFactory = AutocreatingSingleSourceRootMoveDestination(packageWrapper, chosenRoot)
            targetDirFactory.verify(fileToMove)?.let {
                Messages.showMessageDialog(project, it, CommonBundle.getErrorTitle(), Messages.getErrorIcon())
@@ -107,8 +105,7 @@ class PackageDirectoryMismatchInspection : AbstractKotlinInspection() {
        override fun getName() = "Change file's package to $packageName"

        override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
-            val directive = descriptor.psiElement as? KtPackageDirective ?: return
-            val file = directive.containingKtFile
+            val file = descriptor.psiElement as? KtFile ?: return
            KotlinChangePackageRefactoring(file).run(packageFqName)
        }
    }
@@ -0,0 +1,4 @@
+
+class Foo {
+
+}
@@ -0,0 +1,10 @@
+<problems>
+    <problem>
+        <file>rootPackageMatched.kt</file>
+        <line>1</line>
+        <module>testMismatchedProjectAndDirectoryRoot_MismatchedProjectAndDirectoryRoot</module>
+        <entry_point TYPE="file" FQNAME="rootPackageMatched.kt" />
+        <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Package name does not match containing directory</problem_class>
+        <description>Package directive doesn't match file location</description>
+    </problem>
+</problems>
@@ -0,0 +1,3 @@
+{
+    "inspectionClass": "org.jetbrains.kotlin.idea.refactoring.move.changePackage.PackageDirectoryMismatchInspection"
+}
@@ -0,0 +1,6 @@
+
+class Foo
+
+fun foo() {
+
+}
@@ -0,0 +1,6 @@
+<caret>
+class Foo
+
+fun foo() {
+
+}
@@ -0,0 +1,5 @@
+{
+  "mainFile": "source/test.kt",
+  "inspectionClass": "org.jetbrains.kotlin.idea.refactoring.move.changePackage.PackageDirectoryMismatchInspection",
+  "fix": "Move file to source root"
+}
@@ -0,0 +1,7 @@
+package target
+
+class Foo
+
+fun foo() {
+
+}
@@ -0,0 +1,6 @@
+package target
+
+fun test() {
+    Foo()
+    foo()
+}
@@ -0,0 +1,9 @@
+package usages
+
+import target.foo
+import target.Foo
+
+fun test() {
+    Foo()
+    foo()
+}
@@ -0,0 +1,5 @@
+<caret>class Foo
+
+fun foo() {
+
+}
@@ -0,0 +1,9 @@
+package target
+
+import foo
+import Foo
+
+fun test() {
+    Foo()
+    foo()
+}
@@ -0,0 +1,9 @@
+package usages
+
+import foo
+import Foo
+
+fun test() {
+    Foo()
+    foo()
+}
@@ -0,0 +1,5 @@
+{
+  "mainFile": "target/test.kt",
+  "inspectionClass": "org.jetbrains.kotlin.idea.refactoring.move.changePackage.PackageDirectoryMismatchInspection",
+  "fix": "Change file's package to 'target'"
+}
@@ -54,6 +54,11 @@ public class MultiFileInspectionTestGenerated extends AbstractMultiFileInspectio
        runTest("idea/testData/multiFileInspections/mismatchedProjectAndDirectory/mismatchedProjectAndDirectory.test");
    }

+    @TestMetadata("mismatchedProjectAndDirectoryRoot/mismatchedProjectAndDirectoryRoot.test")
+    public void testMismatchedProjectAndDirectoryRoot_MismatchedProjectAndDirectoryRoot() throws Exception {
+        runTest("idea/testData/multiFileInspections/mismatchedProjectAndDirectoryRoot/mismatchedProjectAndDirectoryRoot.test");
+    }
+
    @TestMetadata("platformExtensionReceiverOfInline/platformExtensionReceiverOfInline.test")
    public void testPlatformExtensionReceiverOfInline_PlatformExtensionReceiverOfInline() throws Exception {
        runTest("idea/testData/multiFileInspections/platformExtensionReceiverOfInline/platformExtensionReceiverOfInline.test");
@@ -44,6 +44,11 @@ public class MultiFileLocalInspectionTestGenerated extends AbstractMultiFileLoca
        runTest("idea/testData/multiFileLocalInspections/moveFileToPackageMatchingDirectory/moveToDefaultDirectory/moveToDefaultDirectory.test");
    }

+    @TestMetadata("moveFileToPackageMatchingDirectory/moveToDefaultDirectoryWithoutPackageKeyword/moveToDefaultDirectoryWithoutPackageKeyword.test")
+    public void testMoveFileToPackageMatchingDirectory_moveToDefaultDirectoryWithoutPackageKeyword_MoveToDefaultDirectoryWithoutPackageKeyword() throws Exception {
+        runTest("idea/testData/multiFileLocalInspections/moveFileToPackageMatchingDirectory/moveToDefaultDirectoryWithoutPackageKeyword/moveToDefaultDirectoryWithoutPackageKeyword.test");
+    }
+
    @TestMetadata("moveFileToPackageMatchingDirectory/moveToNonDefaultDirectory/moveToNonDefaultDirectory.test")
    public void testMoveFileToPackageMatchingDirectory_moveToNonDefaultDirectory_MoveToNonDefaultDirectory() throws Exception {
        runTest("idea/testData/multiFileLocalInspections/moveFileToPackageMatchingDirectory/moveToNonDefaultDirectory/moveToNonDefaultDirectory.test");
@@ -69,6 +74,11 @@ public class MultiFileLocalInspectionTestGenerated extends AbstractMultiFileLoca
        runTest("idea/testData/multiFileLocalInspections/reconcilePackageWithDirectory/changeToNonDefaultPackage/changeToNonDefaultPackage.test");
    }

+    @TestMetadata("reconcilePackageWithDirectory/changeToNonDefaultPackageFromRoot/changeToNonDefaultPackageFromRoot.test")
+    public void testReconcilePackageWithDirectory_changeToNonDefaultPackageFromRoot_ChangeToNonDefaultPackageFromRoot() throws Exception {
+        runTest("idea/testData/multiFileLocalInspections/reconcilePackageWithDirectory/changeToNonDefaultPackageFromRoot/changeToNonDefaultPackageFromRoot.test");
+    }
+
    @TestMetadata("reconcilePackageWithDirectory/innerClass/innerClass.test")
    public void testReconcilePackageWithDirectory_innerClass_InnerClass() throws Exception {
        runTest("idea/testData/multiFileLocalInspections/reconcilePackageWithDirectory/innerClass/innerClass.test");