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.

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

Suppress reporting VSTHRD114 diagnostic on Task? returning methods #866

Merged
merged 1 commit into from Jun 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Expand Up @@ -203,6 +203,29 @@ internal override SyntaxNode IsolateMethodName(IObjectCreationOperation objectCr
return objectCreation.Syntax;
}

internal override bool MethodReturnsNullableReferenceType(IMethodSymbol methodSymbol)
{
SyntaxReference? syntaxReference = methodSymbol.DeclaringSyntaxReferences.FirstOrDefault();
if (syntaxReference == null)
{
return false;
}

SyntaxNode syntaxNode = syntaxReference.GetSyntax();
TypeSyntax? returnType = null;

if (syntaxNode is MethodDeclarationSyntax methodDeclSyntax)
{
returnType = methodDeclSyntax.ReturnType;
}
else if (syntaxNode is LocalFunctionStatementSyntax localFunc)
{
returnType = localFunc.ReturnType;
}

return returnType != null && returnType.IsKind(SyntaxKind.NullableType);
}

internal readonly struct ContainingFunctionData
{
internal ContainingFunctionData(CSharpSyntaxNode function, bool isAsync, ParameterListSyntax parameterList, CSharpSyntaxNode blockOrExpression)
Expand Down
@@ -0,0 +1,14 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

namespace Microsoft.VisualStudio.Threading.Analyzers
{
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class CSharpVSTHRD114AvoidReturningNullTaskAnalyzer : AbstractVSTHRD114AvoidReturningNullTaskAnalyzer
{
private protected override LanguageUtils LanguageUtils => CSharpUtils.Instance;
}
}
Expand Up @@ -16,7 +16,7 @@ namespace Microsoft.VisualStudio.Threading.Analyzers
public class VSTHRD114AvoidReturningNullTaskCodeFix : CodeFixProvider
{
private static readonly ImmutableArray<string> ReusableFixableDiagnosticIds = ImmutableArray.Create(
VSTHRD114AvoidReturningNullTaskAnalyzer.Id);
AbstractVSTHRD114AvoidReturningNullTaskAnalyzer.Id);

/// <inheritdoc />
public override ImmutableArray<string> FixableDiagnosticIds => ReusableFixableDiagnosticIds;
Expand Down
Expand Up @@ -87,5 +87,11 @@ internal override SyntaxNode IsolateMethodName(IObjectCreationOperation objectCr
{
return objectCreation.Syntax;
}

internal override bool MethodReturnsNullableReferenceType(IMethodSymbol methodSymbol)
{
// VB.NET doesn't support nullable reference types
return false;
}
}
}
@@ -0,0 +1,14 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

namespace Microsoft.VisualStudio.Threading.Analyzers
{
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;

[DiagnosticAnalyzer(LanguageNames.VisualBasic)]
public sealed class VisualBasicVSTHRD114AvoidReturningNullTaskAnalyzer : AbstractVSTHRD114AvoidReturningNullTaskAnalyzer
{
private protected override LanguageUtils LanguageUtils => VisualBasicUtils.Instance;
}
}
Expand Up @@ -13,8 +13,7 @@ namespace Microsoft.VisualStudio.Threading.Analyzers
/// Finds await expressions on <see cref="Task"/> that do not use <see cref="Task.ConfigureAwait(bool)"/>.
/// Also works on <see cref="ValueTask"/>.
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
public class VSTHRD114AvoidReturningNullTaskAnalyzer : DiagnosticAnalyzer
public abstract class AbstractVSTHRD114AvoidReturningNullTaskAnalyzer : DiagnosticAnalyzer
{
public const string Id = "VSTHRD114";

Expand All @@ -30,28 +29,15 @@ public class VSTHRD114AvoidReturningNullTaskAnalyzer : DiagnosticAnalyzer
/// <inheritdoc />
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Descriptor);

private protected abstract LanguageUtils LanguageUtils { get; }

/// <inheritdoc />
public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze);

context.RegisterOperationAction(Utils.DebuggableWrapper(context => AnalyzerReturnOperation(context)), OperationKind.Return);
}

private static void AnalyzerReturnOperation(OperationAnalysisContext context)
{
var returnOperation = (IReturnOperation)context.Operation;

if (returnOperation.ReturnedValue is { ConstantValue: { HasValue: true, Value: null } } && // could be null for implicit returns
returnOperation.ReturnedValue.Syntax is { } returnedValueSyntax &&
Utils.GetContainingFunctionBlock(returnOperation) is { } block &&
FindOwningSymbol(block, context.ContainingSymbol) is { } method &&
!method.IsAsync &&
Utils.IsTask(method.ReturnType))
{
context.ReportDiagnostic(Diagnostic.Create(Descriptor, returnedValueSyntax.GetLocation()));
}
context.RegisterOperationAction(Utils.DebuggableWrapper(context => this.AnalyzerReturnOperation(context)), OperationKind.Return);
}

private static IMethodSymbol? FindOwningSymbol(IBlockOperation block, ISymbol containingSymbol)
Expand All @@ -68,5 +54,21 @@ private static void AnalyzerReturnOperation(OperationAnalysisContext context)
_ => null,
};
}

private void AnalyzerReturnOperation(OperationAnalysisContext context)
{
var returnOperation = (IReturnOperation)context.Operation;

if (returnOperation.ReturnedValue is { ConstantValue: { HasValue: true, Value: null } } && // could be null for implicit returns
returnOperation.ReturnedValue.Syntax is { } returnedValueSyntax &&
Utils.GetContainingFunctionBlock(returnOperation) is { } block &&
FindOwningSymbol(block, context.ContainingSymbol) is { } method &&
!method.IsAsync &&
Utils.IsTask(method.ReturnType) &&
!this.LanguageUtils.MethodReturnsNullableReferenceType(method))
{
context.ReportDiagnostic(Diagnostic.Create(Descriptor, returnedValueSyntax.GetLocation()));
}
}
}
}
Expand Up @@ -14,5 +14,7 @@ internal abstract class LanguageUtils
internal abstract SyntaxNode IsolateMethodName(IInvocationOperation invocation);

internal abstract SyntaxNode IsolateMethodName(IObjectCreationOperation objectCreation);

internal abstract bool MethodReturnsNullableReferenceType(IMethodSymbol method);
}
}
Expand Up @@ -5,8 +5,8 @@ namespace Microsoft.VisualStudio.Threading.Analyzers.Tests
{
using System.Threading.Tasks;
using Xunit;
using VerifyCS = CSharpCodeFixVerifier<VSTHRD114AvoidReturningNullTaskAnalyzer, CodeAnalysis.Testing.EmptyCodeFixProvider>;
using VerifyVB = VisualBasicCodeFixVerifier<VSTHRD114AvoidReturningNullTaskAnalyzer, CodeAnalysis.Testing.EmptyCodeFixProvider>;
using VerifyCS = CSharpCodeFixVerifier<CSharpVSTHRD114AvoidReturningNullTaskAnalyzer, CodeAnalysis.Testing.EmptyCodeFixProvider>;
using VerifyVB = VisualBasicCodeFixVerifier<VisualBasicVSTHRD114AvoidReturningNullTaskAnalyzer, CodeAnalysis.Testing.EmptyCodeFixProvider>;

public class VSTHRD114AvoidReturningNullTaskAnalyzerTests
{
Expand Down Expand Up @@ -280,6 +280,45 @@ async Task<object> GetTaskObj()
}
}
}
";
await new VerifyCS.Test
{
TestCode = csharpTest,
}.RunAsync();
}

[Fact]
public async Task ReturnNullableTask_NoDiagnostic()
{
var csharpTest = @"
using System;
using System.Threading.Tasks;

class Test
{
public Task? GetTask()
{
return null;
}

public Task<int>? GetTaskOfInt()
{
return null;
}

public Task<object?>? GetTaskOfNullableObject()
{
return null;
}

public void LocalFunc()
{
Task<object>? GetTaskObj()
{
return null;
}
}
}
";
await new VerifyCS.Test
{
Expand Down
Expand Up @@ -5,8 +5,8 @@ namespace Microsoft.VisualStudio.Threading.Analyzers.Tests
{
using System.Threading.Tasks;
using Xunit;
using VerifyCS = CSharpCodeFixVerifier<VSTHRD114AvoidReturningNullTaskAnalyzer, VSTHRD114AvoidReturningNullTaskCodeFix>;
using VerifyVB = VisualBasicCodeFixVerifier<VSTHRD114AvoidReturningNullTaskAnalyzer, VSTHRD114AvoidReturningNullTaskCodeFix>;
using VerifyCS = CSharpCodeFixVerifier<CSharpVSTHRD114AvoidReturningNullTaskAnalyzer, VSTHRD114AvoidReturningNullTaskCodeFix>;
using VerifyVB = VisualBasicCodeFixVerifier<VisualBasicVSTHRD114AvoidReturningNullTaskAnalyzer, VSTHRD114AvoidReturningNullTaskCodeFix>;

public class VSTHRD114AvoidReturningNullTaskCodeFixTests
{
Expand Down