diff --git a/packages/go_router_builder/CHANGELOG.md b/packages/go_router_builder/CHANGELOG.md index d200fcc72f07..85c80a6e140c 100644 --- a/packages/go_router_builder/CHANGELOG.md +++ b/packages/go_router_builder/CHANGELOG.md @@ -1,3 +1,15 @@ +## 4.5.0 + +- Detects routes that resolve to the same URL pattern. Routes are compared by + the whole URL each one resolves to, so a collision is caught wherever the two + routes sit in the route tree, including across shell routes and + `StatefulShellRoute` branches, between relative routes, and between separate + annotations in one library. Paths that differ only in a parameter name, or + only in casing where the earlier route sets `caseSensitive: false`, count as + the same pattern. These are reported as build warnings by default. The new + `duplicate_route_paths` builder option raises them to build errors with + `error`, or silences them with `ignore`. + ## 4.4.0 - Adds `hasOverriddenOnExit` parameter to `GoRouteData.$route` and `RelativeGoRouteData.$route` helper methods for type-safe routes. When set to `true`, enables custom `onExit` callback invocation from route data classes extending `GoRouteData` or `RelativeGoRouteData` when the route is removed from the navigation stack. diff --git a/packages/go_router_builder/README.md b/packages/go_router_builder/README.md index 99d298a489d4..23aeeb272f94 100644 --- a/packages/go_router_builder/README.md +++ b/packages/go_router_builder/README.md @@ -42,6 +42,49 @@ dart run build_runner build Read more about using [`build_runner` on pub.dev](https://pub.dev/packages/build_runner). +### Builder options + +#### `duplicate_route_paths` + +When two routes resolve to the same URL, `go_router` matches the first and the +second becomes unreachable. Navigating to the second one's `location` shows the +first one's page. The builder warns about this at build time. + +Routes are compared by the URL they resolve to, not by the path they declare, so +depth in the route tree does not matter. A route at `section/detail` collides with +a route at `section` holding a child at `detail`. Parameter names are ignored, so +`product/:id` and `product/:productId` are the same URL, though a parameter's +regex constraint still counts, so `product/:id(\d+)` and `product/:id(\w+)` are +not. Casing is ignored when the earlier route sets `caseSensitive: false`, since +it then matches any casing. + +The whole library is compared, `part` files included. Shell routes and +`StatefulShellRoute` branches contribute nothing to the URLs beneath them. + +Use `build.yaml` to change what a duplicate does: + +```yaml +targets: + $default: + builders: + go_router_builder: + options: + duplicate_route_paths: error +``` + +Accepted values are `warning` (the default), `error`, and `ignore`. + +Warning is the default because some duplicates work. Matching backtracks, so +naming one route class twice at the same path, each declaration carrying +different children, is fine. Both resolve to the same class and every child stays +reachable, which makes it a way to group children by feature. The builder still +warns, because it cannot tell that from a mistake, so use `ignore` if you write +it deliberately. Their children are a different story: a child path repeated +across the two declarations is a genuine collision and is reported on its own. + +`error` applies to the whole package with no per-route exception, so it fails on +the deliberate grouping too. + ## Migration Guides - [Migrating to 4.0.0](https://flutter.dev/go/go-router-builder-v4-breaking-changes). @@ -505,4 +548,13 @@ Relative routing methods are not idempotent and will cause an error when the rel To run unit tests, run command `dart tool/run_tests.dart` from `packages/go_router_builder/`. +Each `.dart` file in `test_inputs/` is a test case, paired with a `.expect` file +holding either the generated output or the error message the builder must +produce. Two optional companion files tune a case: + +* `.dart.options` holds a JSON map of builder options, matching what + `build.yaml` would pass to the builder. +* `.dart.warnings` holds the warnings the builder must log, one per line. + An empty file asserts that the builder logs nothing. + To run tests in examples, run `flutter test` from `packages/go_router_builder/example`. diff --git a/packages/go_router_builder/example/lib/all_extension_types.dart b/packages/go_router_builder/example/lib/all_extension_types.dart index eb357df56d45..022eb55d91d2 100644 --- a/packages/go_router_builder/example/lib/all_extension_types.dart +++ b/packages/go_router_builder/example/lib/all_extension_types.dart @@ -26,9 +26,6 @@ part 'all_extension_types.g.dart'; ), TypedGoRoute(path: 'int-route/:requiredIntField'), TypedGoRoute(path: 'num-route/:requiredNumField'), - TypedGoRoute( - path: 'double-route/:requiredDoubleField', - ), TypedGoRoute(path: 'enum-route/:requiredEnumField'), TypedGoRoute( path: 'enhanced-enum-route/:requiredEnumField', diff --git a/packages/go_router_builder/example/lib/all_extension_types.g.dart b/packages/go_router_builder/example/lib/all_extension_types.g.dart index c8ef4167ef92..c188fc92df64 100644 --- a/packages/go_router_builder/example/lib/all_extension_types.g.dart +++ b/packages/go_router_builder/example/lib/all_extension_types.g.dart @@ -45,11 +45,6 @@ RouteBase get $allTypesBaseRoute => GoRouteData.$route( hasOverriddenOnExit: false, factory: $NumExtensionRoute._fromState, ), - GoRouteData.$route( - path: 'double-route/:requiredDoubleField', - hasOverriddenOnExit: false, - factory: $DoubleExtensionRoute._fromState, - ), GoRouteData.$route( path: 'enum-route/:requiredEnumField', hasOverriddenOnExit: false, diff --git a/packages/go_router_builder/example/lib/all_types.dart b/packages/go_router_builder/example/lib/all_types.dart index d984043e3865..7d32ed412e19 100644 --- a/packages/go_router_builder/example/lib/all_types.dart +++ b/packages/go_router_builder/example/lib/all_types.dart @@ -20,7 +20,6 @@ part 'all_types.g.dart'; TypedGoRoute(path: 'double-route/:requiredDoubleField'), TypedGoRoute(path: 'int-route/:requiredIntField'), TypedGoRoute(path: 'num-route/:requiredNumField'), - TypedGoRoute(path: 'double-route/:requiredDoubleField'), TypedGoRoute(path: 'enum-route/:requiredEnumField'), TypedGoRoute( path: 'enhanced-enum-route/:requiredEnumField', diff --git a/packages/go_router_builder/example/lib/all_types.g.dart b/packages/go_router_builder/example/lib/all_types.g.dart index d06b99c7488c..54f22074395d 100644 --- a/packages/go_router_builder/example/lib/all_types.g.dart +++ b/packages/go_router_builder/example/lib/all_types.g.dart @@ -45,11 +45,6 @@ RouteBase get $allTypesBaseRoute => GoRouteData.$route( hasOverriddenOnExit: false, factory: $NumRoute._fromState, ), - GoRouteData.$route( - path: 'double-route/:requiredDoubleField', - hasOverriddenOnExit: false, - factory: $DoubleRoute._fromState, - ), GoRouteData.$route( path: 'enum-route/:requiredEnumField', hasOverriddenOnExit: false, diff --git a/packages/go_router_builder/lib/go_router_builder.dart b/packages/go_router_builder/lib/go_router_builder.dart index 74d48b20c2a7..1c93c4006e27 100644 --- a/packages/go_router_builder/lib/go_router_builder.dart +++ b/packages/go_router_builder/lib/go_router_builder.dart @@ -15,11 +15,13 @@ library go_router_builder; import 'package:build/build.dart'; import 'package:source_gen/source_gen.dart'; +import 'src/duplicate_path_severity.dart'; import 'src/go_router_generator.dart'; /// Supports `package:build_runner` creation and configuration of /// `go_router`. /// /// Not meant to be invoked by hand-authored code. -Builder goRouterBuilder(BuilderOptions options) => - SharedPartBuilder(const [GoRouterGenerator()], 'go_router'); +Builder goRouterBuilder(BuilderOptions options) => SharedPartBuilder([ + GoRouterGenerator(duplicatePathSeverity: duplicatePathSeverityFromOptions(options)), +], 'go_router'); diff --git a/packages/go_router_builder/lib/src/duplicate_path_severity.dart b/packages/go_router_builder/lib/src/duplicate_path_severity.dart new file mode 100644 index 000000000000..4a071a215b13 --- /dev/null +++ b/packages/go_router_builder/lib/src/duplicate_path_severity.dart @@ -0,0 +1,56 @@ +// Copyright 2013 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:build/build.dart'; +import 'package:collection/collection.dart'; + +/// The `build.yaml` option that selects a [DuplicatePathSeverity]. +const String duplicateRoutePathsOption = 'duplicate_route_paths'; + +/// How the builder reports sibling routes that resolve to the same URL pattern. +enum DuplicatePathSeverity { + /// Duplicate paths are not reported at all. + ignore, + + /// Duplicate paths are reported as build warnings. + /// + /// Code is still generated for every route. This is the default, because a + /// duplicate path is legal at runtime and is not always dead code. + /// + /// `go_router` tries sibling routes in declaration order and takes the first + /// one that matches the whole URL. So when two different route classes share + /// a path, navigating to the second class's location lands on the first + /// class's page, which is almost always a mistake. But matching backtracks: + /// when a route matches only a prefix and none of its children complete the + /// URL, matching moves on to the next sibling. Declaring one route class + /// twice with different children is therefore sound, and is one way to group + /// children by feature area. Both shapes are reported, since the builder + /// cannot tell a deliberate grouping from an accidental duplicate. + warning, + + /// Duplicate paths fail the build. + error, +} + +/// Reads the [DuplicatePathSeverity] from `build.yaml` builder [options]. +/// +/// Defaults to [DuplicatePathSeverity.warning] when the option is absent. +DuplicatePathSeverity duplicatePathSeverityFromOptions(BuilderOptions options) { + final Object? value = options.config[duplicateRoutePathsOption]; + if (value == null) { + return DuplicatePathSeverity.warning; + } + final DuplicatePathSeverity? severity = DuplicatePathSeverity.values.firstWhereOrNull( + (DuplicatePathSeverity severity) => severity.name == value, + ); + if (severity == null) { + throw ArgumentError.value( + value, + duplicateRoutePathsOption, + 'Must be one of ' + '${DuplicatePathSeverity.values.map((DuplicatePathSeverity e) => e.name).join(', ')}', + ); + } + return severity; +} diff --git a/packages/go_router_builder/lib/src/go_router_generator.dart b/packages/go_router_builder/lib/src/go_router_generator.dart index 9f0f8414b2c3..4d98b544b95a 100644 --- a/packages/go_router_builder/lib/src/go_router_generator.dart +++ b/packages/go_router_builder/lib/src/go_router_generator.dart @@ -9,6 +9,7 @@ import 'package:analyzer/dart/element/type.dart'; import 'package:build/build.dart'; import 'package:source_gen/source_gen.dart'; +import 'duplicate_path_severity.dart'; import 'route_config.dart'; import 'type_helpers.dart'; @@ -25,7 +26,10 @@ const Map _annotations = { /// A [Generator] for classes annotated with a typed go route annotation. class GoRouterGenerator extends Generator { /// Creates a new instance of [GoRouterGenerator]. - const GoRouterGenerator(); + const GoRouterGenerator({this.duplicatePathSeverity = DuplicatePathSeverity.warning}); + + /// How sibling routes that resolve to the same URL pattern are reported. + final DuplicatePathSeverity duplicatePathSeverity; TypeChecker get _typeChecker => TypeChecker.any( _annotations.keys.map((String annotation) => TypeChecker.fromUrl('$_routeDataUrl#$annotation')), @@ -57,17 +61,26 @@ ${getters.map((String e) => "$e,").join('\n')} /// This public method is for testing purposes and should not be called /// directly. void generateForAnnotation(LibraryReader library, Set values, Set getters) { + // Every annotation in the library contributes a top-level route to the + // generated `$appRoutes`, so they all have to be built before their paths + // can be compared against each other. + final configs = []; for (final AnnotatedElement annotatedElement in library.annotatedWith(_typeChecker)) { - final InfoIterable generatedValue = _generateForAnnotatedElement( - annotatedElement.element, - annotatedElement.annotation, + configs.add( + _configForAnnotatedElement(annotatedElement.element, annotatedElement.annotation), ); + } + + reportDuplicateRoutePaths(configs, duplicatePathSeverity); + + for (final config in configs) { + final InfoIterable generatedValue = config.generateMembers(); getters.add(generatedValue.routeGetterName); values.addAll(generatedValue.members); } } - InfoIterable _generateForAnnotatedElement(Element element, ConstantReader annotation) { + RouteBaseConfig _configForAnnotatedElement(Element element, ConstantReader annotation) { final String typedAnnotation = withoutNullability( annotation.objectValue.type!.getDisplayString(), ); @@ -89,6 +102,6 @@ ${getters.map((String e) => "$e,").join('\n')} ); } - return RouteBaseConfig.fromAnnotation(annotation, element).generateMembers(); + return RouteBaseConfig.fromAnnotation(annotation, element); } } diff --git a/packages/go_router_builder/lib/src/path_utils.dart b/packages/go_router_builder/lib/src/path_utils.dart index e495ce78e8ee..607219ee7de2 100644 --- a/packages/go_router_builder/lib/src/path_utils.dart +++ b/packages/go_router_builder/lib/src/path_utils.dart @@ -18,6 +18,21 @@ Set pathParametersFromPattern(String pattern) => { for (final RegExpMatch match in _parameterRegExp.allMatches(pattern)) match[1]!, }; +/// Replaces the parameter names in a [pattern] with a placeholder, so that +/// patterns differing only in those names compare as equal. +/// +/// A parameter's regex constraint is kept, since it changes which URLs the +/// pattern matches. +/// +/// For example: +/// +/// ```dart +/// normalizePathParameters('item/:id'); // 'item/:_' +/// normalizePathParameters(r'item/:id(\d+)'); // r'item/:_(\d+)' +/// ``` +String normalizePathParameters(String pattern) => + pattern.replaceAllMapped(_parameterRegExp, (Match match) => ':_${match[2] ?? ''}'); + /// Reconstructs the full path from a [pattern] and path parameters. /// /// For example: diff --git a/packages/go_router_builder/lib/src/route_config.dart b/packages/go_router_builder/lib/src/route_config.dart index f5807a0e36ea..7a307bdd80b1 100644 --- a/packages/go_router_builder/lib/src/route_config.dart +++ b/packages/go_router_builder/lib/src/route_config.dart @@ -9,12 +9,14 @@ import 'package:analyzer/dart/constant/value.dart'; import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/dart/element/nullability_suffix.dart'; import 'package:analyzer/dart/element/type.dart'; +import 'package:build/build.dart'; import 'package:collection/collection.dart'; import 'package:meta/meta.dart'; import 'package:path/path.dart' as p; import 'package:source_gen/source_gen.dart'; import 'package:source_helper/source_helper.dart'; +import 'duplicate_path_severity.dart'; import 'path_utils.dart'; import 'type_helpers.dart'; @@ -188,6 +190,37 @@ class StatefulShellBranchConfig extends RouteBaseConfig { mixin _GoRouteMixin on RouteBaseConfig { String get _basePathForLocation; + /// The path this route contributes to the URL, without any parent path. + /// + /// This is the path exactly as written in the annotation. + String get path; + + /// Whether this route only matches a URL that matches its path's casing. + bool get caseSensitive; + + /// The path this route matches, joined with the paths of its ancestors. + /// + /// Shell routes and branches contribute nothing, since they own no path, so + /// this is the URL pattern the route resolves to at runtime. Two routes with + /// the same pattern compete for the same URL no matter where they sit in the + /// tree, which is what [reportDuplicateRoutePaths] compares. + /// + /// [_basePathForLocation] cannot serve here, because a relative route + /// deliberately reports only its own path. + String get _joinedPath { + final pathSegments = []; + + RouteBaseConfig? config = this; + while (config != null) { + if (config case _GoRouteMixin(:final String path)) { + pathSegments.add(path); + } + config = config.parent; + } + + return p.url.joinAll(pathSegments.reversed); + } + late final Set _pathParams = pathParametersFromPattern(_basePathForLocation); // construct path bits using parent bits @@ -400,12 +433,14 @@ class GoRouteConfig extends RouteBaseConfig with _GoRouteMixin { }) : super._(); /// The path of the GoRoute to be created by this configuration. + @override final String path; /// The name of the GoRoute to be created by this configuration. final String? name; /// The case sensitivity of the GoRoute to be created by this configuration. + @override final bool caseSensitive; /// Whether to enable the onExit callback for this route. @@ -418,23 +453,8 @@ class GoRouteConfig extends RouteBaseConfig with _GoRouteMixin { /// The parent navigator key. final String? parentNavigatorKey; - String get _rawJoinedPath { - final pathSegments = []; - - RouteBaseConfig? config = this; - while (config != null) { - if (config - case GoRouteConfig(:final String path) || RelativeGoRouteConfig(:final String path)) { - pathSegments.add(path); - } - config = config.parent; - } - - return p.url.joinAll(pathSegments.reversed); - } - @override - String get _basePathForLocation => _rawJoinedPath; + String get _basePathForLocation => _joinedPath; @override String get _mixinDefinition { @@ -501,9 +521,11 @@ class RelativeGoRouteConfig extends RouteBaseConfig with _GoRouteMixin { }) : super._(); /// The path of the GoRoute to be created by this configuration. + @override final String path; /// The case sensitivity of the GoRoute to be created by this configuration. + @override final bool caseSensitive; /// Whether to enable the onExit callback for this route. @@ -970,3 +992,89 @@ bool $iterablesEqualHelperName(Iterable? iterable1, Iterable? iterable2 if (iterator1.current != iterator2.current) return false; } }'''; + +/// Reports routes that resolve to the same URL pattern, with the severity +/// selected by the `duplicate_route_paths` builder option. +/// +/// [roots] holds every top-level route config in a library, which is the widest +/// scope the builder can see, since it generates one library at a time. +/// +/// Routes are compared by the whole URL pattern they resolve to, their +/// [_GoRouteMixin._joinedPath], rather than by the path each one declares. Two +/// routes at different depths can still land on one URL: a route nested as `a` +/// then `b` resolves to the same place as a route declared at `a/b`. Comparing +/// declared paths level by level misses that, and misses a collision between the +/// children of two declarations that themselves share a path. +/// +/// `go_router` tries routes in declaration order and takes the first whose +/// pattern matches the whole URL, so of two routes sharing a pattern the later +/// can never be the match for it. +/// +/// Parameter names are normalized, since `:id` and `:userId` match the same URL +/// segments. A route that is not case sensitive also shadows a later route whose +/// pattern differs from it only in casing. +void reportDuplicateRoutePaths(List roots, DuplicatePathSeverity severity) { + if (severity == DuplicatePathSeverity.ignore) { + return; + } + + final routes = <_GoRouteMixin>[]; + _collectRoutes(roots, routes); + + // Keyed by exact pattern, then again by folded pattern for the routes that + // match any casing. Both keep their first entry, which is the route + // `go_router` matches, because the walk above is in declaration order. + final seen = {}; + final seenIgnoringCase = {}; + for (final route in routes) { + final String pattern = normalizePathParameters(route._joinedPath); + final String folded = pattern.toLowerCase(); + + final _GoRouteMixin? existing = seen[pattern] ?? seenIgnoringCase[folded]; + if (existing != null) { + final String message = _duplicatePathMessage(existing, route); + if (severity == DuplicatePathSeverity.error) { + throw InvalidGenerationSourceError(message, element: route.routeDataClass); + } + log.warning(message); + } + + seen.putIfAbsent(pattern, () => route); + if (!route.caseSensitive) { + seenIgnoringCase.putIfAbsent(folded, () => route); + } + } +} + +/// Describes the conflict between two routes that resolve to the same URL. +/// +/// One route class declared twice at one URL gets its own wording, since naming +/// that class on both sides of an "and" reads as though two classes were +/// involved. Any other pair names both sides, which stays clear even when the +/// class repeats, because the two patterns differ. +String _duplicatePathMessage(_GoRouteMixin existing, _GoRouteMixin route) { + final String existingPattern = existing._joinedPath; + final String pattern = route._joinedPath; + if (existing.routeDataClass == route.routeDataClass && existingPattern == pattern) { + return 'Duplicate route path detected: ${route._className} is declared more ' + 'than once at "$pattern".'; + } + return 'Duplicate route path detected: ' + '"$existingPattern" from ${existing._className} and ' + '"$pattern" from ${route._className} ' + 'both match the same URL pattern.'; +} + +/// Collects every route in the tree that resolves to a URL of its own, in +/// declaration order. +/// +/// Shell routes and branches own no path, so they contribute nothing themselves, +/// but the routes inside them do and are collected all the same. +void _collectRoutes(List children, List<_GoRouteMixin> result) { + for (final child in children) { + if (child is _GoRouteMixin) { + result.add(child); + } + _collectRoutes(child._children, result); + } +} diff --git a/packages/go_router_builder/pubspec.yaml b/packages/go_router_builder/pubspec.yaml index dc3c84e4bf06..86f2a5f500d0 100644 --- a/packages/go_router_builder/pubspec.yaml +++ b/packages/go_router_builder/pubspec.yaml @@ -2,7 +2,7 @@ name: go_router_builder description: >- A builder that supports generated strongly-typed route helpers for package:go_router -version: 4.4.0 +version: 4.5.0 repository: https://github.com/flutter/packages/tree/main/packages/go_router_builder issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+go_router_builder%22 @@ -29,6 +29,7 @@ dev_dependencies: sdk: flutter go_router: ^17.3.0 leak_tracker_flutter_testing: ">=3.0.0" + logging: ^1.2.0 package_config: ^2.1.1 pub_semver: ^2.1.5 test: ^1.20.0 diff --git a/packages/go_router_builder/test/duplicate_path_severity_test.dart b/packages/go_router_builder/test/duplicate_path_severity_test.dart new file mode 100644 index 000000000000..e18fff1df635 --- /dev/null +++ b/packages/go_router_builder/test/duplicate_path_severity_test.dart @@ -0,0 +1,53 @@ +// Copyright 2013 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:build/build.dart'; +import 'package:go_router_builder/src/duplicate_path_severity.dart'; +import 'package:test/test.dart'; + +void main() { + group('duplicatePathSeverityFromOptions', () { + test('It should warn when the option is absent', () { + expect(duplicatePathSeverityFromOptions(BuilderOptions.empty), DuplicatePathSeverity.warning); + }); + + test('It should read each accepted value', () { + for (final DuplicatePathSeverity severity in DuplicatePathSeverity.values) { + expect( + duplicatePathSeverityFromOptions( + BuilderOptions({duplicateRoutePathsOption: severity.name}), + ), + severity, + ); + } + }); + + test('It should reject an unrecognized value, naming the accepted ones', () { + expect( + () => duplicatePathSeverityFromOptions( + const BuilderOptions({duplicateRoutePathsOption: 'bogus'}), + ), + throwsA( + isA() + .having((ArgumentError e) => e.name, 'name', duplicateRoutePathsOption) + .having((ArgumentError e) => e.invalidValue, 'invalidValue', 'bogus') + .having( + (ArgumentError e) => e.message, + 'message', + allOf(contains('ignore'), contains('warning'), contains('error')), + ), + ), + ); + }); + + test('It should reject a value of the wrong type', () { + expect( + () => duplicatePathSeverityFromOptions( + const BuilderOptions({duplicateRoutePathsOption: true}), + ), + throwsA(isA()), + ); + }); + }); +} diff --git a/packages/go_router_builder/test/path_utils_test.dart b/packages/go_router_builder/test/path_utils_test.dart index ccdd256c53f3..0845b4747eb5 100644 --- a/packages/go_router_builder/test/path_utils_test.dart +++ b/packages/go_router_builder/test/path_utils_test.dart @@ -16,6 +16,43 @@ void main() { }); }); + group('normalizePathParameters', () { + test('It should leave a pattern without parameters alone', () { + expect(normalizePathParameters('/'), '/'); + expect(normalizePathParameters('/user/book'), '/user/book'); + }); + + test('It should make patterns differing only in parameter name equal', () { + expect(normalizePathParameters('/user/:id'), normalizePathParameters('/user/:userId')); + expect( + normalizePathParameters('/user/:id/book/:bookId'), + normalizePathParameters('/user/:a/book/:b'), + ); + }); + + test('It should keep patterns with different literal segments distinct', () { + expect(normalizePathParameters('/user/:id'), isNot(normalizePathParameters('/book/:id'))); + expect( + normalizePathParameters('/user/:id'), + isNot(normalizePathParameters('/user/:id/book')), + ); + }); + + test('It should keep a parameter constraint, which changes what matches', () { + expect(normalizePathParameters(r'/user/:id(\d+)'), r'/user/:_(\d+)'); + expect( + normalizePathParameters(r'/user/:id(\d+)'), + isNot(normalizePathParameters(r'/user/:id(\w+)')), + ); + // A colon inside a constraint is part of the constraint, not a second + // parameter, so these two must stay distinct. + expect( + normalizePathParameters('/user/:id(mon:tue)'), + isNot(normalizePathParameters('/user/:id(mon:wed)')), + ); + }); + }); + group('patternToPath', () { test('It should replace the path parameters with their values', () { expect(patternToPath('/', const {}), '/'); diff --git a/packages/go_router_builder/test_inputs/duplicate_path_across_annotations.dart b/packages/go_router_builder/test_inputs/duplicate_path_across_annotations.dart new file mode 100644 index 000000000000..f873e5086629 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_across_annotations.dart @@ -0,0 +1,14 @@ +// Copyright 2013 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:go_router/go_router.dart'; + +mixin $FirstHomeRoute {} +mixin $SecondHomeRoute {} + +@TypedGoRoute(path: '/home') +class FirstHomeRoute extends GoRouteData with $FirstHomeRoute {} + +@TypedGoRoute(path: '/home') +class SecondHomeRoute extends GoRouteData with $SecondHomeRoute {} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_across_annotations.dart.expect b/packages/go_router_builder/test_inputs/duplicate_path_across_annotations.dart.expect new file mode 100644 index 000000000000..1fd6d5f368a2 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_across_annotations.dart.expect @@ -0,0 +1 @@ +Duplicate route path detected: "/home" from FirstHomeRoute and "/home" from SecondHomeRoute both match the same URL pattern. diff --git a/packages/go_router_builder/test_inputs/duplicate_path_across_annotations.dart.options b/packages/go_router_builder/test_inputs/duplicate_path_across_annotations.dart.options new file mode 100644 index 000000000000..c7f07c4e62c0 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_across_annotations.dart.options @@ -0,0 +1,3 @@ +{ + "duplicate_route_paths": "error" +} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_across_shell_route.dart b/packages/go_router_builder/test_inputs/duplicate_path_across_shell_route.dart new file mode 100644 index 000000000000..021d13bbe9d7 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_across_shell_route.dart @@ -0,0 +1,23 @@ +// Copyright 2013 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// A shell route owns no path, so a route inside one competes with a route +// declared beside the shell. + +import 'package:go_router/go_router.dart'; + +mixin $InsideShellRoute {} +mixin $OutsideShellRoute {} + +@TypedShellRoute( + routes: >[TypedGoRoute(path: '/settings')], +) +class AppShellRouteData extends ShellRouteData { + const AppShellRouteData(); +} + +@TypedGoRoute(path: '/settings') +class OutsideShellRoute extends GoRouteData with $OutsideShellRoute {} + +class InsideShellRoute extends GoRouteData with $InsideShellRoute {} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_across_shell_route.dart.expect b/packages/go_router_builder/test_inputs/duplicate_path_across_shell_route.dart.expect new file mode 100644 index 000000000000..22a7373f7e42 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_across_shell_route.dart.expect @@ -0,0 +1 @@ +Duplicate route path detected: "/settings" from InsideShellRoute and "/settings" from OutsideShellRoute both match the same URL pattern. diff --git a/packages/go_router_builder/test_inputs/duplicate_path_across_shell_route.dart.options b/packages/go_router_builder/test_inputs/duplicate_path_across_shell_route.dart.options new file mode 100644 index 000000000000..c7f07c4e62c0 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_across_shell_route.dart.options @@ -0,0 +1,3 @@ +{ + "duplicate_route_paths": "error" +} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_case_insensitive.dart b/packages/go_router_builder/test_inputs/duplicate_path_case_insensitive.dart new file mode 100644 index 000000000000..b6ae3a81ac00 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_case_insensitive.dart @@ -0,0 +1,25 @@ +// Copyright 2013 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// A route that is not case sensitive matches any casing of its path, so it +// shadows a later route whose path differs from it only in case. + +import 'package:go_router/go_router.dart'; + +mixin $HomeRoute {} +mixin $LowerRoute {} +mixin $UpperRoute {} + +@TypedGoRoute( + path: '/home', + routes: >[ + TypedGoRoute(path: 'details', caseSensitive: false), + TypedGoRoute(path: 'Details', caseSensitive: false), + ], +) +class HomeRoute extends GoRouteData with $HomeRoute {} + +class LowerRoute extends GoRouteData with $LowerRoute {} + +class UpperRoute extends GoRouteData with $UpperRoute {} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_case_insensitive.dart.expect b/packages/go_router_builder/test_inputs/duplicate_path_case_insensitive.dart.expect new file mode 100644 index 000000000000..2acf593f8593 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_case_insensitive.dart.expect @@ -0,0 +1 @@ +Duplicate route path detected: "/home/details" from LowerRoute and "/home/Details" from UpperRoute both match the same URL pattern. diff --git a/packages/go_router_builder/test_inputs/duplicate_path_case_insensitive.dart.options b/packages/go_router_builder/test_inputs/duplicate_path_case_insensitive.dart.options new file mode 100644 index 000000000000..c7f07c4e62c0 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_case_insensitive.dart.options @@ -0,0 +1,3 @@ +{ + "duplicate_route_paths": "error" +} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_case_sensitive_distinct.dart b/packages/go_router_builder/test_inputs/duplicate_path_case_sensitive_distinct.dart new file mode 100644 index 000000000000..99ff3cf3ea3b --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_case_sensitive_distinct.dart @@ -0,0 +1,25 @@ +// Copyright 2013 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Case sensitive routes are the default, and two of them differing in case +// match different URLs, so they must not be reported. + +import 'package:go_router/go_router.dart'; + +mixin $HomeRoute {} +mixin $LowerRoute {} +mixin $UpperRoute {} + +@TypedGoRoute( + path: '/home', + routes: >[ + TypedGoRoute(path: 'details'), + TypedGoRoute(path: 'Details'), + ], +) +class HomeRoute extends GoRouteData with $HomeRoute {} + +class LowerRoute extends GoRouteData with $LowerRoute {} + +class UpperRoute extends GoRouteData with $UpperRoute {} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_case_sensitive_distinct.dart.expect b/packages/go_router_builder/test_inputs/duplicate_path_case_sensitive_distinct.dart.expect new file mode 100644 index 000000000000..91e0205172de --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_case_sensitive_distinct.dart.expect @@ -0,0 +1,77 @@ +RouteBase get $homeRoute => GoRouteData.$route( + path: '/home', + hasOverriddenOnExit: false, + factory: $HomeRoute._fromState, + routes: [ + GoRouteData.$route( + path: 'details', + hasOverriddenOnExit: false, + factory: $LowerRoute._fromState, + ), + GoRouteData.$route( + path: 'Details', + hasOverriddenOnExit: false, + factory: $UpperRoute._fromState, + ), + ], +); + +mixin $HomeRoute on GoRouteData { + static HomeRoute _fromState(GoRouterState state) => HomeRoute(); + + @override + String get location => GoRouteData.$location('/home'); + + @override + void go(BuildContext context) => context.go(location); + + @override + Future push(BuildContext context) => context.push(location); + + @override + void pushReplacement(BuildContext context) => + context.pushReplacement(location); + + @override + void replace(BuildContext context) => context.replace(location); +} + +mixin $LowerRoute on GoRouteData { + static LowerRoute _fromState(GoRouterState state) => LowerRoute(); + + @override + String get location => GoRouteData.$location('/home/details'); + + @override + void go(BuildContext context) => context.go(location); + + @override + Future push(BuildContext context) => context.push(location); + + @override + void pushReplacement(BuildContext context) => + context.pushReplacement(location); + + @override + void replace(BuildContext context) => context.replace(location); +} + +mixin $UpperRoute on GoRouteData { + static UpperRoute _fromState(GoRouterState state) => UpperRoute(); + + @override + String get location => GoRouteData.$location('/home/Details'); + + @override + void go(BuildContext context) => context.go(location); + + @override + Future push(BuildContext context) => context.push(location); + + @override + void pushReplacement(BuildContext context) => + context.pushReplacement(location); + + @override + void replace(BuildContext context) => context.replace(location); +} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_different_param_names.dart b/packages/go_router_builder/test_inputs/duplicate_path_different_param_names.dart new file mode 100644 index 000000000000..78588981c8d7 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_different_param_names.dart @@ -0,0 +1,28 @@ +// Copyright 2013 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:go_router/go_router.dart'; + +mixin $HomeRoute {} +mixin $ProductRoute {} +mixin $VariantRoute {} + +@TypedGoRoute( + path: '/home', + routes: >[ + TypedGoRoute(path: 'item/:productId'), + TypedGoRoute(path: 'item/:variantId'), + ], +) +class HomeRoute extends GoRouteData with $HomeRoute {} + +class ProductRoute extends GoRouteData with $ProductRoute { + const ProductRoute({required this.productId}); + final String productId; +} + +class VariantRoute extends GoRouteData with $VariantRoute { + const VariantRoute({required this.variantId}); + final String variantId; +} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_different_param_names.dart.expect b/packages/go_router_builder/test_inputs/duplicate_path_different_param_names.dart.expect new file mode 100644 index 000000000000..49d740c5d893 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_different_param_names.dart.expect @@ -0,0 +1 @@ +Duplicate route path detected: "/home/item/:productId" from ProductRoute and "/home/item/:variantId" from VariantRoute both match the same URL pattern. diff --git a/packages/go_router_builder/test_inputs/duplicate_path_different_param_names.dart.options b/packages/go_router_builder/test_inputs/duplicate_path_different_param_names.dart.options new file mode 100644 index 000000000000..c7f07c4e62c0 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_different_param_names.dart.options @@ -0,0 +1,3 @@ +{ + "duplicate_route_paths": "error" +} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_ignored.dart b/packages/go_router_builder/test_inputs/duplicate_path_ignored.dart new file mode 100644 index 000000000000..4585aaa954f6 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_ignored.dart @@ -0,0 +1,22 @@ +// Copyright 2013 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:go_router/go_router.dart'; + +mixin $HomeRoute {} +mixin $FirstRoute {} +mixin $SecondRoute {} + +@TypedGoRoute( + path: '/home', + routes: >[ + TypedGoRoute(path: 'details'), + TypedGoRoute(path: 'details'), + ], +) +class HomeRoute extends GoRouteData with $HomeRoute {} + +class FirstRoute extends GoRouteData with $FirstRoute {} + +class SecondRoute extends GoRouteData with $SecondRoute {} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_ignored.dart.expect b/packages/go_router_builder/test_inputs/duplicate_path_ignored.dart.expect new file mode 100644 index 000000000000..8cb238063845 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_ignored.dart.expect @@ -0,0 +1,77 @@ +RouteBase get $homeRoute => GoRouteData.$route( + path: '/home', + hasOverriddenOnExit: false, + factory: $HomeRoute._fromState, + routes: [ + GoRouteData.$route( + path: 'details', + hasOverriddenOnExit: false, + factory: $FirstRoute._fromState, + ), + GoRouteData.$route( + path: 'details', + hasOverriddenOnExit: false, + factory: $SecondRoute._fromState, + ), + ], +); + +mixin $HomeRoute on GoRouteData { + static HomeRoute _fromState(GoRouterState state) => HomeRoute(); + + @override + String get location => GoRouteData.$location('/home'); + + @override + void go(BuildContext context) => context.go(location); + + @override + Future push(BuildContext context) => context.push(location); + + @override + void pushReplacement(BuildContext context) => + context.pushReplacement(location); + + @override + void replace(BuildContext context) => context.replace(location); +} + +mixin $FirstRoute on GoRouteData { + static FirstRoute _fromState(GoRouterState state) => FirstRoute(); + + @override + String get location => GoRouteData.$location('/home/details'); + + @override + void go(BuildContext context) => context.go(location); + + @override + Future push(BuildContext context) => context.push(location); + + @override + void pushReplacement(BuildContext context) => + context.pushReplacement(location); + + @override + void replace(BuildContext context) => context.replace(location); +} + +mixin $SecondRoute on GoRouteData { + static SecondRoute _fromState(GoRouterState state) => SecondRoute(); + + @override + String get location => GoRouteData.$location('/home/details'); + + @override + void go(BuildContext context) => context.go(location); + + @override + Future push(BuildContext context) => context.push(location); + + @override + void pushReplacement(BuildContext context) => + context.pushReplacement(location); + + @override + void replace(BuildContext context) => context.replace(location); +} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_ignored.dart.options b/packages/go_router_builder/test_inputs/duplicate_path_ignored.dart.options new file mode 100644 index 000000000000..bbfbe49c98bf --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_ignored.dart.options @@ -0,0 +1,3 @@ +{ + "duplicate_route_paths": "ignore" +} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_ignored.dart.warnings b/packages/go_router_builder/test_inputs/duplicate_path_ignored.dart.warnings new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/packages/go_router_builder/test_inputs/duplicate_path_in_branches.dart b/packages/go_router_builder/test_inputs/duplicate_path_in_branches.dart new file mode 100644 index 000000000000..66194bc5b4e4 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_in_branches.dart @@ -0,0 +1,34 @@ +// Copyright 2013 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:go_router/go_router.dart'; + +mixin $IdeasRoute {} +mixin $FavoritesRoute {} + +@TypedStatefulShellRoute( + branches: >[ + TypedStatefulShellBranch( + routes: >[TypedGoRoute(path: '/ideas')], + ), + TypedStatefulShellBranch( + routes: >[TypedGoRoute(path: '/ideas')], + ), + ], +) +class AppShellRouteData extends StatefulShellRouteData { + const AppShellRouteData(); +} + +class BranchAData extends StatefulShellBranchData { + const BranchAData(); +} + +class BranchBData extends StatefulShellBranchData { + const BranchBData(); +} + +class IdeasRoute extends GoRouteData with $IdeasRoute {} + +class FavoritesRoute extends GoRouteData with $FavoritesRoute {} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_in_branches.dart.expect b/packages/go_router_builder/test_inputs/duplicate_path_in_branches.dart.expect new file mode 100644 index 000000000000..d6156c399931 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_in_branches.dart.expect @@ -0,0 +1 @@ +Duplicate route path detected: "/ideas" from IdeasRoute and "/ideas" from FavoritesRoute both match the same URL pattern. diff --git a/packages/go_router_builder/test_inputs/duplicate_path_in_branches.dart.options b/packages/go_router_builder/test_inputs/duplicate_path_in_branches.dart.options new file mode 100644 index 000000000000..c7f07c4e62c0 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_in_branches.dart.options @@ -0,0 +1,3 @@ +{ + "duplicate_route_paths": "error" +} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_multi_segment.dart b/packages/go_router_builder/test_inputs/duplicate_path_multi_segment.dart new file mode 100644 index 000000000000..00a20463330d --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_multi_segment.dart @@ -0,0 +1,31 @@ +// Copyright 2013 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// A multi-segment path resolves to the same URL as the equivalent nesting, even +// though the two routes sit at different depths. + +import 'package:go_router/go_router.dart'; + +mixin $HomeRoute {} +mixin $SectionRoute {} +mixin $NestedRoute {} +mixin $MultiSegmentRoute {} + +@TypedGoRoute( + path: '/home', + routes: >[ + TypedGoRoute( + path: 'section', + routes: >[TypedGoRoute(path: 'detail')], + ), + TypedGoRoute(path: 'section/detail'), + ], +) +class HomeRoute extends GoRouteData with $HomeRoute {} + +class SectionRoute extends GoRouteData with $SectionRoute {} + +class NestedRoute extends GoRouteData with $NestedRoute {} + +class MultiSegmentRoute extends GoRouteData with $MultiSegmentRoute {} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_multi_segment.dart.expect b/packages/go_router_builder/test_inputs/duplicate_path_multi_segment.dart.expect new file mode 100644 index 000000000000..4794803e4d5b --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_multi_segment.dart.expect @@ -0,0 +1 @@ +Duplicate route path detected: "/home/section/detail" from NestedRoute and "/home/section/detail" from MultiSegmentRoute both match the same URL pattern. diff --git a/packages/go_router_builder/test_inputs/duplicate_path_multi_segment.dart.options b/packages/go_router_builder/test_inputs/duplicate_path_multi_segment.dart.options new file mode 100644 index 000000000000..c7f07c4e62c0 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_multi_segment.dart.options @@ -0,0 +1,3 @@ +{ + "duplicate_route_paths": "error" +} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_nested_relative_routes.dart b/packages/go_router_builder/test_inputs/duplicate_path_nested_relative_routes.dart new file mode 100644 index 000000000000..4fb4d19695b3 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_nested_relative_routes.dart @@ -0,0 +1,31 @@ +// Copyright 2013 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// A relative route owns its own URL namespace, so `/home/details/edit` and +// `/home/edit` are distinct and must not be reported as duplicates. + +import 'package:go_router/go_router.dart'; + +mixin $HomeRoute {} +mixin $DetailsRoute {} +mixin $NestedEditRoute {} +mixin $SiblingEditRoute {} + +@TypedGoRoute( + path: '/home', + routes: >[ + TypedRelativeGoRoute( + path: 'details', + routes: >[TypedRelativeGoRoute(path: 'edit')], + ), + TypedGoRoute(path: 'edit'), + ], +) +class HomeRoute extends GoRouteData with $HomeRoute {} + +class DetailsRoute extends RelativeGoRouteData with $DetailsRoute {} + +class NestedEditRoute extends RelativeGoRouteData with $NestedEditRoute {} + +class SiblingEditRoute extends GoRouteData with $SiblingEditRoute {} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_nested_relative_routes.dart.expect b/packages/go_router_builder/test_inputs/duplicate_path_nested_relative_routes.dart.expect new file mode 100644 index 000000000000..bb213ea44138 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_nested_relative_routes.dart.expect @@ -0,0 +1,114 @@ +RouteBase get $homeRoute => GoRouteData.$route( + path: '/home', + hasOverriddenOnExit: false, + factory: $HomeRoute._fromState, + routes: [ + RelativeGoRouteData.$route( + path: 'details', + hasOverriddenOnExit: false, + factory: $DetailsRoute._fromState, + routes: [ + RelativeGoRouteData.$route( + path: 'edit', + hasOverriddenOnExit: false, + factory: $NestedEditRoute._fromState, + ), + ], + ), + GoRouteData.$route( + path: 'edit', + hasOverriddenOnExit: false, + factory: $SiblingEditRoute._fromState, + ), + ], +); + +mixin $HomeRoute on GoRouteData { + static HomeRoute _fromState(GoRouterState state) => HomeRoute(); + + @override + String get location => GoRouteData.$location('/home'); + + @override + void go(BuildContext context) => context.go(location); + + @override + Future push(BuildContext context) => context.push(location); + + @override + void pushReplacement(BuildContext context) => + context.pushReplacement(location); + + @override + void replace(BuildContext context) => context.replace(location); +} + +mixin $DetailsRoute on RelativeGoRouteData { + static DetailsRoute _fromState(GoRouterState state) => DetailsRoute(); + + @override + String get subLocation => RelativeGoRouteData.$location('details'); + + @override + String get relativeLocation => './$subLocation'; + + @override + void goRelative(BuildContext context) => context.go(relativeLocation); + + @override + Future pushRelative(BuildContext context) => + context.push(relativeLocation); + + @override + void pushReplacementRelative(BuildContext context) => + context.pushReplacement(relativeLocation); + + @override + void replaceRelative(BuildContext context) => + context.replace(relativeLocation); +} + +mixin $NestedEditRoute on RelativeGoRouteData { + static NestedEditRoute _fromState(GoRouterState state) => NestedEditRoute(); + + @override + String get subLocation => RelativeGoRouteData.$location('edit'); + + @override + String get relativeLocation => './$subLocation'; + + @override + void goRelative(BuildContext context) => context.go(relativeLocation); + + @override + Future pushRelative(BuildContext context) => + context.push(relativeLocation); + + @override + void pushReplacementRelative(BuildContext context) => + context.pushReplacement(relativeLocation); + + @override + void replaceRelative(BuildContext context) => + context.replace(relativeLocation); +} + +mixin $SiblingEditRoute on GoRouteData { + static SiblingEditRoute _fromState(GoRouterState state) => SiblingEditRoute(); + + @override + String get location => GoRouteData.$location('/home/edit'); + + @override + void go(BuildContext context) => context.go(location); + + @override + Future push(BuildContext context) => context.push(location); + + @override + void pushReplacement(BuildContext context) => + context.pushReplacement(location); + + @override + void replace(BuildContext context) => context.replace(location); +} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_nested_relative_routes.dart.warnings b/packages/go_router_builder/test_inputs/duplicate_path_nested_relative_routes.dart.warnings new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/packages/go_router_builder/test_inputs/duplicate_path_nested_under_duplicate_parents.dart b/packages/go_router_builder/test_inputs/duplicate_path_nested_under_duplicate_parents.dart new file mode 100644 index 000000000000..4c11e245c6cf --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_nested_under_duplicate_parents.dart @@ -0,0 +1,34 @@ +// Copyright 2013 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Two declarations of one parent are sound on their own, but their children +// share a URL namespace, so a child path repeated across them is a collision. + +import 'package:go_router/go_router.dart'; + +mixin $HomeRoute {} +mixin $DetailsRoute {} +mixin $FirstSubRoute {} +mixin $SecondSubRoute {} + +@TypedGoRoute( + path: '/home', + routes: >[ + TypedGoRoute( + path: 'details', + routes: >[TypedGoRoute(path: 'sub')], + ), + TypedGoRoute( + path: 'details', + routes: >[TypedGoRoute(path: 'sub')], + ), + ], +) +class HomeRoute extends GoRouteData with $HomeRoute {} + +class DetailsRoute extends GoRouteData with $DetailsRoute {} + +class FirstSubRoute extends GoRouteData with $FirstSubRoute {} + +class SecondSubRoute extends GoRouteData with $SecondSubRoute {} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_nested_under_duplicate_parents.dart.expect b/packages/go_router_builder/test_inputs/duplicate_path_nested_under_duplicate_parents.dart.expect new file mode 100644 index 000000000000..b767ca0f5174 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_nested_under_duplicate_parents.dart.expect @@ -0,0 +1,111 @@ +RouteBase get $homeRoute => GoRouteData.$route( + path: '/home', + hasOverriddenOnExit: false, + factory: $HomeRoute._fromState, + routes: [ + GoRouteData.$route( + path: 'details', + hasOverriddenOnExit: false, + factory: $DetailsRoute._fromState, + routes: [ + GoRouteData.$route( + path: 'sub', + hasOverriddenOnExit: false, + factory: $FirstSubRoute._fromState, + ), + ], + ), + GoRouteData.$route( + path: 'details', + hasOverriddenOnExit: false, + factory: $DetailsRoute._fromState, + routes: [ + GoRouteData.$route( + path: 'sub', + hasOverriddenOnExit: false, + factory: $SecondSubRoute._fromState, + ), + ], + ), + ], +); + +mixin $HomeRoute on GoRouteData { + static HomeRoute _fromState(GoRouterState state) => HomeRoute(); + + @override + String get location => GoRouteData.$location('/home'); + + @override + void go(BuildContext context) => context.go(location); + + @override + Future push(BuildContext context) => context.push(location); + + @override + void pushReplacement(BuildContext context) => + context.pushReplacement(location); + + @override + void replace(BuildContext context) => context.replace(location); +} + +mixin $DetailsRoute on GoRouteData { + static DetailsRoute _fromState(GoRouterState state) => DetailsRoute(); + + @override + String get location => GoRouteData.$location('/home/details'); + + @override + void go(BuildContext context) => context.go(location); + + @override + Future push(BuildContext context) => context.push(location); + + @override + void pushReplacement(BuildContext context) => + context.pushReplacement(location); + + @override + void replace(BuildContext context) => context.replace(location); +} + +mixin $FirstSubRoute on GoRouteData { + static FirstSubRoute _fromState(GoRouterState state) => FirstSubRoute(); + + @override + String get location => GoRouteData.$location('/home/details/sub'); + + @override + void go(BuildContext context) => context.go(location); + + @override + Future push(BuildContext context) => context.push(location); + + @override + void pushReplacement(BuildContext context) => + context.pushReplacement(location); + + @override + void replace(BuildContext context) => context.replace(location); +} + +mixin $SecondSubRoute on GoRouteData { + static SecondSubRoute _fromState(GoRouterState state) => SecondSubRoute(); + + @override + String get location => GoRouteData.$location('/home/details/sub'); + + @override + void go(BuildContext context) => context.go(location); + + @override + Future push(BuildContext context) => context.push(location); + + @override + void pushReplacement(BuildContext context) => + context.pushReplacement(location); + + @override + void replace(BuildContext context) => context.replace(location); +} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_nested_under_duplicate_parents.dart.warnings b/packages/go_router_builder/test_inputs/duplicate_path_nested_under_duplicate_parents.dart.warnings new file mode 100644 index 000000000000..5d3c78c46269 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_nested_under_duplicate_parents.dart.warnings @@ -0,0 +1,2 @@ +Duplicate route path detected: DetailsRoute is declared more than once at "/home/details". +Duplicate route path detected: "/home/details/sub" from FirstSubRoute and "/home/details/sub" from SecondSubRoute both match the same URL pattern. diff --git a/packages/go_router_builder/test_inputs/duplicate_path_relative_routes.dart b/packages/go_router_builder/test_inputs/duplicate_path_relative_routes.dart new file mode 100644 index 000000000000..91564d4d90f9 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_relative_routes.dart @@ -0,0 +1,22 @@ +// Copyright 2013 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:go_router/go_router.dart'; + +mixin $HomeRoute {} +mixin $FirstRoute {} +mixin $SecondRoute {} + +@TypedGoRoute( + path: '/home', + routes: >[ + TypedRelativeGoRoute(path: 'details'), + TypedRelativeGoRoute(path: 'details'), + ], +) +class HomeRoute extends GoRouteData with $HomeRoute {} + +class FirstRoute extends RelativeGoRouteData with $FirstRoute {} + +class SecondRoute extends RelativeGoRouteData with $SecondRoute {} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_relative_routes.dart.expect b/packages/go_router_builder/test_inputs/duplicate_path_relative_routes.dart.expect new file mode 100644 index 000000000000..cf7bfd3a6ebb --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_relative_routes.dart.expect @@ -0,0 +1 @@ +Duplicate route path detected: "/home/details" from FirstRoute and "/home/details" from SecondRoute both match the same URL pattern. diff --git a/packages/go_router_builder/test_inputs/duplicate_path_relative_routes.dart.options b/packages/go_router_builder/test_inputs/duplicate_path_relative_routes.dart.options new file mode 100644 index 000000000000..c7f07c4e62c0 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_relative_routes.dart.options @@ -0,0 +1,3 @@ +{ + "duplicate_route_paths": "error" +} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_same_class.dart b/packages/go_router_builder/test_inputs/duplicate_path_same_class.dart new file mode 100644 index 000000000000..561e0d246e47 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_same_class.dart @@ -0,0 +1,36 @@ +// Copyright 2013 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Declaring one route class twice, so its children can be grouped by feature +// area, works at runtime. It is still reported, because the builder cannot tell +// a deliberate grouping from an accidental duplicate. Use the `ignore` severity +// to opt out. + +import 'package:go_router/go_router.dart'; + +mixin $HomeRoute {} +mixin $DetailsRoute {} +mixin $InvoicesRoute {} +mixin $ShipmentsRoute {} + +@TypedGoRoute( + path: '/home', + routes: >[ + TypedGoRoute( + path: 'details', + routes: >[TypedGoRoute(path: 'invoices')], + ), + TypedGoRoute( + path: 'details', + routes: >[TypedGoRoute(path: 'shipments')], + ), + ], +) +class HomeRoute extends GoRouteData with $HomeRoute {} + +class DetailsRoute extends GoRouteData with $DetailsRoute {} + +class InvoicesRoute extends GoRouteData with $InvoicesRoute {} + +class ShipmentsRoute extends GoRouteData with $ShipmentsRoute {} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_same_class.dart.expect b/packages/go_router_builder/test_inputs/duplicate_path_same_class.dart.expect new file mode 100644 index 000000000000..9fc90475f1d0 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_same_class.dart.expect @@ -0,0 +1 @@ +Duplicate route path detected: DetailsRoute is declared more than once at "/home/details". diff --git a/packages/go_router_builder/test_inputs/duplicate_path_same_class.dart.options b/packages/go_router_builder/test_inputs/duplicate_path_same_class.dart.options new file mode 100644 index 000000000000..c7f07c4e62c0 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_same_class.dart.options @@ -0,0 +1,3 @@ +{ + "duplicate_route_paths": "error" +} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_sibling_routes.dart b/packages/go_router_builder/test_inputs/duplicate_path_sibling_routes.dart new file mode 100644 index 000000000000..24828034ab9f --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_sibling_routes.dart @@ -0,0 +1,28 @@ +// Copyright 2013 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:go_router/go_router.dart'; + +mixin $HomeRoute {} +mixin $SettingsRoute {} +mixin $DuplicateRoute {} + +@TypedGoRoute( + path: '/home', + routes: >[ + TypedGoRoute(path: 'details/:id'), + TypedGoRoute(path: 'details/:id'), + ], +) +class HomeRoute extends GoRouteData with $HomeRoute {} + +class SettingsRoute extends GoRouteData with $SettingsRoute { + const SettingsRoute({required this.id}); + final String id; +} + +class DuplicateRoute extends GoRouteData with $DuplicateRoute { + const DuplicateRoute({required this.id}); + final String id; +} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_sibling_routes.dart.expect b/packages/go_router_builder/test_inputs/duplicate_path_sibling_routes.dart.expect new file mode 100644 index 000000000000..52729fbf2af0 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_sibling_routes.dart.expect @@ -0,0 +1 @@ +Duplicate route path detected: "/home/details/:id" from SettingsRoute and "/home/details/:id" from DuplicateRoute both match the same URL pattern. diff --git a/packages/go_router_builder/test_inputs/duplicate_path_sibling_routes.dart.options b/packages/go_router_builder/test_inputs/duplicate_path_sibling_routes.dart.options new file mode 100644 index 000000000000..c7f07c4e62c0 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_sibling_routes.dart.options @@ -0,0 +1,3 @@ +{ + "duplicate_route_paths": "error" +} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_warns_by_default.dart b/packages/go_router_builder/test_inputs/duplicate_path_warns_by_default.dart new file mode 100644 index 000000000000..4585aaa954f6 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_warns_by_default.dart @@ -0,0 +1,22 @@ +// Copyright 2013 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:go_router/go_router.dart'; + +mixin $HomeRoute {} +mixin $FirstRoute {} +mixin $SecondRoute {} + +@TypedGoRoute( + path: '/home', + routes: >[ + TypedGoRoute(path: 'details'), + TypedGoRoute(path: 'details'), + ], +) +class HomeRoute extends GoRouteData with $HomeRoute {} + +class FirstRoute extends GoRouteData with $FirstRoute {} + +class SecondRoute extends GoRouteData with $SecondRoute {} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_warns_by_default.dart.expect b/packages/go_router_builder/test_inputs/duplicate_path_warns_by_default.dart.expect new file mode 100644 index 000000000000..8cb238063845 --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_warns_by_default.dart.expect @@ -0,0 +1,77 @@ +RouteBase get $homeRoute => GoRouteData.$route( + path: '/home', + hasOverriddenOnExit: false, + factory: $HomeRoute._fromState, + routes: [ + GoRouteData.$route( + path: 'details', + hasOverriddenOnExit: false, + factory: $FirstRoute._fromState, + ), + GoRouteData.$route( + path: 'details', + hasOverriddenOnExit: false, + factory: $SecondRoute._fromState, + ), + ], +); + +mixin $HomeRoute on GoRouteData { + static HomeRoute _fromState(GoRouterState state) => HomeRoute(); + + @override + String get location => GoRouteData.$location('/home'); + + @override + void go(BuildContext context) => context.go(location); + + @override + Future push(BuildContext context) => context.push(location); + + @override + void pushReplacement(BuildContext context) => + context.pushReplacement(location); + + @override + void replace(BuildContext context) => context.replace(location); +} + +mixin $FirstRoute on GoRouteData { + static FirstRoute _fromState(GoRouterState state) => FirstRoute(); + + @override + String get location => GoRouteData.$location('/home/details'); + + @override + void go(BuildContext context) => context.go(location); + + @override + Future push(BuildContext context) => context.push(location); + + @override + void pushReplacement(BuildContext context) => + context.pushReplacement(location); + + @override + void replace(BuildContext context) => context.replace(location); +} + +mixin $SecondRoute on GoRouteData { + static SecondRoute _fromState(GoRouterState state) => SecondRoute(); + + @override + String get location => GoRouteData.$location('/home/details'); + + @override + void go(BuildContext context) => context.go(location); + + @override + Future push(BuildContext context) => context.push(location); + + @override + void pushReplacement(BuildContext context) => + context.pushReplacement(location); + + @override + void replace(BuildContext context) => context.replace(location); +} diff --git a/packages/go_router_builder/test_inputs/duplicate_path_warns_by_default.dart.warnings b/packages/go_router_builder/test_inputs/duplicate_path_warns_by_default.dart.warnings new file mode 100644 index 000000000000..cf7bfd3a6ebb --- /dev/null +++ b/packages/go_router_builder/test_inputs/duplicate_path_warns_by_default.dart.warnings @@ -0,0 +1 @@ +Duplicate route path detected: "/home/details" from FirstRoute and "/home/details" from SecondRoute both match the same URL pattern. diff --git a/packages/go_router_builder/tool/run_tests.dart b/packages/go_router_builder/tool/run_tests.dart index bfd42c78520c..cd57c9454561 100644 --- a/packages/go_router_builder/tool/run_tests.dart +++ b/packages/go_router_builder/tool/run_tests.dart @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:async'; +import 'dart:convert'; import 'dart:io'; import 'dart:isolate'; @@ -9,15 +11,15 @@ import 'package:analyzer/dart/element/element.dart'; import 'package:build/build.dart'; import 'package:build_test/build_test.dart'; import 'package:dart_style/dart_style.dart' as dart_style; +import 'package:go_router_builder/src/duplicate_path_severity.dart'; import 'package:go_router_builder/src/go_router_generator.dart'; +import 'package:logging/logging.dart'; import 'package:package_config/package_config.dart'; import 'package:path/path.dart' as p; import 'package:pub_semver/pub_semver.dart'; import 'package:source_gen/source_gen.dart'; import 'package:test/test.dart'; -const GoRouterGenerator generator = GoRouterGenerator(); - Future main() async { final formatter = dart_style.DartFormatter(languageVersion: await _packageVersion()); final dir = Directory('test_inputs'); @@ -27,7 +29,7 @@ Future main() async { .where((File f) => f.path.endsWith('.dart')) .toList(); for (final file in testFiles) { - final String fileName = file.path.split('/').last; + final String fileName = p.basename(file.path); final expectFile = File(p.join('${file.path}.expect')); if (!expectFile.existsSync()) { throw Exception( @@ -35,7 +37,29 @@ Future main() async { 'Found test input $fileName with out an expect file.', ); } - final String expectResult = expectFile.readAsStringSync().trim(); + final String expectResult = _readAsLf(expectFile).trim(); + + // A test input may declare builder options in a `.options` file holding a + // JSON map, matching what `build.yaml` would pass to the builder. + final optionsFile = File(p.join('${file.path}.options')); + final BuilderOptions builderOptions = optionsFile.existsSync() + ? BuilderOptions(json.decode(optionsFile.readAsStringSync()) as Map) + : BuilderOptions.empty; + final generator = GoRouterGenerator( + duplicatePathSeverity: duplicatePathSeverityFromOptions(builderOptions), + ); + + // A test input declares the warnings the builder must log in a `.warnings` + // file, one per line. Inputs that log nothing need no file, so an input that + // starts logging a warning fails until the warning is declared. + final warningsFile = File(p.join('${file.path}.warnings')); + final List expectedWarnings = warningsFile.existsSync() + ? const LineSplitter() + .convert(_readAsLf(warningsFile)) + .where((String line) => line.isNotEmpty) + .toList() + : []; + test('verify $fileName', () async { // Normalize path separators for cross-platform compatibility final String path = file.path.replaceAll(r'\', '/'); @@ -50,21 +74,45 @@ Future main() async { ); final reader = LibraryReader(element); final results = {}; + // Outside a build step, `package:build`'s `log` falls back to a plain + // logger, whose records reach the root logger. + final warnings = []; + final StreamSubscription logs = Logger.root.onRecord.listen((LogRecord record) { + if (record.level >= Level.WARNING) { + warnings.add(record.message); + } + }); try { generator.generateForAnnotation(reader, results, {}); } on InvalidGenerationSourceError catch (e) { - expect(expectResult, e.message.trim()); + // The generated message is the value under test, so it goes first. + // Reversing these labels the diff backwards on failure. + expect(e.message.trim(), expectResult); return; + } finally { + await logs.cancel(); } + expect(warnings, expectedWarnings); + // Apply consistent formatting to both generated and expected code for comparison. final String generated = formatter.format(results.join('\n\n').trim()); final String expected = formatter.format(expectResult.trim()); expect(generated, equals(expected)); - }, timeout: const Timeout(Duration(seconds: 100))); + // Each case resolves its input against the real SDK and package sources, + // which takes seconds on an idle machine but far longer on a loaded one. + // The generous timeout keeps a busy bot from reporting a false failure. + }, timeout: const Timeout(Duration(minutes: 5))); } } +/// Reads [file] with line endings normalized to `\n`. +/// +/// The repository normalizes text files to the platform's endings on checkout, +/// so these files arrive with `\r\n` on Windows while the values they are +/// compared against always use `\n`. +String _readAsLf(File file) => file.readAsStringSync().replaceAll('\r\n', '\n'); + Future _packageVersion() async { final PackageConfig packageConfig = await loadPackageConfigUri(Isolate.packageConfigSync!); final Uri pkgUri = Platform.script.resolve('../pubspec.yaml');