Skip to content

Commit

Permalink
chore(python): use builtins.str instead of just str in Dict keys
Browse files Browse the repository at this point in the history
Added an integration test to verify parameters named `builtins` or `str`
do not shadow the relevant type names, which used to be problematic
until aws#3848 fixed the overarching issue, hoever the existing test only
covered the case where a parameter name shadowed an namespace/import.

Additionally, changes various `typing.Dict` keys that were mistakenly
spelled out as `str` to use `builtins.str` in order to improve overall
consistency in type declarations (see also aws#3866).

Co-authored-by: rmuller@amazon.fr
  • Loading branch information
RomainMuller committed Dec 1, 2022
1 parent fd06e86 commit 1428647
Show file tree
Hide file tree
Showing 12 changed files with 1,195 additions and 261 deletions.
13 changes: 13 additions & 0 deletions packages/@jsii/python-runtime/tests/test_runtime_type_checking.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,3 +199,16 @@ def test_shadowed_namespaces_are_not_a_problem(self):
num = Number(1337)
# The parameter is named "scope" which shadows the "scope" module...
assert num == subject.use_scope(num)

def test_shadowed_builtins_are_not_a_problem(self):
"""Verifies that a parameter shadowing a built-in name does not cause errors"""

jsii_calc.ParamShadowsBuiltins(
"${not a Python type (builtins)}",
"${not a Python type (str)}",
string_property="Most definitely a string",
boolean_property=True,
struct_property={
"required_string": "Present!",
},
)
28 changes: 28 additions & 0 deletions packages/jsii-calc/lib/compliance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3103,3 +3103,31 @@ export class ParamShadowsScope {
return scope;
}
}

/**
* Validate that parameters named "str" or "builtins" do not shadow the actual
* type names in Python.
*/
export class ParamShadowsBuiltins {
/**
* @param builtins should be set to something that is NOT a valid expression in Python (e.g: "${NOPE}"")
* @param str should be set to something that is NOT a valid expression in Python (e.g: "${NOPE}"")
* @param props should be set to valid values.
*/
public constructor(
builtins: string,
str: string,
props: ParamShadowsBuiltinsProps,
) {
this.consumeArgs(builtins, str, props);
}

private consumeArgs(..._args: unknown[]) {
return;
}
}
export interface ParamShadowsBuiltinsProps {
readonly stringProperty: string;
readonly booleanProperty: boolean;
readonly structProperty: StructA;
}
117 changes: 116 additions & 1 deletion packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -11432,6 +11432,121 @@
"name": "OverrideReturnsObject",
"symbolId": "lib/compliance:OverrideReturnsObject"
},
"jsii-calc.ParamShadowsBuiltins": {
"assembly": "jsii-calc",
"docs": {
"stability": "stable",
"summary": "Validate that parameters named \"str\" or \"builtins\" do not shadow the actual type names in Python."
},
"fqn": "jsii-calc.ParamShadowsBuiltins",
"initializer": {
"docs": {
"stability": "stable"
},
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 3117
},
"parameters": [
{
"docs": {
"summary": "should be set to something that is NOT a valid expression in Python (e.g: \"${NOPE}\"\")."
},
"name": "builtins",
"type": {
"primitive": "string"
}
},
{
"docs": {
"summary": "should be set to something that is NOT a valid expression in Python (e.g: \"${NOPE}\"\")."
},
"name": "str",
"type": {
"primitive": "string"
}
},
{
"docs": {
"summary": "should be set to valid values."
},
"name": "props",
"type": {
"fqn": "jsii-calc.ParamShadowsBuiltinsProps"
}
}
]
},
"kind": "class",
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 3111
},
"name": "ParamShadowsBuiltins",
"symbolId": "lib/compliance:ParamShadowsBuiltins"
},
"jsii-calc.ParamShadowsBuiltinsProps": {
"assembly": "jsii-calc",
"datatype": true,
"docs": {
"stability": "stable"
},
"fqn": "jsii-calc.ParamShadowsBuiltinsProps",
"kind": "interface",
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 3129
},
"name": "ParamShadowsBuiltinsProps",
"properties": [
{
"abstract": true,
"docs": {
"stability": "stable"
},
"immutable": true,
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 3131
},
"name": "booleanProperty",
"type": {
"primitive": "boolean"
}
},
{
"abstract": true,
"docs": {
"stability": "stable"
},
"immutable": true,
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 3130
},
"name": "stringProperty",
"type": {
"primitive": "string"
}
},
{
"abstract": true,
"docs": {
"stability": "stable"
},
"immutable": true,
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 3132
},
"name": "structProperty",
"type": {
"fqn": "jsii-calc.StructA"
}
}
],
"symbolId": "lib/compliance:ParamShadowsBuiltinsProps"
},
"jsii-calc.ParamShadowsScope": {
"assembly": "jsii-calc",
"docs": {
Expand Down Expand Up @@ -18684,5 +18799,5 @@
}
},
"version": "3.20.120",
"fingerprint": "rlyZv1z894G2z+Tv8sRGq/ICddvDv3o3auHbiPYCPZI="
"fingerprint": "kmlc5is+t/xffykk7b4m8jurrxFDkj9pjv2cW/V1J50="
}
2 changes: 1 addition & 1 deletion packages/jsii-pacmak/lib/targets/python.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1207,7 +1207,7 @@ class Struct extends BasePythonClassType {
// Required properties, those will always be put into the dict
assignDictionary(
code,
`${implicitParameter}._values: typing.Dict[str, typing.Any]`,
`${implicitParameter}._values: typing.Dict[builtins.str, typing.Any]`,
members
.filter((m) => !m.optional)
.map(
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 1428647

Please sign in to comment.