Make unnecessary_breaks apply to default clauses
Fixes https://github.com/dart-lang/linter/issues/5061.
Change-Id: Ia5b9db3921d1d3aa95e19bc88cd3b4cde666d919
Bug: https://github.com/dart-lang/linter/issues/5061
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/380285
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
diff --git a/pkg/analysis_server/test/src/services/correction/fix/remove_break_test.dart b/pkg/analysis_server/test/src/services/correction/fix/remove_break_test.dart
index 50ff1a0..d9d4fc6 100644
--- a/pkg/analysis_server/test/src/services/correction/fix/remove_break_test.dart
+++ b/pkg/analysis_server/test/src/services/correction/fix/remove_break_test.dart
@@ -32,6 +32,8 @@
1; break;
case 2:
2; break;
+ default:
+ 3; break;
}
}
''');
@@ -42,6 +44,8 @@
1;
case 2:
2;
+ default:
+ 3;
}
}
''');
@@ -57,6 +61,9 @@
case 2:
2;
break;
+ default:
+ 3;
+ break;
}
}
''');
@@ -67,6 +74,8 @@
1;
case 2:
2;
+ default:
+ 3;
}
}
''');
@@ -107,6 +116,29 @@
''');
}
+ Future<void> test_single_sameLine_default() async {
+ await resolveTestCode('''
+void f() {
+ switch (0) {
+ case 1:
+ 1;
+ default:
+ 2; break;
+ }
+}
+''');
+ await assertHasFix('''
+void f() {
+ switch (0) {
+ case 1:
+ 1;
+ default:
+ 2;
+ }
+}
+''');
+ }
+
Future<void> test_single_separateLine() async {
await resolveTestCode('''
void f() {
@@ -130,4 +162,28 @@
}
''');
}
+
+ Future<void> test_single_separateLine_default() async {
+ await resolveTestCode('''
+void f() {
+ switch (0) {
+ case 1:
+ 1;
+ default:
+ 2;
+ break;
+ }
+}
+''');
+ await assertHasFix('''
+void f() {
+ switch (0) {
+ case 1:
+ 1;
+ default:
+ 2;
+ }
+}
+''');
+ }
}
diff --git a/pkg/linter/lib/src/rules/unnecessary_breaks.dart b/pkg/linter/lib/src/rules/unnecessary_breaks.dart
index faccf37..bf716ac 100644
--- a/pkg/linter/lib/src/rules/unnecessary_breaks.dart
+++ b/pkg/linter/lib/src/rules/unnecessary_breaks.dart
@@ -90,7 +90,7 @@
visitBreakStatement(BreakStatement node) {
if (node.label != null) return;
var parent = node.parent;
- if (parent is SwitchPatternCase) {
+ if (parent is SwitchMember) {
var statements = parent.statements;
if (statements.length == 1) return;
if (node == statements.last) {
diff --git a/pkg/linter/test/rules/unnecessary_breaks_test.dart b/pkg/linter/test/rules/unnecessary_breaks_test.dart
index 0e3ab29..c363f4b 100644
--- a/pkg/linter/test/rules/unnecessary_breaks_test.dart
+++ b/pkg/linter/test/rules/unnecessary_breaks_test.dart
@@ -17,6 +17,73 @@
@override
String get lintRule => 'unnecessary_breaks';
+ test_default() async {
+ await assertDiagnostics(r'''
+f() {
+ switch (1) {
+ case 1:
+ f();
+ default:
+ f();
+ break;
+ }
+}
+''', [
+ lint(74, 6),
+ ]);
+ }
+
+ test_default_empty() async {
+ // We allow the body of a `case` clause to be just a single `break;`,
+ // because that's necessary to prevent the clause from being grouped with
+ // the clause that follows it.
+ //
+ // For a `default` clause, that's not necessary, because the `default`
+ // clause is required to come last. But we allow just a single `break;`
+ // anyway, for consistency.
+ await assertNoDiagnostics(r'''
+f() {
+ switch (1) {
+ case 2:
+ f();
+ default:
+ break;
+ }
+}
+''');
+ }
+
+ test_default_notLast_ok() async {
+ // No lint is needed because there is already a DEAD_CODE warning.
+ await assertDiagnostics(r'''
+f(bool c) {
+ switch (1) {
+ case 1:
+ f(true);
+ default:
+ break;
+ f(true);
+ }
+}
+''', [
+ // No lint.
+ error(WarningCode.DEAD_CODE, 86, 8),
+ ]);
+ }
+
+ test_switch_pre30_default_ok() async {
+ await assertNoDiagnostics(r'''
+// @dart=2.19
+f() {
+ switch (1) {
+ default:
+ f();
+ break;
+ }
+}
+''');
+ }
+
test_switch_pre30_ok() async {
await assertNoDiagnostics(r'''
// @dart=2.19
@@ -46,6 +113,20 @@
]);
}
+ test_switchPatternCase_default_ok() async {
+ await assertNoDiagnostics(r'''
+f(bool c) {
+ switch (1) {
+ case 1:
+ f(true);
+ default:
+ if (c) break;
+ f(true);
+ }
+}
+''');
+ }
+
test_switchPatternCase_empty_ok() async {
await assertNoDiagnostics(r'''
f() {
