Add API for resolving any identifier in the final macro phase.

- Rename declarationOf to typeDeclarationOf.
- Add declarationOf api for general declarations.
- Tighten the type of typeDeclarationOf in the final phase to avoid unnecessary
  casts in user code.
- Refactor message handling a bit to unify the error handling.

Bug: dart-lang/language#3216
Change-Id: Ia61da19374abec77853d37e110a08f7dfe0d3b10
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/314280
Commit-Queue: Jake Macdonald <[email protected]>
Auto-Submit: Jake Macdonald <[email protected]>
Reviewed-by: Bob Nystrom <[email protected]>

Author: jakemac53

pkg/_fe_analyzer_shared/lib/src/macros/api/builders.dart

@@ -90,7 +90,7 @@ abstract interface class DeclarationPhaseIntrospector
   ///
   /// In the definition phase, this will return [IntrospectableType] instances
   /// for all type definitions which can have members (ie: not type aliases).
-  Future<TypeDeclaration> declarationOf(covariant Identifier identifier);
+  Future<TypeDeclaration> typeDeclarationOf(covariant Identifier identifier);
 }
 
 /// The API used by [Macro]s to contribute new (non-type)
@@ -123,6 +123,16 @@ abstract interface class EnumDeclarationBuilder
 /// phase (and later).
 abstract interface class DefinitionPhaseIntrospector
     implements DeclarationPhaseIntrospector {
+  /// Resolves any [identifier] to its [Declaration].
+  ///
+  /// This will return [IntrospectableType] instances for type declarations.
+  Future<Declaration> declarationOf(covariant Identifier identifier);
+
+  /// Resolves an [identifier] referring to a type to its [IntrospectableType]
+  /// declaration.
+  @override
+  Future<IntrospectableType> typeDeclarationOf(covariant Identifier identifier);
+
   /// Infers a real type annotation for [omittedType].
   ///
   /// If no type could be inferred, then a type annotation representing the

pkg/_fe_analyzer_shared/lib/src/macros/executor/builder_impls.dart

@@ -73,8 +73,8 @@ abstract class DeclarationBuilderBase extends TypeBuilderBase
       super.parentLibraryAugmentations});
 
   @override
-  Future<TypeDeclaration> declarationOf(IdentifierImpl identifier) =>
-      introspector.declarationOf(identifier);
+  Future<TypeDeclaration> typeDeclarationOf(IdentifierImpl identifier) =>
+      introspector.typeDeclarationOf(identifier);
 
   @override
   Future<List<ConstructorDeclaration>> constructorsOf(
@@ -160,13 +160,21 @@ class DefinitionBuilderBase extends DeclarationBuilderBase
     super.parentLibraryAugmentations,
   });
 
+  @override
+  Future<Declaration> declarationOf(Identifier identifier) =>
+      introspector.declarationOf(identifier);
+
   @override
   Future<TypeAnnotation> inferType(OmittedTypeAnnotationImpl omittedType) =>
       introspector.inferType(omittedType);
 
   @override
   Future<List<Declaration>> topLevelDeclarationsOf(Library library) =>
       introspector.topLevelDeclarationsOf(library);
+
+  @override
+  Future<IntrospectableType> typeDeclarationOf(Identifier identifier) =>
+      introspector.typeDeclarationOf(identifier);
 }
 
 class TypeDefinitionBuilderImpl extends DefinitionBuilderBase

pkg/_fe_analyzer_shared/lib/src/macros/executor/executor_base.dart

@@ -500,23 +500,32 @@ class TypeIntrospectorRequest extends IntrospectionRequest {
   }
 }
 
-/// A request to get a [TypeDeclaration] for a [StaticType].
+/// A request to get a [Declaration] for an [identifier].
+///
+/// Used for both the `typeDeclarationOf` and `declarationOf` requests. A cast
+/// is done on the client side to ensure only [TypeDeclaration]s are returned
+/// from `typeDeclarationOf`.
 class DeclarationOfRequest extends IntrospectionRequest {
   final IdentifierImpl identifier;
+  final MessageType kind;
 
-  DeclarationOfRequest(this.identifier, super.introspector,
-      {required super.serializationZoneId});
+  DeclarationOfRequest(this.identifier, this.kind, super.introspector,
+      {required super.serializationZoneId})
+      : assert(kind == MessageType.typeDeclarationOfRequest ||
+            kind == MessageType.declarationOfRequest);
 
   /// When deserializing we have already consumed the message type, so we don't
   /// consume it again.
   DeclarationOfRequest.deserialize(
-      super.deserializer, super.serializationZoneId)
-      : identifier = RemoteInstance.deserialize(deserializer),
+      super.deserializer, super.serializationZoneId, this.kind)
+      : assert(kind == MessageType.typeDeclarationOfRequest ||
+            kind == MessageType.declarationOfRequest),
+        identifier = RemoteInstance.deserialize(deserializer),
         super.deserialize();
 
   @override
   void serialize(Serializer serializer) {
-    serializer.addInt(MessageType.declarationOfRequest.index);
+    serializer.addInt(kind.index);
     identifier.serialize(serializer);
     super.serialize(serializer);
   }
@@ -686,9 +695,9 @@ final class ClientDeclarationPhaseIntrospector
   }
 
   @override
-  Future<TypeDeclaration> declarationOf(IdentifierImpl identifier) async {
+  Future<TypeDeclaration> typeDeclarationOf(IdentifierImpl identifier) async {
     DeclarationOfRequest request = new DeclarationOfRequest(
-        identifier, remoteInstance,
+        identifier, MessageType.typeDeclarationOfRequest, remoteInstance,
         serializationZoneId: serializationZoneId);
     return _handleResponse<TypeDeclaration>(await _sendRequest(request));
   }
@@ -731,6 +740,14 @@ final class ClientDefinitionPhaseIntrospector
   ClientDefinitionPhaseIntrospector(super._sendRequest,
       {required super.remoteInstance, required super.serializationZoneId});
 
+  @override
+  Future<Declaration> declarationOf(IdentifierImpl identifier) async {
+    DeclarationOfRequest request = new DeclarationOfRequest(
+        identifier, MessageType.declarationOfRequest, remoteInstance,
+        serializationZoneId: serializationZoneId);
+    return _handleResponse<Declaration>(await _sendRequest(request));
+  }
+
   @override
   Future<TypeAnnotation> inferType(
       OmittedTypeAnnotationImpl omittedType) async {
@@ -747,6 +764,15 @@ final class ClientDefinitionPhaseIntrospector
     return _handleResponse<DeclarationList>(await _sendRequest(request))
         .declarations;
   }
+
+  @override
+  Future<IntrospectableType> typeDeclarationOf(
+      IdentifierImpl identifier) async {
+    DeclarationOfRequest request = new DeclarationOfRequest(
+        identifier, MessageType.typeDeclarationOfRequest, remoteInstance,
+        serializationZoneId: serializationZoneId);
+    return _handleResponse<IntrospectableType>(await _sendRequest(request));
+  }
 }
 
 /// An exception that occurred remotely, the exception object and stack trace
@@ -803,5 +829,6 @@ enum MessageType {
   response,
   staticType,
   topLevelDeclarationsOfRequest,
+  typeDeclarationOfRequest,
   typesOfRequest,
 }

pkg/_fe_analyzer_shared/lib/src/macros/executor/protocol.dart

@@ -127,20 +127,20 @@ Future<void> checkClassDeclaration(ClassDeclaration declaration,
       TypeDeclaration? superclass = declaration.superclass == null
           ? null
           : await introspector
-              .declarationOf(declaration.superclass!.identifier);
+              .typeDeclarationOf(declaration.superclass!.identifier);
       expect(
           expected.superclass, superclass?.identifier.name, '$name.superclass');
       if (superclass is ClassDeclaration) {
         TypeDeclaration? superSuperclass = superclass.superclass == null
             ? null
             : await introspector
-                .declarationOf(superclass.superclass!.identifier);
+                .typeDeclarationOf(superclass.superclass!.identifier);
         expect(expected.superSuperclass, superSuperclass?.identifier.name,
             '$name.superSuperclass');
       }
       List<TypeDeclaration> mixins = [
         for (NamedTypeAnnotation mixin in declaration.mixins)
-          await introspector.declarationOf(mixin.identifier),
+          await introspector.typeDeclarationOf(mixin.identifier),
       ];
       expect(expected.mixins.length, mixins.length, '$name.mixins.length');
       for (int i = 0; i < mixins.length; i++) {
@@ -150,7 +150,7 @@ Future<void> checkClassDeclaration(ClassDeclaration declaration,
 
       List<TypeDeclaration> interfaces = [
         for (NamedTypeAnnotation interface in declaration.interfaces)
-          await introspector.declarationOf(interface.identifier),
+          await introspector.typeDeclarationOf(interface.identifier),
       ];
       expect(expected.interfaces.length, interfaces.length,
           '$name.interfaces.length');
@@ -263,13 +263,13 @@ Future<void> checkTypeDeclarationResolver(
       {bool expectThrows = false}) async {
     if (expectThrows) {
       await throws(() async {
-        await introspector.declarationOf(identifier);
+        await introspector.typeDeclarationOf(identifier);
       }, '$name from $identifier',
           expectedError: (e) => e is! ArgumentError
               ? 'Expected ArgumentError, got ${e.runtimeType}: $e'
               : null);
     } else {
-      TypeDeclaration result = await introspector.declarationOf(identifier);
+      TypeDeclaration result = await introspector.typeDeclarationOf(identifier);
       expect(name, result.identifier.name, '$name from $identifier');
     }
   }

pkg/_fe_analyzer_shared/test/macros/api/api_test_expectations.dart

@@ -107,7 +107,7 @@ class SimpleMacro
   @override
   FutureOr<void> buildDeclarationsForEnumValue(
       EnumValueDeclaration value, EnumDeclarationBuilder builder) async {
-    final parent = await builder.declarationOf(value.definingEnum);
+    final parent = await builder.typeDeclarationOf(value.definingEnum);
     builder.declareInType(DeclarationCode.fromParts([
       parent.identifier,
       ' ${value.identifier.name}ToString() => ',
@@ -256,7 +256,7 @@ class SimpleMacro
   @override
   FutureOr<void> buildDefinitionForEnumValue(
       EnumValueDeclaration value, EnumValueDefinitionBuilder builder) async {
-    final parent = await builder.declarationOf(value.definingEnum)
+    final parent = await builder.typeDeclarationOf(value.definingEnum)
         as IntrospectableEnumDeclaration;
     final constructor = (await builder.constructorsOf(parent)).first;
     final parts = [
@@ -302,8 +302,7 @@ class SimpleMacro
     await buildDefinitionForFunction(method, builder);
 
     // Test the type declaration resolver
-    var parentClass =
-        await builder.declarationOf(method.definingType) as IntrospectableType;
+    var parentClass = await builder.typeDeclarationOf(method.definingType);
     // Should be able to find ourself in the methods of the parent class.
     (await builder.methodsOf(parentClass))
         .singleWhere((m) => m.identifier == method.identifier);
@@ -315,22 +314,22 @@ class SimpleMacro
     // Test the class introspector
     if (parentClass is IntrospectableClassDeclaration) {
       superClass =
-          (await builder.declarationOf(parentClass.superclass!.identifier));
-      interfaces.addAll(await Future.wait(parentClass.interfaces
-          .map((interface) => builder.declarationOf(interface.identifier))));
+          (await builder.typeDeclarationOf(parentClass.superclass!.identifier));
+      interfaces.addAll(await Future.wait(parentClass.interfaces.map(
+          (interface) => builder.typeDeclarationOf(interface.identifier))));
       mixins.addAll(await Future.wait(parentClass.mixins
-          .map((mixins) => builder.declarationOf(mixins.identifier))));
+          .map((mixins) => builder.typeDeclarationOf(mixins.identifier))));
     } else if (parentClass is IntrospectableMixinDeclaration) {
-      superclassConstraints.addAll(await Future.wait(parentClass
-          .superclassConstraints
-          .map((interface) => builder.declarationOf(interface.identifier))));
-      interfaces.addAll(await Future.wait(parentClass.interfaces
-          .map((interface) => builder.declarationOf(interface.identifier))));
+      superclassConstraints.addAll(await Future.wait(
+          parentClass.superclassConstraints.map(
+              (interface) => builder.typeDeclarationOf(interface.identifier))));
+      interfaces.addAll(await Future.wait(parentClass.interfaces.map(
+          (interface) => builder.typeDeclarationOf(interface.identifier))));
     } else if (parentClass is IntrospectableEnumDeclaration) {
-      interfaces.addAll(await Future.wait(parentClass.interfaces
-          .map((interface) => builder.declarationOf(interface.identifier))));
+      interfaces.addAll(await Future.wait(parentClass.interfaces.map(
+          (interface) => builder.typeDeclarationOf(interface.identifier))));
       mixins.addAll(await Future.wait(parentClass.mixins
-          .map((mixins) => builder.declarationOf(mixins.identifier))));
+          .map((mixins) => builder.typeDeclarationOf(mixins.identifier))));
     }
     var fields = (await builder.fieldsOf(parentClass));
     var methods = (await builder.methodsOf(parentClass));

pkg/_fe_analyzer_shared/test/macros/executor/simple_macro.dart

@@ -29,7 +29,7 @@ class TestDeclarationPhaseIntrospector extends TestTypePhaseIntrospector
   final Map<IntrospectableType, List<MethodDeclaration>> methods;
   final Map<Library, List<TypeDeclaration>> libraryTypes;
   final Map<Identifier, StaticType> staticTypes;
-  final Map<Identifier, TypeDeclaration> typeDeclarations;
+  final Map<Identifier, Declaration> identifierDeclarations;
 
   TestDeclarationPhaseIntrospector(
       {required this.constructors,
@@ -38,12 +38,13 @@ class TestDeclarationPhaseIntrospector extends TestTypePhaseIntrospector
       required this.methods,
       required this.libraryTypes,
       required this.staticTypes,
-      required this.typeDeclarations});
+      required this.identifierDeclarations});
 
   @override
-  Future<TypeDeclaration> declarationOf(covariant Identifier identifier) async {
-    var declaration = typeDeclarations[identifier];
-    if (declaration != null) return declaration;
+  Future<TypeDeclaration> typeDeclarationOf(
+      covariant Identifier identifier) async {
+    var declaration = identifierDeclarations[identifier];
+    if (declaration != null) return declaration as TypeDeclaration;
     throw 'No declaration found for ${identifier.name}';
   }
 
@@ -113,17 +114,23 @@ class TestDefinitionsPhaseIntrospector extends TestDeclarationPhaseIntrospector
       required super.methods,
       required super.libraryTypes,
       required super.staticTypes,
-      required super.typeDeclarations});
+      required super.identifierDeclarations});
+  @override
+  Future<Declaration> declarationOf(Identifier identifier) async =>
+      identifierDeclarations[identifier]!;
 
   @override
   Future<TypeAnnotation> inferType(
           TestOmittedTypeAnnotation omittedType) async =>
       omittedType.inferredType!;
 
   @override
-  Future<List<Declaration>> topLevelDeclarationsOf(
-          covariant Library library) async =>
+  Future<List<Declaration>> topLevelDeclarationsOf(Library library) async =>
       libraryDeclarations[library]!;
+
+  @override
+  Future<IntrospectableType> typeDeclarationOf(Identifier identifier) async =>
+      (await super.typeDeclarationOf(identifier)) as IntrospectableType;
 }
 
 /// Knows its inferred type ahead of time.
@@ -493,7 +500,7 @@ class Fixtures {
       type: stringType,
       definingType: myClassType.identifier,
       isStatic: false);
-  static final myInterface = ClassDeclarationImpl(
+  static final myInterface = IntrospectableClassDeclarationImpl(
       id: RemoteInstance.uniqueId,
       identifier: myInterfaceType.identifier,
       library: Fixtures.library,
@@ -525,7 +532,7 @@ class Fixtures {
       typeParameters: [],
       definingType: myClassType.identifier,
       isStatic: false);
-  static final mySuperclass = ClassDeclarationImpl(
+  static final mySuperclass = IntrospectableClassDeclarationImpl(
       id: RemoteInstance.uniqueId,
       identifier: mySuperclassType.identifier,
       library: Fixtures.library,
@@ -648,12 +655,18 @@ class Fixtures {
     stringType.identifier:
         TestNamedStaticType(stringType.identifier, 'dart:core', []),
     myClass.identifier: myClassStaticType,
-  }, typeDeclarations: {
+  }, identifierDeclarations: {
     myClass.identifier: myClass,
     myEnum.identifier: myEnum,
     mySuperclass.identifier: mySuperclass,
     myInterface.identifier: myInterface,
-    myMixin.identifier: myMixin
+    myMixin.identifier: myMixin,
+    myConstructor.identifier: myConstructor,
+    myEnumConstructor.identifier: myEnumConstructor,
+    for (EnumValueDeclaration value in myEnumValues) value.identifier: value,
+    myField.identifier: myField,
+    myMixinMethod.identifier: myMixinMethod,
+    myMethod.identifier: myMethod,
   });
 
   static final testDefinitionPhaseIntrospector =
@@ -674,5 +687,6 @@ class Fixtures {
           },
           libraryTypes: testDeclarationPhaseIntrospector.libraryTypes,
           staticTypes: testDeclarationPhaseIntrospector.staticTypes,
-          typeDeclarations: testDeclarationPhaseIntrospector.typeDeclarations);
+          identifierDeclarations:
+              testDeclarationPhaseIntrospector.identifierDeclarations);
 }

pkg/_fe_analyzer_shared/test/macros/util.dart

@@ -498,18 +498,6 @@ class _DeclarationPhaseIntrospector extends _TypePhaseIntrospector
     throw UnimplementedError();
   }
 
-  @override
-  Future<macro.TypeDeclaration> declarationOf(
-    covariant IdentifierImpl identifier,
-  ) async {
-    final element = identifier.element;
-    if (element is ClassElementImpl) {
-      return declarationBuilder.fromElement.classElement(element);
-    } else {
-      throw ArgumentError('element: $element');
-    }
-  }
-
   @override
   Future<List<macro.FieldDeclaration>> fieldsOf(
     covariant macro.IntrospectableType type,
@@ -536,6 +524,18 @@ class _DeclarationPhaseIntrospector extends _TypePhaseIntrospector
     return _StaticTypeImpl(typeSystem, dartType);
   }
 
+  @override
+  Future<macro.TypeDeclaration> typeDeclarationOf(
+    covariant IdentifierImpl identifier,
+  ) async {
+    final element = identifier.element;
+    if (element is ClassElementImpl) {
+      return declarationBuilder.fromElement.classElement(element);
+    } else {
+      throw ArgumentError('element: $element');
+    }
+  }
+
   @override
   Future<List<macro.TypeDeclaration>> typesOf(covariant macro.Library library) {
     // TODO: implement typesOf