PyCQA/modernize

False Positive With --enforce Option?

DavidMuller opened this issue · 7 comments

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".

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.

$ 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.

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)

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.

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.

@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.

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