Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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 @@ -297,7 +297,7 @@ protected internal virtual FinallyExpression VisitFinallyExpression(FinallyExpre
/// </summary>
/// <param name="property">The original <see cref="PropertyProvider"/>.</param>
/// <returns>Null if it should be removed otherwise the modified version of the <see cref="PropertyProvider"/>.</returns>
protected virtual PropertyProvider? VisitProperty(PropertyProvider property)
protected internal virtual PropertyProvider? VisitProperty(PropertyProvider property)
Comment thread
jorgerangel-msft marked this conversation as resolved.
{
return property;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ protected override string BuildNamespace() => string.IsNullOrEmpty(_inputType?.N
protected static string RemoveUnderscores(string name) => name.Replace("_", string.Empty);

private HashSet<string>? _customMemberNames;
private HashSet<string> CustomMemberNames => _customMemberNames ??= new HashSet<string>(
private protected HashSet<string> CustomMemberNames => _customMemberNames ??= new HashSet<string>(
GetCustomMemberNames(),
StringComparer.OrdinalIgnoreCase);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Linq;
using Microsoft.TypeSpec.Generator.EmitterRpc;
using Microsoft.TypeSpec.Generator.Expressions;
using Microsoft.TypeSpec.Generator.Input;
using Microsoft.TypeSpec.Generator.Input.Extensions;
Expand Down Expand Up @@ -274,5 +276,82 @@ protected override TypeProvider[] BuildSerializationProviders()
return CodeModelGenerator.Instance.TypeFactory.CreateSerializations(_inputType, this).ToArray();
}
protected override bool GetIsEnum() => true;

protected internal override IReadOnlyList<EnumTypeMember>? BuildEnumValuesForBackCompatibility(IReadOnlyList<EnumTypeMember> currentValues)
{
var lastContractProperties = LastContractView?.Properties;
if (lastContractProperties == null || lastContractProperties.Count == 0)
{
return null;
}

var currentNames = new HashSet<string>(currentValues.Select(v => v.Name), StringComparer.OrdinalIgnoreCase);
var lastContractValueFields = new Dictionary<string, FieldProvider>(StringComparer.Ordinal);
foreach (var field in LastContractView!.Fields)
{
lastContractValueFields.TryAdd(field.Name, field);
}

List<EnumTypeMember>? readdedMembers = null;
foreach (var property in lastContractProperties)
{
// Members that still exist in the current spec or are provided by custom code are left untouched.
if (currentNames.Contains(property.Name) || CustomMemberNames.Contains(property.Name))
{
continue;
}

// Honor an intentional removal recorded in the ApiCompat baseline.
if (CodeModelGenerator.Instance.SourceInputModel?.ApiCompatBaseline.IsMemberSuppressed(Type.FullyQualifiedName, property.Name, 0) == true)
{
CodeModelGenerator.Instance.Emitter.Debug(
$"Skipping re-add of enum member '{Name}.{property.Name}'; the removal is accepted in the ApiCompat baseline.",
BackCompatibilityChangeCategory.BaselineAcceptedRemovalSkipped);
continue;
}

if (TryResurrectRemovedMember(property, lastContractValueFields, out var resurrectedMember))
{
(readdedMembers ??= []).Add(resurrectedMember);
CodeModelGenerator.Instance.Emitter.Debug(
$"Re-added enum member '{property.Name}' to enum '{Name}' to preserve a member from the last contract.",
BackCompatibilityChangeCategory.EnumMemberAddedFromLastContract);
}
}

if (readdedMembers == null)
{
return null;
}

// Preserve the current spec order and append the restored members at the end.
return [.. currentValues, .. readdedMembers];
}

private bool TryResurrectRemovedMember(
PropertyProvider lastContractProperty,
IReadOnlyDictionary<string, FieldProvider> lastContractValueFields,
[NotNullWhen(true)] out EnumTypeMember? member)
{
member = null;

// The wire value lives in the private const `<Member>Value` field.
var valueFieldName = $"{lastContractProperty.Name}Value";
Comment thread
jorgerangel-msft marked this conversation as resolved.
if (!lastContractValueFields.TryGetValue(valueFieldName, out var valueField)
|| valueField.InitializationValue is not LiteralExpression { Literal: { } literalValue })
{
return false;
}

var field = new FieldProvider(
FieldModifiers.Private | FieldModifiers.Const,
EnumUnderlyingType,
valueFieldName,
this,
lastContractProperty.Description,
Literal(literalValue));
member = new EnumTypeMember(lastContractProperty.Name, field, literalValue);
return true;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -249,13 +249,9 @@ protected internal override PropertyProvider[] BuildProperties()

private static ValueExpression? GetFieldInitializer(IFieldSymbol fieldSymbol)
Comment thread
jorgerangel-msft marked this conversation as resolved.
Outdated
{
if (fieldSymbol.ContainingType?.TypeKind == TypeKind.Enum)
if (fieldSymbol.HasConstantValue && fieldSymbol.ConstantValue != null)
{
if (fieldSymbol.HasConstantValue && fieldSymbol.ConstantValue != null)
{
return Literal(fieldSymbol.ConstantValue);
}
return null;
return Literal(fieldSymbol.ConstantValue);
}

return null;
Comment thread
jorgerangel-msft marked this conversation as resolved.
Outdated
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -825,7 +825,8 @@ internal void ProcessTypeForBackCompatibility()

IReadOnlyList<EnumTypeMember>? updatedEnumValues = null;
IEnumerable<FieldProvider>? newFields = null;
if (this is EnumProvider)
IEnumerable<PropertyProvider>? newProperties = null;
if (this is EnumProvider enumProvider)
{
var hasFields = LastContractView?.Fields != null && LastContractView.Fields.Count > 0;
if (hasFields)
Expand All @@ -848,21 +849,45 @@ internal void ProcessTypeForBackCompatibility()
updatedEnumValues = newEnumValues;
}

newFields = filteredFields;
// Sync the enum values before rebuilding the member collections from them.
_enumValues = updatedEnumValues;

if (enumProvider.IsExtensible)
{
// Extensible enums carry an extra backing `_value` field and surface members
// as properties, so rebuild both from the updated members. Reuse the
// already-visited field and property instances for members that still exist so
// any visitor mutations are preserved and only the restored members are
// (re)visited below.
var existingFields = new Dictionary<string, FieldProvider>(StringComparer.Ordinal);
foreach (var field in Fields)
{
existingFields[field.Name] = field;
Comment thread
jorgerangel-msft marked this conversation as resolved.
Outdated
}
newFields = ApplyCustomizationFilter(
BuildFields().Select(f => existingFields.TryGetValue(f.Name, out var existing) ? existing : f));

var existingProperties = new Dictionary<string, PropertyProvider>(StringComparer.Ordinal);
foreach (var property in Properties)
{
existingProperties[property.Name] = property;
Comment thread
jorgerangel-msft marked this conversation as resolved.
Outdated
}
newProperties = ApplyCustomizationFilter(
Comment thread
jorgerangel-msft marked this conversation as resolved.
BuildProperties().Select(p => existingProperties.TryGetValue(p.Name, out var existing) ? existing : p));
}
else
{
newFields = filteredFields;
}
}
}
}

var newMethods = hasMethods ? BuildMethodsForBackCompatibility(Methods) : null;
var newConstructors = hasConstructors ? BuildConstructorsForBackCompatibility(Constructors) : null;

if (newFields != null || newMethods != null || newConstructors != null)
if (newFields != null || newProperties != null || newMethods != null || newConstructors != null)
{
if (updatedEnumValues != null)
{
_enumValues = updatedEnumValues;
}
Comment thread
jorgerangel-msft marked this conversation as resolved.

// Back-compatibility processing intentionally runs after the library visitor pass so
// that the contract comparison uses the final, post-visitor member signatures (otherwise
// we could incorrectly decide whether a back-compat member is needed). As a result, any
Expand All @@ -877,12 +902,16 @@ internal void ProcessTypeForBackCompatibility()
{
newConstructors = VisitNewMembers(newConstructors, Constructors, static (member, visitor) => visitor.VisitConstructor(member));
}
if (newProperties != null)
{
newProperties = VisitNewMembers(newProperties, Properties, static (member, visitor) => visitor.VisitProperty(member));
}
if (newFields != null)
{
newFields = VisitNewMembers(newFields, Fields, static (member, visitor) => visitor.VisitField(member));
}

Update(fields: newFields, methods: newMethods, constructors: newConstructors);
Update(fields: newFields, properties: newProperties, methods: newMethods, constructors: newConstructors);
}

// Providers whose attributes depend on final generation decisions build their attributes at write
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ private class TestFilterVisitor : LibraryVisitor
return constructor;
}

protected override PropertyProvider? VisitProperty(PropertyProvider property)
protected internal override PropertyProvider? VisitProperty(PropertyProvider property)
{
if (property.Name == "TestProperty")
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,81 @@ await MockHelpers.LoadMockGeneratorAsync(
Assert.IsNull(fields[1].InitializationValue);
}

// Validates that a member removed from an extensible (string-backed) enum is re-added from the
// last contract. Unlike fixed string enums, an extensible enum stores its wire value in a private
// const `<Member>Value` field, so the value is recoverable (even from a compiled assembly's
// metadata) and the previously shipped member can be restored to avoid a source-breaking removal.
[Test]
public async Task BackCompat_ExtensibleEnumRemovedValueReadded()
{
await MockHelpers.LoadMockGeneratorAsync(
createCSharpTypeCore: (inputType) => typeof(string),
lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync());

// Last contract: Default, Recover, Third. Current input removes "Third".
var input = InputFactory.StringEnum("mockInputEnum", [
("Default", "default"),
("Recover", "recover"),
], isExtensible: true);

var enumType = EnumProvider.Create(input);
Assert.IsFalse(enumType is ApiVersionEnumProvider);

enumType.EnsureBuilt();
enumType.ProcessTypeForBackCompatibility();

// "Third" is re-added (appended after the current members) as a public static property.
var properties = enumType.Properties;
Assert.AreEqual(3, properties.Count);
Assert.AreEqual("Default", properties[0].Name);
Assert.AreEqual("Recover", properties[1].Name);
Assert.AreEqual("Third", properties[2].Name);

// The corresponding enum value carries the wire value recovered from the last contract.
var thirdMember = enumType.EnumValues.SingleOrDefault(v => v.Name == "Third");
Assert.IsNotNull(thirdMember);
Assert.AreEqual("third", thirdMember!.Value);

// Validate the full generated output, including the restored const `<Member>Value` field
// and the preserved backing `_value` field.
var content = new TypeProviderWriter(enumType).Write().Content;
Assert.AreEqual(Helpers.GetExpectedFromFile(), content);
}

// Validates that a removed extensible enum member is NOT re-added when its removal is accepted in
// the ApiCompat baseline (recorded as a MembersMustExist suppression), so the generator honors the
// intentional removal instead of resurrecting it.
[Test]
public async Task BackCompat_ExtensibleEnumRemovedValueNotReaddedWhenBaselineAccepts()
{
var baseline = Helpers.GetApiCompatBaselineFromFile();

await MockHelpers.LoadMockGeneratorAsync(
createCSharpTypeCore: (inputType) => typeof(string),
lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(),
apiCompatBaseline: baseline);

// Last contract: Default, Recover, Third. Current input removes "Third", but the baseline
// accepts that removal, so it must NOT be re-added.
var input = InputFactory.StringEnum("mockInputEnum", [
("Default", "default"),
("Recover", "recover"),
], isExtensible: true);

var enumType = EnumProvider.Create(input);
Assert.IsFalse(enumType is ApiVersionEnumProvider);

enumType.EnsureBuilt();
enumType.ProcessTypeForBackCompatibility();

var properties = enumType.Properties;
Assert.AreEqual(2, properties.Count);
Assert.IsFalse(properties.Any(p => p.Name == "Third"));
Assert.AreEqual("Default", properties[0].Name);
Assert.AreEqual("Recover", properties[1].Name);
Assert.IsFalse(enumType.Fields.Any(f => f.Name == "ThirdValue"));
Comment thread
jorgerangel-msft marked this conversation as resolved.
}

// Validates that a removed integer enum member is NOT re-added when its removal is accepted
// in the ApiCompat baseline (here recorded as an EnumValuesMustMatch suppression), so the
// generator honors the intentional removal instead of resurrecting it.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# The Third extensible-enum member was intentionally removed during migration; suppress the difference
Comment thread
jorgerangel-msft marked this conversation as resolved.
# so the back-compat system honors the removal instead of re-adding it. ApiCompat reports a removed
# extensible-enum member (a public static property) as a MembersMustExist difference on its getter.
MembersMustExist : Member 'public static Sample.Models.MockInputEnum Sample.Models.MockInputEnum.Third.get()' does not exist in the implementation but it does exist in the contract.
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#nullable disable

using System;

namespace Sample.Models
{
public readonly partial struct MockInputEnum : IEquatable<MockInputEnum>
{
private readonly string _value;
private const string DefaultValue = "default";
private const string RecoverValue = "recover";
private const string ThirdValue = "third";

public MockInputEnum(string value)
{
_value = value ?? throw new ArgumentNullException(nameof(value));
}

public static MockInputEnum Default { get; } = new MockInputEnum(DefaultValue);

public static MockInputEnum Recover { get; } = new MockInputEnum(RecoverValue);

public static MockInputEnum Third { get; } = new MockInputEnum(ThirdValue);

public bool Equals(MockInputEnum other) => string.Equals(_value, other._value, StringComparison.InvariantCultureIgnoreCase);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// <auto-generated/>

#nullable disable

using System;
using System.ComponentModel;
using Sample;

namespace Sample.Models
{
public readonly partial struct MockInputEnum : global::System.IEquatable<global::Sample.Models.MockInputEnum>
{
private readonly string _value;
private const string DefaultValue = "default";
private const string RecoverValue = "recover";
private const string ThirdValue = "third";

public MockInputEnum(string value)
{
global::Sample.Argument.AssertNotNull(value, nameof(value));

_value = value;
}

public static global::Sample.Models.MockInputEnum Default { get; } = new global::Sample.Models.MockInputEnum(DefaultValue);

public static global::Sample.Models.MockInputEnum Recover { get; } = new global::Sample.Models.MockInputEnum(RecoverValue);

public static global::Sample.Models.MockInputEnum Third { get; } = new global::Sample.Models.MockInputEnum(ThirdValue);

public static bool operator ==(global::Sample.Models.MockInputEnum left, global::Sample.Models.MockInputEnum right) => left.Equals(right);

public static bool operator !=(global::Sample.Models.MockInputEnum left, global::Sample.Models.MockInputEnum right) => !left.Equals(right);

public static implicit operator global::Sample.Models.MockInputEnum(string value) => new global::Sample.Models.MockInputEnum(value);

public static implicit operator global::Sample.Models.MockInputEnum?(string value) => (value == null) ? null : new global::Sample.Models.MockInputEnum(value);

[global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)]
public override bool Equals(object obj) => ((obj is global::Sample.Models.MockInputEnum other) && this.Equals(other));

public bool Equals(global::Sample.Models.MockInputEnum other) => string.Equals(_value, other._value, global::System.StringComparison.InvariantCultureIgnoreCase);

[global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)]
public override int GetHashCode() => (_value != null) ? global::System.StringComparer.InvariantCultureIgnoreCase.GetHashCode(_value) : 0;

public override string ToString() => _value;
}
}
Loading
Loading