[StrictExhaustiveSwitches] flow through default case to space checking

Author: Adam Cmiel

include/swift/AST/DiagnosticGroups.def

@@ -86,6 +86,7 @@ GROUP(TemporaryPointers, "temporary-pointers")
 GROUP(TrailingClosureMatching, "trailing-closure-matching")
 GROUP(UnknownWarningGroup, "unknown-warning-group")
 GROUP(WeakMutability, "weak-mutability")
+GROUP(StrictExhaustiveSwitches, "strict-exhaustive-switches")
 
 GROUP_LINK(PerformanceHints, ExistentialType)
 GROUP_LINK(PerformanceHints, ReturnTypeImplicitCopy)

include/swift/AST/DiagnosticsSema.def

@@ -114,7 +114,7 @@ ERROR(could_not_find_imported_enum_case,none,
 
 NOTE(did_you_mean_raw_type,none,
      "did you mean to specify a raw type on the enum declaration?", ())
-     
+
 NOTE(did_you_mean_generic_param_as_conformance,none,
      "did you mean to declare %0 as a protocol conformance for %1?", (DeclName, Type))
 
@@ -1302,7 +1302,7 @@ REMARK(transitive_dependency_behavior,none,
        "%1 has %select{a required|an optional|an ignored}2 "
        "transitive dependency on '%0'",
        (StringRef, Identifier, unsigned))
-       
+
 REMARK(explicit_interface_build_skipped,none,
        "Skipped rebuilding module at %0 - up-to-date",
        (StringRef))
@@ -1556,7 +1556,7 @@ ERROR(optional_self_not_unwrapped,none,
       "to make control flow explicit", ())
 NOTE(optional_self_chain,none,
      "reference 'self?.' explicitly", ())
-    
+
 ERROR(missing_unwrap_optional_try,none,
       "value of optional type %0 not unwrapped; did you mean to use 'try!' "
       "or chain with '?'?",
@@ -1567,7 +1567,7 @@ NOTE(missing_forced_downcast, none,
      "did you mean to use 'as!' to force downcast?", ())
 NOTE(missing_optional_downcast, none,
      "did you mean to use 'as?' to conditionally downcast?", ())
-     
+
 WARNING(coercion_may_fail_warning,none,
         "coercion from %0 to %1 may fail; use 'as?' or 'as!' instead",
         (Type, Type))
@@ -4073,7 +4073,7 @@ NOTE(automatic_protocol_synthesis_unsupported,none,
     "automatic synthesis of %0 is not supported for %kindonly1 declarations",
     (const ProtocolDecl *, const NominalTypeDecl *))
 NOTE(comparable_synthesis_raw_value_not_allowed, none,
-     "enum declares raw type %0, preventing synthesized conformance of %1 to %2", 
+     "enum declares raw type %0, preventing synthesized conformance of %1 to %2",
      (Type, Type, Type))
 
 // Dynamic Self
@@ -4679,15 +4679,15 @@ ERROR(unordered_adjacent_operators,none,
 ERROR(missing_builtin_precedence_group,none,
       "broken standard library: missing builtin precedence group %0",
       (Identifier))
-WARNING(nan_comparison, none, 
+WARNING(nan_comparison, none,
        "comparison with '.nan' using %0 is always %select{false|true}1, use "
        "'%2.isNaN' to check if '%3' %select{is not a number|is a number}1",
        (Identifier, bool, StringRef, StringRef))
-WARNING(nan_comparison_without_isnan, none, 
-       "comparison with '.nan' using %0 is always %select{false|true}1", 
+WARNING(nan_comparison_without_isnan, none,
+       "comparison with '.nan' using %0 is always %select{false|true}1",
        (Identifier, bool))
-WARNING(nan_comparison_both_nan, none, 
-       "'.nan' %0 '.nan' is always %select{false|true}1", 
+WARNING(nan_comparison_both_nan, none,
+       "'.nan' %0 '.nan' is always %select{false|true}1",
        (StringRef, bool))
 
 // If you change this, also change enum TryKindForDiagnostics.
@@ -4826,7 +4826,7 @@ ERROR(should_use_empty_dictionary_literal,none,
 WARNING(duplicated_literal_keys_in_dictionary_literal, none,
         "dictionary literal of type %0 has duplicate entries for %1 literal key%select{ %3|}2",
         (Type, StringRef, bool, StringRef))
-NOTE(duplicated_key_declared_here, none, 
+NOTE(duplicated_key_declared_here, none,
      "duplicate key declared here", ())
 
 // Generic specializations
@@ -5732,7 +5732,7 @@ NOTE(property_requires_actor,none,
       "initializer for %kind0 is %1",
       (const VarDecl *, ActorIsolation))
 ERROR(isolated_default_argument_context,none,
-      "%0 default value in a %1 context", 
+      "%0 default value in a %1 context",
       (ActorIsolation, ActorIsolation))
 ERROR(conflicting_default_argument_isolation,none,
       "default argument cannot be both %0 and %1",
@@ -6489,7 +6489,7 @@ ERROR(differentiable_function_type_void_result,
       "differentiable inout parameter, i.e. a non-'@noDerivative' parameter "
       "whose type conforms to 'Differentiable'",
       ())
-ERROR(differentiable_function_type_no_differentiability_parameters, 
+ERROR(differentiable_function_type_no_differentiability_parameters,
       none,
       "'@differentiable' function type requires at least one differentiability "
       "parameter, i.e. a non-'@noDerivative' parameter whose type conforms to "
@@ -7531,6 +7531,9 @@ NOTE(missing_several_cases,none,
 NOTE(missing_unknown_case,none,
      "handle unknown values using \"@unknown default\"", ())
 
+GROUPED_WARNING(decompose_default_case, StrictExhaustiveSwitches,DefaultIgnore,
+  "switch can be made exhaustive without use of 'default'; replace with known cases", ())
+
 NOTE(non_exhaustive_switch_drop_unknown,none,
      "remove '@unknown' to handle remaining values", ())
 
@@ -8569,7 +8572,7 @@ ERROR(safe_and_unsafe_attr,none,
       "%kindbase0 cannot be both '@safe' and '@unsafe'", (const Decl *))
 
 GROUPED_WARNING(unsafe_superclass,StrictMemorySafety,none,
-     "%kindbase0 has superclass involving unsafe type %1", 
+     "%kindbase0 has superclass involving unsafe type %1",
      (const ValueDecl *, Type))
 
 GROUPED_WARNING(conformance_involves_unsafe,StrictMemorySafety,none,

lib/Sema/TypeCheckSwitchStmt.cpp

@@ -973,6 +973,10 @@ namespace {
       }
     }
 
+    static bool hasStrictExhaustiveChecksEnabled(ASTContext &Context) {
+      return !(Context.Diags.isIgnoredDiagnostic(diag::decompose_default_case.ID));
+    }
+
     void checkExhaustiveness(bool limitedChecking) {
       // If the type of the scrutinee is uninhabited, we're already dead.
       // Allow any well-typed patterns through.
@@ -985,11 +989,13 @@ namespace {
       if (limitedChecking) {
         // Reject switch statements with empty blocks.
         if (Switch->getCases().empty())
-          diagnoseMissingCases(RequiresDefault::EmptySwitchBody, Space());
+          diagnoseMissingCases(RequiresDefault::EmptySwitchBody, Space(),
+                               nullptr/* unknown default */, nullptr/* default */);
         return;
       }
 
       const CaseStmt *unknownCase = nullptr;
+      const CaseStmt *defaultCase = nullptr;
       SmallVector<Space, 4> spaces;
       auto &DE = Context.Diags;
       for (auto *caseBlock : Switch->getCases()) {
@@ -1006,8 +1012,14 @@ namespace {
             continue;
 
           // Space is trivially covered with a default clause.
-          if (caseItem.isDefault())
-            return;
+          if (caseItem.isDefault()) {
+            if (!hasStrictExhaustiveChecksEnabled(Context)) {
+              return;
+            }
+            assert(defaultCase == nullptr && "multiple default cases");
+            defaultCase = caseBlock;
+            continue;
+          }
 
           Space projection = projectPattern(caseItem.getPattern());
           bool isRedundant = !projection.isEmpty() &&
@@ -1046,7 +1058,7 @@ namespace {
       auto diff = totalSpace.minus(coveredSpace, DC, &minusCount);
       if (!diff) {
         diagnoseMissingCases(RequiresDefault::SpaceTooLarge, Space(),
-                             unknownCase);
+                             unknownCase, defaultCase);
         return;
       }
 
@@ -1075,19 +1087,19 @@ namespace {
           SmallVector<Space, 4> spaces;
           Space::decomposeDisjuncts(DC, uncovered.getType(), {}, spaces);
           diagnoseMissingCases(RequiresDefault::No, Space::forDisjunct(spaces),
-                               unknownCase);
-        } else {
+                               unknownCase, defaultCase);
+        } else if (!defaultCase) {
           diagnoseMissingCases(Switch->getCases().empty()
                                  ? RequiresDefault::EmptySwitchBody
                                  : RequiresDefault::UncoveredSwitch,
-                               uncovered, unknownCase);
+                               uncovered, unknownCase, defaultCase);
         }
         return;
       }
 
-      diagnoseMissingCases(RequiresDefault::No, uncovered, unknownCase);
+      diagnoseMissingCases(RequiresDefault::No, uncovered, unknownCase, defaultCase);
     }
-    
+
     enum class RequiresDefault {
       No,
       EmptySwitchBody,
@@ -1096,14 +1108,41 @@ namespace {
     };
 
     void diagnoseMissingCases(RequiresDefault defaultReason, Space uncovered,
-                              const CaseStmt *unknownCase = nullptr) {
+                              const CaseStmt *unknownCase, const CaseStmt *defaultCase) {
+      assert(!(unknownCase && defaultCase) && "both @unknown default and default should be caught earlier");
       if (!Switch->getLBraceLoc().isValid()) {
         // There is no '{' in the switch statement, which we already diagnosed
         // in the parser. So there's no real body to speak of and it doesn't
         // make sense to emit diagnostics about missing cases.
         return;
       }
+
+      // Decide whether we want an error or a warning.
+      std::optional<decltype(diag::non_exhaustive_switch)> mainDiagType =
+          diag::non_exhaustive_switch;
+
+      bool downgrade = false;
+
       auto &DE = Context.Diags;
+      if (defaultCase) {
+        if (defaultReason != RequiresDefault::No) {
+          // We actually do need the default,
+          // so it does handle the space, do not emit diagnostics
+          return;
+        }
+
+        // Switch actually is exhaustive with known cases,
+        // so don't emit exhaustive diagnostic
+        mainDiagType = std::nullopt;
+
+        // Warn that the default case is not needed, remove the
+        // default case and enumerate the cases below
+        DE.diagnose(defaultCase->getStartLoc(),
+                    diag::decompose_default_case)
+          .fixItRemoveChars(defaultCase->getStartLoc(),
+                            defaultCase->getEndLoc());
+      }
+
       SourceLoc startLoc = Switch->getStartLoc();
       SourceLoc insertLoc;
       if (unknownCase)
@@ -1114,10 +1153,6 @@ namespace {
       llvm::SmallString<128> buffer;
       llvm::raw_svector_ostream OS(buffer);
 
-      // Decide whether we want an error or a warning.
-      std::optional<decltype(diag::non_exhaustive_switch)> mainDiagType =
-          diag::non_exhaustive_switch;
-      bool downgrade = false;
       if (unknownCase) {
         switch (defaultReason) {
         case RequiresDefault::EmptySwitchBody:
@@ -1224,8 +1259,7 @@ namespace {
 
       // Add notes to explain what's missing.
       auto processUncoveredSpaces =
-          [&](llvm::function_ref<void(const Space &space,
-                                      bool onlyOneUncoveredSpace)> process) {
+          [&](llvm::function_ref<void(const Space &space)> process) {
 
         // Flatten away all disjunctions.
         SmallVector<Space, 4> flats;
@@ -1279,7 +1313,7 @@ namespace {
               continue;
           }
 
-          process(flat, flats.size() == 1);
+          process(flat);
         }
       };
 
@@ -1300,8 +1334,7 @@ namespace {
       SmallString<128> missingSeveralCasesFixIt;
       int diagnosedCases = 0;
 
-      processUncoveredSpaces([&](const Space &space,
-                                 bool onlyOneUncoveredSpace) {
+      processUncoveredSpaces([&](const Space &space) {
         llvm::SmallString<64> fixItBuffer;
         llvm::raw_svector_ostream fixItOS(fixItBuffer);
         if (space.getKind() == SpaceKind::UnknownCase) {