Skip to content

Handle missing branches in BinOp#392

Merged
agronholm merged 3 commits intoagronholm:masterfrom
vthemelis:invalid-bin-op
Sep 3, 2023
Merged

Handle missing branches in BinOp#392
agronholm merged 3 commits intoagronholm:masterfrom
vthemelis:invalid-bin-op

Conversation

@vthemelis
Copy link
Copy Markdown
Contributor

@vthemelis vthemelis commented Aug 28, 2023

Fixes #384 .

The issue is due to the left or right attributes of BinOp in the case that the AnnotationTransformer evaluates any of them to be None.

The code that removes the attributes completely is:
https://github.com/python/cpython/blob/e407cea1938b80b1d469f148a4ea65587820e3eb/Lib/ast.py#L493-L498

This patch removes the crush but I think shows another bug where in the test I added, the argument A is expected to only have a value of None. NB that this is not just wrong but also inconsistent with what happens when using Union instead of |

@vthemelis vthemelis force-pushed the invalid-bin-op branch 2 times, most recently from c006bf4 to 0238b69 Compare August 28, 2023 22:15
@coveralls
Copy link
Copy Markdown

coveralls commented Aug 28, 2023

Coverage Status

coverage: 97.878% (+0.005%) from 97.873% when pulling 97ff305 on vthemelis:invalid-bin-op into 0c118de on agronholm:master.

Comment thread tests/test_transformer.py
@vthemelis vthemelis force-pushed the invalid-bin-op branch 2 times, most recently from 5b5f91f to 3110a50 Compare September 3, 2023 11:56
@vthemelis vthemelis changed the title Trigger the BinOp crush Handle missing branches in BinOp Sep 3, 2023
Comment thread docs/versionhistory.rst Outdated
Comment thread docs/versionhistory.rst Outdated
Comment thread src/typeguard/_transformer.py Outdated
@agronholm agronholm merged commit 041b0e3 into agronholm:master Sep 3, 2023
@agronholm
Copy link
Copy Markdown
Owner

Thanks!

@vthemelis vthemelis deleted the invalid-bin-op branch September 4, 2023 15:23
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.

Possible 4.1.2 regression: AttributeError: 'BinOp' object has no attribute 'left'

3 participants