PyCQA / modernize

Modernizes Python code for eventual Python 3 migration. Built on top of fissix (a fork of lib2to3)
https://modernize.readthedocs.org/
Other
355 stars 51 forks source link

False Positive With --enforce Option? #161

Closed DavidMuller closed 4 years ago

DavidMuller commented 6 years ago

Thank you for all the hard work on this project!

I was wondering if someone can help me understand why the --enforce option is returning a non zero exit code for a file whose only contents is raise RunTimeError('foo'):

$ cat my_file.py 
raise RuntimeError('foo')
$ python-modernize my_file.py --enforce
RefactoringTool: Skipping implicit fixer: idioms
RefactoringTool: Skipping implicit fixer: set_literal
RefactoringTool: Skipping implicit fixer: ws_comma
RefactoringTool: No changes to my_file.py
RefactoringTool: Files that need to be modified:
RefactoringTool: my_file.py
$ echo $?
2 

python-modernize does not have any suggested changes to the file, but somehow the file still appears in the Files that need to be modified section and the exit code is 2?

I'm on Python 2.7.10 and modernize 0.6.

Possibly related SO question: 2to3 says "No changes needed", then "files that need to be modified".

takluyver commented 6 years ago

It does sound similar to the SO question, and it's probably a bug. I don't have time to track down where the bug is and whether it's in modernize or in 2to3, but feel free to post here if anyone can pin it down more.

larserikh commented 6 years ago
$ cat << EOF > hello.py
'hello'
EOF

$ 2to3 hello.py 
RefactoringTool: Skipping optional fixer: buffer
RefactoringTool: Skipping optional fixer: idioms
RefactoringTool: Skipping optional fixer: set_literal
RefactoringTool: Skipping optional fixer: ws_comma
RefactoringTool: No changes to hello.py
RefactoringTool: Files that need to be modified:
RefactoringTool: hello.py

$ 2to3 -x unicode hello.py 
RefactoringTool: Skipping optional fixer: buffer
RefactoringTool: Skipping optional fixer: idioms
RefactoringTool: Skipping optional fixer: set_literal
RefactoringTool: Skipping optional fixer: ws_comma
RefactoringTool: No files need to be modified.

It seems to be the unicode fixer in lib2to3 that's causing these false positives.

rouge8 commented 6 years ago

It seems like there's several fixers with this issue, including numliterals, ws_comma, and dict_six. 😕 (Tested by running python-modernize -f $fix with each individual fix over a file with false positives)

cmin764 commented 4 years ago

Any news here? Is pretty bad that we can't use that as a pre-commit hook since is returning these false-positives.

One potential workaround is to skip under hooks with -x=except,numliterals,ws_comma,dict_six and run them manually or additionally by ignoring the returned error code.

AWhetter commented 4 years ago

It seems like this issue comes from lib2to3. I have a pull request in with cpython to fix those issues. With those fixes in place I'm not seeing any modernize code that's causing this issue from my testing.

graingert commented 4 years ago

@AWhetter lib2to3 is deprecated, can you make your PR into https://github.com/jreese/fissix?

It seems like this issue comes from lib2to3. I have a pull request in with cpython to fix those issues.

graingert commented 4 years ago

@DavidMuller looks like this is fixed in pip install -U fissix "modernize>=0.8rc0"