From 0071a9689f7d75ffc596df7d0a2887f8b977094d Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Thu, 27 Jul 2017 13:20:41 -0700 Subject: [PATCH 1/2] A 2 failing tests for VSTHRD002 --- .../VSTHRD002UseJtfRunAnalyzerTests.cs | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD002UseJtfRunAnalyzerTests.cs b/src/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD002UseJtfRunAnalyzerTests.cs index c545b5bf3..ed2eaaed4 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD002UseJtfRunAnalyzerTests.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD002UseJtfRunAnalyzerTests.cs @@ -263,6 +263,49 @@ void F() { } } } +"; + this.VerifyCSharpDiagnostic(test); + } + + [Fact] + public void DoNotReportWarningOnJTFRun() + { + var test = @" +using System; +using System.Threading.Tasks; +using Microsoft.VisualStudio.Threading; + +class ProjectProperties { + JoinableTaskFactory jtf; + + void F() { + jtf.Run(async delegate { + await Task.Yield(); + }); + } +} +"; + this.VerifyCSharpDiagnostic(test); + } + + [Fact] + public void DoNotReportWarningOnJoinableTaskJoin() + { + var test = @" +using System; +using System.Threading.Tasks; +using Microsoft.VisualStudio.Threading; + +class ProjectProperties { + JoinableTaskFactory jtf; + + void F() { + var jt = jtf.RunAsync(async delegate { + await Task.Yield(); + }); + jt.Join(); + } +} "; this.VerifyCSharpDiagnostic(test); } From 8dcb7bbb4d9e9202c369c27d42c44c3b7c6d22ee Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Thu, 27 Jul 2017 13:25:10 -0700 Subject: [PATCH 2/2] Prevent VSTHRD002 from mis-firing on use of JTF.Run --- .../CommonInterest.cs | 13 +++--- .../VSTHRD002UseJtfRunAnalyzer.cs | 46 +------------------ .../VSTHRD103UseAsyncOptionAnalyzer.cs | 2 +- .../VSTHRD104OfferAsyncOptionAnalyzer.cs | 2 +- 4 files changed, 10 insertions(+), 53 deletions(-) diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs b/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs index 87d125e4b..3b8010b7b 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs @@ -2,6 +2,7 @@ { using System; using System.Collections.Generic; + using System.Linq; using System.Runtime.CompilerServices; using System.Threading.Tasks; using Microsoft.CodeAnalysis; @@ -11,21 +12,21 @@ internal static class CommonInterest { - internal static readonly IReadOnlyList JTFSyncBlockers = new[] + internal static readonly IEnumerable JTFSyncBlockers = new[] { new SyncBlockingMethod(Namespaces.MicrosoftVisualStudioThreading, Types.JoinableTaskFactory.TypeName, Types.JoinableTaskFactory.Run, Types.JoinableTaskFactory.RunAsync), new SyncBlockingMethod(Namespaces.MicrosoftVisualStudioThreading, Types.JoinableTask.TypeName, Types.JoinableTask.Join, Types.JoinableTask.JoinAsync), }; - internal static readonly IReadOnlyList SyncBlockingMethods = new[] + internal static readonly IEnumerable ProblematicSyncBlockingMethods = new[] { - new SyncBlockingMethod(Namespaces.MicrosoftVisualStudioThreading, Types.JoinableTaskFactory.TypeName, Types.JoinableTaskFactory.Run, Types.JoinableTaskFactory.RunAsync), - new SyncBlockingMethod(Namespaces.MicrosoftVisualStudioThreading, Types.JoinableTask.TypeName, Types.JoinableTask.Join, Types.JoinableTask.JoinAsync), new SyncBlockingMethod(Namespaces.SystemThreadingTasks, nameof(Task), nameof(Task.Wait), null), new SyncBlockingMethod(Namespaces.SystemRuntimeCompilerServices, nameof(TaskAwaiter), nameof(TaskAwaiter.GetResult), null), }; - internal static readonly IReadOnlyList LegacyThreadSwitchingMethods = new[] + internal static readonly IEnumerable SyncBlockingMethods = JTFSyncBlockers.Concat(ProblematicSyncBlockingMethods); + + internal static readonly IEnumerable LegacyThreadSwitchingMethods = new[] { new LegacyThreadSwitchingMethod(Namespaces.MicrosoftVisualStudioShell, Types.ThreadHelper.TypeName, Types.ThreadHelper.Invoke), new LegacyThreadSwitchingMethod(Namespaces.MicrosoftVisualStudioShell, Types.ThreadHelper.TypeName, Types.ThreadHelper.InvokeAsync), @@ -76,7 +77,7 @@ internal static void InspectMemberAccess( SyntaxNodeAnalysisContext context, MemberAccessExpressionSyntax memberAccessSyntax, DiagnosticDescriptor descriptor, - IReadOnlyList problematicMethods, + IEnumerable problematicMethods, bool ignoreIfInsideAnonymousDelegate = false) { if (descriptor == null) diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers/VSTHRD002UseJtfRunAnalyzer.cs b/src/Microsoft.VisualStudio.Threading.Analyzers/VSTHRD002UseJtfRunAnalyzer.cs index 1db94df03..e0cd3033d 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers/VSTHRD002UseJtfRunAnalyzer.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers/VSTHRD002UseJtfRunAnalyzer.cs @@ -78,29 +78,7 @@ private void AnalyzeInvocation(SyntaxNodeAnalysisContext context) context, invocationExpressionSyntax.Expression as MemberAccessExpressionSyntax, Descriptor, - CommonInterest.SyncBlockingMethods); - - //if (CommonInterest.ShouldIgnoreContext(context)) - //{ - // return; - //} - - //var node = (InvocationExpressionSyntax)context.Node; - //var invokeMethod = context.SemanticModel.GetSymbolInfo(context.Node).Symbol as IMethodSymbol; - //if (invokeMethod != null) - //{ - // var taskType = context.SemanticModel.Compilation.GetTypeByMetadataName(typeof(Task).FullName); - // if (string.Equals(invokeMethod.Name, nameof(Task.Wait), StringComparison.Ordinal) - // && Utils.IsEqualToOrDerivedFrom(invokeMethod.ContainingType, taskType)) - // { - // context.ReportDiagnostic(Diagnostic.Create(Descriptor, context.Node.GetLocation())); - // } - // else if (string.Equals(invokeMethod.Name, "GetResult", StringComparison.Ordinal) - // && invokeMethod.ContainingType.Name.EndsWith("Awaiter", StringComparison.Ordinal)) - // { - // context.ReportDiagnostic(Diagnostic.Create(Descriptor, context.Node.GetLocation())); - // } - //} + CommonInterest.ProblematicSyncBlockingMethods); } private void AnalyzeMemberAccess(SyntaxNodeAnalysisContext context) @@ -111,28 +89,6 @@ private void AnalyzeMemberAccess(SyntaxNodeAnalysisContext context) memberAccessSyntax, Descriptor, CommonInterest.SyncBlockingProperties); - - //if (CommonInterest.ShouldIgnoreContext(context)) - //{ - // return; - //} - - //if (Utils.IsWithinNameOf(context.Node as ExpressionSyntax)) - //{ - // // We do not consider arguments to nameof( ) because they do not represent invocations of code. - // return; - //} - - //var property = context.SemanticModel.GetSymbolInfo(context.Node).Symbol as IPropertySymbol; - //if (property != null) - //{ - // var taskType = context.SemanticModel.Compilation.GetTypeByMetadataName(typeof(Task).FullName); - // if (string.Equals(property.Name, nameof(Task.Result), StringComparison.Ordinal) - // && Utils.IsEqualToOrDerivedFrom(property.ContainingType, taskType)) - // { - // context.ReportDiagnostic(Diagnostic.Create(Descriptor, context.Node.GetLocation())); - // } - //} } } } \ No newline at end of file diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers/VSTHRD103UseAsyncOptionAnalyzer.cs b/src/Microsoft.VisualStudio.Threading.Analyzers/VSTHRD103UseAsyncOptionAnalyzer.cs index 07585381e..a3ce6f900 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers/VSTHRD103UseAsyncOptionAnalyzer.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers/VSTHRD103UseAsyncOptionAnalyzer.cs @@ -164,7 +164,7 @@ private static bool IsInTaskReturningMethodOrDelegate(SyntaxNodeAnalysisContext && returnType.BelongsToNamespace(Namespaces.SystemThreadingTasks); } - private static bool InspectMemberAccess(SyntaxNodeAnalysisContext context, MemberAccessExpressionSyntax memberAccessSyntax, IReadOnlyList problematicMethods) + private static bool InspectMemberAccess(SyntaxNodeAnalysisContext context, MemberAccessExpressionSyntax memberAccessSyntax, IEnumerable problematicMethods) { if (memberAccessSyntax == null) { diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers/VSTHRD104OfferAsyncOptionAnalyzer.cs b/src/Microsoft.VisualStudio.Threading.Analyzers/VSTHRD104OfferAsyncOptionAnalyzer.cs index 5142f3563..cf588eac8 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers/VSTHRD104OfferAsyncOptionAnalyzer.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers/VSTHRD104OfferAsyncOptionAnalyzer.cs @@ -61,7 +61,7 @@ internal void AnalyzeInvocation(SyntaxNodeAnalysisContext context) this.InspectMemberAccess(context, invocationExpressionSyntax.Expression as MemberAccessExpressionSyntax, CommonInterest.JTFSyncBlockers); } - private void InspectMemberAccess(SyntaxNodeAnalysisContext context, MemberAccessExpressionSyntax memberAccessSyntax, IReadOnlyList problematicMethods) + private void InspectMemberAccess(SyntaxNodeAnalysisContext context, MemberAccessExpressionSyntax memberAccessSyntax, IEnumerable problematicMethods) { if (memberAccessSyntax == null) {