Skip to content

Commit 31422e8

Browse files
authored
Use surrounding packages to infer language version in the CLI. (#1508)
When formatting file paths using the formatter CLI, look for a surrounding package (identified by a package config) to infer the default (i.e. not `// @Dart=`) language version for each formatted file. Currently, this behavior only happens when the tall-style experiment is enabled. When that experiment ships, it will be the default behavior. If the user passes an explicit language version using "--language-version", then no directory searching happens. This only affects the CLI behavior. The library API never searches the file system. Also, when passing input to the formatter CLI over stdin, it doesn't search the file system.
1 parent 2695039 commit 31422e8

11 files changed

+396
-33
lines changed

bin/format.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ void main(List<String> args) async {
138138
if (argResults.rest.isEmpty) {
139139
await formatStdin(options, selection, argResults['stdin-name'] as String);
140140
} else {
141-
formatPaths(options, argResults.rest);
141+
await formatPaths(options, argResults.rest);
142142
}
143143

144144
options.summary.show();

lib/src/cli/format_command.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ class FormatCommand extends Command<int> {
170170
if (argResults.rest.isEmpty) {
171171
await formatStdin(options, selection, stdinName);
172172
} else {
173-
formatPaths(options, argResults.rest);
173+
await formatPaths(options, argResults.rest);
174174
options.summary.show();
175175
}
176176

lib/src/cli/options.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ void defineOptions(ArgParser parser,
7272
help: 'Language version of formatted code.\n'
7373
'Use "latest" to parse as the latest supported version.\n'
7474
'Omit to look for a surrounding package config.',
75-
// TODO(rnystrom): Show this when package config support is implemented.
75+
// TODO(rnystrom): Show this when the tall-style experiment ships.
7676
hide: true);
7777
}
7878

lib/src/dart_formatter.dart

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -244,10 +244,9 @@ class DartFormatter {
244244
sdkLanguageVersion: version, flags: experiments);
245245

246246
return parseString(
247-
content: source,
248-
featureSet: featureSet,
249-
path: uri,
250-
throwIfDiagnostics: false,
251-
);
247+
content: source,
248+
featureSet: featureSet,
249+
path: uri,
250+
throwIfDiagnostics: false);
252251
}
253252
}

lib/src/io.dart

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,13 @@ import 'dart:convert';
66
import 'dart:io';
77

88
import 'package:path/path.dart' as p;
9+
import 'package:pub_semver/pub_semver.dart';
910

1011
import 'cli/formatter_options.dart';
12+
import 'constants.dart';
1113
import 'dart_formatter.dart';
1214
import 'exceptions.dart';
15+
import 'language_version_cache.dart';
1316
import 'source_code.dart';
1417

1518
/// Reads and formats input from stdin until closed.
@@ -59,19 +62,29 @@ $stack''');
5962
}
6063

6164
/// Formats all of the files and directories given by [paths].
62-
void formatPaths(FormatterOptions options, List<String> paths) {
65+
Future<void> formatPaths(FormatterOptions options, List<String> paths) async {
66+
// If the user didn't specify a language version, then look for surrounding
67+
// package configs so we know what language versions to use for the files.
68+
LanguageVersionCache? cache;
69+
if (options.languageVersion == null) {
70+
// TODO(rnystrom): Remove the experiment check when the experiment ships.
71+
if (options.experimentFlags.contains(tallStyleExperimentFlag)) {
72+
cache = LanguageVersionCache();
73+
}
74+
}
75+
6376
for (var path in paths) {
6477
var directory = Directory(path);
6578
if (directory.existsSync()) {
66-
if (!processDirectory(options, directory)) {
79+
if (!await _processDirectory(cache, options, directory)) {
6780
exitCode = 65;
6881
}
6982
continue;
7083
}
7184

7285
var file = File(path);
7386
if (file.existsSync()) {
74-
if (!processFile(options, file)) {
87+
if (!await _processFile(cache, options, file)) {
7588
exitCode = 65;
7689
}
7790
} else {
@@ -85,7 +98,8 @@ void formatPaths(FormatterOptions options, List<String> paths) {
8598
///
8699
/// Returns `true` if successful or `false` if an error occurred in any of the
87100
/// files.
88-
bool processDirectory(FormatterOptions options, Directory directory) {
101+
Future<bool> _processDirectory(LanguageVersionCache? cache,
102+
FormatterOptions options, Directory directory) async {
89103
options.showDirectory(directory.path);
90104

91105
var success = true;
@@ -125,7 +139,9 @@ bool processDirectory(FormatterOptions options, Directory directory) {
125139
continue;
126140
}
127141

128-
if (!processFile(options, entry, displayPath: displayPath)) success = false;
142+
if (!await _processFile(cache, options, entry, displayPath: displayPath)) {
143+
success = false;
144+
}
129145
}
130146

131147
return success;
@@ -134,15 +150,39 @@ bool processDirectory(FormatterOptions options, Directory directory) {
134150
/// Runs the formatter on [file].
135151
///
136152
/// Returns `true` if successful or `false` if an error occurred.
137-
bool processFile(FormatterOptions options, File file, {String? displayPath}) {
153+
Future<bool> _processFile(
154+
LanguageVersionCache? cache, FormatterOptions options, File file,
155+
{String? displayPath}) async {
138156
displayPath ??= file.path;
139157

158+
// Determine what language version to use. If we have a language version
159+
// cache, that implies that we should use the surrounding package config to
160+
// infer the file's language version. Otherwise, use the user-provided
161+
// version.
162+
Version? languageVersion;
163+
if (cache != null) {
164+
try {
165+
// Look for a package config. If we don't find one, default to the latest
166+
// language version.
167+
languageVersion = await cache.find(file);
168+
} catch (error) {
169+
stderr.writeln('Could not read package configuration for '
170+
'$displayPath:\n$error');
171+
stderr.writeln('To avoid searching for a package configuration, '
172+
'specify a language version using "--language-version".');
173+
return false;
174+
}
175+
} else {
176+
languageVersion = options.languageVersion;
177+
}
178+
140179
var formatter = DartFormatter(
141-
languageVersion: options.languageVersion,
180+
languageVersion: languageVersion ?? DartFormatter.latestLanguageVersion,
142181
indent: options.indent,
143182
pageWidth: options.pageWidth,
144183
fixes: options.fixes,
145184
experimentFlags: options.experimentFlags);
185+
146186
try {
147187
var source = SourceCode(file.readAsStringSync(), uri: file.path);
148188
options.beforeFile(file, displayPath);

lib/src/language_version_cache.dart

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
import 'dart:async';
5+
import 'dart:io';
6+
7+
import 'package:package_config/package_config.dart';
8+
import 'package:pub_semver/pub_semver.dart';
9+
10+
import 'profile.dart';
11+
12+
/// Caches the default language version that should be used for files within
13+
/// directories.
14+
///
15+
/// The default language version for a Dart file is found by walking the parent
16+
/// directories of the file being formatted to look for a
17+
/// `.dart_tool/package_config.json` file. When found, we cache the result for
18+
/// the formatted file's parent directory. This way, when formatting multiple
19+
/// files in the same directory, we don't have to look for and read the package
20+
/// config multiple times, which is slow.
21+
///
22+
/// (When formatting dart_style on a Mac laptop, it would spend as much time
23+
/// looking for package configs for each file as it did formatting if we don't
24+
/// cache. Caching makes it ~10x faster to find the language version for each
25+
/// file.)
26+
class LanguageVersionCache {
27+
/// The previously cached default language version for all files immediately
28+
/// within a given directory.
29+
///
30+
/// The version may be `null` if we formatted a file in that directory and
31+
/// discovered that there is no surrounding package.
32+
final Map<String, Version?> _directoryVersions = {};
33+
34+
/// Looks for a package surrounding [file] and, if found, returns the default
35+
/// language version specified by that package.
36+
Future<Version?> find(File file) async {
37+
Profile.begin('look up package config');
38+
try {
39+
// Use the cached version (which may be `null`) if present.
40+
var directory = file.parent.path;
41+
if (_directoryVersions.containsKey(directory)) {
42+
return _directoryVersions[directory];
43+
}
44+
45+
// Otherwise, walk the file system and look for it.
46+
var config = await findPackageConfig(file.parent);
47+
if (config?.packageOf(file.absolute.uri)?.languageVersion
48+
case var languageVersion?) {
49+
// Store the version as pub_semver's [Version] type because that's
50+
// what the analyzer parser uses, which is where the version
51+
// ultimately gets used.
52+
var version = Version(languageVersion.major, languageVersion.minor, 0);
53+
return _directoryVersions[directory] = version;
54+
}
55+
56+
// We weren't able to resolve this file's directory, so don't try again.
57+
return _directoryVersions[directory] = null;
58+
} finally {
59+
Profile.end('look up package config');
60+
}
61+
}
62+
}

pubspec.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ dependencies:
1212
analyzer: '^6.5.0'
1313
args: ">=1.0.0 <3.0.0"
1414
collection: "^1.17.0"
15+
package_config: ^2.1.0
1516
path: ^1.0.0
1617
pub_semver: ">=1.4.4 <3.0.0"
1718
source_span: ^1.4.0

test/command_test.dart

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -576,5 +576,116 @@ main() {
576576
await process.stderr.cancel();
577577
await process.shouldExit(65);
578578
});
579+
580+
group('package config', () {
581+
// TODO(rnystrom): Remove this test when the experiment ships.
582+
test('no package search if experiment is off', () async {
583+
// Put the file in a directory with a malformed package config. If we
584+
// search for it, we should get an error.
585+
await d.dir('foo', [
586+
d.dir('.dart_tool', [
587+
d.file('package_config.json', 'this no good json is bad json'),
588+
]),
589+
d.file('main.dart', 'main(){ }'),
590+
]).create();
591+
592+
var process = await runCommandOnDir();
593+
await process.shouldExit(0);
594+
595+
// Should format the file without any error reading the package config.
596+
await d.dir('foo', [d.file('main.dart', 'main() {}\n')]).validate();
597+
});
598+
599+
test('no package search if language version is specified', () async {
600+
// Put the file in a directory with a malformed package config. If we
601+
// search for it, we should get an error.
602+
await d.dir('foo', [
603+
d.dir('.dart_tool', [
604+
d.file('package_config.json', 'this no good json is bad json'),
605+
]),
606+
d.file('main.dart', 'main(){ }'),
607+
]).create();
608+
609+
var process = await runCommandOnDir(
610+
['--language-version=latest', '--enable-experiment=tall-style']);
611+
await process.shouldExit(0);
612+
613+
// Should format the file without any error reading the package config.
614+
await d.dir('foo', [d.file('main.dart', 'main() {}\n')]).validate();
615+
});
616+
617+
test('default to language version of surrounding package', () async {
618+
// The package config sets the language version to 3.1, but the switch
619+
// case uses a syntax which is valid in earlier versions of Dart but an
620+
// error in 3.0 and later. Verify that the error is reported (instead of
621+
// the formatter falling back to try to parse the file at the older
622+
// language version).
623+
await d.dir('foo', [
624+
packageConfig('foo', 3, 1),
625+
d.file('main.dart', 'main() { switch (o) { case 1 + 2: break; } }'),
626+
]).create();
627+
628+
var path = p.join(d.sandbox, 'foo', 'main.dart');
629+
// TODO(rnystrom): Remove experiment flag when it ships.
630+
var process =
631+
await runCommand([path, '--enable-experiment=tall-style']);
632+
633+
expect(await process.stderr.next,
634+
'Could not format because the source could not be parsed:');
635+
expect(await process.stderr.next, '');
636+
expect(await process.stderr.next, contains(p.join('foo', 'main.dart')));
637+
await process.shouldExit(65);
638+
});
639+
640+
test('language version comment overrides package default', () async {
641+
// The package config sets the language version to 3.1, but the switch
642+
// case uses a syntax which is valid in earlier versions of Dart but an
643+
// error in 3.0 and later. Verify that no error is reported since this
644+
// file opts to the older version.
645+
await d.dir('foo', [
646+
packageConfig('foo', 3, 1),
647+
d.file('main.dart', '''
648+
// @dart=2.19
649+
main() { switch (obj) { case 1 + 2: // Error in 3.1.
650+
} }
651+
'''),
652+
]).create();
653+
654+
var process = await runCommandOnDir();
655+
await process.shouldExit(0);
656+
657+
// Formats the file.
658+
await d.dir('foo', [
659+
d.file('main.dart', '''
660+
// @dart=2.19
661+
main() {
662+
switch (obj) {
663+
case 1 + 2: // Error in 3.1.
664+
}
665+
}
666+
''')
667+
]).validate();
668+
});
669+
670+
test('malformed', () async {
671+
await d.dir('foo', [
672+
d.dir('.dart_tool', [
673+
d.file('package_config.json', 'this no good json is bad json'),
674+
]),
675+
d.file('main.dart', 'main() {}'),
676+
]).create();
677+
678+
var path = p.join(d.sandbox, 'foo', 'main.dart');
679+
// TODO(rnystrom): Remove experiment flag when it ships.
680+
var process =
681+
await runCommand([path, '--enable-experiment=tall-style']);
682+
683+
expect(
684+
await process.stderr.next,
685+
allOf(startsWith('Could not read package configuration for'),
686+
contains(p.join('foo', 'main.dart'))));
687+
await process.shouldExit(65);
688+
});
689+
});
579690
});
580691
}

0 commit comments

Comments
 (0)