From 421a5ce63969ccf2c9e505f28385fd07f3d37cf4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 22 Dec 2025 05:07:20 +0000 Subject: [PATCH 1/6] Initial plan From 667314e9088935d0d83deca5f844f2d19f065379 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 22 Dec 2025 05:14:32 +0000 Subject: [PATCH 2/6] Add fix for list property name conflict Co-authored-by: msyyc <70930885+msyyc@users.noreply.github.com> --- .../pygen/codegen/models/code_model.py | 4 + .../pygen/codegen/models/list_type.py | 20 +- .../test/unittests/test_list_property_name.py | 186 ++++++++++++++++++ 3 files changed, 208 insertions(+), 2 deletions(-) create mode 100644 packages/http-client-python/generator/test/unittests/test_list_property_name.py diff --git a/packages/http-client-python/generator/pygen/codegen/models/code_model.py b/packages/http-client-python/generator/pygen/codegen/models/code_model.py index beba1909cb3..81f5f20bf8b 100644 --- a/packages/http-client-python/generator/pygen/codegen/models/code_model.py +++ b/packages/http-client-python/generator/pygen/codegen/models/code_model.py @@ -491,6 +491,10 @@ def _get_relative_generation_dir(self, root_dir: Path, namespace: str) -> Path: def has_operation_named_list(self) -> bool: return any(o.name.lower() == "list" for c in self.clients for og in c.operation_groups for o in og.operations) + @property + def has_property_named_list(self) -> bool: + return any(p.client_name.lower() == "list" for m in self.model_types for p in m.properties) + @property def has_padded_model_property(self) -> bool: for model_type in self.model_types: diff --git a/packages/http-client-python/generator/pygen/codegen/models/list_type.py b/packages/http-client-python/generator/pygen/codegen/models/list_type.py index 39db5a91534..118550979e0 100644 --- a/packages/http-client-python/generator/pygen/codegen/models/list_type.py +++ b/packages/http-client-python/generator/pygen/codegen/models/list_type.py @@ -41,8 +41,13 @@ def type_annotation(self, **kwargs: Any) -> str: # this means we're version tolerant XML, we just return the XML element return self.element_type.type_annotation(**kwargs) - # if there is a function named `list` we have to make sure there's no conflict with the built-in `list` - list_type = "List" if self.code_model.has_operation_named_list and kwargs.get("is_operation_file") else "list" + # if there is a function/property named `list` we have to make sure there's no conflict with the built-in `list` + is_operation_file = kwargs.get("is_operation_file", False) + use_list_import = ( + (self.code_model.has_operation_named_list and is_operation_file) or + (self.code_model.has_property_named_list and not is_operation_file) + ) + list_type = "List" if use_list_import else "list" return f"{list_type}[{self.element_type.type_annotation(**kwargs)}]" def description(self, *, is_operation_file: bool) -> str: @@ -132,6 +137,17 @@ def from_yaml(cls, yaml_data: dict[str, Any], code_model: "CodeModel") -> "ListT def imports(self, **kwargs: Any) -> FileImport: file_import = FileImport(self.code_model) file_import.merge(self.element_type.imports(**kwargs)) + + # If we're using List instead of list due to naming conflicts, import it + is_operation_file = kwargs.get("is_operation_file", False) + use_list_import = ( + (self.code_model.has_operation_named_list and is_operation_file) or + (self.code_model.has_property_named_list and not is_operation_file) + ) + if use_list_import: + from .imports import ImportType + file_import.add_submodule_import("typing", "List", ImportType.STDLIB) + return file_import @property diff --git a/packages/http-client-python/generator/test/unittests/test_list_property_name.py b/packages/http-client-python/generator/test/unittests/test_list_property_name.py new file mode 100644 index 00000000000..ef627f8f28b --- /dev/null +++ b/packages/http-client-python/generator/test/unittests/test_list_property_name.py @@ -0,0 +1,186 @@ +# ------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for +# license information. +# -------------------------------------------------------------------------- +"""Test that a property named 'list' doesn't cause syntax errors in generated code.""" + +import pytest +from pygen.codegen.models import CodeModel, ListType, Property +from pygen.codegen.models.primitive_types import StringType + + +def test_has_property_named_list(): + """Test that has_property_named_list property works correctly.""" + # Mock a code model with a property named "list" + yaml_data = { + "namespace": "test.namespace", + "clients": [], + "types": [ + { + "type": "model", + "name": "TestModel", + "properties": [ + { + "clientName": "list", + "wireName": "list", + "type": {"type": "list", "elementType": {"type": "string"}}, + "optional": True, + } + ], + "usage": "input,output", + } + ], + } + + options = { + "version-tolerant": True, + "models-mode": "dpg", + } + + code_model = CodeModel(yaml_data, options) + + # Check that the property is detected + assert code_model.has_property_named_list is True + + +def test_list_type_annotation_with_property_named_list(): + """Test that ListType uses 'List' instead of 'list' when property named list exists.""" + # Mock a code model with a property named "list" + yaml_data = { + "namespace": "test.namespace", + "clients": [], + "types": [ + { + "type": "model", + "name": "TestModel", + "properties": [ + { + "clientName": "list", + "wireName": "list", + "type": {"type": "list", "elementType": {"type": "string"}}, + "optional": True, + } + ], + "usage": "input,output", + } + ], + } + + options = { + "version-tolerant": True, + "models-mode": "dpg", + } + + code_model = CodeModel(yaml_data, options) + + # Create a ListType + element_type = StringType({"type": "string"}, code_model) + list_type = ListType( + {"type": "list", "elementType": {"type": "string"}}, + code_model, + element_type, + ) + + # Test annotation in model file (is_operation_file=False) + annotation = list_type.type_annotation(is_operation_file=False) + assert annotation == "List[str]", f"Expected 'List[str]' but got '{annotation}'" + + # Test annotation in operation file (is_operation_file=True) - should use 'list' + annotation = list_type.type_annotation(is_operation_file=True) + assert annotation == "list[str]", f"Expected 'list[str]' but got '{annotation}'" + + +def test_list_type_annotation_without_property_named_list(): + """Test that ListType uses 'list' when no property named list exists.""" + # Mock a code model without a property named "list" + yaml_data = { + "namespace": "test.namespace", + "clients": [], + "types": [ + { + "type": "model", + "name": "TestModel", + "properties": [ + { + "clientName": "items", + "wireName": "items", + "type": {"type": "list", "elementType": {"type": "string"}}, + "optional": True, + } + ], + "usage": "input,output", + } + ], + } + + options = { + "version-tolerant": True, + "models-mode": "dpg", + } + + code_model = CodeModel(yaml_data, options) + + # Create a ListType + element_type = StringType({"type": "string"}, code_model) + list_type = ListType( + {"type": "list", "elementType": {"type": "string"}}, + code_model, + element_type, + ) + + # Test annotation in both contexts - should always use 'list' + annotation = list_type.type_annotation(is_operation_file=False) + assert annotation == "list[str]", f"Expected 'list[str]' but got '{annotation}'" + + annotation = list_type.type_annotation(is_operation_file=True) + assert annotation == "list[str]", f"Expected 'list[str]' but got '{annotation}'" + + +def test_list_import_added_when_needed(): + """Test that List is imported from typing when needed.""" + # Mock a code model with a property named "list" + yaml_data = { + "namespace": "test.namespace", + "clients": [], + "types": [ + { + "type": "model", + "name": "TestModel", + "properties": [ + { + "clientName": "list", + "wireName": "list", + "type": {"type": "list", "elementType": {"type": "string"}}, + "optional": True, + } + ], + "usage": "input,output", + } + ], + } + + options = { + "version-tolerant": True, + "models-mode": "dpg", + } + + code_model = CodeModel(yaml_data, options) + + # Create a ListType + element_type = StringType({"type": "string"}, code_model) + list_type = ListType( + {"type": "list", "elementType": {"type": "string"}}, + code_model, + element_type, + ) + + # Test imports in model file context + file_import = list_type.imports(is_operation_file=False) + + # Check that List is imported + has_list_import = any( + i.module_name == "typing" and i.submodule_name == "List" + for i in file_import.imports + ) + assert has_list_import, "Expected List to be imported from typing" From 41306640e33586536b992e8d151dde3330029cf4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 22 Dec 2025 05:16:58 +0000 Subject: [PATCH 3/6] Use type alias instead of import for List type Co-authored-by: msyyc <70930885+msyyc@users.noreply.github.com> --- .../pygen/codegen/models/list_type.py | 11 ---------- .../codegen/serializers/model_serializer.py | 5 +++++ .../test/unittests/test_list_property_name.py | 22 ++++--------------- 3 files changed, 9 insertions(+), 29 deletions(-) diff --git a/packages/http-client-python/generator/pygen/codegen/models/list_type.py b/packages/http-client-python/generator/pygen/codegen/models/list_type.py index 118550979e0..91fa8b20da9 100644 --- a/packages/http-client-python/generator/pygen/codegen/models/list_type.py +++ b/packages/http-client-python/generator/pygen/codegen/models/list_type.py @@ -137,17 +137,6 @@ def from_yaml(cls, yaml_data: dict[str, Any], code_model: "CodeModel") -> "ListT def imports(self, **kwargs: Any) -> FileImport: file_import = FileImport(self.code_model) file_import.merge(self.element_type.imports(**kwargs)) - - # If we're using List instead of list due to naming conflicts, import it - is_operation_file = kwargs.get("is_operation_file", False) - use_list_import = ( - (self.code_model.has_operation_named_list and is_operation_file) or - (self.code_model.has_property_named_list and not is_operation_file) - ) - if use_list_import: - from .imports import ImportType - file_import.add_submodule_import("typing", "List", ImportType.STDLIB) - return file_import @property diff --git a/packages/http-client-python/generator/pygen/codegen/serializers/model_serializer.py b/packages/http-client-python/generator/pygen/codegen/serializers/model_serializer.py index cf17f26e1fd..18446cef3dc 100644 --- a/packages/http-client-python/generator/pygen/codegen/serializers/model_serializer.py +++ b/packages/http-client-python/generator/pygen/codegen/serializers/model_serializer.py @@ -285,6 +285,11 @@ def imports(self) -> FileImport: file_import.add_submodule_import("typing", "overload", ImportType.STDLIB) file_import.add_submodule_import("typing", "Mapping", ImportType.STDLIB) file_import.add_submodule_import("typing", "Any", ImportType.STDLIB) + + # if there is a property named `list` we have to make sure there's no conflict with the built-in `list` + if self.code_model.has_property_named_list: + file_import.define_mypy_type("List", "list") + return file_import def declare_model(self, model: ModelType) -> str: diff --git a/packages/http-client-python/generator/test/unittests/test_list_property_name.py b/packages/http-client-python/generator/test/unittests/test_list_property_name.py index ef627f8f28b..58f6e860368 100644 --- a/packages/http-client-python/generator/test/unittests/test_list_property_name.py +++ b/packages/http-client-python/generator/test/unittests/test_list_property_name.py @@ -138,7 +138,7 @@ def test_list_type_annotation_without_property_named_list(): def test_list_import_added_when_needed(): - """Test that List is imported from typing when needed.""" + """Test that List type alias is defined when needed.""" # Mock a code model with a property named "list" yaml_data = { "namespace": "test.namespace", @@ -167,20 +167,6 @@ def test_list_import_added_when_needed(): code_model = CodeModel(yaml_data, options) - # Create a ListType - element_type = StringType({"type": "string"}, code_model) - list_type = ListType( - {"type": "list", "elementType": {"type": "string"}}, - code_model, - element_type, - ) - - # Test imports in model file context - file_import = list_type.imports(is_operation_file=False) - - # Check that List is imported - has_list_import = any( - i.module_name == "typing" and i.submodule_name == "List" - for i in file_import.imports - ) - assert has_list_import, "Expected List to be imported from typing" + # The imports are handled at the serializer level, not the ListType level + # So we just verify that has_property_named_list is True + assert code_model.has_property_named_list is True From 69488fb649b3a6e9818c0b908a76d58b3fc76ed8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 22 Dec 2025 05:20:16 +0000 Subject: [PATCH 4/6] Address code review feedback: improve code style and test structure Co-authored-by: msyyc <70930885+msyyc@users.noreply.github.com> --- .../codegen/serializers/model_serializer.py | 2 - .../test/unittests/test_list_property_name.py | 111 +++++------------- 2 files changed, 31 insertions(+), 82 deletions(-) diff --git a/packages/http-client-python/generator/pygen/codegen/serializers/model_serializer.py b/packages/http-client-python/generator/pygen/codegen/serializers/model_serializer.py index 18446cef3dc..d428a113e5e 100644 --- a/packages/http-client-python/generator/pygen/codegen/serializers/model_serializer.py +++ b/packages/http-client-python/generator/pygen/codegen/serializers/model_serializer.py @@ -285,11 +285,9 @@ def imports(self) -> FileImport: file_import.add_submodule_import("typing", "overload", ImportType.STDLIB) file_import.add_submodule_import("typing", "Mapping", ImportType.STDLIB) file_import.add_submodule_import("typing", "Any", ImportType.STDLIB) - # if there is a property named `list` we have to make sure there's no conflict with the built-in `list` if self.code_model.has_property_named_list: file_import.define_mypy_type("List", "list") - return file_import def declare_model(self, model: ModelType) -> str: diff --git a/packages/http-client-python/generator/test/unittests/test_list_property_name.py b/packages/http-client-python/generator/test/unittests/test_list_property_name.py index 58f6e860368..fd6d77ed9f8 100644 --- a/packages/http-client-python/generator/test/unittests/test_list_property_name.py +++ b/packages/http-client-python/generator/test/unittests/test_list_property_name.py @@ -10,10 +10,10 @@ from pygen.codegen.models.primitive_types import StringType -def test_has_property_named_list(): - """Test that has_property_named_list property works correctly.""" - # Mock a code model with a property named "list" - yaml_data = { +@pytest.fixture +def yaml_data_with_list_property(): + """YAML data for a model with a property named 'list'.""" + return { "namespace": "test.namespace", "clients": [], "types": [ @@ -32,22 +32,12 @@ def test_has_property_named_list(): } ], } - - options = { - "version-tolerant": True, - "models-mode": "dpg", - } - - code_model = CodeModel(yaml_data, options) - - # Check that the property is detected - assert code_model.has_property_named_list is True -def test_list_type_annotation_with_property_named_list(): - """Test that ListType uses 'List' instead of 'list' when property named list exists.""" - # Mock a code model with a property named "list" - yaml_data = { +@pytest.fixture +def yaml_data_without_list_property(): + """YAML data for a model without a property named 'list'.""" + return { "namespace": "test.namespace", "clients": [], "types": [ @@ -56,8 +46,8 @@ def test_list_type_annotation_with_property_named_list(): "name": "TestModel", "properties": [ { - "clientName": "list", - "wireName": "list", + "clientName": "items", + "wireName": "items", "type": {"type": "list", "elementType": {"type": "string"}}, "optional": True, } @@ -66,13 +56,26 @@ def test_list_type_annotation_with_property_named_list(): } ], } - - options = { + + +@pytest.fixture +def code_model_options(): + """Common options for CodeModel.""" + return { "version-tolerant": True, "models-mode": "dpg", } - - code_model = CodeModel(yaml_data, options) + + +def test_has_property_named_list(yaml_data_with_list_property, code_model_options): + """Test that has_property_named_list property works correctly.""" + code_model = CodeModel(yaml_data_with_list_property, code_model_options) + assert code_model.has_property_named_list is True + + +def test_list_type_annotation_with_property_named_list(yaml_data_with_list_property, code_model_options): + """Test that ListType uses 'List' instead of 'list' when property named list exists.""" + code_model = CodeModel(yaml_data_with_list_property, code_model_options) # Create a ListType element_type = StringType({"type": "string"}, code_model) @@ -91,35 +94,9 @@ def test_list_type_annotation_with_property_named_list(): assert annotation == "list[str]", f"Expected 'list[str]' but got '{annotation}'" -def test_list_type_annotation_without_property_named_list(): +def test_list_type_annotation_without_property_named_list(yaml_data_without_list_property, code_model_options): """Test that ListType uses 'list' when no property named list exists.""" - # Mock a code model without a property named "list" - yaml_data = { - "namespace": "test.namespace", - "clients": [], - "types": [ - { - "type": "model", - "name": "TestModel", - "properties": [ - { - "clientName": "items", - "wireName": "items", - "type": {"type": "list", "elementType": {"type": "string"}}, - "optional": True, - } - ], - "usage": "input,output", - } - ], - } - - options = { - "version-tolerant": True, - "models-mode": "dpg", - } - - code_model = CodeModel(yaml_data, options) + code_model = CodeModel(yaml_data_without_list_property, code_model_options) # Create a ListType element_type = StringType({"type": "string"}, code_model) @@ -137,35 +114,9 @@ def test_list_type_annotation_without_property_named_list(): assert annotation == "list[str]", f"Expected 'list[str]' but got '{annotation}'" -def test_list_import_added_when_needed(): +def test_list_import_added_when_needed(yaml_data_with_list_property, code_model_options): """Test that List type alias is defined when needed.""" - # Mock a code model with a property named "list" - yaml_data = { - "namespace": "test.namespace", - "clients": [], - "types": [ - { - "type": "model", - "name": "TestModel", - "properties": [ - { - "clientName": "list", - "wireName": "list", - "type": {"type": "list", "elementType": {"type": "string"}}, - "optional": True, - } - ], - "usage": "input,output", - } - ], - } - - options = { - "version-tolerant": True, - "models-mode": "dpg", - } - - code_model = CodeModel(yaml_data, options) + code_model = CodeModel(yaml_data_with_list_property, code_model_options) # The imports are handled at the serializer level, not the ListType level # So we just verify that has_property_named_list is True From efb290e1d57639778732da3a26120539a06d1eb1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 22 Dec 2025 05:34:22 +0000 Subject: [PATCH 5/6] Add changelog entry for list property name fix Co-authored-by: msyyc <70930885+msyyc@users.noreply.github.com> --- .../copilot-fix-list-property-name-2025-12-22-05-33-41.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .chronus/changes/copilot-fix-list-property-name-2025-12-22-05-33-41.md diff --git a/.chronus/changes/copilot-fix-list-property-name-2025-12-22-05-33-41.md b/.chronus/changes/copilot-fix-list-property-name-2025-12-22-05-33-41.md new file mode 100644 index 00000000000..1de960b2340 --- /dev/null +++ b/.chronus/changes/copilot-fix-list-property-name-2025-12-22-05-33-41.md @@ -0,0 +1,7 @@ +--- +changeKind: fix +packages: + - "@typespec/http-client-python" +--- + +Fix syntax error when model property is named "list" by using type alias to avoid naming conflicts From 310582b0270e421ee4ec59152635d1bfd61fa98f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 22 Dec 2025 06:23:58 +0000 Subject: [PATCH 6/6] Remove test file test_list_property_name.py as requested Co-authored-by: msyyc <70930885+msyyc@users.noreply.github.com> --- .../test/unittests/test_list_property_name.py | 123 ------------------ 1 file changed, 123 deletions(-) delete mode 100644 packages/http-client-python/generator/test/unittests/test_list_property_name.py diff --git a/packages/http-client-python/generator/test/unittests/test_list_property_name.py b/packages/http-client-python/generator/test/unittests/test_list_property_name.py deleted file mode 100644 index fd6d77ed9f8..00000000000 --- a/packages/http-client-python/generator/test/unittests/test_list_property_name.py +++ /dev/null @@ -1,123 +0,0 @@ -# ------------------------------------------------------------------------- -# Copyright (c) Microsoft Corporation. All rights reserved. -# Licensed under the MIT License. See License.txt in the project root for -# license information. -# -------------------------------------------------------------------------- -"""Test that a property named 'list' doesn't cause syntax errors in generated code.""" - -import pytest -from pygen.codegen.models import CodeModel, ListType, Property -from pygen.codegen.models.primitive_types import StringType - - -@pytest.fixture -def yaml_data_with_list_property(): - """YAML data for a model with a property named 'list'.""" - return { - "namespace": "test.namespace", - "clients": [], - "types": [ - { - "type": "model", - "name": "TestModel", - "properties": [ - { - "clientName": "list", - "wireName": "list", - "type": {"type": "list", "elementType": {"type": "string"}}, - "optional": True, - } - ], - "usage": "input,output", - } - ], - } - - -@pytest.fixture -def yaml_data_without_list_property(): - """YAML data for a model without a property named 'list'.""" - return { - "namespace": "test.namespace", - "clients": [], - "types": [ - { - "type": "model", - "name": "TestModel", - "properties": [ - { - "clientName": "items", - "wireName": "items", - "type": {"type": "list", "elementType": {"type": "string"}}, - "optional": True, - } - ], - "usage": "input,output", - } - ], - } - - -@pytest.fixture -def code_model_options(): - """Common options for CodeModel.""" - return { - "version-tolerant": True, - "models-mode": "dpg", - } - - -def test_has_property_named_list(yaml_data_with_list_property, code_model_options): - """Test that has_property_named_list property works correctly.""" - code_model = CodeModel(yaml_data_with_list_property, code_model_options) - assert code_model.has_property_named_list is True - - -def test_list_type_annotation_with_property_named_list(yaml_data_with_list_property, code_model_options): - """Test that ListType uses 'List' instead of 'list' when property named list exists.""" - code_model = CodeModel(yaml_data_with_list_property, code_model_options) - - # Create a ListType - element_type = StringType({"type": "string"}, code_model) - list_type = ListType( - {"type": "list", "elementType": {"type": "string"}}, - code_model, - element_type, - ) - - # Test annotation in model file (is_operation_file=False) - annotation = list_type.type_annotation(is_operation_file=False) - assert annotation == "List[str]", f"Expected 'List[str]' but got '{annotation}'" - - # Test annotation in operation file (is_operation_file=True) - should use 'list' - annotation = list_type.type_annotation(is_operation_file=True) - assert annotation == "list[str]", f"Expected 'list[str]' but got '{annotation}'" - - -def test_list_type_annotation_without_property_named_list(yaml_data_without_list_property, code_model_options): - """Test that ListType uses 'list' when no property named list exists.""" - code_model = CodeModel(yaml_data_without_list_property, code_model_options) - - # Create a ListType - element_type = StringType({"type": "string"}, code_model) - list_type = ListType( - {"type": "list", "elementType": {"type": "string"}}, - code_model, - element_type, - ) - - # Test annotation in both contexts - should always use 'list' - annotation = list_type.type_annotation(is_operation_file=False) - assert annotation == "list[str]", f"Expected 'list[str]' but got '{annotation}'" - - annotation = list_type.type_annotation(is_operation_file=True) - assert annotation == "list[str]", f"Expected 'list[str]' but got '{annotation}'" - - -def test_list_import_added_when_needed(yaml_data_with_list_property, code_model_options): - """Test that List type alias is defined when needed.""" - code_model = CodeModel(yaml_data_with_list_property, code_model_options) - - # The imports are handled at the serializer level, not the ListType level - # So we just verify that has_property_named_list is True - assert code_model.has_property_named_list is True