[go_router]refactor runtime check to assert (#1362)
diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md
index b7e3c52..f1481a9 100644
--- a/packages/go_router/CHANGELOG.md
+++ b/packages/go_router/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 3.0.7
+
+- Refactors runtime checks to assertions.
+
## 3.0.6
- Exports inherited_go_router.dart file.
diff --git a/packages/go_router/lib/src/go_route.dart b/packages/go_router/lib/src/go_route.dart
index dbe5882..a50831a 100644
--- a/packages/go_router/lib/src/go_route.dart
+++ b/packages/go_router/lib/src/go_route.dart
@@ -21,51 +21,38 @@
this.builder = _invalidBuilder,
this.routes = const <GoRoute>[],
this.redirect = _noRedirection,
- }) {
- if (path.isEmpty) {
- throw Exception('GoRoute path cannot be empty');
- }
-
- if (name != null && name!.isEmpty) {
- throw Exception('GoRoute name cannot be empty');
- }
-
- if (pageBuilder == null &&
- builder == _invalidBuilder &&
- redirect == _noRedirection) {
- throw Exception(
- 'GoRoute builder parameter not set\n'
- 'See gorouter.dev/redirection#considerations for details',
- );
- }
-
+ }) : assert(path.isNotEmpty, 'GoRoute path cannot be empty'),
+ assert(name == null || name.isNotEmpty, 'GoRoute name cannot be empty'),
+ assert(
+ pageBuilder != null ||
+ builder != _invalidBuilder ||
+ redirect != _noRedirection,
+ 'GoRoute builder parameter not set\nSee gorouter.dev/redirection#considerations for details') {
// cache the path regexp and parameters
_pathRE = patternToRegExp(path, _pathParams);
- // check path params
- final Map<String, List<String>> groupedParams =
- _pathParams.groupListsBy<String>((String p) => p);
- final Map<String, List<String>> dupParams =
- Map<String, List<String>>.fromEntries(
- groupedParams.entries
- .where((MapEntry<String, List<String>> e) => e.value.length > 1),
- );
- if (dupParams.isNotEmpty) {
- throw Exception(
- 'duplicate path params: ${dupParams.keys.join(', ')}',
+ assert(() {
+ // check path params
+ final Map<String, List<String>> groupedParams =
+ _pathParams.groupListsBy<String>((String p) => p);
+ final Map<String, List<String>> dupParams =
+ Map<String, List<String>>.fromEntries(
+ groupedParams.entries
+ .where((MapEntry<String, List<String>> e) => e.value.length > 1),
);
- }
+ assert(dupParams.isEmpty,
+ 'duplicate path params: ${dupParams.keys.join(', ')}');
- // check sub-routes
- for (final GoRoute route in routes) {
- // check paths
- if (route.path != '/' &&
- (route.path.startsWith('/') || route.path.endsWith('/'))) {
- throw Exception(
- 'sub-route path may not start or end with /: ${route.path}',
- );
+ // check sub-routes
+ for (final GoRoute route in routes) {
+ // check paths
+ assert(
+ route.path == '/' ||
+ (!route.path.startsWith('/') && !route.path.endsWith('/')),
+ 'sub-route path may not start or end with /: ${route.path}');
}
- }
+ return true;
+ }());
}
final List<String> _pathParams = <String>[];
diff --git a/packages/go_router/lib/src/go_route_match.dart b/packages/go_router/lib/src/go_route_match.dart
index e489043..345aa5d 100644
--- a/packages/go_router/lib/src/go_route_match.dart
+++ b/packages/go_router/lib/src/go_route_match.dart
@@ -25,14 +25,14 @@
}) : assert(subloc.startsWith('/')),
assert(Uri.parse(subloc).queryParameters.isEmpty),
assert(fullpath.startsWith('/')),
- assert(Uri.parse(fullpath).queryParameters.isEmpty) {
- if (kDebugMode) {
- for (final MapEntry<String, String> p in encodedParams.entries) {
- assert(p.value == Uri.encodeComponent(Uri.decodeComponent(p.value)),
- 'encodedParams[${p.key}] is not encoded properly: "${p.value}"');
- }
- }
- }
+ assert(Uri.parse(fullpath).queryParameters.isEmpty),
+ assert(() {
+ for (final MapEntry<String, String> p in encodedParams.entries) {
+ assert(p.value == Uri.encodeComponent(Uri.decodeComponent(p.value)),
+ 'encodedParams[${p.key}] is not encoded properly: "${p.value}"');
+ }
+ return true;
+ }());
// ignore: public_member_api_docs
factory GoRouteMatch.matchNamed({
@@ -45,22 +45,21 @@
}) {
assert(route.name != null);
assert(route.name!.toLowerCase() == name.toLowerCase());
-
- // check that we have all the params we need
- final List<String> paramNames = <String>[];
- patternToRegExp(fullpath, paramNames);
- for (final String paramName in paramNames) {
- if (!params.containsKey(paramName)) {
- throw Exception('missing param "$paramName" for $fullpath');
+ assert(() {
+ // check that we have all the params we need
+ final List<String> paramNames = <String>[];
+ patternToRegExp(fullpath, paramNames);
+ for (final String paramName in paramNames) {
+ assert(params.containsKey(paramName),
+ 'missing param "$paramName" for $fullpath');
}
- }
- // check that we have don't have extra params
- for (final String key in params.keys) {
- if (!paramNames.contains(key)) {
- throw Exception('unknown param "$key" for $fullpath');
+ // check that we have don't have extra params
+ for (final String key in params.keys) {
+ assert(paramNames.contains(key), 'unknown param "$key" for $fullpath');
}
- }
+ return true;
+ }());
final Map<String, String> encodedParams = <String, String>{
for (final MapEntry<String, String> param in params.entries)
diff --git a/packages/go_router/lib/src/go_router_delegate.dart b/packages/go_router/lib/src/go_router_delegate.dart
index 791fb77..a0cdb59 100644
--- a/packages/go_router/lib/src/go_router_delegate.dart
+++ b/packages/go_router/lib/src/go_router_delegate.dart
@@ -38,14 +38,14 @@
required this.debugLogDiagnostics,
required this.routerNeglect,
this.restorationScopeId,
- }) {
- // check top-level route paths are valid
- for (final GoRoute route in routes) {
- if (!route.path.startsWith('/')) {
- throw Exception('top-level path must start with "/": ${route.path}');
- }
- }
-
+ }) : assert(() {
+ // check top-level route paths are valid
+ for (final GoRoute route in routes) {
+ assert(route.path.startsWith('/'),
+ 'top-level path must start with "/": ${route.path}');
+ }
+ return true;
+ }()) {
// cache the set of named routes for fast lookup
_cacheNamedRoutes(routes, '', _namedMatches);
@@ -110,10 +110,8 @@
if (route.name != null) {
final String name = route.name!.toLowerCase();
- if (namedFullpaths.containsKey(name)) {
- throw Exception('duplication fullpaths for name "$name":'
- '${namedFullpaths[name]!.fullpath}, $fullpath');
- }
+ assert(!namedFullpaths.containsKey(name),
+ 'duplication fullpaths for name "$name":${namedFullpaths[name]!.fullpath}, $fullpath');
// we only have a partial match until we have a location;
// we're really only caching the route and fullpath at this point
@@ -154,12 +152,9 @@
params: params,
queryParams: queryParams,
);
- if (match == null) {
- throw Exception('unknown route name: $name');
- }
-
- assert(identical(match.queryParams, queryParams));
- return _addQueryParams(match.subloc, queryParams);
+ assert(match != null, 'unknown route name: $name');
+ assert(identical(match!.queryParams, queryParams));
+ return _addQueryParams(match!.subloc, queryParams);
}
/// Navigate to the given location.
@@ -179,12 +174,8 @@
/// Pop the top page off the GoRouter's page stack.
void pop() {
_matches.remove(_matches.last);
- if (_matches.isEmpty) {
- throw Exception(
- 'have popped the last page off of the stack; '
- 'there are no pages left to show',
- );
- }
+ assert(_matches.isNotEmpty,
+ 'have popped the last page off of the stack; there are no pages left to show');
notifyListeners();
}
@@ -301,29 +292,28 @@
return false;
}
- if (Uri.tryParse(redir) == null) {
- throw Exception('invalid redirect: $redir');
- }
+ assert(Uri.tryParse(redir) != null, 'invalid redirect: $redir');
- if (redirects.contains(redir)) {
- redirects.add(redir);
- final String msg =
- 'redirect loop detected: ${redirects.join(' => ')}';
- throw Exception(msg);
- }
+ assert(
+ !redirects.contains(redir),
+ 'redirect loop detected: ${<String>[
+ ...redirects,
+ redir
+ ].join(' => ')}');
+ assert(
+ redirects.length < redirectLimit,
+ 'too many redirects: ${<String>[
+ ...redirects,
+ redir
+ ].join(' => ')}');
redirects.add(redir);
- if (redirects.length - 1 > redirectLimit) {
- final String msg = 'too many redirects: ${redirects.join(' => ')}';
- throw Exception(msg);
- }
-
log.info('redirecting to $redir');
return true;
}
// keep looping till we're done redirecting
- for (;;) {
+ while (true) {
final String loc = redirects.last;
// check for top-level redirect
@@ -449,23 +439,20 @@
extra: extra,
).toList();
- if (matchStacks.isEmpty) {
- throw Exception('no routes for location: $location');
- }
+ assert(matchStacks.isNotEmpty, 'no routes for location: $location');
+ assert(() {
+ if (matchStacks.length > 1) {
+ final StringBuffer sb = StringBuffer()
+ ..writeln('too many routes for location: $location');
- if (matchStacks.length > 1) {
- final StringBuffer sb = StringBuffer()
- ..writeln('too many routes for location: $location');
+ for (final List<GoRouteMatch> stack in matchStacks) {
+ sb.writeln(
+ '\t${stack.map((GoRouteMatch m) => m.route.path).join(' => ')}');
+ }
- for (final List<GoRouteMatch> stack in matchStacks) {
- sb.writeln(
- '\t${stack.map((GoRouteMatch m) => m.route.path).join(' => ')}');
+ assert(false, sb.toString());
}
- throw Exception(sb.toString());
- }
-
- if (kDebugMode) {
assert(matchStacks.length == 1);
final GoRouteMatch match = matchStacks.first.last;
final String loc1 = _addQueryParams(match.subloc, match.queryParams);
@@ -475,7 +462,8 @@
// NOTE: match the lower case, since subloc is canonicalized to match the
// path case whereas the location can be any case
assert(loc1.toLowerCase() == loc2.toLowerCase(), '$loc1 != $loc2');
- }
+ return true;
+ }());
return matchStacks.first;
}
diff --git a/packages/go_router/pubspec.yaml b/packages/go_router/pubspec.yaml
index 038fa10..48a8987 100644
--- a/packages/go_router/pubspec.yaml
+++ b/packages/go_router/pubspec.yaml
@@ -1,7 +1,7 @@
name: go_router
description: A declarative router for Flutter based on Navigation 2 supporting
deep linking, data-driven routes and more
-version: 3.0.6
+version: 3.0.7
repository: https://github.com/flutter/packages/tree/main/packages/go_router
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+go_router%22
diff --git a/packages/go_router/test/go_route_test.dart b/packages/go_router/test/go_route_test.dart
index 63b2912..c20d224 100644
--- a/packages/go_router/test/go_route_test.dart
+++ b/packages/go_router/test/go_route_test.dart
@@ -7,6 +7,10 @@
void main() {
test('throws when a builder is not set', () {
- expect(() => GoRoute(path: '/'), throwsException);
+ expect(() => GoRoute(path: '/'), throwsA(isAssertionError));
+ });
+
+ test('throws when a path is empty', () {
+ expect(() => GoRoute(path: ''), throwsA(isAssertionError));
});
}
diff --git a/packages/go_router/test/go_router_delegate_test.dart b/packages/go_router/test/go_router_delegate_test.dart
index d577856..a715332 100644
--- a/packages/go_router/test/go_router_delegate_test.dart
+++ b/packages/go_router/test/go_router_delegate_test.dart
@@ -45,7 +45,7 @@
() => delegate
..pop()
..pop(),
- throwsException,
+ throwsA(isAssertionError),
);
});
});
diff --git a/packages/go_router/test/go_router_test.dart b/packages/go_router/test/go_router_test.dart
index 718aa6e..bc0804e 100644
--- a/packages/go_router/test/go_router_test.dart
+++ b/packages/go_router/test/go_router_test.dart
@@ -56,7 +56,7 @@
test('empty path', () {
expect(() {
GoRoute(path: '');
- }, throwsException);
+ }, throwsA(isAssertionError));
});
test('leading / on sub-route', () {
@@ -71,7 +71,7 @@
),
],
);
- }, throwsException);
+ }, throwsA(isAssertionError));
});
test('trailing / on sub-route', () {
@@ -86,7 +86,7 @@
),
],
);
- }, throwsException);
+ }, throwsA(isAssertionError));
});
test('lack of leading / on top-level route', () {
@@ -95,7 +95,7 @@
GoRoute(path: 'foo', builder: _dummy),
];
_router(routes);
- }, throwsException);
+ }, throwsA(isAssertionError));
});
test('match no routes', () {
@@ -460,13 +460,13 @@
expect(() {
_router(routes);
- }, throwsException);
+ }, throwsA(isAssertionError));
});
test('empty name', () {
expect(() {
GoRoute(name: '', path: '/');
- }, throwsException);
+ }, throwsA(isAssertionError));
});
test('match no routes', () {
@@ -476,7 +476,7 @@
];
final GoRouter router = _router(routes);
router.goNamed('work');
- }, throwsException);
+ }, throwsA(isAssertionError));
});
test('match 2nd top level route', () {
@@ -612,7 +612,7 @@
expect(() {
final GoRouter router = _router(routes);
router.goNamed('person', params: <String, String>{'fid': 'f2'});
- }, throwsException);
+ }, throwsA(isAssertionError));
});
test('match case insensitive w/ params', () {
@@ -661,7 +661,7 @@
expect(() {
final GoRouter router = _router(routes);
router.goNamed('family');
- }, throwsException);
+ }, throwsA(isAssertionError));
});
test('too many params', () {
@@ -677,7 +677,7 @@
final GoRouter router = _router(routes);
router.goNamed('family',
params: <String, String>{'fid': 'f2', 'pid': 'p1'});
- }, throwsException);
+ }, throwsA(isAssertionError));
});
test('sparsely named routes', () {
