From fa9971f668a6d5f4c0c88b20b55b3dc5812c4bca Mon Sep 17 00:00:00 2001 From: ArcturusZhang Date: Sun, 4 Feb 2024 16:26:11 +0800 Subject: [PATCH] refactor to breakingchangeresolver so that it is now just giving us the final list of methods --- .../Common/Input/Source/SourceTypeMapping.cs | 4 +- .../Models/Types/BreakingChangeResolver.cs | 60 +++++++------------ .../Models/Types/ModelFactoryTypeProvider.cs | 6 +- .../Output/Models/Types/SignatureType.cs | 6 +- 4 files changed, 27 insertions(+), 49 deletions(-) diff --git a/src/AutoRest.CSharp/Common/Input/Source/SourceTypeMapping.cs b/src/AutoRest.CSharp/Common/Input/Source/SourceTypeMapping.cs index 03cfa80064e..f1caa148e27 100644 --- a/src/AutoRest.CSharp/Common/Input/Source/SourceTypeMapping.cs +++ b/src/AutoRest.CSharp/Common/Input/Source/SourceTypeMapping.cs @@ -71,8 +71,8 @@ private static bool IsPropertyOrFieldSymbol(ISymbol member) private static bool IsMethodSymbol(ISymbol member) { - // here we exclude those "CompilerGenerated" members - return !member.IsImplicitlyDeclared && member is IMethodSymbol; + // here we exclude those "CompilerGenerated" members and ctor symbols + return !member.IsImplicitlyDeclared && member is IMethodSymbol method && method.MethodKind != MethodKind.Constructor; } public ISymbol? GetMemberByOriginalName(string name) diff --git a/src/AutoRest.CSharp/Common/Output/Models/Types/BreakingChangeResolver.cs b/src/AutoRest.CSharp/Common/Output/Models/Types/BreakingChangeResolver.cs index 9e62a72c104..60af72c19e9 100644 --- a/src/AutoRest.CSharp/Common/Output/Models/Types/BreakingChangeResolver.cs +++ b/src/AutoRest.CSharp/Common/Output/Models/Types/BreakingChangeResolver.cs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -using System; using System.Collections.Generic; using System.ComponentModel; using System.Diagnostics.CodeAnalysis; @@ -18,18 +17,12 @@ namespace AutoRest.CSharp.Output.Models.Types { /// - /// This type holds three portions of codes: - /// - current - /// - custom - /// - baseline contract - /// current union custom compare with baseline contract outputs the changeset, we can apply different rules with it. + /// The class consumes the generated methods and resolves the methods in customization and baseline contract to produce a full list of the methods we should generate. /// internal class BreakingChangeResolver { private readonly SignatureType _customizationType; private readonly SignatureType _baselineContractType; - private IEnumerable? _generatedMethods; - private MethodChangeset? _methodChangeset; /// /// Tracks the changeset of methods between the currently generated methods, and the methods read from the baseline contract @@ -51,25 +44,31 @@ public BreakingChangeResolver(TypeFactory typeFactory, SourceTypeMapping? source _baselineContractType = new SignatureType(typeFactory, contractTypeMapping); } - public void Build(IEnumerable generatedMethods) + public IEnumerable Resolve(IEnumerable generatedMethods) { - if (_generatedMethods is not null) + var generatedMethodSignatures = generatedMethods.Select(m => (MethodSignature)m.Signature); + var methodChangeset = CompareMethods(generatedMethodSignatures.Union(_customizationType.Methods, MethodSignature.ParameterAndReturnTypeEqualityComparer), _baselineContractType.Methods); + + var methodToSkip = generatedMethodSignatures.Intersect(_customizationType.Methods, MethodSignature.ParameterAndReturnTypeEqualityComparer).ToHashSet(MethodSignature.ParameterAndReturnTypeEqualityComparer); + foreach (var method in generatedMethods) { - throw new InvalidOperationException($"{nameof(BreakingChangeResolver)} could only be built once."); + if (methodToSkip.Contains((MethodSignature)method.Signature)) + continue; + + yield return method; } - _generatedMethods = generatedMethods; - _methodChangeset = CompareMethods(generatedMethods.Union(_customizationType.Methods, MethodSignature.ParameterAndReturnTypeEqualityComparer), _baselineContractType.Methods); - } - private IReadOnlyList? _overloadMethods; - public IReadOnlyList OverloadMethods => _overloadMethods ??= EnsureOverloadMethods(); + foreach (var method in BuildOverloadMethods(methodChangeset)) + { + yield return method; + } + } - private IReadOnlyList EnsureOverloadMethods() + private IEnumerable BuildOverloadMethods(MethodChangeset methodChangeset) { - var overloadMethods = new List(); - if (_methodChangeset?.Updated is not { } updated) + if (methodChangeset.Updated is not { } updated) { - return Array.Empty(); + yield break; } foreach (var (current, previous) in updated) @@ -78,10 +77,10 @@ private IReadOnlyList EnsureOverloadMethods() { var overloadMethodSignature = new OverloadMethodSignature(currentMethodToCall, previous.WithParametersRequired(), missingParameters, previous.Description); var previousMethodSignature = overloadMethodSignature.PreviousMethodSignature with { Attributes = new CSharpAttribute[] { new CSharpAttribute(typeof(EditorBrowsableAttribute), FrameworkEnumValue(EditorBrowsableState.Never)) } }; - overloadMethods.Add(new Method(previousMethodSignature, BuildOverloadMethodBody(overloadMethodSignature))); + + yield return new Method(previousMethodSignature, BuildOverloadMethodBody(overloadMethodSignature)); } } - return overloadMethods; } private MethodBodyStatement BuildOverloadMethodBody(OverloadMethodSignature overloadMethodSignature) @@ -103,17 +102,6 @@ private IReadOnlyList BuildOverloadMethodParameters(OverloadMet return parameters; } - private IReadOnlySet? _methodsToSkip; - public IReadOnlySet MethodsToSkip => _methodsToSkip ??= EnsureMethodsToSkip(); - private IReadOnlySet EnsureMethodsToSkip() - { - if (_generatedMethods == null) - { - throw new InvalidOperationException($"{nameof(BreakingChangeResolver)} has not been built yet."); - } - return _generatedMethods.Intersect(_customizationType.Methods, MethodSignature.ParameterAndReturnTypeEqualityComparer).ToHashSet(MethodSignature.ParameterAndReturnTypeEqualityComparer); - } - private bool TryGetPreviousMethodWithLessOptionalParameters(IReadOnlyList currentMethods, MethodSignature previousMethod, [NotNullWhen(true)] out MethodSignature? currentMethodToCall, [NotNullWhen(true)] out IReadOnlyList? missingParameters) { foreach (var item in currentMethods) @@ -165,12 +153,8 @@ private bool CurrentContainAllPreviousParameters(MethodSignature previousMethod, return true; } - private static MethodChangeset? CompareMethods(IEnumerable currentMethods, IEnumerable? previousMethods) + private static MethodChangeset CompareMethods(IEnumerable currentMethods, IEnumerable previousMethods) { - if (previousMethods is null) - { - return null; - } var missing = new List(); var updated = new List(); var set = currentMethods.ToHashSet(MethodSignature.ParameterAndReturnTypeEqualityComparer); diff --git a/src/AutoRest.CSharp/Common/Output/Models/Types/ModelFactoryTypeProvider.cs b/src/AutoRest.CSharp/Common/Output/Models/Types/ModelFactoryTypeProvider.cs index 8ece5de37ad..3909309b963 100644 --- a/src/AutoRest.CSharp/Common/Output/Models/Types/ModelFactoryTypeProvider.cs +++ b/src/AutoRest.CSharp/Common/Output/Models/Types/ModelFactoryTypeProvider.cs @@ -42,10 +42,8 @@ private IEnumerable BuildMethods() // The overloading feature is not enabled, we jsut return the original methods return generatedMethods; } - // build the resolver - BreakingChangeResolver.Build(generatedMethods.Select(x => (MethodSignature)x.Signature)); - // filter out duplicate methods in custom code and combine overload methods - return generatedMethods.Where(m => !BreakingChangeResolver.MethodsToSkip.Contains(m.Signature)).Concat(BreakingChangeResolver.OverloadMethods); + + return BreakingChangeResolver.Resolve(generatedMethods); } public FormattableString Description => $"Model factory for models."; diff --git a/src/AutoRest.CSharp/Common/Output/Models/Types/SignatureType.cs b/src/AutoRest.CSharp/Common/Output/Models/Types/SignatureType.cs index 28ad45d2add..6a23cbb7661 100644 --- a/src/AutoRest.CSharp/Common/Output/Models/Types/SignatureType.cs +++ b/src/AutoRest.CSharp/Common/Output/Models/Types/SignatureType.cs @@ -13,11 +13,7 @@ namespace AutoRest.CSharp.Output.Models.Types { /// - /// This type holds three portions of codes: - /// - current - /// - custom - /// - baseline contract - /// current union custom compare with baseline contract outputs the changeset, we can apply different rules with it. + /// The resolves the symbol to produce the MethodSignatures on the symbol /// internal class SignatureType {