Merge pull request #50984 from Youssef1313/handle-patterns-in-populat…

…e-switch-expression Handle patterns in populate switch expressions
dotnet · Mar 5, 2021 · 2d20863 · 2d20863
2 parents b6f8504 + 39a4ad5
commit 2d20863
Show file tree

Hide file tree

Showing 2 changed files with 236 additions and 20 deletions.
diff --git a/src/Analyzers/CSharp/Tests/PopulateSwitch/PopulateSwitchExpressionTests.cs b/src/Analyzers/CSharp/Tests/PopulateSwitch/PopulateSwitchExpressionTests.cs
@@ -962,7 +962,7 @@ void Method()
             (MyEnum)0 => 1,
             (MyEnum)1 => 2,
             ""Mismatching constant"" => 3,
-            _ => throw new System.NotImplementedException(),
+            MyEnum.FizzBuzz => throw new System.NotImplementedException(),
         }
     }
 }");
@@ -1040,7 +1040,7 @@ void Main()
             Bar.Option1 => 1,
             Bar.Option2 => 2,
             null => null,
-            _ => throw new System.NotImplementedException(),
+            Bar.Option3 => throw new System.NotImplementedException(),
         };
     }
 
@@ -1053,5 +1053,213 @@ public enum Bar
 }
 ");
         }
+
+        [Fact]
+        [WorkItem(50982, "https://github.com/dotnet/roslyn/issues/50982")]
+        public async Task TestOrPatternIsHandled()
+        {
+            await TestInRegularAndScript1Async(
+@"public static class C
+{
+    static bool IsValidValue(E e) 
+    {
+        return e [||]switch
+        {
+            E.A or E.B or E.C => true,
+            _ = false
+        };
+    }
+
+    public enum E
+    {
+        A,
+        B,
+        C,
+        D,
+        E,
+        F,
+        G,
+    }
+}
+",
+@"public static class C
+{
+    static bool IsValidValue(E e) 
+    {
+        return e [||]switch
+        {
+            E.A or E.B or E.C => true,
+            E.D => throw new System.NotImplementedException(),
+            E.E => throw new System.NotImplementedException(),
+            E.F => throw new System.NotImplementedException(),
+            E.G => throw new System.NotImplementedException(),
+            _ = false
+        };
+    }
+
+    public enum E
+    {
+        A,
+        B,
+        C,
+        D,
+        E,
+        F,
+        G,
+    }
+}
+");
+        }
+
+        [Fact]
+        [WorkItem(50982, "https://github.com/dotnet/roslyn/issues/50982")]
+        public async Task TestOrPatternIsHandled_AllEnumValuesAreHandled_NoDiagnostic()
+        {
+            await TestMissingInRegularAndScriptAsync(
+@"public static class C
+{
+    static bool IsValidValue(E e) 
+    {
+        return e [||]switch
+        {
+            (E.A or E.B) or (E.C or E.D) => true,
+            (E.E or E.F) or (E.G) => true,
+            _ = false
+        };
+    }
+
+    public enum E
+    {
+        A,
+        B,
+        C,
+        D,
+        E,
+        F,
+        G,
+    }
+}
+");
+        }
+
+        [Fact]
+        [WorkItem(50982, "https://github.com/dotnet/roslyn/issues/50982")]
+        public async Task TestMixingOrWithAndPatterns()
+        {
+            await TestInRegularAndScript1Async(
+@"public static class C
+{
+    static bool M(E e) 
+    {
+        return e [||]switch
+        {
+            (E.A or E.B) and (E.C or E.D) => true,
+            _ = false
+        };
+    }
+
+    public enum E
+    {
+        A,
+        B,
+        C,
+        D,
+        E,
+        F,
+        G,
+    }
+}
+",
+@"public static class C
+{
+    static bool M(E e) 
+    {
+        return e [||]switch
+        {
+            (E.A or E.B) and (E.C or E.D) => true,
+            E.A => throw new System.NotImplementedException(),
+            E.B => throw new System.NotImplementedException(),
+            E.C => throw new System.NotImplementedException(),
+            E.D => throw new System.NotImplementedException(),
+            E.E => throw new System.NotImplementedException(),
+            E.F => throw new System.NotImplementedException(),
+            E.G => throw new System.NotImplementedException(),
+            _ = false
+        };
+    }
+
+    public enum E
+    {
+        A,
+        B,
+        C,
+        D,
+        E,
+        F,
+        G,
+    }
+}
+"
+);
+        }
+
+        [Fact]
+        [WorkItem(50982, "https://github.com/dotnet/roslyn/issues/50982")]
+        public async Task TestMixingOrWithAndPatterns2()
+        {
+            await TestInRegularAndScript1Async(
+@"public static class C
+{
+    static bool M(E e) 
+    {
+        return e [||]switch
+        {
+            (E.A or E.B) or (E.C and E.D) => true,
+            _ = false
+        };
+    }
+
+    public enum E
+    {
+        A,
+        B,
+        C,
+        D,
+        E,
+        F,
+        G,
+    }
+}
+",
+@"public static class C
+{
+    static bool M(E e) 
+    {
+        return e [||]switch
+        {
+            (E.A or E.B) or (E.C and E.D) => true,
+            E.C => throw new System.NotImplementedException(),
+            E.D => throw new System.NotImplementedException(),
+            E.E => throw new System.NotImplementedException(),
+            E.F => throw new System.NotImplementedException(),
+            E.G => throw new System.NotImplementedException(),
+            _ = false
+        };
+    }
+
+    public enum E
+    {
+        A,
+        B,
+        C,
+        D,
+        E,
+        F,
+        G,
+    }
+}
+"
+);
+        }
     }
 }
diff --git a/src/Analyzers/Core/Analyzers/PopulateSwitch/PopulateSwitchExpressionHelpers.cs b/src/Analyzers/Core/Analyzers/PopulateSwitch/PopulateSwitchExpressionHelpers.cs
@@ -19,8 +19,6 @@ public static ICollection<ISymbol> GetMissingEnumMembers(ISwitchExpressionOperat
             var switchExpression = operation.Value;
             var switchExpressionType = switchExpression?.Type;
 
-            var enumMembers = new Dictionary<long, ISymbol>();
-
             // Check if the type of the expression is a nullable INamedTypeSymbol
             // if the type is both nullable and an INamedTypeSymbol extract the type argument from the nullable
             // and check if it is of enum type
@@ -29,36 +27,46 @@ public static ICollection<ISymbol> GetMissingEnumMembers(ISwitchExpressionOperat
 
             if (switchExpressionType?.TypeKind == TypeKind.Enum)
             {
-                if (!PopulateSwitchStatementHelpers.TryGetAllEnumMembers(switchExpressionType, enumMembers) ||
-                    !TryRemoveExistingEnumMembers(operation, enumMembers))
+                var enumMembers = new Dictionary<long, ISymbol>();
+                if (PopulateSwitchStatementHelpers.TryGetAllEnumMembers(switchExpressionType, enumMembers))
                 {
-                    return SpecializedCollections.EmptyCollection<ISymbol>();
+                    RemoveExistingEnumMembers(operation, enumMembers);
+                    return enumMembers.Values;
                 }
             }
 
-            return enumMembers.Values;
+            return SpecializedCollections.EmptyCollection<ISymbol>();
         }
 
-        private static bool TryRemoveExistingEnumMembers(
+        private static void RemoveExistingEnumMembers(
             ISwitchExpressionOperation operation, Dictionary<long, ISymbol> enumMembers)
         {
             foreach (var arm in operation.Arms)
             {
-                if (arm.Pattern is IConstantPatternOperation constantPattern)
+                RemoveIfConstantPatternHasValue(arm.Pattern, enumMembers);
+                if (arm.Pattern is IBinaryPatternOperation binaryPattern)
                 {
-                    var constantValue = constantPattern.Value.ConstantValue;
-                    if (!constantValue.HasValue)
-                    {
-                        // We had a case which didn't resolve properly.
-                        // Assume the switch is complete.
-                        return false;
-                    }
-
-                    enumMembers.Remove(IntegerUtilities.ToInt64(constantValue.Value));
+                    HandleBinaryPattern(binaryPattern, enumMembers);
                 }
             }
+        }
+
+        private static void HandleBinaryPattern(IBinaryPatternOperation? binaryPattern, Dictionary<long, ISymbol> enumMembers)
+        {
+            if (binaryPattern?.OperatorKind == BinaryOperatorKind.Or)
+            {
+                RemoveIfConstantPatternHasValue(binaryPattern.LeftPattern, enumMembers);
+                RemoveIfConstantPatternHasValue(binaryPattern.RightPattern, enumMembers);
+
+                HandleBinaryPattern(binaryPattern.LeftPattern as IBinaryPatternOperation, enumMembers);
+                HandleBinaryPattern(binaryPattern.RightPattern as IBinaryPatternOperation, enumMembers);
+            }
+        }
 
-            return true;
+        private static void RemoveIfConstantPatternHasValue(IOperation operation, Dictionary<long, ISymbol> enumMembers)
+        {
+            if (operation is IConstantPatternOperation { Value: { ConstantValue: { HasValue: true, Value: var value } } })
+                enumMembers.Remove(IntegerUtilities.ToInt64(value));
         }
 
         public static bool HasDefaultCase(ISwitchExpressionOperation operation)