From fad2d5309a5a2172e04a4205da1e28c6e6e4552d Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Fri, 28 Feb 2020 22:03:31 +0000 Subject: [PATCH 01/13] Add skeleton for fixing multiline from imports --- autoflake.py | 119 +++++++++++++++++++++++++++++++++++++++++++--- test_autoflake.py | 35 ++++++++++++++ 2 files changed, 147 insertions(+), 7 deletions(-) diff --git a/autoflake.py b/autoflake.py index ac33696..97e2a11 100755 --- a/autoflake.py +++ b/autoflake.py @@ -271,6 +271,105 @@ def multiline_statement(line, previous_line=''): return True +class Continuation(object): + """Allows a rewrite operation to span multiple lines. + + In the main rewrite loop, every time a helper function returns a + ``Continuation`` object instead of a string, this object will be called + with the following line. + """ + def __init__(self, line): + self.accumulator = collections.deque([line]) + + def __call__(self, line): + """Process line considering the accumulator. + + Return self to keep processing the following lines or a string + with the final result of all the lines processed at once. + """ + raise NotImplementedError("Abstract method needs to be overwritten") + + +def _valid_char_in_line(char, line): + """Returns True if a char appears in the line and is not commented""" + comment_index = line.find('#') + char_index = line.find(char) + valid_char_in_line = ( + char_index >= 0 and + (comment_index > char_index or comment_index < 0) + ) + return valid_char_in_line + + +class FilterMultilineFromImport(Continuation): + IMPORT_RE = re.compile(r'\bimport\b') + INDENTATION_RE = re.compile(r'^\s*\(?') + REST_RE = re.compile(r'\s*[)#\\].*$') + TRAILING_RE = re.compile(r'\s*,?\s*$') + + def __init__(self, line): + self.parentesized = '(' in line + self.from_, imports = self.IMPORT_RE.split(line, maxsplit=1) + Continuation.__init__(self, imports) + + def is_over(self, line): + """Returns True if the multiline import statement is over""" + if self.parentesized: + return _valid_char_in_line(')', line) + + return not _valid_char_in_line('\\', line) + + def parse_line(self, line): + """Break the line in indentation, actual imports and the rest. + + Returns a tuple with 3 strings: indentation, the actual modules being + imported and the rest (comments, line continuation (``\\``) and + eventual hanging commas). + """ + indentation = "" + imports = line + rest = "" + + match = self.INDENTATION_RE.search(imports) + if match: + indentation = match.group(0) + imports = imports.replace(indentation, '', 1) + + match = self.REST_RE.search(imports) + if match: + rest = match.group(0) + imports = imports.replace(rest, '', 1) + + match = self.TRAILING_RE.search(imports) + if match: + rest = match.group(0) + rest + imports = imports.rstrip(" \t,") + + return (indentation, imports, rest) + + def fix_line(self, line): + indentation, imports, rest = self.parse_line(line) + if imports: + pretend_single_line = self.from_ + " from " + imports + # TODO: call original filter_from_import + _, fixed = self.IMPORT_RE.split(pretend_single_line, maxsplit=1) + line = indentation + fixed + rest + if line.strip(): + return line + else: + # This will skip when the line is left empty + return "" + + def __call__(self, line): + self.accumulator.append(line) + if not self.is_over(line): + return self + new_text = "".join(self.fix_line(line) for line in self.accumulator) + # TODO: remove duplicated commas + return new_text + + + def filter_from_import(line, unused_module): """Parse and filter ``from something import a, b, c``. @@ -387,26 +486,32 @@ def filter_code(source, additional_imports=None, sio = io.StringIO(source) previous_line = '' + result = None for line_number, line in enumerate(sio.readlines(), start=1): - if '#' in line: - yield line + if isinstance(result, Continuation): + result = result(line) + elif '#' in line: + result = line elif line_number in marked_import_line_numbers: - yield filter_unused_import( + result = filter_unused_import( line, unused_module=marked_unused_module[line_number], remove_all_unused_imports=remove_all_unused_imports, imports=imports, previous_line=previous_line) elif line_number in marked_variable_line_numbers: - yield filter_unused_variable(line) + result = filter_unused_variable(line) elif line_number in marked_key_line_numbers: - yield filter_duplicate_key(line, line_messages[line_number], + result = filter_duplicate_key(line, line_messages[line_number], line_number, marked_key_line_numbers, source) elif line_number in marked_star_import_line_numbers: - yield filter_star_import(line, undefined_names) + result = filter_star_import(line, undefined_names) else: - yield line + result = line + + if not isinstance(result, Continuation): + yield result previous_line = line diff --git a/test_autoflake.py b/test_autoflake.py index 859c5ee..8b2e7e0 100755 --- a/test_autoflake.py +++ b/test_autoflake.py @@ -16,6 +16,7 @@ import unittest import autoflake +from autoflake import FilterMultilineFromImport ROOT_DIRECTORY = os.path.abspath(os.path.dirname(__file__)) @@ -1624,6 +1625,40 @@ def test_end_to_end_with_error(self): process.communicate()[1].decode()) +class MultilineFromImportTests(unittest.TestCase): + def test_is_over(self): + filt = FilterMultilineFromImport('from . import (') + self.assertEqual(filt.is_over('module)'), True) + self.assertEqual(filt.is_over(' )'), True) + self.assertEqual(filt.is_over(' ) # comment'), True) + self.assertEqual(filt.is_over('# )'), False) + self.assertEqual(filt.is_over('module'), False) + self.assertEqual(filt.is_over('module, \\'), False) + self.assertEqual(filt.is_over(''), False) + + filt = FilterMultilineFromImport('from . import module, \\') + self.assertEqual(filt.is_over('module'), True) + self.assertEqual(filt.is_over(''), True) + self.assertEqual(filt.is_over('m1, m2 # comment with \\'), True) + self.assertEqual(filt.is_over('m1, m2 \\'), False) + self.assertEqual(filt.is_over('m1, m2 \\ #'), False) + self.assertEqual(filt.is_over('m1, m2 \\ # comment with \\'), False) + self.assertEqual(filt.is_over('\\'), False) + + def test_parse_line(self): + filt = FilterMultilineFromImport('from . import (') + self.assertEqual(filt.parse_line(" a, b"), (" ", "a, b", "")) + self.assertEqual(filt.parse_line("a, b, "), ("", "a, b", ", ")) + self.assertEqual(filt.parse_line("a, b , "), ("", "a, b", " , ")) + self.assertEqual(filt.parse_line("a, b"), ("", "a, b", "")) + self.assertEqual(filt.parse_line("a, b "), ("", "a, b", " ")) + self.assertEqual(filt.parse_line("a, b,"), ("", "a, b", ",")) + self.assertEqual(filt.parse_line(" "), (" ", "", "")) + self.assertEqual(filt.parse_line("a, b)"), ("", "a, b", ")")) + self.assertEqual(filt.parse_line("a, b, ) "), ("", "a, b", ", ) ")) + self.assertEqual(filt.parse_line("a, b\\ "), ("", "a, b", "\\ ")) + + @contextlib.contextmanager def temporary_file(contents, directory='.', suffix='.py', prefix=''): """Write contents to temporary file and yield it.""" From fc7860f6bb2fc6813cda31a6165966afe5b973ef Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Sat, 29 Feb 2020 00:37:12 +0000 Subject: [PATCH 02/13] Rename "Contination" to "PendingFix" --- autoflake.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/autoflake.py b/autoflake.py index 97e2a11..7f0e32b 100755 --- a/autoflake.py +++ b/autoflake.py @@ -271,11 +271,11 @@ def multiline_statement(line, previous_line=''): return True -class Continuation(object): +class PendingFix(object): """Allows a rewrite operation to span multiple lines. In the main rewrite loop, every time a helper function returns a - ``Continuation`` object instead of a string, this object will be called + ``PendingFix`` object instead of a string, this object will be called with the following line. """ def __init__(self, line): @@ -301,7 +301,7 @@ def _valid_char_in_line(char, line): return valid_char_in_line -class FilterMultilineFromImport(Continuation): +class FilterMultilineFromImport(PendingFix): IMPORT_RE = re.compile(r'\bimport\b') INDENTATION_RE = re.compile(r'^\s*\(?') REST_RE = re.compile(r'\s*[)#\\].*$') @@ -310,7 +310,7 @@ class FilterMultilineFromImport(Continuation): def __init__(self, line): self.parentesized = '(' in line self.from_, imports = self.IMPORT_RE.split(line, maxsplit=1) - Continuation.__init__(self, imports) + PendingFix.__init__(self, imports) def is_over(self, line): """Returns True if the multiline import statement is over""" @@ -488,7 +488,7 @@ def filter_code(source, additional_imports=None, previous_line = '' result = None for line_number, line in enumerate(sio.readlines(), start=1): - if isinstance(result, Continuation): + if isinstance(result, PendingFix): result = result(line) elif '#' in line: result = line @@ -510,7 +510,7 @@ def filter_code(source, additional_imports=None, else: result = line - if not isinstance(result, Continuation): + if not isinstance(result, PendingFix): yield result previous_line = line From 642450bf419c343a75f466035877e5eb7aec8949 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Sat, 29 Feb 2020 06:14:59 +0000 Subject: [PATCH 03/13] Add fix for the multiline from import problem --- autoflake.py | 140 ++++++++++++++++++++++++-------------- test_autoflake.py | 169 ++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 230 insertions(+), 79 deletions(-) diff --git a/autoflake.py b/autoflake.py index 7f0e32b..1f47888 100755 --- a/autoflake.py +++ b/autoflake.py @@ -35,6 +35,7 @@ import os import re import signal +import string import sys import tokenize @@ -302,14 +303,20 @@ def _valid_char_in_line(char, line): class FilterMultilineFromImport(PendingFix): - IMPORT_RE = re.compile(r'\bimport\b') - INDENTATION_RE = re.compile(r'^\s*\(?') - REST_RE = re.compile(r'\s*[)#\\].*$') - TRAILING_RE = re.compile(r'\s*,?\s*$') + IMPORT_RE = re.compile(r'\s*\bimport\b\s*') + INDENTATION_RE = re.compile(r'^\s*') + HANGING_LPAR_RE = re.compile(r'^\s*\(\s*(#.*)?$') + BASE_RE = re.compile(r'\bfrom\s+([^ ]+)') - def __init__(self, line): + Parsed = collections.namedtuple('Parsed', + 'indentation leading_comma imports ' + 'trailing_comma line_continuation') + + def __init__(self, line, unused_module=()): + self.unused = unused_module self.parentesized = '(' in line self.from_, imports = self.IMPORT_RE.split(line, maxsplit=1) + self.base = self.BASE_RE.search(self.from_).group(1) PendingFix.__init__(self, imports) def is_over(self, line): @@ -320,54 +327,91 @@ def is_over(self, line): return not _valid_char_in_line('\\', line) def parse_line(self, line): - """Break the line in indentation, actual imports and the rest. - - Returns a tuple with 3 strings: indentation, the actual modules being - imported and the rest (comments, line continuation (``\\``) and - eventual hanging commas). - """ - indentation = "" - imports = line - rest = "" - - match = self.INDENTATION_RE.search(imports) - if match: - indentation = match.group(0) - imports = imports.replace(indentation, '', 1) - - match = self.REST_RE.search(imports) - if match: - rest = match.group(0) - imports = imports.replace(rest, '', 1) - - match = self.TRAILING_RE.search(imports) - if match: - rest = match.group(0) + rest - imports = imports.rstrip(" \t,") - - return (indentation, imports, rest) + """Break the line in the different components""" + match = self.INDENTATION_RE.search(line) + indentation = match.group(0) + # Remove the extra space (indentation already stored) so we can detect + # leading commas and line continuation + imports = line.strip() + leading_comma = ',' if imports and imports[0] == ',' else '' + line_continuation = ' \\' if imports and imports[-1] == '\\' else '' + + # Remove the line continuation (already stored) and hanging pars so we + # can detect trailing commas + imports = imports.strip(string.whitespace + '()\\') + trailing_comma = ',' if imports and imports[-1] == ',' else '' + + # Finally remove the remaining trailing/leading chars + imports = imports.strip(string.whitespace + ',') + + return self.Parsed(indentation, leading_comma, imports, + trailing_comma, line_continuation) def fix_line(self, line): - indentation, imports, rest = self.parse_line(line) - if imports: - pretend_single_line = self.from_ + " from " + imports - # TODO: call original filter_from_import - _, fixed = self.IMPORT_RE.split(pretend_single_line, maxsplit=1) - line = indentation + fixed + rest - if line.strip(): + parsed = self.parse_line(line) + if not parsed.imports: + # Do nothing if the line does not contain any actual import return line - else: - # This will skip when the line is left empty - return "" + imports = [x.strip() for x in parsed.imports.split(',')] + clean_imports = _filter_imports(imports, self.base, self.unused) + ending = get_line_ending(line).lstrip(' \t') + + if not clean_imports: + # Even when there is no import left, a single comma might still be + # necessary + if parsed.leading_comma and parsed.trailing_comma: + return (parsed.indentation + parsed.leading_comma + + parsed.line_continuation + ending) + else: + # The line can be removed + return None + + if parsed.leading_comma: + # By inserting an empty element at the beginning of the list we + # force a trailing comma correctly separated by a space + clean_imports.insert(0, '') + import_str = ', '.join(clean_imports) + + return (parsed.indentation + import_str + parsed.trailing_comma + + parsed.line_continuation + ending) + + def fix(self, accumulated): + new_lines = collections.deque() + for line in accumulated: + # Don't change if the line contains a comment or is empty except + # for trailing characters (but preserve indentation) + comment = '#' in line + without_trailing = line.rstrip(string.whitespace + '()\\') + empty = len(without_trailing.strip()) < 1 + if comment or empty: + ending = get_line_ending(line).lstrip(' \t') + new_lines.append(line.rstrip() + ending) + else: + new_lines.append(self.fix_line(line)) + + code = ''.join(x for x in new_lines if x) # Filter None out + ending = get_line_ending(code).lstrip(' \t') + code = code.rstrip(string.whitespace + '\\') # Avoid extra \ + if self.parentesized: + if '(' not in code: + code = '(' + code.lstrip(' \t') + if ')' not in code: + code = code + ')' + + return self.from_ + ' import ' + code + ending def __call__(self, line): self.accumulator.append(line) if not self.is_over(line): return self - new_text = "".join(self.fix_line(line) for line in self.accumulator) - # TODO: remove duplicated commas - return new_text + return self.fix(self.accumulator) + + +def _filter_imports(imports, base_module, unused_module): + # We compare full module name (``a.module`` not `module`) to + # guarantee the exact same module as detected from pyflakes. + return [x for x in imports if base_module + '.' + x not in unused_module] def filter_from_import(line, unused_module): @@ -381,15 +425,11 @@ def filter_from_import(line, unused_module): base_module = re.search(pattern=r'\bfrom\s+([^ ]+)', string=indentation).group(1) - # Create an imported module list with base module name - # ex ``from a import b, c as d`` -> ``['a.b', 'a.c as d']`` - imports = re.split(pattern=r',', string=imports.strip()) - imports = [base_module + '.' + x.strip() for x in imports] + imports = re.split(pattern=r'\s*,\s*', string=imports.strip()) # We compare full module name (``a.module`` not `module`) to # guarantee the exact same module as detected from pyflakes. - filtered_imports = [x.replace(base_module + '.', '') - for x in imports if x not in unused_module] + filtered_imports = _filter_imports(imports, base_module, unused_module) # All of the import in this statement is unused if not filtered_imports: diff --git a/test_autoflake.py b/test_autoflake.py index 8b2e7e0..92baad5 100755 --- a/test_autoflake.py +++ b/test_autoflake.py @@ -6,6 +6,7 @@ from __future__ import unicode_literals import contextlib +import functools import io import os import re @@ -16,7 +17,6 @@ import unittest import autoflake -from autoflake import FilterMultilineFromImport ROOT_DIRECTORY = os.path.abspath(os.path.dirname(__file__)) @@ -1627,36 +1627,147 @@ def test_end_to_end_with_error(self): class MultilineFromImportTests(unittest.TestCase): def test_is_over(self): - filt = FilterMultilineFromImport('from . import (') - self.assertEqual(filt.is_over('module)'), True) - self.assertEqual(filt.is_over(' )'), True) - self.assertEqual(filt.is_over(' ) # comment'), True) - self.assertEqual(filt.is_over('# )'), False) - self.assertEqual(filt.is_over('module'), False) - self.assertEqual(filt.is_over('module, \\'), False) - self.assertEqual(filt.is_over(''), False) - - filt = FilterMultilineFromImport('from . import module, \\') - self.assertEqual(filt.is_over('module'), True) - self.assertEqual(filt.is_over(''), True) - self.assertEqual(filt.is_over('m1, m2 # comment with \\'), True) - self.assertEqual(filt.is_over('m1, m2 \\'), False) - self.assertEqual(filt.is_over('m1, m2 \\ #'), False) - self.assertEqual(filt.is_over('m1, m2 \\ # comment with \\'), False) - self.assertEqual(filt.is_over('\\'), False) + filt = autoflake.FilterMultilineFromImport('from . import (') + self.assertTrue(filt.is_over('module)')) + self.assertTrue(filt.is_over(' )')) + self.assertTrue(filt.is_over(' ) # comment')) + self.assertFalse(filt.is_over('# )')) + self.assertFalse(filt.is_over('module')) + self.assertFalse(filt.is_over('module, \\')) + self.assertFalse(filt.is_over('')) + + filt = autoflake.FilterMultilineFromImport('from . import module, \\') + self.assertTrue(filt.is_over('module')) + self.assertTrue(filt.is_over('')) + self.assertTrue(filt.is_over('m1, m2 # comment with \\')) + self.assertFalse(filt.is_over('m1, m2 \\')) + self.assertFalse(filt.is_over('m1, m2 \\ #')) + self.assertFalse(filt.is_over('m1, m2 \\ # comment with \\')) + self.assertFalse(filt.is_over('\\')) + + def assert_parse(self, line, result): + self.assertEqual(tuple(self.parser.parse_line(line)), result) def test_parse_line(self): - filt = FilterMultilineFromImport('from . import (') - self.assertEqual(filt.parse_line(" a, b"), (" ", "a, b", "")) - self.assertEqual(filt.parse_line("a, b, "), ("", "a, b", ", ")) - self.assertEqual(filt.parse_line("a, b , "), ("", "a, b", " , ")) - self.assertEqual(filt.parse_line("a, b"), ("", "a, b", "")) - self.assertEqual(filt.parse_line("a, b "), ("", "a, b", " ")) - self.assertEqual(filt.parse_line("a, b,"), ("", "a, b", ",")) - self.assertEqual(filt.parse_line(" "), (" ", "", "")) - self.assertEqual(filt.parse_line("a, b)"), ("", "a, b", ")")) - self.assertEqual(filt.parse_line("a, b, ) "), ("", "a, b", ", ) ")) - self.assertEqual(filt.parse_line("a, b\\ "), ("", "a, b", "\\ ")) + self.parser = autoflake.FilterMultilineFromImport('from . import (') + self.assert_parse(' a, b', (' ', '' , 'a, b', '' , '')) # noqa + self.assert_parse('a, b, ', ('' , '' , 'a, b', ',', '')) # noqa + self.assert_parse(' ,a, b , ', (' ' , ',', 'a, b', ',', '')) # noqa + self.assert_parse('a, b', ('' , '' , 'a, b', '' , '')) # noqa + self.assert_parse('a, b ', ('' , '' , 'a, b', '' , '')) # noqa + self.assert_parse('a, b,', ('' , '' , 'a, b', ',', '')) # noqa + self.assert_parse(' ', (' ', '' , '' , '' , '')) # noqa + self.assert_parse(' (', (' ' , '' , '' , '' , '')) # noqa + self.assert_parse('a, b)', ('' , '' , 'a, b', '' , '')) # noqa + self.assert_parse('a, b, ) ', ('' , '' , 'a, b', ',', '')) # noqa + self.assert_parse('a, b\\ ', ('' , '' , 'a, b', '' , ' \\')) # noqa + self.assert_parse(' ,a, )\\', (' ' , ',', 'a' , ',', ' \\')) # noqa + self.assert_parse(' (a, )\\', (' ' , '' , 'a' , ',', ' \\')) # noqa + self.assert_parse(' \\ ', (' ' , '' , '' , '' , ' \\')) # noqa + + UNUSED = ['third_party.lib' + str(x) for x in (1, 3, 4)] + + def assert_fix(self, lines, result): + fixer = autoflake.FilterMultilineFromImport(lines[0], self.UNUSED) + fixed = functools.reduce(lambda acc, x: acc(x), lines[1:], fixer) + self.assertEqual(fixed, result) + + def test_fix(self): + # Example m0 (isort) + self.assert_fix([ + 'from third_party import (lib1, lib2, lib3,\n', + ' lib4, lib5, lib6)\n' + ], + 'from third_party import (lib2,\n' + ' lib5, lib6)\n' + ) + + # Example m1(isort) + self.assert_fix([ + 'from third_party import (lib1,\n', + ' lib2,\n', + ' lib3,\n', + ' lib4,\n', + ' lib5,\n', + ' lib6)\n' + ], + 'from third_party import (lib2,\n' + ' lib5,\n' + ' lib6)\n' + ) + + # Example m2 (isort) + self.assert_fix([ + 'from third_party import \\\n', + ' lib1, lib2, lib3, \\\n', + ' lib4, lib5, lib6\n' + ], + 'from third_party import \\\n' + ' lib2, \\\n' + ' lib5, lib6\n' + ) + + # Example m3 (isort) + self.assert_fix([ + 'from third_party import (\n', + ' lib1,\n', + ' lib2,\n', + ' lib3,\n', + ' lib4,\n', + ' lib5\n', + ')\n' + ], + 'from third_party import (\n' + ' lib2,\n' + ' lib5\n' + ')\n' + ) + + # Example m4 (isort) + self.assert_fix([ + 'from third_party import (\n', + ' lib1, lib2, lib3, lib4,\n', + ' lib5, lib6)\n' + ], + 'from third_party import (\n' + ' lib2,\n' + ' lib5, lib6)\n' + ) + + # Example m5 (isort) + self.assert_fix([ + 'from third_party import (\n', + ' lib1, lib2, lib3, lib4,\n', + ' lib5, lib6\n', + ')\n' + ], + 'from third_party import (\n' + ' lib2,\n' + ' lib5, lib6\n' + ')\n' + ) + + # Some Deviations + self.assert_fix([ + 'from third_party import ( # comment\n', + ' lib1,\\\n', # only unused + line continuation + ' lib2, \n', + ' libA\n', # used import with no commas + ' ,lib3, \n', # leading and trailing commas with unused import + ' libB, \n', + ' \\ \n', # empty line with continuation + ' lib4, # noqa \n', # unused import with comment + ')\n' + ], + 'from third_party import ( # comment\n' + ' lib2,\n' + ' libA\n' + ' ,\n' + ' libB,\n' + ' \\\n' + ' lib4, # noqa\n' + ')\n', + ) @contextlib.contextmanager From ecefaea8c2cb7ff722d3f1e3e4bc0e46b03d9cdc Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Sat, 29 Feb 2020 06:42:16 +0000 Subject: [PATCH 04/13] Add fix for multiline imports without from --- autoflake.py | 15 +++++++------ test_autoflake.py | 54 ++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 58 insertions(+), 11 deletions(-) diff --git a/autoflake.py b/autoflake.py index 1f47888..986fc04 100755 --- a/autoflake.py +++ b/autoflake.py @@ -302,8 +302,8 @@ def _valid_char_in_line(char, line): return valid_char_in_line -class FilterMultilineFromImport(PendingFix): - IMPORT_RE = re.compile(r'\s*\bimport\b\s*') +class FilterMultilineImport(PendingFix): + IMPORT_RE = re.compile(r'\bimport\b\s*') INDENTATION_RE = re.compile(r'^\s*') HANGING_LPAR_RE = re.compile(r'^\s*\(\s*(#.*)?$') BASE_RE = re.compile(r'\bfrom\s+([^ ]+)') @@ -316,7 +316,8 @@ def __init__(self, line, unused_module=()): self.unused = unused_module self.parentesized = '(' in line self.from_, imports = self.IMPORT_RE.split(line, maxsplit=1) - self.base = self.BASE_RE.search(self.from_).group(1) + match = self.BASE_RE.search(self.from_) + self.base = match.group(1) if match else None PendingFix.__init__(self, imports) def is_over(self, line): @@ -398,7 +399,7 @@ def fix(self, accumulated): if ')' not in code: code = code + ')' - return self.from_ + ' import ' + code + ending + return self.from_ + 'import ' + code + ending def __call__(self, line): self.accumulator.append(line) @@ -408,10 +409,12 @@ def __call__(self, line): return self.fix(self.accumulator) -def _filter_imports(imports, base_module, unused_module): +def _filter_imports(imports, base_module=None, unused_module=()): # We compare full module name (``a.module`` not `module`) to # guarantee the exact same module as detected from pyflakes. - return [x for x in imports if base_module + '.' + x not in unused_module] + def full_name(name): + return name if base_module is None else base_module + '.' + name + return [x for x in imports if full_name(x) not in unused_module] def filter_from_import(line, unused_module): diff --git a/test_autoflake.py b/test_autoflake.py index 92baad5..745342a 100755 --- a/test_autoflake.py +++ b/test_autoflake.py @@ -1627,7 +1627,7 @@ def test_end_to_end_with_error(self): class MultilineFromImportTests(unittest.TestCase): def test_is_over(self): - filt = autoflake.FilterMultilineFromImport('from . import (') + filt = autoflake.FilterMultilineImport('from . import (') self.assertTrue(filt.is_over('module)')) self.assertTrue(filt.is_over(' )')) self.assertTrue(filt.is_over(' ) # comment')) @@ -1636,7 +1636,7 @@ def test_is_over(self): self.assertFalse(filt.is_over('module, \\')) self.assertFalse(filt.is_over('')) - filt = autoflake.FilterMultilineFromImport('from . import module, \\') + filt = autoflake.FilterMultilineImport('from . import module, \\') self.assertTrue(filt.is_over('module')) self.assertTrue(filt.is_over('')) self.assertTrue(filt.is_over('m1, m2 # comment with \\')) @@ -1649,7 +1649,7 @@ def assert_parse(self, line, result): self.assertEqual(tuple(self.parser.parse_line(line)), result) def test_parse_line(self): - self.parser = autoflake.FilterMultilineFromImport('from . import (') + self.parser = autoflake.FilterMultilineImport('from . import (') self.assert_parse(' a, b', (' ', '' , 'a, b', '' , '')) # noqa self.assert_parse('a, b, ', ('' , '' , 'a, b', ',', '')) # noqa self.assert_parse(' ,a, b , ', (' ' , ',', 'a, b', ',', '')) # noqa @@ -1665,14 +1665,16 @@ def test_parse_line(self): self.assert_parse(' (a, )\\', (' ' , '' , 'a' , ',', ' \\')) # noqa self.assert_parse(' \\ ', (' ' , '' , '' , '' , ' \\')) # noqa - UNUSED = ['third_party.lib' + str(x) for x in (1, 3, 4)] + unused = () def assert_fix(self, lines, result): - fixer = autoflake.FilterMultilineFromImport(lines[0], self.UNUSED) + fixer = autoflake.FilterMultilineImport(lines[0], self.unused) fixed = functools.reduce(lambda acc, x: acc(x), lines[1:], fixer) self.assertEqual(fixed, result) def test_fix(self): + self.unused = ['third_party.lib' + str(x) for x in (1, 3, 4)] + # Example m0 (isort) self.assert_fix([ 'from third_party import (lib1, lib2, lib3,\n', @@ -1769,6 +1771,48 @@ def test_fix(self): ')\n', ) + self.unused = ['lib' + str(x) for x in (1, 3, 4)] + + # Multiline but not "from" + self.assert_fix([ + 'import \\\n', + ' lib1, lib2, lib3, \\\n', + ' lib4, lib5, lib6\n' + ], + 'import \\\n' + ' lib2, \\\n' + ' lib5, lib6\n' + ) + self.assert_fix([ + 'import lib1, lib2, lib3, \\\n', + ' lib4, lib5, lib6\n' + ], + 'import lib2, \\\n' + ' lib5, lib6\n' + ) + + # Problematic example without "from" + self.assert_fix([ + 'import \\\n', + ' lib1,\\\n', + ' lib2, \\\n', + ' libA\\\n', # used import with no commas + ' ,lib3, \\\n', # leading and trailing commas with unused + ' libB, \\\n', + ' \\ \n', # empty line with continuation + ' lib4 # noqa \\\n' # unused import with comment + '\n' + ], + 'import \\\n' + ' lib2, \\\n' + ' libA \\\n' + ' , \\\n' + ' libB, \\\n' + ' \\\n' + ' lib4 # noqa\n' + '\n' + ) + @contextlib.contextmanager def temporary_file(contents, directory='.', suffix='.py', prefix=''): From 8603988bcd067c6d0f9be444c5468a78543b3a99 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Sat, 29 Feb 2020 06:59:42 +0000 Subject: [PATCH 05/13] Add fix for multiline relative imports --- autoflake.py | 7 +++++-- test_autoflake.py | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/autoflake.py b/autoflake.py index 986fc04..5b65bef 100755 --- a/autoflake.py +++ b/autoflake.py @@ -409,11 +409,14 @@ def __call__(self, line): return self.fix(self.accumulator) -def _filter_imports(imports, base_module=None, unused_module=()): +def _filter_imports(imports, parent=None, unused_module=()): # We compare full module name (``a.module`` not `module`) to # guarantee the exact same module as detected from pyflakes. + sep = '' if parent and parent[0] == '.' else '.' + def full_name(name): - return name if base_module is None else base_module + '.' + name + return name if parent is None else parent + sep + name + return [x for x in imports if full_name(x) not in unused_module] diff --git a/test_autoflake.py b/test_autoflake.py index 745342a..8b11fed 100755 --- a/test_autoflake.py +++ b/test_autoflake.py @@ -1771,6 +1771,45 @@ def test_fix(self): ')\n', ) + def test_fix_relative(self): + # Example m0 (isort) + self.unused = ['.lib' + str(x) for x in (1, 3, 4)] + self.assert_fix([ + 'from . import (lib1, lib2, lib3,\n', + ' lib4, lib5, lib6)\n' + ], + 'from . import (lib2,\n' + ' lib5, lib6)\n' + ) + + # Example m1(isort) + self.unused = ['..lib' + str(x) for x in (1, 3, 4)] + self.assert_fix([ + 'from .. import (lib1,\n', + ' lib2,\n', + ' lib3,\n', + ' lib4,\n', + ' lib5,\n', + ' lib6)\n' + ], + 'from .. import (lib2,\n' + ' lib5,\n' + ' lib6)\n' + ) + + # Example m2 (isort) + self.unused = ['...lib' + str(x) for x in (1, 3, 4)] + self.assert_fix([ + 'from ... import \\\n', + ' lib1, lib2, lib3, \\\n', + ' lib4, lib5, lib6\n' + ], + 'from ... import \\\n' + ' lib2, \\\n' + ' lib5, lib6\n' + ) + + def test_fix_without_from(self): self.unused = ['lib' + str(x) for x in (1, 3, 4)] # Multiline but not "from" From 0ab76cc071390efadb7eb66c7a06b703099a58a1 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Sun, 1 Mar 2020 20:11:56 +0000 Subject: [PATCH 06/13] Fix other edge cases of multiline import Such as: - leading/trailing commas/line continuation - try/except - semicolon - empty imports as result In some cases the approach of "not changing anything if it is too risk" was adopted (as already happens in other parts of the code). --- autoflake.py | 85 ++++++++++++++++++----- test_autoflake.py | 169 +++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 227 insertions(+), 27 deletions(-) diff --git a/autoflake.py b/autoflake.py index 5b65bef..27b82da 100755 --- a/autoflake.py +++ b/autoflake.py @@ -32,6 +32,7 @@ import distutils.sysconfig import fnmatch import io +import itertools import os import re import signal @@ -251,10 +252,6 @@ def multiline_import(line, previous_line=''): if symbol in line: return True - # Ignore doctests. - if line.lstrip().startswith('>'): - return True - return multiline_statement(line, previous_line) @@ -302,6 +299,26 @@ def _valid_char_in_line(char, line): return valid_char_in_line +def _fix_leading_comma(text): + r"""Fix the situation ``\\\s*,`` or ``\(\s*,`` (+consider comments)""" + # Calculate indentation absorbing the leading ( or \into it + content = text.lstrip(string.whitespace + '\\(') + indent = text.replace(content, '') + + if content.startswith('#'): + # Pop comment from content into `indent` + lines = content.splitlines(keepends=True) + indent += lines[0] # <-- comment + content = content.replace(lines[0], '') + # Pop indentation from the second line into `indent` + orig_content = content + content = orig_content.lstrip() + indent += orig_content.replace(content, '') + + content = content.lstrip(string.whitespace + ',') + return indent + content + + class FilterMultilineImport(PendingFix): IMPORT_RE = re.compile(r'\bimport\b\s*') INDENTATION_RE = re.compile(r'^\s*') @@ -312,21 +329,36 @@ class FilterMultilineImport(PendingFix): 'indentation leading_comma imports ' 'trailing_comma line_continuation') - def __init__(self, line, unused_module=()): + def __init__(self, line, previous_line='', unused_module=()): self.unused = unused_module - self.parentesized = '(' in line + self.parenthesized = '(' in line self.from_, imports = self.IMPORT_RE.split(line, maxsplit=1) match = self.BASE_RE.search(self.from_) self.base = match.group(1) if match else None + self.give_up = False + + if '\\' in previous_line: + # Ignore things like "try: \ import" ... + self.give_up = True + + self.analyze(line) + PendingFix.__init__(self, imports) - def is_over(self, line): + def is_over(self, line=None): """Returns True if the multiline import statement is over""" - if self.parentesized: + if line is None: + line = self.accumulator[-1] + + if self.parenthesized: return _valid_char_in_line(')', line) return not _valid_char_in_line('\\', line) + def analyze(self, line): + if any(ch in line for ch in ';:'): + self.give_up = True + def parse_line(self, line): """Break the line in the different components""" match = self.INDENTATION_RE.search(line) @@ -377,6 +409,7 @@ def fix_line(self, line): parsed.line_continuation + ending) def fix(self, accumulated): + """Given a collection of accumulated lines, fix the entire import.""" new_lines = collections.deque() for line in accumulated: # Don't change if the line contains a comment or is empty except @@ -390,21 +423,33 @@ def fix(self, accumulated): else: new_lines.append(self.fix_line(line)) - code = ''.join(x for x in new_lines if x) # Filter None out + code = ''.join(x for x in new_lines if x) # Filter out 'None's + code = _fix_leading_comma(code) ending = get_line_ending(code).lstrip(' \t') - code = code.rstrip(string.whitespace + '\\') # Avoid extra \ - if self.parentesized: + code = code.rstrip(string.whitespace + '\\,') # Avoid trailing \ , + if self.parenthesized: # Add parenthesis if needed if '(' not in code: - code = '(' + code.lstrip(' \t') + code = '(' + code.lstrip(' \t,') if ')' not in code: code = code + ')' + code += ending - return self.from_ + 'import ' + code + ending + # Replace empty imports with a pass + empty = len(code.strip(string.whitespace + '\\()')) < 1 + if empty: + indentation = self.INDENTATION_RE.search(self.from_).group(0) + return indentation + 'pass' + ending - def __call__(self, line): - self.accumulator.append(line) + return self.from_ + 'import ' + code + + def __call__(self, line=None): + if line: + self.accumulator.append(line) + self.analyze(line) if not self.is_over(line): return self + if self.give_up: + return self.from_ + 'import ' + ''.join(self.accumulator) return self.fix(self.accumulator) @@ -412,7 +457,7 @@ def __call__(self, line): def _filter_imports(imports, parent=None, unused_module=()): # We compare full module name (``a.module`` not `module`) to # guarantee the exact same module as detected from pyflakes. - sep = '' if parent and parent[0] == '.' else '.' + sep = '' if parent and parent[-1] == '.' else '.' def full_name(name): return name if parent is None else parent + sep + name @@ -579,9 +624,15 @@ def filter_star_import(line, marked_star_import_undefined_name): def filter_unused_import(line, unused_module, remove_all_unused_imports, imports, previous_line=''): """Return line if used, otherwise return None.""" - if multiline_import(line, previous_line): + # Ignore doctests. + if line.lstrip().startswith('>'): return line + if multiline_import(line, previous_line): + filt = FilterMultilineImport(line, previous_line, unused_module) + return filt() + # TODO consider the other params: remove_all_unused_imports, imports + is_from_import = line.lstrip().startswith('from') if ',' in line and not is_from_import: diff --git a/test_autoflake.py b/test_autoflake.py index 8b11fed..7246920 100755 --- a/test_autoflake.py +++ b/test_autoflake.py @@ -423,13 +423,12 @@ def test_filter_from_import_remove_all(self): unused_module=['foo.abc', 'foo.subprocess', 'foo.math'])) - def test_filter_code_should_ignore_multiline_imports(self): + def test_filter_code_multiline_imports(self): self.assertEqual( r"""\ import os pass -import os, \ - math, subprocess +import os os.foo() """, ''.join(autoflake.filter_code(r"""\ @@ -438,6 +437,39 @@ def test_filter_code_should_ignore_multiline_imports(self): import os, \ math, subprocess os.foo() +"""))) + + def test_filter_code_multiline_from_imports(self): + self.assertEqual( + r"""\ +import os +pass +from os.path import ( + join, +) +join('a', 'b') +pass +os.foo() +from os.path import \ + isdir +isdir('42') +""", + ''.join(autoflake.filter_code(r"""\ +import os +import re +from os.path import ( + exists, + join, +) +join('a', 'b') +from os.path import \ + abspath, basename, \ + commonpath +os.foo() +from os.path import \ + isfile \ + , isdir +isdir('42') """))) def test_filter_code_should_ignore_semicolons(self): @@ -1645,6 +1677,11 @@ def test_is_over(self): self.assertFalse(filt.is_over('m1, m2 \\ # comment with \\')) self.assertFalse(filt.is_over('\\')) + # "Multiline" imports that are not really multiline + filt = autoflake.FilterMultilineImport('import os; ' + 'import math, subprocess') + self.assertTrue(filt.is_over()) + def assert_parse(self, line, result): self.assertEqual(tuple(self.parser.parse_line(line)), result) @@ -1668,7 +1705,8 @@ def test_parse_line(self): unused = () def assert_fix(self, lines, result): - fixer = autoflake.FilterMultilineImport(lines[0], self.unused) + fixer = autoflake.FilterMultilineImport(lines[0], + unused_module=self.unused) fixed = functools.reduce(lambda acc, x: acc(x), lines[1:], fixer) self.assertEqual(fixed, result) @@ -1698,6 +1736,20 @@ def test_fix(self): ' lib6)\n' ) + # Variation m1(isort) + self.assert_fix([ + 'from third_party import (lib1\n', + ' ,lib2\n', + ' ,lib3\n', + ' ,lib4\n', + ' ,lib5\n', + ' ,lib6)\n' + ], + 'from third_party import (lib2\n' + ' , lib5\n' + ' , lib6)\n' + ) + # Example m2 (isort) self.assert_fix([ 'from third_party import \\\n', @@ -1752,8 +1804,8 @@ def test_fix(self): # Some Deviations self.assert_fix([ 'from third_party import ( # comment\n', - ' lib1,\\\n', # only unused + line continuation - ' lib2, \n', + ' lib1\\\n', # only unused + line continuation + ' ,lib2, \n', ' libA\n', # used import with no commas ' ,lib3, \n', # leading and trailing commas with unused import ' libB, \n', @@ -1771,6 +1823,30 @@ def test_fix(self): ')\n', ) + def test_indentation(self): + # Some weird indentation examples + self.unused = ['third_party.lib' + str(x) for x in (1, 3, 4)] + self.assert_fix([ + ' from third_party import (\n', + ' lib1, lib2, lib3, lib4,\n', + ' lib5, lib6\n', + ')\n' + ], + ' from third_party import (\n' + ' lib2,\n' + ' lib5, lib6\n' + ')\n' + ) + self.assert_fix([ + '\tfrom third_party import \\\n', + '\t\tlib1, lib2, lib3, \\\n', + '\t\tlib4, lib5, lib6\n' + ], + '\tfrom third_party import \\\n' + '\t\tlib2, \\\n' + '\t\tlib5, lib6\n' + ) + def test_fix_relative(self): # Example m0 (isort) self.unused = ['.lib' + str(x) for x in (1, 3, 4)] @@ -1809,18 +1885,35 @@ def test_fix_relative(self): ' lib5, lib6\n' ) + # Example m3 (isort) + self.unused = ['.parent.lib' + str(x) for x in (1, 3, 4)] + self.assert_fix([ + 'from .parent import (\n', + ' lib1,\n', + ' lib2,\n', + ' lib3,\n', + ' lib4,\n', + ' lib5\n', + ')\n' + ], + 'from .parent import (\n' + ' lib2,\n' + ' lib5\n' + ')\n' + ) + def test_fix_without_from(self): self.unused = ['lib' + str(x) for x in (1, 3, 4)] # Multiline but not "from" self.assert_fix([ 'import \\\n', - ' lib1, lib2, lib3, \\\n', - ' lib4, lib5, lib6\n' + ' lib1, lib2, lib3 \\\n', + ' ,lib4, lib5, lib6\n' ], 'import \\\n' - ' lib2, \\\n' - ' lib5, lib6\n' + ' lib2 \\\n' + ' , lib5, lib6\n' ) self.assert_fix([ 'import lib1, lib2, lib3, \\\n', @@ -1852,6 +1945,62 @@ def test_fix_without_from(self): '\n' ) + def test_give_up_on_semicolon(self): + self.unused = ['lib' + str(x) for x in (1, 3, 4)] + self.assert_fix([ + 'import \\\n', + ' lib1, lib2, lib3, \\\n', + ' lib4, lib5; import lib6\n' + ], + 'import \\\n' + ' lib1, lib2, lib3, \\\n' + ' lib4, lib5; import lib6\n' + ) + self.unused = ['.lib' + str(x) for x in (1, 3, 4)] + self.assert_fix([ + 'from . import ( # comment\n', + ' lib1,\\\n', # only unused + line continuation + ' lib2, \n', + ' libA\n', # used import with no commas + ' ,lib3, \n', # leading and trailing commas with unused import + ' libB, \n', + ' \\ \n', # empty line with continuation + ' lib4, # noqa \n', # unused import with comment + ') ; import sys\n' + ], + 'from . import ( # comment\n' + ' lib1,\\\n' + ' lib2, \n' + ' libA\n' + ' ,lib3, \n' + ' libB, \n' + ' \\ \n' + ' lib4, # noqa \n' + ') ; import sys\n' + ) + + def test_no_empty_imports(self): + self.unused = ['lib' + str(x) for x in (1, 3, 4)] + self.assert_fix([ + 'import \\\n', + ' lib1, lib3, \\\n', + ' lib4 \n' + ], + 'pass\n' + ) + + # Indented parenthesized block + self.unused = ['.parent.lib' + str(x) for x in (1, 3, 4)] + self.assert_fix([ + '\t\tfrom .parent import (\n', + ' lib1,\n', + ' lib3,\n', + ' lib4,\n', + ')\n' + ], + '\t\tpass\n' + ) + @contextlib.contextmanager def temporary_file(contents, directory='.', suffix='.py', prefix=''): From 15e10443e65cc2893a5b02f2154e649b73ab6de7 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Sun, 1 Mar 2020 23:48:56 +0000 Subject: [PATCH 07/13] Deal with CLI options in FilterMultilineImport --- autoflake.py | 32 ++++++++++++++++++++++++-------- test_autoflake.py | 47 ++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 68 insertions(+), 11 deletions(-) diff --git a/autoflake.py b/autoflake.py index 27b82da..69b91e8 100755 --- a/autoflake.py +++ b/autoflake.py @@ -319,6 +319,13 @@ def _fix_leading_comma(text): return indent + content +def _top_module(module_name): + """Return the name of the top level module in the hierarchy""" + if module_name[0] == '.': + return '%LOCAL_MODULE%' + return module_name.split('.')[0] + + class FilterMultilineImport(PendingFix): IMPORT_RE = re.compile(r'\bimport\b\s*') INDENTATION_RE = re.compile(r'^\s*') @@ -329,16 +336,26 @@ class FilterMultilineImport(PendingFix): 'indentation leading_comma imports ' 'trailing_comma line_continuation') - def __init__(self, line, previous_line='', unused_module=()): - self.unused = unused_module + def __init__(self, line, unused_module=(), remove_all_unused_imports=False, + safe_to_remove=SAFE_IMPORTS, previous_line=''): + self.remove = unused_module self.parenthesized = '(' in line self.from_, imports = self.IMPORT_RE.split(line, maxsplit=1) match = self.BASE_RE.search(self.from_) self.base = match.group(1) if match else None self.give_up = False + if not remove_all_unused_imports: + if self.base and _top_module(self.base) not in safe_to_remove: + self.give_up = True + else: + self.remove = [ + x for x in unused_module + if _top_module(x) in safe_to_remove + ] + if '\\' in previous_line: - # Ignore things like "try: \ import" ... + # Ignore tricky things like "try: \ import" ... self.give_up = True self.analyze(line) @@ -386,7 +403,7 @@ def fix_line(self, line): # Do nothing if the line does not contain any actual import return line imports = [x.strip() for x in parsed.imports.split(',')] - clean_imports = _filter_imports(imports, self.base, self.unused) + clean_imports = _filter_imports(imports, self.base, self.remove) ending = get_line_ending(line).lstrip(' \t') if not clean_imports: @@ -477,9 +494,6 @@ def filter_from_import(line, unused_module): string=indentation).group(1) imports = re.split(pattern=r'\s*,\s*', string=imports.strip()) - - # We compare full module name (``a.module`` not `module`) to - # guarantee the exact same module as detected from pyflakes. filtered_imports = _filter_imports(imports, base_module, unused_module) # All of the import in this statement is unused @@ -629,7 +643,9 @@ def filter_unused_import(line, unused_module, remove_all_unused_imports, return line if multiline_import(line, previous_line): - filt = FilterMultilineImport(line, previous_line, unused_module) + filt = FilterMultilineImport(line, unused_module, + remove_all_unused_imports, + imports, previous_line) return filt() # TODO consider the other params: remove_all_unused_imports, imports diff --git a/test_autoflake.py b/test_autoflake.py index 7246920..0828661 100755 --- a/test_autoflake.py +++ b/test_autoflake.py @@ -1704,9 +1704,12 @@ def test_parse_line(self): unused = () - def assert_fix(self, lines, result): - fixer = autoflake.FilterMultilineImport(lines[0], - unused_module=self.unused) + def assert_fix(self, lines, result, remove_all=True): + fixer = autoflake.FilterMultilineImport( + lines[0], + remove_all_unused_imports=remove_all, + unused_module=self.unused + ) fixed = functools.reduce(lambda acc, x: acc(x), lines[1:], fixer) self.assertEqual(fixed, result) @@ -2001,6 +2004,44 @@ def test_no_empty_imports(self): '\t\tpass\n' ) + def test_without_remove_all(self): + self.unused = ['lib' + str(x) for x in (1, 3, 4)] + self.assert_fix([ + 'import \\\n', + ' lib1,\\\n', + ' lib3,\\\n', + ' lib4\n', + ], + 'import \\\n' + ' lib1, \\\n' + ' lib3, \\\n' + ' lib4\n', + remove_all=False + ) + + self.unused += ['os.path.' + x for x in ('dirname', 'isdir', 'join')] + self.assert_fix([ + 'from os.path import (\n', + ' dirname,\n', + ' isdir,\n', + ' join,\n', + ')\n' + ], + 'pass\n', + remove_all=False + ) + self.assert_fix([ + 'import \\\n', + ' os.path.dirname, \\\n', + ' lib1, \\\n', + ' lib3\n', + ], + 'import \\\n' + ' lib1, \\\n' + ' lib3\n', + remove_all=False + ) + @contextlib.contextmanager def temporary_file(contents, directory='.', suffix='.py', prefix=''): From e1808f176f0222f105995402356f20a10415efcf Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Mon, 2 Mar 2020 00:46:13 +0000 Subject: [PATCH 08/13] Simplify multiline imports by ignoring comments Unfortunately treating inline comments inside imports increases the complexity in the implementation. This change reduces complexity by refusing to change multiline import statements that contain comments. This approach is also used in other parts of the code. --- autoflake.py | 66 ++++++++++++++++------------------------------- test_autoflake.py | 47 ++++++++++++++++----------------- 2 files changed, 44 insertions(+), 69 deletions(-) diff --git a/autoflake.py b/autoflake.py index 69b91e8..b3e4f36 100755 --- a/autoflake.py +++ b/autoflake.py @@ -304,17 +304,7 @@ def _fix_leading_comma(text): # Calculate indentation absorbing the leading ( or \into it content = text.lstrip(string.whitespace + '\\(') indent = text.replace(content, '') - - if content.startswith('#'): - # Pop comment from content into `indent` - lines = content.splitlines(keepends=True) - indent += lines[0] # <-- comment - content = content.replace(lines[0], '') - # Pop indentation from the second line into `indent` - orig_content = content - content = orig_content.lstrip() - indent += orig_content.replace(content, '') - + # Now it is safe to remove the leading comma and the following whitespace content = content.lstrip(string.whitespace + ',') return indent + content @@ -326,10 +316,14 @@ def _top_module(module_name): return module_name.split('.')[0] +def _modules_to_remove(unused_modules, safe_to_remove=SAFE_IMPORTS): + """Discard unused modules that are not safe to remove from the list""" + return [x for x in unused_modules if _top_module(x) in safe_to_remove] + + class FilterMultilineImport(PendingFix): IMPORT_RE = re.compile(r'\bimport\b\s*') INDENTATION_RE = re.compile(r'^\s*') - HANGING_LPAR_RE = re.compile(r'^\s*\(\s*(#.*)?$') BASE_RE = re.compile(r'\bfrom\s+([^ ]+)') Parsed = collections.namedtuple('Parsed', @@ -349,10 +343,7 @@ def __init__(self, line, unused_module=(), remove_all_unused_imports=False, if self.base and _top_module(self.base) not in safe_to_remove: self.give_up = True else: - self.remove = [ - x for x in unused_module - if _top_module(x) in safe_to_remove - ] + self.remove = _modules_to_remove(self.remove, safe_to_remove) if '\\' in previous_line: # Ignore tricky things like "try: \ import" ... @@ -364,8 +355,7 @@ def __init__(self, line, unused_module=(), remove_all_unused_imports=False, def is_over(self, line=None): """Returns True if the multiline import statement is over""" - if line is None: - line = self.accumulator[-1] + line = line or self.accumulator[-1] if self.parenthesized: return _valid_char_in_line(')', line) @@ -373,7 +363,7 @@ def is_over(self, line=None): return not _valid_char_in_line('\\', line) def analyze(self, line): - if any(ch in line for ch in ';:'): + if any(ch in line for ch in ';:#'): self.give_up = True def parse_line(self, line): @@ -404,7 +394,7 @@ def fix_line(self, line): return line imports = [x.strip() for x in parsed.imports.split(',')] clean_imports = _filter_imports(imports, self.base, self.remove) - ending = get_line_ending(line).lstrip(' \t') + ending = get_line_ending(line).strip(' \t') if not clean_imports: # Even when there is no import left, a single comma might still be @@ -427,37 +417,25 @@ def fix_line(self, line): def fix(self, accumulated): """Given a collection of accumulated lines, fix the entire import.""" - new_lines = collections.deque() - for line in accumulated: - # Don't change if the line contains a comment or is empty except - # for trailing characters (but preserve indentation) - comment = '#' in line - without_trailing = line.rstrip(string.whitespace + '()\\') - empty = len(without_trailing.strip()) < 1 - if comment or empty: - ending = get_line_ending(line).lstrip(' \t') - new_lines.append(line.rstrip() + ending) - else: - new_lines.append(self.fix_line(line)) - - code = ''.join(x for x in new_lines if x) # Filter out 'None's - code = _fix_leading_comma(code) - ending = get_line_ending(code).lstrip(' \t') - code = code.rstrip(string.whitespace + '\\,') # Avoid trailing \ , + new_lines = [self.fix_line(x) for x in accumulated] + imports = ''.join(x for x in new_lines if x) # Filter out 'None's + imports = _fix_leading_comma(imports) + ending = get_line_ending(imports).strip(' \t') + imports = imports.rstrip(string.whitespace + '\\,') # Trailing \ , if self.parenthesized: # Add parenthesis if needed - if '(' not in code: - code = '(' + code.lstrip(' \t,') - if ')' not in code: - code = code + ')' - code += ending + if '(' not in imports: + imports = '(' + imports.lstrip(' \t,') + if ')' not in imports: + imports = imports + ')' + imports += ending # Replace empty imports with a pass - empty = len(code.strip(string.whitespace + '\\()')) < 1 + empty = len(imports.strip(string.whitespace + '\\()')) < 1 if empty: indentation = self.INDENTATION_RE.search(self.from_).group(0) return indentation + 'pass' + ending - return self.from_ + 'import ' + code + return self.from_ + 'import ' + imports def __call__(self, line=None): if line: diff --git a/test_autoflake.py b/test_autoflake.py index 0828661..f7c0048 100755 --- a/test_autoflake.py +++ b/test_autoflake.py @@ -1659,23 +1659,23 @@ def test_end_to_end_with_error(self): class MultilineFromImportTests(unittest.TestCase): def test_is_over(self): - filt = autoflake.FilterMultilineImport('from . import (') - self.assertTrue(filt.is_over('module)')) - self.assertTrue(filt.is_over(' )')) - self.assertTrue(filt.is_over(' ) # comment')) + filt = autoflake.FilterMultilineImport('from . import (\n') + self.assertTrue(filt.is_over('module)\n')) + self.assertTrue(filt.is_over(' )\n')) + self.assertTrue(filt.is_over(' ) # comment\n')) self.assertFalse(filt.is_over('# )')) - self.assertFalse(filt.is_over('module')) - self.assertFalse(filt.is_over('module, \\')) - self.assertFalse(filt.is_over('')) - - filt = autoflake.FilterMultilineImport('from . import module, \\') - self.assertTrue(filt.is_over('module')) - self.assertTrue(filt.is_over('')) - self.assertTrue(filt.is_over('m1, m2 # comment with \\')) - self.assertFalse(filt.is_over('m1, m2 \\')) - self.assertFalse(filt.is_over('m1, m2 \\ #')) - self.assertFalse(filt.is_over('m1, m2 \\ # comment with \\')) - self.assertFalse(filt.is_over('\\')) + self.assertFalse(filt.is_over('module\n')) + self.assertFalse(filt.is_over('module, \\\n')) + self.assertFalse(filt.is_over('\n')) + + filt = autoflake.FilterMultilineImport('from . import module, \\\n') + self.assertTrue(filt.is_over('module\n')) + self.assertTrue(filt.is_over('\n')) + self.assertTrue(filt.is_over('m1, m2 # comment with \\\n')) + self.assertFalse(filt.is_over('m1, m2 \\\n')) + self.assertFalse(filt.is_over('m1, m2 \\ #\n')) + self.assertFalse(filt.is_over('m1, m2 \\ # comment with \\\n')) + self.assertFalse(filt.is_over('\\\n')) # "Multiline" imports that are not really multiline filt = autoflake.FilterMultilineImport('import os; ' @@ -1806,23 +1806,22 @@ def test_fix(self): # Some Deviations self.assert_fix([ - 'from third_party import ( # comment\n', + 'from third_party import (\n', ' lib1\\\n', # only unused + line continuation ' ,lib2, \n', ' libA\n', # used import with no commas ' ,lib3, \n', # leading and trailing commas with unused import ' libB, \n', - ' \\ \n', # empty line with continuation - ' lib4, # noqa \n', # unused import with comment + ' \\\n', # empty line with continuation + ' lib4,\n', # unused import with comment ')\n' ], - 'from third_party import ( # comment\n' + 'from third_party import (\n' ' lib2,\n' ' libA\n' ' ,\n' ' libB,\n' ' \\\n' - ' lib4, # noqa\n' ')\n', ) @@ -1935,16 +1934,14 @@ def test_fix_without_from(self): ' ,lib3, \\\n', # leading and trailing commas with unused ' libB, \\\n', ' \\ \n', # empty line with continuation - ' lib4 # noqa \\\n' # unused import with comment + ' lib4\\\n', # unused import with comment '\n' ], 'import \\\n' ' lib2, \\\n' ' libA \\\n' ' , \\\n' - ' libB, \\\n' - ' \\\n' - ' lib4 # noqa\n' + ' libB\n' '\n' ) From 416382000df53e6370c17e61aa587c58ca482e0e Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Mon, 2 Mar 2020 04:12:37 +0000 Subject: [PATCH 09/13] Simplify logic in multiline imports ... By using the existing code as a template. --- autoflake.py | 124 ++++++++++++++------------------------ test_autoflake.py | 148 +++++++++++++++++++++++++++++----------------- 2 files changed, 140 insertions(+), 132 deletions(-) diff --git a/autoflake.py b/autoflake.py index b3e4f36..ebfc141 100755 --- a/autoflake.py +++ b/autoflake.py @@ -299,16 +299,6 @@ def _valid_char_in_line(char, line): return valid_char_in_line -def _fix_leading_comma(text): - r"""Fix the situation ``\\\s*,`` or ``\(\s*,`` (+consider comments)""" - # Calculate indentation absorbing the leading ( or \into it - content = text.lstrip(string.whitespace + '\\(') - indent = text.replace(content, '') - # Now it is safe to remove the leading comma and the following whitespace - content = content.lstrip(string.whitespace + ',') - return indent + content - - def _top_module(module_name): """Return the name of the top level module in the hierarchy""" if module_name[0] == '.': @@ -321,14 +311,26 @@ def _modules_to_remove(unused_modules, safe_to_remove=SAFE_IMPORTS): return [x for x in unused_modules if _top_module(x) in safe_to_remove] +def _segment_module(segment): + """Extract the module identifier inside the segment. + + It might be the case the segment does not have a module (e.g. is composed + just by a parenthesis or line continuation and whitespace). In this + scenario we just keep the segment... These characters are not valid in + identifiers, so they will never be contained in the list of unused modules + anyway. + """ + return segment.strip(string.whitespace + ',\\()') or segment + + class FilterMultilineImport(PendingFix): IMPORT_RE = re.compile(r'\bimport\b\s*') INDENTATION_RE = re.compile(r'^\s*') BASE_RE = re.compile(r'\bfrom\s+([^ ]+)') - - Parsed = collections.namedtuple('Parsed', - 'indentation leading_comma imports ' - 'trailing_comma line_continuation') + SEGMENT_RE = re.compile( + r'([^,\s]+(?:[\s\\]+as[\s\\]+[^,\s]+)?[,\s\\)]*)', re.M) + # ^ module + comma + following space (including new line and continuation) + IDENTIFIER_RE = re.compile(r'[^,\s]+') def __init__(self, line, unused_module=(), remove_all_unused_imports=False, safe_to_remove=SAFE_IMPORTS, previous_line=''): @@ -366,76 +368,40 @@ def analyze(self, line): if any(ch in line for ch in ';:#'): self.give_up = True - def parse_line(self, line): - """Break the line in the different components""" - match = self.INDENTATION_RE.search(line) - indentation = match.group(0) - # Remove the extra space (indentation already stored) so we can detect - # leading commas and line continuation - imports = line.strip() - leading_comma = ',' if imports and imports[0] == ',' else '' - line_continuation = ' \\' if imports and imports[-1] == '\\' else '' - - # Remove the line continuation (already stored) and hanging pars so we - # can detect trailing commas - imports = imports.strip(string.whitespace + '()\\') - trailing_comma = ',' if imports and imports[-1] == ',' else '' - - # Finally remove the remaining trailing/leading chars - imports = imports.strip(string.whitespace + ',') - - return self.Parsed(indentation, leading_comma, imports, - trailing_comma, line_continuation) - - def fix_line(self, line): - parsed = self.parse_line(line) - if not parsed.imports: - # Do nothing if the line does not contain any actual import - return line - imports = [x.strip() for x in parsed.imports.split(',')] - clean_imports = _filter_imports(imports, self.base, self.remove) - ending = get_line_ending(line).strip(' \t') - - if not clean_imports: - # Even when there is no import left, a single comma might still be - # necessary - if parsed.leading_comma and parsed.trailing_comma: - return (parsed.indentation + parsed.leading_comma + - parsed.line_continuation + ending) - else: - # The line can be removed - return None - - if parsed.leading_comma: - # By inserting an empty element at the beginning of the list we - # force a trailing comma correctly separated by a space - clean_imports.insert(0, '') - import_str = ', '.join(clean_imports) - - return (parsed.indentation + import_str + parsed.trailing_comma + - parsed.line_continuation + ending) - def fix(self, accumulated): """Given a collection of accumulated lines, fix the entire import.""" - new_lines = [self.fix_line(x) for x in accumulated] - imports = ''.join(x for x in new_lines if x) # Filter out 'None's - imports = _fix_leading_comma(imports) - ending = get_line_ending(imports).strip(' \t') - imports = imports.rstrip(string.whitespace + '\\,') # Trailing \ , - if self.parenthesized: # Add parenthesis if needed - if '(' not in imports: - imports = '(' + imports.lstrip(' \t,') - if ')' not in imports: - imports = imports + ')' - imports += ending - - # Replace empty imports with a pass - empty = len(imports.strip(string.whitespace + '\\()')) < 1 + + old_imports = ''.join(accumulated) + # Split imports into segments that contain the module name + + # comma + whitespace and eventual \ ( ) chars + segments = [x for x in self.SEGMENT_RE.findall(old_imports) if x] + modules = [_segment_module(x) for x in segments] + keep = _filter_imports(modules, self.base, self.remove) + + # Short-circuit if no import was discarded + if len(keep) == len(segments): + return self.from_ + 'import ' + ''.join(accumulated) + + # Since it is very difficult to deal with all the line breaks and + # continuations, let's use the code layout that already exists and + # just replace the module identifiers inside the first N-1 segments + # + the last segment + templates = list(zip(modules, segments)) + templates = templates[:len(keep)-1] + templates[-1:] + # It is important to keep the last segment, since it might contain + # important chars like `)` + new_imports = ''.join( + template.replace(module, keep[i]) + for i, (module, template) in enumerate(templates) + ) + + # Replace empty imports with a "pass" statement + empty = len(new_imports.strip(string.whitespace + '\\(),')) < 1 if empty: indentation = self.INDENTATION_RE.search(self.from_).group(0) - return indentation + 'pass' + ending + return indentation + 'pass' + get_line_ending(old_imports) - return self.from_ + 'import ' + imports + return self.from_ + 'import ' + new_imports def __call__(self, line=None): if line: diff --git a/test_autoflake.py b/test_autoflake.py index f7c0048..6594259 100755 --- a/test_autoflake.py +++ b/test_autoflake.py @@ -1682,26 +1682,6 @@ def test_is_over(self): 'import math, subprocess') self.assertTrue(filt.is_over()) - def assert_parse(self, line, result): - self.assertEqual(tuple(self.parser.parse_line(line)), result) - - def test_parse_line(self): - self.parser = autoflake.FilterMultilineImport('from . import (') - self.assert_parse(' a, b', (' ', '' , 'a, b', '' , '')) # noqa - self.assert_parse('a, b, ', ('' , '' , 'a, b', ',', '')) # noqa - self.assert_parse(' ,a, b , ', (' ' , ',', 'a, b', ',', '')) # noqa - self.assert_parse('a, b', ('' , '' , 'a, b', '' , '')) # noqa - self.assert_parse('a, b ', ('' , '' , 'a, b', '' , '')) # noqa - self.assert_parse('a, b,', ('' , '' , 'a, b', ',', '')) # noqa - self.assert_parse(' ', (' ', '' , '' , '' , '')) # noqa - self.assert_parse(' (', (' ' , '' , '' , '' , '')) # noqa - self.assert_parse('a, b)', ('' , '' , 'a, b', '' , '')) # noqa - self.assert_parse('a, b, ) ', ('' , '' , 'a, b', ',', '')) # noqa - self.assert_parse('a, b\\ ', ('' , '' , 'a, b', '' , ' \\')) # noqa - self.assert_parse(' ,a, )\\', (' ' , ',', 'a' , ',', ' \\')) # noqa - self.assert_parse(' (a, )\\', (' ' , '' , 'a' , ',', ' \\')) # noqa - self.assert_parse(' \\ ', (' ' , '' , '' , '' , ' \\')) # noqa - unused = () def assert_fix(self, lines, result, remove_all=True): @@ -1721,8 +1701,7 @@ def test_fix(self): 'from third_party import (lib1, lib2, lib3,\n', ' lib4, lib5, lib6)\n' ], - 'from third_party import (lib2,\n' - ' lib5, lib6)\n' + 'from third_party import (lib2, lib5, lib6)\n' ) # Example m1(isort) @@ -1749,8 +1728,8 @@ def test_fix(self): ' ,lib6)\n' ], 'from third_party import (lib2\n' - ' , lib5\n' - ' , lib6)\n' + ' ,lib5\n' + ' ,lib6)\n' ) # Example m2 (isort) @@ -1760,8 +1739,7 @@ def test_fix(self): ' lib4, lib5, lib6\n' ], 'from third_party import \\\n' - ' lib2, \\\n' - ' lib5, lib6\n' + ' lib2, lib5, lib6\n' ) # Example m3 (isort) @@ -1787,8 +1765,7 @@ def test_fix(self): ' lib5, lib6)\n' ], 'from third_party import (\n' - ' lib2,\n' - ' lib5, lib6)\n' + ' lib2, lib5, lib6)\n' ) # Example m5 (isort) @@ -1799,8 +1776,7 @@ def test_fix(self): ')\n' ], 'from third_party import (\n' - ' lib2,\n' - ' lib5, lib6\n' + ' lib2, lib5, lib6\n' ')\n' ) @@ -1817,14 +1793,52 @@ def test_fix(self): ')\n' ], 'from third_party import (\n' - ' lib2,\n' - ' libA\n' - ' ,\n' + ' lib2\\\n' + ' ,libA, \n' ' libB,\n' - ' \\\n' ')\n', ) + self.assert_fix([ + 'from third_party import (\n', + ' lib1\n', + ',\n', + ' lib2\n', + ',\n', + ' lib3\n', + ',\n', + ' lib4\n', + ',\n', + ' lib5\n', + ')\n' + ], + 'from third_party import (\n' + ' lib2\n' + ',\n' + ' lib5\n' + ')\n' + ) + + self.assert_fix([ + 'from third_party import (\n', + ' lib1 \\\n', + ', \\\n', + ' lib2 \\\n', + ',\\\n', + ' lib3\n', + ',\n', + ' lib4\n', + ',\n', + ' lib5 \\\n', + ')\n' + ], + 'from third_party import (\n' + ' lib2 \\\n' + ', \\\n' + ' lib5 \\\n' + ')\n' + ) + def test_indentation(self): # Some weird indentation examples self.unused = ['third_party.lib' + str(x) for x in (1, 3, 4)] @@ -1835,8 +1849,7 @@ def test_indentation(self): ')\n' ], ' from third_party import (\n' - ' lib2,\n' - ' lib5, lib6\n' + ' lib2, lib5, lib6\n' ')\n' ) self.assert_fix([ @@ -1845,8 +1858,7 @@ def test_indentation(self): '\t\tlib4, lib5, lib6\n' ], '\tfrom third_party import \\\n' - '\t\tlib2, \\\n' - '\t\tlib5, lib6\n' + '\t\tlib2, lib5, lib6\n' ) def test_fix_relative(self): @@ -1856,8 +1868,7 @@ def test_fix_relative(self): 'from . import (lib1, lib2, lib3,\n', ' lib4, lib5, lib6)\n' ], - 'from . import (lib2,\n' - ' lib5, lib6)\n' + 'from . import (lib2, lib5, lib6)\n' ) # Example m1(isort) @@ -1883,8 +1894,7 @@ def test_fix_relative(self): ' lib4, lib5, lib6\n' ], 'from ... import \\\n' - ' lib2, \\\n' - ' lib5, lib6\n' + ' lib2, lib5, lib6\n' ) # Example m3 (isort) @@ -1914,15 +1924,13 @@ def test_fix_without_from(self): ' ,lib4, lib5, lib6\n' ], 'import \\\n' - ' lib2 \\\n' - ' , lib5, lib6\n' + ' lib2, lib5, lib6\n' ) self.assert_fix([ 'import lib1, lib2, lib3, \\\n', ' lib4, lib5, lib6\n' ], - 'import lib2, \\\n' - ' lib5, lib6\n' + 'import lib2, lib5, lib6\n' ) # Problematic example without "from" @@ -1938,13 +1946,31 @@ def test_fix_without_from(self): '\n' ], 'import \\\n' - ' lib2, \\\n' - ' libA \\\n' - ' , \\\n' - ' libB\n' + ' lib2,\\\n' + ' libA, \\\n' + ' libB\\\n' '\n' ) + self.unused = ['lib{}.x.y.z'.format(x) for x in (1, 3, 4)] + self.assert_fix([ + 'import \\\n', + ' lib1.x.y.z \\', + ' , \\\n', + ' lib2.x.y.z \\\n', + ' , \\\n', + ' lib3.x.y.z \\\n', + ' , \\\n', + ' lib4.x.y.z \\\n', + ' , \\\n', + ' lib5.x.y.z\n' + ], + 'import \\\n' + ' lib2.x.y.z \\' + ' , \\\n' + ' lib5.x.y.z\n' + ) + def test_give_up_on_semicolon(self): self.unused = ['lib' + str(x) for x in (1, 3, 4)] self.assert_fix([ @@ -1979,6 +2005,22 @@ def test_give_up_on_semicolon(self): ') ; import sys\n' ) + def test_just_one_import(self): + self.unused = ['lib2'] + self.assert_fix([ + 'import \\\n', + ' lib1\n' + ], + 'import \\\n' + ' lib1\n' + ) + self.assert_fix([ + 'import \\\n', + ' lib2\n' + ], + 'pass\n' + ) + def test_no_empty_imports(self): self.unused = ['lib' + str(x) for x in (1, 3, 4)] self.assert_fix([ @@ -1986,7 +2028,7 @@ def test_no_empty_imports(self): ' lib1, lib3, \\\n', ' lib4 \n' ], - 'pass\n' + 'pass \n' ) # Indented parenthesized block @@ -2010,8 +2052,8 @@ def test_without_remove_all(self): ' lib4\n', ], 'import \\\n' - ' lib1, \\\n' - ' lib3, \\\n' + ' lib1,\\\n' + ' lib3,\\\n' ' lib4\n', remove_all=False ) From 5a1bd3e4ec5617c0f29ef353f74f6afa4e1ef015 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Mon, 2 Mar 2020 06:07:20 +0000 Subject: [PATCH 10/13] Fix docstrings according to pydocstyle --- autoflake.py | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/autoflake.py b/autoflake.py index ebfc141..35dcf87 100755 --- a/autoflake.py +++ b/autoflake.py @@ -276,7 +276,9 @@ class PendingFix(object): ``PendingFix`` object instead of a string, this object will be called with the following line. """ + def __init__(self, line): + """Analyse and store the first line.""" self.accumulator = collections.deque([line]) def __call__(self, line): @@ -289,7 +291,7 @@ def __call__(self, line): def _valid_char_in_line(char, line): - """Returns True if a char appears in the line and is not commented""" + """Return True if a char appears in the line and is not commented.""" comment_index = line.find('#') char_index = line.find(char) valid_char_in_line = ( @@ -300,14 +302,14 @@ def _valid_char_in_line(char, line): def _top_module(module_name): - """Return the name of the top level module in the hierarchy""" + """Return the name of the top level module in the hierarchy.""" if module_name[0] == '.': return '%LOCAL_MODULE%' return module_name.split('.')[0] def _modules_to_remove(unused_modules, safe_to_remove=SAFE_IMPORTS): - """Discard unused modules that are not safe to remove from the list""" + """Discard unused modules that are not safe to remove from the list.""" return [x for x in unused_modules if _top_module(x) in safe_to_remove] @@ -324,6 +326,14 @@ def _segment_module(segment): class FilterMultilineImport(PendingFix): + """Remove unused imports from multiline import statements. + + This class handles both the cases: "from imports" and "direct imports". + + Some limitations exist (e.g. imports with comments, lines joined by ``;``, + etc). In these cases, the statement is left unchanged to avoid problems. + """ + IMPORT_RE = re.compile(r'\bimport\b\s*') INDENTATION_RE = re.compile(r'^\s*') BASE_RE = re.compile(r'\bfrom\s+([^ ]+)') @@ -334,6 +344,7 @@ class FilterMultilineImport(PendingFix): def __init__(self, line, unused_module=(), remove_all_unused_imports=False, safe_to_remove=SAFE_IMPORTS, previous_line=''): + """Receive the same parameters as ``filter_unused_import``.""" self.remove = unused_module self.parenthesized = '(' in line self.from_, imports = self.IMPORT_RE.split(line, maxsplit=1) @@ -356,7 +367,7 @@ def __init__(self, line, unused_module=(), remove_all_unused_imports=False, PendingFix.__init__(self, imports) def is_over(self, line=None): - """Returns True if the multiline import statement is over""" + """Return True if the multiline import statement is over.""" line = line or self.accumulator[-1] if self.parenthesized: @@ -365,12 +376,12 @@ def is_over(self, line=None): return not _valid_char_in_line('\\', line) def analyze(self, line): + """Decide if the statement will be fixed or left unchanged.""" if any(ch in line for ch in ';:#'): self.give_up = True def fix(self, accumulated): """Given a collection of accumulated lines, fix the entire import.""" - old_imports = ''.join(accumulated) # Split imports into segments that contain the module name + # comma + whitespace and eventual \ ( ) chars @@ -404,6 +415,7 @@ def fix(self, accumulated): return self.from_ + 'import ' + new_imports def __call__(self, line=None): + """Accumulate all the lines in the import and then trigger the fix.""" if line: self.accumulator.append(line) self.analyze(line) From 5a5acc6590d48c37181b506554da8a8575065d2a Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Mon, 2 Mar 2020 06:11:13 +0000 Subject: [PATCH 11/13] Remove unused itertools --- autoflake.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/autoflake.py b/autoflake.py index 35dcf87..47bd242 100755 --- a/autoflake.py +++ b/autoflake.py @@ -32,7 +32,6 @@ import distutils.sysconfig import fnmatch import io -import itertools import os import re import signal @@ -564,8 +563,8 @@ def filter_code(source, additional_imports=None, result = filter_unused_variable(line) elif line_number in marked_key_line_numbers: result = filter_duplicate_key(line, line_messages[line_number], - line_number, marked_key_line_numbers, - source) + line_number, marked_key_line_numbers, + source) elif line_number in marked_star_import_line_numbers: result = filter_star_import(line, undefined_names) else: From d86860d5a5f2e17bab85f017fd7a9ce5a72b254f Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Mon, 2 Mar 2020 06:15:58 +0000 Subject: [PATCH 12/13] Remove solved TODO comment --- autoflake.py | 1 - 1 file changed, 1 deletion(-) diff --git a/autoflake.py b/autoflake.py index 47bd242..cc80bcd 100755 --- a/autoflake.py +++ b/autoflake.py @@ -602,7 +602,6 @@ def filter_unused_import(line, unused_module, remove_all_unused_imports, remove_all_unused_imports, imports, previous_line) return filt() - # TODO consider the other params: remove_all_unused_imports, imports is_from_import = line.lstrip().startswith('from') From 06431f6b0c9a1ac5dd44f499b96d91bb3c5348dc Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Mon, 2 Mar 2020 12:19:49 +0000 Subject: [PATCH 13/13] Add examples from issue #8 and fix edge cases --- autoflake.py | 37 +++++++++++++++++++++--------------- test_autoflake.py | 48 ++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 67 insertions(+), 18 deletions(-) diff --git a/autoflake.py b/autoflake.py index cc80bcd..c384930 100755 --- a/autoflake.py +++ b/autoflake.py @@ -382,6 +382,7 @@ def analyze(self, line): def fix(self, accumulated): """Given a collection of accumulated lines, fix the entire import.""" old_imports = ''.join(accumulated) + ending = get_line_ending(old_imports) # Split imports into segments that contain the module name + # comma + whitespace and eventual \ ( ) chars segments = [x for x in self.SEGMENT_RE.findall(old_imports) if x] @@ -392,26 +393,32 @@ def fix(self, accumulated): if len(keep) == len(segments): return self.from_ + 'import ' + ''.join(accumulated) - # Since it is very difficult to deal with all the line breaks and - # continuations, let's use the code layout that already exists and - # just replace the module identifiers inside the first N-1 segments - # + the last segment - templates = list(zip(modules, segments)) - templates = templates[:len(keep)-1] + templates[-1:] - # It is important to keep the last segment, since it might contain - # important chars like `)` - new_imports = ''.join( - template.replace(module, keep[i]) - for i, (module, template) in enumerate(templates) - ) + fixed = '' + if keep: + # Since it is very difficult to deal with all the line breaks and + # continuations, let's use the code layout that already exists and + # just replace the module identifiers inside the first N-1 segments + # + the last segment + templates = list(zip(modules, segments)) + templates = templates[:len(keep)-1] + templates[-1:] + # It is important to keep the last segment, since it might contain + # important chars like `)` + fixed = ''.join( + template.replace(module, keep[i]) + for i, (module, template) in enumerate(templates) + ) + + # Fix the edge case: inline parenthesis + just one surviving import + if self.parenthesized and any(ch not in fixed for ch in '()'): + fixed = fixed.strip(string.whitespace + '()') + ending # Replace empty imports with a "pass" statement - empty = len(new_imports.strip(string.whitespace + '\\(),')) < 1 + empty = len(fixed.strip(string.whitespace + '\\(),')) < 1 if empty: indentation = self.INDENTATION_RE.search(self.from_).group(0) - return indentation + 'pass' + get_line_ending(old_imports) + return indentation + 'pass' + ending - return self.from_ + 'import ' + new_imports + return self.from_ + 'import ' + fixed def __call__(self, line=None): """Accumulate all the lines in the import and then trigger the fix.""" diff --git a/test_autoflake.py b/test_autoflake.py index 6594259..cd279a2 100755 --- a/test_autoflake.py +++ b/test_autoflake.py @@ -1663,6 +1663,7 @@ def test_is_over(self): self.assertTrue(filt.is_over('module)\n')) self.assertTrue(filt.is_over(' )\n')) self.assertTrue(filt.is_over(' ) # comment\n')) + self.assertTrue(filt.is_over('from module import (a, b)\n')) self.assertFalse(filt.is_over('# )')) self.assertFalse(filt.is_over('module\n')) self.assertFalse(filt.is_over('module, \\\n')) @@ -1690,7 +1691,7 @@ def assert_fix(self, lines, result, remove_all=True): remove_all_unused_imports=remove_all, unused_module=self.unused ) - fixed = functools.reduce(lambda acc, x: acc(x), lines[1:], fixer) + fixed = functools.reduce(lambda acc, x: acc(x), lines[1:], fixer()) self.assertEqual(fixed, result) def test_fix(self): @@ -1971,7 +1972,8 @@ def test_fix_without_from(self): ' lib5.x.y.z\n' ) - def test_give_up_on_semicolon(self): + def test_give_up(self): + # Semicolon self.unused = ['lib' + str(x) for x in (1, 3, 4)] self.assert_fix([ 'import \\\n', @@ -1982,6 +1984,7 @@ def test_give_up_on_semicolon(self): ' lib1, lib2, lib3, \\\n' ' lib4, lib5; import lib6\n' ) + # Comments self.unused = ['.lib' + str(x) for x in (1, 3, 4)] self.assert_fix([ 'from . import ( # comment\n', @@ -2005,7 +2008,7 @@ def test_give_up_on_semicolon(self): ') ; import sys\n' ) - def test_just_one_import(self): + def test_just_one_import_used(self): self.unused = ['lib2'] self.assert_fix([ 'import \\\n', @@ -2020,6 +2023,45 @@ def test_just_one_import(self): ], 'pass\n' ) + # Example from issue #8 + self.unused = ['re.subn'] + self.assert_fix([ + '\tfrom re import (subn)\n', + ], + '\tpass\n', + ) + + def test_just_one_import_left(self): + # Examples from issue #8 + self.unused = ['math.sqrt'] + self.assert_fix([ + 'from math import (\n', + ' sqrt,\n', + ' log\n', + ' )\n' + ], + 'from math import (\n' + ' log\n' + ' )\n' + ) + self.unused = ['module.b'] + self.assert_fix([ + 'from module import (a, b)\n', + ], + 'from module import a\n', + ) + self.assert_fix([ + 'from module import (a,\n', + ' b)\n', + ], + 'from module import a\n', + ) + self.unused = [] + self.assert_fix([ + 'from re import (subn)\n', + ], + 'from re import (subn)\n', + ) def test_no_empty_imports(self): self.unused = ['lib' + str(x) for x in (1, 3, 4)]