Prepend a prefix to field names to allow use as Python identifiers (#206)

* Prepend a prefix to invalid field names (e.g. starting with number)
* Make prefix configurable
* Strip all invalid characters from identifiers
* Prevent keywords in identifiers

Co-authored-by: Dylan Anthony <[email protected]>

Author: kalzoo

CHANGELOG.md

@@ -5,6 +5,16 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## Unreleased
+
+### Changes
+
+### Fixes
+
+- Prefix generated identifiers to allow leading digits in field names (#206 - @kalzoo).
+
+### Additions
+
 ## 0.6.1 - 2020-09-26
 
 ### Changes

README.md

@@ -112,5 +112,16 @@ project_name_override: my-special-project-name
 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_".
+
+Example:
+
+```yaml
+field_prefix: attr_
+```
+
 [changelog.md]: CHANGELOG.md
 [poetry]: https://python-poetry.org/

end_to_end_tests/config.yml

@@ -8,3 +8,4 @@ class_overrides:
   NestedListOfEnumsItemItem:
     class_name: AnEnumValue
     module_name: an_enum_value
+field_prefix: attr_

end_to_end_tests/golden-record/my_test_api_client/models/a_model.py

@@ -17,6 +17,7 @@ class AModel:
     a_camel_date_time: Union[datetime.datetime, datetime.date]
     a_date: datetime.date
     nested_list_of_enums: Optional[List[List[DifferentEnum]]] = None
+    attr_1_leading_digit: Optional[str] = None
 
     def to_dict(self) -> Dict[str, Any]:
         an_enum_value = self.an_enum_value.value
@@ -44,12 +45,15 @@ def to_dict(self) -> Dict[str, Any]:
 
                 nested_list_of_enums.append(nested_list_of_enums_item)
 
+        attr_1_leading_digit = self.attr_1_leading_digit
+
         return {
             "an_enum_value": an_enum_value,
             "some_dict": some_dict,
             "aCamelDateTime": a_camel_date_time,
             "a_date": a_date,
             "nested_list_of_enums": nested_list_of_enums,
+            "1_leading_digit": attr_1_leading_digit,
         }
 
     @staticmethod
@@ -84,10 +88,13 @@ def _parse_a_camel_date_time(data: Dict[str, Any]) -> Union[datetime.datetime, d
 
             nested_list_of_enums.append(nested_list_of_enums_item)
 
+        attr_1_leading_digit = d.get("1_leading_digit")
+
         return AModel(
             an_enum_value=an_enum_value,
             some_dict=some_dict,
             a_camel_date_time=a_camel_date_time,
             a_date=a_date,
             nested_list_of_enums=nested_list_of_enums,
+            attr_1_leading_digit=attr_1_leading_digit,
         )

end_to_end_tests/openapi.json

@@ -564,6 +564,10 @@
             "title": "A Date",
             "type": "string",
             "format": "date"
+          },
+          "1_leading_digit": {
+            "title": "Leading Digit",
+            "type": "string"
           }
         },
         "description": "A Model for testing all the ways custom objects can be used "

openapi_python_client/config.py

@@ -14,22 +14,27 @@ class Config(BaseModel):
     class_overrides: Optional[Dict[str, ClassOverride]]
     project_name_override: Optional[str]
     package_name_override: Optional[str]
+    field_prefix: Optional[str]
 
     def load_config(self) -> None:
-        """ Loads config from provided Path """
+        """ Sets globals based on Config """
+        from openapi_python_client import Project
 
-        if self.class_overrides is not None:
-            from .parser import reference
+        from . import utils
+        from .parser import reference
 
+        if self.class_overrides is not None:
             for class_name, class_data in self.class_overrides.items():
                 reference.class_overrides[class_name] = reference.Reference(**dict(class_data))
 
-        from openapi_python_client import Project
-
         Project.project_name_override = self.project_name_override
         Project.package_name_override = self.package_name_override
 
+        if self.field_prefix is not None:
+            utils.FIELD_PREFIX = self.field_prefix
+
     @staticmethod
     def load_from_path(path: Path) -> None:
+        """ Creates a Config from provided JSON or YAML file and sets a bunch of globals from it """
         config_data = yaml.safe_load(path.read_text())
         Config(**config_data).load_config()

openapi_python_client/parser/properties.py

@@ -36,7 +36,7 @@ class Property:
     python_name: str = field(init=False)
 
     def __post_init__(self) -> None:
-        self.python_name = utils.snake_case(self.name)
+        self.python_name = utils.to_valid_python_identifier(utils.snake_case(self.name))
         if self.default is not None:
             self.default = self._validate_default(default=self.default)
 

openapi_python_client/utils.py

@@ -5,6 +5,7 @@
 
 
 def sanitize(value: str) -> str:
+    """ Removes every character that isn't 0-9, A-Z, a-z, ' ', -, or _ """
     return re.sub(r"[^\w _\-]+", "", value)
 
 
@@ -34,3 +35,23 @@ def kebab_case(value: str) -> str:
 
 def remove_string_escapes(value: str) -> str:
     return value.replace('"', r"\"")
+
+
+# This can be changed by config.Config.load_config
+FIELD_PREFIX = "field_"
+
+
+def to_valid_python_identifier(value: str) -> str:
+    """
+    Given a string, attempt to coerce it into a valid Python identifier by stripping out invalid characters and, if
+    necessary, prepending a prefix.
+
+    See:
+        https://docs.python.org/3/reference/lexical_analysis.html#identifiers
+    """
+    new_value = fix_keywords(sanitize(value))
+
+    if new_value.isidentifier():
+        return new_value
+
+    return f"{FIELD_PREFIX}{new_value}"

tests/test_config.py

@@ -33,3 +33,12 @@ def test_project_and_package_name_overrides(self):
 
         assert Project.project_name_override == "project-name"
         assert Project.package_name_override == "package_name"
+
+    def test_field_prefix(self):
+        Config(field_prefix="blah").load_config()
+
+        from openapi_python_client import utils
+
+        assert utils.FIELD_PREFIX == "blah"
+
+        utils.FIELD_PREFIX = "field_"

tests/test_openapi_parser/test_properties.py

@@ -36,24 +36,21 @@ def test_get_type_string(self):
     def test_to_string(self, mocker):
         from openapi_python_client.parser.properties import Property
 
-        name = mocker.MagicMock()
-        snake_case = mocker.patch("openapi_python_client.utils.snake_case")
+        name = "test"
         p = Property(name=name, required=True, default=None, nullable=False)
         get_type_string = mocker.patch.object(p, "get_type_string")
 
-        assert p.to_string() == f"{snake_case(name)}: {get_type_string()}"
+        assert p.to_string() == f"{name}: {get_type_string()}"
         p.required = False
-        assert p.to_string() == f"{snake_case(name)}: {get_type_string()} = None"
+        assert p.to_string() == f"{name}: {get_type_string()} = None"
 
         p.default = "TEST"
-        assert p.to_string() == f"{snake_case(name)}: {get_type_string()} = TEST"
+        assert p.to_string() == f"{name}: {get_type_string()} = TEST"
 
-    def test_get_imports(self, mocker):
+    def test_get_imports(self):
         from openapi_python_client.parser.properties import Property
 
-        name = mocker.MagicMock()
-        mocker.patch("openapi_python_client.utils.snake_case")
-        p = Property(name=name, required=True, default=None, nullable=False)
+        p = Property(name="test", required=True, default=None, nullable=False)
         assert p.get_imports(prefix="") == set()
 
         p.required = False
@@ -90,12 +87,10 @@ def test__validate_default(self):
 
 
 class TestDateTimeProperty:
-    def test_get_imports(self, mocker):
+    def test_get_imports(self):
         from openapi_python_client.parser.properties import DateTimeProperty
 
-        name = mocker.MagicMock()
-        mocker.patch("openapi_python_client.utils.snake_case")
-        p = DateTimeProperty(name=name, required=True, default=None, nullable=False)
+        p = DateTimeProperty(name="test", required=True, default=None, nullable=False)
         assert p.get_imports(prefix="...") == {
             "import datetime",
             "from typing import cast",
@@ -121,12 +116,10 @@ def test__validate_default(self):
 
 
 class TestDateProperty:
-    def test_get_imports(self, mocker):
+    def test_get_imports(self):
         from openapi_python_client.parser.properties import DateProperty
 
-        name = mocker.MagicMock()
-        mocker.patch("openapi_python_client.utils.snake_case")
-        p = DateProperty(name=name, required=True, default=None, nullable=False)
+        p = DateProperty(name="test", required=True, default=None, nullable=False)
         assert p.get_imports(prefix="...") == {
             "import datetime",
             "from typing import cast",
@@ -152,13 +145,11 @@ def test__validate_default(self):
 
 
 class TestFileProperty:
-    def test_get_imports(self, mocker):
+    def test_get_imports(self):
         from openapi_python_client.parser.properties import FileProperty
 
-        name = mocker.MagicMock()
-        mocker.patch("openapi_python_client.utils.snake_case")
         prefix = ".."
-        p = FileProperty(name=name, required=True, default=None, nullable=False)
+        p = FileProperty(name="test", required=True, default=None, nullable=False)
         assert p.get_imports(prefix=prefix) == {"from ..types import File"}
 
         p.required = False
@@ -342,9 +333,8 @@ def test__validate_default(self, mocker):
 
 class TestEnumProperty:
     def test___post_init__(self, mocker):
-        name = mocker.MagicMock()
+        name = "test"
 
-        snake_case = mocker.patch("openapi_python_client.utils.snake_case")
         fake_reference = mocker.MagicMock(class_name="MyTestEnum")
         deduped_reference = mocker.MagicMock(class_name="Deduped")
         from_ref = mocker.patch(
@@ -361,7 +351,7 @@ def test___post_init__(self, mocker):
         )
 
         assert enum_property.default == "Deduped.SECOND"
-        assert enum_property.python_name == snake_case(name)
+        assert enum_property.python_name == name
         from_ref.assert_has_calls([mocker.call("a_title"), mocker.call("MyTestEnum1")])
         assert enum_property.reference == deduped_reference
         assert properties._existing_enums == {"MyTestEnum": fake_dup_enum, "Deduped": enum_property}
@@ -383,7 +373,7 @@ def test___post_init__(self, mocker):
             name=name, required=True, default="second", values=values, title="a_title", nullable=False
         )
         assert enum_property.default == "MyTestEnum.SECOND"
-        assert enum_property.python_name == snake_case(name)
+        assert enum_property.python_name == name
         from_ref.assert_called_once_with("a_title")
         assert enum_property.reference == fake_reference
         assert len(properties._existing_enums) == 2
@@ -558,10 +548,8 @@ class TestDictProperty:
     def test_get_imports(self, mocker):
         from openapi_python_client.parser.properties import DictProperty
 
-        name = mocker.MagicMock()
-        mocker.patch("openapi_python_client.utils.snake_case")
         prefix = mocker.MagicMock()
-        p = DictProperty(name=name, required=True, default=None, nullable=False)
+        p = DictProperty(name="test", required=True, default=None, nullable=False)
         assert p.get_imports(prefix=prefix) == {
             "from typing import Dict",
         }

tests/test_utils.py

@@ -34,3 +34,10 @@ def test_no_string_escapes():
 
 def test__fix_keywords():
     assert utils.fix_keywords("None") == "None_"
+
+
+def test_to_valid_python_identifier():
+    assert utils.to_valid_python_identifier("valid") == "valid"
+    assert utils.to_valid_python_identifier("1") == "field_1"
+    assert utils.to_valid_python_identifier("$") == "field_"
+    assert utils.to_valid_python_identifier("for") == "for_"