JBKahn / flake8-print

flake8
MIT License
119 stars 22 forks source link

Fix reading from a closed stdin buffer #36

Closed seporaitis closed 4 years ago

seporaitis commented 5 years ago

Summary: Couple of flake8 plugins rely on pycodestyle.stdin_get_value, which does not cache the string, after reading it from stdin and closing the buffer. This means that if there are two plugins using this function - the second plugin relying on this function, cannot read from stdin, because it was closed, and ValueError: I/O operation on closed file gets raised.

Fix: use flake8.utils.stdin_get_value, which caches the stdin, so that multiple plugins can use it. It is based on flake8-commas that does not seem to have this issue.

Steps to reproduce:

requirements.txt:

flake8==3.7.6
flake8-tuple==0.2.13
flake8-print==3.1.0

Run:

python -m flake8 - < path/to/python/file.py

Expected output: lint successfully. Actual output:

Traceback (most recent call last):
  File "/usr/local/Cellar/python/3.7.2_2/Frameworks/Python.framework/Versions/3.7/lib/python3.7/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/usr/local/Cellar/python/3.7.2_2/Frameworks/Python.framework/Versions/3.7/lib/python3.7/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/Users/julius/Work/workenv/lib/python3.7/site-packages/flake8/__main__.py", line 4, in <module>
    cli.main()
  File "/Users/julius/Work/workenv/lib/python3.7/site-packages/flake8/main/cli.py", line 18, in main
    app.run(argv)
  File "/Users/julius/Work/workenv/lib/python3.7/site-packages/flake8/main/application.py", line 394, in run
    self._run(argv)
  File "/Users/julius/Work/workenv/lib/python3.7/site-packages/flake8/main/application.py", line 382, in _run
    self.run_checks()
  File "/Users/julius/Work/workenv/lib/python3.7/site-packages/flake8/main/application.py", line 301, in run_checks
    self.file_checker_manager.run()
  File "/Users/julius/Work/workenv/lib/python3.7/site-packages/flake8/checker.py", line 330, in run
    self.run_serial()
  File "/Users/julius/Work/workenv/lib/python3.7/site-packages/flake8/checker.py", line 314, in run_serial
    checker.run_checks()
  File "/Users/julius/Work/workenv/lib/python3.7/site-packages/flake8/checker.py", line 608, in run_checks
    self.run_ast_checks()
  File "/Users/julius/Work/workenv/lib/python3.7/site-packages/flake8/checker.py", line 504, in run_ast_checks
    for (line_number, offset, text, check) in runner:
  File "/Users/julius/Work/workenv/lib/python3.7/site-packages/flake8_tuple.py", line 48, in run
    lines = get_lines(self.filename)
  File "/Users/julius/Work/workenv/lib/python3.7/site-packages/flake8_tuple.py", line 33, in get_lines
    return pep8.stdin_get_value().splitlines(True)
  File "/Users/julius/Work/workenv/lib/python3.7/site-packages/pycodestyle.py", line 1726, in stdin_get_value
    return TextIOWrapper(sys.stdin.buffer, errors='ignore').read()
ValueError: I/O operation on closed file

Additional comments:

In my environment this error got triggered in flake8-print and flake8-tuple. I am simultaneously doing a similar fix to flake8-tuple as well. However, there must be a third plugin that uses pycodestyle.stdin_get_value first time. I will investigate and do a similar fix as well.

I bumped version up as it would be great to release this fix quickly - it would save my eyes from emacs+flycheck blowing up with an error taking half the buffer on every file save. :)

Let me know if I can improve this to expedite the release.

Thank you.

adamchainz commented 5 years ago

Presumably the try/except ImportError is due to the function moving between different versions of flake8?

seporaitis commented 5 years ago

If understand correctly this pull request on flake8-commas the answer is yes.

seporaitis commented 5 years ago

@JBKahn is there anything I could do to improve this PR so it is easier for you to merge it?

seporaitis commented 5 years ago

@JBKahn ping

nndii commented 5 years ago

@JBKahn ping

JBKahn commented 4 years ago

Sorry it took so long but this didn't pass the tests and I had no time to really look at it @nndii . I have to at least ensure tests pass before merging.

In this case, the tests weren't passing with older versions of flake8 that I'm willing to simply stop supporting. Thank you for your patience and thanks for the PR @seporaitis