Skip to content

Special-case diagnostic for when you just need @unknown default #21695

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -4223,6 +4223,9 @@ NOTE(redundant_particular_literal_case_here,none,
"first occurrence of identical literal pattern is here", ())

WARNING(non_exhaustive_switch_warn,none, "switch must be exhaustive", ())
WARNING(non_exhaustive_switch_unknown_only,none,
"switch covers known cases, but %0 may have additional unknown values"
"%select{|, possibly added in future versions}1", (Type, bool))

WARNING(override_nsobject_hashvalue,none,
"override of 'NSObject.hashValue' is deprecated; "
Expand Down
24 changes: 18 additions & 6 deletions lib/Sema/TypeCheckSwitchStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1015,7 +1015,8 @@ namespace {
bool InEditor = TC.Context.LangOpts.DiagnosticsEditorMode;

// Decide whether we want an error or a warning.
auto mainDiagType = diag::non_exhaustive_switch;
Optional<decltype(diag::non_exhaustive_switch)> mainDiagType =
diag::non_exhaustive_switch;
if (unknownCase) {
switch (defaultReason) {
case RequiresDefault::EmptySwitchBody:
Expand All @@ -1042,16 +1043,25 @@ namespace {
switch (uncovered.checkDowngradeToWarning()) {
case DowngradeToWarning::No:
break;
case DowngradeToWarning::ForUnknownCase:
case DowngradeToWarning::ForUnknownCase: {
if (TC.Context.LangOpts.DebuggerSupport ||
TC.Context.LangOpts.Playground ||
!TC.getLangOpts().EnableNonFrozenEnumExhaustivityDiagnostics) {
// Don't require covering unknown cases in the debugger or in
// playgrounds.
return;
}
// Missing '@unknown' is just a warning.
mainDiagType = diag::non_exhaustive_switch_warn;
assert(defaultReason == RequiresDefault::No);
Type subjectType = Switch->getSubjectExpr()->getType();
bool shouldIncludeFutureVersionComment = false;
if (auto *theEnum = subjectType->getEnumOrBoundGenericEnum()) {
shouldIncludeFutureVersionComment =
theEnum->getParentModule()->isSystemModule();
}
TC.diagnose(startLoc, diag::non_exhaustive_switch_unknown_only,
subjectType, shouldIncludeFutureVersionComment);
mainDiagType = None;
}
break;
}

Expand All @@ -1066,7 +1076,7 @@ namespace {
return;
case RequiresDefault::UncoveredSwitch: {
OS << tok::kw_default << ":\n" << placeholder << "\n";
TC.diagnose(startLoc, mainDiagType);
TC.diagnose(startLoc, mainDiagType.getValue());
TC.diagnose(startLoc, diag::missing_several_cases, /*default*/true)
.fixItInsert(insertLoc, buffer.str());
}
Expand All @@ -1083,7 +1093,9 @@ namespace {
// If there's nothing else to diagnose, bail.
if (uncovered.isEmpty()) return;

TC.diagnose(startLoc, mainDiagType);
// Check if we still have to emit the main diganostic.
if (mainDiagType.hasValue())
TC.diagnose(startLoc, mainDiagType.getValue());

// Add notes to explain what's missing.
auto processUncoveredSpaces =
Expand Down
4 changes: 1 addition & 3 deletions test/ClangImporter/enum-error.swift
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,9 @@ func testError() {
// CHECK: sil_witness_table shared [serialized] ExhaustiveError: _BridgedStoredNSError module __ObjC
let terr = getErr()
switch (terr) { case .TENone, .TEOne, .TETwo: break }
// EXHAUSTIVE: [[@LINE-1]]:{{.+}}: warning: switch must be exhaustive
// EXHAUSTIVE: [[@LINE-1]]:{{.+}}: warning: switch covers known cases, but 'TestError.Code' may have additional unknown values
// EXHAUSTIVE: [[@LINE-2]]:{{.+}}: note: handle unknown values using "@unknown default"

// FIXME: This should still be an error because there are /known/ cases that
// aren't covered.
switch (terr) { case .TENone, .TEOne: break }
// EXHAUSTIVE: [[@LINE-1]]:{{.+}}: error: switch must be exhaustive
// EXHAUSTIVE: [[@LINE-2]]:{{.+}}: note: add missing case: '.TETwo'
Expand Down
50 changes: 50 additions & 0 deletions test/ClangImporter/enum-exhaustivity-system.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// RUN: %target-swift-frontend -typecheck %s -Xcc -isystem -Xcc %S/Inputs/custom-modules -verify -enable-nonfrozen-enum-exhaustivity-diagnostics

import EnumExhaustivity

func test(_ value: RegularEnum, _ exhaustiveValue: ExhaustiveEnum) {
switch value { // expected-warning {{switch covers known cases, but 'RegularEnum' may have additional unknown values, possibly added in future versions}} expected-note {{handle unknown values using "@unknown default"}}
case .A: break
case .B: break
}

switch exhaustiveValue { // always okay
case .A: break
case .B: break
}
}

func testAttributes(
_ rete: RegularEnumTurnedExhaustive,
_ arete: AnotherRegularEnumTurnedExhaustive,
_ retetb: RegularEnumTurnedExhaustiveThenBackViaAPINotes,
_ fdte: ForwardDeclaredTurnedExhaustive,
_ fdo: ForwardDeclaredOnly
) {
switch rete {
case .A, .B: break
}

switch arete {
case .A, .B: break
}

switch retetb { // expected-warning {{switch covers known cases, but 'RegularEnumTurnedExhaustiveThenBackViaAPINotes' may have additional unknown values, possibly added in future versions}} expected-note {{handle unknown values using "@unknown default"}}
case .A, .B: break
}

switch fdte {
case .A, .B: break
}

switch fdo {
case .A, .B: break
}
}

func testUnavailableCases(_ value: UnavailableCases) {
switch value { // okay
case .A: break
case .B: break
}
}
4 changes: 2 additions & 2 deletions test/ClangImporter/enum-exhaustivity.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import EnumExhaustivity

func test(_ value: RegularEnum, _ exhaustiveValue: ExhaustiveEnum) {
switch value { // expected-error {{switch must be exhaustive}} expected-note {{handle unknown values using "@unknown default"}}
switch value { // expected-error {{switch covers known cases, but 'RegularEnum' may have additional unknown values}} expected-note {{handle unknown values using "@unknown default"}}
case .A: break
case .B: break
}
Expand All @@ -43,7 +43,7 @@ func testAttributes(
case .A, .B: break
}

switch retetb { // expected-error {{switch must be exhaustive}} expected-note {{handle unknown values using "@unknown default"}}
switch retetb { // expected-error {{switch covers known cases, but 'RegularEnumTurnedExhaustiveThenBackViaAPINotes' may have additional unknown values}} expected-note {{handle unknown values using "@unknown default"}}
case .A, .B: break
}

Expand Down
4 changes: 2 additions & 2 deletions test/ClangImporter/enum-inferred-exhaustivity.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@

func test(_ value: EnumWithDefaultExhaustivity) {
// We want to assume such enums are non-frozen.
switch value { // expected-error {{switch must be exhaustive}} expected-note {{handle unknown values using "@unknown default"}}
switch value { // expected-error {{switch covers known cases, but 'EnumWithDefaultExhaustivity' may have additional unknown values}} expected-note {{handle unknown values using "@unknown default"}}
case .loneCase: break
}
}

func test(_ value: EnumWithSpecialAttributes) {
// Same, but with the attributes macro shipped in the Xcode 9 SDKs.
switch value { // expected-error {{switch must be exhaustive}} expected-note {{handle unknown values using "@unknown default"}}
switch value { // expected-error {{switch covers known cases, but 'EnumWithSpecialAttributes' may have additional unknown values}} expected-note {{handle unknown values using "@unknown default"}}
case .loneCase: break
}
}
4 changes: 2 additions & 2 deletions test/ClangImporter/enum-new.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ _ = .Red as Color
_ = .Cyan as MoreColor

func test() {
switch getColor() { // expected-warning {{switch must be exhaustive}} expected-note{{handle unknown values using "@unknown default"}}
switch getColor() { // expected-warning {{switch covers known cases, but 'Color' may have additional unknown values}} expected-note{{handle unknown values using "@unknown default"}}
case .Red, .Blue, .Green: break
}

switch getMoreColor() { // expected-warning {{switch must be exhaustive}} expected-note{{handle unknown values using "@unknown default"}}
switch getMoreColor() { // expected-warning {{switch covers known cases, but 'MoreColor' may have additional unknown values}} expected-note{{handle unknown values using "@unknown default"}}
case .Yellow, .Magenta, .Black, .Cyan: break
}

Expand Down
2 changes: 1 addition & 1 deletion test/ClangImporter/enum-objc.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// REQUIRES: objc_interop

func test(_ value: SwiftEnum, _ exhaustiveValue: ExhaustiveEnum) {
switch value { // expected-warning {{switch must be exhaustive}} expected-note {{handle unknown values using "@unknown default"}}
switch value { // expected-warning {{switch covers known cases, but 'SwiftEnum' may have additional unknown values}} expected-note {{handle unknown values using "@unknown default"}}
case .one: break
case .two: break
case .three: break
Expand Down
12 changes: 6 additions & 6 deletions test/Sema/exhaustive_switch.swift
Original file line number Diff line number Diff line change
Expand Up @@ -802,7 +802,7 @@ public func testNonExhaustive(_ value: NonExhaustive, _ payload: NonExhaustivePa
case .a: break
}

switch value { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{handle unknown values using "@unknown default"}} {{3-3=@unknown default:\n<#fatalError#>()\n}}
switch value { // expected-warning {{switch covers known cases, but 'NonExhaustive' may have additional unknown values}} {{none}} expected-note {{handle unknown values using "@unknown default"}} {{3-3=@unknown default:\n<#fatalError#>()\n}}
case .a: break
case .b: break
}
Expand Down Expand Up @@ -834,7 +834,7 @@ public func testNonExhaustive(_ value: NonExhaustive, _ payload: NonExhaustivePa
}

// Test being part of other spaces.
switch value as Optional { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.some(_)'}}
switch value as Optional { // expected-warning {{switch covers known cases, but 'Optional<NonExhaustive>' may have additional unknown values}} {{none}} expected-note {{add missing case: '.some(_)'}}
case .a?: break
case .b?: break
case nil: break
Expand All @@ -852,7 +852,7 @@ public func testNonExhaustive(_ value: NonExhaustive, _ payload: NonExhaustivePa
case nil: break
} // no-warning

switch (value, flag) { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '(_, false)'}}
switch (value, flag) { // expected-warning {{switch covers known cases, but '(NonExhaustive, Bool)' may have additional unknown values}} {{none}} expected-note {{add missing case: '(_, false)'}}
case (.a, _): break
case (.b, false): break
case (_, true): break
Expand All @@ -865,7 +865,7 @@ public func testNonExhaustive(_ value: NonExhaustive, _ payload: NonExhaustivePa
@unknown case _: break
} // no-warning

switch (flag, value) { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '(false, _)'}}
switch (flag, value) { // expected-warning {{switch covers known cases, but '(Bool, NonExhaustive)' may have additional unknown values}} {{none}} expected-note {{add missing case: '(false, _)'}}
case (_, .a): break
case (false, .b): break
case (true, _): break
Expand All @@ -878,7 +878,7 @@ public func testNonExhaustive(_ value: NonExhaustive, _ payload: NonExhaustivePa
@unknown case _: break
} // no-warning

switch (value, value) { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '(_, _)'}}
switch (value, value) { // expected-warning {{switch covers known cases, but '(NonExhaustive, NonExhaustive)' may have additional unknown values}} {{none}} expected-note {{add missing case: '(_, _)'}}
case (.a, _), (_, .a): break
case (.b, _), (_, .b): break
}
Expand All @@ -894,7 +894,7 @@ public func testNonExhaustive(_ value: NonExhaustive, _ payload: NonExhaustivePa
case .a: break
}

switch payload { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{handle unknown values using "@unknown default"}} {{3-3=@unknown default:\n<#fatalError#>()\n}}
switch payload { // expected-warning {{switch covers known cases, but 'NonExhaustivePayload' may have additional unknown values}} {{none}} expected-note {{handle unknown values using "@unknown default"}} {{3-3=@unknown default:\n<#fatalError#>()\n}}
case .a: break
case .b: break
}
Expand Down
2 changes: 1 addition & 1 deletion test/Sema/exhaustive_switch_testable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func testFrozen(_ e: FrozenEnum) -> Int {
}

func testNonFrozen(_ e: NonFrozenEnum) -> Int {
// VERIFY-NON-FROZEN: exhaustive_switch_testable.swift:[[@LINE+1]]:{{[0-9]+}}: warning: switch must be exhaustive
// VERIFY-NON-FROZEN: exhaustive_switch_testable.swift:[[@LINE+1]]:{{[0-9]+}}: warning: switch covers known cases, but 'NonFrozenEnum' may have additional unknown values
switch e {
case .a: return 1
case .b, .c: return 2
Expand Down
2 changes: 1 addition & 1 deletion test/stmt/nonexhaustive_switch_stmt_editor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public func testNonExhaustive(_ value: NonExhaustive) {
case .a: break
}

switch value { // expected-warning {{switch must be exhaustive}}
switch value { // expected-warning {{switch covers known cases, but 'NonExhaustive' may have additional unknown values}}
// expected-note@-1 {{handle unknown values using "@unknown default"}} {{3-3=@unknown default:\n<#fatalError()#>\n}}
case .a: break
case .b: break
Expand Down