Repository: tefra/xsdata (python)

Issue:
originalCase naming convention can lead to invalid Python syntax
I've found a couple of cases where use of the `originalCase` naming convention will retain names that are auto-generated by the library's internals, which in many cases are not valid Python syntax and will generate unexecutable code.

## Example 1

Consider the following schema, where we have an unnamed `xs:any` element:

```xml
<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema">
  <xs:complexType name="foo">
    <xs:sequence>
      <xs:any/>
    </xs:sequence>
  </xs:complexType>
</xs:schema>
```

For unnamed elements such as the above, the library will generate a placeholder name "`any_element`" for the dataclass field.

However with a field naming convention of `<FieldName case="originalCase" safePrefix="value"/>` set in a config file, the following dataclass is produced:

```python
@dataclass
class Foo:
    @any_element: Optional[object] = field(
        default=None,
        metadata={
            "type": "wildcard",
            "namespace": "##any",
        }
    )
```

The `@` prefix at the start of `@any_element` is invalid variable naming syntax.

## Example 2

If two elements in different schemas have the same name and reference one another, the library will attempt to import one with a new name using Python's `from A import B as C` syntax.

For example with the following two schemas:

```xml
<!-- schema1.xsd -->
<xs:schema xmlns:test1="urn:schema1" xmlns:xs="http://www.w3.org/2001/XMLSchema" targetNamespace="urn:test1" elementFormDefault="unqualified" attributeFormDefault="unqualified">
    <xs:simpleType name="clashingName">
        <xs:annotation>
            <xs:documentation>Base type</xs:documentation>
        </xs:annotation>
        <xs:restriction base="xs:token">
            <xs:enumeration value="OPTION_A"/>
            <xs:enumeration value="OPTION_B"/>
        </xs:restriction>
    </xs:simpleType>
</xs:schema>
```

```xml
<!-- schema2.xsd -->
<xs:schema xmlns:test1="urn:test1" xmlns:test2="urn:test2" xmlns:xs="http://www.w3.org/2001/XMLSchema" targetNamespace="urn:test2" elementFormDefault="unqualified" attributeFormDefault="unqualified">
    <xs:import namespace="urn:test1" schemaLocation="schema1.xsd"/>
    <xs:complexType name="clashingName">
        <xs:annotation>
            <xs:documentation>Higher level object that contains the clashingName element from schema1.</xs:documentation>
        </xs:annotation>
        <xs:sequence>
            <xs:element name="title" type="xs:string"/>
            <xs:element name="value" type="test1:clashingName"/>
        </xs:sequence>
    </xs:complexType>
</xs:schema>
```

With a class naming convention of `<ClassName case="originalCase" safePrefix="value"/>` set in a config file, the following dataclass is produced for schema2:

```python
from dataclasses import dataclass, field
from typing import Optional
from generated.schema1 import clashingName as schema1:clashingName

__NAMESPACE__ = "urn:test2"


@dataclass
class clashingName:
    """
    Higher level object that contains the clashingName element from schema1.
    """
    title: Optional[str] = field(
        default=None,
        metadata={
            "type": "Element",
            "namespace": "",
            "required": True,
        }
    )
    value: Optional[schema1:clashingName] = field(
        default=None,
        metadata={
            "type": "Element",
            "namespace": "",
            "required": True,
        }
    )
```

The named import contains a `:` to separate the module from the dataclass, which is invalid Python syntax.

In both cases I would expect that the library would always generate valid Python syntax, even when using internally generated variable names.

Pull request:
Generate valid variable names when using originalCase naming convention
## 📒 Description

When using `originalCase` naming syntax for classes or fields where the names have been generated by the library, ensure that only valid characters are used for the variable in the generated Python code.


Resolves #880 

## 🔗 What I've Done

Implements a new method in the `utils.text` library named `variable()`, which will use some simple regex matching to strip out anything except alphanumeric characters and underscores from the variable name. It will also ensure that a variable name does not start with a digit character.

This resolves both of the examples listed in #880.


## 💬 Comments

None

## 🛫 Checklist

- [ ] Updated docs
- [x] Added unit-tests
- [ ] [Sample tests](https://github.com/tefra/xsdata-samples) pass
- [x] [W3C tests](https://github.com/tefra/xsdata-w3c-tests) pass


Patch:
diff --git a/xsdata/formats/dataclass/filters.py b/xsdata/formats/dataclass/filters.py
index e7e114b3..926d871d 100644
--- a/xsdata/formats/dataclass/filters.py
+++ b/xsdata/formats/dataclass/filters.py
@@ -319,7 +319,7 @@ class Filters:
         if not slug or not slug[0].isalpha():
             return self.safe_name(f"{prefix}_{name}", prefix, name_case, **kwargs)
 
-        result = name_case(name, **kwargs)
+        result = text.variable(name_case(name, **kwargs))
         if text.is_reserved(result):
             return self.safe_name(f"{name}_{prefix}", prefix, name_case, **kwargs)
 
diff --git a/xsdata/utils/text.py b/xsdata/utils/text.py
index 2dee853f..92171168 100644
--- a/xsdata/utils/text.py
+++ b/xsdata/utils/text.py
@@ -226,3 +226,11 @@ def alnum(value: str) -> str:
     """Return a lower case version of the string only with ascii alphanumerical
     characters."""
     return "".join(filter(__alnum_ascii__.__contains__, value)).lower()
+
+
+def variable(value: str) -> str:
+    """Returns a version of the string that will be a valid Python variable."""
+    # Strip out all characters that are not alphanumeric or underscores
+    value = re.sub(r"\W", "", value)
+    # Then strip out leading digit and underscore characters
+    return re.sub(r"^[^a-zA-Z]+", "", value)


Test patch:
diff --git a/tests/formats/dataclass/test_filters.py b/tests/formats/dataclass/test_filters.py
index 6b984741..449611cb 100644
--- a/tests/formats/dataclass/test_filters.py
+++ b/tests/formats/dataclass/test_filters.py
@@ -159,6 +159,9 @@ class FiltersTests(FactoryTestCase):
         self.assertEqual("value_minus_1_1", self.filters.field_name("-1.1", "cls"))
         self.assertEqual("cbad", self.filters.field_name("abcd", "cls"))
 
+        self.filters.field_case = NameCase.ORIGINAL
+        self.assertEqual("foo", self.filters.field_name("@foo", "cls"))
+
     def test_constant_name(self):
         self.filters.substitutions[ObjectType.FIELD]["ABC"] = "CBA"
 
diff --git a/tests/utils/test_text.py b/tests/utils/test_text.py
index 699b128d..5e33cc07 100644
--- a/tests/utils/test_text.py
+++ b/tests/utils/test_text.py
@@ -15,6 +15,7 @@ from xsdata.utils.text import screaming_snake_case
 from xsdata.utils.text import snake_case
 from xsdata.utils.text import split_words
 from xsdata.utils.text import StringType
+from xsdata.utils.text import variable
 
 
 class TextTests(TestCase):
@@ -143,6 +144,13 @@ class TextTests(TestCase):
         self.assertEqual("foo1", alnum(" foo*1"))
         self.assertEqual("1", alnum(" βιβλίο*1"))
 
+    def test_variable(self):
+        self.assertEqual("foO1", variable("foO1"))
+        self.assertEqual("foo_1", variable("foo_1"))
+        self.assertEqual("foo1", variable("foo-1"))
+        self.assertEqual("foo1", variable("@foo1"))
+        self.assertEqual("foo1", variable("1foo1"))
+
     def test_classify(self):
         for ltr in string.ascii_uppercase:
             self.assertEqual(StringType.UPPER, classify(ltr))