From 321dfea36ab7cb0b7bbc7a71e9d51a325ddcefa4 Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Wed, 3 Sep 2025 06:43:02 -0700 Subject: [PATCH] [analyzer/cfe] Always use pre-built SDK for formatting generated files. Change the following code paths so that they format Dart code by running `tools/sdks/dart-sdk/bin/dart format` as a subprocess: - `generate_messages.dart` (this is the CFE tool that generates `pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart` and `pkg/front_end/lib/src/codes/cfe_codes_generated.dart`). - `generated_files_up_to_date_git_test.dart` (this is a CFE test that verifies that `pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart` and `pkg/front_end/lib/src/codes/cfe_codes_generated.dart` are up to date). - `GeneratedFile.generate` (this is the common method used by most analyzer code generators). - `GeneratedContentExtension.check` (this is used by analyzer tests to verify that generated files are up to date). - `relevance_table_generator.dart` (this is an `analysis_server` tool that generates `relevance_tables*.g.dart` files). Previously, these tools were inconsistent; some of them invoked the formatter through its Dart API, and some of them ran `dart format` as a subprocess via whatever `dart` executable was currently being used for the parent process. This led to a risk that a generated file might be immediately rejected by its own "up to date" test, or by the presubmit script. Standardizing on the use of `tools/sdks/dart-sdk/bin/dart format` (which is what the presubmit script uses) should address this risk. Addresses code review comments: https://dart-review.googlesource.com/c/sdk/+/447920/comment/e9e1aa39_6fffc7e5/. Change-Id: I6a6a69643a69505dc6e30547e0e784358458c357 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/448161 Commit-Queue: Paul Berry Reviewed-by: Jens Johansen --- .../lib/generated_content_check.dart | 3 +-- pkg/analyzer_utilities/lib/tools.dart | 24 ++++++++++++++++++- pkg/analyzer_utilities/pubspec.yaml | 2 +- .../generated_files_up_to_date_git_test.dart | 12 ++++++---- 4 files changed, 32 insertions(+), 9 deletions(-) diff --git a/pkg/analyzer_utilities/lib/generated_content_check.dart b/pkg/analyzer_utilities/lib/generated_content_check.dart index 72207db1f16..28d70d5228e 100644 --- a/pkg/analyzer_utilities/lib/generated_content_check.dart +++ b/pkg/analyzer_utilities/lib/generated_content_check.dart @@ -2,7 +2,6 @@ import 'dart:async'; import 'dart:io'; import 'package:analyzer_utilities/tools.dart'; -import 'package:dart_style/dart_style.dart'; import 'package:package_config/package_config.dart'; import 'package:path/path.dart'; import 'package:pub_semver/pub_semver.dart'; @@ -27,7 +26,7 @@ Future _formatText(String text, {required String pkgPath}) async { throw StateError('Could not find a Dart language version for "$pkgPath"'); } var version = Version(languageVersion.major, languageVersion.minor, 0); - return DartFormatter(languageVersion: version).format(text); + return DartFormat.formatString(text, languageVersion: version); } extension GeneratedContentExtension on GeneratedContent { diff --git a/pkg/analyzer_utilities/lib/tools.dart b/pkg/analyzer_utilities/lib/tools.dart index daaab210dc7..a5526286950 100644 --- a/pkg/analyzer_utilities/lib/tools.dart +++ b/pkg/analyzer_utilities/lib/tools.dart @@ -8,11 +8,13 @@ library; import 'dart:async'; import 'dart:io'; +import 'package:analyzer_testing/package_root.dart'; import 'package:analyzer_utilities/html_dom.dart' as dom; import 'package:analyzer_utilities/html_generator.dart'; import 'package:analyzer_utilities/text_formatter.dart'; import 'package:collection/collection.dart'; import 'package:path/path.dart'; +import 'package:pub_semver/pub_semver.dart'; final RegExp trailingSpacesInLineRegExp = RegExp(r' +$', multiLine: true); final RegExp trailingWhitespaceRegExp = RegExp(r'[\n ]+$'); @@ -243,13 +245,33 @@ class CodeGeneratorSettings { /// A utility class for invoking 'dart format'. class DartFormat { - static String get _dartPath => Platform.resolvedExecutable; + static final String _dartPath = + // TODO(paulberry): check that this is the right path on Windows. + join(packageRoot, '..', 'tools', 'sdks', 'dart-sdk', 'bin', 'dart'); static void formatFile(File file) { var result = Process.runSync(_dartPath, ['format', file.path]); _throwIfExitCode(result); } + static String formatString(String text, {required Version languageVersion}) { + var tempDir = Directory.systemTemp.createTempSync('format'); + try { + var file = File(join(tempDir.path, 'input.dart')); + file.writeAsStringSync(text); + var result = Process.runSync(_dartPath, [ + 'format', + file.path, + '--language-version', + '${languageVersion.major}.${languageVersion.minor}', + ]); + _throwIfExitCode(result); + return file.readAsStringSync(); + } finally { + tempDir.deleteSync(recursive: true); + } + } + static void _throwIfExitCode(ProcessResult result) { if (result.exitCode != 0) throw result.stderr as Object; } diff --git a/pkg/analyzer_utilities/pubspec.yaml b/pkg/analyzer_utilities/pubspec.yaml index 35984ca5cd8..e47397557ef 100644 --- a/pkg/analyzer_utilities/pubspec.yaml +++ b/pkg/analyzer_utilities/pubspec.yaml @@ -10,8 +10,8 @@ resolution: workspace # Use 'any' constraints here; we get our versions from the DEPS file. dependencies: analyzer: any + analyzer_testing: any collection: any - dart_style: any package_config: any path: any pub_semver: any diff --git a/pkg/front_end/test/generated_files_up_to_date_git_test.dart b/pkg/front_end/test/generated_files_up_to_date_git_test.dart index 74a81165424..88da3e7b182 100644 --- a/pkg/front_end/test/generated_files_up_to_date_git_test.dart +++ b/pkg/front_end/test/generated_files_up_to_date_git_test.dart @@ -4,7 +4,7 @@ import "dart:io" show File, exitCode; -import 'package:dart_style/dart_style.dart' show DartFormatter; +import "package:analyzer_utilities/tools.dart" show DartFormat; import "../tool/generate_experimental_flags.dart" as generateExperimentalFlags; import "../tool/generate_messages.dart" as generateMessages; @@ -133,18 +133,20 @@ void messages() { Uri generatedFile = generateMessages.computeSharedGeneratedFile(repoDir); check( - new DartFormatter( + DartFormat.formatString( + messages.sharedMessages, languageVersion: getPackageVersionFor('_fe_analyzer_shared'), - ).format(messages.sharedMessages), + ), generatedFile, "dart pkg/front_end/tool/generate_messages.dart", ); Uri cfeGeneratedFile = generateMessages.computeCfeGeneratedFile(repoDir); check( - new DartFormatter( + DartFormat.formatString( + messages.cfeMessages, languageVersion: getPackageVersionFor('front_end'), - ).format(messages.cfeMessages), + ), cfeGeneratedFile, "dart pkg/front_end/tool/generate_messages.dart", );