Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PyImportSort: Option to treat imports separately #1356

Merged
merged 1 commit into from May 6, 2017
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -18,6 +18,64 @@ class PyImportSortBear(LocalBear):
LICENSE = 'AGPL-3.0'
CAN_FIX = {'Formatting'}

@staticmethod
def _seperate_imports(file):
import_stmts = []
tmp = []
paren = False
for lineno, lines in enumerate(file, start=1):
if 'import' in lines.split() or paren:
# To ensure that
# from x import ( y,
# z) type of imports are not treated as
# different sections
if '(' in lines and ')' not in lines:
paren = True
if ')' in lines:
paren = False
tmp.append((lineno, lines))
else:
if tmp:
import_stmts.append(tmp)
tmp = []
# To ensure that if the last line of a file is an import statement
# it doesn't get ignored
if tmp:
import_stmts.append(tmp)
tmp = []
return import_stmts

def _get_diff(self):
if self.treat_seperated_imports_independently:
import_stmts = PyImportSortBear._seperate_imports(self.file)
sorted_imps = []
for units in import_stmts:
sort_imports = SortImports(file_contents=''.
join([x[1] for x in units]),
**self.isort_settings)
sort_imports = sort_imports.output.splitlines(True)
sorted_imps.append((units, sort_imports))

diff = Diff(self.file)
for old, new in sorted_imps:
start = old[0][0]
end = start + len(old) - 1
diff.delete_lines(start, end)
assert isinstance(new, list)
diff.add_lines(start, list(new))

if diff.modified != diff._file:
return diff
else:
sort_imports = SortImports(file_contents=''.join(self.file),
**self.isort_settings)

new_file = tuple(sort_imports.output.splitlines(True))
if new_file != tuple(self.file):
diff = Diff.from_string_arrays(self.file, new_file)
return diff

This comment has been minimized.

Copy link
@gitmate-bot

gitmate-bot Mar 15, 2017

Collaborator

E127 continuation line over-indented for visual indent'

PycodestyleBear (E127), severity NORMAL, section autopep8.

This comment has been minimized.

Copy link
@gitmate-bot

gitmate-bot Mar 15, 2017

Collaborator

The code does not comply to PEP8.

PEP8Bear, severity NORMAL, section autopep8.

The issue can be fixed by applying the following patch:

--- a/bears/python/PyImportSortBear.py
+++ b/bears/python/PyImportSortBear.py
@@ -239,7 +239,7 @@
         self.file = file
         self.filename = filename
         self.treat_seperated_imports_independently = \
-                                        treat_seperated_imports_independently
+            treat_seperated_imports_independently
 
         diff = self._get_diff()
 
return None

@deprecate_settings(indent_size='tab_width')
def run(self, filename, file,
use_parentheses_in_import: bool=True,
@@ -47,7 +105,8 @@ def run(self, filename, file,
known_third_party_imports: typed_list(str)=(),
known_standard_library_imports: typed_list(str)=None,
max_line_length: int=79,
imports_forced_to_top: typed_list(str)=()):
imports_forced_to_top: typed_list(str)=(),
treat_seperated_imports_independently: bool=False):

This comment has been minimized.

Copy link
@sils

sils Mar 12, 2017

Member

missing documentation

This comment has been minimized.

Copy link
@jayvdb
"""
Raise issues related to sorting imports, segregating imports into
various sections, and also adding comments on top of each import
@@ -145,6 +204,9 @@ def run(self, filename, file,
dependencies that occur in many projects.
:param max_line_length:
Maximum number of characters for a line.
:param treat_seperated_imports_independently:
Treat import statements seperated by one or more blank line or any
statement other than an import statement as an independent bunch.
"""
isort_settings = dict(
use_parentheses=use_parentheses_in_import,
@@ -177,12 +239,15 @@ def run(self, filename, file,
isort_settings['known_standard_library'] = (
known_standard_library_imports)

sort_imports = SortImports(file_contents=''.join(file),
**isort_settings)
new_file = tuple(sort_imports.output.splitlines(True))
self.isort_settings = isort_settings
self.file = file
self.filename = filename
self.treat_seperated_imports_independently = \
treat_seperated_imports_independently

diff = self._get_diff()

if new_file != tuple(file):
diff = Diff.from_string_arrays(file, new_file)
if diff:
yield Result(self,
'Imports can be sorted.',
affected_code=diff.affected_code(filename),
@@ -1,5 +1,11 @@
from bears.python.PyImportSortBear import PyImportSortBear
from coalib.testing.LocalBearTestHelper import verify_local_bear

This comment has been minimized.

Copy link
@gitmate-bot

gitmate-bot Mar 15, 2017

Collaborator

The code does not comply to PEP8.

PEP8Bear, severity NORMAL, section autopep8.

The issue can be fixed by applying the following patch:

--- a/tests/python/PyImportSortBearTest.py
+++ b/tests/python/PyImportSortBearTest.py
@@ -8,6 +8,7 @@
 from coalib.settings.Setting import Setting
 
 from coalib.results.Diff import Diff
+
 
 class PyImportSortBearTest(unittest.TestCase):
 
import unittest
from queue import Queue

from coalib.settings.Section import Section

from coalib.results.Diff import Diff

PyImportSortBearTest = verify_local_bear(PyImportSortBear,
('import os\nimport sys\n',
@@ -27,3 +33,88 @@
('import sys\n\nimport datetime\n',),
('import datetime\nimport sys\n',),
settings={'known_third_party_imports': 'datetime'})


class PyImportSortBearTest(unittest.TestCase):

def setUp(self):
self.queue = Queue()
self.section = Section('PyImportSortBear')
self.uut = PyImportSortBear(self.section,
self.queue)

def test_isort_settings(self):
test_file = """
import os
import re
import requests
""".splitlines(True)

test_file2 = """
import re
import os
import requests
""".splitlines(True)

self.assertEqual(list(self.uut.run('', test_file)), [])
self.assertEqual(list(self.uut.run('',
test_file2))[0].diffs[''].modified,
['\n', 'import os\n', 'import re\n', '\n',
'import requests\n'])

def test_treat_seperated_imports_independently(self):
test_file = (

This comment has been minimized.

Copy link
@meetmangukiya

meetmangukiya Apr 3, 2017

Author Member

this is more of a regression test for this bug

This comment has been minimized.

Copy link
@jayvdb

jayvdb Apr 8, 2017

Member

It is too big. Can you create a simpler example of the bug being fixed.

This comment has been minimized.

Copy link
@jayvdb
"""
import re
import requests
from urllib.parse import urlparse
from coalib.results.Diff import Diff
from coalib.bears.LocalBear import LocalBear
""".splitlines(True)
)

expected = (
"""
import re
from urllib.parse import urlparse
import requests
from coalib.bears.LocalBear import LocalBear
from coalib.results.Diff import Diff
""".splitlines(True)
)
diff = Diff(expected)
settings = {'treat_seperated_imports_independently': True}
self.assertEqual(list(self.uut.run('',
test_file,
**settings))[0].diffs[''].modified,
diff.modified)

self.assertEqual(list(self.uut.run('',
['import curses\n', 'import this\n',
'coala = "coala"\n'],
**settings)), [])

def test_import_seperation(self):
file = (
"""
from x import (y,
z,
w)
import this
import a, b
""".splitlines(True)
)
seperated = [[(2, 'from x import (y,\n'),
(3, ' z,\n'),
(4, ' w)\n'),
(5, 'import this\n')],
[(7, 'import a, b\n')]]

self.assertEqual(self.uut._seperate_imports(file),
seperated)
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.