fix: handle reserved words in TypedDict field names comprehensively

- Use builtins.X qualification when a TypedDict field wire_name shadows
  a Python builtin type name (e.g. int, str, list) that appears in
  type annotations within the same class
- Add functional TypedDict form for models with Python keyword wire_names
  (e.g. and, class, for) since keywords can't be identifiers in class bodies
- Only import builtins when actually needed (when shadowed builtins appear
  in annotations)
- Remove the previous sorting workaround

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: iscai-msft

packages/http-client-python/generator/pygen/codegen/serializers/types_serializer.py

@@ -3,6 +3,8 @@
 # Licensed under the MIT License. See License.txt in the project root for
 # license information.
 # --------------------------------------------------------------------------
+import keyword
+import re
 from typing import Optional
 from ..models import ModelType, CodeModel
 from ..models.imports import FileImport, ImportType
@@ -12,6 +14,23 @@
 from .import_serializer import FileImportSerializer
 from .base_serializer import BaseSerializer
 
+# Python builtin type names that can be shadowed by TypedDict field wire_names.
+# When a field name matches one of these, all references to that builtin in type
+# annotations within the same class are qualified as builtins.X.
+_BUILTIN_TYPE_NAMES = frozenset({
+    "int", "str", "float", "bool", "list", "dict", "tuple", "set",
+    "bytes", "type", "object", "complex", "frozenset", "bytearray", "memoryview",
+})
+
+
+def _qualify_shadowed_builtins(annotation: str, shadowed: frozenset[str]) -> str:
+    """Replace bare builtin type references with builtins.X when shadowed by a field name."""
+    if not shadowed:
+        return annotation
+    for name in shadowed:
+        annotation = re.sub(rf"\b{name}\b", f"builtins.{name}", annotation)
+    return annotation
+
 
 class TypesSerializer(BaseSerializer):
     def __init__(
@@ -39,7 +58,6 @@ def discriminated_base_models(self) -> list[ModelType]:
         """
         bases = [m for m in self._models if m.base != "json" and m.discriminated_subtypes]
         base_names = {m.name for m in bases}
-        # Sort: models whose subtypes include other discriminated bases must come after them
         sorted_bases: list[ModelType] = []
         visited: set[str] = set()
 
@@ -62,6 +80,30 @@ def discriminated_subtypes_union(self, model: ModelType) -> str:
         subtype_names = [s.name for s in subtypes]
         return f"{model.name} = Union[{', '.join(subtype_names)}]"
 
+    @staticmethod
+    def has_keyword_wire_names(model: ModelType) -> bool:
+        """Whether any property wire_name is a Python keyword (requires functional TypedDict form)."""
+        return any(keyword.iskeyword(p.wire_name) for p in model.properties)
+
+    @staticmethod
+    def get_shadowed_builtins(model: ModelType) -> frozenset[str]:
+        """Return the set of builtin type names shadowed by property wire_names in this model.
+
+        Only includes a builtin if it is both used as a wire_name AND referenced
+        in a type annotation within the same model (otherwise no shadowing occurs).
+        """
+        wire_builtins = {p.wire_name for p in model.properties if p.wire_name in _BUILTIN_TYPE_NAMES}
+        if not wire_builtins:
+            return frozenset()
+        # Check which of these builtins actually appear in type annotations
+        used = set()
+        for prop in model.properties:
+            annotation = prop.type_annotation()
+            for name in wire_builtins:
+                if re.search(rf"\b{name}\b", annotation):
+                    used.add(name)
+        return frozenset(used)
+
     def imports(self) -> FileImport:
         file_import = FileImport(self.code_model)
 
@@ -72,6 +114,7 @@ def imports(self) -> FileImport:
             if self.discriminated_base_models:
                 file_import.add_submodule_import("typing", "Union", ImportType.STDLIB)
             has_required = False
+            needs_builtins = False
             for model in td_models:
                 file_import.merge(
                     model.imports(
@@ -90,6 +133,8 @@ def imports(self) -> FileImport:
                     )
                     if not (prop.optional or prop.client_default_value is not None):
                         has_required = True
+                if self.get_shadowed_builtins(model):
+                    needs_builtins = True
                 for parent in model.parents:
                     if parent.client_namespace != model.client_namespace and not parent.discriminated_subtypes:
                         file_import.add_submodule_import(
@@ -102,27 +147,55 @@ def imports(self) -> FileImport:
                         )
             if has_required:
                 file_import.add_submodule_import("typing_extensions", "Required", ImportType.STDLIB)
+            if needs_builtins:
+                file_import.add_import("builtins", ImportType.STDLIB)
         return file_import
 
     def declare_model(self, model: ModelType) -> str:
+        """Generate the class declaration or functional form for a TypedDict model.
+
+        Uses functional form when any property wire_name is a Python keyword
+        (e.g. 'and', 'class') since keywords can't be identifiers in class bodies.
+        """
+        if self.has_keyword_wire_names(model):
+            return ""  # functional form is rendered separately
         non_discriminated_parents = [p for p in model.parents if not p.discriminated_subtypes]
         if non_discriminated_parents:
             basename = ", ".join([m.name for m in non_discriminated_parents])
             return f"class {model.name}({basename}):{model.pylint_disable()}"
         return f"class {model.name}(TypedDict, total=False):{model.pylint_disable()}"
 
-    # Python builtin type names that can be shadowed by TypedDict field names
-    _BUILTIN_TYPE_NAMES = frozenset({
-        "int", "str", "float", "bool", "list", "dict", "tuple", "set",
-        "bytes", "type", "object", "complex", "frozenset", "bytearray", "memoryview",
-    })
+    def declare_functional_model(self, model: ModelType) -> str:
+        """Generate a functional-form TypedDict for models with keyword wire_names.
+
+        Functional form is required when any field name is a Python keyword.
+        All fields (including inherited) are included since functional form
+        can't specify a base class.
+        """
+        shadowed = self.get_shadowed_builtins(model)
+        entries: list[str] = []
+        for prop in model.properties:
+            type_annotation = prop.type_annotation(
+                serialize_namespace=self.serialize_namespace,
+                serialize_namespace_type=NamespaceType.TYPES_FILE,
+            )
+            type_annotation = _qualify_shadowed_builtins(type_annotation, shadowed)
+            is_optional = prop.optional or prop.client_default_value is not None
+            if is_optional:
+                entries.append(f'    "{prop.wire_name}": {type_annotation},')
+            else:
+                entries.append(f'    "{prop.wire_name}": Required[{type_annotation}],')
+        fields = "\n".join(entries)
+        return f'{model.name} = TypedDict("{model.name}", {{\n{fields}\n}}, total=False)'
 
     @staticmethod
     def get_properties_to_declare(model: ModelType) -> list[Property]:
+        if TypesSerializer.has_keyword_wire_names(model):
+            return []  # functional form handles all properties
         non_discriminated_parents = [p for p in model.parents if not p.discriminated_subtypes]
         if non_discriminated_parents:
             parent_properties = [p for bm in non_discriminated_parents for p in bm.properties]
-            properties_to_declare = [
+            return [
                 p
                 for p in model.properties
                 if not any(
@@ -132,20 +205,14 @@ def get_properties_to_declare(model: ModelType) -> list[Property]:
                     for pp in parent_properties
                 )
             ]
-        else:
-            properties_to_declare = list(model.properties)
-        # Move properties whose wire_name shadows a Python builtin type to the end,
-        # so they don't shadow the builtin in subsequent type annotations.
-        properties_to_declare.sort(
-            key=lambda p: p.wire_name in TypesSerializer._BUILTIN_TYPE_NAMES
-        )
-        return properties_to_declare
+        return list(model.properties)
 
-    def declare_property(self, prop: Property) -> str:
+    def declare_property(self, prop: Property, shadowed_builtins: frozenset[str]) -> str:
         type_annotation = prop.type_annotation(
             serialize_namespace=self.serialize_namespace,
             serialize_namespace_type=NamespaceType.TYPES_FILE,
         )
+        type_annotation = _qualify_shadowed_builtins(type_annotation, shadowed_builtins)
         is_optional = prop.optional or prop.client_default_value is not None
         if is_optional:
             return f"{prop.wire_name}: {type_annotation}"

packages/http-client-python/generator/pygen/codegen/templates/types.py.jinja2

@@ -7,6 +7,21 @@
 {% import 'operation_tools.jinja2' as op_tools %}
 {% import "macros.jinja2" as macros %}
 {% for model in models %}
+{% if serializer.has_keyword_wire_names(model) %}
+
+
+{{ serializer.declare_functional_model(model) }}
+"""{{ op_tools.wrap_string(model.description(is_operation_file=False), "\n") }}
+
+{% if model.properties != None %}
+    {% for p in model.properties %}
+        {% for line in serializer.variable_documentation_string(p) %}
+{{ macros.wrap_model_string(line, '\n') -}}
+        {% endfor %}
+    {% endfor %}
+{% endif %}
+"""
+{% else %}
 
 
 {{ serializer.declare_model(model) }}
@@ -21,13 +36,15 @@
     {% endif %}
     """
 
+    {% set shadowed = serializer.get_shadowed_builtins(model) %}
     {% for p in serializer.get_properties_to_declare(model)%}
-    {{ serializer.declare_property(p) }}
+    {{ serializer.declare_property(p, shadowed) }}
         {% set prop_description = p.description(is_operation_file=False).replace('"', '\\"') %}
         {% if prop_description %}
     """{{ macros.wrap_model_string(prop_description, '\n    ', '\"\"\"') -}}
         {% endif %}
     {% endfor %}
+{% endif %}
 {% endfor %}
 {% for model in discriminated_bases %}
 {{ serializer.discriminated_subtypes_union(model) }}