Move/Change Package: Package name validation

#EA-70631 Fixed

idea/idea-analysis/src/org/jetbrains/kotlin/idea/intentions/JetSelfTargetingIntention.kt

@@ -87,7 +87,7 @@ public abstract class JetSelfTargetingIntention<TElement : JetElement>(
         return !isIntentionBaseInspectionEnabled(project, target)
     }
 
-    private fun isIntentionBaseInspectionEnabled(project: Project, target: TElement): Boolean {
+    protected fun isIntentionBaseInspectionEnabled(project: Project, target: TElement): Boolean {
         val inspection = findInspection(javaClass) ?: return false
 
         val key = HighlightDisplayKey.find(inspection.shortName)

idea/src/org/jetbrains/kotlin/idea/refactoring/jetRefactoringUtil.kt

@@ -73,6 +73,7 @@ import org.jetbrains.kotlin.idea.util.ProjectRootsUtil
 import org.jetbrains.kotlin.idea.util.string.collapseSpaces
 import org.jetbrains.kotlin.j2k.ConverterSettings
 import org.jetbrains.kotlin.j2k.JavaToKotlinConverter
+import org.jetbrains.kotlin.name.FqNameBase
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.psi.codeFragmentUtil.suppressDiagnosticsInDebugMode
 import org.jetbrains.kotlin.psi.psiUtil.*
@@ -665,3 +666,5 @@ public fun invokeOnceOnCommandFinish(action: () -> Unit) {
 }
 
 public fun String.quoteIfNeeded(): String = if (KotlinNameSuggester.isIdentifier(this)) this else "`$this`"
+
+public fun FqNameBase.hasIdentifiersOnly(): Boolean = pathSegments().all { KotlinNameSuggester.isIdentifier(it.asString()) }

idea/src/org/jetbrains/kotlin/idea/refactoring/move/changePackage/ChangePackageIntention.kt

@@ -16,22 +16,26 @@
 
 package org.jetbrains.kotlin.idea.refactoring.move.changePackage;
 
-import com.intellij.codeInsight.lookup.LookupElement
 import com.intellij.codeInsight.lookup.LookupElementBuilder
 import com.intellij.codeInsight.template.*
-import com.intellij.codeInsight.template.impl.TemplateManagerImpl
 import com.intellij.codeInsight.template.impl.TemplateState
 import com.intellij.openapi.application.ApplicationManager
 import com.intellij.openapi.editor.Editor
-import com.intellij.openapi.util.Disposer
+import com.intellij.openapi.util.TextRange
 import com.intellij.psi.PsiDocumentManager
 import org.jetbrains.kotlin.idea.codeInsight.CodeInsightUtils
-import org.jetbrains.kotlin.idea.core.refactoring.canRefactor
+import org.jetbrains.kotlin.idea.core.refactoring.hasIdentifiersOnly
 import org.jetbrains.kotlin.idea.intentions.JetSelfTargetingOffsetIndependentIntention
+import org.jetbrains.kotlin.idea.util.application.executeWriteCommand
 import org.jetbrains.kotlin.name.FqName
+import org.jetbrains.kotlin.name.FqNameUnsafe
 import org.jetbrains.kotlin.psi.JetPackageDirective
 
 public class ChangePackageIntention: JetSelfTargetingOffsetIndependentIntention<JetPackageDirective>(javaClass(), "Change package") {
+    companion object {
+        private val PACKAGE_NAME_VAR = "PACKAGE_NAME"
+    }
+
     override fun isApplicableTo(element: JetPackageDirective) = element.getPackageNameExpression() != null
 
     override fun applyTo(element: JetPackageDirective, editor: Editor) {
@@ -48,25 +52,47 @@ public class ChangePackageIntention: JetSelfTargetingOffsetIndependentIntention<
         val builder = TemplateBuilderImpl(file)
         builder.replaceElement(
                 nameExpression,
+                PACKAGE_NAME_VAR,
                 object: Expression() {
                     override fun calculateQuickResult(context: ExpressionContext?) = TextResult(currentName)
                     override fun calculateResult(context: ExpressionContext?) = TextResult(currentName)
                     override fun calculateLookupItems(context: ExpressionContext?) = arrayOf(LookupElementBuilder.create(currentName))
-                }
+                },
+                true
         )
+
+        var enteredName: String? = null
+        var affectedRange: TextRange? = null
+
         editor.getCaretModel().moveToOffset(0)
         TemplateManager.getInstance(project).startTemplate(
                 editor,
                 builder.buildInlineTemplate(),
                 object: TemplateEditingAdapter() {
+                    override fun beforeTemplateFinished(state: TemplateState?, template: Template?) {
+                        if (state == null) return
+                        enteredName = state.getVariableValue(PACKAGE_NAME_VAR)!!.text
+                        affectedRange = state.getSegmentRange(0)
+                    }
+
                     override fun templateFinished(template: Template?, brokenOff: Boolean) {
-                        if (!brokenOff) {
+                        if (brokenOff || enteredName == null || affectedRange == null) return
-                            // Restore original name and run refactoring
+
-                            val packageDirective = file.getPackageDirective()!!
+                        // Restore original name and run refactoring
-                            val newFqName = packageDirective.getFqName()
+
-                            packageDirective.setFqName(FqName(currentName))
+                        val document = editor.document
-                            KotlinChangePackageRefactoring(file).run(newFqName)
+                        project.executeWriteCommand(text) {
+                            document.replaceString(affectedRange!!.startOffset, affectedRange!!.endOffset, currentName)
                         }
+                        PsiDocumentManager.getInstance(project).commitDocument(document)
+                        PsiDocumentManager.getInstance(project).doPostponedOperationsAndUnblockDocument(document)
+
+                        if (!FqNameUnsafe(enteredName!!).hasIdentifiersOnly()) {
+                            CodeInsightUtils.showErrorHint(project, editor, "$enteredName is not a valid package name", "Change package", null)
+                            return
+                        }
+
+                        KotlinChangePackageRefactoring(file).run(FqName(enteredName!!))
                     }
                 }
         )

idea/src/org/jetbrains/kotlin/idea/refactoring/move/changePackage/ChangePackageToMatchDirectoryIntention.kt

@@ -17,11 +17,11 @@
 package org.jetbrains.kotlin.idea.refactoring.move.changePackage
 
 import com.intellij.openapi.editor.Editor
-import org.jetbrains.kotlin.idea.core.refactoring.canRefactor
-import org.jetbrains.kotlin.idea.intentions.JetSelfTargetingOffsetIndependentIntention
-import org.jetbrains.kotlin.psi.JetPackageDirective
 import org.jetbrains.kotlin.idea.core.getFqNameByDirectory
 import org.jetbrains.kotlin.idea.core.packageMatchesDirectory
+import org.jetbrains.kotlin.idea.core.refactoring.hasIdentifiersOnly
+import org.jetbrains.kotlin.idea.intentions.JetSelfTargetingOffsetIndependentIntention
+import org.jetbrains.kotlin.psi.JetPackageDirective
 
 public class ChangePackageToMatchDirectoryIntention : JetSelfTargetingOffsetIndependentIntention<JetPackageDirective>(
         javaClass(), "", "Change file's package to match directory"
@@ -30,11 +30,23 @@ public class ChangePackageToMatchDirectoryIntention : JetSelfTargetingOffsetInde
         val file = element.getContainingJetFile()
         if (file.packageMatchesDirectory()) return false
 
-        setText("Change file's package to '${file.getFqNameByDirectory().asString()}'")
+        val fqNameByDirectory = file.getFqNameByDirectory()
+        if (!fqNameByDirectory.hasIdentifiersOnly()) {
+            if (isIntentionBaseInspectionEnabled(file.project, element)) {
+                text = "File package doesn't match directory"
+                return true
+            }
+            return false
+        }
+
+        text = "Change file's package to '${fqNameByDirectory.asString()}'"
         return true
     }
 
     override fun applyTo(element: JetPackageDirective, editor: Editor) {
-        KotlinChangePackageRefactoring(element.getContainingJetFile()).run(element.getContainingFile().getFqNameByDirectory())
+        val file = element.getContainingJetFile()
+        val newFqName = file.getFqNameByDirectory()
+        if (!newFqName.hasIdentifiersOnly()) return
+        KotlinChangePackageRefactoring(file).run(newFqName)
     }
 }

idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveFilesOrDirectories/MoveKotlinFileHandler.kt

@@ -24,8 +24,10 @@ import com.intellij.refactoring.move.moveFilesOrDirectories.MoveFilesOrDirectori
 import com.intellij.usageView.UsageInfo
 import org.jetbrains.kotlin.idea.JetLanguage
 import org.jetbrains.kotlin.idea.codeInsight.shorten.runWithElementsToShortenIsEmptyIgnored
+import org.jetbrains.kotlin.idea.core.KotlinNameSuggester
 import org.jetbrains.kotlin.idea.core.getPackage
 import org.jetbrains.kotlin.idea.core.packageMatchesDirectory
+import org.jetbrains.kotlin.idea.core.refactoring.hasIdentifiersOnly
 import org.jetbrains.kotlin.idea.refactoring.move.*
 import org.jetbrains.kotlin.idea.refactoring.move.moveTopLevelDeclarations.*
 import org.jetbrains.kotlin.name.FqNameUnsafe
@@ -57,6 +59,7 @@ public class MoveKotlinFileHandler : MoveFileHandler() {
 
         val newPackageName = FqNameUnsafe(newPackage.qualifiedName)
         if (oldPackageName.asString() == newPackageName.asString()) return null
+        if (newPackageName != UNKNOWN_PACKAGE_FQ_NAME && !newPackageName.hasIdentifiersOnly()) return null
 
         return PackageNameInfo(oldPackageName, newPackageName)
     }

idea/testData/refactoring/move/kotlin/moveFile/invalidTargetPackage/after/p/123/main.kt

@@ -0,0 +1,5 @@
+package p
+
+fun foo() {
+
+}

idea/testData/refactoring/move/kotlin/moveFile/invalidTargetPackage/after/q/usage.kt

@@ -0,0 +1,7 @@
+package q
+
+import p.foo
+
+fun bar() {
+    foo()
+}

idea/testData/refactoring/move/kotlin/moveFile/invalidTargetPackage/before/p/main.kt

@@ -0,0 +1,5 @@
+package p
+
+fun foo() {
+
+}

idea/testData/refactoring/move/kotlin/moveFile/invalidTargetPackage/before/q/usage.kt

@@ -0,0 +1,7 @@
+package q
+
+import p.foo
+
+fun bar() {
+    foo()
+}

idea/testData/refactoring/move/kotlin/moveFile/invalidTargetPackage/moveFileToInvalidPackage.test

@@ -0,0 +1,5 @@
+{
+  "mainFile": "p/main.kt",
+  "type": "MOVE_FILES",
+  "targetPackage": "p.123"
+}

idea/tests/org/jetbrains/kotlin/idea/refactoring/move/JetMoveTestGenerated.java

@@ -245,6 +245,12 @@ public class JetMoveTestGenerated extends AbstractJetMoveTest {
         doTest(fileName);
     }
 
+    @TestMetadata("kotlin/moveFile/invalidTargetPackage/moveFileToInvalidPackage.test")
+    public void testKotlin_moveFile_invalidTargetPackage_MoveFileToInvalidPackage() throws Exception {
+        String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/move/kotlin/moveFile/invalidTargetPackage/moveFileToInvalidPackage.test");
+        doTest(fileName);
+    }
+
     @TestMetadata("kotlin/moveFile/moveFileToFile/moveFileToFile.test")
     public void testKotlin_moveFile_moveFileToFile_MoveFileToFile() throws Exception {
         String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/move/kotlin/moveFile/moveFileToFile/moveFileToFile.test");