From 8c31a5c569cd914c543460c2d0fd8fa100e5542e Mon Sep 17 00:00:00 2001 From: Vladimir Krivosheev Date: Fri, 12 Jun 2020 17:49:32 +0200 Subject: [PATCH] IDEA-242675 Opening folder without .idea creates corrupted project GitOrigin-RevId: a75f221451f0964d29464bb7a3308cff1b049f0f --- .../src/com/intellij/openapi/vfs/VfsUtil.java | 6 +- .../copy/CopyFilesOrDirectoriesDialog.java | 2 +- .../intellij/ide/CommandLineProcessor.java | 8 +- .../ide/CommandLineProcessorResult.kt | 4 +- .../intellij/ide/actions/OpenFileAction.java | 46 +++++--- .../com/intellij/ide/impl/OpenProjectTask.kt | 2 +- .../com/intellij/ide/impl/ProjectUtil.java | 3 +- .../fileChooser/ex/FileChooserDialogImpl.java | 6 +- .../fileChooser/impl/FileChooserUtil.java | 19 +++- .../platform/PlatformProjectOpenProcessor.kt | 18 ++- .../openapi/project/OpenProjectTest.kt | 104 ++++++++++++++++++ 11 files changed, 183 insertions(+), 35 deletions(-) create mode 100644 platform/platform-tests/testSrc/com/intellij/openapi/project/OpenProjectTest.kt diff --git a/platform/analysis-api/src/com/intellij/openapi/vfs/VfsUtil.java b/platform/analysis-api/src/com/intellij/openapi/vfs/VfsUtil.java index 2fad0908798c..c432c53571b2 100644 --- a/platform/analysis-api/src/com/intellij/openapi/vfs/VfsUtil.java +++ b/platform/analysis-api/src/com/intellij/openapi/vfs/VfsUtil.java @@ -347,8 +347,8 @@ public final class VfsUtil extends VfsUtilCore { public static @Nullable VirtualFile createDirectoryIfMissing(@NotNull String directoryPath) throws IOException { return createDirectoryIfMissing(LocalFileSystem.getInstance(), directoryPath); } - - public static @Nullable VirtualFile createDirectoryIfMissing(@NotNull VirtualFileSystem fileSystem, + + public static @Nullable VirtualFile createDirectoryIfMissing(@NotNull VirtualFileSystem fileSystem, @NotNull String directoryPath) throws IOException { String path = FileUtil.toSystemIndependentName(directoryPath); VirtualFile file = fileSystem.refreshAndFindFileByPath(path); @@ -409,7 +409,7 @@ public final class VfsUtil extends VfsUtilCore { } public static @Nullable VirtualFile getUserHomeDir() { - final String path = SystemProperties.getUserHome(); + String path = SystemProperties.getUserHome(); return LocalFileSystem.getInstance().findFileByPath(FileUtil.toSystemIndependentName(path)); } diff --git a/platform/lang-impl/src/com/intellij/refactoring/copy/CopyFilesOrDirectoriesDialog.java b/platform/lang-impl/src/com/intellij/refactoring/copy/CopyFilesOrDirectoriesDialog.java index 4cecf90cecd9..0e07ba317c45 100644 --- a/platform/lang-impl/src/com/intellij/refactoring/copy/CopyFilesOrDirectoriesDialog.java +++ b/platform/lang-impl/src/com/intellij/refactoring/copy/CopyFilesOrDirectoriesDialog.java @@ -318,7 +318,7 @@ public class CopyFilesOrDirectoriesDialog extends RefactoringDialog implements D Messages.showErrorDialog(myProject, RefactoringBundle.message("cannot.create.directory"), RefactoringBundle.message("error.title")); return; } - FileChooserUtil.setLastOpenedFile(myProject, myTargetDirectory.getVirtualFile()); + FileChooserUtil.setLastOpenedFile(myProject, myTargetDirectory.getVirtualFile().toNioPath()); try { for (PsiElement element : myElements) { diff --git a/platform/platform-impl/src/com/intellij/ide/CommandLineProcessor.java b/platform/platform-impl/src/com/intellij/ide/CommandLineProcessor.java index e540cf1f37a9..014b5ca2456a 100644 --- a/platform/platform-impl/src/com/intellij/ide/CommandLineProcessor.java +++ b/platform/platform-impl/src/com/intellij/ide/CommandLineProcessor.java @@ -21,8 +21,10 @@ import com.intellij.openapi.vfs.VirtualFile; import com.intellij.openapi.wm.IdeFocusManager; import com.intellij.openapi.wm.IdeFrame; import com.intellij.platform.CommandLineProjectOpenProcessor; +import com.intellij.platform.PlatformProjectOpenProcessor; import com.intellij.pom.Navigatable; import com.intellij.util.PlatformUtils; +import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -44,8 +46,10 @@ public final class CommandLineProcessor { private CommandLineProcessor() { } - private static @NotNull CommandLineProcessorResult doOpenFileOrProject(Path file, boolean shouldWait) { - OpenProjectTask openProjectOptions = new OpenProjectTask(); + // public for testing + @ApiStatus.Internal + public static @NotNull CommandLineProcessorResult doOpenFileOrProject(@NotNull Path file, boolean shouldWait) { + OpenProjectTask openProjectOptions = PlatformProjectOpenProcessor.createOptionsToOpenDotIdeaOrCreateNewIfNotExists(file, null); // do not check for .ipr files in specified directory (@develar: it is existing behaviour, I am not fully sure that it is correct) openProjectOptions.checkDirectoryForFileBasedProjects = false; Project project = ProjectUtil.openOrImport(file, openProjectOptions); diff --git a/platform/platform-impl/src/com/intellij/ide/CommandLineProcessorResult.kt b/platform/platform-impl/src/com/intellij/ide/CommandLineProcessorResult.kt index 077ef47ba7cf..1f47bfb69ba5 100644 --- a/platform/platform-impl/src/com/intellij/ide/CommandLineProcessorResult.kt +++ b/platform/platform-impl/src/com/intellij/ide/CommandLineProcessorResult.kt @@ -3,9 +3,11 @@ package com.intellij.ide import com.intellij.openapi.project.Project import com.intellij.openapi.ui.Messages +import org.jetbrains.annotations.ApiStatus import java.util.concurrent.Future -internal data class CommandLineProcessorResult(val project: Project?, val future: Future) { +@ApiStatus.Internal +data class CommandLineProcessorResult(val project: Project?, val future: Future) { companion object { @JvmStatic fun createError(message: String): CommandLineProcessorResult { diff --git a/platform/platform-impl/src/com/intellij/ide/actions/OpenFileAction.java b/platform/platform-impl/src/com/intellij/ide/actions/OpenFileAction.java index f34475e2fceb..381010f1f46c 100644 --- a/platform/platform-impl/src/com/intellij/ide/actions/OpenFileAction.java +++ b/platform/platform-impl/src/com/intellij/ide/actions/OpenFileAction.java @@ -14,6 +14,7 @@ import com.intellij.ide.lightEdit.LightEditUtil; import com.intellij.ide.util.PsiNavigationSupport; import com.intellij.openapi.actionSystem.AnAction; import com.intellij.openapi.actionSystem.AnActionEvent; +import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.fileChooser.FileChooser; import com.intellij.openapi.fileChooser.FileChooserDescriptor; import com.intellij.openapi.fileChooser.FileChooserDescriptorFactory; @@ -35,6 +36,7 @@ import com.intellij.platform.PlatformProjectOpenProcessor; import com.intellij.projectImport.ProjectAttachProcessor; import com.intellij.projectImport.ProjectOpenProcessor; import com.intellij.util.PlatformUtils; +import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -49,9 +51,9 @@ import static com.intellij.ide.lightEdit.LightEditFeatureUsagesUtil.OpenPlace.We public class OpenFileAction extends AnAction implements DumbAware, LightEditCompatible { @Override public void actionPerformed(@NotNull AnActionEvent e) { - final Project project = e.getProject(); - final boolean showFiles = project != null || PlatformProjectOpenProcessor.getInstanceIfItExists() != null; - final FileChooserDescriptor descriptor = showFiles ? new ProjectOrFileChooserDescriptor() : new ProjectOnlyFileChooserDescriptor(); + Project project = e.getProject(); + boolean showFiles = project != null || PlatformProjectOpenProcessor.getInstanceIfItExists() != null; + FileChooserDescriptor descriptor = showFiles ? new ProjectOrFileChooserDescriptor() : new ProjectOnlyFileChooserDescriptor(); VirtualFile toSelect = null; if (StringUtil.isNotEmpty(GeneralSettings.getInstance().getDefaultProjectDirectory())) { @@ -98,19 +100,7 @@ public class OpenFileAction extends AnAction implements DumbAware, LightEditComp private static void doOpenFile(@Nullable Project project, @NotNull VirtualFile file) { Path filePath = file.toNioPath(); if (Files.isDirectory(filePath)) { - boolean canAttach = ProjectAttachProcessor.canAttachToProject(); - boolean preferAttach = PlatformUtils.isDataGrip() && project != null && canAttach && !ProjectUtil.isValidProjectPath(filePath); - Project openedProject; - if (preferAttach && PlatformProjectOpenProcessor.attachToProject(project, filePath, null)) { - return; - } - else if (canAttach) { - openedProject = ProjectManagerEx.getInstanceEx().openProject(filePath, PlatformProjectOpenProcessor.createOptionsToOpenDotIdeaOrCreateNewIfNotExists(filePath, project)); - } - else { - openedProject = ProjectUtil.openOrImport(filePath, OpenProjectTask.withProjectToClose(project)); - } - FileChooserUtil.setLastOpenedFile(openedProject, file); + openExistingDir(filePath, project); return; } @@ -123,7 +113,7 @@ public class OpenFileAction extends AnAction implements DumbAware, LightEditComp else if (answer == Messages.YES) { Project openedProject = ProjectUtil.openOrImport(filePath, OpenProjectTask.withProjectToClose(project)); if (openedProject != null) { - FileChooserUtil.setLastOpenedFile(openedProject, file); + FileChooserUtil.setLastOpenedFile(openedProject, filePath); } return; } @@ -146,6 +136,28 @@ public class OpenFileAction extends AnAction implements DumbAware, LightEditComp } } + // public for testing + @ApiStatus.Internal + public static @Nullable Project openExistingDir(@NotNull Path file, @Nullable Project currentProject) { + Project openedProject; + boolean canAttach = ProjectAttachProcessor.canAttachToProject(); + boolean preferAttach = currentProject != null && canAttach && PlatformUtils.isDataGrip() && !ProjectUtil.isValidProjectPath(file); + if (preferAttach && PlatformProjectOpenProcessor.attachToProject(currentProject, file, null)) { + return null; + } + else if (canAttach) { + OpenProjectTask options = PlatformProjectOpenProcessor.createOptionsToOpenDotIdeaOrCreateNewIfNotExists(file, currentProject); + openedProject = ProjectManagerEx.getInstanceEx().openProject(file, options); + } + else { + openedProject = ProjectUtil.openOrImport(file, OpenProjectTask.withProjectToClose(currentProject)); + } + if (!ApplicationManager.getApplication().isUnitTestMode()) { + FileChooserUtil.setLastOpenedFile(openedProject, file); + } + return openedProject; + } + @Messages.YesNoCancelResult private static int shouldOpenNewProject(@Nullable Project project, @NotNull VirtualFile file) { if (file.getFileType() instanceof ProjectFileType) { diff --git a/platform/platform-impl/src/com/intellij/ide/impl/OpenProjectTask.kt b/platform/platform-impl/src/com/intellij/ide/impl/OpenProjectTask.kt index f36d785218b7..aab042e4e62e 100644 --- a/platform/platform-impl/src/com/intellij/ide/impl/OpenProjectTask.kt +++ b/platform/platform-impl/src/com/intellij/ide/impl/OpenProjectTask.kt @@ -80,5 +80,5 @@ data class OpenProjectTask(val forceOpenInNewFrame: Boolean = false, /** Used only by [ProjectUtil.openOrImport] */ @JvmField - var checkDirectoryForFileBasedProjects = true + internal var checkDirectoryForFileBasedProjects = true } \ No newline at end of file diff --git a/platform/platform-impl/src/com/intellij/ide/impl/ProjectUtil.java b/platform/platform-impl/src/com/intellij/ide/impl/ProjectUtil.java index a3e8bce05fee..eb8282f33d32 100644 --- a/platform/platform-impl/src/com/intellij/ide/impl/ProjectUtil.java +++ b/platform/platform-impl/src/com/intellij/ide/impl/ProjectUtil.java @@ -159,7 +159,8 @@ public final class ProjectUtil { } if (isValidProjectPath(file)) { - return ProjectManagerEx.getInstanceEx().openProject(file, options); + // see OpenProjectTest.`open valid existing project dir with inability to attach using OpenFileAction` test about why `runConfigurators = true` is specified here + return ProjectManagerEx.getInstanceEx().openProject(file, options.withRunConfigurators()); } if (options.checkDirectoryForFileBasedProjects && Files.isDirectory(file)) { diff --git a/platform/platform-impl/src/com/intellij/openapi/fileChooser/ex/FileChooserDialogImpl.java b/platform/platform-impl/src/com/intellij/openapi/fileChooser/ex/FileChooserDialogImpl.java index cc1bd5c3c0e0..b6e50d1b88b0 100644 --- a/platform/platform-impl/src/com/intellij/openapi/fileChooser/ex/FileChooserDialogImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/fileChooser/ex/FileChooserDialogImpl.java @@ -1,4 +1,4 @@ -// Copyright 2000-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +// Copyright 2000-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package com.intellij.openapi.fileChooser.ex; import com.intellij.icons.AllIcons; @@ -159,7 +159,9 @@ public class FileChooserDialogImpl extends DialogWrapper implements FileChooserD } void storeSelection(@Nullable VirtualFile file) { - FileChooserUtil.setLastOpenedFile(myProject, file); + if (file != null) { + FileChooserUtil.setLastOpenedFile(myProject, file.toNioPath()); + } if (file != null && file.getFileSystem() instanceof LocalFileSystem) { saveRecent(file.getPath()); } diff --git a/platform/platform-impl/src/com/intellij/openapi/fileChooser/impl/FileChooserUtil.java b/platform/platform-impl/src/com/intellij/openapi/fileChooser/impl/FileChooserUtil.java index 3b24a6128efb..0c89ea2ca4f3 100644 --- a/platform/platform-impl/src/com/intellij/openapi/fileChooser/impl/FileChooserUtil.java +++ b/platform/platform-impl/src/com/intellij/openapi/fileChooser/impl/FileChooserUtil.java @@ -8,6 +8,7 @@ import com.intellij.openapi.fileChooser.PathChooserDialog; import com.intellij.openapi.project.Project; import com.intellij.openapi.project.ProjectUtil; import com.intellij.openapi.util.SystemInfo; +import com.intellij.openapi.util.io.FileUtil; import com.intellij.openapi.vfs.LocalFileSystem; import com.intellij.openapi.vfs.VfsUtil; import com.intellij.openapi.vfs.VirtualFile; @@ -16,6 +17,7 @@ import com.intellij.util.containers.ContainerUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import java.nio.file.Path; import java.util.List; public final class FileChooserUtil { @@ -33,8 +35,14 @@ public final class FileChooserUtil { return path != null ? LocalFileSystem.getInstance().findFileByPath(path) : null; } + /** + * @deprecated Use {@link #setLastOpenedFile(Project, Path)} + */ + @Deprecated public static void setLastOpenedFile(@Nullable Project project, @Nullable VirtualFile file) { - if (file == null) return; + if (file == null) { + return; + } if (project == null) { PropertiesComponent.getInstance().setValue(LAST_OPENED_FILE_PATH, file.getPath()); } @@ -43,6 +51,15 @@ public final class FileChooserUtil { } } + public static void setLastOpenedFile(@Nullable Project project, @NotNull Path file) { + if (project == null) { + PropertiesComponent.getInstance().setValue(LAST_OPENED_FILE_PATH, FileUtil.toSystemIndependentName(file.toString())); + } + else if (!project.isDisposed()) { + PropertiesComponent.getInstance(project).setValue(LAST_OPENED_FILE_PATH, FileUtil.toSystemIndependentName(file.toString())); + } + } + @Nullable public static VirtualFile getFileToSelect(@NotNull FileChooserDescriptor descriptor, @Nullable Project project, @Nullable VirtualFile toSelect, @Nullable VirtualFile lastPath) { diff --git a/platform/platform-impl/src/com/intellij/platform/PlatformProjectOpenProcessor.kt b/platform/platform-impl/src/com/intellij/platform/PlatformProjectOpenProcessor.kt index 6c8d0ef8617d..376bccc3e7a9 100644 --- a/platform/platform-impl/src/com/intellij/platform/PlatformProjectOpenProcessor.kt +++ b/platform/platform-impl/src/com/intellij/platform/PlatformProjectOpenProcessor.kt @@ -190,19 +190,25 @@ class PlatformProjectOpenProcessor : ProjectOpenProcessor(), CommandLineProjectO } /** - * If project file in IDEA format (.idea directory or .ipr file) exists, just open it. + * If project file in IDEA format (.idea directory or .ipr file) exists, open it and run configurators if no modules. + * * If doesn't exists, create a new project using default project template and run configurators (something that creates module). * (at the moment of creation project file in IDEA format will be removed if any). + * + * This method must be not used in tests. + * + * See OpenProjectTest. */ @ApiStatus.Internal @JvmStatic fun createOptionsToOpenDotIdeaOrCreateNewIfNotExists(projectDir: Path, projectToClose: Project?): OpenProjectTask { - val validProjectPath = ProjectUtil.isValidProjectPath(projectDir) - // doesn't make sense to use default project as template in tests - return OpenProjectTask(runConfigurators = !validProjectPath, - isNewProject = !validProjectPath, + // doesn't make sense to refresh or to use default project as template in tests + val isUnitTestMode = ApplicationManager.getApplication().isUnitTestMode + return OpenProjectTask(runConfigurators = true, + isNewProject = !ProjectUtil.isValidProjectPath(projectDir), projectToClose = projectToClose, - useDefaultProjectAsTemplate = !ApplicationManager.getApplication().isUnitTestMode) + isRefreshVfsNeeded = !isUnitTestMode, + useDefaultProjectAsTemplate = !isUnitTestMode) } } diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/project/OpenProjectTest.kt b/platform/platform-tests/testSrc/com/intellij/openapi/project/OpenProjectTest.kt new file mode 100644 index 000000000000..d0cb9ac3dc37 --- /dev/null +++ b/platform/platform-tests/testSrc/com/intellij/openapi/project/OpenProjectTest.kt @@ -0,0 +1,104 @@ +// Copyright 2000-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +package com.intellij.openapi.project + +import com.intellij.ide.CommandLineProcessor +import com.intellij.ide.actions.OpenFileAction +import com.intellij.openapi.module.ModuleManager +import com.intellij.platform.ModuleAttachProcessor +import com.intellij.projectImport.ProjectAttachProcessor +import com.intellij.testFramework.* +import com.intellij.testFramework.assertions.Assertions.assertThat +import com.intellij.util.io.createDirectories +import org.junit.ClassRule +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.Parameterized +import java.nio.file.Path + +// terms: +// valid: .idea exists +// clean: .idea doesn't exists +// existing: project directory exists + +// with ability to attach - there is some defined ProjectAttachProcessor extension (e.g. WS, PS). +// with inability to attach - there is no any defined ProjectAttachProcessor extension (e.g. IU, IC). + +@RunWith(Parameterized::class) +internal class OpenProjectTest(private val opener: Opener) { + companion object { + @JvmField + @ClassRule + val appRule = ApplicationRule() + + @JvmStatic + @Parameterized.Parameters(name = "{0}") + fun params(): Iterable { + return listOf( + Opener("OpenFileAction") { OpenFileAction.openExistingDir(it, null)!! }, + Opener("CLI") { CommandLineProcessor.doOpenFileOrProject(it, false).project!! } + ) + } + } + + @JvmField + @Rule + val tempDir = TemporaryDirectory() + + @JvmField + @Rule + val disposableRule = DisposableRule() + + @Test + fun `open valid existing project dir with ability to attach`() { + ExtensionTestUtil.maskExtensions(ProjectAttachProcessor.EP_NAME, listOf(ModuleAttachProcessor()), disposableRule.disposable) + val projectDir = tempDir.newPath("project") + projectDir.resolve(".idea").createDirectories() + openUsingOpenFileActionAndAssertThatProjectContainsOneModule(projectDir) + } + + @Test + fun `open clean existing project dir with ability to attach`() { + ExtensionTestUtil.maskExtensions(ProjectAttachProcessor.EP_NAME, listOf(ModuleAttachProcessor()), disposableRule.disposable) + val projectDir = tempDir.newPath("project") + projectDir.createDirectories() + openUsingOpenFileActionAndAssertThatProjectContainsOneModule(projectDir) + } + + @Test + fun `open valid existing project dir with inability to attach`() { + // Regardless of product (Idea vs PhpStorm), if .idea directory exists, but no modules, we must run configurators to add some module. + // Maybe not fully clear why it is performed as part of project opening and silently, but it is existing behaviour. + // So, existing behaviour should be preserved and any changes should be done not as part of task "use unified API to open project", but separately later. + ExtensionTestUtil.maskExtensions(ProjectAttachProcessor.EP_NAME, listOf(), disposableRule.disposable) + val projectDir = tempDir.newPath("project") + projectDir.resolve(".idea").createDirectories() + openUsingOpenFileActionAndAssertThatProjectContainsOneModule(projectDir) + } + + @Test + fun `open clean existing project dir with inability to attach`() { + ExtensionTestUtil.maskExtensions(ProjectAttachProcessor.EP_NAME, listOf(), disposableRule.disposable) + val projectDir = tempDir.newPath("project") + projectDir.createDirectories() + openUsingOpenFileActionAndAssertThatProjectContainsOneModule(projectDir) + } + + private fun openUsingOpenFileActionAndAssertThatProjectContainsOneModule(projectDir: Path) { + val project = opener.opener(projectDir)!! + try { + assertThatProjectContainsOneModule(project) + } + finally { + PlatformTestUtil.forceCloseProjectWithoutSaving(project) + } + } +} + +internal class Opener(private val name: String, val opener: (Path) -> Project?) { + override fun toString() = name +} + +private fun assertThatProjectContainsOneModule(project: Project) { + assertThat(ModuleManager.getInstance(project).modules).hasSize(1) +} \ No newline at end of file