Merge pull request #24173 from MaStr11/SuppressIDE0041_UseIsNullcheck…

…_ForUnconstraintGenericParameters Suppress 'IDE0041 Null check can be simplified' for unconstraint generic parameters.
dotnet · Feb 14, 2018 · 367b725 · 367b725
2 parents 55c74cc + bffe221
commit 367b725
Show file tree

Hide file tree

Showing 6 changed files with 165 additions and 14 deletions.
diff --git a/src/EditorFeatures/CSharpTest/UseIsNullCheck/UseIsNullCheckTests.cs b/src/EditorFeatures/CSharpTest/UseIsNullCheck/UseIsNullCheckTests.cs
@@ -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;
+        }
+    }
+}
+");
+        }
     }
 }
diff --git a/src/EditorFeatures/VisualBasicTest/UseIsNullCheck/UseIsNullCheckTests.vb b/src/EditorFeatures/VisualBasicTest/UseIsNullCheck/UseIsNullCheckTests.vb
@@ -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

diff --git a/src/Features/CSharp/Portable/UseIsNullCheck/CSharpUseIsNullCheckCodeFixProvider.cs b/src/Features/CSharp/Portable/UseIsNullCheck/CSharpUseIsNullCheckCodeFixProvider.cs
@@ -17,12 +17,28 @@ protected override string GetIsNullTitle()
         protected override string GetIsNotNullTitle()
             => GetIsNullTitle();
 
-        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);
     }
 }
diff --git a/src/Features/Core/Portable/UseIsNullCheck/AbstractUseIsNullCodeFixProvider.cs b/src/Features/Core/Portable/UseIsNullCheck/AbstractUseIsNullCodeFixProvider.cs
@@ -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)
         {
@@ -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,
@@ -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)
             {
             }

diff --git a/src/Features/Core/Portable/UseIsNullCheck/AbstractUseIsNullDiagnosticAnalyzer.cs b/src/Features/Core/Portable/UseIsNullCheck/AbstractUseIsNullDiagnosticAnalyzer.cs
@@ -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;
@@ -13,6 +14,7 @@ internal abstract class AbstractUseIsNullCheckDiagnosticAnalyzer<
         : AbstractCodeStyleDiagnosticAnalyzer
         where TLanguageKindEnum : struct
     {
+
         protected AbstractUseIsNullCheckDiagnosticAnalyzer(LocalizableString title)
             : base(IDEDiagnosticIds.UseIsNullCheckDiagnosticId,
                    title,
@@ -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)
                 {
@@ -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))
@@ -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)
@@ -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));
     }

diff --git a/src/Features/VisualBasic/Portable/UseIsNullCheck/VisualBasicUseIsNullCheckCodeFixProvider.vb b/src/Features/VisualBasic/Portable/UseIsNullCheck/VisualBasicUseIsNullCheckCodeFixProvider.vb
@@ -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()