Forbid 'default' as a case constant or a pattern. (#23629)

* Forbid 'default' as a case constant or a pattern. Fixes #23499
dotnet · Dec 10, 2017 · ac2da0f · ac2da0f
1 parent 1688be0
commit ac2da0f
Show file tree

Hide file tree

Showing 26 changed files with 266 additions and 194 deletions.
diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs
@@ -2682,7 +2682,7 @@ private BoundExpression BindIsOperator(BinaryExpressionSyntax node, DiagnosticBa
                 }
 
                 var boundConstantPattern = BindConstantPattern(
-                    node.Right, operand.Type, node.Right, node.Right.HasErrors, isPatternDiagnostics, out wasExpression, wasSwitchCase: false);
+                    node.Right, operand.Type, node.Right, node.Right.HasErrors, isPatternDiagnostics, out wasExpression);
                 boundConstantPattern.WasCompilerGenerated = true;
                 if (wasExpression)
                 {

diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs
@@ -41,8 +41,7 @@ internal BoundPattern BindPattern(
             PatternSyntax node,
             TypeSymbol operandType,
             bool hasErrors,
-            DiagnosticBag diagnostics,
-            bool wasSwitchCase = false)
+            DiagnosticBag diagnostics)
         {
             switch (node.Kind())
             {
@@ -51,33 +50,40 @@ internal BoundPattern BindPattern(
                         (DeclarationPatternSyntax)node, operandType, hasErrors, diagnostics);
 
                 case SyntaxKind.ConstantPattern:
+                    var constantPattern = (ConstantPatternSyntax)node;
                     return BindConstantPattern(
-                        (ConstantPatternSyntax)node, operandType, hasErrors, diagnostics, wasSwitchCase);
+                        constantPattern, operandType, constantPattern.Expression, hasErrors, diagnostics, out bool wasExpression);
 
                 default:
                     throw ExceptionUtilities.UnexpectedValue(node.Kind());
             }
         }
 
-        private BoundConstantPattern BindConstantPattern(
-            ConstantPatternSyntax node,
+        internal BoundConstantPattern BindConstantPattern(
+            CSharpSyntaxNode node,
             TypeSymbol operandType,
+            ExpressionSyntax patternExpression,
             bool hasErrors,
             DiagnosticBag diagnostics,
-            bool wasSwitchCase)
+            out bool wasExpression)
         {
-            bool wasExpression;
-            return BindConstantPattern(node, operandType, node.Expression, hasErrors, diagnostics, out wasExpression, wasSwitchCase);
+            var innerExpression = patternExpression.SkipParens();
+            if (innerExpression.Kind() == SyntaxKind.DefaultLiteralExpression)
+            {
+                diagnostics.Add(ErrorCode.ERR_DefaultInPattern, innerExpression.Location);
+                hasErrors = true;
+            }
+
+            return BindConstantAsPattern(node, operandType, patternExpression, hasErrors, diagnostics, out wasExpression);
         }
 
-        internal BoundConstantPattern BindConstantPattern(
+        internal BoundConstantPattern BindConstantAsPattern(
             CSharpSyntaxNode node,
             TypeSymbol operandType,
             ExpressionSyntax patternExpression,
             bool hasErrors,
             DiagnosticBag diagnostics,
-            out bool wasExpression,
-            bool wasSwitchCase)
+            out bool wasExpression)
         {
             var expression = BindValue(patternExpression, diagnostics, BindValueKind.RValue);
             ConstantValue constantValueOpt = null;

diff --git a/src/Compilers/CSharp/Portable/Binder/PatternSwitchBinder.cs b/src/Compilers/CSharp/Portable/Binder/PatternSwitchBinder.cs
@@ -209,11 +209,17 @@ private BoundPatternSwitchLabel BindPatternSwitchSectionLabel(
                 case SyntaxKind.CaseSwitchLabel:
                     {
                         var caseLabelSyntax = (CaseSwitchLabelSyntax)node;
-                        bool wasExpression;
-                        var pattern = sectionBinder.BindConstantPattern(
-                            node, SwitchGoverningType, caseLabelSyntax.Value, node.HasErrors, diagnostics, out wasExpression, wasSwitchCase: true);
-                        pattern.WasCompilerGenerated = true;
+                        var pattern = sectionBinder.BindConstantAsPattern(
+                            node, SwitchGoverningType, caseLabelSyntax.Value, node.HasErrors, diagnostics, out bool wasExpression);
                         bool hasErrors = pattern.HasErrors;
+                        SyntaxNode innerValueSyntax = caseLabelSyntax.Value.SkipParens();
+                        if (innerValueSyntax.Kind() == SyntaxKind.DefaultLiteralExpression)
+                        {
+                            diagnostics.Add(ErrorCode.ERR_DefaultInSwitch, innerValueSyntax.Location);
+                            hasErrors = true;
+                        }
+
+                        pattern.WasCompilerGenerated = true;
                         var constantValue = pattern.ConstantValue;
                         if (!hasErrors &&
                             (object)constantValue != null &&
@@ -224,11 +230,6 @@ private BoundPatternSwitchLabel BindPatternSwitchSectionLabel(
                             hasErrors = true;
                         }
 
-                        if (caseLabelSyntax.Value.Kind() == SyntaxKind.DefaultLiteralExpression)
-                        {
-                            diagnostics.Add(ErrorCode.WRN_DefaultInSwitch, caseLabelSyntax.Value.Location);
-                        }
-
                         // Until we've determined whether or not the switch label is reachable, we assume it
                         // is. The caller updates isReachable after determining if the label is subsumed.
                         const bool isReachable = true;
@@ -259,7 +260,7 @@ private BoundPatternSwitchLabel BindPatternSwitchSectionLabel(
                     {
                         var matchLabelSyntax = (CasePatternSwitchLabelSyntax)node;
                         var pattern = sectionBinder.BindPattern(
-                            matchLabelSyntax.Pattern, SwitchGoverningType, node.HasErrors, diagnostics, wasSwitchCase: true);
+                            matchLabelSyntax.Pattern, SwitchGoverningType, node.HasErrors, diagnostics);
                         return new BoundPatternSwitchLabel(node, label, pattern,
                             matchLabelSyntax.WhenClause != null ? sectionBinder.BindBooleanExpression(matchLabelSyntax.WhenClause.Condition, diagnostics) : null,
                             true, node.HasErrors);

diff --git a/src/Compilers/CSharp/Portable/Binder/SwitchBinder.cs b/src/Compilers/CSharp/Portable/Binder/SwitchBinder.cs
@@ -242,7 +242,7 @@ private void BuildSwitchLabels(SyntaxList<SwitchLabelSyntax> labelsSyntax, Binde
                         // bind the pattern, to cause its pattern variables to be inferred if necessary
                         var matchLabel = (CasePatternSwitchLabelSyntax)labelSyntax;
                         var pattern = sectionBinder.BindPattern(
-                            matchLabel.Pattern, SwitchGoverningType, labelSyntax.HasErrors, tempDiagnosticBag, wasSwitchCase: true);
+                            matchLabel.Pattern, SwitchGoverningType, labelSyntax.HasErrors, tempDiagnosticBag);
                         break;
 
                     default:
@@ -623,9 +623,10 @@ private BoundSwitchLabel BindSwitchSectionLabel(SwitchLabelSyntax node, Binder s
                         hasErrors = true;
                     }
 
-                    if (caseLabelSyntax.Value.Kind() == SyntaxKind.DefaultLiteralExpression)
+                    SyntaxNode innerValueSyntax = caseLabelSyntax.Value.SkipParens();
+                    if (innerValueSyntax.Kind() == SyntaxKind.DefaultLiteralExpression)
                     {
-                        diagnostics.Add(ErrorCode.WRN_DefaultInSwitch, caseLabelSyntax.Value.Location);
+                        diagnostics.Add(ErrorCode.ERR_DefaultInSwitch, innerValueSyntax.Location);
                     }
 
                     // LabelSymbols for all the switch case labels are created by BuildLabels().

diff --git a/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs b/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs
diff --git a/src/Compilers/CSharp/Portable/CSharpResources.resx b/src/Compilers/CSharp/Portable/CSharpResources.resx
@@ -5151,11 +5151,8 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
   <data name="ERR_TupleInferredNamesNotAvailable" xml:space="preserve">
     <value>Tuple element name '{0}' is inferred. Please use language version {1} or greater to access an element by its inferred name.</value>
   </data>
-  <data name="WRN_DefaultInSwitch" xml:space="preserve">
-    <value>Did you mean to use the default switch label (`default:`) rather than `case default:`? If you really mean to use the default literal, consider `case (default):` or another literal (`case 0:` or `case null:`) as appropriate.</value>
-  </data>
-  <data name="WRN_DefaultInSwitch_Title" xml:space="preserve">
-    <value>Did you mean to use the default switch label (`default:`) rather than `case default:`? If you really mean to use the default literal, consider `case (default):` or another literal (`case 0:` or `case null:`) as appropriate.</value>
+  <data name="ERR_DefaultInSwitch" xml:space="preserve">
+    <value>A default literal 'default' is not valid as a case constant. Use another literal (e.g. '0' or 'null') as appropriate. If you intended to write the default label, use 'default:' without 'case'.</value>
   </data>
   <data name="ERR_VoidInTuple" xml:space="preserve">
     <value>A tuple may not contain a value of type 'void'.</value>
@@ -5241,4 +5238,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
   <data name="ERR_ConditionalInInterpolation" xml:space="preserve">
     <value>A conditional expression cannot be used directly in a string interpolation because the ':' ends the interpolation. Parenthesize the conditional expression.</value>
   </data>
+  <data name="ERR_DefaultInPattern" xml:space="preserve">
+    <value>A default literal 'default' is not valid as a pattern. Use another literal (e.g. '0' or 'null') as appropriate. To match everything, use a discard pattern 'var _'.</value>
+  </data>
 </root>
diff --git a/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs b/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
@@ -1487,7 +1487,7 @@ internal enum ErrorCode
         ERR_BadOpOnNullOrDefault = 8310,
         ERR_BadDynamicMethodArgDefaultLiteral = 8311,
         ERR_DefaultLiteralNotValid = 8312,
-        WRN_DefaultInSwitch = 8313,
+        ERR_DefaultInSwitch = 8313,
         ERR_PatternWrongGenericTypeInVersion = 8314,
         ERR_AmbigBinaryOpsOnDefault = 8315,
 
@@ -1551,5 +1551,6 @@ internal enum ErrorCode
 
         ERR_ConditionalInInterpolation = 8361,
         ERR_CantUseVoidInArglist = 8362,
+        ERR_DefaultInPattern = 8363,
     }
 }
diff --git a/src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs b/src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs
@@ -321,7 +321,6 @@ internal static int GetWarningLevel(ErrorCode code)
                 case ErrorCode.WRN_AttributeIgnoredWhenPublicSigning:
                 case ErrorCode.WRN_TupleLiteralNameMismatch:
                 case ErrorCode.WRN_Experimental:
-                case ErrorCode.WRN_DefaultInSwitch:
                     return 1;
                 default:
                     return 0;

diff --git a/src/Compilers/CSharp/Portable/Generated/ErrorFacts.Generated.cs b/src/Compilers/CSharp/Portable/Generated/ErrorFacts.Generated.cs
@@ -174,7 +174,6 @@ public static bool IsWarning(ErrorCode code)
                 case ErrorCode.WRN_AttributeIgnoredWhenPublicSigning:
                 case ErrorCode.WRN_TupleLiteralNameMismatch:
                 case ErrorCode.WRN_Experimental:
-                case ErrorCode.WRN_DefaultInSwitch:
                 case ErrorCode.WRN_UnreferencedLocalFunction:
                 case ErrorCode.WRN_FilterIsConstantFalse:
                 case ErrorCode.WRN_FilterIsConstantFalseRedundantTryCatch:

diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf
@@ -8435,16 +8435,6 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
         <target state="new">Tuple element name '{0}' is inferred. Please use language version {1} or greater to access an element by its inferred name.</target>
         <note />
       </trans-unit>
-      <trans-unit id="WRN_DefaultInSwitch">
-        <source>Did you mean to use the default switch label (`default:`) rather than `case default:`? If you really mean to use the default literal, consider `case (default):` or another literal (`case 0:` or `case null:`) as appropriate.</source>
-        <target state="new">Did you mean to use the default switch label (`default:`) rather than `case default:`? If you really mean to use the default literal, consider `case (default):` or another literal (`case 0:` or `case null:`) as appropriate.</target>
-        <note />
-      </trans-unit>
-      <trans-unit id="WRN_DefaultInSwitch_Title">
-        <source>Did you mean to use the default switch label (`default:`) rather than `case default:`? If you really mean to use the default literal, consider `case (default):` or another literal (`case 0:` or `case null:`) as appropriate.</source>
-        <target state="new">Did you mean to use the default switch label (`default:`) rather than `case default:`? If you really mean to use the default literal, consider `case (default):` or another literal (`case 0:` or `case null:`) as appropriate.</target>
-        <note />
-      </trans-unit>
       <trans-unit id="ERR_VoidInTuple">
         <source>A tuple may not contain a value of type 'void'.</source>
         <target state="new">A tuple may not contain a value of type 'void'.</target>
@@ -8590,6 +8580,16 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
         <target state="new">A conditional expression cannot be used directly in a string interpolation because the ':' ends the interpolation. Parenthesize the conditional expression.</target>
         <note />
       </trans-unit>
+      <trans-unit id="ERR_DefaultInSwitch">
+        <source>A default literal 'default' is not valid as a case constant. Use another literal (e.g. '0' or 'null') as appropriate. If you intended to write the default label, use 'default:' without 'case'.</source>
+        <target state="new">A default literal 'default' is not valid as a case constant. Use another literal (e.g. '0' or 'null') as appropriate. If you intended to write the default label, use 'default:' without 'case'.</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="ERR_DefaultInPattern">
+        <source>A default literal 'default' is not valid as a pattern. Use another literal (e.g. '0' or 'null') as appropriate. To match everything, use a discard pattern 'var _'.</source>
+        <target state="new">A default literal 'default' is not valid as a pattern. Use another literal (e.g. '0' or 'null') as appropriate. To match everything, use a discard pattern 'var _'.</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>
diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf
@@ -8435,16 +8435,6 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
         <target state="new">Tuple element name '{0}' is inferred. Please use language version {1} or greater to access an element by its inferred name.</target>
         <note />
       </trans-unit>
-      <trans-unit id="WRN_DefaultInSwitch">
-        <source>Did you mean to use the default switch label (`default:`) rather than `case default:`? If you really mean to use the default literal, consider `case (default):` or another literal (`case 0:` or `case null:`) as appropriate.</source>
-        <target state="new">Did you mean to use the default switch label (`default:`) rather than `case default:`? If you really mean to use the default literal, consider `case (default):` or another literal (`case 0:` or `case null:`) as appropriate.</target>
-        <note />
-      </trans-unit>
-      <trans-unit id="WRN_DefaultInSwitch_Title">
-        <source>Did you mean to use the default switch label (`default:`) rather than `case default:`? If you really mean to use the default literal, consider `case (default):` or another literal (`case 0:` or `case null:`) as appropriate.</source>
-        <target state="new">Did you mean to use the default switch label (`default:`) rather than `case default:`? If you really mean to use the default literal, consider `case (default):` or another literal (`case 0:` or `case null:`) as appropriate.</target>
-        <note />
-      </trans-unit>
       <trans-unit id="ERR_VoidInTuple">
         <source>A tuple may not contain a value of type 'void'.</source>
         <target state="new">A tuple may not contain a value of type 'void'.</target>
@@ -8590,6 +8580,16 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
         <target state="new">A conditional expression cannot be used directly in a string interpolation because the ':' ends the interpolation. Parenthesize the conditional expression.</target>
         <note />
       </trans-unit>
+      <trans-unit id="ERR_DefaultInSwitch">
+        <source>A default literal 'default' is not valid as a case constant. Use another literal (e.g. '0' or 'null') as appropriate. If you intended to write the default label, use 'default:' without 'case'.</source>
+        <target state="new">A default literal 'default' is not valid as a case constant. Use another literal (e.g. '0' or 'null') as appropriate. If you intended to write the default label, use 'default:' without 'case'.</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="ERR_DefaultInPattern">
+        <source>A default literal 'default' is not valid as a pattern. Use another literal (e.g. '0' or 'null') as appropriate. To match everything, use a discard pattern 'var _'.</source>
+        <target state="new">A default literal 'default' is not valid as a pattern. Use another literal (e.g. '0' or 'null') as appropriate. To match everything, use a discard pattern 'var _'.</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>