Skip to content

Commit

Permalink
Prevent SemaphoreSlim.Wait(0) from triggering CA1849
Browse files Browse the repository at this point in the history
  • Loading branch information
CollinAlpert committed May 10, 2024
1 parent bc8aca0 commit 6a4cd45
Show file tree
Hide file tree
Showing 3 changed files with 247 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Analyzer.Utilities;
using Analyzer.Utilities.Extensions;
using Microsoft.CodeAnalysis;
Expand Down Expand Up @@ -83,6 +85,20 @@ public override void Initialize(AnalysisContext context)
return;
}
INamedTypeSymbol? semaphoreSlimType = wellKnownTypeProvider.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemThreadingSemaphoreSlim);
INamedTypeSymbol? timeSpanType = wellKnownTypeProvider.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemTimeSpan);
INamedTypeSymbol intType = context.Compilation.GetSpecialType(SpecialType.System_Int32);
IFieldSymbol? timeSpanZero = timeSpanType?.GetMembers(nameof(TimeSpan.Zero))
.OfType<IFieldSymbol>()
.FirstOrDefault();
ISet<IMethodSymbol> semaphoreSlimWaitMethods = semaphoreSlimType
?.GetMembers(nameof(SemaphoreSlim.Wait))
.OfType<IMethodSymbol>()
.Where(m => m.Parameters.Length > 0
&& (SymbolEqualityComparer.Default.Equals(m.Parameters[0].Type, intType)
|| SymbolEqualityComparer.Default.Equals(m.Parameters[0].Type, timeSpanType)))
.ToSet() ?? ImmutableHashSet<IMethodSymbol>.Empty;
ImmutableArray<IMethodSymbol> excludedMethods = GetExcludedMethods(wellKnownTypeProvider);
context.RegisterOperationAction(context =>
{
Expand All @@ -91,7 +107,9 @@ public override void Initialize(AnalysisContext context)
if (context.Operation is IInvocationOperation invocationOperation)
{
var methodSymbol = invocationOperation.TargetMethod;
if (excludedMethods.Contains(methodSymbol.OriginalDefinition, SymbolEqualityComparer.Default) || InspectAndReportBlockingMemberAccess(context, methodSymbol, syncBlockingSymbols, SymbolKind.Method))
if (excludedMethods.Contains(methodSymbol.OriginalDefinition, SymbolEqualityComparer.Default)
|| InspectAndReportBlockingMemberAccess(context, methodSymbol, syncBlockingSymbols, SymbolKind.Method)
|| IsSemaphoreSlimWaitWithZeroArgumentInvocation(invocationOperation, timeSpanZero, semaphoreSlimWaitMethods))
{
// Don't return double-diagnostics.
return;
Expand Down Expand Up @@ -148,6 +166,21 @@ public override void Initialize(AnalysisContext context)
});
}

private static bool IsSemaphoreSlimWaitWithZeroArgumentInvocation(IInvocationOperation invocation, IFieldSymbol? timeSpanZero, ISet<IMethodSymbol> semaphoreSlimWaitMethods)
{
if(!semaphoreSlimWaitMethods.Contains(invocation.TargetMethod, SymbolEqualityComparer.Default) || invocation.Arguments.IsEmpty)
{
return false;
}

IOperation argumentValue = invocation.Arguments[0].Value;

return argumentValue.HasConstantValue(0)
|| timeSpanZero is not null
&& argumentValue is IFieldReferenceOperation fieldReference
&& SymbolEqualityComparer.Default.Equals(fieldReference.Field, timeSpanZero);
}

private static SymbolDisplayFormat GetLanguageSpecificFormat(IOperation operation) =>
operation.Language == LanguageNames.CSharp ? SymbolDisplayFormat.CSharpShortErrorMessageFormat : SymbolDisplayFormat.VisualBasicShortErrorMessageFormat;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1343,6 +1343,218 @@ public async Task<string> Foo()
return VerifyCS.VerifyAnalyzerAsync(code);
}

[Fact, WorkItem(7271, "https://github.com/dotnet/roslyn-analyzers/issues/7271")]
public Task WhenPassingZeroToSemaphoreSlimWait_NoDiagnostic()
{
const string code = """
using System;
using System.Threading;
using System.Threading.Tasks;

class Test
{
async Task M()
{
SemaphoreSlim s = new SemaphoreSlim(0);
s.Wait(0);
}
}
""";

return VerifyCS.VerifyAnalyzerAsync(code);
}

[Fact, WorkItem(7271, "https://github.com/dotnet/roslyn-analyzers/issues/7271")]
public Task WhenPassingZeroWithCancellationTokenToSemaphoreSlimWait_NoDiagnostic()
{
const string code = """
using System;
using System.Threading;
using System.Threading.Tasks;

class Test
{
async Task M()
{
SemaphoreSlim s = new SemaphoreSlim(0);
s.Wait(0, CancellationToken.None);
}
}
""";

return VerifyCS.VerifyAnalyzerAsync(code);
}

[Fact, WorkItem(7271, "https://github.com/dotnet/roslyn-analyzers/issues/7271")]
public Task WhenPassingTimeSpanZeroToSemaphoreSlimWait_NoDiagnostic()
{
const string code = """
using System;
using System.Threading;
using System.Threading.Tasks;

class Test
{
async Task M()
{
SemaphoreSlim s = new SemaphoreSlim(0);
s.Wait(TimeSpan.Zero);
}
}
""";

return VerifyCS.VerifyAnalyzerAsync(code);
}

[Fact, WorkItem(7271, "https://github.com/dotnet/roslyn-analyzers/issues/7271")]
public Task WhenPassingTimeSpanZeroWithCancellationTokenToSemaphoreSlimWait_NoDiagnostic()
{
const string code = """
using System;
using System.Threading;
using System.Threading.Tasks;

class Test
{
async Task M()
{
SemaphoreSlim s = new SemaphoreSlim(0);
s.Wait(TimeSpan.Zero, CancellationToken.None);
}
}
""";

return VerifyCS.VerifyAnalyzerAsync(code);
}

[Theory, WorkItem(7271, "https://github.com/dotnet/roslyn-analyzers/issues/7271")]
[InlineData("1")]
[InlineData("500")]
public Task WhenPassingNonZeroToSemaphoreSlimWait_Diagnostic(string nonZero)
{
var code = $$"""
using System;
using System.Threading;
using System.Threading.Tasks;

class Test
{
async Task M()
{
SemaphoreSlim s = new SemaphoreSlim(0);
{|#0:s.Wait({{nonZero}})|};
}
}
""";
var result = new DiagnosticResult(UseAsyncMethodInAsyncContext.Descriptor)
.WithLocation(0)
.WithArguments("SemaphoreSlim.Wait(int)", "SemaphoreSlim.WaitAsync()");

return VerifyCS.VerifyAnalyzerAsync(code, result);
}

[Theory, WorkItem(7271, "https://github.com/dotnet/roslyn-analyzers/issues/7271")]
[InlineData("1")]
[InlineData("500")]
public Task WhenPassingNonZeroWithCancellationTokenToSemaphoreSlimWait_Diagnostic(string nonZero)
{
var code = $$"""
using System;
using System.Threading;
using System.Threading.Tasks;

class Test
{
async Task M()
{
SemaphoreSlim s = new SemaphoreSlim(0);
{|#0:s.Wait({{nonZero}}, CancellationToken.None)|};
}
}
""";
var result = new DiagnosticResult(UseAsyncMethodInAsyncContext.Descriptor)
.WithLocation(0)
.WithArguments("SemaphoreSlim.Wait(int, CancellationToken)", "SemaphoreSlim.WaitAsync()");

return VerifyCS.VerifyAnalyzerAsync(code, result);
}

[Theory, WorkItem(7271, "https://github.com/dotnet/roslyn-analyzers/issues/7271")]
[InlineData("TimeSpan.FromSeconds(30)")]
[InlineData("TimeSpan.Parse(\"0:32:0\")")]
public Task WhenPassingNonZeroTimeSpanToSemaphoreSlimWait_Diagnostic(string nonZero)
{
var code = $$"""
using System;
using System.Threading;
using System.Threading.Tasks;

class Test
{
async Task M()
{
SemaphoreSlim s = new SemaphoreSlim(0);
{|#0:s.Wait({{nonZero}})|};
}
}
""";
var result = new DiagnosticResult(UseAsyncMethodInAsyncContext.Descriptor)
.WithLocation(0)
.WithArguments("SemaphoreSlim.Wait(TimeSpan)", "SemaphoreSlim.WaitAsync()");

return VerifyCS.VerifyAnalyzerAsync(code, result);
}

[Theory, WorkItem(7271, "https://github.com/dotnet/roslyn-analyzers/issues/7271")]
[InlineData("TimeSpan.FromSeconds(30)")]
[InlineData("TimeSpan.Parse(\"0:32:0\")")]
public Task WhenPassingNonZeroTimeSpanWithCancellationTokenToSemaphoreSlimWait_Diagnostic(string nonZero)
{
var code = $$"""
using System;
using System.Threading;
using System.Threading.Tasks;

class Test
{
async Task M()
{
SemaphoreSlim s = new SemaphoreSlim(0);
{|#0:s.Wait({{nonZero}}, CancellationToken.None)|};
}
}
""";
var result = new DiagnosticResult(UseAsyncMethodInAsyncContext.Descriptor)
.WithLocation(0)
.WithArguments("SemaphoreSlim.Wait(TimeSpan, CancellationToken)", "SemaphoreSlim.WaitAsync()");

return VerifyCS.VerifyAnalyzerAsync(code, result);
}

[Fact, WorkItem(7271, "https://github.com/dotnet/roslyn-analyzers/issues/7271")]
public Task WhenPassingCancellationTokenToSemaphoreSlimWait_Diagnostic()
{
const string code = """
using System;
using System.Threading;
using System.Threading.Tasks;

class Test
{
async Task M()
{
SemaphoreSlim s = new SemaphoreSlim(0);
{|#0:s.Wait(CancellationToken.None)|};
}
}
""";
var result = new DiagnosticResult(UseAsyncMethodInAsyncContext.Descriptor)
.WithLocation(0)
.WithArguments("SemaphoreSlim.Wait(CancellationToken)", "SemaphoreSlim.WaitAsync()");

return VerifyCS.VerifyAnalyzerAsync(code, result);
}

private static async Task CreateCSTestAndRunAsync(string testCS)
{
var csTestVerify = new VerifyCS.Test
Expand Down
1 change: 1 addition & 0 deletions src/Utilities/Compiler/WellKnownTypeNames.cs
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,7 @@ internal static class WellKnownTypeNames
public const string SystemThreadingCancellationToken = "System.Threading.CancellationToken";
public const string SystemThreadingInterlocked = "System.Threading.Interlocked";
public const string SystemThreadingMonitor = "System.Threading.Monitor";
public const string SystemThreadingSemaphoreSlim = "System.Threading.SemaphoreSlim";
public const string SystemThreadingSpinLock = "System.Threading.SpinLock";
public const string SystemThreadingTasksConfigureAwaitOptions = "System.Threading.Tasks.ConfigureAwaitOptions";
public const string SystemThreadingTasksTaskAsyncEnumerableExtensions = "System.Threading.Tasks.TaskAsyncEnumerableExtensions";
Expand Down

0 comments on commit 6a4cd45

Please sign in to comment.