diff --git a/pkg/_macros/CHANGELOG.md b/pkg/_macros/CHANGELOG.md index 12a9300a245..0156f6a8fd0 100644 --- a/pkg/_macros/CHANGELOG.md +++ b/pkg/_macros/CHANGELOG.md @@ -1,3 +1,8 @@ +## 0.1.5 + +- Handle ParallelWaitError with DiagnosticException errors nicely. +- Fix a bug where we weren't reporting diagnostics for nested builders. + ## 0.1.4 - Improve formatting of constructor initializer augmentations. diff --git a/pkg/_macros/lib/src/executor/builder_impls.dart b/pkg/_macros/lib/src/executor/builder_impls.dart index eac681f80fc..013f118f100 100644 --- a/pkg/_macros/lib/src/executor/builder_impls.dart +++ b/pkg/_macros/lib/src/executor/builder_impls.dart @@ -12,7 +12,7 @@ import 'response_impls.dart'; abstract class TypeBuilderBase implements TypePhaseIntrospector, Builder { /// All the collected diagnostics for this builder. - final List _diagnostics = []; + final List _diagnostics; /// If execution was stopped by an exception, the exception. MacroExceptionImpl? _exception; @@ -60,7 +60,9 @@ abstract class TypeBuilderBase implements TypePhaseIntrospector, Builder { List? parentLibraryAugmentations, Map>? parentMixinAugmentations, Map>? parentTypeAugmentations, - }) : _enumValueAugmentations = parentEnumValueAugmentations ?? {}, + List? parentDiagnostics, + }) : _diagnostics = parentDiagnostics ?? [], + _enumValueAugmentations = parentEnumValueAugmentations ?? {}, _interfaceAugmentations = parentInterfaceAugmentations ?? {}, _libraryAugmentations = parentLibraryAugmentations ?? [], _mixinAugmentations = parentMixinAugmentations ?? {}, @@ -152,6 +154,7 @@ abstract class DeclarationBuilderBase extends TypeBuilderBase DeclarationPhaseIntrospector get introspector; DeclarationBuilderBase({ + super.parentDiagnostics, super.parentEnumValueAugmentations, super.parentInterfaceAugmentations, super.parentLibraryAugmentations, @@ -241,6 +244,7 @@ class DefinitionBuilderBase extends DeclarationBuilderBase DefinitionBuilderBase( this.introspector, { + super.parentDiagnostics, super.parentEnumValueAugmentations, super.parentInterfaceAugmentations, super.parentLibraryAugmentations, @@ -273,6 +277,7 @@ class TypeDefinitionBuilderImpl extends DefinitionBuilderBase TypeDefinitionBuilderImpl( this.declaration, super.introspector, { + super.parentDiagnostics, super.parentEnumValueAugmentations, super.parentInterfaceAugmentations, super.parentLibraryAugmentations, @@ -288,6 +293,7 @@ class TypeDefinitionBuilderImpl extends DefinitionBuilderBase .firstWhere((constructor) => constructor.identifier == identifier) as ConstructorDeclarationImpl; return ConstructorDefinitionBuilderImpl(constructor, introspector, + parentDiagnostics: _diagnostics, parentTypeAugmentations: _typeAugmentations, parentLibraryAugmentations: _libraryAugmentations); } @@ -297,6 +303,7 @@ class TypeDefinitionBuilderImpl extends DefinitionBuilderBase FieldDeclaration field = (await introspector.fieldsOf(declaration)) .firstWhere((field) => field.identifier == identifier); return VariableDefinitionBuilderImpl(field, introspector, + parentDiagnostics: _diagnostics, parentTypeAugmentations: _typeAugmentations, parentLibraryAugmentations: _libraryAugmentations); } @@ -307,6 +314,7 @@ class TypeDefinitionBuilderImpl extends DefinitionBuilderBase .firstWhere((method) => method.identifier == identifier) as MethodDeclarationImpl; return FunctionDefinitionBuilderImpl(method, introspector, + parentDiagnostics: _diagnostics, parentTypeAugmentations: _typeAugmentations, parentLibraryAugmentations: _libraryAugmentations); } @@ -336,6 +344,7 @@ class EnumDefinitionBuilderImpl extends TypeDefinitionBuilderImpl return EnumValueDefinitionBuilderImpl( entry, introspector, + parentDiagnostics: _diagnostics, parentEnumValueAugmentations: _enumValueAugmentations, parentInterfaceAugmentations: _interfaceAugmentations, parentLibraryAugmentations: _libraryAugmentations, @@ -352,6 +361,7 @@ class EnumValueDefinitionBuilderImpl extends DefinitionBuilderBase EnumValueDefinitionBuilderImpl( this.declaration, super.introspector, { + super.parentDiagnostics, super.parentEnumValueAugmentations, super.parentInterfaceAugmentations, super.parentLibraryAugmentations, @@ -375,6 +385,7 @@ class FunctionDefinitionBuilderImpl extends DefinitionBuilderBase FunctionDefinitionBuilderImpl( this.declaration, super.introspector, { + super.parentDiagnostics, super.parentEnumValueAugmentations, super.parentInterfaceAugmentations, super.parentLibraryAugmentations, @@ -404,6 +415,7 @@ class ConstructorDefinitionBuilderImpl extends DefinitionBuilderBase ConstructorDefinitionBuilderImpl( this.declaration, super.introspector, { + super.parentDiagnostics, super.parentEnumValueAugmentations, super.parentInterfaceAugmentations, super.parentLibraryAugmentations, @@ -436,6 +448,7 @@ class VariableDefinitionBuilderImpl extends DefinitionBuilderBase VariableDefinitionBuilderImpl( this.declaration, super.introspector, { + super.parentDiagnostics, super.parentEnumValueAugmentations, super.parentInterfaceAugmentations, super.parentLibraryAugmentations, @@ -473,6 +486,7 @@ class LibraryDefinitionBuilderImpl extends DefinitionBuilderBase LibraryDefinitionBuilderImpl( this.library, super.introspector, { + super.parentDiagnostics, super.parentEnumValueAugmentations, super.parentInterfaceAugmentations, super.parentLibraryAugmentations, @@ -487,6 +501,7 @@ class LibraryDefinitionBuilderImpl extends DefinitionBuilderBase .firstWhere((declaration) => declaration.identifier == identifier) as FunctionDeclarationImpl; return FunctionDefinitionBuilderImpl(function, introspector, + parentDiagnostics: _diagnostics, parentTypeAugmentations: _typeAugmentations, parentLibraryAugmentations: _libraryAugmentations); } @@ -497,6 +512,7 @@ class LibraryDefinitionBuilderImpl extends DefinitionBuilderBase .firstWhere((declaration) => declaration.identifier == identifier) as TypeDeclaration; return TypeDefinitionBuilderImpl(type, introspector, + parentDiagnostics: _diagnostics, parentTypeAugmentations: _typeAugmentations, parentLibraryAugmentations: _libraryAugmentations); } @@ -508,6 +524,7 @@ class LibraryDefinitionBuilderImpl extends DefinitionBuilderBase .firstWhere((declaration) => declaration.identifier == identifier) as VariableDeclarationImpl; return VariableDefinitionBuilderImpl(variable, introspector, + parentDiagnostics: _diagnostics, parentTypeAugmentations: _typeAugmentations, parentLibraryAugmentations: _libraryAugmentations); } diff --git a/pkg/_macros/lib/src/executor/execute_macro.dart b/pkg/_macros/lib/src/executor/execute_macro.dart index de9df195f37..e544e2931ea 100644 --- a/pkg/_macros/lib/src/executor/execute_macro.dart +++ b/pkg/_macros/lib/src/executor/execute_macro.dart @@ -2,6 +2,8 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. +import 'dart:async'; + import '../api.dart'; import '../executor.dart'; import 'builder_impls.dart'; @@ -61,15 +63,7 @@ Future executeTypesMacro( 'macro: $macro\ntarget: $target'); } } catch (e, s) { - if (e is DiagnosticException) { - builder.report(e.diagnostic); - } else if (e is MacroExceptionImpl) { - // Preserve `MacroException`s thrown by SDK tools. - builder.failWithException(e); - } else { - // Convert exceptions thrown by macro implementations into diagnostics. - builder.report(_unexpectedExceptionDiagnostic(e, s)); - } + _handleError(e, s, builder); } return builder.result; } @@ -141,15 +135,7 @@ Future executeDeclarationsMacro(Macro macro, 'macro: $macro\ntarget: $target'); } } catch (e, s) { - if (e is DiagnosticException) { - builder.report(e.diagnostic); - } else if (e is MacroExceptionImpl) { - // Preserve `MacroException`s thrown by SDK tools. - builder.failWithException(e); - } else { - // Convert exceptions thrown by macro implementations into diagnostics. - builder.report(_unexpectedExceptionDiagnostic(e, s)); - } + _handleError(e, s, builder); } return builder.result; } @@ -216,19 +202,136 @@ Future executeDefinitionMacro(Macro macro, Object target, 'macro: $macro\ntarget: $target'); } } catch (e, s) { - if (e is DiagnosticException) { - builder.report(e.diagnostic); - } else if (e is MacroExceptionImpl) { - // Preserve `MacroException`s thrown by SDK tools. - builder.failWithException(e); - } else { - // Convert exceptions thrown by macro implementations into diagnostics. - builder.report(_unexpectedExceptionDiagnostic(e, s)); - } + _handleError(e, s, builder); } return builder.result; } +/// Handles macro execution errors, specifically handling [DiagnosticException]s +/// and [MacroException]s in the expected ways. +/// +/// Also unwraps [ParallelWaitError]s and [AsyncError]s, such that we can +/// recognize properly the nested errors if they are of specially handled types. +void _handleError( + Object error, StackTrace stackTrace, TypeBuilderBase builder) { + switch (error) { + case ParallelWaitError(errors: List errors): + _handleErrors(errors, stackTrace, builder); + case ParallelWaitError(errors: (var e1,)): + _handleErrors([e1], stackTrace, builder); + case ParallelWaitError( + errors: ( + var e1, + var e2, + ) + ): + _handleErrors([e1, e2], stackTrace, builder); + case ParallelWaitError( + errors: ( + var e1, + var e2, + var e3, + ) + ): + _handleErrors([e1, e2, e3], stackTrace, builder); + case ParallelWaitError( + errors: ( + var e1, + var e2, + var e3, + var e4, + ) + ): + _handleErrors([e1, e2, e3, e4], stackTrace, builder); + case ParallelWaitError( + errors: ( + var e1, + var e2, + var e3, + var e4, + var e5, + ) + ): + _handleErrors([e1, e2, e3, e4, e5], stackTrace, builder); + case ParallelWaitError( + errors: ( + var e1, + var e2, + var e3, + var e4, + var e5, + var e6, + ) + ): + _handleErrors([e1, e2, e3, e4, e5, e6], stackTrace, builder); + case ParallelWaitError( + errors: ( + var e1, + var e2, + var e3, + var e4, + var e5, + var e6, + var e7, + ) + ): + _handleErrors([e1, e2, e3, e4, e5, e6, e7], stackTrace, builder); + case ParallelWaitError( + errors: ( + var e1, + var e2, + var e3, + var e4, + var e5, + var e6, + var e7, + var e8, + ) + ): + _handleErrors([e1, e2, e3, e4, e5, e6, e7, e8], stackTrace, builder); + case ParallelWaitError( + errors: ( + var e1, + var e2, + var e3, + var e4, + var e5, + var e6, + var e7, + var e8, + var e9, + ) + ): + _handleErrors([e1, e2, e3, e4, e5, e6, e7, e8, e9], stackTrace, builder); + // Unwrap async errors. + case AsyncError(): + _handleError(error.error, error.stackTrace, builder); + // Custom diagnostics from macros, these should just be reported. + case DiagnosticException(): + builder.report(error.diagnostic); + // Preserve `MacroException`s thrown by SDK tools. + case MacroExceptionImpl(): + builder.failWithException(error); + case _: + // Convert exceptions thrown by macro implementations into diagnostics. + builder.report(_unexpectedExceptionDiagnostic(error, stackTrace)); + } +} + +/// Handles a number of [errors], ignoring null values. +/// +/// This is used for parallel wait scenarios such as [Future.wait]. +void _handleErrors( + List errors, StackTrace outerStackTrace, TypeBuilderBase builder) { + for (var error in errors) { + if (error == null) continue; + // Passing the outerStackTrace here is the best we can do - but most of the + // time `error` will actually be an `AsyncError`, and we will end up using + // that stack trace anyways. + _handleError(error, outerStackTrace, builder); + } +} + // It's a bug in the macro but we need to show something to the user; put the // debug detail in a context message and suggest reporting to the author. Diagnostic _unexpectedExceptionDiagnostic( diff --git a/pkg/_macros/pubspec.yaml b/pkg/_macros/pubspec.yaml index 93b14280bb3..f6f14d76874 100644 --- a/pkg/_macros/pubspec.yaml +++ b/pkg/_macros/pubspec.yaml @@ -1,5 +1,5 @@ name: _macros -version: 0.1.4 +version: 0.1.5 description: >- This is a private SDK vendored package, which is re-exported by the public `macros` package, which is a pub package. Every change to this package is diff --git a/pkg/front_end/test/macros/application/data/tests/crash.dart.expect b/pkg/front_end/test/macros/application/data/tests/crash.dart.expect index 9521b232eed..5373a5b5326 100644 --- a/pkg/front_end/test/macros/application/data/tests/crash.dart.expect +++ b/pkg/front_end/test/macros/application/data/tests/crash.dart.expect @@ -7,7 +7,7 @@ library; // ^ // org-dartlang-test:///a/b/c/main.dart:7:2: Context: Error in buildTypesForClass // #0 CrashTypesMacro.buildTypesForClass (package:macro/crash.dart:13:5) -// #1 executeTypesMacro (package:_macros/src/executor/execute_macro.dart:37:21) +// #1 executeTypesMacro (package:_macros/src/executor/execute_macro.dart:39:21) // #2 MacroExpansionClient._executeTypesPhase (package:_macros/src/executor/client.dart:219:17) // #3 MacroExpansionClient._handleMessage. (package:_macros/src/executor/client.dart:145:18) // #4 _rootRun (dart:async/zone.dart:1399:13) @@ -33,7 +33,7 @@ library; // ^ // org-dartlang-test:///a/b/c/main.dart:8:2: Context: Error in buildDeclarationsForClass // #0 CrashDeclarationsMacro.buildDeclarationsForClass (package:macro/crash.dart:22:5) -// #1 executeDeclarationsMacro (package:_macros/src/executor/execute_macro.dart:113:21) +// #1 executeDeclarationsMacro (package:_macros/src/executor/execute_macro.dart:107:21) // #2 MacroExpansionClient._executeDeclarationsPhase (package:_macros/src/executor/client.dart:247:43) // #3 MacroExpansionClient._handleMessage. (package:_macros/src/executor/client.dart:135:18) // #4 _rootRun (dart:async/zone.dart:1399:13) @@ -59,7 +59,7 @@ library; // ^ // org-dartlang-test:///a/b/c/main.dart:9:2: Context: Error in buildDefinitionForClass // #0 CrashDefinitionMacro.buildDefinitionForClass (package:macro/crash.dart:31:5) -// #1 executeDefinitionMacro (package:_macros/src/executor/execute_macro.dart:181:21) +// #1 executeDefinitionMacro (package:_macros/src/executor/execute_macro.dart:167:21) // #2 MacroExpansionClient._executeDefinitionsPhase (package:_macros/src/executor/client.dart:276:17) // #3 MacroExpansionClient._handleMessage. (package:_macros/src/executor/client.dart:140:18) // #4 _rootRun (dart:async/zone.dart:1399:13) diff --git a/pkg/macros/CHANGELOG.md b/pkg/macros/CHANGELOG.md index 3b322f08054..5553ad873ba 100644 --- a/pkg/macros/CHANGELOG.md +++ b/pkg/macros/CHANGELOG.md @@ -1,3 +1,8 @@ +## 0.1.0-main.5 + +- Handle ParallelWaitError with DiagnosticException errors nicely. +- Fix a bug where we weren't reporting diagnostics for nested builders. + ## 0.1.0-main.4 - Improve formatting of constructor initializer augmentations. diff --git a/pkg/macros/pubspec.yaml b/pkg/macros/pubspec.yaml index 07d2dc269bb..6fdea15dc16 100644 --- a/pkg/macros/pubspec.yaml +++ b/pkg/macros/pubspec.yaml @@ -1,5 +1,5 @@ name: macros -version: 0.1.0-main.4 +version: 0.1.0-main.5 description: >- This package is for macro authors, and exposes the APIs necessary to write a macro. It exports the APIs from the private `_macros` SDK vendored package. @@ -11,4 +11,4 @@ environment: dependencies: _macros: sdk: dart - version: 0.1.4 + version: 0.1.5