From 9e44e9ce9593ce42f3dc1f01a04bcce5f7e2aa4e Mon Sep 17 00:00:00 2001 From: Mikhail Golubev Date: Wed, 9 Sep 2015 21:53:55 +0300 Subject: [PATCH] PY-12018 Optimize imports detects that imports are reordered It also doesn't leave stale IMPORT_GROUP_BEGIN flags in user data that confuses formatter afterwards. --- .../imports/PyImportOptimizer.java | 76 +++++++++++-------- .../refactoring/move/module/after/src/a.py | 2 +- .../after/src/subpkg/m1.py | 8 +- 3 files changed, 51 insertions(+), 35 deletions(-) diff --git a/python/src/com/jetbrains/python/codeInsight/imports/PyImportOptimizer.java b/python/src/com/jetbrains/python/codeInsight/imports/PyImportOptimizer.java index be747a858ade..4e581f85d353 100644 --- a/python/src/com/jetbrains/python/codeInsight/imports/PyImportOptimizer.java +++ b/python/src/com/jetbrains/python/codeInsight/imports/PyImportOptimizer.java @@ -15,6 +15,7 @@ */ package com.jetbrains.python.codeInsight.imports; +import com.google.common.collect.Ordering; import com.intellij.codeInspection.LocalInspectionToolSession; import com.intellij.lang.ImportOptimizer; import com.intellij.psi.PsiElement; @@ -28,6 +29,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -75,7 +77,6 @@ public class PyImportOptimizer implements ImportOptimizer { private final List myProjectImports = new ArrayList(); private final List myImportBlock; private final PyElementGenerator myGenerator; - private boolean myMissorted = false; private ImportSorter(PyFile file) { myFile = file; @@ -87,6 +88,7 @@ public class PyImportOptimizer implements ImportOptimizer { if (myImportBlock.isEmpty()) { return; } + boolean hasSplittedImports = false; LanguageLevel langLevel = LanguageLevel.forElement(myFile); for (PyImportStatementBase importStatement : myImportBlock) { if (importStatement instanceof PyFromImportStatement && ((PyFromImportStatement)importStatement).isFromFuture()) { @@ -94,7 +96,7 @@ public class PyImportOptimizer implements ImportOptimizer { } if (importStatement instanceof PyImportStatement && importStatement.getImportElements().length > 1) { for (PyImportElement importElement : importStatement.getImportElements()) { - myMissorted = true; + hasSplittedImports = true; // getText() for ImportElement includes alias final PyImportStatement splitImport = myGenerator.createImportStatement(langLevel, importElement.getText(), null); prioritize(splitImport, importElement.resolve()); @@ -112,11 +114,18 @@ public class PyImportOptimizer implements ImportOptimizer { prioritize(importStatement, toImport); } } - if (myMissorted || needBlankLinesBetweenGroups()) { + if (hasSplittedImports || needBlankLinesBetweenGroups() || groupsNotSorted()) { applyResults(); } } + private boolean groupsNotSorted() { + final Ordering importOrdering = Ordering.from(AddImportHelper.IMPORT_BY_NAME_COMPARATOR); + return SORT_IMPORTS && (!importOrdering.isOrdered(myBuiltinImports) || + !importOrdering.isOrdered(myProjectImports) || + !importOrdering.isOrdered(myThirdPartyImports)); + } + private boolean needBlankLinesBetweenGroups() { int nonEmptyGroups = 0; if (myBuiltinImports.size() > 0) nonEmptyGroups++; @@ -134,17 +143,11 @@ public class PyImportOptimizer implements ImportOptimizer { : AddImportHelper.getImportPriority(myFile, (PsiFileSystemItem)toImport); if (priority == AddImportHelper.ImportPriority.BUILTIN) { myBuiltinImports.add(importStatement); - if (!myThirdPartyImports.isEmpty() || !myProjectImports.isEmpty()) { - myMissorted = true; - } } else if (priority == AddImportHelper.ImportPriority.THIRD_PARTY) { myThirdPartyImports.add(importStatement); - if (!myProjectImports.isEmpty()) { - myMissorted = true; - } } - else { + else if (priority == AddImportHelper.ImportPriority.PROJECT) { myProjectImports.add(importStatement); } } @@ -155,23 +158,48 @@ public class PyImportOptimizer implements ImportOptimizer { Collections.sort(myThirdPartyImports, AddImportHelper.IMPORT_BY_NAME_COMPARATOR); Collections.sort(myProjectImports, AddImportHelper.IMPORT_BY_NAME_COMPARATOR); } + markGroupStarts(); + addImports(); - markGroupBegin(myThirdPartyImports); - markGroupBegin(myProjectImports); - - addImports(myBuiltinImports); - addImports(myThirdPartyImports); - addImports(myProjectImports); final PsiElement lastElement = myImportBlock.get(myImportBlock.size() - 1); final PyImportStatementBase firstNonFutureImport = findFirstNonFutureImport(); if (firstNonFutureImport != null) { myFile.deleteChildRange(firstNonFutureImport, lastElement); } - for (PyImportStatementBase anImport : myBuiltinImports) { - anImport.putCopyableUserData(PyBlock.IMPORT_GROUP_BEGIN, null); + } + + private void markGroupStarts() { + boolean firstNonEmptyGroup = true; + for (List imports : getImportGroupsInOrder()) { + boolean firstImportInGroup = true; + for (PyImportStatementBase statement : imports) { + if (!firstNonEmptyGroup && firstImportInGroup) { + statement.putCopyableUserData(PyBlock.IMPORT_GROUP_BEGIN, true); + } + else { + statement.putCopyableUserData(PyBlock.IMPORT_GROUP_BEGIN, null); + } + firstImportInGroup = false; + firstNonEmptyGroup = false; + } } } + private void addImports() { + final PyImportStatementBase anchor = findFirstNonFutureImport(); + for (List imports : getImportGroupsInOrder()) { + for (PyImportStatementBase newImport : imports) { + myFile.addBefore(newImport, anchor); + } + } + } + + @NotNull + private List> getImportGroupsInOrder() { + return Arrays.asList(myBuiltinImports, myThirdPartyImports, myProjectImports); + } + + @Nullable private PyImportStatementBase findFirstNonFutureImport() { for (PyImportStatementBase importStatement : myImportBlock) { if (!(importStatement instanceof PyFromImportStatement && ((PyFromImportStatement)importStatement).isFromFuture())) { @@ -180,17 +208,5 @@ public class PyImportOptimizer implements ImportOptimizer { } return null; } - - private static void markGroupBegin(@NotNull List imports) { - if (imports.size() > 0) { - imports.get(0).putCopyableUserData(PyBlock.IMPORT_GROUP_BEGIN, true); - } - } - - private void addImports(final List imports) { - for (PyImportStatementBase newImport : imports) { - myFile.addBefore(newImport, findFirstNonFutureImport()); - } - } } } diff --git a/python/testData/refactoring/move/module/after/src/a.py b/python/testData/refactoring/move/module/after/src/a.py index 37425aac803c..97e6ebff9c01 100644 --- a/python/testData/refactoring/move/module/after/src/a.py +++ b/python/testData/refactoring/move/module/after/src/a.py @@ -1,7 +1,7 @@ from p1 import m1 +import p1 from p1.m1 import foo import p1.m1 -import p1 p1.m1.foo m1.bar diff --git a/python/testData/refactoring/move/relativeImportsToModulesInSameMovedPackageNotUpdated/after/src/subpkg/m1.py b/python/testData/refactoring/move/relativeImportsToModulesInSameMovedPackageNotUpdated/after/src/subpkg/m1.py index 4a40476cffa0..659649343f8e 100644 --- a/python/testData/refactoring/move/relativeImportsToModulesInSameMovedPackageNotUpdated/after/src/subpkg/m1.py +++ b/python/testData/refactoring/move/relativeImportsToModulesInSameMovedPackageNotUpdated/after/src/subpkg/m1.py @@ -1,8 +1,8 @@ -from . import m2 import m3 -import subsubpkg as foo -from . import subsubpkg as bar -from .subsubpkg import m4 from pkg import m5 +import subsubpkg as foo +from .subsubpkg import m4 +from . import m2 +from . import subsubpkg as bar print(m2, m3, m4, m5, foo, bar)