Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,7 @@ internal void ProcessTypeForBackCompatibility()
{
var hasMethods = LastContractView?.Methods != null && LastContractView.Methods.Count > 0;
var hasConstructors = LastContractView?.Constructors != null && LastContractView.Constructors.Count > 0;
var hasAttributes = LastContractView?.Attributes is { Count: > 0 };

IReadOnlyList<EnumTypeMember>? updatedEnumValues = null;
IEnumerable<FieldProvider>? newFields = null;
Expand Down Expand Up @@ -735,7 +736,17 @@ internal void ProcessTypeForBackCompatibility()
var newMethods = hasMethods ? BuildMethodsForBackCompatibility(Methods) : null;
var newConstructors = hasConstructors ? BuildConstructorsForBackCompatibility(Constructors) : null;

if (newFields != null || newMethods != null || newConstructors != null)
IReadOnlyList<AttributeStatement>? newAttributes = null;
if (hasAttributes)
{
var backCompatAttributes = BuildAttributesForBackCompatibility(Attributes);
if (backCompatAttributes.Count != Attributes.Count)
{
newAttributes = backCompatAttributes;
}
}

if (newFields != null || newMethods != null || newConstructors != null || newAttributes != null)
{
if (updatedEnumValues != null)
{
Expand All @@ -761,7 +772,7 @@ internal void ProcessTypeForBackCompatibility()
newFields = VisitNewMembers(newFields, Fields, static (member, visitor) => visitor.VisitField(member));
}

Update(fields: newFields, methods: newMethods, constructors: newConstructors);
Update(fields: newFields, methods: newMethods, constructors: newConstructors, attributes: newAttributes);
}
}

Expand Down Expand Up @@ -819,6 +830,56 @@ protected internal virtual IReadOnlyList<MethodProvider> BuildMethodsForBackComp
protected internal virtual IReadOnlyList<ConstructorProvider> BuildConstructorsForBackCompatibility(IEnumerable<ConstructorProvider> originalConstructors)
=> [.. originalConstructors];

/// <summary>
/// Adds any back-compatibility attributes from the last contract that are not already present in
/// <paramref name="originalAttributes"/> (or the custom-code attributes). CodeGen-specific
/// attributes are never restored. The original attributes are returned unchanged when there is
/// nothing new to add.
/// </summary>
protected internal virtual IReadOnlyList<AttributeStatement> BuildAttributesForBackCompatibility(IEnumerable<AttributeStatement> originalAttributes)
{
var original = originalAttributes as IReadOnlyList<AttributeStatement> ?? [.. originalAttributes];

if (LastContractView?.Attributes is not { Count: > 0 } lastContractAttributes)
{
return original;
}

// Track the original (generated + custom) attributes so we only add back-compat attributes
// that don't already exist.
var seen = new HashSet<string>();
foreach (var attribute in original)
{
seen.Add(attribute.ToDisplayString());
}
foreach (var attribute in CustomCodeView?.Attributes ?? [])
{
seen.Add(attribute.ToDisplayString());
}

List<AttributeStatement>? merged = null;
foreach (var attribute in lastContractAttributes)
{
if (ShouldPreserveLastContractAttribute(attribute) && seen.Add(attribute.ToDisplayString()))
{
merged ??= [.. original];
merged.Add(attribute);
}
}

return merged ?? original;
}

private static bool ShouldPreserveLastContractAttribute(AttributeStatement attribute)
{
var attributeName = attribute.Data?.AttributeClass?.Name;
return attributeName is not
(CodeGenAttributes.CodeGenSuppressAttributeName or
CodeGenAttributes.CodeGenMemberAttributeName or
CodeGenAttributes.CodeGenTypeAttributeName or
CodeGenAttributes.CodeGenSerializationAttributeName);
}

Comment thread
jorgerangel-msft marked this conversation as resolved.
private IReadOnlyList<EnumTypeMember>? _enumValues;

private bool ShouldGenerate(ConstructorProvider constructor)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// <auto-generated/>

#nullable disable

using System;

namespace Test
{
[global::System.ObsoleteAttribute("bc")]
public partial class BackCompatAttributeType
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
using System;

namespace Test
{
[Obsolete("bc")]
public class BackCompatAttributeType
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// <auto-generated/>

#nullable disable

using System;

namespace Test
{
[global::System.ObsoleteAttribute("bc")]
public partial class BackCompatAttributeType
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
using System;

namespace Test
{
[Obsolete("bc")]
public class BackCompatAttributeType
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// <auto-generated/>

#nullable disable

using System;

namespace Test
{
[global::System.ObsoleteAttribute]
[global::System.ObsoleteAttribute]
[global::System.ObsoleteAttribute("This is obsolete")]
public partial class TestName
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// <auto-generated/>

#nullable disable

namespace Test
{
public partial class BackCompatAttributeType
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
using System;
using Microsoft.TypeSpec.Generator.Customizations;

namespace Test
{
[CodeGenSuppress("Foo")]
public class BackCompatAttributeType
{
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,97 @@ public void Setup()
MockHelpers.LoadMockGenerator();
}

[Test]
Comment thread
jorgerangel-msft marked this conversation as resolved.
public void BuildAttributesForBackCompatibilityReturnsGeneratedAttributesWhenNoLastContract()
{
var provider = new AttributeTestProvider();

// With no last contract to restore attributes from, the method only ever adds new back-compat
// attributes, so it returns the original attributes unchanged (without deduplicating them).
var attributes = provider.GetBackCompatibilityAttributes();
provider.Update(attributes: attributes);

Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider));
}

[Test]
public async Task BuildAttributesForBackCompatibilityAddsAttributeFromLastContract()
{
await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync());
var provider = new AttributeTestProvider(name: "BackCompatAttributeType");

// The last contract declares [Obsolete("bc")] which is not present in the original set, so it
// should be appended to the result.
var attributes = provider.GetBackCompatibilityAttributes([]);
provider.Update(attributes: attributes);

Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider));
}

[Test]
public async Task BuildAttributesForBackCompatibilityDoesNotDuplicateExistingAttribute()
{
await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync());
var provider = new AttributeTestProvider(name: "BackCompatAttributeType");

// The original set already contains the same [Obsolete("bc")] attribute that the last contract
// declares, so nothing new is added and the original list is returned unchanged.
IReadOnlyList<AttributeStatement> original =
[
new AttributeStatement(typeof(ObsoleteAttribute), Snippet.Literal("bc")),
];
var attributes = provider.GetBackCompatibilityAttributes(original);

// No new attribute is added, so the original list is returned by reference (no allocation).
Assert.AreSame(original, attributes);

provider.Update(attributes: attributes);
Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider));
}

[Test]
public async Task BuildAttributesForBackCompatibilitySkipsCodeGenAttributes()
{
await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync());
var provider = new AttributeTestProvider(name: "BackCompatAttributeType");

// The last contract only declares a CodeGen-specific attribute, which is never restored, so the
// original (empty) list is returned unchanged.
var original = Array.Empty<AttributeStatement>();
var attributes = provider.GetBackCompatibilityAttributes(original);

// Nothing is restored, so the original list is returned by reference (no allocation).
Assert.AreSame(original, attributes);

provider.Update(attributes: attributes);
Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider));
}

private static string Write(TypeProvider provider) =>
CodeModelGenerator.Instance.GetWriter(provider).Write().Content;

private class AttributeTestProvider : TestTypeProvider
{
public AttributeTestProvider(string? name = null)
: base(name: name)
{
}

protected override TypeSignatureModifiers BuildDeclarationModifiers() =>
TypeSignatureModifiers.Public | TypeSignatureModifiers.Class;

protected override IReadOnlyList<MethodBodyStatement> BuildAttributes() =>
[
new AttributeStatement(typeof(ObsoleteAttribute)),
new AttributeStatement(typeof(ObsoleteAttribute)),
new AttributeStatement(typeof(ObsoleteAttribute), Snippet.Literal("This is obsolete")),
];

public IReadOnlyList<AttributeStatement> GetBackCompatibilityAttributes() => BuildAttributesForBackCompatibility(Attributes);

public IReadOnlyList<AttributeStatement> GetBackCompatibilityAttributes(IEnumerable<AttributeStatement> original) => BuildAttributesForBackCompatibility(original);
}

[Test]
public void TestUpdateCanonicalView()
{
Expand Down