Skip to content

Commit

Permalink
Detect never ending loops
Browse files Browse the repository at this point in the history
  • Loading branch information
GerardSmit committed Mar 23, 2024
1 parent 28761db commit 5e26068
Show file tree
Hide file tree
Showing 16 changed files with 200 additions and 5 deletions.
1 change: 1 addition & 0 deletions src/Zomp.SyncMethodGenerator/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
19 changes: 17 additions & 2 deletions src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory;
using Zomp.SyncMethodGenerator.Visitors;
using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory;

namespace Zomp.SyncMethodGenerator;

Expand Down Expand Up @@ -2081,11 +2082,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);
}

Expand All @@ -2095,10 +2098,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))
{
Expand All @@ -2109,6 +2114,16 @@ 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))
{
if (!StopIterationVisitor.Instance.Visit(ws.Statement))
{
rewriter.diagnostics.Add(ReportedDiagnostic.Create(EndlessLoop, ws.WhileKeyword.GetLocation()));
}

removeRemaining = true;
}
}

return ProcessStatements(statements, extraNodeInfoList);
Expand Down
9 changes: 9 additions & 0 deletions src/Zomp.SyncMethodGenerator/DiagnosticMessages.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
}
6 changes: 3 additions & 3 deletions src/Zomp.SyncMethodGenerator/Models/ReportedDiagnostic.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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"/>.
Expand All @@ -20,7 +20,7 @@ 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>
Expand All @@ -30,7 +30,7 @@ public static implicit operator Diagnostic(ReportedDiagnostic diagnostic)
/// <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(DiagnosticDescriptor descriptor, Location location, string? trivia = null)
{
return new(descriptor, location.SourceTree?.FilePath ?? string.Empty, location.SourceSpan, location.GetLineSpan().Span, trivia);
}
Expand Down
33 changes: 33 additions & 0 deletions src/Zomp.SyncMethodGenerator/Visitors/BreakVisitor.cs
Original file line number Diff line number Diff line change
@@ -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;
}
}
23 changes: 23 additions & 0 deletions src/Zomp.SyncMethodGenerator/Visitors/StopIterationVisitor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
namespace Zomp.SyncMethodGenerator.Visitors;

internal sealed class StopIterationVisitor : CSharpSyntaxVisitor<bool>
{
public static readonly StopIterationVisitor Instance = new();

public override bool VisitReturnStatement(ReturnStatementSyntax node) => true;

public override bool VisitThrowExpression(ThrowExpressionSyntax node) => true;

public override bool DefaultVisit(SyntaxNode node)
{
foreach (var child in node.ChildNodes())
{
if (Visit(child))
{
return true;
}
}

return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
19 changes: 19 additions & 0 deletions tests/GenerationSandbox.Tests/WhileNotCancelled.cs
Original file line number Diff line number Diff line change
@@ -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();
}
}
32 changes: 32 additions & 0 deletions tests/Generator.Tests/IsCancellationRequestedTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,37 @@ 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);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
Diagnostics: [
{
Id: ZSMGEN004,
Title: The while loop will never end after transformation,
Severity: Warning,
WarningLevel: 1,
Location: : (1,8)-(1,13),
MessageFormat: After transformation, it is detected that the while loop will never end,
Message: After transformation, it is detected that the while loop will never end,
Category: SyncMethodGenerator
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
//HintName: Test.Class.MethodAsync.g.cs
while (true)
{
global::System.Threading.Thread.Sleep(120000);
break;
}

throw new global::System.OperationCanceledException();
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
//HintName: Test.Class.MethodAsync.g.cs
while (true)
{
global::System.Threading.Thread.Sleep(120000);
while (true) break;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
Diagnostics: [
{
Id: ZSMGEN004,
Title: The while loop will never end after transformation,
Severity: Warning,
WarningLevel: 1,
Location: : (1,8)-(1,13),
MessageFormat: After transformation, it is detected that the while loop will never end,
Message: After transformation, it is detected that the while loop will never end,
Category: SyncMethodGenerator
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
//HintName: Test.Class.MethodAsync.g.cs
while (true)
{
global::System.Threading.Thread.Sleep(120000);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
Diagnostics: [
{
Id: ZSMGEN004,
Title: The while loop will never end after transformation,
Severity: Warning,
WarningLevel: 1,
Location: : (1,8)-(1,13),
MessageFormat: After transformation, it is detected that the while loop will never end,
Message: After transformation, it is detected that the while loop will never end,
Category: SyncMethodGenerator
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@


0 comments on commit 5e26068

Please sign in to comment.