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 'IDE0041 Null check can be simplified' for unconstraint generic parameters. #24173

Original file line number Diff line number Diff line change
Expand Up @@ -217,5 +217,85 @@ void M(string s1, string s2)
}
}");
}

[WorkItem(23581, "https://github.com/dotnet/roslyn/issues/23581")]
[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseIsNullCheck)]
public async Task TestValueParameterTypeIsUnconstrainedGeneric()
{
await TestInRegularAndScriptAsync(
@"
class C
{
public static void NotNull<T>(T value)
{
if ([||]ReferenceEquals(value, null))
{
return;
}
}
}
", @"
class C
{
public static void NotNull<T>(T value)
{
if (value == null)
{
return;
}
}
}
");
}

[WorkItem(23581, "https://github.com/dotnet/roslyn/issues/23581")]
[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseIsNullCheck)]
public async Task TestValueParameterTypeIsRefConstraintGeneric()
{
await TestInRegularAndScriptAsync(
@"
class C
{
public static void NotNull<T>(T value) where T:class
{
if ([||]ReferenceEquals(value, null))
{
return;
}
}
}
",
@"
class C
{
public static void NotNull<T>(T value) where T:class
{
if (value is null)
{
return;
}
}
}
");
}

[WorkItem(23581, "https://github.com/dotnet/roslyn/issues/23581")]
[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseIsNullCheck)]
public async Task TestValueParameterTypeIsValueConstraintGeneric()
{
await TestMissingAsync(
@"
class C
{
public static void NotNull<T>(T value) where T:struct
{
if ([||]ReferenceEquals(value, null))
{
return;
}
}
}
");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,21 @@ class C
return
end if
end sub
end class")
End Function

<WorkItem(23581, "https://github.com/dotnet/roslyn/issues/23581")>
<Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseIsNullCheck)>
Public Async Function TestValueParameterTypeIsValueConstraintGeneric() As Task
Await TestMissingInRegularAndScriptAsync(
"Imports System

class C
sub M(Of T As Structure)(v as T)
if ([||]ReferenceEquals(Nothing, v))
return
end if
end sub
end class")
End Function
End Class
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,28 @@ protected override string GetIsNullTitle()
protected override string GetIsNotNullTitle()
=> GetIsNullTitle();
Copy link
Member

Choose a reason for hiding this comment

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

the feature is literall the "Use 'is null'" feature :) You're now using "== null".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. But I thought we agreed on that. Did I got you wrong?


protected override SyntaxNode CreateIsNullCheck(SyntaxNode argument)
private static SyntaxNode CreateEqualsNullCheck(SyntaxNode argument, SyntaxKind comparisonOperator)
=> SyntaxFactory.BinaryExpression(
comparisonOperator,
(ExpressionSyntax)argument,
SyntaxFactory.LiteralExpression(SyntaxKind.NullLiteralExpression)).Parenthesize();

private static SyntaxNode CreateIsNullCheck(SyntaxNode argument)
=> SyntaxFactory.IsPatternExpression(
(ExpressionSyntax)argument,
SyntaxFactory.ConstantPattern(SyntaxFactory.LiteralExpression(SyntaxKind.NullLiteralExpression))).Parenthesize();

protected override SyntaxNode CreateIsNotNullCheck(SyntaxNode notExpression, SyntaxNode argument)
private static SyntaxNode CreateIsNotNullCheck(SyntaxNode notExpression, SyntaxNode argument)
=> ((PrefixUnaryExpressionSyntax)notExpression).WithOperand((ExpressionSyntax)CreateIsNullCheck(argument));

protected override SyntaxNode CreateNullCheck(SyntaxNode argument, bool isUnconstrainedGeneric)
=> isUnconstrainedGeneric
? CreateEqualsNullCheck(argument, SyntaxKind.EqualsExpression)
: CreateIsNullCheck(argument);

protected override SyntaxNode CreateNotNullCheck(SyntaxNode notExpression, SyntaxNode argument, bool isUnconstrainedGeneric)
=> isUnconstrainedGeneric
? CreateEqualsNullCheck(argument, SyntaxKind.NotEqualsExpression)
: CreateIsNotNullCheck(notExpression, argument);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,15 @@ namespace Microsoft.CodeAnalysis.UseIsNullCheck
internal abstract class AbstractUseIsNullCheckCodeFixProvider : SyntaxEditorBasedCodeFixProvider
{
public const string Negated = nameof(Negated);
public const string UnconstrainedGeneric = nameof(UnconstrainedGeneric);

public override ImmutableArray<string> FixableDiagnosticIds
=> ImmutableArray.Create(IDEDiagnosticIds.UseIsNullCheckDiagnosticId);

protected abstract string GetIsNullTitle();
protected abstract string GetIsNotNullTitle();
protected abstract SyntaxNode CreateIsNullCheck(SyntaxNode argument);
protected abstract SyntaxNode CreateIsNotNullCheck(SyntaxNode notExpression, SyntaxNode argument);
protected abstract SyntaxNode CreateNullCheck(SyntaxNode argument, bool isUnconstrainedGeneric);
protected abstract SyntaxNode CreateNotNullCheck(SyntaxNode notExpression, SyntaxNode argument, bool isUnconstrainedGeneric);

public override Task RegisterCodeFixesAsync(CodeFixContext context)
{
Expand All @@ -49,14 +50,17 @@ protected override Task FixAllAsync(
{
var invocation = diagnostic.AdditionalLocations[0].FindNode(getInnermostNodeForTie: true, cancellationToken: cancellationToken);
var negate = diagnostic.Properties.ContainsKey(Negated);
var isUnconstrainedGeneric = diagnostic.Properties.ContainsKey(UnconstrainedGeneric);

var arguments = syntaxFacts.GetArgumentsOfInvocationExpression(invocation);
var argument = syntaxFacts.IsNullLiteralExpression(syntaxFacts.GetExpressionOfArgument(arguments[0]))
? syntaxFacts.GetExpressionOfArgument(arguments[1])
: syntaxFacts.GetExpressionOfArgument(arguments[0]);

var toReplace = negate ? invocation.Parent : invocation;
var replacement = negate ? CreateIsNotNullCheck(invocation.Parent, argument) : CreateIsNullCheck(argument);
var replacement = negate
? CreateNotNullCheck(invocation.Parent, argument, isUnconstrainedGeneric)
: CreateNullCheck(argument, isUnconstrainedGeneric);

editor.ReplaceNode(
toReplace,
Expand All @@ -68,7 +72,7 @@ protected override Task FixAllAsync(

private class MyCodeAction : CodeAction.DocumentChangeAction
{
public MyCodeAction(string title, Func<CancellationToken, Task<Document>> createChangedDocument)
public MyCodeAction(string title, Func<CancellationToken, Task<Document>> createChangedDocument)
: base(title, createChangedDocument, title)
{
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

using System.Collections.Immutable;
using System.Linq;
using System.Threading;
using Microsoft.CodeAnalysis.CodeStyle;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.LanguageServices;
Expand All @@ -13,6 +14,7 @@ internal abstract class AbstractUseIsNullCheckDiagnosticAnalyzer<
: AbstractCodeStyleDiagnosticAnalyzer
where TLanguageKindEnum : struct
{

protected AbstractUseIsNullCheckDiagnosticAnalyzer(LocalizableString title)
: base(IDEDiagnosticIds.UseIsNullCheckDiagnosticId,
title,
Expand All @@ -27,7 +29,8 @@ public override bool OpenFileOnly(Workspace workspace)
=> false;

protected override void InitializeWorker(AnalysisContext context)
=> context.RegisterCompilationStartAction(compilationContext => {
=> context.RegisterCompilationStartAction(compilationContext =>
{
var objectType = compilationContext.Compilation.GetSpecialType(SpecialType.System_Object);
if (objectType != null)
{
Expand All @@ -49,7 +52,7 @@ protected override void InitializeWorker(AnalysisContext context)
private void AnalyzeSyntax(SyntaxNodeAnalysisContext context, IMethodSymbol referenceEqualsMethod)
{
var cancellationToken = context.CancellationToken;

var semanticModel = context.SemanticModel;
var syntaxTree = semanticModel.SyntaxTree;
if (!IsLanguageVersionSupported(syntaxTree.Options))
Expand Down Expand Up @@ -108,9 +111,29 @@ private void AnalyzeSyntax(SyntaxNodeAnalysisContext context, IMethodSymbol refe
return;
}

var properties = ImmutableDictionary<string, string>.Empty;

var genericParameterSymbol = GetGenericParameterSymbol(syntaxFacts, semanticModel, arguments[0], arguments[1], cancellationToken);
if (genericParameterSymbol != null)
{
if (genericParameterSymbol.HasValueTypeConstraint)
{
// 'is null' would generate error CS0403: Cannot convert null to type parameter 'T' because it could be a non-nullable value type. Consider using 'default(T)' instead.
// '== null' would generate error CS0019: Operator '==' cannot be applied to operands of type 'T' and '<null>'
// 'Is Nothing' would generate error BC30020: 'Is' operator does not accept operands of type 'T'. Operands must be reference or nullable types.
return;
}

if (!genericParameterSymbol.HasReferenceTypeConstraint)
{
// Needs special casing for C# as long as
// https://github.com/dotnet/csharplang/issues/1284
// is not implemented.
properties = properties.Add(AbstractUseIsNullCheckCodeFixProvider.UnconstrainedGeneric, "");
}
}

var additionalLocations = ImmutableArray.Create(invocation.GetLocation());
var properties = ImmutableDictionary<string, string>.Empty;

var negated = syntaxFacts.IsLogicalNotExpression(invocation.Parent);
if (negated)
Expand All @@ -125,7 +148,20 @@ private void AnalyzeSyntax(SyntaxNodeAnalysisContext context, IMethodSymbol refe
additionalLocations, properties));
}

private bool MatchesPattern(ISyntaxFactsService syntaxFacts, SyntaxNode node1, SyntaxNode node2)
private static ITypeParameterSymbol GetGenericParameterSymbol(ISyntaxFactsService syntaxFacts, SemanticModel semanticModel, SyntaxNode node1, SyntaxNode node2, CancellationToken cancellationToken)
{
var valueNode = syntaxFacts.IsNullLiteralExpression(syntaxFacts.GetExpressionOfArgument(node1)) ? node2 : node1;
var argumentExpression = syntaxFacts.GetExpressionOfArgument(valueNode);
if (argumentExpression != null)
{
var parameterType = semanticModel.GetTypeInfo(argumentExpression, cancellationToken).Type;
return parameterType as ITypeParameterSymbol;
}

return default;
}

private static bool MatchesPattern(ISyntaxFactsService syntaxFacts, SyntaxNode node1, SyntaxNode node2)
=> syntaxFacts.IsNullLiteralExpression(syntaxFacts.GetExpressionOfArgument(node1)) &&
!syntaxFacts.IsNullLiteralExpression(syntaxFacts.GetExpressionOfArgument(node2));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,20 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseIsNullCheck
Inherits AbstractUseIsNullCheckCodeFixProvider

Protected Overrides Function GetIsNullTitle() As String
Return VBFeaturesResources.use_Is_Nothing_check
Return VBFeaturesResources.Use_Is_Nothing_check
End Function

Protected Overrides Function GetIsNotNullTitle() As String
Return VBFeaturesResources.use_IsNot_Nothing_check
Return VBFeaturesResources.Use_IsNot_Nothing_check
End Function

Protected Overrides Function CreateIsNullCheck(argument As SyntaxNode) As SyntaxNode
Protected Overrides Function CreateNullCheck(argument As SyntaxNode, isUnconstrainedGeneric As Boolean) As SyntaxNode
Return SyntaxFactory.IsExpression(
DirectCast(argument, ExpressionSyntax).Parenthesize(),
SyntaxFactory.NothingLiteralExpression(SyntaxFactory.Token(SyntaxKind.NothingKeyword))).Parenthesize()
End Function

Protected Overrides Function CreateIsNotNullCheck(notExpression As SyntaxNode, argument As SyntaxNode) As SyntaxNode
Protected Overrides Function CreateNotNullCheck(notExpression As SyntaxNode, argument As SyntaxNode, isUnconstrainedGeneric As Boolean) As SyntaxNode
Return SyntaxFactory.IsNotExpression(
DirectCast(argument, ExpressionSyntax).Parenthesize(),
SyntaxFactory.NothingLiteralExpression(SyntaxFactory.Token(SyntaxKind.NothingKeyword))).Parenthesize()
Expand Down