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

SyntaxError rewriting specific combination of imports and defs #490

Closed
asottile opened this issue Jul 11, 2019 · 4 comments · Fixed by #491
Closed

SyntaxError rewriting specific combination of imports and defs #490

asottile opened this issue Jul 11, 2019 · 4 comments · Fixed by #491

Comments

@asottile
Copy link
Contributor


Python Code

def setup():
    pass

from a import b
from b import c
from b import d

Command Line and Configuration

Command Line

$ autopep8 -i t.py
Traceback (most recent call last):
  File "/tmp/pyqtgraph/venv/bin/autopep8", line 10, in <module>
    sys.exit(main())
  File "/tmp/pyqtgraph/venv/lib/python3.6/site-packages/autopep8.py", line 4200, in main
    ret = fix_multiple_files(args.files, args, sys.stdout)
  File "/tmp/pyqtgraph/venv/lib/python3.6/site-packages/autopep8.py", line 4100, in fix_multiple_files
    ret = _fix_file((name, options, output))
  File "/tmp/pyqtgraph/venv/lib/python3.6/site-packages/autopep8.py", line 4080, in _fix_file
    return fix_file(*parameters)
  File "/tmp/pyqtgraph/venv/lib/python3.6/site-packages/autopep8.py", line 3454, in fix_file
    fixed_source = fix_lines(fixed_source, options, filename=filename)
  File "/tmp/pyqtgraph/venv/lib/python3.6/site-packages/autopep8.py", line 3434, in fix_lines
    fixed_source = fix.fix()
  File "/tmp/pyqtgraph/venv/lib/python3.6/site-packages/autopep8.py", line 593, in fix
    aggressive=self.options.aggressive))
  File "/tmp/pyqtgraph/venv/lib/python3.6/site-packages/autopep8.py", line 535, in _fix_source
    modified_lines = fix(result)
  File "/tmp/pyqtgraph/venv/lib/python3.6/site-packages/autopep8.py", line 889, in fix_e402
    line_index)
  File "/tmp/pyqtgraph/venv/lib/python3.6/site-packages/autopep8.py", line 1442, in get_module_imports_on_top_of_file
    if cnt == import_line_index or is_future_import(line):
  File "/tmp/pyqtgraph/venv/lib/python3.6/site-packages/autopep8.py", line 1412, in is_future_import
    nodes = ast.parse(line)
  File "/usr/lib/python3.6/ast.py", line 35, in parse
    return compile(source, filename, mode, PyCF_ONLY_AST)
  File "<unknown>", line 2
    def setup():
               ^
SyntaxError: unexpected EOF while parsing

Your Environment

$ python --version --version
Python 3.6.8 (default, Jan 14 2019, 11:02:34) 
[GCC 8.0.1 20180414 (experimental) [trunk revision 259383]]
$ autopep8 --version
autopep8 1.4.4 (pycodestyle: 2.5.0)

Analysis

it appears to be happening while checking for __future__ imports -- the import reordering code is combining a from import line with the def line causing the "check if this import is a __future__ import" code to parse too much

@Hanaasagi
Copy link
Contributor

line was combined in fix_402

autopep8/autopep8.py

Lines 887 to 890 in fff90d4

if not (target in self.imports and self.imports[target] != line_index):
mod_offset = get_module_imports_on_top_of_file(self.source,
line_index)
self.source[mod_offset] = line + self.source[mod_offset]

But I am not sure is a good idea to not join the line here, beacause it will cause the line number not matched with pycodestyle check result.

If I split and get first line in is_future_import, it will throw error with following code

a = 1
from csv import (
    reader,
    writer,
)
import os

Maybe we should change the way to check __future__ import, ast.parse always check the code syntax.

@Hanaasagi
Copy link
Contributor

Or we do something like

autopep8/autopep8.py

Lines 880 to 886 in fff90d4

for i in range(1, 100):
line = "".join(self.source[line_index:line_index+i])
try:
generate_tokens("".join(line))
except (SyntaxError, tokenize.TokenError):
continue
break

when meet SyntaxError, merge next line and check again until get a complete code.

@asottile
Copy link
Contributor Author

this also works:

diff --git a/autopep8.py b/autopep8.py
index 01b90d6..05b818c 100755
--- a/autopep8.py
+++ b/autopep8.py
@@ -1409,7 +1409,10 @@ def get_module_imports_on_top_of_file(source, import_line_index):
         return line and (line[0] == '"' or line[0] == "'")
 
     def is_future_import(line):
-        nodes = ast.parse(line)
+        try:
+            nodes = ast.parse(line)
+        except SyntaxError:
+            return False
         for n in nodes.body:
             if isinstance(n, ast.ImportFrom) and n.module == '__future__':
                 return True

it assumes that a moved import is never a __future__ import (it would already be a SyntaxError if it were)

@Hanaasagi
Copy link
Contributor

There some more complicated case, like

from __future__ import (
    absolute_import,
    print_function,
)
a = 1
import os

from __future__ import ( will also cause a SyntaxError.
I'm sorry this problem is due to #461 , I will fix it in the last few days..

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.

2 participants