Crash on functions with certain typehinting

[kmantel]

Simple typehints seem to be supported, but others cause bowler to crash. For example,

foo.py:

import typing


def foo(bar: typing.List[str]):
    pass

Running

bowler.Query('foo.py').select_function('foo').add_argument('baz', 1).diff()

Produces

Skipping foo.py: failed to transform because 'Node' object has no attribute 'value'
Traceback (most recent call last):
File "~/.pyenv/versions/pnl368/lib/python3.6/site-packages/bowler/tool.py", line 209, in refactor_queue
    hunks = self.refactor_file(filename)
File "~/.pyenv/versions/pnl368/lib/python3.6/site-packages/bowler/tool.py", line 171, in refactor_file
    tree = self.refactor_string(input, filename)
File "~/.pyenv/versions/pnl368/lib/python3.6/site-packages/fissix/refactor.py", line 377, in refactor_string
    self.refactor_tree(tree, name)
File "~/.pyenv/versions/pnl368/lib/python3.6/site-packages/fissix/refactor.py", line 417, in refactor_tree
    self.traverse_by(self.bmi_post_order_heads, tree.post_order())
File "~/.pyenv/versions/pnl368/lib/python3.6/site-packages/fissix/refactor.py", line 493, in traverse_by
    new = fixer.transform(node, results)
File "~/.pyenv/versions/pnl368/lib/python3.6/site-packages/bowler/query.py", line 962, in transform
    returned_node = callback(node, capture, filename)
File "~/.pyenv/versions/pnl368/lib/python3.6/site-packages/bowler/query.py", line 746, in add_argument_transform
    spec = FunctionSpec.build(node, capture)
File "~/.pyenv/versions/pnl368/lib/python3.6/site-packages/bowler/imr.py", line 214, in build
    arguments = FunctionArgument.build_list(args, is_def)
File "~/.pyenv/versions/pnl368/lib/python3.6/site-packages/bowler/imr.py", line 89, in build_list
    arg = cls.build(leaf, is_def)
File "~/.pyenv/versions/pnl368/lib/python3.6/site-packages/bowler/imr.py", line 50, in build
    kwargs["annotation"] = leaf.children[-1].value
AttributeError: 'Node' object has no attribute 'value'
AttributeError: 'Node' object has no attribute 'value'
<bowler.query.Query object at 0x7fe8de0d8860>

While replacing typing.List[str] with just str produces the expected result

[turbaszek]

With @olchas we encountered the same issue. How can we help to solve this bug?

[thatch]

@jreese is just checking whether leaf.children[-1] is a Leaf (->store .value) or Node (store the whole thing?) enough of a fix here, or are there downstream dependencies on this that would break. I suspect any type checking we might have is disabled by using kwargs.

[thatch]

1. Store the entire node in kwargs["annotation"] seems the way to go. There may be corresponding changes where that's pulled back out
2. Add a test case near https://github.com/facebookincubator/Bowler/blob/master/bowler/tests/query.py#L297 that includes a type hint of typing.List[str] in the first post
3. Make sure the tests pass
4. Submit a PR.

Some getting started docs are at https://github.com/facebookincubator/Bowler/blob/master/CONTRIBUTING.md

[MatthewMarroquin]

@thatch Taking a look at this; first time working with unit tests any documentation on this?

[MdAfzaalkhan]

@thatch is this still up can i submit a PR for this ?

[StunningShield4504]

For Python 3.8 or lower : That code is correct
For Python 3.9 or later :
def foo(bar: list[str]):
pass

[thatch]

@MdAfzaalkhan sure! The dotted notation is legal in 3.9+ as well, especially for third-party types.