From a04bede589bf31825b5d02ac0af15562354927ac Mon Sep 17 00:00:00 2001 From: Mikhail Golubev Date: Wed, 25 Mar 2015 17:56:35 +0300 Subject: [PATCH] PY-15347 Optimize imports only after moved element was deleted from original file Also I updated test data for existing test where because of sorting of imports new from-import was added *after* existing star-import (and thus star-import was indeed optimized out). As result that test didn't manage to detect the new problem. --- .../move/PyMoveModuleMembersProcessor.java | 21 ++++++++++++------- .../after/src/{b.py => zzz.py} | 0 .../move/starImportWithUsages/before/src/a.py | 2 +- .../before/src/{b.py => zzz.py} | 0 .../python/refactoring/PyMoveTest.java | 2 +- 5 files changed, 16 insertions(+), 9 deletions(-) rename python/testData/refactoring/move/starImportWithUsages/after/src/{b.py => zzz.py} (100%) rename python/testData/refactoring/move/starImportWithUsages/before/src/{b.py => zzz.py} (100%) diff --git a/python/src/com/jetbrains/python/refactoring/move/PyMoveModuleMembersProcessor.java b/python/src/com/jetbrains/python/refactoring/move/PyMoveModuleMembersProcessor.java index eb020bb9d902..f8df8b4a7991 100644 --- a/python/src/com/jetbrains/python/refactoring/move/PyMoveModuleMembersProcessor.java +++ b/python/src/com/jetbrains/python/refactoring/move/PyMoveModuleMembersProcessor.java @@ -15,6 +15,7 @@ */ package com.jetbrains.python.refactoring.move; +import com.google.common.collect.Lists; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.command.CommandProcessor; import com.intellij.openapi.project.Project; @@ -157,15 +158,22 @@ public class PyMoveModuleMembersProcessor extends BaseRefactoringProcessor { final PsiElement newElementBody = addToFile(oldElementBody, destination, usages); final PsiNamedElement newElement = PyMoveModuleMembersHelper.extractNamedElement(newElementBody); assert newElement != null; + final List usageFilesToOptimize = Lists.newArrayList(); for (UsageInfo usage : usages) { final PsiElement usageElement = usage.getElement(); if (usageElement != null) { - updateUsage(usageElement, element, newElement); + final boolean optimize = updateUsage(usageElement, element, newElement); + if (optimize) { + usageFilesToOptimize.add(usageElement.getContainingFile()); + } } } PyClassRefactoringUtil.restoreNamedReferences(newElementBody, element, myElements); // TODO: Remove extra empty lines after the removed element oldElementBody.delete(); + for (PsiFile usageFile : usageFilesToOptimize) { + PyClassRefactoringUtil.optimizeImports(usageFile); + } if (file != null) { PyClassRefactoringUtil.optimizeImports(file); } @@ -205,12 +213,12 @@ public class PyMoveModuleMembersProcessor extends BaseRefactoringProcessor { } } - private static void updateUsage(@NotNull PsiElement usage, @NotNull PsiNamedElement oldElement, @NotNull PsiNamedElement newElement) { + private static boolean updateUsage(@NotNull PsiElement usage, @NotNull PsiNamedElement oldElement, @NotNull PsiNamedElement newElement) { // TODO: Respect the qualified import style if (usage instanceof PyQualifiedExpression) { PyQualifiedExpression expr = (PyQualifiedExpression)usage; if (oldElement instanceof PyClass && PyNames.INIT.equals(expr.getName())) { - return; + return false; } if (expr.isQualified()) { final PyElementGenerator generator = PyElementGenerator.getInstance(expr.getProject()); @@ -221,7 +229,7 @@ public class PyMoveModuleMembersProcessor extends BaseRefactoringProcessor { } if (usage instanceof PyStringLiteralExpression) { for (PsiReference ref : usage.getReferences()) { - if ((ref instanceof PyDunderAllReference)) { + if (ref instanceof PyDunderAllReference) { usage.delete(); } else { @@ -242,11 +250,10 @@ public class PyMoveModuleMembersProcessor extends BaseRefactoringProcessor { } if (resolvesToLocalStarImport(usage)) { PyClassRefactoringUtil.insertImport(usage, newElement); - if (usageFile != null) { - PyClassRefactoringUtil.optimizeImports(usageFile); - } + return usageFile != null; } } + return false; } diff --git a/python/testData/refactoring/move/starImportWithUsages/after/src/b.py b/python/testData/refactoring/move/starImportWithUsages/after/src/zzz.py similarity index 100% rename from python/testData/refactoring/move/starImportWithUsages/after/src/b.py rename to python/testData/refactoring/move/starImportWithUsages/after/src/zzz.py diff --git a/python/testData/refactoring/move/starImportWithUsages/before/src/a.py b/python/testData/refactoring/move/starImportWithUsages/before/src/a.py index 6c3eacab5b29..c3fbffb33c5f 100644 --- a/python/testData/refactoring/move/starImportWithUsages/before/src/a.py +++ b/python/testData/refactoring/move/starImportWithUsages/before/src/a.py @@ -1,3 +1,3 @@ -from b import * +from zzz import * print(f()) \ No newline at end of file diff --git a/python/testData/refactoring/move/starImportWithUsages/before/src/b.py b/python/testData/refactoring/move/starImportWithUsages/before/src/zzz.py similarity index 100% rename from python/testData/refactoring/move/starImportWithUsages/before/src/b.py rename to python/testData/refactoring/move/starImportWithUsages/before/src/zzz.py diff --git a/python/testSrc/com/jetbrains/python/refactoring/PyMoveTest.java b/python/testSrc/com/jetbrains/python/refactoring/PyMoveTest.java index 37c3d40f2c07..c9ff861ca900 100644 --- a/python/testSrc/com/jetbrains/python/refactoring/PyMoveTest.java +++ b/python/testSrc/com/jetbrains/python/refactoring/PyMoveTest.java @@ -156,7 +156,7 @@ public class PyMoveTest extends PyTestCase { doMoveFileTest("p1/p2/m1.py", "nonp3"); } - // PY-6432 + // PY-6432, PY-15347 public void testStarImportWithUsages() { doMoveSymbolTest("f", "c.py"); }