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 @@ -57,6 +57,9 @@ public RestClientProvider(InputClient inputClient, ClientProvider clientProvider

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

protected override IReadOnlyList<MethodProvider> BuildMethodsForBackCompatibility(IEnumerable<MethodProvider> originalMethods)
=> [.. originalMethods];

protected override PropertyProvider[] BuildProperties()
{
return [.. _pipelineMessage20xClassifiers.Values.OrderBy(v => v.Name)];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2888,6 +2888,59 @@ public async Task BackCompatibility_ProtocolMethodParamOrderChanged()
result);
}

// The rest client is a partial of the same type as its ClientProvider. Back-compatibility must be
// applied only once (by the ClientProvider); otherwise the base pass would add duplicate overloads to
// the RestClientProvider partial (CS0111). Here the convenience delegation target lives in custom
// code shared by both partials, and the last contract published the nullable overload.
[Test]
public async Task BackCompatibility_RestClientProviderDoesNotDuplicateOverloads()
{
var enumType = InputFactory.StringEnum(
"fileFormatType",
[("Document", "Document"), ("Glossary", "Glossary")],
usage: InputModelTypeUsage.Input,
isExtensible: true);

var operation = InputFactory.Operation(
"GetData",
parameters: [InputFactory.QueryParameter("type", enumType, isRequired: true)],
responses: [InputFactory.OperationResponse([200], bodytype: InputPrimitiveType.String)]);

var method = InputFactory.BasicServiceMethod(
"GetData",
operation,
parameters: [InputFactory.MethodParameter("type", enumType, isRequired: true, location: InputRequestLocation.Query)]);

var client = InputFactory.Client(TestClientName, methods: [method]);

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

var clientProvider = generator.Object.OutputLibrary.TypeProviders.OfType<ClientProvider>().Single();
var restClientProvider = generator.Object.OutputLibrary.TypeProviders.OfType<RestClientProvider>().Single();

clientProvider.ProcessTypeForBackCompatibility();
restClientProvider.ProcessTypeForBackCompatibility();

static int NullableEnumOverloads(TypeProvider provider) => provider.Methods.Count(m =>
m.Signature.Name.StartsWith("GetData")
&& m.Signature.Parameters.Count >= 1
&& m.Signature.Parameters[0].Type is { IsValueType: true, IsNullable: true });

// The sync + async nullable back-compat overloads are added on the ClientProvider only.
Assert.AreEqual(2, NullableEnumOverloads(clientProvider));
Assert.AreEqual(0, NullableEnumOverloads(restClientProvider), "RestClientProvider must not add duplicate back-compat overloads.");

// Validate the generated output: the hidden nullable overloads appear on the ClientProvider
// partial and are absent from the RestClientProvider partial.
var clientFile = new TypeProviderWriter(new FilteredMethodsTypeProvider(clientProvider, name => name is "GetData" or "GetDataAsync")).Write();
var restClientFile = new TypeProviderWriter(new FilteredMethodsTypeProvider(restClientProvider, name => name is "GetData" or "GetDataAsync")).Write();
Assert.AreEqual(Helpers.GetExpectedFromFile("Client"), clientFile.Content);
Assert.AreEqual(Helpers.GetExpectedFromFile("RestClient"), restClientFile.Content);
}

[Test]
public async Task BackCompatibility_ConvenienceMethodParamOrderChanged()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// <auto-generated/>

#nullable disable

using System.ClientModel;
using System.ClientModel.Primitives;
using System.ComponentModel;
using System.Threading;
using System.Threading.Tasks;
using Sample.Models;

namespace Sample
{
public partial class TestClient
{
public virtual global::System.ClientModel.ClientResult GetData(string @type, global::System.ClientModel.Primitives.RequestOptions options = null)
{
using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateGetDataRequest(@type, options);
return global::System.ClientModel.ClientResult.FromResponse(Pipeline.ProcessMessage(message, options));
}

public virtual async global::System.Threading.Tasks.Task<global::System.ClientModel.ClientResult> GetDataAsync(string @type, global::System.ClientModel.Primitives.RequestOptions options = null)
{
using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateGetDataRequest(@type, options);
return global::System.ClientModel.ClientResult.FromResponse(await Pipeline.ProcessMessageAsync(message, options).ConfigureAwait(false));
}

#pragma warning disable AZC0002 // Back-compat overload preserves the previous method signature where CancellationToken was the trailing parameter. Making it optional would introduce an ambiguous call with the new method.
[global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)]
public virtual global::System.ClientModel.ClientResult<string> GetData(global::Sample.Models.FileFormatType? @type = default, global::System.Threading.CancellationToken cancellationToken = default)
{
global::Sample.Argument.AssertNotNull(@type, nameof(@type));
return this.GetData(@type: @type.Value, cancellationToken: cancellationToken);
}
#pragma warning restore AZC0002 // Back-compat overload preserves the previous method signature where CancellationToken was the trailing parameter. Making it optional would introduce an ambiguous call with the new method.

#pragma warning disable AZC0002 // Back-compat overload preserves the previous method signature where CancellationToken was the trailing parameter. Making it optional would introduce an ambiguous call with the new method.
[global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)]
public virtual global::System.Threading.Tasks.Task<global::System.ClientModel.ClientResult<string>> GetDataAsync(global::Sample.Models.FileFormatType? @type = default, global::System.Threading.CancellationToken cancellationToken = default)
{
global::Sample.Argument.AssertNotNull(@type, nameof(@type));
return this.GetDataAsync(@type: @type.Value, cancellationToken: cancellationToken);
}
#pragma warning restore AZC0002 // Back-compat overload preserves the previous method signature where CancellationToken was the trailing parameter. Making it optional would introduce an ambiguous call with the new method.
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
using System;
using System.ClientModel;
using System.Threading;
using System.Threading.Tasks;

namespace Sample.Models
{
public readonly partial struct FileFormatType
{
}
}

namespace Sample
{
public partial class TestClient
{
public virtual ClientResult<string> GetData(global::Sample.Models.FileFormatType type, CancellationToken cancellationToken = default)
{
throw new NotImplementedException();
}

public virtual Task<ClientResult<string>> GetDataAsync(global::Sample.Models.FileFormatType type, CancellationToken cancellationToken = default)
{
throw new NotImplementedException();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
using System;
using System.ClientModel;
using System.Threading;
using System.Threading.Tasks;

namespace Sample.Models
{
public readonly partial struct FileFormatType
{
}
}

namespace Sample
{
public partial class TestClient
{
public virtual ClientResult<string> GetData(global::Sample.Models.FileFormatType? type = default, CancellationToken cancellationToken = default)
{
throw new NotImplementedException();
}

public virtual Task<ClientResult<string>> GetDataAsync(global::Sample.Models.FileFormatType? type = default, CancellationToken cancellationToken = default)
{
throw new NotImplementedException();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// <auto-generated/>

#nullable disable

namespace Sample
{
public partial class TestClient
{
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,12 @@ public enum BackCompatibilityChangeCategory
/// <summary>A back-compat overload of a client method was added because new optional non-body parameter(s) were introduced relative to the last contract.</summary>
SvcMethodNewOptionalParameterOverloadAdded,

/// <summary>A back-compat overload of a client method was added because a value-type parameter's nullability was removed (e.g. <c>T?</c> -&gt; <c>T</c>) relative to the last contract.</summary>
SvcMethodParameterNullabilityChangeOverloadAdded,

/// <summary>A back-compat reduced-arity overload of a client method was added because a nullable parameter changed from optional to required relative to the last contract.</summary>
SvcMethodParameterOptionalityRestorationOverloadAdded,

/// <summary>A back-compat change was skipped because the removal was accepted in the ApiCompat baseline.</summary>
BaselineAcceptedRemovalSkipped,

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,8 @@ public void WriteBufferedMessages()
BackCompatibilityChangeCategory.ModelFactoryMethodAdded => "Model Factory Method Added For Back-Compat",
BackCompatibilityChangeCategory.ModelFactoryMethodSkipped => "Model Factory Method Back-Compat Skipped",
BackCompatibilityChangeCategory.SvcMethodNewOptionalParameterOverloadAdded => "Method Back-Compat Overload Added For New Optional Parameter",
BackCompatibilityChangeCategory.SvcMethodParameterNullabilityChangeOverloadAdded => "Method Back-Compat Overload Added For Parameter Nullability Change",
BackCompatibilityChangeCategory.SvcMethodParameterOptionalityRestorationOverloadAdded => "Method Back-Compat Overload Added For Parameter Optionality Change",
BackCompatibilityChangeCategory.BaselineAcceptedRemovalSkipped => "Back-Compat Skipped For ApiCompat Baseline Accepted Removal",
BackCompatibilityChangeCategory.EnumMemberAddedFromLastContract => "Enum Member Added From Last Contract",
_ => category.ToString(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,25 @@ public bool AreNamesEqual(CSharpType? other)
return true;
}

/// <summary>
/// Checks whether two <see cref="CSharpType"/> instances have equal names, optionally also requiring
/// their nullability to match. Unlike <see cref="Equals(CSharpType, bool)"/>, this compares only
/// names (and generic argument names), which is useful when comparing against a type read from source
/// that may not carry the same metadata (for example an extensible enum, read back as a struct).
/// </summary>
/// <param name="other">The instance to compare to.</param>
/// <param name="checkNullability">When <c>true</c>, the two types must also have the same <see cref="IsNullable"/> value.</param>
/// <returns><c>true</c> if the names (and, when requested, nullability) are equal; <c>false</c> otherwise.</returns>
internal bool AreNamesEqual(CSharpType? other, bool checkNullability)
{
if (!AreNamesEqual(other))
{
return false;
}

return !checkNullability || IsNullable == other!.IsNullable;
}

private bool IsNameMatch(CSharpType other)
{
if (string.IsNullOrEmpty(Namespace))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,18 @@ public void Update(string? name = default, FormattableString? description = defa
}
}

public static readonly IEqualityComparer<MethodSignatureBase> SignatureComparer = new MethodSignatureBaseEqualityComparer();
public static readonly IEqualityComparer<MethodSignatureBase> SignatureComparer = new MethodSignatureBaseEqualityComparer(checkNullability: false);
internal static readonly IEqualityComparer<MethodSignatureBase> SignatureComparerIncludingNullability = new MethodSignatureBaseEqualityComparer(checkNullability: true);

private class MethodSignatureBaseEqualityComparer : IEqualityComparer<MethodSignatureBase>
{
private readonly bool _checkNullability;

public MethodSignatureBaseEqualityComparer(bool checkNullability)
{
_checkNullability = checkNullability;
}

public bool Equals(MethodSignatureBase? x, MethodSignatureBase? y)
{
if (ReferenceEquals(x, y))
Expand Down Expand Up @@ -160,7 +168,7 @@ public bool Equals(MethodSignatureBase? x, MethodSignatureBase? y)

for (int i = 0; i < x.Parameters.Count; i++)
{
if (!x.Parameters[i].Type.AreNamesEqual(y.Parameters[i].Type))
if (!x.Parameters[i].Type.AreNamesEqual(y.Parameters[i].Type, _checkNullability))
{
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ public static MethodSignature BuildPartialSignature(
// Clones a ParameterProvider with a new name (and optionally without its default value)
// while preserving all generator metadata. Returns the source unchanged when no change is
// needed.
private static ParameterProvider CloneParameterWithName(
internal static ParameterProvider CloneParameterWithName(
ParameterProvider source,
string newName,
bool removeDefault)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -861,7 +861,7 @@ protected internal virtual IReadOnlyList<MethodProvider> BuildMethodsForBackComp
}

BackCompatHelper.RestorePreviousParameterNames(this, methods);
BackCompatHelper.AddOverloadsForNewOptionalParameters(this, methods);
BackCompatHelper.AddBackCompatOverloads(this, methods);

return methods;
}
Expand Down Expand Up @@ -994,7 +994,8 @@ private bool ShouldGenerate(MethodProvider method)
continue;
}

if (MethodSignatureBase.SignatureComparer.Equals(customMethod.Signature, method.Signature))
if (MethodSignatureBase.SignatureComparer.Equals(customMethod.Signature, method.Signature)
&& !MethodProviderHelpers.DiffersByValueTypeParameterNullability(customMethod.Signature, method.Signature))
{
return false;
}
Expand Down
Loading
Loading