diff --git a/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart b/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart index c2be3f9f812..1691a69ba4b 100644 --- a/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart +++ b/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart @@ -4592,7 +4592,7 @@ class _FlowAnalysisImpl? breakState = context._breakModel; // If there is an implicit fall-through default, join it to any breaks. - if (!isExhaustive) breakState = _join(breakState, context._previous); + if (!isExhaustive) breakState = _join(breakState, context._unmatched); // If there were no breaks (neither implicit nor explicit), then // `breakState` will be `null`. This means this is an empty switch diff --git a/pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart b/pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart index e9cabd03e02..fa7f1bd1d3f 100644 --- a/pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart +++ b/pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart @@ -9131,6 +9131,106 @@ main() { ]); }); }); + + group('Trivial exhaustiveness:', () { + // Although flow analysis doesn't attempt to do full exhaustiveness + // checking on switch statements, it understands that if any single case + // fully covers the matched value type, the switch statement is + // exhaustive. (Such a switch is called "trivially exhaustive"). + // + // Note that we don't test all possible patterns, because the flow + // analysis logic for detecting trivial exhaustiveness builds on the + // logic for tracking the "unmatched" state, which is tested elsewhere. + test('exhaustive', () { + h.run([ + switch_(expr('Object'), [ + wildcard().switchCase.then([ + return_(), + ]), + ]), + checkReachable(false), + ]); + }); + + test('exhaustive but a reachable switch case completes', () { + // In this case, even though the switch is trivially exhaustive, the + // code after the switch is reachable because one of the reachable + // switch cases completes normally. + h.run([ + switch_(expr('Object'), [ + wildcard(type: 'int').switchCase.then([ + checkReachable(true), + ]), + wildcard().switchCase.then([ + return_(), + ]), + ]), + checkReachable(true), + ]); + }); + + test('exhaustive but an unreachable switch case completes', () { + // In this case, even though the `int` case completes normally, that + // case is unreachable, so the code after the switch is unreachable. + h.run([ + switch_(expr('Object'), [ + wildcard().switchCase.then([ + return_(), + ]), + wildcard(type: 'int').switchCase.then([ + checkReachable(false), + ]), + ]), + checkReachable(false), + ]); + }); + + test('exhaustive but a reachable switch case breaks', () { + // In this case, even though the switch is trivially exhaustive, the + // code after the switch is reachable because one of the reachable + // switch cases ends in a break. + h.run([ + switch_(expr('Object'), [ + wildcard(type: 'int').switchCase.then([ + checkReachable(true), + break_(), + ]), + wildcard().switchCase.then([ + return_(), + ]), + ]), + checkReachable(true), + ]); + }); + + test('exhaustive but an unreachable switch case breaks', () { + // In this case, even though the `int` case breaks, that case is + // unreachable, so the code after the switch is unreachable. + h.run([ + switch_(expr('Object'), [ + wildcard().switchCase.then([ + return_(), + ]), + wildcard(type: 'int').switchCase.then([ + checkReachable(false), + break_(), + ]), + ]), + checkReachable(false), + ]); + }); + + test('not exhaustive', () { + h.run([ + switch_(expr('Object'), [ + wildcard(type: 'int').switchCase.then([ + return_(), + ]), + ]), + checkReachable(true), + ]); + }); + }); }); group('Variable pattern:', () { diff --git a/pkg/nnbd_migration/lib/src/edge_builder.dart b/pkg/nnbd_migration/lib/src/edge_builder.dart index bfb2a59d5df..26d125601c5 100644 --- a/pkg/nnbd_migration/lib/src/edge_builder.dart +++ b/pkg/nnbd_migration/lib/src/edge_builder.dart @@ -1960,6 +1960,8 @@ class EdgeBuilder extends GeneralizingAstVisitor var hasLabel = member.labels.isNotEmpty; _flowAnalysis!.switchStatement_beginAlternatives(); _flowAnalysis!.switchStatement_beginAlternative(); + _flowAnalysis!.constantPattern_end(node.expression, scrutineeType, + patternsEnabled: false); _flowAnalysis!.switchStatement_endAlternative(null, {}); _flowAnalysis! .switchStatement_endAlternatives(node, hasLabels: hasLabel); diff --git a/tests/language/patterns/switch_trivial_exhaustiveness_error_test.dart b/tests/language/patterns/switch_trivial_exhaustiveness_error_test.dart new file mode 100644 index 00000000000..a0aa0526301 --- /dev/null +++ b/tests/language/patterns/switch_trivial_exhaustiveness_error_test.dart @@ -0,0 +1,685 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// SharedOptions=--enable-experiment=patterns,records + +// Flow analysis doesn't do full exhaustiveness analysis of switch statements, +// but it detects if a switch statement is "trivially exhaustive". A switch +// statement is trivially exhaustive if it has at least one case that fully +// covers the matched value type. +// +// Also, flow analysis understands that after a case that fully covers the +// matched value type, any further cases are unreachable. +// +// We detect whether flow analysis considers the switch exhaustive by assigning +// to a nullable variable in all cases (this promotes the variable to +// non-nullable), and seeing whether the promotion lasts after the switch. + +import '../static_type_helper.dart'; + +void testTwoCasesSecondExhaustive(Object x) { + // Trivially exhaustive because the second case fully covers the matched type + bool? y; + switch (x) { + case int _: + y = true; + case _: + y = true; + } + y.expectStaticType>(); +} + +void testTwoCasesNotExhaustive(Object x) { + // Not exhaustive because neither case fully covers the matched type + bool? y; + switch (x) { + case int _: + y = true; + case String _: + y = true; + } + y.expectStaticType>(); +} + +void testUnreachableCase(Object x) { + // Not only is this switch trivially exhaustive, but also the second case is + // unreachable, and hence `y` remains promoted after the switch. + bool? y; + switch (x) { + case _: + y = true; + case int _: +// ^^^^ +// [analyzer] HINT.UNREACHABLE_SWITCH_CASE + y = null; + } + y.expectStaticType>(); +} + +void testCastWhereSubpatternAlwaysMatches(Object x) { + // Trivially exhaustive + bool? y; + switch (x) { + case _ as int: + y = true; + } + y.expectStaticType>(); +} + +void testCastWhereSubpatternMatchesCastType(Object x) { + // Trivially exhaustive + bool? y; + switch (x) { + case bool() as bool: + y = true; + } + y.expectStaticType>(); +} + +void testCastWhereSubpatternMayFailToMatch(Object x) { + // Not exhaustive + bool? y; + switch (x) { + case (== 0) as int: + y = true; + } + y.expectStaticType>(); +} + +void testListEmpty(List x) { + // Not exhaustive + bool? y; + switch (x) { + case []: + y = true; + } + y.expectStaticType>(); +} + +void testListContainingNonRestPattern(List x) { + // Not exhaustive + bool? y; + switch (x) { + case [_]: + y = true; + } + y.expectStaticType>(); +} + +void testListContainingNonRestPatternAndRestPattern(List x) { + // Not exhaustive + bool? y; + switch (x) { + case [_, ...]: + y = true; + } + y.expectStaticType>(); +} + +void testListContainingRestPatternAndNonRestPattern(List x) { + // Not exhaustive + bool? y; + switch (x) { + case [..., _]: + y = true; + } + y.expectStaticType>(); +} + +void testListContainingOnlyRestPattern(List x) { + // TODO(paulberry): this should be trivially exhaustive + bool? y; + switch (x) { + case [...]: + y = true; + } + y.expectStaticType>(); +} + +void testListContainingOnlyRestPatternWithSubpatternWildcard(List x) { + // TODO(paulberry): this should be trivially exhaustive + bool? y; + switch (x) { + case [..._]: + y = true; + } + y.expectStaticType>(); +} + +void testListContainingOnlyRestPatternWithSubpatternAnyList(List x) { + // TODO(paulberry): this should be trivially exhaustive + bool? y; + switch (x) { + case [...[...]]: + y = true; + } + y.expectStaticType>(); +} + +void testListContainingOnlyRestPatternWithSubpatternObjectPattern( + List x) { + // TODO(paulberry): this should be trivially exhaustive + bool? y; + switch (x) { + case [...List()]: + y = true; + } + y.expectStaticType>(); +} + +void testListContainingOnlyRestPatternWithSubpatternOther(List x) { + // Not exhaustive + bool? y; + switch (x) { + case [...List(length: 1)]: + y = true; + } + y.expectStaticType>(); +} + +void testListSupertype(List x) { + // TODO(paulberry): this should be trivially exhaustive + bool? y; + switch (x) { + case [...]: + y = true; + } + y.expectStaticType>(); +} + +void testListSubtype(List x) { + // Not exhaustive + bool? y; + switch (x) { + case [...]: + y = true; + } + y.expectStaticType>(); +} + +void testListSubtypeObject(Object x) { + // Not exhaustive + bool? y; + switch (x) { + case [...]: + y = true; + } + y.expectStaticType>(); +} + +void testListUnrelatedType(List x) { + // Not exhaustive + bool? y; + switch (x) { + case [...]: + y = true; + } + y.expectStaticType>(); +} + +void testMap(Map x) { + // Not exhaustive + bool? y; + switch (x) { + case {0: _}: + y = true; + } + y.expectStaticType>(); +} + +void testLogicalAndBothMatch(Object x) { + // Trivially exhaustive because both subpatterns always match + bool? y; + switch (x) { + case _ && _: + y = true; + } + y.expectStaticType>(); +} + +void testLogicalAndLhsMatches(Object x) { + // Not exhaustive because only the LHS always matches + bool? y; + switch (x) { + case _ && int _: + y = true; + } + y.expectStaticType>(); +} + +void testLogicalAndRhsMatches(Object x) { + // Not exhaustive because only the RHS always matches + bool? y; + switch (x) { + case _ && int _: + y = true; + } + y.expectStaticType>(); +} + +void testLogicalAndNeitherMatches(Object x) { + // Not exhaustive because neither side always matches + bool? y; + switch (x) { + case int _ && String _: + y = true; + } + y.expectStaticType>(); +} + +void testLogicalOrBothMatch(Object x) { + // Trivially exhaustive because both subpatterns always match + bool? y; + switch (x) { + case _ || _: + y = true; + } + y.expectStaticType>(); +} + +void testLogicalOrLhsMatches(Object x) { + // Trivially exhaustive because the LHS always matches + bool? y; + switch (x) { + case _ || int _: + y = true; + } + y.expectStaticType>(); +} + +void testLogicalOrRhsMatches(Object x) { + // Trivially exhaustive because the RHS always matches + bool? y; + switch (x) { + case _ || int _: + y = true; + } + y.expectStaticType>(); +} + +void testLogicalOrNeitherMatches(Object x) { + // Not exhaustive because neither side always matches + bool? y; + switch (x) { + case int _ || String _: + y = true; + } + y.expectStaticType>(); +} + +void testNullCheckAlwaysMatches(Object x) { + // TODO(paulberry): should be trivially exhaustive because the matched value + // type is non-nullable and the subpattern always matches + bool? y; + switch (x) { + case _?: + // ^ + // [analyzer] STATIC_WARNING.UNNECESSARY_NULL_CHECK_PATTERN + // [cfe] The null-check pattern will have no effect because the matched type isn't nullable. + y = true; + } + y.expectStaticType>(); +} + +void testNullCheckNullableMatchedValueType(Object? x) { + // Not exhaustive because the matched value type is nullable + bool? y; + switch (x) { + case _?: + y = true; + } + y.expectStaticType>(); +} + +void testNullCheckSubpatternMayFailToMatch(Object x) { + // Not exhaustive because the subpattern may fail to match + bool? y; + switch (x) { + case int _?: + // ^ + // [analyzer] STATIC_WARNING.UNNECESSARY_NULL_CHECK_PATTERN + // [cfe] The null-check pattern will have no effect because the matched type isn't nullable. + y = true; + } + y.expectStaticType>(); +} + +void testNullAssertSubpatternAlwaysMatches(Object? x) { + // Trivially exhaustive because the subpattern always matches + bool? y; + switch (x) { + case _!: + y = true; + } + y.expectStaticType>(); +} + +void testNullAssertSubpatternAlwaysMatchesObjectPattern(bool? x) { + // Trivially exhaustive because the subpattern always matches + bool? y; + switch (x) { + case bool()!: + y = true; + } + y.expectStaticType>(); +} + +void testNullAssertSubpatternMayFailToMatch(Object x) { + // Not exhaustive because the subpattern may fail to match + bool? y; + switch (x) { + case int _!: + // ^ + // [analyzer] STATIC_WARNING.UNNECESSARY_NULL_ASSERT_PATTERN + // [cfe] The null-assert pattern will have no effect because the matched type isn't nullable. + y = true; + } + y.expectStaticType>(); +} + +void testObjectSubtype(Object x) { + // Not exhaustive + bool? y; + switch (x) { + case int(): + y = true; + } + y.expectStaticType>(); +} + +void testObjectSupertype(Object x) { + // Trivially exhaustive + bool? y; + switch (x) { + case dynamic(): + y = true; + } + y.expectStaticType>(); +} + +void testObjectUnrelatedType(List x) { + // Not exhaustive + bool? y; + switch (x) { + case List(): + y = true; + } + y.expectStaticType>(); +} + +void testObjectSubpatternAlwaysMatches(Object x) { + // Trivially exhaustive because the hashCode always matches + bool? y; + switch (x) { + case Object(hashCode: _): + y = true; + } + y.expectStaticType>(); +} + +void testObjectSubpatternMayFailToMatch(Object x) { + // Not exhaustive because the hashCode may fail to match + bool? y; + switch (x) { + case Object(hashCode: == 0): + y = true; + } + y.expectStaticType>(); +} + +void testObjectTwoSubpatternsBothMatch(Object x) { + // Trivially exhaustive because both subpatterns always match + bool? y; + switch (x) { + case Object(hashCode: _, runtimeType: _): + y = true; + } + y.expectStaticType>(); +} + +void testObjectTwoSubpatternsFirstMatches(Object x) { + // Not exhaustive because the runtimeType may not match + bool? y; + switch (x) { + case Object(hashCode: _, runtimeType: == int): + y = true; + } + y.expectStaticType>(); +} + +void testObjectTwoSubpatternsSecondMatches(Object x) { + // Not exhaustive because the hashCode may not match + bool? y; + switch (x) { + case Object(hashCode: == 0, runtimeType: _): + y = true; + } + y.expectStaticType>(); +} + +void testObjectTwoSubpatternsNeitherMatches(Object x) { + // Not exhaustive because neither subpattern always matches + bool? y; + switch (x) { + case Object(hashCode: == 0, runtimeType: == int): + y = true; + } + y.expectStaticType>(); +} + +void testRecordSubtype(Object x) { + // Not exhaustive + bool? y; + switch (x) { + case (_, _): + y = true; + } + y.expectStaticType>(); +} + +void testRecordMatchingType((Object, Object) x) { + // Trivially exhaustive + bool? y; + switch (x) { + case (_, _): + y = true; + } + y.expectStaticType>(); +} + +void testRecordUnrelatedType(List x) { + // Not exhaustive + bool? y; + switch (x) { + case (_, _): + y = true; + } + y.expectStaticType>(); +} + +void testRecordSubpatternAlwaysMatches((Object,) x) { + // Trivially exhaustive because the subpattern always matches + bool? y; + switch (x) { + case (_,): + y = true; + } + y.expectStaticType>(); +} + +void testRecordSubpatternMayFailToMatch((Object,) x) { + // Not exhaustive because the hashCode may fail to match + bool? y; + switch (x) { + case (int _,): + y = true; + } + y.expectStaticType>(); +} + +void testRecordTwoSubpatternsBothMatch((Object, Object) x) { + // Trivially exhaustive because both subpatterns always match + bool? y; + switch (x) { + case (_, _): + y = true; + } + y.expectStaticType>(); +} + +void testRecordTwoSubpatternsFirstMatches((Object, Object) x) { + // Not exhaustive because the second subpattern may not match + bool? y; + switch (x) { + case (_, int _): + y = true; + } + y.expectStaticType>(); +} + +void testRecordTwoSubpatternsSecondMatches((Object, Object) x) { + // Not exhaustive because the first subpattern may not match + bool? y; + switch (x) { + case (int _, _): + y = true; + } + y.expectStaticType>(); +} + +void testRecordTwoSubpatternsNeitherMatches((Object, Object) x) { + // Not exhaustive because neither subpattern always matches + bool? y; + switch (x) { + case (int _, int _): + y = true; + } + y.expectStaticType>(); +} + +void testVariableSubtype(Object x) { + // Not exhaustive because Object !<: int + bool? y; + switch (x) { + case int v: + y = true; + } + y.expectStaticType>(); +} + +void testVariableSupertype(Object x) { + // Trivially exhaustive because Object <: Object? + bool? y; + switch (x) { + case Object? v: + y = true; + } + y.expectStaticType>(); +} + +void testVariableUnrelatedType(List x) { + // Not exhaustive because List !<: List + bool? y; + switch (x) { + case List v: + y = true; + } + y.expectStaticType>(); +} + +void testVariableUntyped(Object x) { + // Trivially exhaustive because an untyped variable always matches + bool? y; + switch (x) { + case var v: + y = true; + } + y.expectStaticType>(); +} + +void testWildcardSubtype(Object x) { + // Not exhaustive because Object !<: int + bool? y; + switch (x) { + case int _: + y = true; + } + y.expectStaticType>(); +} + +void testWildcardSupertype(Object x) { + // Trivially exhaustive because Object <: Object? + bool? y; + switch (x) { + case Object? _: + y = true; + } + y.expectStaticType>(); +} + +void testWildcardUnrelatedType(List x) { + // Not exhaustive because List !<: List + bool? y; + switch (x) { + case List _: + y = true; + } + y.expectStaticType>(); +} + +void testWildcardUntyped(Object x) { + // Trivially exhaustive because an untyped wildcard always matches + bool? y; + switch (x) { + case _: + y = true; + } + y.expectStaticType>(); +} + +void testRelationalNotEqualsNullWithNonNullableScrutinee(Object x) { + // TODO(paulberry): this should be trivially exhaustive + bool? y; + switch (x) { + case != null: + y = true; + } + y.expectStaticType>(); +} + +void testRelationalNotEqualsNullWithNullableScrutinee(Object? x) { + // Not exhaustive + bool? y; + switch (x) { + case != null: + y = true; + } + y.expectStaticType>(); +} + +void testRelationalEqualsNullWithNullScrutinee(Null x) { + // Trivially exhaustive + bool? y; + switch (x) { +//^^^^^^ +// [analyzer] COMPILE_TIME_ERROR.NON_EXHAUSTIVE_SWITCH_STATEMENT + // ^ + // [cfe] The type 'Null' is not exhaustively matched by the switch cases since it doesn't match 'null'. + case == null: + y = true; + } + y.expectStaticType>(); +} + +void testRelationalEqualsNullWithOtherScrutinee(Object x) { + // Not exhaustive + bool? y; + switch (x) { + case == null: + y = true; + } + y.expectStaticType>(); +} + +main() {}