From c854ac090fc7af21a1af2052172e096191fe3bdc Mon Sep 17 00:00:00 2001 From: David Morgan Date: Thu, 8 Aug 2024 14:45:59 +0200 Subject: [PATCH] Use package:_macro_host in _cfe_macros. --- .../lib/macro_implementation.dart | 16 +- pkgs/_analyzer_macros/test/analyzer_test.dart | 2 +- .../_cfe_macros/lib/macro_implementation.dart | 174 ++++++++++++++++++ pkgs/_cfe_macros/pubspec.yaml | 9 +- pkgs/_cfe_macros/test/cfe_test.dart | 116 ++---------- pubspec.yaml | 2 +- 6 files changed, 207 insertions(+), 112 deletions(-) create mode 100644 pkgs/_cfe_macros/lib/macro_implementation.dart diff --git a/pkgs/_analyzer_macros/lib/macro_implementation.dart b/pkgs/_analyzer_macros/lib/macro_implementation.dart index 2c999d0f..7cf7853c 100644 --- a/pkgs/_analyzer_macros/lib/macro_implementation.dart +++ b/pkgs/_analyzer_macros/lib/macro_implementation.dart @@ -12,12 +12,12 @@ import 'package:macros/macros.dart'; import 'package:macros/src/executor.dart' as injected; /// Injected macro implementation for the analyzer. -class MacroImplementation implements injected.MacroImplementation { +class AnalyzerMacroImplementation implements injected.MacroImplementation { final Uri packageConfig; final Map macroImplByName; final MacroHost _host; - MacroImplementation._(this.packageConfig, this.macroImplByName, this._host); + AnalyzerMacroImplementation._(this.packageConfig, this.macroImplByName, this._host); /// Starts the macro host. /// @@ -27,11 +27,11 @@ class MacroImplementation implements injected.MacroImplementation { /// [macroImplByName] identifies macros, it's a placeholder until we identify /// macros using package config. Keys are macro annotation qualified names /// (`uri#name`) and values are macro implementation qualified names. - static Future start({ + static Future start({ required Uri packageConfig, required Map macroImplByName, }) async => - MacroImplementation._( + AnalyzerMacroImplementation._( packageConfig, macroImplByName, await MacroHost.serve( @@ -54,7 +54,7 @@ class AnalyzerQueryService implements QueryService { } class AnalyzerMacroPackageConfigs implements injected.MacroPackageConfigs { - final MacroImplementation _impl; + final AnalyzerMacroImplementation _impl; AnalyzerMacroPackageConfigs(this._impl); @@ -64,7 +64,7 @@ class AnalyzerMacroPackageConfigs implements injected.MacroPackageConfigs { } class AnalyzerMacroRunner implements injected.MacroRunner { - final MacroImplementation _impl; + final AnalyzerMacroImplementation _impl; AnalyzerMacroRunner(this._impl); @@ -77,7 +77,7 @@ class AnalyzerMacroRunner implements injected.MacroRunner { } class AnalyzerRunningMacro implements injected.RunningMacro { - final MacroImplementation _impl; + final AnalyzerMacroImplementation _impl; final QualifiedName name; final QualifiedName implementation; late final Future _started; @@ -86,7 +86,7 @@ class AnalyzerRunningMacro implements injected.RunningMacro { // TODO(davidmorgan): should this be async, removing the need for `_started`? // If so the API injected into analyzer+CFE needs to change to be async. - static AnalyzerRunningMacro run(MacroImplementation impl, QualifiedName name, + static AnalyzerRunningMacro run(AnalyzerMacroImplementation impl, QualifiedName name, QualifiedName implementation) { final result = AnalyzerRunningMacro._(impl, name, implementation); // TODO(davidmorgan): this is currently what starts the macro running, diff --git a/pkgs/_analyzer_macros/test/analyzer_test.dart b/pkgs/_analyzer_macros/test/analyzer_test.dart index 9394cc22..8211d4d8 100644 --- a/pkgs/_analyzer_macros/test/analyzer_test.dart +++ b/pkgs/_analyzer_macros/test/analyzer_test.dart @@ -23,7 +23,7 @@ void main() { final contextCollection = AnalysisContextCollection(includedPaths: [directory.path]); analysisContext = contextCollection.contexts.first; - injected.macroImplementation = await MacroImplementation.start( + injected.macroImplementation = await AnalyzerMacroImplementation.start( packageConfig: Isolate.packageConfigSync!, macroImplByName: { 'package:_test_macros/declare_x_macro.dart#DeclareX': diff --git a/pkgs/_cfe_macros/lib/macro_implementation.dart b/pkgs/_cfe_macros/lib/macro_implementation.dart new file mode 100644 index 00000000..659ee0cc --- /dev/null +++ b/pkgs/_cfe_macros/lib/macro_implementation.dart @@ -0,0 +1,174 @@ +// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file +// 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 'package:_macro_host/macro_host.dart'; +import 'package:dart_model/dart_model.dart'; +// ignore: implementation_imports +import 'package:front_end/src/macros/macro_injected_impl.dart' as injected; +import 'package:macro_service/macro_service.dart'; +import 'package:macros/macros.dart'; +// ignore: implementation_imports +import 'package:macros/src/executor.dart' as injected; + +/// Injected macro implementation for the analyzer. +class CfeMacroImplementation implements injected.MacroImplementation { + final Uri packageConfig; + final Map macroImplByName; + final MacroHost _host; + + CfeMacroImplementation._(this.packageConfig, this.macroImplByName, this._host); + + /// Starts the macro host. + /// + /// [packageConfig] is the package config of the workspace of the code being + /// edited. + /// + /// [macroImplByName] identifies macros, it's a placeholder until we identify + /// macros using package config. Keys are macro annotation qualified names + /// (`uri#name`) and values are macro implementation qualified names. + static Future start({ + required Uri packageConfig, + required Map macroImplByName, + }) async => + CfeMacroImplementation._( + packageConfig, + macroImplByName, + await MacroHost.serve( + macroImplByName: macroImplByName, + queryService: CfeQueryService())); + + @override + injected.MacroPackageConfigs get packageConfigs => + CfeMacroPackageConfigs(this); + + @override + injected.MacroRunner get macroRunner => CfeMacroRunner(this); +} + +class CfeQueryService implements QueryService { + @override + Future handle(QueryRequest request) async { + return QueryResponse(); + } +} + +class CfeMacroPackageConfigs implements injected.MacroPackageConfigs { + final CfeMacroImplementation _impl; + + CfeMacroPackageConfigs(this._impl); + + @override + bool isMacro(Uri uri, String name) => + _impl._host.isMacro(_impl.packageConfig, QualifiedName('$uri#$name')); +} + +class CfeMacroRunner implements injected.MacroRunner { + final CfeMacroImplementation _impl; + + CfeMacroRunner(this._impl); + + @override + injected.RunningMacro run(Uri uri, String name) => CfeRunningMacro.run( + _impl, + QualifiedName('$uri#$name'), + // Look up from the macro name to its implementation. + QualifiedName(_impl.macroImplByName['$uri#$name']!)); +} + +class CfeRunningMacro implements injected.RunningMacro { + final CfeMacroImplementation _impl; + final QualifiedName name; + final QualifiedName implementation; + late final Future _started; + + CfeRunningMacro._(this._impl, this.name, this.implementation); + + // TODO(davidmorgan): should this be async, removing the need for `_started`? + // If so the API injected into analyzer+CFE needs to change to be async. + static CfeRunningMacro run(CfeMacroImplementation impl, QualifiedName name, + QualifiedName implementation) { + final result = CfeRunningMacro._(impl, name, implementation); + // TODO(davidmorgan): this is currently what starts the macro running, + // make it explicit. + result._started = + impl._host.queryMacroPhases(impl.packageConfig, implementation); + return result; + } + + @override + Future executeDeclarationsPhase(MacroTarget target, + DeclarationPhaseIntrospector declarationsPhaseIntrospector) async { + await _started; + return CfeMacroExecutionResult( + target, await _impl._host.augment(name, AugmentRequest(phase: 2))); + } + + @override + Future executeDefinitionsPhase(MacroTarget target, + DefinitionPhaseIntrospector definitionPhaseIntrospector) async { + await _started; + return CfeMacroExecutionResult( + target, await _impl._host.augment(name, AugmentRequest(phase: 3))); + } + + @override + Future executeTypesPhase( + MacroTarget target, TypePhaseIntrospector typePhaseIntrospector) async { + await _started; + return CfeMacroExecutionResult( + target, await _impl._host.augment(name, AugmentRequest(phase: 1))); + } +} + +/// Converts [AugmentResponse] to [injected.MacroExecutionResult]. +/// +/// TODO(davidmorgan): add to `AugmentationResponse` to cover all the +/// functionality of `MacroExecutionResult`. +class CfeMacroExecutionResult implements injected.MacroExecutionResult { + final MacroTarget target; + final AugmentResponse augmentResponse; + + CfeMacroExecutionResult(this.target, this.augmentResponse); + + @override + List get diagnostics => []; + + @override + Map> get enumValueAugmentations => {}; + + @override + MacroException? get exception => null; + + @override + Map get extendsTypeAugmentations => {}; + + @override + Map> get interfaceAugmentations => + {}; + + @override + Iterable get libraryAugmentations => {}; + + @override + Map> get mixinAugmentations => {}; + + @override + Iterable get newTypeNames => []; + + @override + void serialize(Object serializer) => throw UnimplementedError(); + + @override + Map> get typeAugmentations => { + // TODO(davidmorgan): this assumes augmentations are for the macro + // application target. Instead, it should be explicit in + // `AugmentResponse`. + // TODO(davidmorgan): empty augmentations response breaks the test, + // it's not clear why. + if (augmentResponse.augmentations.isNotEmpty) + (target as Declaration).identifier: augmentResponse.augmentations + .map((a) => DeclarationCode.fromParts([a.code])) + .toList(), + }; +} diff --git a/pkgs/_cfe_macros/pubspec.yaml b/pkgs/_cfe_macros/pubspec.yaml index 193a1e68..2030efd7 100644 --- a/pkgs/_cfe_macros/pubspec.yaml +++ b/pkgs/_cfe_macros/pubspec.yaml @@ -6,9 +6,14 @@ resolution: workspace environment: sdk: ^3.6.0-114.0.dev +dependencies: + _macro_host: any + dart_model: any + front_end: any + macro_service: any + macros: any + dev_dependencies: dart_flutter_team_lints: ^3.0.0 - front_end: any frontend_server: any - macros: ^0.1.2-main.4 test: ^1.25.0 diff --git a/pkgs/_cfe_macros/test/cfe_test.dart b/pkgs/_cfe_macros/test/cfe_test.dart index d60ebb7a..305e5cfd 100644 --- a/pkgs/_cfe_macros/test/cfe_test.dart +++ b/pkgs/_cfe_macros/test/cfe_test.dart @@ -5,20 +5,17 @@ import 'dart:io'; import 'dart:isolate'; +import 'package:_cfe_macros/macro_implementation.dart'; import 'package:front_end/src/macros/macro_injected_impl.dart' as injected; import 'package:frontend_server/compute_kernel.dart'; -import 'package:macros/macros.dart'; -import 'package:macros/src/executor.dart'; import 'package:test/test.dart'; void main() { group('CFE with injected macro implementation', () { late File productPlatformDill; late Directory tempDir; - late TestMacroPackageConfigs packageConfigs; - late TestMacroRunner runner; - setUp(() { + setUp(() async { // Set up CFE. // TODO(davidmorgan): this dill comes from the Dart SDK running the test, @@ -32,12 +29,20 @@ void main() { tempDir = Directory.systemTemp.createTempSync('cfe_test'); // Inject test macro implementation. - packageConfigs = TestMacroPackageConfigs(); - runner = TestMacroRunner(); - injected.macroImplementation = injected.MacroImplementation( - packageConfigs: packageConfigs, - macroRunner: runner, - ); + // TODO(davidmorgan): the CFE passes us a mix of absolute file paths and + // package URIs, fix this properly. + final macroFileUri = Directory.current.uri + .resolve('../_test_macros/lib/declare_x_macro.dart'); + injected.macroImplementation = await CfeMacroImplementation.start( + packageConfig: Isolate.packageConfigSync!, + macroImplByName: { + '$macroFileUri#DeclareX': + 'package:_test_macros/declare_x_macro.dart' + '#DeclareXImplementation', + 'package:_test_macros/declare_x_macro.dart#DeclareX': + 'package:_test_macros/declare_x_macro.dart' + '#DeclareXImplementation' + }); }); test('discovers macros, runs them, applies augmentations', () async { @@ -59,98 +64,9 @@ void main() { '--enable-experiment=macros', ]); - expect(packageConfigs.macroWasFound, true); - expect(runner.macroWasRun, true); - // The source has a main method that uses the new declaration, so the // compile can only succeed if it was added by the macro. expect(computeKernelResult.succeeded, true); }); }); } - -class TestMacroPackageConfigs implements injected.MacroPackageConfigs { - bool macroWasFound = false; - - @override - bool isMacro(Uri uri, String name) { - final result = - uri.toString().endsWith('_test_macros/lib/declare_x_macro.dart') && - name == 'DeclareX'; - if (result == true) { - macroWasFound = true; - } - return result; - } -} - -class TestMacroRunner implements injected.MacroRunner { - bool macroWasRun = false; - - @override - injected.RunningMacro run(Uri uri, String name) { - macroWasRun = true; - return TestRunningMacro(); - } -} - -class TestRunningMacro implements injected.RunningMacro { - @override - Future executeDeclarationsPhase(MacroTarget target, - DeclarationPhaseIntrospector declarationsPhaseIntrospector) async { - return TestMacroExecutionResult(typeAugmentations: { - (target as Declaration).identifier: [ - DeclarationCode.fromParts(['int get x => 3;']) - ], - }); - } - - @override - Future executeDefinitionsPhase(MacroTarget target, - DefinitionPhaseIntrospector definitionPhaseIntrospector) async { - return TestMacroExecutionResult(); - } - - @override - Future executeTypesPhase( - MacroTarget target, TypePhaseIntrospector typePhaseIntrospector) async { - return TestMacroExecutionResult(); - } -} - -class TestMacroExecutionResult implements MacroExecutionResult { - @override - List get diagnostics => []; - - @override - Map> get enumValueAugmentations => {}; - - @override - MacroException? get exception => null; - - @override - Map get extendsTypeAugmentations => {}; - - @override - Map> get interfaceAugmentations => - {}; - - @override - Iterable get libraryAugmentations => {}; - - @override - Map> get mixinAugmentations => {}; - - @override - Iterable get newTypeNames => []; - - @override - void serialize(Object serializer) => throw UnimplementedError(); - - @override - Map> typeAugmentations; - - TestMacroExecutionResult( - {Map>? typeAugmentations}) - : typeAugmentations = typeAugmentations ?? {}; -} diff --git a/pubspec.yaml b/pubspec.yaml index 895b9d05..70b63b35 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -76,7 +76,7 @@ dependency_overrides: git: url: https://github.com/dart-lang/sdk.git path: pkg/front_end - ref: 3.6.0-114.0.dev + ref: f55692ab753621eb355f380e83f37e63c4a1c8e0 frontend_server: git: url: https://github.com/dart-lang/sdk.git