From 28643f3fc148a60e6d91bffac2cf6e1c4e226458 Mon Sep 17 00:00:00 2001 From: David Morgan Date: Mon, 11 Nov 2024 17:06:17 +0100 Subject: [PATCH] Fix "without macros" benchmark: notify analyzer of new files. (#138) --- .../lib/analyzer_macro_runner.dart | 4 +-- pkgs/_macro_tool/lib/cfe_macro_runner.dart | 6 ++-- pkgs/_macro_tool/lib/macro_runner.dart | 2 +- pkgs/_macro_tool/lib/macro_tool.dart | 15 ++++---- pkgs/_macro_tool/lib/source_file.dart | 36 +++++++++++++++---- 5 files changed, 41 insertions(+), 22 deletions(-) diff --git a/pkgs/_macro_tool/lib/analyzer_macro_runner.dart b/pkgs/_macro_tool/lib/analyzer_macro_runner.dart index 2117681d..f6a543b1 100644 --- a/pkgs/_macro_tool/lib/analyzer_macro_runner.dart +++ b/pkgs/_macro_tool/lib/analyzer_macro_runner.dart @@ -32,8 +32,8 @@ class AnalyzerMacroRunner implements MacroRunner { analysisContext = contextCollection.contexts.single; } - void notifyChange(SourceFile sourceFile) { - analysisContext.changeFile(sourceFile.path); + void notifyChange(String sourcePath) { + analysisContext.changeFile(sourcePath); } @override diff --git a/pkgs/_macro_tool/lib/cfe_macro_runner.dart b/pkgs/_macro_tool/lib/cfe_macro_runner.dart index ab5d4223..1136131e 100644 --- a/pkgs/_macro_tool/lib/cfe_macro_runner.dart +++ b/pkgs/_macro_tool/lib/cfe_macro_runner.dart @@ -25,9 +25,8 @@ class CfeMacroRunner implements MacroRunner { CfeMacroRunner({required this.workspacePath, required this.packageConfigPath}) : sourceFiles = SourceFile.findDartInWorkspace(workspacePath); - void notifyChange(SourceFile sourceFile) { - throw UnimplementedError( - 'CfeMacroRunner does not support incremental compilation.'); + void notifyChange(String sourcePath) { + // No incremental compile. } File get _productPlatformDill { @@ -66,6 +65,7 @@ class CfeMacroRunner implements MacroRunner { final packagesUri = Uri.file(packageConfigPath); final computeKernelResult = await computeKernel([ + '--enable-experiment=macros', '--no-summary', '--no-summary-only', '--target=vm', diff --git a/pkgs/_macro_tool/lib/macro_runner.dart b/pkgs/_macro_tool/lib/macro_runner.dart index 27d841f6..682908c2 100644 --- a/pkgs/_macro_tool/lib/macro_runner.dart +++ b/pkgs/_macro_tool/lib/macro_runner.dart @@ -20,7 +20,7 @@ abstract interface class MacroRunner { /// Notifies the host that a file changed. /// /// Call this on changed files then call [run] again for an incremental run. - void notifyChange(SourceFile sourceFile); + void notifyChange(String sourcePath); } /// [MacroRunner] result for the whole workspace. diff --git a/pkgs/_macro_tool/lib/macro_tool.dart b/pkgs/_macro_tool/lib/macro_tool.dart index 2aa6e2e8..97ebd307 100644 --- a/pkgs/_macro_tool/lib/macro_tool.dart +++ b/pkgs/_macro_tool/lib/macro_tool.dart @@ -43,7 +43,7 @@ class MacroTool { for (final result in _applyResult!.fileResults) { if (result.output != null) { - result.sourceFile.writeOutput(result.output!); + result.sourceFile.writeOutput(macroRunner, result.output!); } } } @@ -60,7 +60,7 @@ class MacroTool { for (final result in _applyResult!.fileResults) { if (result.output != null) { - result.sourceFile.patchForAnalyzer(); + result.sourceFile.patchForAnalyzer(macroRunner); } } } @@ -77,7 +77,7 @@ class MacroTool { for (final result in _applyResult!.fileResults) { if (result.output != null) { - result.sourceFile.patchForCfe(); + result.sourceFile.patchForCfe(macroRunner); } } } @@ -110,7 +110,7 @@ class MacroTool { /// and/or [bustCaches]. void revert() { for (final sourceFile in macroRunner.sourceFiles) { - sourceFile.revert(); + sourceFile.revert(macroRunner); } } @@ -167,11 +167,8 @@ class MacroTool { void bustCaches() { var cacheBusterFound = false; for (final sourceFile in macroRunner.sourceFiles) { - if (sourceFile.bustCaches()) { + if (sourceFile.bustCaches(macroRunner)) { cacheBusterFound = true; - // Notify the macro runner of the change so that it will be picked up - // by the next incremental run. - macroRunner.notifyChange(sourceFile); } } if (!cacheBusterFound) { @@ -195,7 +192,7 @@ class MacroTool { _applyResult = await macroRunner.run(); for (final result in _applyResult!.fileResults) { if (result.output != null) { - result.sourceFile.writeOutput(result.output!); + result.sourceFile.writeOutput(macroRunner, result.output!); } } if (_applyResult!.allErrors.isNotEmpty) { diff --git a/pkgs/_macro_tool/lib/source_file.dart b/pkgs/_macro_tool/lib/source_file.dart index 31110f46..58f6f34a 100644 --- a/pkgs/_macro_tool/lib/source_file.dart +++ b/pkgs/_macro_tool/lib/source_file.dart @@ -7,6 +7,8 @@ import 'dart:math'; import 'package:path/path.dart' as p; +import 'macro_runner.dart'; + final _random = Random.secure(); /// Dart source file manipulation for `_macro_tool`. @@ -23,26 +25,33 @@ extension type SourceFile(String path) implements String { .whereType() .where((f) => f.path.endsWith('.dart')) .map((f) => SourceFile(f.path)) - .toList(); + .toList() + ..sort((a, b) => a.path.compareTo(b.path)); /// The source file path plus `.macro_tool_output`. String get toolOutputPath => '$path.macro_tool_output'; /// Writes [output] to [toolOutputPath]. - void writeOutput(String output) { + /// + /// [macroRunner] is notified of the change. + void writeOutput(MacroRunner macroRunner, String output) { File(toolOutputPath).writeAsStringSync(output); + macroRunner.notifyChange(toolOutputPath); } /// Patches [path] so the analyzer can analyze it without running macros. /// /// Adds a `part` statement referencing [toolOutputPath]. - void patchForAnalyzer() { + /// + /// [macroRunner] is notified of the change. + void patchForAnalyzer(MacroRunner macroRunner) { final partName = p.basename(toolOutputPath); final line = "part '$partName'; $_addedMarker\n"; final file = File(path); file.writeAsStringSync(_insertAfterLastImport( line, _removeToolAddedLinesFromSource(file.readAsStringSync()))); + macroRunner.notifyChange(path); } String _insertAfterLastImport(String line, String source) { @@ -60,28 +69,38 @@ extension type SourceFile(String path) implements String { /// /// This means changing [toolOutputPath] from using parts to library /// augmentations and adding `import augment` to [path]. - void patchForCfe() { + /// + /// [macroRunner] is notified of the change. + void patchForCfe(MacroRunner macroRunner) { final partName = p.basename(toolOutputPath); final line = "import augment '$partName'; $_addedMarker\n"; final file = File(path); file.writeAsStringSync( line + _removeToolAddedLinesFromSource(file.readAsStringSync())); + macroRunner.notifyChange(path); final toolOutputFile = File(toolOutputPath); toolOutputFile.writeAsStringSync(toolOutputFile .readAsStringSync() .replaceAll('part of ', 'augment library ')); + macroRunner.notifyChange(toolOutputPath); } /// Reverts changes to source from any of [patchForAnalyzer], [patchForCfe] /// and/or [bustCaches]. - void revert() { + /// + /// [macroRunner] is notified of the change. + void revert(MacroRunner macroRunner) { final file = File(path); file.writeAsStringSync(_resetCacheBusters( _removeToolAddedLinesFromSource(file.readAsStringSync()))); + macroRunner.notifyChange(path); final toolOutputFile = File(toolOutputPath); - if (toolOutputFile.existsSync()) toolOutputFile.deleteSync(); + if (toolOutputFile.existsSync()) { + toolOutputFile.deleteSync(); + macroRunner.notifyChange(toolOutputPath); + } } /// Returns [source] with lines added by [_addImportAugment] removed. @@ -96,7 +115,9 @@ extension type SourceFile(String path) implements String { /// If there is an augmentation output file, updates that too. /// /// Returns whether any change was made to a file. - bool bustCaches() { + /// + /// [macroRunner] is notified of the change. + bool bustCaches(MacroRunner macroRunner) { final token = _random.nextInt(1 << 32).toRadixString(16) + _random.nextInt(1 << 32).toRadixString(16); var cacheBusterFound = false; @@ -111,6 +132,7 @@ extension type SourceFile(String path) implements String { cacheBusterFound = true; file.writeAsStringSync( source.replaceAll(_cacheBusterRegexp, '$_cacheBusterString$token')); + macroRunner.notifyChange(path); } } return cacheBusterFound;