Tokenization: improve EOF and multi-line statement interaction

[jayaddison]

This is the kind of change that can cause subtle problems in future, so it's one that should be reviewed and considered carefully.

Currently during fuzz testing, hypothesmith is able to find some example cases where apparently-valid Python code that contains a multi-line indicator (\ character) immediately prior to the EOF indicator (without a newline) fails during black's tokenization stage.

This has been reported in #1012 and more recently #1960.

As per the notes in #1012, using code of this kind as input will successfully compile and run in Python.

This changeset permits the black tokenizer to continue when an EOF token is encountered following a multi-line indicator.

Resolves #1012.

[jayaddison]

Maybe worth a little more pause here; looking at the current upstream blib2to3, this TokenError exception is still raised when an EOF is encountered during a continued statement (ref).

[jayaddison]

Eh, ok - I'm going to mark this as ready for review again, having read up a bit more on the status of blib2to3 via bugs.python.org and understanding that black's vendored version has already diverged a little bit. Still feels maybe non-ideal, but I don't have any other changes planned here.

[ichard26]

Eh, ok - I'm going to mark this as ready for review again, having read up a bit more on the status of blib2to3 via bugs.python.org and understanding that black's vendored version has already diverged a little bit. Still feels maybe non-ideal, but I don't have any other changes planned here.

We've modified our version of lib2to3 a lot already so this isn't a big deal.

[ichard26]

Actually within a minute I created something that seems to hang Black?

test = (1
        +
        2

This is definitely a no-no even if it's invalid code (EOF in multi-line continuation) IMO.

[ichard26]

Yeah, this seems to be hanging up tokenize.py generate_tokens... cProfile-ing Black on that file and killing it after 30ish seconds of nothing leads to this (partial) call graph:

edit: you can get something similar via

$ python -m cProfile -o out.pstats -m black test.py
$ gprof2dot out.pstats -z __init__:366:main -n0.01 -e0.01 | dot -Tsvg -o out.svg  # `gprof2dot` is from the `yelp-gprof2dot` PyPI package

[jayaddison]

Thanks @ichard26 - I'll take a look into that.

[jayaddison]

The latest pushed changes may appear to work based on the test coverage, but in fact there are code output equivalence issues. The following input file (with no closing end-of-line) produces a different abstract syntax tree during the second formatter pass:

def foo(a):
    pass\

(it works with the --fast CLI flag provided, since that skips the second pass)

[jayaddison]

I think this is closable as a wont-fix; I'll add some more explanation soon in the linked issue.

[jayaddison]

(in hindsight, assuming a wont-fix resolution wasn't sensible of me; this could still be addressed if there's appetite to)

[jayaddison]

(just cleaning up and closing some old / stale pull requests that I've been working on. this turned out to be a more-involved fix than it looked like originally; some adjustments were made to the hyptothesmith config in #1998 to mitigate the test failures that were occurring as a result)

[ichard26]

@jayaddison and we really appreciate it btw! We want to refactor the single-file Black package into a multi-module package to improve maintainability (see GH-1746). Unfortunately such a change would break basically every PR, so we're working on reducing the PR backlog.