Detect never ending loops

src/Zomp.SyncMethodGenerator/AnalyzerReleases.Unshipped.md

@@ -8,3 +8,4 @@ Rule ID | Category | Severity | Notes
 ZSMGEN001 | Preprocessor | Error | DiagnosticMessages
 ZSMGEN002 | Preprocessor | Error | DiagnosticMessages
 ZSMGEN003 | Preprocessor | Error | DiagnosticMessages
+ZSMGEN004 | SyncMethodGenerator | Warning | DiagnosticMessages

src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs

@@ -1,4 +1,5 @@
-using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory;
+using Zomp.SyncMethodGenerator.Visitors;
+using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory;
 
 namespace Zomp.SyncMethodGenerator;
 
@@ -92,6 +93,11 @@ private enum SpecialMethod
     /// </summary>
     public ImmutableArray<ReportedDiagnostic> Diagnostics => diagnostics.ToImmutable();
 
+    /// <summary>
+    /// Gets the path of the current file.
+    /// </summary>
+    public string Path => semanticModel.SyntaxTree.FilePath;
+
     /// <inheritdoc/>
     public override SyntaxNode? VisitConditionalAccessExpression(ConditionalAccessExpressionSyntax node)
     {
@@ -318,7 +324,7 @@ static BinaryExpressionSyntax CheckNull(ExpressionSyntax expr) => BinaryExpressi
         bool IsValidParameter(ParameterSyntax ps, int i)
         {
             var leading = ps.GetLeadingTrivia();
-            var extra = ProcessTrivia(leading, ds);
+            var extra = ProcessTrivia(leading, ds, ps);
             if (extra is not null && extra.AdditionalStatements.Count > 0)
             {
                 modifications.Add(i, new List<StatementSyntax>(extra.AdditionalStatements));
@@ -782,7 +788,7 @@ List<SyntaxTrivia> RemoveFirstEndIf(SyntaxTriviaList list)
         var retVal = @base.WithStatements(newStatements);
 
         var lastToken = retVal.CloseBraceToken;
-        if (ProcessTrivia(node.CloseBraceToken.LeadingTrivia, statementProcessor.DirectiveStack) is var (_, newStatements2, newTrivia))
+        if (ProcessTrivia(node.CloseBraceToken.LeadingTrivia, statementProcessor.DirectiveStack, node) is var (_, newStatements2, newTrivia, _))
         {
             var oldStatements = retVal.Statements.ToList();
             oldStatements.AddRange([.. newStatements2]);
@@ -1343,7 +1349,7 @@ private static SyntaxList<StatementSyntax> ProcessStatements(SyntaxList<Statemen
                 continue;
             }
 
-            var (statementGetsDropped, statements, trivia) = extra;
+            var (statementGetsDropped, statements, trivia, _) = extra;
 
             for (var j = 0; j < statements.Count; ++j)
             {
@@ -1669,7 +1675,7 @@ private bool PreProcess(
         for (var i = 0; i < statements.Count; ++i)
         {
             var statement = statements[i];
-            if (ProcessTrivia(statement.GetLeadingTrivia(), directiveStack) is not { } eni)
+            if (ProcessTrivia(statement.GetLeadingTrivia(), directiveStack, statement) is not { } eni)
             {
                 return false;
             }
@@ -1693,7 +1699,7 @@ private bool PreProcess(
                 eni = eni with { DropOriginal = true };
             }
 
-            extraNodeInfoList.Add(i, eni);
+            extraNodeInfoList.Add(i, eni with { OriginalStatement = statement });
         }
 
         return true;
@@ -1726,7 +1732,7 @@ BinaryExpressionSyntax be
 
                 if (syncOnlyDirectiveType == SyncOnlyDirectiveType.Invalid)
                 {
-                    var d = ReportedDiagnostic.Create(InvalidCondition, trivia.GetLocation(), trivia.ToString());
+                    var d = ReportedDiagnostic.Create(Path, InvalidCondition, trivia.GetLocation(), trivia.ToString());
                     diagnostics.Add(d);
                     return;
                 }
@@ -1737,7 +1743,7 @@ BinaryExpressionSyntax be
                     {
                         if (isStackSyncOnly ^ syncOnlyDirectiveType == SyncOnlyDirectiveType.SyncOnly)
                         {
-                            var d = ReportedDiagnostic.Create(InvalidNesting, trivia.GetLocation(), trivia.ToString());
+                            var d = ReportedDiagnostic.Create(Path, InvalidNesting, trivia.GetLocation(), trivia.ToString());
                             diagnostics.Add(d);
                             return;
                         }
@@ -1797,7 +1803,7 @@ BinaryExpressionSyntax be
                 }
                 else
                 {
-                    var d = ReportedDiagnostic.Create(InvalidElif, trivia.GetLocation(), trivia.ToString());
+                    var d = ReportedDiagnostic.Create(Path, InvalidElif, trivia.GetLocation(), trivia.ToString());
                     diagnostics.Add(d);
                     return;
                 }
@@ -1822,7 +1828,7 @@ BinaryExpressionSyntax be
         }
     }
 
-    private ExtraNodeInfo? ProcessTrivia(SyntaxTriviaList syntaxTriviaList, DirectiveStack directiveStack)
+    private ExtraNodeInfo? ProcessTrivia(SyntaxTriviaList syntaxTriviaList, DirectiveStack directiveStack, SyntaxNode originalNode)
     {
         var statements = new List<StatementSyntax>();
         var triviaList = new List<SyntaxTrivia>();
@@ -1847,7 +1853,7 @@ BinaryExpressionSyntax be
             statements.Add(statement);
         });
 
-        return new(false, List(statements), triviaList);
+        return new(false, List(statements), triviaList, originalNode);
     }
 
     private SyncOnlyAttributeContext ProcessSyncOnlyAttributes(SyntaxTriviaList syntaxTriviaList, DirectiveStack directiveStack)
@@ -2069,10 +2075,10 @@ private sealed class DirectiveStack()
 
     private sealed record SyncOnlyAttributeContext(SyntaxList<AttributeListSyntax> Attributes, IList<SyntaxTrivia> LeadingTrivia);
 
-    private sealed record ExtraNodeInfo(bool DropOriginal, SyntaxList<StatementSyntax> AdditionalStatements, IList<SyntaxTrivia> LeadingTrivia)
+    private sealed record ExtraNodeInfo(bool DropOriginal, SyntaxList<StatementSyntax> AdditionalStatements, IList<SyntaxTrivia> LeadingTrivia, SyntaxNode OriginalStatement)
     {
-        public ExtraNodeInfo(bool dropOriginal)
-            : this(dropOriginal, SyntaxFactory.List(Array.Empty<StatementSyntax>()), Array.Empty<SyntaxTrivia>())
+        public ExtraNodeInfo(bool dropOriginal, SyntaxNode originalStatement)
+            : this(dropOriginal, SyntaxFactory.List(Array.Empty<StatementSyntax>()), Array.Empty<SyntaxTrivia>(), originalStatement)
         {
         }
 
@@ -2081,11 +2087,13 @@ public ExtraNodeInfo(bool dropOriginal)
 
     private sealed class StatementProcessor
     {
+        private readonly AsyncToSyncRewriter rewriter;
         private readonly DirectiveStack directiveStack = new();
         private readonly Dictionary<int, ExtraNodeInfo> extraNodeInfoList = [];
 
         public StatementProcessor(AsyncToSyncRewriter rewriter, SyntaxList<StatementSyntax> statements)
         {
+            this.rewriter = rewriter;
             HasErrors = !rewriter.PreProcess(statements, extraNodeInfoList, directiveStack);
         }
 
@@ -2095,10 +2103,12 @@ public StatementProcessor(AsyncToSyncRewriter rewriter, SyntaxList<StatementSynt
 
         public SyntaxList<StatementSyntax> PostProcess(SyntaxList<StatementSyntax> statements)
         {
+            var removeRemaining = false;
+
             for (var i = 0; i < statements.Count; ++i)
             {
                 var statement = statements[i];
-                if (CanDropEmptyStatement(statement))
+                if (removeRemaining || CanDropEmptyStatement(statement))
                 {
                     if (extraNodeInfoList.TryGetValue(i, out var zz))
                     {
@@ -2109,6 +2119,21 @@ public SyntaxList<StatementSyntax> PostProcess(SyntaxList<StatementSyntax> state
                         extraNodeInfoList.Add(i, true);
                     }
                 }
+
+                if (!removeRemaining && statement is WhileStatementSyntax { Condition: LiteralExpressionSyntax ls } ws && ls.IsKind(SyntaxKind.TrueLiteralExpression) && !BreakVisitor.Instance.Visit(ws.Statement))
+                {
+                    var originalStatement = extraNodeInfoList.TryGetValue(i, out var zz) && zz.OriginalStatement is WhileStatementSyntax os
+                        ? os : null;
+
+                    if (originalStatement != null && !new StopIterationVisitor(rewriter.semanticModel).Visit(originalStatement.Statement))
+                    {
+                        var location = originalStatement.WhileKeyword.GetLocation() ?? Location.None;
+
+                        rewriter.diagnostics.Add(ReportedDiagnostic.Create(rewriter.Path, EndlessLoop, location));
+                    }
+
+                    removeRemaining = true;
+                }
             }
 
             return ProcessStatements(statements, extraNodeInfoList);

src/Zomp.SyncMethodGenerator/DiagnosticMessages.cs

@@ -26,5 +26,14 @@ internal static class DiagnosticMessages
         DiagnosticSeverity.Error,
         isEnabledByDefault: true);
 
+    internal static readonly DiagnosticDescriptor EndlessLoop = new(
+        id: "ZSMGEN004",
+        title: "While loop will never end after transformation",
+        messageFormat: "It is detected that the while loop will never end after transforming to synchronous version",
+        category: SyncMethodGenerator,
+        DiagnosticSeverity.Warning,
+        isEnabledByDefault: true);
+
     private const string Preprocessor = "Preprocessor";
+    private const string SyncMethodGenerator = "SyncMethodGenerator";
 }

src/Zomp.SyncMethodGenerator/Models/ReportedDiagnostic.cs

@@ -9,7 +9,7 @@
 /// <param name="LineSpan">Line span.</param>
 /// <param name="Trivia">Trivia.</param>
 /// <see href="https://github.com/dotnet/roslyn/issues/62269#issuecomment-1170760367" />
-internal sealed record ReportedDiagnostic(DiagnosticDescriptor Descriptor, string FilePath, TextSpan TextSpan, LinePositionSpan LineSpan, string Trivia)
+internal sealed record ReportedDiagnostic(DiagnosticDescriptor Descriptor, string FilePath, TextSpan TextSpan, LinePositionSpan LineSpan, string? Trivia = null)
 {
     /// <summary>
     /// Implicitly converts <see cref="ReportedDiagnostic"/> to <see cref="Diagnostic"/>.
@@ -20,18 +20,19 @@ public static implicit operator Diagnostic(ReportedDiagnostic diagnostic)
         return Diagnostic.Create(
             descriptor: diagnostic.Descriptor,
             location: Location.Create(diagnostic.FilePath, diagnostic.TextSpan, diagnostic.LineSpan),
-            messageArgs: new object[] { diagnostic.Trivia });
+            messageArgs: diagnostic.Trivia is null ? [] : [diagnostic.Trivia]);
     }
 
     /// <summary>
     /// Creates a new <see cref="ReportedDiagnostic"/> from <see cref="DiagnosticDescriptor"/> and <see cref="Location"/>.
     /// </summary>
+    /// <param name="filePath">File path.</param>
     /// <param name="descriptor">Descriptor.</param>
     /// <param name="location">Location.</param>
     /// <param name="trivia">Trivia.</param>
     /// <returns>A new <see cref="ReportedDiagnostic"/>.</returns>
-    public static ReportedDiagnostic Create(DiagnosticDescriptor descriptor, Location location, string trivia)
+    public static ReportedDiagnostic Create(string filePath, DiagnosticDescriptor descriptor, Location location, string? trivia = null)
     {
-        return new(descriptor, location.SourceTree?.FilePath ?? string.Empty, location.SourceSpan, location.GetLineSpan().Span, trivia);
+        return new(descriptor, filePath, location.SourceSpan, location.GetLineSpan().Span, trivia);
     }
 }

src/Zomp.SyncMethodGenerator/Visitors/BreakVisitor.cs

@@ -0,0 +1,33 @@
+namespace Zomp.SyncMethodGenerator.Visitors;
+
+internal sealed class BreakVisitor : CSharpSyntaxVisitor<bool>
+{
+    public static readonly BreakVisitor Instance = new();
+
+    public override bool VisitBreakStatement(BreakStatementSyntax node) => true;
+
+    public override bool VisitGotoStatement(GotoStatementSyntax node) => false;
+
+    public override bool VisitWhileStatement(WhileStatementSyntax node) => false;
+
+    public override bool VisitDoStatement(DoStatementSyntax node) => false;
+
+    public override bool VisitForStatement(ForStatementSyntax node) => false;
+
+    public override bool VisitForEachStatement(ForEachStatementSyntax node) => false;
+
+    public override bool VisitForEachVariableStatement(ForEachVariableStatementSyntax node) => false;
+
+    public override bool DefaultVisit(SyntaxNode node)
+    {
+        foreach (var child in node.ChildNodes())
+        {
+            if (Visit(child))
+            {
+                return true;
+            }
+        }
+
+        return false;
+    }
+}

src/Zomp.SyncMethodGenerator/Visitors/StopIterationVisitor.cs

@@ -0,0 +1,33 @@
+namespace Zomp.SyncMethodGenerator.Visitors;
+
+internal sealed class StopIterationVisitor(SemanticModel semanticModel) : CSharpSyntaxVisitor<bool>
+{
+    public override bool VisitReturnStatement(ReturnStatementSyntax node) => true;
+
+    public override bool VisitThrowExpression(ThrowExpressionSyntax node) => true;
+
+    public override bool VisitInvocationExpression(InvocationExpressionSyntax node)
+    {
+        var methodSymbol = semanticModel.GetSymbolInfo(node).Symbol as IMethodSymbol;
+
+        if (methodSymbol?.GetAttributes().Any(a => a.AttributeClass?.Name == "DoesNotReturnAttribute") ?? false)
+        {
+            return true;
+        }
+
+        return base.VisitInvocationExpression(node);
+    }
+
+    public override bool DefaultVisit(SyntaxNode node)
+    {
+        foreach (var child in node.ChildNodes())
+        {
+            if (Visit(child))
+            {
+                return true;
+            }
+        }
+
+        return false;
+    }
+}

tests/GenerationSandbox.Tests/GenerationSandbox.Tests.csproj

@@ -7,6 +7,7 @@
     <NoWarn>$(NoWarn);IDE0035</NoWarn>
     <NoWarn>$(NoWarn);RS1035</NoWarn>
     <NoWarn>$(NoWarn);SA1201;SA1402;SA1404</NoWarn>
+    <WarningsNotAsErrors>$(WarningsNotAsErrors);ZSMGEN004</WarningsNotAsErrors>
     <ImplicitUsings>false</ImplicitUsings>
     <TargetFrameworks>net7.0;net6.0</TargetFrameworks>
     <TargetFrameworks Condition="'$(OS)' == 'Windows_NT'">$(TargetFrameworks);net472</TargetFrameworks>

tests/GenerationSandbox.Tests/WhileNotCancelled.cs

@@ -0,0 +1,19 @@
+using System;
+using System.Threading;
+using System.Threading.Tasks;
+
+namespace GenerationSandbox.Tests;
+
+internal static partial class WhileNotCancelled
+{
+    [Zomp.SyncMethodGenerator.CreateSyncVersion]
+    public static async ValueTask SleepAsync(CancellationToken ct)
+    {
+        while (!ct.IsCancellationRequested)
+        {
+            await Task.Delay(120000, ct);
+        }
+
+        throw new OperationCanceledException();
+    }
+}

tests/Generator.Tests/IsCancellationRequestedTests.cs

@@ -25,4 +25,55 @@ public Task IfNotCancelled() => $$"""
     await Task.Delay(120000, ct);
 }
 """.Verify(sourceType: SourceType.MethodBody);
+
+    [Fact]
+    public Task WhileNotCancelledThrow() => $$"""
+while (!ct.IsCancellationRequested)
+{
+    await Task.Delay(120000, ct);
+}
+
+throw new OperationCanceledException();
+""".Verify(sourceType: SourceType.MethodBody);
+
+    [Fact]
+    public Task WhileNotCancelledBreakThrow() => $$"""
+while (!ct.IsCancellationRequested)
+{
+    await Task.Delay(120000, ct);
+    break;
+}
+
+throw new OperationCanceledException();
+""".Verify(sourceType: SourceType.MethodBody);
+
+    [Fact]
+    public Task WhileNotCancelledInvalidBreakThrow() => $$"""
+while (!ct.IsCancellationRequested)
+{
+    await Task.Delay(120000, ct);
+    while (true) break;
+}
+
+throw new OperationCanceledException();
+""".Verify(sourceType: SourceType.MethodBody);
+
+#if NET6_0_OR_GREATER
+
+    [Fact]
+    public Task WhileCancelled() => $$"""
+[System.Diagnostics.CodeAnalysis.DoesNotReturn]
+public void ThrowError() => throw new Exception();
+
+[Zomp.SyncMethodGenerator.CreateSyncVersion]
+public async Task CallProgressMethodAsync(CancellationToken ct)
+{
+    while (!ct.IsCancellationRequested)
+    {
+        ThrowError();
+    }
+}
+""".Verify();
+
+#endif
 }

tests/Generator.Tests/Snapshots/IsCancellationRequestedTests.WhileCancelled#CallProgressMethodAsync.g.verified.cs

@@ -0,0 +1,8 @@
+//HintName: Test.Class.CallProgressMethodAsync.g.cs
+public void CallProgressMethod()
+{
+    while (true)
+    {
+        ThrowError();
+    }
+}