From b27eed527c539498e89eb245bdba0bd11567c1fb Mon Sep 17 00:00:00 2001 From: Mikhail Golubev Date: Fri, 9 Nov 2018 13:43:53 +0300 Subject: [PATCH] PY-8415 Consider source roots when selecting the closest root containing new file --- .../PyExtractSuperclassHelper.java | 14 ++-- .../after/src/src/pkg/__init__.py | 0 .../after/src/src/pkg/subpkg/__init__.py | 0 .../after/src/src/pkg/subpkg/a.py | 1 + .../after/src/src/pkg/subpkg/b.py | 2 + .../before/src/src/pkg/__init__.py | 0 .../before/src/src/pkg/subpkg/__init__.py | 0 .../before/src/src/pkg/subpkg/a.py | 2 + .../python/refactoring/PyMoveTest.java | 83 ++++++++++--------- 9 files changed, 59 insertions(+), 43 deletions(-) create mode 100644 python/testData/refactoring/move/moveSymbolDoesntCreateInitPyInSourceRoot/after/src/src/pkg/__init__.py create mode 100644 python/testData/refactoring/move/moveSymbolDoesntCreateInitPyInSourceRoot/after/src/src/pkg/subpkg/__init__.py create mode 100644 python/testData/refactoring/move/moveSymbolDoesntCreateInitPyInSourceRoot/after/src/src/pkg/subpkg/a.py create mode 100644 python/testData/refactoring/move/moveSymbolDoesntCreateInitPyInSourceRoot/after/src/src/pkg/subpkg/b.py create mode 100644 python/testData/refactoring/move/moveSymbolDoesntCreateInitPyInSourceRoot/before/src/src/pkg/__init__.py create mode 100644 python/testData/refactoring/move/moveSymbolDoesntCreateInitPyInSourceRoot/before/src/src/pkg/subpkg/__init__.py create mode 100644 python/testData/refactoring/move/moveSymbolDoesntCreateInitPyInSourceRoot/before/src/src/pkg/subpkg/a.py diff --git a/python/src/com/jetbrains/python/refactoring/classes/extractSuperclass/PyExtractSuperclassHelper.java b/python/src/com/jetbrains/python/refactoring/classes/extractSuperclass/PyExtractSuperclassHelper.java index 1cb870027ff6..9b423281d842 100644 --- a/python/src/com/jetbrains/python/refactoring/classes/extractSuperclass/PyExtractSuperclassHelper.java +++ b/python/src/com/jetbrains/python/refactoring/classes/extractSuperclass/PyExtractSuperclassHelper.java @@ -32,6 +32,7 @@ import com.intellij.refactoring.listeners.RefactoringEventData; import com.intellij.refactoring.listeners.RefactoringEventListener; import com.intellij.util.Function; import com.intellij.util.PathUtil; +import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.JBIterable; import com.jetbrains.python.PyNames; import com.jetbrains.python.PythonFileType; @@ -43,10 +44,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.io.IOException; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; +import java.util.*; /** * @author Dennis.Ushakov @@ -211,7 +209,13 @@ public final class PyExtractSuperclassHelper { // NOTE: we don't canonicalize target; must be ok in reasonable cases, and is far easier in unit test mode target = FileUtil.toSystemIndependentName(target); - for (VirtualFile file : ProjectRootManager.getInstance(project).getContentRoots()) { + final ProjectRootManager projectRootManager = ProjectRootManager.getInstance(project); + final List allRoots = new ArrayList<>(); + ContainerUtil.addAll(allRoots, projectRootManager.getContentRoots()); + ContainerUtil.addAll(allRoots, projectRootManager.getContentSourceRoots()); + // Check deepest roots first + allRoots.sort(Comparator.comparingInt((VirtualFile vf) -> vf.getPath().length()).reversed()); + for (VirtualFile file : allRoots) { final String rootPath = file.getPath(); if (target.startsWith(rootPath)) { relativePath = target.substring(rootPath.length()); diff --git a/python/testData/refactoring/move/moveSymbolDoesntCreateInitPyInSourceRoot/after/src/src/pkg/__init__.py b/python/testData/refactoring/move/moveSymbolDoesntCreateInitPyInSourceRoot/after/src/src/pkg/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/python/testData/refactoring/move/moveSymbolDoesntCreateInitPyInSourceRoot/after/src/src/pkg/subpkg/__init__.py b/python/testData/refactoring/move/moveSymbolDoesntCreateInitPyInSourceRoot/after/src/src/pkg/subpkg/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/python/testData/refactoring/move/moveSymbolDoesntCreateInitPyInSourceRoot/after/src/src/pkg/subpkg/a.py b/python/testData/refactoring/move/moveSymbolDoesntCreateInitPyInSourceRoot/after/src/src/pkg/subpkg/a.py new file mode 100644 index 000000000000..8b137891791f --- /dev/null +++ b/python/testData/refactoring/move/moveSymbolDoesntCreateInitPyInSourceRoot/after/src/src/pkg/subpkg/a.py @@ -0,0 +1 @@ + diff --git a/python/testData/refactoring/move/moveSymbolDoesntCreateInitPyInSourceRoot/after/src/src/pkg/subpkg/b.py b/python/testData/refactoring/move/moveSymbolDoesntCreateInitPyInSourceRoot/after/src/src/pkg/subpkg/b.py new file mode 100644 index 000000000000..1dea43b79188 --- /dev/null +++ b/python/testData/refactoring/move/moveSymbolDoesntCreateInitPyInSourceRoot/after/src/src/pkg/subpkg/b.py @@ -0,0 +1,2 @@ +class MyClass: + pass \ No newline at end of file diff --git a/python/testData/refactoring/move/moveSymbolDoesntCreateInitPyInSourceRoot/before/src/src/pkg/__init__.py b/python/testData/refactoring/move/moveSymbolDoesntCreateInitPyInSourceRoot/before/src/src/pkg/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/python/testData/refactoring/move/moveSymbolDoesntCreateInitPyInSourceRoot/before/src/src/pkg/subpkg/__init__.py b/python/testData/refactoring/move/moveSymbolDoesntCreateInitPyInSourceRoot/before/src/src/pkg/subpkg/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/python/testData/refactoring/move/moveSymbolDoesntCreateInitPyInSourceRoot/before/src/src/pkg/subpkg/a.py b/python/testData/refactoring/move/moveSymbolDoesntCreateInitPyInSourceRoot/before/src/src/pkg/subpkg/a.py new file mode 100644 index 000000000000..07d9e0efebfa --- /dev/null +++ b/python/testData/refactoring/move/moveSymbolDoesntCreateInitPyInSourceRoot/before/src/src/pkg/subpkg/a.py @@ -0,0 +1,2 @@ +class MyClass: + pass diff --git a/python/testSrc/com/jetbrains/python/refactoring/PyMoveTest.java b/python/testSrc/com/jetbrains/python/refactoring/PyMoveTest.java index bcd013288724..0b68eb7f5d97 100644 --- a/python/testSrc/com/jetbrains/python/refactoring/PyMoveTest.java +++ b/python/testSrc/com/jetbrains/python/refactoring/PyMoveTest.java @@ -24,6 +24,7 @@ import com.intellij.psi.codeStyle.CommonCodeStyleSettings; import com.intellij.psi.search.ProjectScope; import com.intellij.refactoring.move.moveFilesOrDirectories.MoveFilesOrDirectoriesProcessor; import com.intellij.testFramework.PlatformTestUtil; +import com.intellij.util.Consumer; import com.intellij.util.IncorrectOperationException; import com.intellij.util.SystemProperties; import com.intellij.util.containers.ContainerUtil; @@ -41,6 +42,7 @@ import org.jetbrains.annotations.Nullable; import java.io.IOException; import java.util.Collection; +import java.util.Collections; import java.util.List; import static com.jetbrains.python.refactoring.move.moduleMembers.PyMoveModuleMembersHelper.isMovableModuleMember; @@ -431,62 +433,67 @@ public class PyMoveTest extends PyTestCase { doMoveSymbolsTest("dst.py", "func"); } - private void doMoveFileTest(String fileName, String toDirName) { - Project project = myFixture.getProject(); - PsiManager manager = PsiManager.getInstance(project); + // PY-8415 + public void testMoveSymbolDoesntCreateInitPyInSourceRoot() { + doComparingDirectories(testDir -> { + final VirtualFile sourceRoot = testDir.findFileByRelativePath("src"); + runWithSourceRoots(Collections.singletonList(sourceRoot), () -> { + moveSymbols(testDir, "src/pkg/subpkg/b.py", "MyClass"); + }); + }); + } - String root = "/refactoring/move/" + getTestName(true); - String rootBefore = root + "/before/src"; - String rootAfter = root + "/after/src"; + private void doComparingDirectories(@NotNull Consumer testDirConsumer) { + final String root = "/refactoring/move/" + getTestName(true); + final String rootBefore = root + "/before/src"; + final String rootAfter = root + "/after/src"; - VirtualFile dir1 = myFixture.copyDirectoryToProject(rootBefore, ""); - PsiDocumentManager.getInstance(project).commitAllDocuments(); + final VirtualFile testDir = myFixture.copyDirectoryToProject(rootBefore, ""); + PsiDocumentManager.getInstance(myFixture.getProject()).commitAllDocuments(); + + testDirConsumer.consume(testDir); - VirtualFile virtualFile = dir1.findFileByRelativePath(fileName); - assertNotNull(virtualFile); - PsiElement file = manager.findFile(virtualFile); - if (file == null) { - file = manager.findDirectory(virtualFile); - } - assertNotNull(file); - VirtualFile toVirtualDir = dir1.findFileByRelativePath(toDirName); - assertNotNull(toVirtualDir); - PsiDirectory toDir = manager.findDirectory(toVirtualDir); - new MoveFilesOrDirectoriesProcessor(project, new PsiElement[]{file}, toDir, false, false, null, null).run(); - - VirtualFile dir2 = getVirtualFileByName(PythonTestUtil.getTestDataPath() + rootAfter); + final VirtualFile expectedDir = getVirtualFileByName(PythonTestUtil.getTestDataPath() + rootAfter); try { - PlatformTestUtil.assertDirectoriesEqual(dir2, dir1); + PlatformTestUtil.assertDirectoriesEqual(expectedDir, testDir); } catch (IOException e) { throw new RuntimeException(e); } } - private void doMoveSymbolsTest(@NotNull String toFileName, String... symbolNames) { - String root = "/refactoring/move/" + getTestName(true); - String rootBefore = root + "/before/src"; - String rootAfter = root + "/after/src"; - VirtualFile dir1 = myFixture.copyDirectoryToProject(rootBefore, ""); - PsiDocumentManager.getInstance(myFixture.getProject()).commitAllDocuments(); + private void doMoveFileTest(String fileName, String toDirName) { + doComparingDirectories(testDir -> { + final Project project = myFixture.getProject(); + final PsiManager manager = PsiManager.getInstance(project); + final VirtualFile virtualFile = testDir.findFileByRelativePath(fileName); + assertNotNull(virtualFile); + PsiElement file = manager.findFile(virtualFile); + if (file == null) { + file = manager.findDirectory(virtualFile); + } + assertNotNull(file); + final VirtualFile toVirtualDir = testDir.findFileByRelativePath(toDirName); + assertNotNull(toVirtualDir); + final PsiDirectory toDir = manager.findDirectory(toVirtualDir); + new MoveFilesOrDirectoriesProcessor(project, new PsiElement[]{file}, toDir, false, false, null, null).run(); + }); + } + private void doMoveSymbolsTest(@NotNull String toFileName, String... symbolNames) { + doComparingDirectories(testDir -> moveSymbols(testDir, toFileName, symbolNames)); + } + + private void moveSymbols(@NotNull VirtualFile testDir, @NotNull String toFileName, @NotNull String... symbolNames) { final PsiNamedElement[] symbols = ContainerUtil.map2Array(symbolNames, PsiNamedElement.class, name -> { final PsiNamedElement found = findFirstNamedElement(name); assertNotNull("Symbol '" + name + "' does not exist", found); return found; }); - VirtualFile toVirtualFile = dir1.findFileByRelativePath(toFileName); - String path = toVirtualFile != null ? toVirtualFile.getPath() : (dir1.getPath() + "/" + toFileName); + final VirtualFile toVirtualFile = testDir.findFileByRelativePath(toFileName); + final String path = toVirtualFile != null ? toVirtualFile.getPath() : (testDir.getPath() + "/" + toFileName); new PyMoveModuleMembersProcessor(symbols, path).run(); - - VirtualFile dir2 = getVirtualFileByName(PythonTestUtil.getTestDataPath() + rootAfter); - try { - PlatformTestUtil.assertDirectoriesEqual(dir2, dir1); - } - catch (IOException e) { - throw new RuntimeException(e); - } }