fix(dart_frog_gen): detect rogue routes (#248)

Author: felangel

packages/dart_frog_gen/lib/src/build_route_configuration.dart

@@ -28,6 +28,7 @@ RouteConfiguration buildRouteConfiguration(Directory directory) {
     if (globalMiddleware != null) globalMiddleware
   ];
   final routes = <RouteFile>[];
+  final rogueRoutes = <RouteFile>[];
   final directories = _getRouteDirectories(
     directory: routesDirectory,
     routesDirectory: routesDirectory,
@@ -40,13 +41,15 @@ RouteConfiguration buildRouteConfiguration(Directory directory) {
         endpoints[endpoint]!.add(file);
       }
     },
+    onRogueRoute: rogueRoutes.add,
   );
   final publicDirectory = Directory(path.join(directory.path, 'public'));
   return RouteConfiguration(
     globalMiddleware: globalMiddleware,
     middleware: middleware,
     directories: directories,
     routes: routes,
+    rogueRoutes: rogueRoutes,
     endpoints: endpoints,
     serveStaticFiles: publicDirectory.existsSync(),
   );
@@ -58,6 +61,7 @@ List<RouteDirectory> _getRouteDirectories({
   required void Function(RouteFile route) onRoute,
   required void Function(MiddlewareFile route) onMiddleware,
   required void Function(String endpoint, RouteFile file) onEndpoint,
+  required void Function(RouteFile route) onRogueRoute,
 }) {
   final directories = <RouteDirectory>[];
   final entities = directory.listSync().sorted();
@@ -87,11 +91,13 @@ List<RouteDirectory> _getRouteDirectories({
       directory: directory,
       routesDirectory: routesDirectory,
       onRoute: onRoute,
+      onRogueRoute: onRogueRoute,
     ),
     ..._getRouteFilesForDynamicDirectories(
       directory: directory,
       routesDirectory: routesDirectory,
       onRoute: onRoute,
+      onRogueRoute: onRogueRoute,
     ),
   ];
 
@@ -123,6 +129,7 @@ List<RouteDirectory> _getRouteDirectories({
           onRoute: onRoute,
           onMiddleware: onMiddleware,
           onEndpoint: onEndpoint,
+          onRogueRoute: onRogueRoute,
         ),
       );
     }
@@ -135,6 +142,7 @@ List<RouteFile> _getRouteFilesForDynamicDirectories({
   required Directory directory,
   required Directory routesDirectory,
   required void Function(RouteFile route) onRoute,
+  required void Function(RouteFile route) onRogueRoute,
   String prefix = '',
 }) {
   final files = <RouteFile>[];
@@ -149,12 +157,14 @@ List<RouteFile> _getRouteFilesForDynamicDirectories({
       directory: dynamicDirectory,
       routesDirectory: routesDirectory,
       onRoute: onRoute,
+      onRogueRoute: onRogueRoute,
       prefix: newPrefix,
     );
     final dynamicSubset = _getRouteFilesForDynamicDirectories(
       directory: dynamicDirectory,
       routesDirectory: routesDirectory,
       onRoute: onRoute,
+      onRogueRoute: onRogueRoute,
       prefix: newPrefix,
     );
     files.addAll([...subset, ...dynamicSubset]);
@@ -166,6 +176,7 @@ List<RouteFile> _getRouteFiles({
   required Directory directory,
   required Directory routesDirectory,
   required void Function(RouteFile route) onRoute,
+  required void Function(RouteFile route) onRogueRoute,
   String prefix = '',
 }) {
   final files = <RouteFile>[];
@@ -175,6 +186,10 @@ List<RouteFile> _getRouteFiles({
       ? directorySegment
       : '/$directorySegment';
   final entities = directory.listSync().sorted();
+  final subDirectories = entities
+      .whereType<Directory>()
+      .map((directory) => path.basename(directory.path))
+      .toSet();
   entities.where((e) => e.isRoute).cast<File>().forEach((entity) {
     final filePath = path
         .relative(entity.path, from: routesDirectory.path)
@@ -209,6 +224,10 @@ List<RouteFile> _getRouteFiles({
     );
     onRoute(route);
     files.add(route);
+
+    if (subDirectories.contains(path.basenameWithoutExtension(filePath))) {
+      onRogueRoute(route);
+    }
   });
   return files;
 }
@@ -261,6 +280,7 @@ class RouteConfiguration {
     required this.directories,
     required this.routes,
     required this.endpoints,
+    required this.rogueRoutes,
     this.serveStaticFiles = false,
   });
 
@@ -283,6 +303,34 @@ class RouteConfiguration {
 
   /// A map of all endpoint paths to resolved route files.
   final Map<String, List<RouteFile>> endpoints;
+
+  /// List of all rogue routes.
+  ///
+  /// A route is considered rogue when it is defined outside
+  /// of an existing subdirectory with the same name.
+  ///
+  /// For example:
+  ///
+  /// ```
+  /// ├── routes
+  /// │   ├── foo
+  /// │   │   └── example.dart
+  /// │   ├── foo.dart
+  /// ```
+  ///
+  /// In the above scenario, `foo.dart` is rogue because it is defined
+  /// outside of the existing `foo` directory.
+  ///
+  /// Instead, `foo.dart` should be renamed to `index.dart` and placed within
+  /// the `foo` directory like:
+  ///
+  /// ```
+  /// ├── routes
+  /// │   ├── foo
+  /// │   │   ├── example.dart
+  /// │   │   └── index.dart
+  /// ```
+  final List<RouteFile> rogueRoutes;
 }
 
 /// {@template route_directory}
@@ -336,7 +384,7 @@ class RouteDirectory {
 }
 
 /// {@template route_file}
-/// A class containing metadata regarding a route directory.
+/// A class containing metadata regarding a route file.
 /// {@endtemplate}
 class RouteFile {
   /// {@macro route_file}
@@ -346,7 +394,7 @@ class RouteFile {
     required this.route,
   });
 
-  /// The alias for the current directory.
+  /// The alias for the current file.
   final String name;
 
   /// The import path for the current instance.

packages/dart_frog_gen/test/src/build_route_configuration_test.dart

@@ -696,5 +696,114 @@ void main() {
         }),
       );
     });
+
+    test('detects rogue routes.', () {
+      const expected = [
+        {
+          'name': '_',
+          'route': '/',
+          'middleware': false,
+          'files': [
+            {'name': 'api', 'path': '../routes/api.dart', 'route': '/api'}
+          ]
+        },
+        {
+          'name': '_api',
+          'route': '/api',
+          'middleware': false,
+          'files': [
+            {'name': 'api_v1', 'path': '../routes/api/v1.dart', 'route': '/v1'},
+            {
+              'name': r'api_$id',
+              'path': '../routes/api/[id].dart',
+              'route': '/<id>'
+            }
+          ]
+        },
+        {
+          'name': '_api_v1',
+          'route': '/api/v1',
+          'middleware': false,
+          'files': [
+            {
+              'name': 'api_v1_hello',
+              'path': '../routes/api/v1/hello.dart',
+              'route': '/hello'
+            }
+          ]
+        }
+      ];
+      final directory = Directory(
+        path.join(
+          Directory.current.path,
+          'test',
+          '.fixtures',
+          'rogue_routes',
+        ),
+      )..createSync(recursive: true);
+      final routes = Directory(path.join(directory.path, 'routes'))
+        ..createSync();
+      File(path.join(routes.path, 'api.dart')).createSync();
+      final apiDirectory = Directory(path.join(routes.path, 'api'))
+        ..createSync();
+      File(path.join(apiDirectory.path, '[id].dart')).createSync();
+      File(path.join(apiDirectory.path, 'v1.dart')).createSync();
+      final v1Directory = Directory(path.join(apiDirectory.path, 'v1'))
+        ..createSync();
+      File(path.join(v1Directory.path, 'hello.dart')).createSync();
+      final configuration = buildRouteConfiguration(directory);
+      expect(
+        configuration.directories.map((d) => d.toJson()).toList(),
+        equals(expected),
+      );
+      expect(
+        configuration.endpoints,
+        equals({
+          '/api': [
+            isA<RouteFile>().having(
+              (r) => r.path,
+              'path',
+              '../routes/api.dart',
+            )
+          ],
+          '/api/v1': [
+            isA<RouteFile>().having(
+              (r) => r.path,
+              'path',
+              '../routes/api/v1.dart',
+            )
+          ],
+          '/api/<id>': [
+            isA<RouteFile>().having(
+              (r) => r.path,
+              'path',
+              '../routes/api/[id].dart',
+            )
+          ],
+          '/api/v1/hello': [
+            isA<RouteFile>().having(
+              (r) => r.path,
+              'path',
+              '../routes/api/v1/hello.dart',
+            )
+          ],
+        }),
+      );
+      expect(
+        configuration.rogueRoutes,
+        equals([
+          isA<RouteFile>().having(
+            (r) => r.path,
+            'path',
+            '../routes/api.dart',
+          ),
+          isA<RouteFile>().having(
+            (r) => r.path,
+            'path',
+            '../routes/api/v1.dart',
+          )
+        ]),
+      );
+    });
   });
 }