From c628ec52212a5dc5e7c77da49a9af240a3acf708 Mon Sep 17 00:00:00 2001 From: Mikhail Golubev Date: Mon, 15 Jun 2015 20:24:17 +0300 Subject: [PATCH] PY-10553 Correctly update "from imports" if moved module and imported symbol share name Check that reference to moved module is contained in PyImportElement part of a "from import" statement before calling PyClassRefactoringUtil#updateImportOfElement. Because this method relies solely on name equality, it can accidentally replace "from import" of particular symbol from module with the "from import" of that module itself. --- .../refactoring/classes/PyClassRefactoringUtil.java | 8 ++++++++ .../python/refactoring/move/PyMoveFileHandler.java | 4 +++- .../after/src/Animals/Cat.py | 4 ++++ .../after/src/Animals/__init__.py | 9 +++++++++ .../after/src/Animals/test/Carnivore.py | 2 ++ .../after/src/Animals/test/__init__.py | 0 .../before/src/Animals/Carnivore.py | 2 ++ .../before/src/Animals/Cat.py | 4 ++++ .../before/src/Animals/__init__.py | 9 +++++++++ .../before/src/Animals/test/__init__.py | 0 .../com/jetbrains/python/refactoring/PyMoveTest.java | 5 +++++ 11 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/after/src/Animals/Cat.py create mode 100644 python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/after/src/Animals/__init__.py create mode 100644 python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/after/src/Animals/test/Carnivore.py create mode 100644 python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/after/src/Animals/test/__init__.py create mode 100644 python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/before/src/Animals/Carnivore.py create mode 100644 python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/before/src/Animals/Cat.py create mode 100644 python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/before/src/Animals/__init__.py create mode 100644 python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/before/src/Animals/test/__init__.py diff --git a/python/src/com/jetbrains/python/refactoring/classes/PyClassRefactoringUtil.java b/python/src/com/jetbrains/python/refactoring/classes/PyClassRefactoringUtil.java index 85251eb252cb..29d8b1e10461 100644 --- a/python/src/com/jetbrains/python/refactoring/classes/PyClassRefactoringUtil.java +++ b/python/src/com/jetbrains/python/refactoring/classes/PyClassRefactoringUtil.java @@ -428,6 +428,14 @@ public final class PyClassRefactoringUtil { }); } + /** + * Updates the import statement if the given PSI element has the same name as one of the import elements of that statement. + * It means that you should be careful it you actually want to update the source part of a "from import" statement, because in cases + * like {@code from foo import foo} this method may do not what you expect. + * + * @param importStatement parent import statement that contains reference to given element + * @param element PSI element reference to which should be updated + */ public static void updateImportOfElement(@NotNull PyImportStatementBase importStatement, @NotNull PsiNamedElement element) { final String name = getOriginalName(element); if (name != null) { diff --git a/python/src/com/jetbrains/python/refactoring/move/PyMoveFileHandler.java b/python/src/com/jetbrains/python/refactoring/move/PyMoveFileHandler.java index c08c9941c383..3f7b12d0f55d 100644 --- a/python/src/com/jetbrains/python/refactoring/move/PyMoveFileHandler.java +++ b/python/src/com/jetbrains/python/refactoring/move/PyMoveFileHandler.java @@ -178,7 +178,9 @@ public class PyMoveFileHandler extends MoveFileHandler { // TODO: Retarget qualified expressions in docstrings if (importStmt != null) { updatedFiles.add(file); - PyClassRefactoringUtil.updateImportOfElement(importStmt, newElement); + if (PsiTreeUtil.getParentOfType(element, PyImportElement.class) != null) { + PyClassRefactoringUtil.updateImportOfElement(importStmt, newElement); + } if (importStmt instanceof PyFromImportStatement && PsiTreeUtil.getParentOfType(element, PyImportElement.class) != null) { continue; } diff --git a/python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/after/src/Animals/Cat.py b/python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/after/src/Animals/Cat.py new file mode 100644 index 000000000000..c0d380fb0cb2 --- /dev/null +++ b/python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/after/src/Animals/Cat.py @@ -0,0 +1,4 @@ +from Animals.test.Carnivore import Carnivore + +class Cat(Carnivore): + pass \ No newline at end of file diff --git a/python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/after/src/Animals/__init__.py b/python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/after/src/Animals/__init__.py new file mode 100644 index 000000000000..d7733f312c17 --- /dev/null +++ b/python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/after/src/Animals/__init__.py @@ -0,0 +1,9 @@ +# encoding: utf-8 +# +# +# +# Author: Markus Thielen +# +# Copyright (c) 2013 thi.guten Software Development +# +__author__ = 'east825' diff --git a/python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/after/src/Animals/test/Carnivore.py b/python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/after/src/Animals/test/Carnivore.py new file mode 100644 index 000000000000..6dbb02ee8d2a --- /dev/null +++ b/python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/after/src/Animals/test/Carnivore.py @@ -0,0 +1,2 @@ +class Carnivore(object): + pass \ No newline at end of file diff --git a/python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/after/src/Animals/test/__init__.py b/python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/after/src/Animals/test/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/before/src/Animals/Carnivore.py b/python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/before/src/Animals/Carnivore.py new file mode 100644 index 000000000000..6dbb02ee8d2a --- /dev/null +++ b/python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/before/src/Animals/Carnivore.py @@ -0,0 +1,2 @@ +class Carnivore(object): + pass \ No newline at end of file diff --git a/python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/before/src/Animals/Cat.py b/python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/before/src/Animals/Cat.py new file mode 100644 index 000000000000..415869e31429 --- /dev/null +++ b/python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/before/src/Animals/Cat.py @@ -0,0 +1,4 @@ +from Animals.Carnivore import Carnivore + +class Cat(Carnivore): + pass \ No newline at end of file diff --git a/python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/before/src/Animals/__init__.py b/python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/before/src/Animals/__init__.py new file mode 100644 index 000000000000..d7733f312c17 --- /dev/null +++ b/python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/before/src/Animals/__init__.py @@ -0,0 +1,9 @@ +# encoding: utf-8 +# +# +# +# Author: Markus Thielen +# +# Copyright (c) 2013 thi.guten Software Development +# +__author__ = 'east825' diff --git a/python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/before/src/Animals/test/__init__.py b/python/testData/refactoring/move/moveModuleWithSameNameAsSymbolInside/before/src/Animals/test/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/python/testSrc/com/jetbrains/python/refactoring/PyMoveTest.java b/python/testSrc/com/jetbrains/python/refactoring/PyMoveTest.java index 5bd85a97663f..2761e949dcc5 100644 --- a/python/testSrc/com/jetbrains/python/refactoring/PyMoveTest.java +++ b/python/testSrc/com/jetbrains/python/refactoring/PyMoveTest.java @@ -314,6 +314,11 @@ public class PyMoveTest extends PyTestCase { } } + // PY-10553 + public void testMoveModuleWithSameNameAsSymbolInside() { + doMoveFileTest("Animals/Carnivore.py", "Animals/test"); + } + // PY-15324 public void testInterdependentSymbols() { doMoveSymbolsTest("b.py", "f", "A");