Differentiate types of request explicitly, remove ListOfService.

pkgs/_macro_host/lib/macro_host.dart

@@ -14,32 +14,19 @@ import 'package:macro_service/macro_service.dart';
 ///
 /// Tools that want to support macros, such as the Analyzer and the CFE, can
 /// do so by running a `MacroHost` and providing their own `HostService`.
-class MacroHost implements HostService {
+class MacroHost {
+  final _HostService _hostService;
   final MacroServer macroServer;
-  final ListOfServices services;
   final MacroBuilder macroBuilder = MacroBuilder();
   final MacroRunner macroRunner = MacroRunner();
 
-  // TODO(davidmorgan): this should be per macro, as part of tracking per-macro
-  // lifecycle state.
-  Completer<Set<int>>? _macroPhases;
-
-  MacroHost._(this.macroServer, this.services) {
-    services.services.insert(0, this);
-  }
+  MacroHost._(this.macroServer, this._hostService);
 
-  /// Starts a macro host serving the provided [service].
-  ///
-  /// The service passed in should handle introspection RPCs, it does not need
-  /// to handle others.
-  ///
-  /// TODO(davidmorgan): make this split clearer, it should be in the protocol
-  /// definition somewhere which requests the host handles.
-  static Future<MacroHost> serve({required HostService service}) async {
-    final listOfServices = ListOfServices();
-    listOfServices.services.add(service);
-    final server = await MacroServer.serve(service: listOfServices);
-    return MacroHost._(server, listOfServices);
+  /// Starts a macro host with introspection queries handled by [queryService].
+  static Future<MacroHost> serve({required QueryService queryService}) async {
+    final hostService = _HostService(queryService);
+    final server = await MacroServer.serve(service: hostService);
+    return MacroHost._(server, hostService);
   }
 
   /// Whether [name] is a macro according to that package's `pubspec.yaml`.
@@ -54,11 +41,13 @@ class MacroHost implements HostService {
       Uri packageConfig, QualifiedName name) async {
     // TODO(davidmorgan): track macro lifecycle, correctly run once per macro
     // code change including if queried multiple times before response returns.
-    if (_macroPhases != null) return _macroPhases!.future;
-    _macroPhases = Completer();
+    if (_hostService._macroPhases != null) {
+      return _hostService._macroPhases!.future;
+    }
+    _hostService._macroPhases = Completer();
     final macroBundle = await macroBuilder.build(packageConfig, [name]);
     macroRunner.start(macroBundle: macroBundle, endpoint: macroServer.endpoint);
-    return _macroPhases!.future;
+    return _hostService._macroPhases!.future;
   }
 
   /// Sends [request] to the macro with [name].
@@ -70,34 +59,35 @@ class MacroHost implements HostService {
         name, HostRequest.augmentRequest(request));
     return response.asAugmentResponse;
   }
+}
+
+class _HostService implements HostService {
+  final QueryService queryService;
+  // TODO(davidmorgan): this should be per macro, as part of tracking per-macro
+  // lifecycle state.
+  Completer<Set<int>>? _macroPhases;
+
+  _HostService(this.queryService);
 
   /// Handle requests that are for the host.
   @override
-  Future<Response?> handle(MacroRequest request) async {
+  Future<Response> handle(MacroRequest request) async {
     switch (request.type) {
       case MacroRequestType.macroStartedRequest:
         _macroPhases!.complete(request
             .asMacroStartedRequest.macroDescription.runsInPhases
             .toSet());
         return Response.macroStartedResponse(MacroStartedResponse());
+      case MacroRequestType.queryRequest:
+        return Response.queryResponse(
+            await queryService.handle(request.asQueryRequest));
       default:
-        return null;
+        return Response.errorResponse(ErrorResponse(error: 'unsupported'));
     }
   }
 }
 
-// TODO(davidmorgan): this is used to handle some requests in the host while
-// letting some fall through to the passed in service. Differentiate in a
-// better way.
-class ListOfServices implements HostService {
-  List<HostService> services = [];
-
-  @override
-  Future<Response> handle(MacroRequest request) async {
-    for (final service in services) {
-      final result = await service.handle(request);
-      if (result != null) return result;
-    }
-    throw StateError('No service handled: $request');
-  }
+/// Service provided by the frontend the host integrates with.
+abstract interface class QueryService {
+  Future<QueryResponse> handle(QueryRequest request);
 }

pkgs/_macro_host/test/macro_host_test.dart

@@ -12,8 +12,8 @@ import 'package:test/test.dart';
 void main() {
   group(MacroHost, () {
     test('hosts a macro, receives augmentations', () async {
-      final service = TestMacroHostService();
-      final host = await MacroHost.serve(service: service);
+      final service = TestQueryService();
+      final host = await MacroHost.serve(queryService: service);
 
       final macroName = QualifiedName(
           'package:_test_macros/declare_x_macro.dart#DeclareXImplementation');
@@ -29,8 +29,8 @@ void main() {
     });
 
     test('hosts a macro, responds to queries', () async {
-      final service = TestMacroHostService();
-      final host = await MacroHost.serve(service: service);
+      final service = TestQueryService();
+      final host = await MacroHost.serve(queryService: service);
 
       final macroName = QualifiedName(
           'package:_test_macros/query_class.dart#QueryClassImplementation');
@@ -48,13 +48,10 @@ void main() {
   });
 }
 
-class TestMacroHostService implements HostService {
+class TestQueryService implements QueryService {
   @override
-  Future<Response?> handle(MacroRequest request) async {
-    if (request.type == MacroRequestType.queryRequest) {
-      return Response.queryResponse(QueryResponse(
-          model: Model(uris: {'package:foo/foo.dart': Library()})));
-    }
-    return null;
+  Future<QueryResponse> handle(QueryRequest request) async {
+    return QueryResponse(
+        model: Model(uris: {'package:foo/foo.dart': Library()}));
   }
 }

pkgs/_macro_server/test/macro_server_test.dart

@@ -13,7 +13,7 @@ import 'package:test/test.dart';
 void main() {
   group(MacroServer, () {
     test('serves a macro service', () async {
-      final service = TestMacroHostService();
+      final service = TestHostService();
       final server = await MacroServer.serve(service: service);
 
       await MacroClient.run(
@@ -26,18 +26,18 @@ void main() {
   });
 }
 
-class TestMacroHostService implements HostService {
+class TestHostService implements HostService {
   final StreamController<MacroStartedRequest> _macroStartedRequestsController =
       StreamController();
   Stream<MacroStartedRequest> get macroStartedRequests =>
       _macroStartedRequestsController.stream;
 
   @override
-  Future<Response?> handle(MacroRequest request) async {
+  Future<Response> handle(MacroRequest request) async {
     if (request.type == MacroRequestType.macroStartedRequest) {
       _macroStartedRequestsController.add(request.asMacroStartedRequest);
       return Response.macroStartedResponse(MacroStartedResponse());
     }
-    return null;
+    return Response.errorResponse(ErrorResponse(error: 'unimplemented'));
   }
 }

pkgs/macro_service/lib/src/macro_service.dart

@@ -10,7 +10,7 @@ abstract interface class HostService {
   ///
   /// Returns `null` if the request is of a type not handled by this service
   /// instance.
-  Future<Response?> handle(MacroRequest request);
+  Future<Response> handle(MacroRequest request);
 }
 
 /// Service provided by the macro to the host.

pkgs/macro_service/lib/src/macro_service.g.dart

@@ -28,6 +28,18 @@ extension type AugmentResponse.fromJson(Map<String, Object?> node) {
       (node['augmentations'] as List).cast();
 }
 
+/// Request could not be handled.
+extension type ErrorResponse.fromJson(Map<String, Object?> node) {
+  ErrorResponse({
+    String? error,
+  }) : this.fromJson({
+          if (error != null) 'error': error,
+        });
+
+  /// The error.
+  String get error => node['error'] as String;
+}
+
 /// A macro host server endpoint. TODO(davidmorgan): this should be a oneOf supporting different types of connection. TODO(davidmorgan): it's not clear if this belongs in this package! But, where else?
 extension type HostEndpoint.fromJson(Map<String, Object?> node) {
   HostEndpoint({
@@ -157,6 +169,7 @@ extension type QueryResponse.fromJson(Map<String, Object?> node) {
 enum ResponseType {
   unknown,
   augmentResponse,
+  errorResponse,
   macroStartedResponse,
   queryResponse;
 }
@@ -165,6 +178,8 @@ extension type Response.fromJson(Map<String, Object?> node) {
   static Response augmentResponse(AugmentResponse augmentResponse) =>
       Response.fromJson(
           {'type': 'AugmentResponse', 'value': augmentResponse.node});
+  static Response errorResponse(ErrorResponse errorResponse) =>
+      Response.fromJson({'type': 'ErrorResponse', 'value': errorResponse.node});
   static Response macroStartedResponse(
           MacroStartedResponse macroStartedResponse) =>
       Response.fromJson(
@@ -175,6 +190,8 @@ extension type Response.fromJson(Map<String, Object?> node) {
     switch (node['type'] as String) {
       case 'AugmentResponse':
         return ResponseType.augmentResponse;
+      case 'ErrorResponse':
+        return ResponseType.errorResponse;
       case 'MacroStartedResponse':
         return ResponseType.macroStartedResponse;
       case 'QueryResponse':
@@ -191,6 +208,13 @@ extension type Response.fromJson(Map<String, Object?> node) {
     return AugmentResponse.fromJson(node['value'] as Map<String, Object?>);
   }
 
+  ErrorResponse get asErrorResponse {
+    if (node['type'] != 'ErrorResponse') {
+      throw StateError('Not a ErrorResponse.');
+    }
+    return ErrorResponse.fromJson(node['value'] as Map<String, Object?>);
+  }
+
   MacroStartedResponse get asMacroStartedResponse {
     if (node['type'] != 'MacroStartedResponse') {
       throw StateError('Not a MacroStartedResponse.');

schemas/macro_service.schema.json

@@ -26,6 +26,16 @@
         }
       }
     },
+    "ErrorResponse": {
+      "type": "object",
+      "description": "Request could not be handled.",
+      "properties": {
+        "error": {
+          "description": "The error.",
+          "type": "string"
+        }
+      }
+    },
     "HostEndpoint": {
       "type": "object",
       "description": "A macro host server endpoint. TODO(davidmorgan): this should be a oneOf supporting different types of connection. TODO(davidmorgan): it's not clear if this belongs in this package! But, where else?",
@@ -91,6 +101,7 @@
       "$description": "A response to a [MacroRequest] or [HostRequest].",
       "oneOf": [
         {"$ref": "#/$defs/AugmentResponse"},
+        {"$ref": "#/$defs/ErrorResponse"},
         {"$ref": "#/$defs/MacroStartedResponse"},
         {"$ref": "#/$defs/QueryResponse"}
       ]