Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose public setter of namespace #5928

Merged
merged 14 commits into from
Feb 12, 2025
Merged

Expose public setter of namespace #5928

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
928dce2
Add abstraction for setting namespace of Model and Enum.
live1206 Feb 10, 2025
15efe67
Add abstrction to set model factory namespace
live1206 Feb 10, 2025
8a5c0df
Merge remote-tracking branch 'origin/main' into update-provider-names…
live1206 Feb 10, 2025
b347766
revert namespace abstraction, add public setter instead
live1206 Feb 11, 2025
4d1e781
expose EnumProvider to let sub-plugin identify
live1206 Feb 11, 2025
cc4d378
expose ModelFactoryProvider
live1206 Feb 11, 2025
2b80faf
Merge remote-tracking branch 'origin/main' into update-provider-names…
live1206 Feb 11, 2025
d9798a6
remove Namespace from TypeProvider, use TypeProvider.Type.Namespace f…
live1206 Feb 11, 2025
b058ac6
expose ExtensibleEnumSerializationProvider and FixedEnumSerialization…
live1206 Feb 12, 2025
98b2501
remove duplicate instances of model factory
live1206 Feb 12, 2025
1972665
remove GenerateModelFactory from configuration
live1206 Feb 12, 2025
28df98a
lift ModelFactoryProvider instance to TypeProvider instead of the sta…
live1206 Feb 12, 2025
2040566
cleanup
live1206 Feb 12, 2025
f54b2f3
move ModelFactoryProvider instance to OutputLibrary
live1206 Feb 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...osoft.TypeSpec.Generator.ClientModel/src/Providers/ExtensibleEnumSerializationProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace Microsoft.TypeSpec.Generator.ClientModel.Providers
/// <summary>
/// This defines a class with extension methods for enums to convert an enum to its underlying value, or from its underlying value to an instance of the enum
/// </summary>
internal partial class ExtensibleEnumSerializationProvider : TypeProvider
public partial class ExtensibleEnumSerializationProvider : TypeProvider
{
private readonly InputEnumType _enumType;
private TypeProvider _enumProvider;
Expand Down
2 changes: 1 addition & 1 deletion .../Microsoft.TypeSpec.Generator.ClientModel/src/Providers/FixedEnumSerializationProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace Microsoft.TypeSpec.Generator.ClientModel.Providers
/// <summary>
/// This defines a class with extension methods for enums to convert an enum to its underlying value, or from its underlying value to an instance of the enum
/// </summary>
internal class FixedEnumSerializationProvider : TypeProvider
public class FixedEnumSerializationProvider : TypeProvider
{
private readonly InputEnumType _enumType;
private TypeProvider _enumProvider;
Expand Down
2 changes: 1 addition & 1 deletion ...rp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/RestClientProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public RestClientProvider(InputClient inputClient, ClientProvider clientProvider

protected override string BuildName() => _inputClient.Name.ToCleanName();

protected override string BuildNamespace() => ClientProvider.Namespace;
protected override string BuildNamespace() => ClientProvider.Type.Namespace;

protected override PropertyProvider[] BuildProperties()
{
Expand Down
6 changes: 0 additions & 6 deletions packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Configuration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,6 @@ private static class Options
/// </summary>
internal bool ClearOutputFolder { get; private set; }

/// <summary>
/// Whether we will generate model factory for this library.
/// If true (default), the model factory will be generated. If false, the model factory will not be generated.
/// </summary>
internal bool GenerateModelFactory { get; private set; }

/// <summary>
/// True if a sample project should be generated.
/// </summary>
Expand Down
7 changes: 5 additions & 2 deletions packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/OutputLibrary.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using System.Collections.Generic;
using System.Linq;
using Microsoft.TypeSpec.Generator.Providers;
Expand All @@ -16,6 +17,8 @@ public IReadOnlyList<TypeProvider> TypeProviders
internal set => _typeProviders = value;
}

internal Lazy<ModelFactoryProvider> ModelFactory { get; } = new(() => new ModelFactoryProvider(CodeModelPlugin.Instance.InputLibrary.InputNamespace.Models));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no need to use Lazy here. We can just cache the value in private field.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JoshLove-msft
If you are saying like

internal ModelFactoryProvider ModelFactory { get; } = new ModelFactoryProvider(CodeModelPlugin.Instance.InputLibrary.InputNamespace.Models);

This will not work, because the field will be initialized while creating OutputLibrary instance, and it need CodeModelPlugin.Instance to be created. But CodeModelPlugin.Instance depends on the instance of OutputLibrary, we have a circular dependency here.

If you are saying like

internal ModelFactoryProvider ModelFactory { get; } = _field ??= new ModelFactoryProvider(CodeModelPlugin.Instance.InputLibrary.InputNamespace.Models);

why is this better than using Lazy?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm suggesting that we use a private field to cache the value in the getter rather than using Lazy. Lazy is useful when there are concurrency concerns to ensure that the value is initialized only once. We don't have such concerns here, and there is a performance penalty to using Lazy.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thought it's a good thing to be thread-safe ready. But if that's not what we need for now, sure.


private static TypeProvider[] BuildEnums()
{
var input = CodeModelPlugin.Instance.InputLibrary.InputNamespace;
Expand Down Expand Up @@ -79,9 +82,9 @@ .. BuildModelFactory()
];
}

private static TypeProvider[] BuildModelFactory()
private TypeProvider[] BuildModelFactory()
{
var modelFactory = ModelFactoryProvider.FromInputLibrary();
var modelFactory = ModelFactory.Value;
return modelFactory.Methods.Count > 0 ? [modelFactory] : [];
}
}
Expand Down
4 changes: 2 additions & 2 deletions ...sharp/generator/Microsoft.TypeSpec.Generator/src/PostProcessing/GeneratedCodeWorkspace.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,10 +211,10 @@ internal static Project AddDirectory(Project project, string directory, Func<str
/// </summary>
public async Task PostProcessAsync()
{
var modelFactory = ModelFactoryProvider.FromInputLibrary();
var modelFactory = CodeModelPlugin.Instance.OutputLibrary.ModelFactory.Value;
var postProcessor = new PostProcessor(
[.. CodeModelPlugin.Instance.TypeFactory.UnionTypes, .. CodeModelPlugin.Instance.TypesToKeep],
modelFactoryFullName: $"{modelFactory.Namespace}.{modelFactory.Name}");
modelFactoryFullName: $"{modelFactory.Type.Namespace}.{modelFactory.Name}");
switch (Configuration.UnreferencedTypesHandling)
{
case Configuration.UnreferencedTypesHandlingOption.KeepAll:
Expand Down
2 changes: 1 addition & 1 deletion ...es/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Primitives/CSharpType.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ internal CSharpType(
_underlyingType = underlyingEnumType;
}

public string Namespace { get; private init; }
public string Namespace { get; set; }
public string Name { get; private init; }
public CSharpType? DeclaringType { get; private init; }
public bool IsValueType { get; private init; }
Expand Down
2 changes: 1 addition & 1 deletion ...ient-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/CanonicalTypeProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public CanonicalTypeProvider(TypeProvider generatedTypeProvider, InputType? inpu

protected override string BuildName() => _generatedTypeProvider.Name;

protected override string BuildNamespace() => _generatedTypeProvider.Namespace;
protected override string BuildNamespace() => _generatedTypeProvider.Type.Namespace;

protected override TypeSignatureModifiers BuildDeclarationModifiers() => _generatedTypeProvider.DeclarationModifiers;

Expand Down
2 changes: 1 addition & 1 deletion ...s/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/EnumProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

namespace Microsoft.TypeSpec.Generator.Providers
{
internal abstract class EnumProvider : TypeProvider
public abstract class EnumProvider : TypeProvider
{
private readonly InputEnumType _inputType;

Expand Down
6 changes: 2 additions & 4 deletions ...lient-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,14 @@

namespace Microsoft.TypeSpec.Generator.Providers
{
internal class ModelFactoryProvider : TypeProvider
public class ModelFactoryProvider : TypeProvider
{
private const string ModelFactorySuffix = "ModelFactory";
private const string AdditionalBinaryDataParameterName = "additionalBinaryDataProperties";

private readonly IEnumerable<InputModelType> _models;

public static ModelFactoryProvider FromInputLibrary() => new ModelFactoryProvider(CodeModelPlugin.Instance.InputLibrary.InputNamespace.Models);

private ModelFactoryProvider(IEnumerable<InputModelType> models)
internal ModelFactoryProvider(IEnumerable<InputModelType> models)
{
_models = models;
}
Expand Down
3 changes: 0 additions & 3 deletions ...s/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/TypeProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,6 @@ private IReadOnlyList<FieldProvider> BuildAllCustomFields()

private string? _name;

public string Namespace => _namespace ??= BuildNamespace();
private string? _namespace;

protected virtual FormattableString Description { get; } = FormattableStringHelpers.Empty;

private XmlDocProvider? _xmlDocs;
Expand Down
2 changes: 1 addition & 1 deletion ...sharp/generator/Microsoft.TypeSpec.Generator/test/Providers/CanonicalTypeProviderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public void ValidateName()
[Test]
public void ValidateNamespace()
{
Assert.AreEqual(_typeProvider.Namespace, _typeProvider.CanonicalView.Namespace);
Assert.AreEqual(_typeProvider.Type.Namespace, _typeProvider.CanonicalView.Type.Namespace);
}

[Test]
Expand Down
15 changes: 8 additions & 7 deletions ...r/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,24 @@ namespace Microsoft.TypeSpec.Generator.Tests.Providers.ModelFactories
public class ModelFactoryProviderTests
{
private static readonly InputModelType[] ModelList = GetTestModels();
private CodeModelPlugin _instance;

public ModelFactoryProviderTests()
{
MockHelpers.LoadMockPlugin(inputModelTypes: ModelList);
_instance = MockHelpers.LoadMockPlugin(inputModelTypes: ModelList).Object;
}

[Test]
public void SkipInternalModels()
{
var modelFactory = ModelFactoryProvider.FromInputLibrary();
var modelFactory = _instance.OutputLibrary.ModelFactory.Value;
Assert.AreEqual(ModelList.Length - ModelList.Where(m => m.Access == "internal").Count(), modelFactory.Methods.Count);
}

[Test]
public void ListParamShape()
{
var modelFactory = ModelFactoryProvider.FromInputLibrary();
var modelFactory = _instance.OutputLibrary.ModelFactory.Value;
var models = ModelList.Select(CodeModelPlugin.Instance.TypeFactory.CreateModel);
foreach (var model in models)
{
Expand All @@ -54,7 +55,7 @@ public void ListParamShape()
[Test]
public void DictionaryParamShape()
{
var modelFactory = ModelFactoryProvider.FromInputLibrary();
var modelFactory = _instance.OutputLibrary.ModelFactory.Value;
var models = ModelList.Select(CodeModelPlugin.Instance.TypeFactory.CreateModel);
foreach (var model in models)
{
Expand All @@ -77,7 +78,7 @@ public void DictionaryParamShape()
[Test]
public void DiscriminatorEnumParamShape()
{
var modelFactory = ModelFactoryProvider.FromInputLibrary();
var modelFactory = _instance.OutputLibrary.ModelFactory.Value;
var models = ModelList.Select(CodeModelPlugin.Instance.TypeFactory.CreateModel);
foreach (var model in models)
{
Expand All @@ -99,7 +100,7 @@ public void DiscriminatorEnumParamShape()
[Test]
public void AdditionalPropertiesParamShape()
{
var modelFactory = ModelFactoryProvider.FromInputLibrary();
var modelFactory = _instance.OutputLibrary.ModelFactory.Value;
var models = ModelList.Select(CodeModelPlugin.Instance.TypeFactory.CreateModel);
foreach (var model in models)
{
Expand All @@ -123,7 +124,7 @@ public void AdditionalPropertiesParamShape()
[Test]
public void ModelFactoryName()
{
var modelFactory = ModelFactoryProvider.FromInputLibrary();
var modelFactory = _instance.OutputLibrary.ModelFactory.Value;
Assert.AreEqual("SampleNamespaceModelFactory", modelFactory.Name);
}

Expand Down
Loading