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

Handle exception assignment to attribute (py2) #115

Merged
merged 1 commit into from
Jun 8, 2019

Conversation

lordmauve
Copy link
Contributor

Fix for a crash in a little-used piece of Python 2 syntax.

Apparently you can assign to an attribute in a Python 2 exception handler:

except Exception as foo.bar:

or (same AST)

except Exception, foo.bar:

Unfortunately pep8-naming doesn't handle this, and crashes with

...
  File "/home/mauve/dev/pep8-naming/.tox/py27/local/lib/python2.7/site-packages/pep8ext_naming.py", line 425, in visit_excepthandler
    for error in self._find_errors(node, parents, ignore):
  File "/home/mauve/dev/pep8-naming/.tox/py27/local/lib/python2.7/site-packages/pep8ext_naming.py", line 385, in _find_errors
    for name in _extract_names(assignment_target):
  File "/home/mauve/dev/pep8-naming/.tox/py27/local/lib/python2.7/site-packages/pep8ext_naming.py", line 477, in _extract_names
    yield assignment_target.name.id
AttributeError: 'Attribute' object has no attribute 'id'

(It is a SyntaxError in Python 3.)

@sigmavirus24
Copy link
Member

👍🏼

run_tests.py Outdated
@@ -99,7 +99,7 @@ def test_file(filename, lines, code, options):
for lineno, col_offset, msg, instance in checker.run():
found_errors.add(error_format(msg.split()[0], **locals()))

if not found_errors and code == 'Okay': # Expected PASS
if not found_errors and code.startswith('Okay'): # Expected PASS
Copy link
Member

Choose a reason for hiding this comment

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

This change appears to be unrelated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, yeah, I originally added a comment to the test to explain it more, but ended up taking it out

Copy link
Member

Choose a reason for hiding this comment

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

Got it. Could you remove this part of your change then?

Copy link
Member

Choose a reason for hiding this comment

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

@lordmauve, just wanted to follow up on this. Then I'm happy to merge.

@lordmauve
Copy link
Contributor Author

Hi, sorry, been on holiday. I'll do this when I get a chance

@jparise jparise merged commit bbf5723 into PyCQA:master Jun 8, 2019
@jparise
Copy link
Member

jparise commented Jun 8, 2019

Thanks!

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