(python): CDK has incorrect Python type signatures

rectalogic commented 3 years ago

Most of the CDK type signatures use List instead of Sequence. List is invariant http://mypy.readthedocs.io/en/latest/common_issues.html#variance

So this means we can't do something simple like pass a list of ec2.CfnSecurityGroup.IngressProperty to a ec2.CfnSecurityGroup without mypy errors.

Reproduction Steps

Run mypy on this code fragment:

from aws_cdk import (
    core,
    aws_ec2 as ec2,
)

stack = core.Stack()
ingress_rules = [
    ec2.CfnSecurityGroup.IngressProperty(
        from_port=22,
        to_port=22,
        ip_protocol="tcp",
        cidr_ip="0.0.0.0/0",
    )
]
ec2.CfnSecurityGroup(
    stack,
    "SecurityGroup",
    group_description="Sample SG",
    security_group_ingress=ingress_rules,
)

What did you expect to happen?

mypy should succeed

What actually happened?

mypy failed

$ ../../env/bin/mypy /tmp/invariant.py
/tmp/invariant.py:19: error: Argument "security_group_ingress" to "CfnSecurityGroup" has incompatible type "List[IngressProperty]"; expected "Union[IResolvable, List[Union[IResolvable, IngressProperty]], None]"
/tmp/invariant.py:19: note: "List" is invariant -- see http://mypy.readthedocs.io/en/latest/common_issues.html#variance
/tmp/invariant.py:19: note: Consider using "Sequence" instead, which is covariant
Found 1 error in 1 file (checked 1 source file)

Environment

CDK CLI Version : 1.88.0 (build f65009b)
Framework Version: 1.90.1
Node.js Version: v14.15.0
OS : macOS 10.15.7
Language (Version): Python 3.8.7, mypy 0.812

Other

In this case security_group_ingress is declared in the cdk as

security_group_ingress: typing.Optional[typing.Union[aws_cdk.core.IResolvable, typing.List[typing.Union[aws_cdk.core.IResolvable, "CfnSecurityGroup.IngressProperty"]]]] = None

Since List is invariant, we must pass exactly this, so a list of CfnSecurityGroup.IngressProperty is not valid. This declaration should use Sequence instead of List and this is true of many cdk type signatures.

This is :bug: Bug Report

MrArnoldPalmer commented 3 years ago

@RomainMuller wondering what you think about this? This would be a breaking change for jsii packages targeting python. I do agree that the list invariability makes cdk usage annoying in a lot of cases. For example, to get the above example working by using an explicit type definition you have to do:

        ingress_rules: List[Union[ec2.CfnSecurityGroup.IngressProperty, core.IResolvable]] = [
            ec2.CfnSecurityGroup.IngressProperty(
                from_port=22,
                to_port=22,
                ip_protocol="tcp",
                cidr_ip="0.0.0.0/0",
            )
        ]

        ec2.CfnSecurityGroup(
            self,
            "SecurityGroup",
            group_description="sample sg",
            security_group_ingress=ingress_rules,
        )

Is there a cleaner workaround?

RomainMuller commented 3 years ago

If we accept sequences on inputs, we're not breaking anything. We may need to continue returning lists on outputs though.

pahud commented 3 months ago

transferring to jsii

aws / jsii