Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Python 2 only syntax detection #2592

Merged
merged 3 commits into from Nov 12, 2021

Conversation

ichard26
Copy link
Collaborator

@ichard26 ichard26 commented Nov 6, 2021

Description

First of all this fixes a mistake I made in Python 2 deprecation PR
using token.* to check for print/exec statements. Turns out that
for nodes with a type value higher than 256 its numeric type isn't
guaranteed to be constant. Using syms.* instead fixes this.

Also add support for the following cases:

print "hello, world!"

exec "print('hello, world!')"

def set_position((x, y), value):
    pass

try:
    pass
except Exception, err:
    pass

raise RuntimeError, "I feel like crashing today :p"

`wow_these_really_did_exist`

10L

This should also fix the Python 2 related failures in #2586.

Checklist - did you ...

  • Add a CHANGELOG entry if necessary?
  • Add / update tests if necessary?
  • Add new / update outdated documentation? -> /na

First of all this fixes a mistake I made in Python 2 deprecation PR
using token.* to check for print/exec statements. Turns out that
for nodes with a type value higher than 256 its numeric type isn't
guaranteed to be constant. Using syms.* instead fixes this.

Also add support for the following cases:

    print "hello, world!"

    exec "print('hello, world!')"

    def set_position((x, y), value):
        pass

    try:
        pass
    except Exception, err:
        pass

    raise RuntimeError, "I feel like crashing today :p"

    `wow_these_really_did_exist`

    10L
Copy link
Collaborator

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

src/black/__init__.py Outdated Show resolved Hide resolved
src/black/__init__.py Show resolved Hide resolved
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
@JelleZijlstra
Copy link
Collaborator

Thank you!

@ichard26
Copy link
Collaborator Author

cc @Zac-HD

Traceback (most recent call last):
  File "/home/runner/work/black/black/.tox/fuzz/lib/python3.9/site-packages/hypothesmith/syntactic.py", line 110, in draw_symbol
    compile(
SystemError: Negative size passed to PyUnicode_New

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/runner/work/black/black/fuzz.py", line 73, in <module>
    test_idempotent_any_syntatically_valid_python()
  File "/home/runner/work/black/black/fuzz.py", line 20, in test_idempotent_any_syntatically_valid_python
    max_examples=1000,  # roughly 1k tests/minute, or half that under coverage
  File "/home/runner/work/black/black/.tox/fuzz/lib/python3.9/site-packages/hypothesis/core.py", line 1199, in wrapped_test
    raise the_error_hypothesis_found
  File "/home/runner/work/black/black/.tox/fuzz/lib/python3.9/site-packages/hypothesmith/syntactic.py", line 89, in do_draw
    result = super().do_draw(data)
  File "/home/runner/work/black/black/.tox/fuzz/lib/python3.9/site-packages/hypothesis/extra/lark.py", line 153, in do_draw
    self.draw_symbol(data, start, state)
  File "/home/runner/work/black/black/.tox/fuzz/lib/python3.9/site-packages/hypothesmith/syntactic.py", line 107, in draw_symbol
    super().draw_symbol(data, symbol, draw_state)
  File "/home/runner/work/black/black/.tox/fuzz/lib/python3.9/site-packages/hypothesis/extra/lark.py", line 181, in draw_symbol
    self.draw_symbol(data, e, draw_state)
  File "/home/runner/work/black/black/.tox/fuzz/lib/python3.9/site-packages/hypothesmith/syntactic.py", line 107, in draw_symbol
    super().draw_symbol(data, symbol, draw_state)
  File "/home/runner/work/black/black/.tox/fuzz/lib/python3.9/site-packages/hypothesis/extra/lark.py", line 181, in draw_symbol
    self.draw_symbol(data, e, draw_state)
  File "/home/runner/work/black/black/.tox/fuzz/lib/python3.9/site-packages/hypothesmith/syntactic.py", line 107, in draw_symbol
    super().draw_symbol(data, symbol, draw_state)
  File "/home/runner/work/black/black/.tox/fuzz/lib/python3.9/site-packages/hypothesis/extra/lark.py", line 181, in draw_symbol
    self.draw_symbol(data, e, draw_state)
  File "/home/runner/work/black/black/.tox/fuzz/lib/python3.9/site-packages/hypothesmith/syntactic.py", line 129, in draw_symbol
    raise type(err)(
SystemError: compile('delA\\ \n#\n', '<string>', 'single') raised SystemError: Negative size passed to PyUnicode_New

https://github.com/psf/black/runs/4184619816?check_suite_focus=true#step:5:85

Seems like the fuzz issue might be related to https://bugs.python.org/issue45738. Anyhow we can't do anything about it ourselves :)

@ichard26 ichard26 merged commit 0753d99 into psf:main Nov 12, 2021
@ichard26 ichard26 deleted the improve-python-2-detection branch November 12, 2021 01:28
@Zac-HD
Copy link
Contributor

Zac-HD commented Nov 12, 2021

Thanks, Zac-HD/hypothesmith#16 is open and I guess I'm adding another workaround for an upstream bug.

JelleZijlstra added a commit that referenced this pull request Nov 16, 2021
* Improve Python 2 only syntax detection

First of all this fixes a mistake I made in Python 2 deprecation PR
using token.* to check for print/exec statements. Turns out that
for nodes with a type value higher than 256 its numeric type isn't
guaranteed to be constant. Using syms.* instead fixes this.

Also add support for the following cases:

    print "hello, world!"

    exec "print('hello, world!')"

    def set_position((x, y), value):
        pass

    try:
        pass
    except Exception, err:
        pass

    raise RuntimeError, "I feel like crashing today :p"

    `wow_these_really_did_exist`

    10L

* Add octal support, more test cases, and fixup long ints

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants