Handle diagnostics from nested builders, as well as ParallelWaitErrors with diagnostic exceptions and AsyncErrors. This fixes diagnostic reporting in the updated JSON macro which uses Future.wait and Record.wait, as well as nested builders. Change-Id: I0f80ae7f93b1ecaf24ed98512cd8a4f86e6b125a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/365840 Commit-Queue: Leaf Petersen <leafp@google.com> Auto-Submit: Jake Macdonald <jakemac@google.com> Reviewed-by: Bob Nystrom <rnystrom@google.com> Commit-Queue: Jake Macdonald <jakemac@google.com> Reviewed-by: Leaf Petersen <leafp@google.com>
diff --git a/pkg/_macros/CHANGELOG.md b/pkg/_macros/CHANGELOG.md index 12a9300..0156f6a 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 eac681f..013f118 100644 --- a/pkg/_macros/lib/src/executor/builder_impls.dart +++ b/pkg/_macros/lib/src/executor/builder_impls.dart
@@ -12,7 +12,7 @@ abstract class TypeBuilderBase implements TypePhaseIntrospector, Builder { /// All the collected diagnostics for this builder. - final List<Diagnostic> _diagnostics = []; + final List<Diagnostic> _diagnostics; /// If execution was stopped by an exception, the exception. MacroExceptionImpl? _exception; @@ -60,7 +60,9 @@ List<DeclarationCode>? parentLibraryAugmentations, Map<IdentifierImpl, List<TypeAnnotationCode>>? parentMixinAugmentations, Map<IdentifierImpl, List<DeclarationCode>>? parentTypeAugmentations, - }) : _enumValueAugmentations = parentEnumValueAugmentations ?? {}, + List<Diagnostic>? parentDiagnostics, + }) : _diagnostics = parentDiagnostics ?? [], + _enumValueAugmentations = parentEnumValueAugmentations ?? {}, _interfaceAugmentations = parentInterfaceAugmentations ?? {}, _libraryAugmentations = parentLibraryAugmentations ?? [], _mixinAugmentations = parentMixinAugmentations ?? {}, @@ -152,6 +154,7 @@ DeclarationPhaseIntrospector get introspector; DeclarationBuilderBase({ + super.parentDiagnostics, super.parentEnumValueAugmentations, super.parentInterfaceAugmentations, super.parentLibraryAugmentations, @@ -241,6 +244,7 @@ DefinitionBuilderBase( this.introspector, { + super.parentDiagnostics, super.parentEnumValueAugmentations, super.parentInterfaceAugmentations, super.parentLibraryAugmentations, @@ -273,6 +277,7 @@ TypeDefinitionBuilderImpl( this.declaration, super.introspector, { + super.parentDiagnostics, super.parentEnumValueAugmentations, super.parentInterfaceAugmentations, super.parentLibraryAugmentations, @@ -288,6 +293,7 @@ .firstWhere((constructor) => constructor.identifier == identifier) as ConstructorDeclarationImpl; return ConstructorDefinitionBuilderImpl(constructor, introspector, + parentDiagnostics: _diagnostics, parentTypeAugmentations: _typeAugmentations, parentLibraryAugmentations: _libraryAugmentations); } @@ -297,6 +303,7 @@ 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 @@ .firstWhere((method) => method.identifier == identifier) as MethodDeclarationImpl; return FunctionDefinitionBuilderImpl(method, introspector, + parentDiagnostics: _diagnostics, parentTypeAugmentations: _typeAugmentations, parentLibraryAugmentations: _libraryAugmentations); } @@ -336,6 +344,7 @@ return EnumValueDefinitionBuilderImpl( entry, introspector, + parentDiagnostics: _diagnostics, parentEnumValueAugmentations: _enumValueAugmentations, parentInterfaceAugmentations: _interfaceAugmentations, parentLibraryAugmentations: _libraryAugmentations, @@ -352,6 +361,7 @@ EnumValueDefinitionBuilderImpl( this.declaration, super.introspector, { + super.parentDiagnostics, super.parentEnumValueAugmentations, super.parentInterfaceAugmentations, super.parentLibraryAugmentations, @@ -375,6 +385,7 @@ FunctionDefinitionBuilderImpl( this.declaration, super.introspector, { + super.parentDiagnostics, super.parentEnumValueAugmentations, super.parentInterfaceAugmentations, super.parentLibraryAugmentations, @@ -404,6 +415,7 @@ ConstructorDefinitionBuilderImpl( this.declaration, super.introspector, { + super.parentDiagnostics, super.parentEnumValueAugmentations, super.parentInterfaceAugmentations, super.parentLibraryAugmentations, @@ -436,6 +448,7 @@ VariableDefinitionBuilderImpl( this.declaration, super.introspector, { + super.parentDiagnostics, super.parentEnumValueAugmentations, super.parentInterfaceAugmentations, super.parentLibraryAugmentations, @@ -473,6 +486,7 @@ LibraryDefinitionBuilderImpl( this.library, super.introspector, { + super.parentDiagnostics, super.parentEnumValueAugmentations, super.parentInterfaceAugmentations, super.parentLibraryAugmentations, @@ -487,6 +501,7 @@ .firstWhere((declaration) => declaration.identifier == identifier) as FunctionDeclarationImpl; return FunctionDefinitionBuilderImpl(function, introspector, + parentDiagnostics: _diagnostics, parentTypeAugmentations: _typeAugmentations, parentLibraryAugmentations: _libraryAugmentations); } @@ -497,6 +512,7 @@ .firstWhere((declaration) => declaration.identifier == identifier) as TypeDeclaration; return TypeDefinitionBuilderImpl(type, introspector, + parentDiagnostics: _diagnostics, parentTypeAugmentations: _typeAugmentations, parentLibraryAugmentations: _libraryAugmentations); } @@ -508,6 +524,7 @@ .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 de9df19..e544e29 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 @@ '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 @@ '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 @@ '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<Object?> 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<Object?> 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 93b1428..f6f14d7 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 9521b23..5373a5b 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 @@ // ^ // 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.<anonymous closure> (package:_macros/src/executor/client.dart:145:18) // #4 _rootRun (dart:async/zone.dart:1399:13) @@ -33,7 +33,7 @@ // ^ // 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.<anonymous closure> (package:_macros/src/executor/client.dart:135:18) // #4 _rootRun (dart:async/zone.dart:1399:13) @@ -59,7 +59,7 @@ // ^ // 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.<anonymous closure> (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 3b322f0..5553ad8 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 07d2dc2..6fdea15 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 @@ dependencies: _macros: sdk: dart - version: 0.1.4 + version: 0.1.5