Macro. Report MACRO_DEFINITION_APPLICATION_SAME_LIBRARY_CYCLE.

Bug: #54915
Change-Id: I0eb2d56b9fc4ad95c41de5681e22d55923845632
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/354382
Commit-Queue: Konstantin Shcheglov <[email protected]>
Reviewed-by: Phil Quitslund <[email protected]>

Author: scheglov

pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml

@@ -1034,6 +1034,8 @@ CompileTimeErrorCode.LIST_ELEMENT_TYPE_NOT_ASSIGNABLE:
   status: noFix
 CompileTimeErrorCode.MACRO_DECLARATIONS_PHASE_INTROSPECTION_CYCLE:
   status: needsEvaluation
+CompileTimeErrorCode.MACRO_DEFINITION_APPLICATION_SAME_LIBRARY_CYCLE:
+  status: noFix
 CompileTimeErrorCode.MACRO_APPLICATION_ARGUMENT_ERROR:
   status: noFix
 CompileTimeErrorCode.MACRO_ERROR:

pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart

@@ -85,6 +85,7 @@ class LibraryAnalyzer {
       : _testingData = testingData,
         _typeSystemOperations = typeSystemOperations {
     _libraryVerificationContext = LibraryVerificationContext(
+      libraryKind: _library,
       constructorFieldsVerifier: ConstructorFieldsVerifier(
         typeSystem: _typeSystem,
       ),

pkg/analyzer/lib/src/dart/analysis/library_graph.dart

@@ -26,6 +26,9 @@ class LibraryCycle {
   /// The libraries that belong to this cycle.
   final List<LibraryFileKind> libraries;
 
+  /// The URIs of [libraries].
+  final Set<Uri> libraryUris;
+
   /// The library cycles that this cycle references directly.
   final Set<LibraryCycle> directDependencies;
 
@@ -89,6 +92,7 @@ class LibraryCycle {
 
   LibraryCycle({
     required this.libraries,
+    required this.libraryUris,
     required this.directDependencies,
     required this.apiSignature,
     required this.implSignature,
@@ -212,9 +216,11 @@ class _LibraryWalker extends graph.DependencyWalker<_LibraryNode> {
 
     // Fill the cycle with libraries.
     var libraries = <LibraryFileKind>[];
+    var libraryUris = <Uri>{};
     for (var node in scc) {
       final file = node.kind.file;
       libraries.add(node.kind);
+      libraryUris.add(file.uri);
 
       apiSignature.addLanguageVersion(file.packageLanguageVersion);
       apiSignature.addString(file.uriStr);
@@ -241,6 +247,7 @@ class _LibraryWalker extends graph.DependencyWalker<_LibraryNode> {
     // Create the LibraryCycle instance for the cycle.
     var cycle = LibraryCycle(
       libraries: libraries.toFixedList(),
+      libraryUris: libraryUris,
       directDependencies: directDependencies,
       apiSignature: apiSignature.toHex(),
       implSignature: implSignature.toHex(),

pkg/analyzer/lib/src/error/codes.g.dart

@@ -3055,6 +3055,18 @@ class CompileTimeErrorCode extends AnalyzerErrorCode {
         "Try removing one or more macro applications to break the cycle.",
   );
 
+  ///  Parameters:
+  ///  0: the name of the macro class
+  static const CompileTimeErrorCode
+      MACRO_DEFINITION_APPLICATION_SAME_LIBRARY_CYCLE = CompileTimeErrorCode(
+    'MACRO_DEFINITION_APPLICATION_SAME_LIBRARY_CYCLE',
+    "The macro '{0}' can't be applied in the same library cycle where it is "
+        "defined.",
+    correctionMessage:
+        "Try moving it to a different library that does not import the one "
+        "where it is applied.",
+  );
+
   ///  Reported when the macro uses `Builder.report()` with `Severity.error`.
   ///  Parameters:
   ///  0: the message

pkg/analyzer/lib/src/error/error_code_values.g.dart

@@ -325,6 +325,7 @@ const List<ErrorCode> errorCodeValues = [
   CompileTimeErrorCode.LIST_ELEMENT_TYPE_NOT_ASSIGNABLE,
   CompileTimeErrorCode.MACRO_APPLICATION_ARGUMENT_ERROR,
   CompileTimeErrorCode.MACRO_DECLARATIONS_PHASE_INTROSPECTION_CYCLE,
+  CompileTimeErrorCode.MACRO_DEFINITION_APPLICATION_SAME_LIBRARY_CYCLE,
   CompileTimeErrorCode.MACRO_ERROR,
   CompileTimeErrorCode.MACRO_INTERNAL_EXCEPTION,
   CompileTimeErrorCode.MAIN_FIRST_POSITIONAL_PARAMETER_TYPE,

pkg/analyzer/lib/src/generated/error_verifier.dart

@@ -54,6 +54,7 @@ import 'package:analyzer/src/generated/parser.dart' show ParserErrorCode;
 import 'package:analyzer/src/generated/this_access_tracker.dart';
 import 'package:analyzer/src/summary2/macro_application_error.dart';
 import 'package:analyzer/src/summary2/macro_type_location.dart';
+import 'package:analyzer/src/utilities/extensions/element.dart';
 import 'package:analyzer/src/utilities/extensions/object.dart';
 import 'package:analyzer/src/utilities/extensions/string.dart';
 import 'package:collection/collection.dart';
@@ -6113,10 +6114,12 @@ class HiddenElements {
 /// Information to pass from from the defining unit to augmentations.
 class LibraryVerificationContext {
   final duplicationDefinitionContext = DuplicationDefinitionContext();
+  final LibraryFileKind libraryKind;
   final ConstructorFieldsVerifier constructorFieldsVerifier;
   final Map<FileState, UnitAnalysis> units;
 
   LibraryVerificationContext({
+    required this.libraryKind,
     required this.constructorFieldsVerifier,
     required this.units,
   });
@@ -6148,6 +6151,10 @@ class LibraryVerificationContext {
       entity: node,
     );
   }
+
+  bool libraryCycleContains(Uri uri) {
+    return libraryKind.libraryCycle.libraryUris.contains(uri);
+  }
 }
 
 class _MacroDiagnosticsReporter {
@@ -6162,6 +6169,8 @@ class _MacroDiagnosticsReporter {
   });
 
   void report() {
+    _reportApplicationFromSameLibraryCycle();
+
     for (final diagnostic in element.macroDiagnostics) {
       switch (diagnostic) {
         case ArgumentMacroDiagnostic():
@@ -6182,7 +6191,7 @@ class _MacroDiagnosticsReporter {
     final target = object.target;
     switch (target) {
       case ApplicationMacroDiagnosticTarget():
-        final node = _annotationNode(element, target.annotationIndex);
+        final node = element.annotationAst(target.annotationIndex);
         return DiagnosticMessageImpl(
           filePath: element.source!.fullName,
           length: node.length,
@@ -6208,8 +6217,30 @@ class _MacroDiagnosticsReporter {
     }
   }
 
+  void _reportApplicationFromSameLibraryCycle() {
+    for (var annotation in element.metadata) {
+      var element = annotation.element;
+      if (element is! ConstructorElementImpl) continue;
+
+      var macroElement = element.enclosingElement;
+      if (macroElement is! ClassElementImpl) continue;
+      if (!macroElement.isMacro) continue;
+
+      var macroUri = macroElement.library.source.uri;
+      if (!libraryContext.libraryCycleContains(macroUri)) continue;
+
+      errorReporter.atNode(
+        _annotationNameIdentifier(annotation),
+        CompileTimeErrorCode.MACRO_DEFINITION_APPLICATION_SAME_LIBRARY_CYCLE,
+        arguments: [
+          macroElement.name,
+        ],
+      );
+    }
+  }
+
   void _reportArgument(ArgumentMacroDiagnostic diagnostic) {
-    var annotation = _annotationNode(element, diagnostic.annotationIndex);
+    var annotation = element.annotationAst(diagnostic.annotationIndex);
     var arguments = annotation.arguments!.arguments;
     errorReporter.atNode(
       arguments[diagnostic.argumentIndex],
@@ -6231,7 +6262,7 @@ class _MacroDiagnosticsReporter {
     final target = diagnostic.message.target;
     switch (target) {
       case ApplicationMacroDiagnosticTarget():
-        var node = _annotationNode(element, target.annotationIndex);
+        var node = element.annotationAst(target.annotationIndex);
         errorReporter.reportError(
           AnalysisError.forValues(
             source: element.source!,
@@ -6262,10 +6293,7 @@ class _MacroDiagnosticsReporter {
         if (location == null) {
           return;
         }
-        var node = _annotationNode(
-          target.element,
-          target.annotationIndex,
-        );
+        var node = target.element.annotationAst(target.annotationIndex);
         location.unitAnalysis.errorReporter.reportError(
           AnalysisError.forValues(
             source: target.element.source!,
@@ -6301,7 +6329,7 @@ class _MacroDiagnosticsReporter {
 
   void _reportException(ExceptionMacroDiagnostic diagnostic) {
     errorReporter.atNode(
-      _annotationNode(element, diagnostic.annotationIndex),
+      element.annotationAst(diagnostic.annotationIndex),
       CompileTimeErrorCode.MACRO_INTERNAL_EXCEPTION,
       arguments: [
         diagnostic.message,
@@ -6343,24 +6371,30 @@ class _MacroDiagnosticsReporter {
 
   void _reportInvalidTarget(InvalidMacroTargetDiagnostic diagnostic) {
     errorReporter.atNode(
-      _annotationNode(element, diagnostic.annotationIndex),
+      element.annotationAst(diagnostic.annotationIndex),
       CompileTimeErrorCode.INVALID_MACRO_APPLICATION_TARGET,
       arguments: [
         diagnostic.supportedKinds.commaSeparatedWithOr,
       ],
     );
   }
 
-  static AnnotationImpl _annotationNode(ElementImpl element, int index) {
-    var annotation = element.metadata[index];
-    return annotation.annotationAst;
+  static SimpleIdentifier _annotationNameIdentifier(
+    ElementAnnotationImpl annotation,
+  ) {
+    var fullName = annotation.annotationAst.name;
+    if (fullName is PrefixedIdentifierImpl) {
+      return fullName.identifier;
+    } else {
+      return fullName as SimpleIdentifierImpl;
+    }
   }
 
   static SimpleIdentifier _macroAnnotationNameIdentifier({
     required ElementImpl element,
     required int annotationIndex,
   }) {
-    var annotationNode = _annotationNode(element, annotationIndex);
+    var annotationNode = element.annotationAst(annotationIndex);
     var fullName = annotationNode.name;
     if (fullName is PrefixedIdentifierImpl) {
       return fullName.identifier;

pkg/analyzer/lib/src/utilities/extensions/element.dart

@@ -5,6 +5,7 @@
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/dart/element/nullability_suffix.dart';
 import 'package:analyzer/dart/element/type.dart';
+import 'package:analyzer/src/dart/ast/ast.dart';
 import 'package:analyzer/src/dart/element/element.dart';
 
 extension ElementExtension on Element {
@@ -27,6 +28,12 @@ extension ElementExtension on Element {
   }
 }
 
+extension ElementImplExtension on ElementImpl {
+  AnnotationImpl annotationAst(int index) {
+    return metadata[index].annotationAst;
+  }
+}
+
 extension ListOfTypeParameterElementExtension on List<TypeParameterElement> {
   List<TypeParameterType> instantiateNone() {
     return map((e) {

pkg/analyzer/messages.yaml

@@ -9550,6 +9550,12 @@ CompileTimeErrorCode:
     comment: |-
       Parameters:
       0: the name of the introspected declaration
+  MACRO_DEFINITION_APPLICATION_SAME_LIBRARY_CYCLE:
+    problemMessage: "The macro '{0}' can't be applied in the same library cycle where it is defined."
+    correctionMessage: Try moving it to a different library that does not import the one where it is applied.
+    comment: |-
+      Parameters:
+      0: the name of the macro class
   MAIN_FIRST_POSITIONAL_PARAMETER_TYPE:
     problemMessage: "The type of the first positional parameter of the 'main' function must be a supertype of 'List<String>'."
     correctionMessage: Try changing the type of the parameter.

pkg/analyzer/test/src/dart/resolution/macro_test.dart

@@ -248,6 +248,53 @@ class A3 {}
     ]);
   }
 
+  test_diagnostic_definitionApplication_sameLibrary() async {
+    await assertErrorsInCode('''
+import 'package:_fe_analyzer_shared/src/macros/api.dart';
+
+macro class MyMacro implements ClassDefinitionMacro {
+  const MyMacro();
+
+  @override
+  buildDefinitionForClass(declaration, builder) async {}
+}
+
+@MyMacro()
+class A {}
+''', [
+      error(
+          CompileTimeErrorCode.MACRO_DEFINITION_APPLICATION_SAME_LIBRARY_CYCLE,
+          206,
+          7),
+    ]);
+  }
+
+  test_diagnostic_definitionApplication_sameLibraryCycle() async {
+    newFile('$testPackageLibPath/a.dart', r'''
+import 'package:_fe_analyzer_shared/src/macros/api.dart';
+import 'test.dart';
+
+macro class MyMacro implements ClassDefinitionMacro {
+  const MyMacro();
+
+  @override
+  buildDefinitionForClass(declaration, builder) async {}
+}
+''');
+
+    await assertErrorsInCode('''
+import 'a.dart';
+
+@MyMacro()
+class A {}
+''', [
+      error(
+          CompileTimeErrorCode.MACRO_DEFINITION_APPLICATION_SAME_LIBRARY_CYCLE,
+          19,
+          7),
+    ]);
+  }
+
   test_diagnostic_invalidTarget_wantsClassOrMixin_hasFunction() async {
     await assertErrorsInCode('''
 import 'diagnostic.dart';