From bf38e01d473cebb1fd21ff88bea3b4dd0ab9332c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=A7=91=F0=9F=8F=BB=E2=80=8D=F0=9F=92=BB=20Romain=20M?= =?UTF-8?q?arcadier?= Date: Wed, 27 Jul 2022 16:52:14 +0200 Subject: [PATCH 1/2] feat(python): formally allow dicts to be passed in lieu of structs Formalize the contract that it is allowed to pass a dict in places where a struct instance is expected (this provides less type checking guarantees, and the developer is responsible for passing the right keys in). This should address a false-positive issue with the runtime type-checking introduced in 1.63.0 (#3660). --- packages/jsii-pacmak/lib/targets/python.ts | 1 + .../lib/targets/python/type-name.ts | 27 ++++++-- .../__snapshots__/target-python.test.js.snap | 64 ++++++++++++------- .../test/targets/python/type-name.test.ts | 32 ++++++++++ 4 files changed, 97 insertions(+), 27 deletions(-) diff --git a/packages/jsii-pacmak/lib/targets/python.ts b/packages/jsii-pacmak/lib/targets/python.ts index df70511d2f..3f085f25b1 100644 --- a/packages/jsii-pacmak/lib/targets/python.ts +++ b/packages/jsii-pacmak/lib/targets/python.ts @@ -2530,6 +2530,7 @@ class PythonGenerator extends Generator { emittedTypes: new Set(), resolver, submodule: assm.name, + typeResolver: (fqn) => resolver.dereference(fqn), }); } diff --git a/packages/jsii-pacmak/lib/targets/python/type-name.ts b/packages/jsii-pacmak/lib/targets/python/type-name.ts index f9410ef746..f26100d818 100644 --- a/packages/jsii-pacmak/lib/targets/python/type-name.ts +++ b/packages/jsii-pacmak/lib/targets/python/type-name.ts @@ -10,6 +10,7 @@ import { PrimitiveTypeReference, isUnionTypeReference, Type, + isInterfaceType, } from '@jsii/spec'; import { toSnakeCase } from 'codemaker'; import { createHash } from 'crypto'; @@ -37,6 +38,9 @@ export interface NamingContext { /** The assembly in which the PythonType is expressed. */ readonly assembly: Assembly; + /** A resolver to obtain complete information about a type. */ + readonly typeResolver: (fqn: string) => Type; + /** The submodule of the assembly in which the PythonType is expressed (could be the module root) */ readonly submodule: string; @@ -293,14 +297,27 @@ class UserType implements TypeName { submodule, surroundingTypeFqns, typeAnnotation = true, + parameterType, + typeResolver, }: NamingContext) { const { assemblyName, packageName, pythonFqn } = toPythonFqn( this.#fqn, assembly, ); + + // If this is a type annotation for a parameter, allow dicts to be passed where structs are expected. + const type = typeResolver(this.#fqn); + const isStruct = isInterfaceType(type) && !!type.datatype; + const wrapType = + typeAnnotation && parameterType && isStruct + ? (pyType: string) => + `typing.Union[${pyType}, typing.Dict[str, typing.Any]]` + : (pyType: string) => pyType; + if (assemblyName !== assembly.name) { return { - pythonType: pythonFqn, + // If it's a struct, then we allow passing as a dict, too... + pythonType: wrapType(pythonFqn), requiredImport: { sourcePackage: packageName, item: '', @@ -330,8 +347,8 @@ class UserType implements TypeName { ) { // Possibly a forward reference, outputting the stringifierd python FQN return { - pythonType: JSON.stringify( - pythonFqn.substring(submodulePythonName.length + 1), + pythonType: wrapType( + JSON.stringify(pythonFqn.substring(submodulePythonName.length + 1)), ), }; } @@ -345,7 +362,9 @@ class UserType implements TypeName { // We'll just make a module-qualified reference at this point. return { - pythonType: pythonFqn.substring(submodulePythonName.length + 1), + pythonType: wrapType( + pythonFqn.substring(submodulePythonName.length + 1), + ), }; } diff --git a/packages/jsii-pacmak/test/generated-code/__snapshots__/target-python.test.js.snap b/packages/jsii-pacmak/test/generated-code/__snapshots__/target-python.test.js.snap index 2cbf902c76..f210e1dcbf 100644 --- a/packages/jsii-pacmak/test/generated-code/__snapshots__/target-python.test.js.snap +++ b/packages/jsii-pacmak/test/generated-code/__snapshots__/target-python.test.js.snap @@ -3591,13 +3591,13 @@ class ClassWithContainerTypes( ): def __init__( self, - array: typing.Sequence["DummyObj"], - record: typing.Mapping[builtins.str, "DummyObj"], - obj: typing.Mapping[builtins.str, "DummyObj"], + array: typing.Sequence[typing.Union["DummyObj", typing.Dict[str, typing.Any]]], + record: typing.Mapping[builtins.str, typing.Union["DummyObj", typing.Dict[str, typing.Any]]], + obj: typing.Mapping[builtins.str, typing.Union["DummyObj", typing.Dict[str, typing.Any]]], *, - array_prop: typing.Sequence["DummyObj"], - obj_prop: typing.Mapping[builtins.str, "DummyObj"], - record_prop: typing.Mapping[builtins.str, "DummyObj"], + array_prop: typing.Sequence[typing.Union["DummyObj", typing.Dict[str, typing.Any]]], + obj_prop: typing.Mapping[builtins.str, typing.Union["DummyObj", typing.Dict[str, typing.Any]]], + record_prop: typing.Mapping[builtins.str, typing.Union["DummyObj", typing.Dict[str, typing.Any]]], ) -> None: ''' :param array: - @@ -4030,9 +4030,9 @@ class ContainerProps: def __init__( self, *, - array_prop: typing.Sequence["DummyObj"], - obj_prop: typing.Mapping[builtins.str, "DummyObj"], - record_prop: typing.Mapping[builtins.str, "DummyObj"], + array_prop: typing.Sequence[typing.Union["DummyObj", typing.Dict[str, typing.Any]]], + obj_prop: typing.Mapping[builtins.str, typing.Union["DummyObj", typing.Dict[str, typing.Any]]], + record_prop: typing.Mapping[builtins.str, typing.Union["DummyObj", typing.Dict[str, typing.Any]]], ) -> None: ''' :param array_prop: @@ -5116,7 +5116,7 @@ class EraseUndefinedHashValues( @builtins.classmethod def does_key_exist( cls, - opts: "EraseUndefinedHashValuesOptions", + opts: typing.Union["EraseUndefinedHashValuesOptions", typing.Dict[str, typing.Any]], key: builtins.str, ) -> builtins.bool: '''Returns \`\`true\`\` if \`\`key\`\` is defined in \`\`opts\`\`. @@ -7812,7 +7812,11 @@ class JsonFormatter(metaclass=jsii.JSIIMeta, jsii_type="jsii-calc.JsonFormatter" class LevelOne(metaclass=jsii.JSIIMeta, jsii_type="jsii-calc.LevelOne"): '''Validates that nested classes get correct code generation for the occasional forward reference.''' - def __init__(self, *, prop: "LevelOne.PropProperty") -> None: + def __init__( + self, + *, + prop: typing.Union["LevelOne.PropProperty", typing.Dict[str, typing.Any]], + ) -> None: ''' :param prop: ''' @@ -7865,7 +7869,11 @@ class LevelOne(metaclass=jsii.JSIIMeta, jsii_type="jsii-calc.LevelOne"): name_mapping={"prop": "prop"}, ) class PropProperty: - def __init__(self, *, prop: "LevelOne.PropBooleanValue") -> None: + def __init__( + self, + *, + prop: typing.Union["LevelOne.PropBooleanValue", typing.Dict[str, typing.Any]], + ) -> None: ''' :param prop: ''' @@ -7902,7 +7910,11 @@ class LevelOne(metaclass=jsii.JSIIMeta, jsii_type="jsii-calc.LevelOne"): name_mapping={"prop": "prop"}, ) class LevelOneProps: - def __init__(self, *, prop: LevelOne.PropProperty) -> None: + def __init__( + self, + *, + prop: typing.Union[LevelOne.PropProperty, typing.Dict[str, typing.Any]], + ) -> None: ''' :param prop: ''' @@ -8999,7 +9011,7 @@ class RootStruct: self, *, string_prop: builtins.str, - nested_struct: typing.Optional[NestedStruct] = None, + nested_struct: typing.Optional[typing.Union[NestedStruct, typing.Dict[str, typing.Any]]] = None, ) -> None: '''This is here to check that we can pass a nested struct into a kwargs by specifying it as an in-line dictionary. @@ -9055,7 +9067,7 @@ class RootStructValidator( cls, *, string_prop: builtins.str, - nested_struct: typing.Optional[NestedStruct] = None, + nested_struct: typing.Optional[typing.Union[NestedStruct, typing.Dict[str, typing.Any]]] = None, ) -> None: ''' :param string_prop: May not be empty. @@ -9628,7 +9640,7 @@ class StructB: *, required_string: builtins.str, optional_boolean: typing.Optional[builtins.bool] = None, - optional_struct_a: typing.Optional[StructA] = None, + optional_struct_a: typing.Optional[typing.Union[StructA, typing.Dict[str, typing.Any]]] = None, ) -> None: '''This intentionally overlaps with StructA (where only requiredString is provided) to test htat the kernel properly disambiguates those. @@ -9761,7 +9773,7 @@ class StructPassing(metaclass=jsii.JSIIMeta, jsii_type="jsii-calc.StructPassing" _positional: jsii.Number, *, required: builtins.str, - second_level: typing.Union[jsii.Number, SecondLevelStruct], + second_level: typing.Union[jsii.Number, typing.Union[SecondLevelStruct, typing.Dict[str, typing.Any]]], optional: typing.Optional[builtins.str] = None, ) -> "TopLevelStruct": ''' @@ -9786,7 +9798,10 @@ class StructUnionConsumer( ): @jsii.member(jsii_name="isStructA") # type: ignore[misc] @builtins.classmethod - def is_struct_a(cls, struct: typing.Union[StructA, StructB]) -> builtins.bool: + def is_struct_a( + cls, + struct: typing.Union[typing.Union[StructA, typing.Dict[str, typing.Any]], typing.Union[StructB, typing.Dict[str, typing.Any]]], + ) -> builtins.bool: ''' :param struct: - ''' @@ -9797,7 +9812,10 @@ class StructUnionConsumer( @jsii.member(jsii_name="isStructB") # type: ignore[misc] @builtins.classmethod - def is_struct_b(cls, struct: typing.Union[StructA, StructB]) -> builtins.bool: + def is_struct_b( + cls, + struct: typing.Union[typing.Union[StructA, typing.Dict[str, typing.Any]], typing.Union[StructB, typing.Dict[str, typing.Any]]], + ) -> builtins.bool: ''' :param struct: - ''' @@ -10276,7 +10294,7 @@ class TopLevelStruct: self, *, required: builtins.str, - second_level: typing.Union[jsii.Number, SecondLevelStruct], + second_level: typing.Union[jsii.Number, typing.Union[SecondLevelStruct, typing.Dict[str, typing.Any]]], optional: typing.Optional[builtins.str] = None, ) -> None: ''' @@ -11228,7 +11246,7 @@ class SupportsNiceJavaBuilder( self, id: jsii.Number, default_bar: typing.Optional[jsii.Number] = None, - props: typing.Optional[SupportsNiceJavaBuilderProps] = None, + props: typing.Optional[typing.Union[SupportsNiceJavaBuilderProps, typing.Dict[str, typing.Any]]] = None, *rest: builtins.str, ) -> None: ''' @@ -12235,7 +12253,7 @@ class MyClass( @jsii.member(jsii_name="bar") def bar( self, - _bar: typing.Mapping[builtins.str, scope.jsii_calc_base.BaseProps], + _bar: typing.Mapping[builtins.str, typing.Union[scope.jsii_calc_base.BaseProps, typing.Dict[str, typing.Any]]], ) -> None: ''' :param _bar: - @@ -12374,7 +12392,7 @@ class MyStruct: def __init__( self, *, - base_map: typing.Mapping[builtins.str, scope.jsii_calc_base.BaseProps], + base_map: typing.Mapping[builtins.str, typing.Union[scope.jsii_calc_base.BaseProps, typing.Dict[str, typing.Any]]], numbers: typing.Sequence[scope.jsii_calc_lib.Number], ) -> None: ''' diff --git a/packages/jsii-pacmak/test/targets/python/type-name.test.ts b/packages/jsii-pacmak/test/targets/python/type-name.test.ts index e4bc575ff3..c69277ff2c 100644 --- a/packages/jsii-pacmak/test/targets/python/type-name.test.ts +++ b/packages/jsii-pacmak/test/targets/python/type-name.test.ts @@ -4,6 +4,8 @@ import { NamedTypeReference, PrimitiveType, SchemaVersion, + Type, + TypeKind, TypeReference, } from '@jsii/spec'; @@ -57,6 +59,11 @@ const assembly: Assembly = { fqn: `@foo/bar.other.${OTHER_SUBMODULE_TYPE}`, namespace: 'other', }, + [`@foo/bar.Struct`]: { + datatype: true, + fqn: `@foo/bar.Struct`, + kind: TypeKind.Interface, + }, }, } as any; @@ -78,6 +85,15 @@ describe(toTypeName, () => { readonly inSubmodule?: string; /** The nesting context in which to generate names (if not provided, none) */ readonly inNestingContext?: readonly string[]; + /** Additional context keys to register */ + readonly context?: Omit< + NamingContext, + | 'assembly' + | 'emittedTypes' + | 'surroundingTypeFqns' + | 'submodule' + | 'typeResolver' + >; }; const examples: readonly Example[] = [ @@ -221,14 +237,30 @@ describe(toTypeName, () => { }, inSubmodule: `${assembly.name}.submodule`, }, + // ############################# SPECIAL CASES############################## + { + name: 'Struct parameter type annotation', + input: { fqn: `${assembly.name}.Struct` }, + forwardPythonType: `typing.Union["Struct", typing.Dict[str, typing.Any]]`, + pythonType: `typing.Union[Struct, typing.Dict[str, typing.Any]]`, + context: { + typeAnnotation: true, + parameterType: true, + }, + }, ]; for (const example of examples) { const context: NamingContext = { + ...example.context, assembly, emittedTypes: new Set(), surroundingTypeFqns: example.inNestingContext, submodule: example.inSubmodule ?? assembly.name, + typeResolver: (fqn) => { + const type = assembly.types?.[fqn]; + return type ?? ({ fqn } as any as Type); + }, }; const contextWithEmittedType: NamingContext = { ...context, From 44a97d1a85e20db25a7af9aa7cba82e58287b00d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=A7=91=F0=9F=8F=BB=E2=80=8D=F0=9F=92=BB=20Romain=20M?= =?UTF-8?q?arcadier?= Date: Wed, 27 Jul 2022 17:13:05 +0200 Subject: [PATCH 2/2] release: v1.63.1 --- CHANGELOG.md | 7 +++++++ lerna.json | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a6b832256f..654af3a097 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines. +## [1.63.1](https://github.com/aws/jsii/compare/v1.63.0...v1.63.1) (2022-07-26) + + +### Features + +* **python:** formally allow dicts to be passed in lieu of structs ([#3683](https://github.com/aws/jsii/pull/3683)) + ## [1.63.0](https://github.com/aws/jsii/compare/v1.62.0...v1.63.0) (2022-07-26) diff --git a/lerna.json b/lerna.json index 6a17a1ece5..f08bf14207 100644 --- a/lerna.json +++ b/lerna.json @@ -10,5 +10,5 @@ "rejectCycles": true } }, - "version": "1.63.0" + "version": "1.63.1" }