Add additional handling to python identifier sanitization. Make field prefix configurable.

Author: dbanty

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,7 +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
-    field_1_leading_digit: Optional[str] = None
+    attr_1_leading_digit: Optional[str] = None
 
     def to_dict(self) -> Dict[str, Any]:
         an_enum_value = self.an_enum_value.value
@@ -45,15 +45,15 @@ def to_dict(self) -> Dict[str, Any]:
 
                 nested_list_of_enums.append(nested_list_of_enums_item)
 
-        field_1_leading_digit = self.field_1_leading_digit
+        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": field_1_leading_digit,
+            "1_leading_digit": attr_1_leading_digit,
         }
 
     @staticmethod
@@ -88,13 +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)
 
-        field_1_leading_digit = d.get("1_leading_digit")
+        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,
-            field_1_leading_digit=field_1_leading_digit,
+            attr_1_leading_digit=attr_1_leading_digit,
         )

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/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)
 
 
@@ -36,27 +37,21 @@ def remove_string_escapes(value: str) -> str:
     return value.replace('"', r"\"")
 
 
-def to_valid_python_identifier(value: str) -> str:
-    """
-    Given a string, attempt to coerce it into a valid Python identifier.
-
-    If valid, return it unmodified.
+# This can be changed by config.Config.load_config
+FIELD_PREFIX = "field"
 
-    If invalid, prepend a fixed prefix. This resolves some problems caused by the string's leading
-    character.
 
-    If that prefix does not make it a valid identifier - there are unsupported non-leading
-    characters - raise a ValueError.
+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
     """
-    if value.isidentifier():
-        return value
-
-    new_value = f"field_{value}"
+    new_value = fix_keywords(sanitize(value))
 
     if new_value.isidentifier():
         return new_value
 
-    raise ValueError(f"Cannot convert {value} to a valid python identifier")
+    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

@@ -38,8 +38,8 @@ def test__fix_keywords():
     assert utils.fix_keywords("None") == "None_"
 
 
-def to_valid_python_identifier():
+def test_to_valid_python_identifier():
     assert utils.to_valid_python_identifier("valid") == "valid"
     assert utils.to_valid_python_identifier("1") == "field_1"
-    with pytest.raises(ValueError):
-        utils.to_valid_python_identifier("&")
+    assert utils.to_valid_python_identifier("$") == "field_"
+    assert utils.to_valid_python_identifier("for") == "for_"