Skip to content

Commit b48efe2

Browse files
Copilotjorgerangel-msftJoshLove-msft
authored
Fix malformed namespace generation for unresolved types in customization code (#9578)
## Fix for Malformed Namespace Generation in Customization Code - [x] Understand root cause: Roslyn cannot resolve generated types in customization code, leading to malformed namespaces - [x] Move fix from TypeSymbolExtensions to CSharpTypeExtensions.EnsureNamespace per code review feedback - [x] Enhanced EnsureNamespace to look up types by name when spec property is null - [x] Handle edge case where types are renamed with CodeGenType attribute - [x] Add TypeProvidersByName dictionary for efficient name-based lookup - [x] Remove redundant code and unnecessary variables - [x] Add comprehensive tests validating the fix - [x] Validate with existing test suite - all 2260 tests pass (including 2 new tests) ### Changes Made **Modified:** `TypeFactory.cs` - Added `TypeProvidersByName` dictionary mapping C# type names to TypeProviders for efficient O(1) lookup - Populated dictionary when adding types to CSharpTypeMap in CreateModel and CreateEnum **Modified:** `CSharpTypeExtensions.cs` - Enhanced `EnsureNamespace` method to handle custom properties without corresponding spec properties - Added `TryFindCSharpTypeByName` with inline TypeFactory access for cleaner code - Returns properly qualified CSharpType by resolving through TypeProvidersByName **Added Tests:** `ModelCustomizationTests.cs` - `CanAddPropertyReferencingGeneratedType`: Validates custom property `IList<Bar>` gets proper namespace "Sample.Models" - `CanAddPropertyReferencingRenamedGeneratedType`: Validates custom property referencing CodeGenType renamed type gets proper namespace - Both tests confirm namespaces are correctly resolved instead of being empty or malformed ### Testing All tests pass, confirming the fix resolves malformed namespace generation for customization code. <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>[http-client-csharp] malformed code is generated on some kind of customization code</issue_title> > <issue_description>Sometimes, in customized code, roslyn cannot analyze the correct namespace of some symbols (or maybe not?), and code like this is generated: > <img width="1850" height="878" alt="Image" src="https://github.com/user-attachments/assets/1630f69a-b9c1-49f6-8073-386152826431" /> > > We need to fix this. > The above code was generated when we have the following customization: > <img width="1613" height="694" alt="Image" src="https://github.com/user-attachments/assets/f1df4239-5382-4acd-8e76-13dcd72c707f" /> > > To reproduce: > Prepare typespec: > ``` > model Foo { > bar?: Bar; > } > > model Bar { > b?: string; > } > ``` > In the generated code, add the following: > ``` > [CodeGenSerialization(nameof(Bars), "bars"] > public partial class Foo > { > public IList<Bar> Bars {get;} > } > ``` > then generate the code.</issue_description> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > <comment_new><author>@JoshLove-msft</author><body> > @ArcturusZhang can you please add repro instructions?</body></comment_new> > <comment_new><author>@ArcturusZhang</author><body> > > [@ArcturusZhang](https://github.com/ArcturusZhang) can you please add repro instructions? > > added some reproduction steps.</body></comment_new> > <comment_new><author>@jorgerangel-msft</author><body> > The root cause of this may be that we are not successfully reading the namespace of "non-primitive" generated types in the symbols therefore when we write them out, there are no namespaces.</body></comment_new> > </comments> > </details> <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes #9403 <!-- START COPILOT CODING AGENT TIPS --> --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey). --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> Co-authored-by: JoshLove-msft <54595583+JoshLove-msft@users.noreply.github.com>
1 parent 5799e1b commit b48efe2

File tree

6 files changed

+124
-0
lines changed

6 files changed

+124
-0
lines changed

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/TypeFactory.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ public class TypeFactory
2323
private Dictionary<InputModelType, ModelProvider?> InputTypeToModelProvider { get; } = [];
2424

2525
public IDictionary<CSharpType, TypeProvider?> CSharpTypeMap { get; } = new Dictionary<CSharpType, TypeProvider?>(CSharpType.IgnoreNullableComparer);
26+
27+
// Maps C# type names to TypeProviders for efficient lookup when resolving types by name
28+
internal IDictionary<string, TypeProvider> TypeProvidersByName { get; } = new Dictionary<string, TypeProvider>();
29+
2630
private Dictionary<EnumCacheKey, EnumProvider?> EnumCache { get; } = [];
2731

2832
private Dictionary<InputType, CSharpType?> TypeCache { get; } = [];
@@ -197,6 +201,7 @@ protected internal TypeFactory()
197201
if (modelProvider != null)
198202
{
199203
CSharpTypeMap[modelProvider.Type] = modelProvider;
204+
TypeProvidersByName[modelProvider.Type.Name] = modelProvider;
200205
}
201206
return modelProvider;
202207
}
@@ -255,6 +260,7 @@ protected internal TypeFactory()
255260
if (enumProvider != null)
256261
{
257262
CSharpTypeMap[enumProvider.Type] = enumProvider;
263+
TypeProvidersByName[enumProvider.Type.Name] = enumProvider;
258264
}
259265

260266
return enumProvider;

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/CSharpTypeExtensions.cs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,18 @@ private static CSharpType EnsureNamespace(InputProperty? specProperty, CSharpTyp
4747
inputType = GetInputEnumType(specProperty?.Type);
4848
}
4949

50+
// If we still don't have an input type (e.g., custom property without spec property),
51+
// try to look up the type by name in the TypeFactory
52+
if (inputType == null)
53+
{
54+
// Try to resolve by looking up the CSharp type directly
55+
var resolvedType = TryFindCSharpTypeByName(type.Name);
56+
if (resolvedType != null)
57+
{
58+
return type.IsNullable ? resolvedType.WithNullable(true) : resolvedType;
59+
}
60+
}
61+
5062
if (inputType == null)
5163
{
5264
return type;
@@ -64,6 +76,18 @@ private static CSharpType EnsureNamespace(InputProperty? specProperty, CSharpTyp
6476
return type;
6577
}
6678

79+
private static CSharpType? TryFindCSharpTypeByName(string typeName)
80+
{
81+
// Look up type provider by name using the efficient name-based dictionary
82+
// This handles cases where the type is renamed using CodeGenType attribute
83+
if (CodeModelGenerator.Instance.TypeFactory.TypeProvidersByName.TryGetValue(typeName, out var typeProvider))
84+
{
85+
return typeProvider.Type;
86+
}
87+
88+
return null;
89+
}
90+
6791
private static bool IsCustomizedEnumProperty(
6892
InputProperty? inputProperty,
6993
CSharpType customType,

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelCustomizationTests.cs

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1475,6 +1475,69 @@ public async Task CanCustomizeBaseModelWithSpecBase()
14751475
Assert.AreEqual("CustomBaseProp", modelProvider.BaseTypeProvider.Properties[0].Name);
14761476
}
14771477

1478+
[Test]
1479+
public async Task CanAddPropertyReferencingGeneratedType()
1480+
{
1481+
// Create Bar model that will be referenced by the custom property
1482+
var barModel = InputFactory.Model("Bar", properties: [
1483+
InputFactory.Property("b", InputPrimitiveType.String)
1484+
], usage: InputModelTypeUsage.Input);
1485+
1486+
var inputModel = InputFactory.Model("mockInputModel", properties: [
1487+
InputFactory.Property("Prop1", InputPrimitiveType.String)
1488+
], usage: InputModelTypeUsage.Input);
1489+
1490+
var mockGenerator = await MockHelpers.LoadMockGeneratorAsync(
1491+
inputModelTypes: [inputModel, barModel],
1492+
compilation: async () => await Helpers.GetCompilationFromDirectoryAsync());
1493+
1494+
var modelTypeProvider = mockGenerator.Object.OutputLibrary.TypeProviders.Single(t => t.Name == "MockInputModel");
1495+
AssertCommon(modelTypeProvider, "Sample.Models", "MockInputModel");
1496+
1497+
// Validate that the custom property Bars exists in the canonical view
1498+
var barsProperty = modelTypeProvider.CanonicalView.Properties.FirstOrDefault(p => p.Name == "Bars");
1499+
Assert.IsNotNull(barsProperty, "Bars property should exist in canonical view");
1500+
1501+
// Validate that the Bars property has IList<Bar> type with proper namespace
1502+
Assert.IsTrue(barsProperty!.Type.IsList);
1503+
var elementType = barsProperty.Type.ElementType;
1504+
Assert.AreEqual("Bar", elementType.Name);
1505+
Assert.AreEqual("Sample.Models", elementType.Namespace, "Bar type should have proper namespace");
1506+
Assert.IsFalse(string.IsNullOrEmpty(elementType.Namespace), "Element type namespace should not be empty");
1507+
}
1508+
1509+
[Test]
1510+
public async Task CanAddPropertyReferencingRenamedGeneratedType()
1511+
{
1512+
// Create Bar model that will be renamed to RenamedBar via CodeGenType
1513+
var barModel = InputFactory.Model("Bar", properties: [
1514+
InputFactory.Property("b", InputPrimitiveType.String)
1515+
], usage: InputModelTypeUsage.Input);
1516+
1517+
var inputModel = InputFactory.Model("mockInputModel", properties: [
1518+
InputFactory.Property("Prop1", InputPrimitiveType.String)
1519+
], usage: InputModelTypeUsage.Input);
1520+
1521+
var mockGenerator = await MockHelpers.LoadMockGeneratorAsync(
1522+
inputModelTypes: [inputModel, barModel],
1523+
compilation: async () => await Helpers.GetCompilationFromDirectoryAsync());
1524+
1525+
var modelTypeProvider = mockGenerator.Object.OutputLibrary.TypeProviders.Single(t => t.Name == "MockInputModel");
1526+
AssertCommon(modelTypeProvider, "Sample.Models", "MockInputModel");
1527+
1528+
// Validate that the custom property RenamedBars exists in the canonical view
1529+
var renamedBarsProperty = modelTypeProvider.CanonicalView.Properties.FirstOrDefault(p => p.Name == "RenamedBars");
1530+
Assert.IsNotNull(renamedBarsProperty, "RenamedBars property should exist in canonical view");
1531+
1532+
// Validate that the RenamedBars property has IList<RenamedBar> type with proper namespace
1533+
// Even though Bar is the TypeSpec name, the C# code uses RenamedBar
1534+
Assert.IsTrue(renamedBarsProperty!.Type.IsList);
1535+
var elementType = renamedBarsProperty.Type.ElementType;
1536+
Assert.AreEqual("RenamedBar", elementType.Name);
1537+
Assert.AreEqual("Sample.Models", elementType.Namespace, "RenamedBar type should have proper namespace");
1538+
Assert.IsFalse(string.IsNullOrEmpty(elementType.Namespace), "Element type namespace should not be empty");
1539+
}
1540+
14781541
private class NameSpaceVisitor : LibraryVisitor
14791542
{
14801543
protected override TypeProvider? VisitType(TypeProvider type)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
#nullable disable
2+
3+
using System.Collections.Generic;
4+
5+
namespace Sample.Models;
6+
7+
internal partial class MockInputModel
8+
{
9+
public IList<Bar> Bars { get; }
10+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
#nullable disable
2+
3+
using SampleTypeSpec;
4+
using Microsoft.TypeSpec.Generator.Customizations;
5+
6+
namespace Sample.Models;
7+
8+
[CodeGenType("Bar")]
9+
internal partial class RenamedBar
10+
{
11+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
#nullable disable
2+
3+
using System.Collections.Generic;
4+
5+
namespace Sample.Models;
6+
7+
internal partial class MockInputModel
8+
{
9+
public IList<RenamedBar> RenamedBars { get; }
10+
}

0 commit comments

Comments
 (0)