Skip to content

[Sema/IDE] Emit same diagnostics for missing switch cases independent of editor mode #75792

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
Aug 13, 2024
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
76 changes: 31 additions & 45 deletions lib/Sema/TypeCheckSwitchStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1124,8 +1124,6 @@ namespace {
llvm::SmallString<128> buffer;
llvm::raw_svector_ostream OS(buffer);

bool InEditor = Context.LangOpts.DiagnosticsEditorMode;

// Decide whether we want an error or a warning.
std::optional<decltype(diag::non_exhaustive_switch)> mainDiagType =
diag::non_exhaustive_switch;
Expand Down Expand Up @@ -1274,61 +1272,49 @@ namespace {
}
};

// If editing is enabled, emit a formatted error of the form:
// Emit a formatted note of the form, which has a Fix-It associated with
// it, primarily to be used in IDEs:
//
// switch must be exhaustive, do you want to add missing cases?
// case (.none, .some(_)):
// <#code#>
// case (.some(_), .none):
// <#code#>
//
// else:
//
// switch must be exhaustive, consider adding missing cases:
// To also provide actionable output for command line errors, emit notes
// like the following:
//
// missing case '(.none, .some(_))'
// missing case '(.some(_), .none)'
if (InEditor) {
buffer.clear();

bool alreadyEmittedSomething = false;
processUncoveredSpaces([&](const Space &space,
bool onlyOneUncoveredSpace) {
if (space.getKind() == SpaceKind::UnknownCase) {
OS << "@unknown " << tok::kw_default;
if (onlyOneUncoveredSpace) {
OS << ":\n<#fatalError()#>\n";
DE.diagnose(startLoc, diag::missing_unknown_case)
.fixItInsert(insertLoc, buffer.str());
alreadyEmittedSomething = true;
return;
}
} else {
OS << tok::kw_case << " ";
space.show(OS);
}
OS << ":\n" << placeholder << "\n";
});

if (!alreadyEmittedSomething) {
DE.diagnose(startLoc, diag::missing_several_cases, false)
.fixItInsert(insertLoc, buffer.str());
SmallString<128> missingSeveralCasesFixIt;
int diagnosedCases = 0;

processUncoveredSpaces([&](const Space &space,
bool onlyOneUncoveredSpace) {
llvm::SmallString<64> fixItBuffer;
llvm::raw_svector_ostream fixItOS(fixItBuffer);
if (space.getKind() == SpaceKind::UnknownCase) {
fixItOS << "@unknown " << tok::kw_default << ":\n<#fatalError()#>\n";
DE.diagnose(startLoc, diag::missing_unknown_case)
.fixItInsert(insertLoc, fixItBuffer.str());
} else {
llvm::SmallString<64> spaceBuffer;
llvm::raw_svector_ostream spaceOS(spaceBuffer);
space.show(spaceOS);

fixItOS << tok::kw_case << " " << spaceBuffer << ":\n"
<< placeholder << "\n";
DE.diagnose(startLoc, diag::missing_particular_case,
spaceBuffer.str())
.fixItInsert(insertLoc, fixItBuffer);
}
diagnosedCases += 1;
missingSeveralCasesFixIt += fixItBuffer;
});

} else {
processUncoveredSpaces([&](const Space &space,
bool onlyOneUncoveredSpace) {
if (space.getKind() == SpaceKind::UnknownCase) {
auto note = DE.diagnose(startLoc, diag::missing_unknown_case);
if (onlyOneUncoveredSpace)
note.fixItInsert(insertLoc, "@unknown default:\n<#fatalError#>()\n");
return;
}

buffer.clear();
space.show(OS);
DE.diagnose(startLoc, diag::missing_particular_case, buffer.str());
});
if (diagnosedCases > 1) {
DE.diagnose(startLoc, diag::missing_several_cases, false)
.fixItInsert(insertLoc, missingSeveralCasesFixIt.str());
}
}

Expand Down
1 change: 1 addition & 0 deletions test/ClangImporter/availability_open_enums.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ func exhaustiveSwitch(e: NSEnumAddedCasesIn2017) {
switch e { // expected-error{{switch must be exhaustive}}
// expected-note@-1{{add missing case: '.newCaseOne'}}
// expected-note@-2{{handle unknown values using "@unknown default"}}
// expected-note@-3 {{add missing cases}}
case .existingCaseOne:
return
case .existingCaseTwo:
Expand Down
1 change: 1 addition & 0 deletions test/ClangImporter/enum-dataflow.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ case .original:
switch aliasOriginal { // expected-error {{switch must be exhaustive}}
// expected-note@-1 {{add missing case: '.original'}}
// expected-note@-2 {{add missing case: '.differentValue'}}
// expected-note@-3 {{add missing cases}}
case .bySameValue:
break
}
1 change: 1 addition & 0 deletions test/ClangImporter/swift2_warnings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ func useLowercasedEnumCase(x: NSRuncingMode) {
switch x { // expected-error {{switch must be exhaustive}}
// expected-note@-1 {{add missing case: '.mince'}}
// expected-note@-2 {{add missing case: '.quince'}}
// expected-note@-3 {{add missing cases}}
case .Mince: return // expected-error {{'Mince' has been renamed to 'mince'}} {{11-16=mince}}
case .Quince: return // expected-error {{'Quince' has been renamed to 'quince'}} {{11-17=quince}}
}
Expand Down
41 changes: 27 additions & 14 deletions test/Compatibility/exhaustive_switch.swift
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ func checkDiagnosticMinimality(x: Runcible?) {
// expected-note@-1 {{add missing case: '(.fork, _)'}}
// expected-note@-2 {{add missing case: '(.hat, .hat)'}}
// expected-note@-3 {{add missing case: '(_, .fork)'}}
// expected-note@-4 {{add missing cases}} {{+11:3-3=case (.fork, _):\n<#code#>\ncase (.hat, .hat):\n<#code#>\ncase (_, .fork):\n<#code#>\n}}
case (.spoon, .spoon):
break
case (.spoon, .hat):
Expand All @@ -381,6 +382,7 @@ func checkDiagnosticMinimality(x: Runcible?) {
// expected-note@-2 {{add missing case: '(.hat, .spoon)'}}
// expected-note@-3 {{add missing case: '(.spoon, .hat)'}}
// expected-note@-4 {{add missing case: '(_, .fork)'}}
// expected-note@-5 {{add missing cases}} {{+10:3-3=case (.fork, _):\n<#code#>\ncase (.hat, .spoon):\n<#code#>\ncase (.spoon, .hat):\n<#code#>\ncase (_, .fork):\n<#code#>\n}}
case (.spoon, .spoon):
break
case (.hat, .hat):
Expand All @@ -405,6 +407,7 @@ enum LargeSpaceEnum {
func notQuiteBigEnough() -> Bool {
switch (LargeSpaceEnum.case1, LargeSpaceEnum.case2) { // expected-error {{switch must be exhaustive}}
// expected-note@-1 110 {{add missing case:}}
// expected-note@-2 {{add missing cases}}
case (.case0, .case0): return true
case (.case1, .case1): return true
case (.case2, .case2): return true
Expand Down Expand Up @@ -443,6 +446,7 @@ enum ContainsOverlyLargeEnum {
func quiteBigEnough() -> Bool {
switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) { // expected-error {{switch must be exhaustive}}
// expected-note@-1 132 {{add missing case:}}
// expected-note@-2 {{add missing cases}}
case (.case0, .case0): return true
case (.case1, .case1): return true
case (.case2, .case2): return true
Expand Down Expand Up @@ -543,7 +547,7 @@ func quiteBigEnough() -> Bool {
}

// Make sure we haven't just stopped emitting diagnostics.
switch OverlyLargeSpaceEnum.case1 { // expected-error {{switch must be exhaustive}} expected-note 12 {{add missing case}}
switch OverlyLargeSpaceEnum.case1 { // expected-error {{switch must be exhaustive}} expected-note 12 {{add missing case}} expected-note {{add missing cases}}
}
}

Expand All @@ -564,12 +568,14 @@ indirect enum MutuallyRecursive {
func infinitelySized() -> Bool {
switch (InfinitelySized.one, InfinitelySized.one) { // expected-error {{switch must be exhaustive}}
// expected-note@-1 8 {{add missing case:}}
// expected-note@-2 {{add missing cases}}
case (.one, .one): return true
case (.two, .two): return true
}

switch (MutuallyRecursive.one, MutuallyRecursive.one) { // expected-error {{switch must be exhaustive}}
// expected-note@-1 8 {{add missing case:}}
// expected-note@-2 {{add missing cases}}
case (.one, .one): return true
case (.two, .two): return true
}
Expand Down Expand Up @@ -840,7 +846,7 @@ public enum NonExhaustivePayload {
// case.
@inlinable
public func testNonExhaustive(_ value: NonExhaustive, _ payload: NonExhaustivePayload, for interval: TemporalProxy, flag: Bool) {
switch value { // expected-error {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b'}} {{none}}
switch value { // expected-error {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b'}} {{+2:3-3=case .b:\n<#code#>\n}}
case .a: break
}

Expand All @@ -861,12 +867,16 @@ public func testNonExhaustive(_ value: NonExhaustive, _ payload: NonExhaustivePa
@unknown case _: break // no-warning
}

switch value { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b'}} {{none}}
switch value { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b'}} {{+2:3-3=case .b:\n<#code#>\n}}
case .a: break
@unknown case _: break
}

switch value { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.a'}} {{none}} expected-note {{add missing case: '.b'}} {{none}}
switch value {
// expected-warning@-1 {{switch must be exhaustive}} {{none}}
// expected-note@-2 {{add missing case: '.a'}} {{+5:3-3=case .a:\n<#code#>\n}}
// expected-note@-3 {{add missing case: '.b'}} {{+5:3-3=case .b:\n<#code#>\n}}
// expected-note@-4 {{add missing cases}} {{+5:3-3=case .a:\n<#code#>\ncase .b:\n<#code#>\n}}
@unknown case _: break
}

Expand Down Expand Up @@ -932,7 +942,7 @@ public func testNonExhaustive(_ value: NonExhaustive, _ payload: NonExhaustivePa
} // no-warning

// Test payloaded enums.
switch payload { // expected-error {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(_)'}} {{none}}
switch payload { // expected-error {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(_)'}} {{+2:3-3=case .b(_):\n<#code#>\n}}
case .a: break
}

Expand All @@ -953,17 +963,17 @@ public func testNonExhaustive(_ value: NonExhaustive, _ payload: NonExhaustivePa
@unknown case _: break // no-warning
}

switch payload { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(_)'}} {{none}}
switch payload { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(_)'}} {{+2:3-3=case .b(_):\n<#code#>\n}}
case .a: break
@unknown case _: break
}

switch payload { // expected-error {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(true)'}} {{none}}
switch payload { // expected-error {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(true)'}} {{+3:3-3=case .b(true):\n<#code#>\n}}
case .a: break
case .b(false): break
}

switch payload { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(true)'}} {{none}}
switch payload { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(true)'}} {{+3:3-3=case .b(true):\n<#code#>\n}}
case .a: break
case .b(false): break
@unknown case _: break
Expand Down Expand Up @@ -1017,12 +1027,15 @@ public func testNonExhaustiveWithinModule(_ value: NonExhaustive, _ payload: Non
@unknown case _: break // no-warning
}

switch value { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b'}} {{none}}
switch value { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b'}} {{+2:3-3=case .b:\n<#code#>\n}}
case .a: break
@unknown case _: break
}

switch value { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.a'}} {{none}} expected-note {{add missing case: '.b'}} {{none}}
switch value { // expected-warning {{switch must be exhaustive}} {{none}}
// expected-note@-1 {{add missing case: '.a'}} {{+4:3-3=case .a:\n<#code#>\n}}
// expected-note@-2 {{add missing case: '.b'}} {{+4:3-3=case .b:\n<#code#>\n}}
// expected-note@-3 {{add missing cases}} {{+4:3-3=case .a:\n<#code#>\ncase .b:\n<#code#>\n}}
@unknown case _: break
}

Expand Down Expand Up @@ -1071,7 +1084,7 @@ public func testNonExhaustiveWithinModule(_ value: NonExhaustive, _ payload: Non
}

// Test payloaded enums.
switch payload { // expected-error {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(_)'}} {{none}}
switch payload { // expected-error {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(_)'}} {{+2:3-3=case .b(_):\n<#code#>\n}}
case .a: break
}

Expand All @@ -1092,17 +1105,17 @@ public func testNonExhaustiveWithinModule(_ value: NonExhaustive, _ payload: Non
@unknown case _: break // no-warning
}

switch payload { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(_)'}} {{none}}
switch payload { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(_)'}} {{+2:3-3=case .b(_):\n<#code#>\n}}
case .a: break
@unknown case _: break
}

switch payload { // expected-error {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(true)'}} {{none}}
switch payload { // expected-error {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(true)'}} {{+3:3-3=case .b(true):\n<#code#>\n}}
case .a: break
case .b(false): break
}

switch payload { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(true)'}} {{none}}
switch payload { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(true)'}} {{+3:3-3=case .b(true):\n<#code#>\n}}
case .a: break
case .b(false): break
@unknown case _: break
Expand Down
8 changes: 6 additions & 2 deletions test/FixCode/fixits-empty-switch-multifile.swift
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
// RUN: not %swift -typecheck -target %target-triple %s %S/Inputs/fixits-enum-multifile.swift -emit-fixits-path %t.remap -I %S/Inputs -diagnostics-editor-mode
// RUN: c-arcmt-test %t.remap | arcmt-test -verify-transformed-files %s.result
// RUN: %target-typecheck-verify-swift %s %S/Inputs/fixits-enum-multifile.swift -I %S/Inputs

func foo1(_ e: EMulti) {
switch e {
// expected-error@-1 {{switch must be exhaustive}}
// expected-note@-2 {{add missing case: '.e1'}} {{+6:3-3=case .e1:\n<#code#>\n}}
// expected-note@-3 {{add missing case: '.e2'}} {{+6:3-3=case .e2:\n<#code#>\n}}
// expected-note@-4 {{add missing case: '.e3(_)'}} {{+6:3-3=case .e3(_):\n<#code#>\n}}
// expected-note@-5 {{add missing cases}} {{+6:3-3=case .e1:\n<#code#>\ncase .e2:\n<#code#>\ncase .e3(_):\n<#code#>\n}}
}
}
13 changes: 0 additions & 13 deletions test/FixCode/fixits-empty-switch-multifile.swift.result

This file was deleted.

10 changes: 7 additions & 3 deletions test/FixCode/fixits-empty-switch.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// RUN: not %swift -typecheck -target %target-triple %s -emit-fixits-path %t.remap -I %S/Inputs -diagnostics-editor-mode
// RUN: c-arcmt-test %t.remap | arcmt-test -verify-transformed-files %s.result
// RUN: %target-typecheck-verify-swift %s -I %S/Inputs -diagnostics-editor-mode

enum E1 {
case e1
Expand All @@ -9,10 +8,15 @@ enum E1 {

func foo1(_ e: E1) {
switch e {
//expected-error@-1 {{switch must be exhaustive}}
//expected-note@-2 {{add missing case: '.e1'}} {{+6:3-3=case .e1:\n<#code#>\n}}
//expected-note@-3 {{add missing case: '.e2'}} {{+6:3-3=case .e2:\n<#code#>\n}}
//expected-note@-4 {{add missing case: '.e3'}} {{+6:3-3=case .e3:\n<#code#>\n}}
//expected-note@-5 {{add missing cases}} {{+6:3-3=case .e1:\n<#code#>\ncase .e2:\n<#code#>\ncase .e3:\n<#code#>\n}}
}
}

func foo1 (_ i : Int) {
switch i {
switch i { // expected-error {{'switch' statement body must have at least one 'case' or 'default' block; add a default case}} {{+1:3-3=default:\n<#code#>\n}}
}
}
26 changes: 0 additions & 26 deletions test/FixCode/fixits-empty-switch.swift.result

This file was deleted.

Loading