Add types to definition.py in rosidl_parser

[InvincibleRMC]

Adding types to defintion.py to solve warnings raised in ros2/rosidl_python#206 and improve readability.

[fujitatomoya]

lgtm with green CI. I would like to have 2nd review.

[clalancette]

I've left one comment with a possible problem; otherwise this looks good to me.

[clalancette]

I could be wrong, but shouldn't this be:

Suggested change

	) -> List[IdlContentElementT]:
	) -> List[IdlContentElement]:

[InvincibleRMC]

Due to the isinstance check we know that they have to match type_ so using IdlContentElementT is correct.

return [e for e in self.elements if isinstance(e, type_)]

This make it so when you use get_elements_of_type it knows that returned List only has objects of type Action

foo = IdlContent().get_elements_of_type(Action)
reveal_type(foo)  # List[Action]

[clalancette]

I guess I don't understand then. This method is essentially returning a subset of self.elements, which is typed as List[IdlContentElement]. Thus it seems to me that the type of self.elements and the return type of get_elements_of_type should be the same, no?

[clalancette]

Actually, never mind that last comment. I guess I don't understand https://github.com/ros2/rosidl/pull/791/files#diff-0734e88e493f2337273d2bcfe9baee715ded7b09926541c65b53cd46bb9de41fR821-R822 ; can you explain more about that?

[InvincibleRMC]

Ok so in line 821 is a Union of all possible entries into the elements list. I got these from the extract_content_from_ast in ros_idl.parser. These are all the possible values that can go into the elements list.

[InvincibleRMC]

Line 822 is a TypeVar that is upper bound by the TypeAlias from 821. Because of the isinstance check inside the list interpolation we know that the returned List will be a subset of self.elements which all match type_. Since we know the the list in a subset of only type_ objects we can make the function generic over type_ and have the list match the type of type_. I'm sorry if my explanations are not super clear, for more information check out mypy's page about generic functions.

[sloretz]

@Mergifyio update

[mergify]

update

✅ Branch has been successfully updated

[sloretz]

Pulls: #791
Gist: https://gist.githubusercontent.com/sloretz/2c0fe5761de1071f7ded2b57f31a82d9/raw/c5235f4fdd81c7d9d5e27a042385ff870be0cee4/ros2.repos
BUILD args: --packages-above-and-dependencies rosidl_parser
TEST args: --packages-above rosidl_parser
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/14723

• Linux
• Linux-aarch64
• Linux-rhel
• Windows

[sloretz]

Thank you for the PR!