From 47c81970e5bd57a8857e4b06b9f88f03616d6bbe Mon Sep 17 00:00:00 2001 From: Gerard Smit Date: Thu, 25 Jan 2024 21:25:54 +0100 Subject: [PATCH 01/10] Fix dropping default ValueTask --- src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs | 6 ++++++ ...edCompletedValueTask#DoSomethingAsync.g.verified.cs | 2 ++ tests/Generator.Tests/UnitTests.cs | 10 ++++++++++ 3 files changed, 18 insertions(+) create mode 100644 tests/Generator.Tests/Snapshots/UnitTests.DropUnawaitedCompletedValueTask#DoSomethingAsync.g.verified.cs diff --git a/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs b/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs index e70c067..41f5b93 100644 --- a/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs +++ b/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs @@ -1158,6 +1158,7 @@ private static bool ShouldRemoveType(ITypeSymbol symbol) private static bool ShouldRemoveArgument(ISymbol symbol) => symbol switch { + IPropertySymbol { Name: "CompletedTask", ContainingType.Name: "Task" or "ValueTask" } => true, IMethodSymbol ms => IsSpecialMethod(ms) == SpecialMethod.None && ((ShouldRemoveType(ms.ReturnType) && ms.MethodKind != MethodKind.LocalFunction) @@ -1519,9 +1520,14 @@ private bool ShouldRemoveArrowExpression(ArrowExpressionClauseSyntax? arrowNulla AwaitExpressionSyntax ae => ShouldRemoveArgument(ae.Expression), AssignmentExpressionSyntax ae => ShouldRemoveArgument(ae.Right), GenericNameSyntax gn => HasSymbolAndShouldBeRemoved(gn), + LiteralExpressionSyntax le => ShouldRemoveLiteral(le), _ => false, }; + private bool ShouldRemoveLiteral(LiteralExpressionSyntax literalExpression) + => literalExpression.Token.IsKind(SyntaxKind.DefaultKeyword) + && semanticModel.GetTypeInfo(literalExpression).Type is { Name: "ValueTask" }; + /// /// Keeps track of nested directives. /// diff --git a/tests/Generator.Tests/Snapshots/UnitTests.DropUnawaitedCompletedValueTask#DoSomethingAsync.g.verified.cs b/tests/Generator.Tests/Snapshots/UnitTests.DropUnawaitedCompletedValueTask#DoSomethingAsync.g.verified.cs new file mode 100644 index 0000000..e6230c5 --- /dev/null +++ b/tests/Generator.Tests/Snapshots/UnitTests.DropUnawaitedCompletedValueTask#DoSomethingAsync.g.verified.cs @@ -0,0 +1,2 @@ +//HintName: Test.Class.DoSomethingAsync.g.cs +public static void DoSomething() { } diff --git a/tests/Generator.Tests/UnitTests.cs b/tests/Generator.Tests/UnitTests.cs index 060f663..9836bb6 100644 --- a/tests/Generator.Tests/UnitTests.cs +++ b/tests/Generator.Tests/UnitTests.cs @@ -106,6 +106,16 @@ public async Task ExecAsync(CancellationToken ct) public Task DropUnawaitedCompletedTask(string statement) => $""" [Zomp.SyncMethodGenerator.CreateSyncVersion] public static Task DoSomethingAsync() {statement} +""".Verify(disableUnique: true); + + [Theory] + [InlineData("{ return default; }")] + [InlineData("{ return ValueTask.CompletedTask; }")] + [InlineData("{ return ValueTask.CompletedTask; Console.WriteLine(\"123\"); }")] + [InlineData("=> ValueTask.CompletedTask;")] + public Task DropUnawaitedCompletedValueTask(string statement) => $""" +[Zomp.SyncMethodGenerator.CreateSyncVersion] +public static ValueTask DoSomethingAsync() {statement} """.Verify(disableUnique: true); [Fact] From d42e8abdcf390f616ce20a3a39e8b44ee410f437 Mon Sep 17 00:00:00 2001 From: Gerard Smit Date: Thu, 25 Jan 2024 21:35:43 +0100 Subject: [PATCH 02/10] Don't drop default when there is a return type --- src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs | 2 +- ...epDefaultValueTaskWithResult#ReturnDefault.g.verified.cs | 2 ++ tests/Generator.Tests/UnitTests.cs | 6 ++++++ 3 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 tests/Generator.Tests/Snapshots/UnitTests.KeepDefaultValueTaskWithResult#ReturnDefault.g.verified.cs diff --git a/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs b/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs index 41f5b93..d2ebbbe 100644 --- a/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs +++ b/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs @@ -1526,7 +1526,7 @@ private bool ShouldRemoveArrowExpression(ArrowExpressionClauseSyntax? arrowNulla private bool ShouldRemoveLiteral(LiteralExpressionSyntax literalExpression) => literalExpression.Token.IsKind(SyntaxKind.DefaultKeyword) - && semanticModel.GetTypeInfo(literalExpression).Type is { Name: "ValueTask" }; + && semanticModel.GetTypeInfo(literalExpression).Type is INamedTypeSymbol { Name: "ValueTask", IsGenericType: false }; /// /// Keeps track of nested directives. diff --git a/tests/Generator.Tests/Snapshots/UnitTests.KeepDefaultValueTaskWithResult#ReturnDefault.g.verified.cs b/tests/Generator.Tests/Snapshots/UnitTests.KeepDefaultValueTaskWithResult#ReturnDefault.g.verified.cs new file mode 100644 index 0000000..78aabf8 --- /dev/null +++ b/tests/Generator.Tests/Snapshots/UnitTests.KeepDefaultValueTaskWithResult#ReturnDefault.g.verified.cs @@ -0,0 +1,2 @@ +//HintName: Test.Class.ReturnDefault.g.cs +public static int ReturnDefault() => default; diff --git a/tests/Generator.Tests/UnitTests.cs b/tests/Generator.Tests/UnitTests.cs index 9836bb6..b8262a5 100644 --- a/tests/Generator.Tests/UnitTests.cs +++ b/tests/Generator.Tests/UnitTests.cs @@ -118,6 +118,12 @@ public Task DropUnawaitedCompletedValueTask(string statement) => $""" public static ValueTask DoSomethingAsync() {statement} """.Verify(disableUnique: true); + [Fact] + public Task KeepDefaultValueTaskWithResult() => $""" +[Zomp.SyncMethodGenerator.CreateSyncVersion] +public static ValueTask ReturnDefault() => default; +""".Verify(); + [Fact] public Task MultipleClasses() => """ namespace NsOne From ff91d4d1d60b6f0023d2df910bc641211055bbc2 Mon Sep 17 00:00:00 2001 From: Gerard Smit Date: Thu, 25 Jan 2024 21:39:06 +0100 Subject: [PATCH 03/10] Validate namespace --- src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs b/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs index d2ebbbe..fddd67e 100644 --- a/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs +++ b/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs @@ -1158,7 +1158,7 @@ private static bool ShouldRemoveType(ITypeSymbol symbol) private static bool ShouldRemoveArgument(ISymbol symbol) => symbol switch { - IPropertySymbol { Name: "CompletedTask", ContainingType.Name: "Task" or "ValueTask" } => true, + IPropertySymbol { Name: "CompletedTask" } ps => ps.Type.ToString() is TaskType or ValueTaskType, IMethodSymbol ms => IsSpecialMethod(ms) == SpecialMethod.None && ((ShouldRemoveType(ms.ReturnType) && ms.MethodKind != MethodKind.LocalFunction) @@ -1526,7 +1526,8 @@ private bool ShouldRemoveArrowExpression(ArrowExpressionClauseSyntax? arrowNulla private bool ShouldRemoveLiteral(LiteralExpressionSyntax literalExpression) => literalExpression.Token.IsKind(SyntaxKind.DefaultKeyword) - && semanticModel.GetTypeInfo(literalExpression).Type is INamedTypeSymbol { Name: "ValueTask", IsGenericType: false }; + && semanticModel.GetTypeInfo(literalExpression).Type is INamedTypeSymbol { Name: "ValueTask", IsGenericType: false } t + && t.ToString() == ValueTaskType; /// /// Keeps track of nested directives. From c3237165af0d39330bd60580da315a0e524f21d6 Mon Sep 17 00:00:00 2001 From: Gerard Smit Date: Thu, 25 Jan 2024 22:06:38 +0100 Subject: [PATCH 04/10] Drop object creation of ValueTask --- .../AsyncToSyncRewriter.cs | 34 +++++++++++++++++++ ...alueTaskInstance#ReturnAsync.g.verified.cs | 2 ++ tests/Generator.Tests/UnitTests.cs | 9 +++++ 3 files changed, 45 insertions(+) create mode 100644 tests/Generator.Tests/Snapshots/UnitTests.ReturnValueTaskInstance#ReturnAsync.g.verified.cs diff --git a/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs b/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs index fddd67e..26e9cfa 100644 --- a/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs +++ b/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs @@ -498,11 +498,31 @@ bool IsValidParameter(ParameterSyntax ps) return @base; } + /// + public override SyntaxNode? VisitImplicitObjectCreationExpression(ImplicitObjectCreationExpressionSyntax node) + { + var @base = base.VisitImplicitObjectCreationExpression(node); + var symbol = GetSymbol(node); + + if (ShouldRemoveObjectCreation(node, symbol, out var expression)) + { + return expression; + } + + return @base; + } + /// public override SyntaxNode? VisitObjectCreationExpression(ObjectCreationExpressionSyntax node) { var @base = (ObjectCreationExpressionSyntax)base.VisitObjectCreationExpression(node)!; var symbol = GetSymbol(node); + + if (ShouldRemoveObjectCreation(node, symbol, out var expression)) + { + return expression; + } + if (symbol is null or { ContainingType.IsGenericType: true } or INamedTypeSymbol { IsGenericType: true }) @@ -1202,6 +1222,20 @@ private static TypeSyntax GetReturnType(TypeSyntax returnType, INamedTypeSymbol private static string Global(string type) => $"global::{type}"; + private static bool ShouldRemoveObjectCreation(BaseObjectCreationExpressionSyntax node, ISymbol? symbol, out SyntaxNode? singleArgExpression) + { + if (symbol is IMethodSymbol { ReceiverType: INamedTypeSymbol { Name: "ValueTask", IsGenericType: true } type } + && GetNameWithoutTypeParams(type) is ValueTaskType + && node.ArgumentList is { Arguments: [var singleArg] }) + { + singleArgExpression = singleArg.Expression; + return true; + } + + singleArgExpression = default; + return false; + } + private static InvocationExpressionSyntax UnwrapExtension(InvocationExpressionSyntax ies, bool isMemory, IMethodSymbol reducedFrom, ExpressionSyntax expression) { var arguments = ies.ArgumentList.Arguments; diff --git a/tests/Generator.Tests/Snapshots/UnitTests.ReturnValueTaskInstance#ReturnAsync.g.verified.cs b/tests/Generator.Tests/Snapshots/UnitTests.ReturnValueTaskInstance#ReturnAsync.g.verified.cs new file mode 100644 index 0000000..24f7972 --- /dev/null +++ b/tests/Generator.Tests/Snapshots/UnitTests.ReturnValueTaskInstance#ReturnAsync.g.verified.cs @@ -0,0 +1,2 @@ +//HintName: Test.Class.ReturnAsync.g.cs +public static int Return() { return 1; } diff --git a/tests/Generator.Tests/UnitTests.cs b/tests/Generator.Tests/UnitTests.cs index b8262a5..febd3b4 100644 --- a/tests/Generator.Tests/UnitTests.cs +++ b/tests/Generator.Tests/UnitTests.cs @@ -124,6 +124,15 @@ public Task KeepDefaultValueTaskWithResult() => $""" public static ValueTask ReturnDefault() => default; """.Verify(); + [Theory] + [InlineData("{ return new(1); }")] + [InlineData("{ return new ValueTask(1); }")] + [InlineData("{ return Task.FromResult(1); }")] + public Task ReturnValueTaskInstance(string statement) => $""" +[Zomp.SyncMethodGenerator.CreateSyncVersion] +public static ValueTask ReturnAsync() {statement} +""".Verify(disableUnique: true); + [Fact] public Task MultipleClasses() => """ namespace NsOne From 2d2443b6e8434915657355dddbfc8e653838da9e Mon Sep 17 00:00:00 2001 From: Gerard Smit Date: Thu, 25 Jan 2024 22:18:41 +0100 Subject: [PATCH 05/10] Only test ValueTask.CompletedTask on .NET Core ValueTask.CompletedTask is not available in .NET Framework --- tests/Generator.Tests/UnitTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/Generator.Tests/UnitTests.cs b/tests/Generator.Tests/UnitTests.cs index febd3b4..115f970 100644 --- a/tests/Generator.Tests/UnitTests.cs +++ b/tests/Generator.Tests/UnitTests.cs @@ -110,9 +110,11 @@ public static Task DoSomethingAsync() {statement} [Theory] [InlineData("{ return default; }")] +#if NETCOREAPP1_0_OR_GREATER [InlineData("{ return ValueTask.CompletedTask; }")] [InlineData("{ return ValueTask.CompletedTask; Console.WriteLine(\"123\"); }")] [InlineData("=> ValueTask.CompletedTask;")] +#endif public Task DropUnawaitedCompletedValueTask(string statement) => $""" [Zomp.SyncMethodGenerator.CreateSyncVersion] public static ValueTask DoSomethingAsync() {statement} From b5b698b2e721b502efde7fb7a964e89fb9030940 Mon Sep 17 00:00:00 2001 From: Gerard Smit Date: Thu, 25 Jan 2024 22:28:11 +0100 Subject: [PATCH 06/10] Renamed ShouldRemoveObjectCreation --- .../AsyncToSyncRewriter.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs b/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs index 26e9cfa..1a379bf 100644 --- a/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs +++ b/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs @@ -504,9 +504,9 @@ bool IsValidParameter(ParameterSyntax ps) var @base = base.VisitImplicitObjectCreationExpression(node); var symbol = GetSymbol(node); - if (ShouldRemoveObjectCreation(node, symbol, out var expression)) + if (TryReplaceObjectCreation(node, symbol, out var replacement)) { - return expression; + return replacement; } return @base; @@ -518,9 +518,9 @@ bool IsValidParameter(ParameterSyntax ps) var @base = (ObjectCreationExpressionSyntax)base.VisitObjectCreationExpression(node)!; var symbol = GetSymbol(node); - if (ShouldRemoveObjectCreation(node, symbol, out var expression)) + if (TryReplaceObjectCreation(node, symbol, out var replacement)) { - return expression; + return replacement; } if (symbol is null @@ -1222,17 +1222,17 @@ private static TypeSyntax GetReturnType(TypeSyntax returnType, INamedTypeSymbol private static string Global(string type) => $"global::{type}"; - private static bool ShouldRemoveObjectCreation(BaseObjectCreationExpressionSyntax node, ISymbol? symbol, out SyntaxNode? singleArgExpression) + private static bool TryReplaceObjectCreation(BaseObjectCreationExpressionSyntax node, ISymbol? symbol, out SyntaxNode? replacement) { if (symbol is IMethodSymbol { ReceiverType: INamedTypeSymbol { Name: "ValueTask", IsGenericType: true } type } && GetNameWithoutTypeParams(type) is ValueTaskType && node.ArgumentList is { Arguments: [var singleArg] }) { - singleArgExpression = singleArg.Expression; + replacement = singleArg.Expression; return true; } - singleArgExpression = default; + replacement = default; return false; } From 2e7c0cbb8bb0a6d366c763287eac70f961a9c671 Mon Sep 17 00:00:00 2001 From: Gerard Smit Date: Thu, 25 Jan 2024 22:32:48 +0100 Subject: [PATCH 07/10] Move string to a constant --- src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs b/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs index 1a379bf..a1cea63 100644 --- a/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs +++ b/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs @@ -27,6 +27,7 @@ internal sealed class AsyncToSyncRewriter(SemanticModel semanticModel) : CSharpS private const string IAsyncEnumerator = "System.Collections.Generic.IAsyncEnumerator"; private const string FromResult = "FromResult"; private const string Delay = "Delay"; + private const string CompletedTask = "CompletedTask"; private static readonly HashSet Drops = [IProgressInterface, CancellationTokenType]; private static readonly HashSet InterfacesToDrop = [IProgressInterface, IAsyncResultInterface]; private static readonly Dictionary Replacements = new() @@ -1178,7 +1179,7 @@ private static bool ShouldRemoveType(ITypeSymbol symbol) private static bool ShouldRemoveArgument(ISymbol symbol) => symbol switch { - IPropertySymbol { Name: "CompletedTask" } ps => ps.Type.ToString() is TaskType or ValueTaskType, + IPropertySymbol { Name: CompletedTask } ps => ps.Type.ToString() is TaskType or ValueTaskType, IMethodSymbol ms => IsSpecialMethod(ms) == SpecialMethod.None && ((ShouldRemoveType(ms.ReturnType) && ms.MethodKind != MethodKind.LocalFunction) From 2a89966945209b9b85fc2df30202bd4be0f9933c Mon Sep 17 00:00:00 2001 From: Gerard Smit Date: Thu, 25 Jan 2024 23:58:38 +0100 Subject: [PATCH 08/10] Remove new ValueTask --- src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs | 7 ++++++- tests/Generator.Tests/UnitTests.cs | 2 ++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs b/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs index a1cea63..ba8d0be 100644 --- a/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs +++ b/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs @@ -1550,7 +1550,8 @@ private bool ShouldRemoveArrowExpression(ArrowExpressionClauseSyntax? arrowNulla MemberAccessExpressionSyntax mae => ShouldRemoveArgument(mae.Name), PostfixUnaryExpressionSyntax pue => ShouldRemoveArgument(pue.Operand), PrefixUnaryExpressionSyntax pue => ShouldRemoveArgument(pue.Operand), - ObjectCreationExpressionSyntax oe => ShouldRemoveArgument(oe.Type), + ObjectCreationExpressionSyntax oe => ShouldRemoveArgument(oe.Type) || ShouldRemoveObjectCreation(oe), + ImplicitObjectCreationExpressionSyntax oe => ShouldRemoveObjectCreation(oe), ConditionalAccessExpressionSyntax cae => ShouldRemoveArgument(cae.Expression), AwaitExpressionSyntax ae => ShouldRemoveArgument(ae.Expression), AssignmentExpressionSyntax ae => ShouldRemoveArgument(ae.Right), @@ -1564,6 +1565,10 @@ private bool ShouldRemoveLiteral(LiteralExpressionSyntax literalExpression) && semanticModel.GetTypeInfo(literalExpression).Type is INamedTypeSymbol { Name: "ValueTask", IsGenericType: false } t && t.ToString() == ValueTaskType; + private bool ShouldRemoveObjectCreation(BaseObjectCreationExpressionSyntax oe) + => GetSymbol(oe) is IMethodSymbol { ReceiverType: INamedTypeSymbol { Name: "ValueTask", IsGenericType: false } type } + && type.ToString() is ValueTaskType; + /// /// Keeps track of nested directives. /// diff --git a/tests/Generator.Tests/UnitTests.cs b/tests/Generator.Tests/UnitTests.cs index 115f970..ee2b34b 100644 --- a/tests/Generator.Tests/UnitTests.cs +++ b/tests/Generator.Tests/UnitTests.cs @@ -110,6 +110,8 @@ public static Task DoSomethingAsync() {statement} [Theory] [InlineData("{ return default; }")] + [InlineData("{ return new(); }")] + [InlineData("{ return new ValueTask(); }")] #if NETCOREAPP1_0_OR_GREATER [InlineData("{ return ValueTask.CompletedTask; }")] [InlineData("{ return ValueTask.CompletedTask; Console.WriteLine(\"123\"); }")] From 6977d1dfa1c6f376293bfd83f4c6795be9da1d5e Mon Sep 17 00:00:00 2001 From: Gerard Smit Date: Fri, 26 Jan 2024 00:00:59 +0100 Subject: [PATCH 09/10] Fix incorrect test --- tests/Generator.Tests/UnitTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Generator.Tests/UnitTests.cs b/tests/Generator.Tests/UnitTests.cs index ee2b34b..e60a0a8 100644 --- a/tests/Generator.Tests/UnitTests.cs +++ b/tests/Generator.Tests/UnitTests.cs @@ -131,7 +131,7 @@ public Task KeepDefaultValueTaskWithResult() => $""" [Theory] [InlineData("{ return new(1); }")] [InlineData("{ return new ValueTask(1); }")] - [InlineData("{ return Task.FromResult(1); }")] + [InlineData("{ return ValueTask.FromResult(1); }")] public Task ReturnValueTaskInstance(string statement) => $""" [Zomp.SyncMethodGenerator.CreateSyncVersion] public static ValueTask ReturnAsync() {statement} From 4607909a20d3d5627c5d0c2681a1012207f49bdf Mon Sep 17 00:00:00 2001 From: Gerard Smit Date: Fri, 26 Jan 2024 00:07:49 +0100 Subject: [PATCH 10/10] Fix broken test --- tests/Generator.Tests/UnitTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/Generator.Tests/UnitTests.cs b/tests/Generator.Tests/UnitTests.cs index e60a0a8..07cb056 100644 --- a/tests/Generator.Tests/UnitTests.cs +++ b/tests/Generator.Tests/UnitTests.cs @@ -131,7 +131,9 @@ public Task KeepDefaultValueTaskWithResult() => $""" [Theory] [InlineData("{ return new(1); }")] [InlineData("{ return new ValueTask(1); }")] +#if NETCOREAPP1_0_OR_GREATER [InlineData("{ return ValueTask.FromResult(1); }")] +#endif public Task ReturnValueTaskInstance(string statement) => $""" [Zomp.SyncMethodGenerator.CreateSyncVersion] public static ValueTask ReturnAsync() {statement}