PyCQA / redbaron

Bottom-up approach to refactoring in python
http://redbaron.pycqa.org/
691 stars 74 forks source link

Crash when no new line at the end of file. #128

Open kayhayen opened 7 years ago

kayhayen commented 7 years ago

This is from CPython 2.7 test suite, "test_posix.py", the markup won't allow me to put the precise code here, but for this example:

if __name__ == '__main__':
    test_main()

If you remove the final new line from a file, the baron cannot strip it anymore and says:

baron.parser.ParsingError: Error, got an unexpected token $end here:

 461 
 462 
 463 def test_main():
 464     test_support.run_unittest(PosixTester, PosixGroupsTester)
 465 
 466 if __name__ == '__main__':
 467     test_main()
 468 <---- here
ibizaman commented 7 years ago

Sorry for the breaking changes.

As you said, a good thing would be use those tests for baron/redbaron directly. And why not just go to the top X pypi packages and at least test we can open all .py files there and dump them correctly.

I didn't check thoroughly yet but @kayhayen @Psycojoker do you know if we can simply copy excerpts of the CPython test files here without having license problems?

kayhayen commented 7 years ago

This is not a regression AFAIK, but not 100% sure of that. It's just what sprung into my face when I did the test with the broken version of RedBaron, that was a different one than the one related to comments after block begins.

License wise PSF license is very liberal. I think it's close to Apache 2.0 license. But excerpts would be fair use anyway, and never be subject to the need of licensing. INAL though. But seriously, those tests are intended to be used by PyPy, Jython, and even Nuitka, to test their compliance with Python language.

Psycojoker commented 7 years ago

This one is really weird, I'm not able to reproduce it. I've been trying to following on baron and redbaron (the pypi versions) without being able to raise any exception:

In [1]: from baron import parse
In [2]: from redbaron import RedBaron
In [3]: from urllib import urlopen
In [4]: text = urlopen("https://raw.githubusercontent.com/python/cpython/2.7/Lib/test/test_posix.py").read()
In [5]: RedBaron(text.rstrip())
In [6]: parse(text.rstrip())
In [7]: RedBaron(("if __name__ == '__main__':\n    test_main()"))
In [8]: parse(("if __name__ == '__main__':\n    test_main()"))

Am I missing something?

kayhayen commented 7 years ago

This is the file I am having the crash with. The modification might be from Debian source package, but it should not matter.

test_posix.zip

Please try this file instead. I needed to zip it, since Github refuses .py files.

kayhayen commented 7 years ago

In my usage of Redbaron, I have had this:

red = RedBaron(old_code.rstrip()+'\n')

That probably explains the issue with reproducing it. I think at some point, I noticed RedBaron was crashing with the last line not ending in a new line. Obviously that is me changing the input though, which explains the problem with reproducing maybe. Sorry about that. But I still think it's likely a valid issue.