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

Imports are always moved to top of the file #459

Closed
Tadaboody opened this issue Dec 13, 2018 · 8 comments · Fixed by #467
Closed

Imports are always moved to top of the file #459

Tadaboody opened this issue Dec 13, 2018 · 8 comments · Fixed by #467
Assignees

Comments

@Tadaboody
Copy link

Tadaboody commented Dec 13, 2018

Running autopep8 on

"""Module docstring"""
a = 5
import random
import itertools

results in

import itertools
import random
"""module docstring"""
a = 5

Instead of

"""module docstring"""
import itertools
import random
a = 5
@hhatto hhatto self-assigned this Dec 18, 2018
@Tadaboody Tadaboody changed the title Imports are moved above module docstring Imports are always moved to top of the file Dec 30, 2018
@Tadaboody
Copy link
Author

Tadaboody commented Dec 30, 2018

@hhatto Imports are also moved above __future__ imports. This creates a syntax error.

@hhatto hhatto pinned this issue Dec 30, 2018
@Hanaasagi
Copy link
Contributor

@Tadaboody. emm, I meet the same problem. Maybe this patch can solve that imports are moved above from __future__ import

def get_module_imports_on_top_of_file(source, import_line_index):
    """return import or from keyword position

    example:
      > 0: import sys
        1: import os
        2:
        3: def function():
    """
    def is_string_literal(line):
        if line[0] in 'uUbB':
            line = line[1:]
        if line and line[0] in 'rR':
            line = line[1:]
        return line and (line[0] == '"' or line[0] == "'")

    def is_future_import(line):
        nodes = ast.parse(line)
        for node in nodes.body:
            if isinstance(node, ast.ImportFrom) and node.module == '__future__':
                return True
        return False

    allowed_try_keywords = ('try', 'except', 'else', 'finally')
    for cnt, line in enumerate(source):
        if not line.rstrip():
            continue
        elif line.startswith('#'):
            continue
        if line.startswith('import ') or line.startswith('from '):
            if cnt == import_line_index or is_future_import(line):
                continue
            return cnt
        elif pycodestyle.DUNDER_REGEX.match(line):
            continue
        elif any(line.startswith(kw) for kw in allowed_try_keywords):
            continue
        elif is_string_literal(line):
            return cnt
        else:
            return cnt
    return 0

It adapts to following situation

import math
a = 1
import collections
from __future__ import print_function
import functools

But it can't solve that

import collections
from __future__ import print_function
a = 1

Because, the above code will pass the check of pycodestyle. It only checks E402(Module level import not at top of file), but pyflakes contains additional check F404(from __future__ imports must occur at the beginning of the file)

By the way, I think PEP8 is just code style, not a lint. It considers that the problem is not caused by autopep8, actually it's not a tool to fix SyntaxError.

Reference Flake8Rules

@Tadaboody
Copy link
Author

@Hanaasagi Will it fix the case of turning

from __future__ import print_function
print(3)
import functools
print(4)

into

import functools
from __future__ import print_function
print(3)
print(4)

Thus creating a syntax error by using autopep8?

@Hanaasagi
Copy link
Contributor

I think so. autopep8 depends on pycodestyle, above code will report that

t.py:3:1: E402 module level import not at top of file

And this patch using ast module to check whether the target line contains a __future__ import. if has, it will insert to next suitable position.

@waspinator
Copy link

imports are also moved above sys.path.insert(), which breaks the import statement.

@hhatto
Copy link
Owner

hhatto commented Jan 12, 2019

It is resolved in this issue:

  • Change the import statement that it does not moved above the module document by E402 (Thanks @Hanaasagi )
  • Change the import statement that it does not moved above the from __future__ import XX by E402 ( Thanks @Hanaasagi )

@Hanaasagi
Copy link
Contributor

I'd like to resolve the remianing issue.

@Hanaasagi
Copy link
Contributor

#467

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants