Skip to content
Merged
Show file tree
Hide file tree
Changes from 15 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 @@ -38,6 +38,9 @@ protected override string BuildRelativeFilePath()
protected override IReadOnlyList<MethodProvider> BuildMethodsForBackCompatibility(IEnumerable<MethodProvider> originalMethods)
=> [.. originalMethods];

protected override IReadOnlyList<EnumTypeMember>? BuildEnumValuesForBackCompatibility(IReadOnlyList<EnumTypeMember> originalEnumValues)
=> base.BuildEnumValuesForBackCompatibility(originalEnumValues);

protected override MethodProvider[] BuildMethods()
{
// for string-based extensible enums, we are using `ToString` as its serialization
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1121,11 +1121,11 @@ private static void UpdateParameterNameWithBackCompat(InputParameter inputParame
// the current service method (allowing for sync/async pairing) so that a common
// parameter name (e.g. "id") on multiple methods can't cross-match.
var lastContractMethods = backCompatProvider.LastContractView?.Methods;
IEnumerable<MethodProvider>? scopedMethods = lastContractMethods;
if (lastContractMethods != null && serviceMethod != null)
IEnumerable<MethodProvider>? scopedMethods = lastContractMethods?.Where(m => MethodSignatureHelper.IsPublicApi(m.Signature.Modifiers));
if (scopedMethods != null && serviceMethod != null)
{
var serviceMethodName = serviceMethod.Name;
scopedMethods = lastContractMethods.Where(m =>
scopedMethods = scopedMethods.Where(m =>
string.Equals(m.Signature.Name, serviceMethodName, StringComparison.OrdinalIgnoreCase) ||
string.Equals(m.Signature.Name, serviceMethodName + "Async", StringComparison.OrdinalIgnoreCase));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,7 @@ private bool BuildNeedsBackCompatAdditionalProperties()
}

bool needsBackCompat = LastContractView.Properties.Any(p =>
MethodSignatureHelper.IsPublicApi(p.Modifiers) &&
p.Name == AdditionalPropertiesHelper.DefaultAdditionalPropertiesPropertyName);

if (needsBackCompat)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
using System.Reflection;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.TypeSpec.Generator.Expressions;
using Microsoft.TypeSpec.Generator.ClientModel.Providers;
using Microsoft.TypeSpec.Generator.Expressions;
using Microsoft.TypeSpec.Generator.Input;
using Microsoft.TypeSpec.Generator.Input.Extensions;
using Microsoft.TypeSpec.Generator.Primitives;
Expand Down Expand Up @@ -697,6 +697,34 @@ public async Task ParameterNamePreservedFromLastContractView()
"When 'oldParam' is preserved, the renamed 'newParam' must not appear.");
}

[Test]
public async Task ParameterNameNotPreservedFromInternalLastContractMethod()
{
var queryParam = InputFactory.QueryParameter("oldParam", InputPrimitiveType.String, isRequired: true);
queryParam.Update(name: "newParam");

var operation = InputFactory.Operation("GetSomething", parameters: [queryParam]);
var serviceMethod = InputFactory.BasicServiceMethod("GetSomething", operation);
var client = InputFactory.Client("TestClient", methods: [serviceMethod]);

var generator = await MockHelpers.LoadMockGeneratorAsync(
clients: () => [client],
lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync());

var clientProvider = generator.Object.OutputLibrary.TypeProviders.OfType<ClientProvider>().FirstOrDefault();
Assert.IsNotNull(clientProvider);
Assert.IsNotNull(clientProvider!.LastContractView);

var protocolParams = RestClientProvider.GetMethodParameters(serviceMethod, ScmMethodKind.Protocol, clientProvider!);

Assert.IsNotNull(
protocolParams.FirstOrDefault(p => string.Equals(p.Name, "newParam", StringComparison.Ordinal)),
"Parameter name should not be restored from internal last-contract methods.");
Assert.IsNull(
protocolParams.FirstOrDefault(p => string.Equals(p.Name, "oldParam", StringComparison.Ordinal)),
"Only public last-contract methods should be used for parameter name back compatibility.");
Comment thread
jorgerangel-msft marked this conversation as resolved.
Outdated
}

[Test]
public void ExactNameMethodParameterPreservedInRestClient()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#nullable disable

using System.ClientModel;
using System.ClientModel.Primitives;
using System.Threading.Tasks;

namespace Sample
{
public partial class TestClient
{
internal virtual Task<ClientResult> GetSomethingAsync(string oldParam, RequestOptions options = null) { return null; }
internal virtual ClientResult GetSomething(string oldParam, RequestOptions options = null) { return null; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License.

using System;
using System.Diagnostics.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;

namespace Microsoft.TypeSpec.Generator.Expressions
Expand All @@ -14,26 +15,43 @@ public sealed record LiteralExpression(object? Literal) : ValueExpression
{
internal override void Write(CodeWriter writer)
{
writer.AppendRaw(Literal switch
writer.AppendRaw(Format(Literal) ?? throw new NotImplementedException());
}

/// <summary>
/// Creates a <see cref="LiteralExpression"/> when <paramref name="value"/> maps to a renderable literal.
/// </summary>
internal static bool TryCreate(object? value, [NotNullWhen(true)] out LiteralExpression? literal)
{
if (Format(value) is null)
{
null => "null",
string s => SyntaxFactory.Literal(s).ToString(),
int i => SyntaxFactory.Literal(i).ToString(),
uint ui => SyntaxFactory.Literal(ui).ToString(),
long l => SyntaxFactory.Literal(l).ToString(),
ulong ul => SyntaxFactory.Literal(ul).ToString(),
byte b => SyntaxFactory.Literal((int)b).ToString(),
sbyte sb => SyntaxFactory.Literal((int)sb).ToString(),
short s => SyntaxFactory.Literal((int)s).ToString(),
ushort us => SyntaxFactory.Literal((uint)us).ToString(),
decimal d => SyntaxFactory.Literal(d).ToString(),
double d => SyntaxFactory.Literal(d).ToString(),
float f => SyntaxFactory.Literal(f).ToString(),
char c => SyntaxFactory.Literal(c).ToString(),
bool b => b ? "true" : "false",
BinaryData bd => bd.ToArray().Length == 0 ? "new byte[] { }" : SyntaxFactory.Literal(bd.ToString()).ToString(),
_ => throw new NotImplementedException()
});
literal = null;
return false;
}

literal = new LiteralExpression(value);
return true;
}

private static string? Format(object? literal) => literal switch
{
null => "null",
string s => SyntaxFactory.Literal(s).ToString(),
int i => SyntaxFactory.Literal(i).ToString(),
uint ui => SyntaxFactory.Literal(ui).ToString(),
long l => SyntaxFactory.Literal(l).ToString(),
ulong ul => SyntaxFactory.Literal(ul).ToString(),
byte b => SyntaxFactory.Literal((int)b).ToString(),
sbyte sb => SyntaxFactory.Literal((int)sb).ToString(),
short s => SyntaxFactory.Literal((int)s).ToString(),
ushort us => SyntaxFactory.Literal((uint)us).ToString(),
decimal d => SyntaxFactory.Literal(d).ToString(),
double d => SyntaxFactory.Literal(d).ToString(),
float f => SyntaxFactory.Literal(f).ToString(),
char c => SyntaxFactory.Literal(c).ToString(),
bool b => b ? "true" : "false",
BinaryData bd => bd.ToArray().Length == 0 ? "new byte[] { }" : SyntaxFactory.Literal(bd.ToString()).ToString(),
_ => null
};
}
}
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 @@ -233,7 +233,8 @@ .. _assemblyMetadataReferences.Value.Concat(CodeModelGenerator.Instance.Addition
project = project
.AddMetadataReferences(metadataReferences)
.WithCompilationOptions(new CSharpCompilationOptions(
OutputKind.DynamicallyLinkedLibrary, metadataReferenceResolver: _metadataReferenceResolver.Value, nullableContextOptions: NullableContextOptions.Disable));
OutputKind.DynamicallyLinkedLibrary, metadataReferenceResolver: _metadataReferenceResolver.Value, nullableContextOptions: NullableContextOptions.Disable)
.WithMetadataImportOptions(MetadataImportOptions.All));
return await project.GetCompilationAsync();
}

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,83 @@ 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
.Where(p => MethodSignatureHelper.IsPublicApi(p.Modifiers));

if (lastContractProperties == null || !lastContractProperties.Any())
{
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>? restoredMembers = 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))
{
(restoredMembers ??= []).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 (restoredMembers == null)
{
return null;
}

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

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,
initializationValue: Literal(literalValue));
member = new EnumTypeMember(lastContractProperty.Name, field, literalValue);
return true;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ protected internal sealed override IReadOnlyList<MethodProvider> BuildMethodsFor

foreach (var previousMethod in LastContractView.Methods)
{
if (currentMethodSignatures.Contains(previousMethod.Signature))
if (!MethodSignatureHelper.IsPublicApi(previousMethod.Signature.Modifiers) ||
currentMethodSignatures.Contains(previousMethod.Signature))
{
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ _inputModel.BaseModel is not null

private IDictionary<string, CSharpType> LastContractPropertiesMap
=> _lastContractPropertiesMap ??= LastContractView?.Properties
.Where(p => MethodProviderHelpers.IsPublicApi(p.Modifiers))
.Where(p => MethodSignatureHelper.IsPublicApi(p.Modifiers))
.ToDictionary(p => p.Name, p => p.Type) ?? [];

private IDictionary<string, CSharpType>? _lastContractPropertiesMap;
Expand Down Expand Up @@ -629,9 +629,8 @@ protected internal override PropertyProvider[] BuildProperties()

// Apply back-compat type replacement only for properties on the public API
// surface: changing the type of an internal/private generated property is not
// a source-breaking change, and the last-contract map already excludes
// non-public-API entries.
if (MethodProviderHelpers.IsPublicApi(outputProperty.Modifiers) &&
// a source-breaking change
if (MethodSignatureHelper.IsPublicApi(outputProperty.Modifiers) &&
LastContractPropertiesMap.TryGetValue(outputProperty.Name, out var lastContractPropertyType) &&
!lastContractPropertyType.Equals(outputProperty.Type))
{
Expand Down Expand Up @@ -1380,6 +1379,7 @@ private bool ShouldUseObjectAdditionalProperties()

// Check if the property exists in the last contract by name
var lastContractProperty = LastContractView.Properties.FirstOrDefault(p =>
MethodSignatureHelper.IsPublicApi(p.Modifiers) &&
p.Name == AdditionalPropertiesHelper.DefaultAdditionalPropertiesPropertyName);

if (lastContractProperty == null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,18 +247,13 @@ protected internal override PropertyProvider[] BuildProperties()
return null;
}

private static ValueExpression? GetFieldInitializer(IFieldSymbol fieldSymbol)
private static LiteralExpression? GetFieldInitializer(IFieldSymbol fieldSymbol)
{
if (fieldSymbol.ContainingType?.TypeKind == TypeKind.Enum)
{
if (fieldSymbol.HasConstantValue && fieldSymbol.ConstantValue != null)
{
return Literal(fieldSymbol.ConstantValue);
}
return null;
}

return null;
return fieldSymbol.HasConstantValue &&
fieldSymbol.ConstantValue != null &&
LiteralExpression.TryCreate(fieldSymbol.ConstantValue, out var initializer)
? initializer
: null;
}

private static string? GetOriginalName(ISymbol symbol)
Expand Down
Loading
Loading