From ca38c62c4c13c1bcbbde9fde365756069b11bbe0 Mon Sep 17 00:00:00 2001 From: Max Komarychev Date: Thu, 17 Nov 2022 15:46:48 +0200 Subject: [PATCH 1/3] fix: Treat leading underscore as a sign of invalid identifier There are 2 reasons for this change: - leading underscores have special meaning and therefore we should prefix those properties to avoid unexpected semantics - in my particular use case my team is working with a schema which has the following fields in an object: "field" and "_field" and current code strips leading underscore and produces invalid data object with 2 fields of the same name --- README.md | 2 ++ .../golden-record/my_test_api_client/models/a_model.py | 8 ++++++++ end_to_end_tests/openapi.json | 4 ++++ openapi_python_client/utils.py | 2 +- 4 files changed, 15 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index c5f869d52..3f7c86366 100644 --- a/README.md +++ b/README.md @@ -129,6 +129,8 @@ package_name_override: my_extra_special_package_name When generating properties, the `name` attribute of the OpenAPI schema will be used. When the `name` is not a valid Python identifier (e.g. begins with a number) this string will be prepended. Defaults to "field\_". +It will also be used to prefix fields in schema starting with "_" in order to avoid +ambigous semantics. Example: diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/a_model.py b/end_to_end_tests/golden-record/my_test_api_client/models/a_model.py index 889237c7b..9c1740dc8 100644 --- a/end_to_end_tests/golden-record/my_test_api_client/models/a_model.py +++ b/end_to_end_tests/golden-record/my_test_api_client/models/a_model.py @@ -35,6 +35,7 @@ class AModel: a_nullable_date (Optional[datetime.date]): a_not_required_date (Union[Unset, datetime.date]): attr_1_leading_digit (Union[Unset, str]): + attr_leading_underscore (Union[Unset, str]): required_nullable (Optional[str]): not_required_nullable (Union[Unset, None, str]): not_required_not_nullable (Union[Unset, str]): @@ -62,6 +63,7 @@ class AModel: nested_list_of_enums: Union[Unset, List[List[DifferentEnum]]] = UNSET a_not_required_date: Union[Unset, datetime.date] = UNSET attr_1_leading_digit: Union[Unset, str] = UNSET + attr_leading_underscore: Union[Unset, str] = UNSET not_required_nullable: Union[Unset, None, str] = UNSET not_required_not_nullable: Union[Unset, str] = UNSET not_required_one_of_models: Union["FreeFormModel", "ModelWithUnionProperty", Unset] = UNSET @@ -123,6 +125,7 @@ def to_dict(self) -> Dict[str, Any]: a_not_required_date = self.a_not_required_date.isoformat() attr_1_leading_digit = self.attr_1_leading_digit + attr_leading_underscore = self.attr_leading_underscore required_nullable = self.required_nullable not_required_nullable = self.not_required_nullable not_required_not_nullable = self.not_required_not_nullable @@ -207,6 +210,8 @@ def to_dict(self) -> Dict[str, Any]: field_dict["a_not_required_date"] = a_not_required_date if attr_1_leading_digit is not UNSET: field_dict["1_leading_digit"] = attr_1_leading_digit + if attr_leading_underscore is not UNSET: + field_dict["_leading_underscore"] = attr_leading_underscore if not_required_nullable is not UNSET: field_dict["not_required_nullable"] = not_required_nullable if not_required_not_nullable is not UNSET: @@ -313,6 +318,8 @@ def _parse_one_of_models(data: object) -> Union["FreeFormModel", "ModelWithUnion attr_1_leading_digit = d.pop("1_leading_digit", UNSET) + attr_leading_underscore = d.pop("_leading_underscore", UNSET) + required_nullable = d.pop("required_nullable") not_required_nullable = d.pop("not_required_nullable", UNSET) @@ -447,6 +454,7 @@ def _parse_not_required_nullable_one_of_models( a_nullable_date=a_nullable_date, a_not_required_date=a_not_required_date, attr_1_leading_digit=attr_1_leading_digit, + attr_leading_underscore=attr_leading_underscore, required_nullable=required_nullable, not_required_nullable=not_required_nullable, not_required_not_nullable=not_required_not_nullable, diff --git a/end_to_end_tests/openapi.json b/end_to_end_tests/openapi.json index e0f064728..3b8a35bee 100644 --- a/end_to_end_tests/openapi.json +++ b/end_to_end_tests/openapi.json @@ -1327,6 +1327,10 @@ "title": "Leading Digit", "type": "string" }, + "_leading_underscore": { + "title": "Leading Underscore", + "type": "string" + }, "required_nullable": { "title": "Required AND Nullable", "type": "string", diff --git a/openapi_python_client/utils.py b/openapi_python_client/utils.py index cb1ac611c..9015f0905 100644 --- a/openapi_python_client/utils.py +++ b/openapi_python_client/utils.py @@ -12,7 +12,7 @@ class PythonIdentifier(str): def __new__(cls, value: str, prefix: str) -> "PythonIdentifier": new_value = fix_reserved_words(snake_case(sanitize(value))) - if not new_value.isidentifier(): + if not new_value.isidentifier() or value[0] == "_": new_value = f"{prefix}{new_value}" return str.__new__(cls, new_value) From 04c997e39ed57d7f55c685fc8914a952ceedc27d Mon Sep 17 00:00:00 2001 From: Dylan Anthony Date: Fri, 6 Jan 2023 15:26:04 -0700 Subject: [PATCH 2/3] docs: tweak new README entry --- README.md | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/README.md b/README.md index 926aaf861..6e1a93906 100644 --- a/README.md +++ b/README.md @@ -127,10 +127,7 @@ package_name_override: my_extra_special_package_name ### field_prefix -When generating properties, the `name` attribute of the OpenAPI schema will be used. When the `name` is not a valid -Python identifier (e.g. begins with a number) this string will be prepended. Defaults to "field\_". -It will also be used to prefix fields in schema starting with "_" in order to avoid -ambigous semantics. +When generating properties, the `name` attribute of the OpenAPI schema will be used. When the `name` is not a valid Python identifier (e.g. begins with a number) this string will be prepended. Defaults to "field\_". It will also be used to prefix fields in schema starting with "_" in order to avoid ambiguous semantics. Example: From da228888351a30c8a2d13616e35bdb9e4eacfe37 Mon Sep 17 00:00:00 2001 From: Dylan Anthony Date: Fri, 6 Jan 2023 15:27:08 -0700 Subject: [PATCH 3/3] chore(style): `[0] ==` -> `startswith` --- openapi_python_client/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openapi_python_client/utils.py b/openapi_python_client/utils.py index 9015f0905..c16237533 100644 --- a/openapi_python_client/utils.py +++ b/openapi_python_client/utils.py @@ -12,7 +12,7 @@ class PythonIdentifier(str): def __new__(cls, value: str, prefix: str) -> "PythonIdentifier": new_value = fix_reserved_words(snake_case(sanitize(value))) - if not new_value.isidentifier() or value[0] == "_": + if not new_value.isidentifier() or value.startswith("_"): new_value = f"{prefix}{new_value}" return str.__new__(cls, new_value)