Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Prevent SemaphoreSlim.Wait(0) from triggering CA1849 #7310

Merged
merged 6 commits into from
Jun 4, 2024
Merged

Prevent SemaphoreSlim.Wait(0) from triggering CA1849 #7310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6a4cd45
Prevent SemaphoreSlim.Wait(0) from triggering CA1849
CollinAlpert May 10, 2024
2ab7ba1
Remove unnecessary using
CollinAlpert May 10, 2024
c71561d
Fix formatting
CollinAlpert May 10, 2024
c455f63
Apply code review suggestions.
CollinAlpert May 18, 2024
e264af8
Remove unnecessary using.
CollinAlpert May 18, 2024
6d45c87
Add Debug.Assert
CollinAlpert May 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
37 changes: 36 additions & 1 deletion src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/UseAsyncMethodInAsyncContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Linq;
using System.Threading;
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();
ImmutableArray<IMethodSymbol> semaphoreSlimWaitWithTimeoutMethods = 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)))
.ToImmutableArray() ?? ImmutableArray<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, semaphoreSlimWaitWithTimeoutMethods))
{
// Don't return double-diagnostics.
return;
Expand Down Expand Up @@ -148,6 +166,23 @@ public override void Initialize(AnalysisContext context)
});
}

private static bool IsSemaphoreSlimWaitWithZeroArgumentInvocation(IInvocationOperation invocation, IFieldSymbol? timeSpanZero, ImmutableArray<IMethodSymbol> semaphoreSlimWaitWithTimeoutMethods)
{
if (!semaphoreSlimWaitWithTimeoutMethods.Contains(invocation.TargetMethod, SymbolEqualityComparer.Default))
{
return false;
}

Debug.Assert(!invocation.Arguments.IsEmpty);

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
212 changes: 212 additions & 0 deletions ...lyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/UseAsyncMethodInAsyncContextTests.cs
View file
Open in desktop
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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💭 I feel like all four of the "zero" cases could be in one [Theory], but broken out is fine.

}
}
""";

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
View file
Open in desktop
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