Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ protected override IReadOnlyList<MethodBodyStatement> BuildAttributes()
(HashSet<CSharpType> buildableTypes, HashSet<TypeProvider> buildableProviders) = CollectBuildableTypes();
foreach (var type in buildableTypes)
{
if (customizedBuildableTypes.Contains(GetTypeIdentity(type)))
if (!type.FrameworkType.IsVisible || customizedBuildableTypes.Contains(GetTypeIdentity(type)))
{
continue;
}
Expand All @@ -68,7 +68,9 @@ protected override IReadOnlyList<MethodBodyStatement> BuildAttributes()
}
foreach (var provider in buildableProviders)
{
if (!ShouldWriteProvider(provider) || customizedBuildableTypes.Contains(GetTypeIdentity(provider.Type)))
Comment thread
jorgerangel-msft marked this conversation as resolved.
if (!IsPublicApi(provider)
|| !ShouldWriteProvider(provider)
|| customizedBuildableTypes.Contains(GetTypeIdentity(provider.Type)))
Comment thread
jorgerangel-msft marked this conversation as resolved.
{
continue;
}
Expand All @@ -87,6 +89,12 @@ protected override IReadOnlyList<MethodBodyStatement> BuildAttributes()
return attributes.OrderBy(a => GetSimpleTypeName(a.Key)).Select(kvp => kvp.Value).ToList();
}

// protected internal is part of the public API surface, but private protected is not
private static bool IsPublicApi(TypeProvider provider)
=> provider.DeclarationModifiers.HasFlag(TypeSignatureModifiers.Public)
|| (provider.DeclarationModifiers.HasFlag(TypeSignatureModifiers.Protected)
&& !provider.DeclarationModifiers.HasFlag(TypeSignatureModifiers.Private));

protected override IReadOnlyList<MethodBodyStatement> BuildAttributesForBackCompatibility(IReadOnlyList<MethodBodyStatement> originalAttributes)
{
if (LastContractView?.Attributes is not { Count: > 0 })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,69 @@ public void ValidateModelReaderWriterBuildableAttributesAreGenerated()
Assert.AreEqual(1, buildableAttributes.Count(), "Exactly one ModelReaderWriterBuildableAttribute should be generated for TestModel");
}

[TestCase("public", 1)]
[TestCase("internal", 0)]
public void BuildableAttributesRespectModelAccessibility(string access, int expectedCount)
{
MockHelpers.LoadMockGenerator(
inputModels: () => [InputFactory.Model("TestModel", access: access)]);

var contextDefinition = new ModelReaderWriterContextDefinition();

Assert.AreEqual(expectedCount, GetBuildableAttributes(contextDefinition).Count);
}

[Test]
public async Task BuildableAttributesRespectCustomizedAccessibility()
{
await MockHelpers.LoadMockGeneratorAsync(
inputModels: () =>
[
InputFactory.Model("InternalModel"),
InputFactory.Model("PublicModel", access: "internal")
],
compilation: async () => await Helpers.GetCompilationFromDirectoryAsync());

var contextDefinition = new ModelReaderWriterContextDefinition();
var file = new TypeProviderWriter(contextDefinition).Write();

Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content);
}

[TestCase(TypeSignatureModifiers.Public, "Public")]
[TestCase(TypeSignatureModifiers.Internal, "Internal")]
[TestCase(TypeSignatureModifiers.Private, "Private")]
[TestCase(TypeSignatureModifiers.Protected, "Protected")]
[TestCase(TypeSignatureModifiers.Protected | TypeSignatureModifiers.Internal, "ProtectedInternal")]
[TestCase(TypeSignatureModifiers.Private | TypeSignatureModifiers.Protected, "PrivateProtected")]
public void BuildableAttributesRespectUpdatedAccessibility(TypeSignatureModifiers accessibility, string expectedFile)
{
var provider = new TestMrwSerialization(implementsPersistableModel: true, includeDepModelProperty: true);
MockHelpers.LoadMockGenerator(createOutputLibrary: () => new TestOutputLibrary([provider]));

_ = provider.Type;
provider.Update(modifiers: accessibility | TypeSignatureModifiers.Class);

var contextDefinition = new ModelReaderWriterContextDefinition();
var file = new TypeProviderWriter(contextDefinition).Write();

Assert.AreEqual(Helpers.GetExpectedFromFile(expectedFile), file.Content);
}

[Test]
public void NonPublicFrameworkTypesDoNotContributeBuildableAttributes()
{
MockHelpers.LoadMockGenerator(
inputModels: () => [InputFactory.Model("PublicModel")],
createCSharpTypeCore: _ => new CSharpType(typeof(TestInternalType)),
createCSharpTypeCoreFallback: input => input == InputPrimitiveType.String);

var contextDefinition = new ModelReaderWriterContextDefinition();
var file = new TypeProviderWriter(contextDefinition).Write();

Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content);
}

[TestCase(true)]
[TestCase(false)]
public void ValidateModelReaderWriterBuildableAttributesAreGeneratedForNonModelsThatImplementMRW(bool implementsIPersistable)
Expand Down Expand Up @@ -1198,7 +1261,7 @@ BinaryData IPersistableModel<DependencyModel>.Write(ModelReaderWriterOptions opt
}

[Experimental("TEST001")]
private class ExperimentalDependencyModel : IJsonModel<ExperimentalDependencyModel>
public class ExperimentalDependencyModel : IJsonModel<ExperimentalDependencyModel>
{
ExperimentalDependencyModel? IJsonModel<ExperimentalDependencyModel>.Create(ref Utf8JsonReader reader, ModelReaderWriterOptions options)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// <auto-generated/>

#nullable disable

using System.ClientModel.Primitives;
using Sample.Models;

namespace Sample
{
[global::System.ClientModel.Primitives.ModelReaderWriterBuildableAttribute(typeof(global::Sample.Models.PublicModel))]
public partial class SampleContext : global::System.ClientModel.Primitives.ModelReaderWriterContext
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

namespace Sample.Models
{
internal partial class InternalModel
{
}

public partial class PublicModel
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// <auto-generated/>

#nullable disable

using System.ClientModel.Primitives;
using Microsoft.TypeSpec.Generator.ClientModel.Tests.Providers.Definitions;

namespace Sample
{
[global::System.ClientModel.Primitives.ModelReaderWriterBuildableAttribute(typeof(global::Microsoft.TypeSpec.Generator.ClientModel.Tests.Providers.Definitions.ModelReaderWriterContextDefinitionTests.DependencyModel))]
public partial class SampleContext : global::System.ClientModel.Primitives.ModelReaderWriterContext
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// <auto-generated/>

#nullable disable

using System.ClientModel.Primitives;
using Microsoft.TypeSpec.Generator.ClientModel.Tests.Providers.Definitions;

namespace Sample
{
[global::System.ClientModel.Primitives.ModelReaderWriterBuildableAttribute(typeof(global::Microsoft.TypeSpec.Generator.ClientModel.Tests.Providers.Definitions.ModelReaderWriterContextDefinitionTests.DependencyModel))]
public partial class SampleContext : global::System.ClientModel.Primitives.ModelReaderWriterContext
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// <auto-generated/>

#nullable disable

using System.ClientModel.Primitives;
using Microsoft.TypeSpec.Generator.ClientModel.Tests.Providers.Definitions;

namespace Sample
{
[global::System.ClientModel.Primitives.ModelReaderWriterBuildableAttribute(typeof(global::Microsoft.TypeSpec.Generator.ClientModel.Tests.Providers.Definitions.ModelReaderWriterContextDefinitionTests.DependencyModel))]
public partial class SampleContext : global::System.ClientModel.Primitives.ModelReaderWriterContext
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// <auto-generated/>

#nullable disable

using System.ClientModel.Primitives;
using Microsoft.TypeSpec.Generator.ClientModel.Tests.Providers.Definitions;

namespace Sample
{
[global::System.ClientModel.Primitives.ModelReaderWriterBuildableAttribute(typeof(global::Microsoft.TypeSpec.Generator.ClientModel.Tests.Providers.Definitions.ModelReaderWriterContextDefinitionTests.DependencyModel))]
[global::System.ClientModel.Primitives.ModelReaderWriterBuildableAttribute(typeof(global::Sample.TestMrwSerialization))]
public partial class SampleContext : global::System.ClientModel.Primitives.ModelReaderWriterContext
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// <auto-generated/>

#nullable disable

using System.ClientModel.Primitives;
using Microsoft.TypeSpec.Generator.ClientModel.Tests.Providers.Definitions;

namespace Sample
{
[global::System.ClientModel.Primitives.ModelReaderWriterBuildableAttribute(typeof(global::Microsoft.TypeSpec.Generator.ClientModel.Tests.Providers.Definitions.ModelReaderWriterContextDefinitionTests.DependencyModel))]
[global::System.ClientModel.Primitives.ModelReaderWriterBuildableAttribute(typeof(global::Sample.TestMrwSerialization))]
public partial class SampleContext : global::System.ClientModel.Primitives.ModelReaderWriterContext
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// <auto-generated/>

#nullable disable

using System.ClientModel.Primitives;
using Microsoft.TypeSpec.Generator.ClientModel.Tests.Providers.Definitions;

namespace Sample
{
[global::System.ClientModel.Primitives.ModelReaderWriterBuildableAttribute(typeof(global::Microsoft.TypeSpec.Generator.ClientModel.Tests.Providers.Definitions.ModelReaderWriterContextDefinitionTests.DependencyModel))]
[global::System.ClientModel.Primitives.ModelReaderWriterBuildableAttribute(typeof(global::Sample.TestMrwSerialization))]
public partial class SampleContext : global::System.ClientModel.Primitives.ModelReaderWriterContext
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// <auto-generated/>

#nullable disable

using System.ClientModel.Primitives;
using Sample.Models;

namespace Sample
{
[global::System.ClientModel.Primitives.ModelReaderWriterBuildableAttribute(typeof(global::Sample.Models.PublicModel))]
public partial class SampleContext : global::System.ClientModel.Primitives.ModelReaderWriterContext
{
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,9 @@ namespace SampleTypeSpec
[ModelReaderWriterBuildable(typeof(Friend))]
[ModelReaderWriterBuildable(typeof(GetNotebookResult))]
[ModelReaderWriterBuildable(typeof(GetWidgetMetricsResult))]
[ModelReaderWriterBuildable(typeof(ListWithContinuationTokenHeaderResponseResult))]
[ModelReaderWriterBuildable(typeof(ListWithContinuationTokenResult))]
[ModelReaderWriterBuildable(typeof(ListWithNextLinkResult))]
[ModelReaderWriterBuildable(typeof(ListWithStringNextLinkResult))]
[ModelReaderWriterBuildable(typeof(ModelWithEmbeddedNonBodyParameters))]
[ModelReaderWriterBuildable(typeof(ModelWithRequiredNullableProperties))]
[ModelReaderWriterBuildable(typeof(NullableDynamicModel))]
[ModelReaderWriterBuildable(typeof(PageThing))]
[ModelReaderWriterBuildable(typeof(Pet))]
[ModelReaderWriterBuildable(typeof(Plant))]
[ModelReaderWriterBuildable(typeof(RenamedModelCustom))]
Expand All @@ -37,9 +32,6 @@ namespace SampleTypeSpec
[ModelReaderWriterBuildable(typeof(StreamingItem))]
[ModelReaderWriterBuildable(typeof(Thing))]
[ModelReaderWriterBuildable(typeof(Tree))]
[ModelReaderWriterBuildable(typeof(UnknownAnimal))]
[ModelReaderWriterBuildable(typeof(UnknownPet))]
[ModelReaderWriterBuildable(typeof(UnknownPlant))]
[ModelReaderWriterBuildable(typeof(Wrapper))]
[ModelReaderWriterBuildable(typeof(XmlAdvancedModel))]
[ModelReaderWriterBuildable(typeof(XmlItem))]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ namespace _Type.Model.Inheritance.EnumDiscriminator
[ModelReaderWriterBuildable(typeof(Dog))]
[ModelReaderWriterBuildable(typeof(Golden))]
[ModelReaderWriterBuildable(typeof(Snake))]
[ModelReaderWriterBuildable(typeof(UnknownDog))]
[ModelReaderWriterBuildable(typeof(UnknownSnake))]
public partial class _TypeModelInheritanceEnumDiscriminatorContext : ModelReaderWriterContext
{
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ namespace _Type.Model.Inheritance.NestedDiscriminator
[ModelReaderWriterBuildable(typeof(Salmon))]
[ModelReaderWriterBuildable(typeof(SawShark))]
[ModelReaderWriterBuildable(typeof(Shark))]
[ModelReaderWriterBuildable(typeof(UnknownFish))]
[ModelReaderWriterBuildable(typeof(UnknownShark))]
public partial class _TypeModelInheritanceNestedDiscriminatorContext : ModelReaderWriterContext
{
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@ namespace _Type.Model.Inheritance.SingleDiscriminator
[ModelReaderWriterBuildable(typeof(SeaGull))]
[ModelReaderWriterBuildable(typeof(Sparrow))]
[ModelReaderWriterBuildable(typeof(TRex))]
[ModelReaderWriterBuildable(typeof(UnknownBird))]
Comment thread
jorgerangel-msft marked this conversation as resolved.
[ModelReaderWriterBuildable(typeof(UnknownDinosaur))]
[ModelReaderWriterBuildable(typeof(UnknownFish))]
public partial class _TypeModelInheritanceSingleDiscriminatorContext : ModelReaderWriterContext
{
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ namespace _Type.Property.AdditionalProperties
[ModelReaderWriterBuildable(typeof(SpreadRecordForNonDiscriminatedUnion3))]
[ModelReaderWriterBuildable(typeof(SpreadRecordForUnion))]
[ModelReaderWriterBuildable(typeof(SpreadStringRecord))]
[ModelReaderWriterBuildable(typeof(UnknownExtendsUnknownAdditionalPropertiesDiscriminated))]
[ModelReaderWriterBuildable(typeof(UnknownIsUnknownAdditionalPropertiesDiscriminated))]
[ModelReaderWriterBuildable(typeof(WidgetData0))]
[ModelReaderWriterBuildable(typeof(WidgetData1))]
[ModelReaderWriterBuildable(typeof(WidgetData2))]
Expand Down
Loading