Skip to content

Commit a791115

Browse files
fix(http-client-python): generate valid Python type annotations in types.py (#11638)
The Python client emitter generated `types.py` files with invalid type annotations for `azure-search-documents`, producing 4 MyPy errors across three generated files. This fixes the three distinct codegen bugs behind them. Each bug below shows the generated-code output before and after the fix. ## Bug 1: inconsistent internal-enum export and reference `azure/search/documents/types.py` ```diff from typing import TYPE_CHECKING, Union from typing_extensions import TypedDict if TYPE_CHECKING: - from .models import SemanticQueryRewritesResultType + from .models._enums import SemanticQueryRewritesResultType class SearchDocumentsResult(TypedDict, total=False): - semanticQueryRewritesResultType: Union[str, "_enums.SemanticQueryRewritesResultType"] + semanticQueryRewritesResultType: Union[str, "SemanticQueryRewritesResultType"] ``` Before, the import pulled the bare name from the public `.models` package, where the internal enum is not re-exported (`Module "...models" has no attribute "SemanticQueryRewritesResultType" [attr-defined]`), and the annotation referenced an undefined `_enums.` prefix (`Name "_enums" is not defined [name-defined]`). After, the enum symbol is imported directly from the private `._enums` submodule and the annotation uses the matching bare forward reference. Enums stay private (no change to the public `__all__`). ## Bug 2: duplicate enum import `azure/search/documents/knowledgebases/types.py` ```diff - from typing import TYPE_CHECKING, Union + from typing import Union from typing_extensions import TypedDict from ..indexes.models._enums import KnowledgeSourceKind - if TYPE_CHECKING: - from ..indexes.models import KnowledgeSourceKind - class KnowledgeSource(TypedDict, total=False): kind: Union[str, "KnowledgeSourceKind"] ``` Before, the same symbol was imported at runtime from its `_enums` submodule and again under `if TYPE_CHECKING:` from the public package (`Name "KnowledgeSourceKind" already defined [no-redef]`). After, a dedup pass drops the `TYPE_CHECKING` duplicate whose bound name is already imported at runtime; the runtime import is sufficient for the annotations. ## Bug 3: TypedDict requiredness override via inheritance `azure/search/documents/indexes/types.py` ```diff class SearchIndexerKnowledgeStoreProjectionSelector(TypedDict, total=False): referenceKeyName: str generatedKeyName: str - class SearchIndexerKnowledgeStoreTableProjectionSelector( - SearchIndexerKnowledgeStoreProjectionSelector - ): + class SearchIndexerKnowledgeStoreTableProjectionSelector(TypedDict, total=False): + referenceKeyName: str generatedKeyName: Required[str] tableName: Required[str] ``` Before, the child subclassed the parent and redeclared `generatedKeyName` as `Required[str]`, which PEP 589 forbids (`Overwriting TypedDict field "generatedKeyName" while extending [misc]`). After, the child renders as a flat, non-inheriting `TypedDict` that lists every field (inherited plus own) directly, so requiredness is expressed without illegal inheritance. ## Notes for reviewers - The internal-enum import intentionally imports the bare symbol from `_enums` rather than the `_enums` module, which also avoids name collisions when internal enums come from several sibling namespaces. - Regression tests were added to `tests/unit/test_typeddict.py` covering all three cases. ## Validation - `test_typeddict.py` + `test_enums.py`: 50 passed (42 existing + 8 new). - pylint 10.00/10 on the changed files; black clean under the project config; mypy introduces no new errors (the pre-existing errors are unchanged on the baseline). - Reproduced the fix end to end against the issue's errors: a minimal `azure/search/documents` package built from the buggy forms reproduces the `attr-defined`, `name-defined`, and `misc` (TypedDict) errors under `mypy --python-version 3.10`, and the fixed forms type-check with exit 0. Bug 2's duplicate import is confirmed removed at the serializer level (baseline emits two imports of `KnowledgeSourceKind`, the fix emits one). Fixes: #11626 --------- Co-authored-by: iscai-msft <43154838+iscai-msft@users.noreply.github.com> Copilot-Session: 1a8cce88-1663-4cc6-a634-2e83e35d04b7
1 parent 48030ba commit a791115

6 files changed

Lines changed: 356 additions & 94 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
changeKind: fix
3+
packages:
4+
- "@typespec/http-client-python"
5+
---
6+
7+
Fix invalid Python type annotations generated in `types.py` files. Internal enums used as TypedDict fields are now imported (as a bare symbol) from their private `_enums` submodule so the annotation resolves; duplicate runtime + `TYPE_CHECKING` imports of the same symbol are deduplicated to avoid `no-redef`; and TypedDicts that change an inherited field's requiredness are emitted as a flat (non-inheriting) TypedDict to satisfy PEP 589.

cspell.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ words:
6262
- Declipse
6363
- dedented
6464
- dedup
65+
- dedupe
6566
- Dedupes
6667
- deps
6768
- deser
@@ -240,6 +241,7 @@ words:
240241
- reactivex
241242
- recase
242243
- recorda
244+
- redef
243245
- regen
244246
- rehype
245247
- reinjected

packages/http-client-python/generator/pygen/codegen/models/enum_type.py

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,10 @@ def type_annotation(self, **kwargs: Any) -> str:
203203
model_alias = self.code_model.get_unique_models_alias(serialize_namespace, self.client_namespace)
204204
module_name = f"{model_alias}."
205205
file_name = f"{self.code_model.enums_filename}." if self.internal else ""
206+
if serialize_namespace_type == NamespaceType.TYPES_FILE:
207+
# In types.py the enum symbol is imported directly (bare name), so no ``_enums.``
208+
# module prefix even for internal enums — the prefix would be an undefined name.
209+
file_name = ""
206210
model_name = module_name + file_name + self.name
207211
# we don't need quoted annotation in operation files, and need it in model folder files.
208212
if not kwargs.get("is_operation_file", False):
@@ -305,9 +309,17 @@ def imports(self, **kwargs: Any) -> FileImport:
305309
typing_section=TypingSection.REGULAR,
306310
)
307311
elif serialize_namespace_type == NamespaceType.TYPES_FILE:
308-
# Import enum name directly to avoid dotted forward refs in TypedDict annotations
312+
# Import the enum symbol directly to avoid dotted forward refs in TypedDict
313+
# annotations. Internal enums are not re-exported from the public ``models``
314+
# package, so import them from the private ``_enums`` submodule instead — the
315+
# bare-symbol import (rather than the ``_enums`` module) also avoids name
316+
# collisions when internal enums come from several sibling namespaces.
317+
module_name = f"models.{self.code_model.enums_filename}" if self.internal else "models"
318+
enums_module = self.code_model.get_relative_import_path(
319+
serialize_namespace, self.client_namespace, module_name=module_name
320+
)
309321
file_import.add_submodule_import(
310-
f"{relative_path}models" if relative_path != "." else ".models",
322+
enums_module,
311323
self.name,
312324
ImportType.LOCAL,
313325
typing_section=TypingSection.TYPING,

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,32 @@ def _add_type_checking_import(self):
8484
if any(self.file_import.get_imports_from_section(TypingSection.TYPING)):
8585
self.file_import.add_submodule_import("typing", "TYPE_CHECKING", ImportType.STDLIB)
8686

87+
def _dedupe_typing_imports(self):
88+
"""Drop TYPE_CHECKING imports whose bound name is already imported at runtime.
89+
90+
A name imported in the regular (runtime) section is also available during type checking, so a
91+
duplicate import under ``if TYPE_CHECKING:`` is redundant and triggers mypy's ``no-redef``
92+
error. This can happen when the same symbol is imported from two different modules — e.g. a
93+
cross-namespace enum imported at runtime from its ``_enums`` submodule and again for its
94+
annotation from the public ``models`` package. The runtime import is sufficient.
95+
"""
96+
regular_bound_names = {
97+
(i.alias or i.submodule_name)
98+
for i in self.file_import.get_imports_from_section(TypingSection.REGULAR)
99+
if i.submodule_name
100+
}
101+
if not regular_bound_names:
102+
return
103+
self.file_import.imports = [
104+
i
105+
for i in self.file_import.imports
106+
if not (
107+
i.typing_section == TypingSection.TYPING
108+
and i.submodule_name
109+
and (i.alias or i.submodule_name) in regular_bound_names
110+
)
111+
]
112+
87113
def _add_sys_import_if_needed(self):
88114
all_imports = list(self.file_import.get_imports_from_section(TypingSection.REGULAR)) + list(
89115
self.file_import.get_imports_from_section(TypingSection.TYPING)
@@ -106,6 +132,7 @@ def declare_definition(type_name: str, type_definition: TypeDefinition) -> list[
106132
return "\n".join(declarations)
107133

108134
def __str__(self) -> str:
135+
self._dedupe_typing_imports()
109136
self._add_type_checking_import()
110137
self._add_sys_import_if_needed()
111138
regular_imports = ""

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

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,39 @@ def has_keyword_wire_names(model: ModelType) -> bool:
243243
"""Whether any property wire_name is a Python keyword or requires functional TypedDict form."""
244244
return any(keyword.iskeyword(p.wire_name) or not p.wire_name.isidentifier() for p in model.properties)
245245

246+
@staticmethod
247+
def needs_flat_typeddict(model: ModelType) -> bool:
248+
"""Whether a TypedDict must be emitted in flat (non-inheriting) form.
249+
250+
PEP 589 forbids changing an inherited TypedDict field's requiredness in a subclass. When a
251+
child redeclares an inherited field with a different requiredness (e.g. the parent renders it
252+
as optional via ``total=False`` while the child needs ``Required[...]``), subclassing would
253+
emit an illegal ``Overwriting TypedDict field ... while extending`` construct. Such models are
254+
instead rendered as a flat TypedDict that lists every field (inherited + own) directly.
255+
256+
Models with keyword wire_names already flatten all fields via the functional form, so they
257+
never hit this path.
258+
"""
259+
if TypesSerializer.has_keyword_wire_names(model):
260+
return False
261+
non_discriminated_parents = [p for p in model.parents if not p.discriminated_subtypes]
262+
if not non_discriminated_parents:
263+
return False
264+
for parent in non_discriminated_parents:
265+
for parent_prop in parent.properties:
266+
child_prop = next(
267+
(p for p in model.properties if p.client_name == parent_prop.client_name),
268+
None,
269+
)
270+
if child_prop is None or child_prop is parent_prop:
271+
# Not overridden by the child (same object is reused when inherited unchanged).
272+
continue
273+
parent_required = not (parent_prop.optional or parent_prop.client_default_value is not None)
274+
child_required = not (child_prop.optional or child_prop.client_default_value is not None)
275+
if parent_required != child_required:
276+
return True
277+
return False
278+
246279
def get_shadowed_builtins(self, model: ModelType) -> frozenset[str]:
247280
"""Return the set of builtin type names shadowed by property wire_names in this model.
248281
@@ -303,7 +336,11 @@ def imports(self) -> FileImport:
303336
if self.get_shadowed_builtins(model):
304337
needs_builtins = True
305338
for parent in model.parents:
306-
if parent.client_namespace != model.client_namespace and not parent.discriminated_subtypes:
339+
if (
340+
parent.client_namespace != model.client_namespace
341+
and not parent.discriminated_subtypes
342+
and not self.needs_flat_typeddict(model)
343+
):
307344
# Import parent class from sibling namespace's types module
308345
file_import.add_submodule_import(
309346
self.code_model.get_relative_import_path(
@@ -329,7 +366,7 @@ def declare_model(self, model: ModelType) -> str:
329366
if self.has_keyword_wire_names(model):
330367
return "" # functional form is rendered separately
331368
non_discriminated_parents = [p for p in model.parents if not p.discriminated_subtypes]
332-
if non_discriminated_parents:
369+
if non_discriminated_parents and not self.needs_flat_typeddict(model):
333370
basename = ", ".join([m.name for m in non_discriminated_parents])
334371
return f"class {model.name}({basename}):{model.pylint_disable()}"
335372
return f"class {model.name}(TypedDict, total=False):{model.pylint_disable()}"
@@ -362,7 +399,7 @@ def get_properties_to_declare(model: ModelType) -> list[Property]:
362399
if TypesSerializer.has_keyword_wire_names(model):
363400
return [] # functional form handles all properties
364401
non_discriminated_parents = [p for p in model.parents if not p.discriminated_subtypes]
365-
if non_discriminated_parents:
402+
if non_discriminated_parents and not TypesSerializer.needs_flat_typeddict(model):
366403
parent_properties = [p for bm in non_discriminated_parents for p in bm.properties]
367404
return [
368405
p

0 commit comments

Comments
 (0)