Improve analyzer UseConditionalAccessInsteadOfConditionalExpression (…

…RCS1206)
dotnet · Feb 1, 2020 · 88dd4ce · 88dd4ce
1 parent e0fdc38
commit 88dd4ce
Show file tree

Hide file tree

Showing 3 changed files with 99 additions and 2 deletions.
diff --git a/src/Analyzers.CodeFixes/CSharp/Refactorings/SimplifyNullCheckRefactoring.cs b/src/Analyzers.CodeFixes/CSharp/Refactorings/SimplifyNullCheckRefactoring.cs
@@ -80,8 +80,12 @@ internal static class SimplifyNullCheckRefactoring
             if (newNode == null)
                 newNode = ParseExpression(whenNotNull.ToString().Insert(expression.Span.End - whenNotNull.SpanStart, "?"));
 
-            if (coalesce || !semanticModel.GetTypeSymbol(whenNotNull, cancellationToken).IsReferenceTypeOrNullableType())
+            if (coalesce
+                || (!semanticModel.GetTypeSymbol(whenNotNull, cancellationToken).IsReferenceTypeOrNullableType()
+                    && (whenNull as DefaultExpressionSyntax)?.Type.IsKind(SyntaxKind.NullableType) != true))
+            {
                 newNode = CoalesceExpression(newNode.Parenthesize(), whenNull.Parenthesize());
+            }
 
             newNode = newNode
                 .WithTriviaFrom(conditionalExpression)

diff --git a/src/Analyzers/CSharp/Analysis/SimplifyNullCheckAnalyzer.cs b/src/Analyzers/CSharp/Analysis/SimplifyNullCheckAnalyzer.cs
@@ -168,7 +168,8 @@ public static void AnalyzeConditionalExpression(SyntaxNodeAnalysisContext contex
 
                 if (typeSymbol?.IsErrorType() == false
                     && (typeSymbol.IsReferenceType || typeSymbol.IsValueType)
-                    && semanticModel.IsDefaultValue(typeSymbol, whenNull, cancellationToken)
+                    && (semanticModel.IsDefaultValue(typeSymbol, whenNull, cancellationToken)
+                        || IsDefaultOfNullableStruct(typeSymbol, whenNull, semanticModel, cancellationToken))
                     && !CSharpUtility.ContainsOutArgumentWithLocal(whenNotNull, semanticModel, cancellationToken)
                     && !conditionalExpressionInfo.ConditionalExpression.IsInExpressionTree(semanticModel, cancellationToken))
                 {
@@ -179,5 +180,29 @@ public static void AnalyzeConditionalExpression(SyntaxNodeAnalysisContext contex
                 }
             }
         }
+
+        private static bool IsDefaultOfNullableStruct(
+            ITypeSymbol typeSymbol,
+            ExpressionSyntax whenNull,
+            SemanticModel semanticModel,
+            CancellationToken cancellationToken)
+        {
+            if (typeSymbol.IsValueType
+                && !typeSymbol.IsNullableType()
+                && whenNull.IsKind(SyntaxKind.DefaultExpression))
+            {
+                var defaultExpression = (DefaultExpressionSyntax)whenNull;
+
+                TypeSyntax type = defaultExpression.Type;
+
+                if (type.IsKind(SyntaxKind.NullableType)
+                    && semanticModel.GetTypeSymbol(type, cancellationToken)?.IsNullableOf(typeSymbol) == true)
+                {
+                    return true;
+                }
+            }
+
+            return false;
+        }
     }
 }
diff --git a/src/Tests/Analyzers.Tests/RCS1206UseConditionalAccessInsteadOfConditionalExpressionTests.cs b/src/Tests/Analyzers.Tests/RCS1206UseConditionalAccessInsteadOfConditionalExpressionTests.cs
@@ -213,6 +213,74 @@ void M()
 ");
         }
 
+        [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UseConditionalAccessInsteadOfConditionalExpression)]
+        public async Task Test_StructAndDefaultOfNullable()
+        {
+            await VerifyDiagnosticAndFixAsync(@"
+using System;
+
+class C
+{
+    void M()
+    {
+        C x = null;
+
+        int? y = [|(x != null) ? x.M2() : default(int?)|];
+    }
+
+    int M2() => default;
+}
+", @"
+using System;
+
+class C
+{
+    void M()
+    {
+        C x = null;
+
+        int? y = x?.M2();
+    }
+
+    int M2() => default;
+}
+");
+        }
+
+        [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UseConditionalAccessInsteadOfConditionalExpression)]
+        public async Task Test_DefaultOfNullableAndStruct()
+        {
+            await VerifyDiagnosticAndFixAsync(@"
+using System;
+
+class C
+{
+    void M()
+    {
+        C x = null;
+
+        int? y = [|(x == null) ? default(int?) : x.M2()|];
+    }
+
+    int M2() => default;
+}
+", @"
+using System;
+
+class C
+{
+    void M()
+    {
+        C x = null;
+
+        int? y = x?.M2();
+    }
+
+    int M2() => default;
+}
+");
+        }
+
         [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UseConditionalAccessInsteadOfConditionalExpression)]
         public async Task TestNoDiagnostic()
         {