diff --git a/CHANGELOG.md b/CHANGELOG.md index fcf44b45..b7ef3ae4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,19 @@ arguments as the `dart format` command. You can invoke it with `dart run dart_style:format `. +* **Treat the `--stdin-name` name as a path when inferring language version.** + When reading input on stdin, the formatter still needs to know what language + version to parse the code as. If the `--stdin-name` option is set, then that + is treated as a file path and the formatter looks for a package config + surrounding that file path to infer the language version from. + + If you don't want that behavior, pass in an explicit language version using + `--language-version=`, or use `--language-version=latest` to parse the input + using the latest language version supported by the formatter. + + If `--stdin-name` and `--language-version` are both omitted, then parses + stdin using the latest supported language version. + ## 2.3.7 * Allow passing a language version to `DartFomatter()`. Formatted code will be diff --git a/lib/src/cli/format_command.dart b/lib/src/cli/format_command.dart index cba4d16c..5618261f 100644 --- a/lib/src/cli/format_command.dart +++ b/lib/src/cli/format_command.dart @@ -107,8 +107,12 @@ class FormatCommand extends Command { help: 'Track selection (given as "start:length") through formatting.', hide: !verbose); argParser.addOption('stdin-name', - help: 'Use this path in error messages when input is read from stdin.', - defaultsTo: 'stdin', + help: + 'The path that code read from stdin is treated as coming from.\n\n' + 'This path is used in error messages and also to locate a\n' + 'surrounding package to infer the code\'s language version.\n' + 'To avoid searching for a surrounding package config, pass\n' + 'in a language version using --language-version.', hide: !verbose); } @@ -223,7 +227,7 @@ class FormatCommand extends Command { if (argResults.wasParsed('stdin-name') && argResults.rest.isNotEmpty) { usageException('Cannot pass --stdin-name when not reading from stdin.'); } - var stdinName = argResults['stdin-name'] as String; + var stdinName = argResults['stdin-name'] as String?; var options = FormatterOptions( languageVersion: languageVersion, diff --git a/lib/src/io.dart b/lib/src/io.dart index 96429ece..b2def3d0 100644 --- a/lib/src/io.dart +++ b/lib/src/io.dart @@ -17,7 +17,7 @@ import 'source_code.dart'; /// Reads and formats input from stdin until closed. Future formatStdin( - FormatterOptions options, List? selection, String name) async { + FormatterOptions options, List? selection, String? path) async { var selectionStart = 0; var selectionLength = 0; @@ -26,19 +26,35 @@ Future formatStdin( selectionLength = selection[1]; } + var languageVersion = options.languageVersion; + if (languageVersion == null && path != null) { + // TODO(rnystrom): Remove the experiment check when the experiment ships. + if (options.experimentFlags.contains(tallStyleExperimentFlag)) { + var cache = LanguageVersionCache(); + languageVersion = await cache.find(File(path), path); + + // If the lookup failed, don't try to parse the code. + if (languageVersion == null) return; + } + } + + // If they didn't specify a version or a path, default to the latest. + languageVersion ??= DartFormatter.latestLanguageVersion; + + var name = path ?? 'stdin'; + var completer = Completer(); var input = StringBuffer(); stdin.transform(const Utf8Decoder()).listen(input.write, onDone: () { var formatter = DartFormatter( - languageVersion: - options.languageVersion ?? DartFormatter.latestLanguageVersion, + languageVersion: languageVersion!, indent: options.indent, pageWidth: options.pageWidth, experimentFlags: options.experimentFlags); try { options.beforeFile(null, name); var source = SourceCode(input.toString(), - uri: name, + uri: path, selectionStart: selectionStart, selectionLength: selectionLength); var output = formatter.formatSource(source); @@ -138,17 +154,10 @@ Future _processFile( // version. Version? languageVersion; if (cache != null) { - try { - // Look for a package config. If we don't find one, default to the latest - // language version. - languageVersion = await cache.find(file); - } catch (error) { - stderr.writeln('Could not read package configuration for ' - '$displayPath:\n$error'); - stderr.writeln('To avoid searching for a package configuration, ' - 'specify a language version using "--language-version".'); - return false; - } + languageVersion = await cache.find(file, displayPath); + + // If the lookup failed, don't try to parse the file. + if (languageVersion == null) return false; } else { languageVersion = options.languageVersion; } diff --git a/lib/src/language_version_cache.dart b/lib/src/language_version_cache.dart index 7242ff75..14e04d50 100644 --- a/lib/src/language_version_cache.dart +++ b/lib/src/language_version_cache.dart @@ -33,7 +33,7 @@ class LanguageVersionCache { /// Looks for a package surrounding [file] and, if found, returns the default /// language version specified by that package. - Future find(File file) async { + Future find(File file, String displayPath) async { Profile.begin('look up package config'); try { // Use the cached version (which may be `null`) if present. @@ -55,6 +55,12 @@ class LanguageVersionCache { // We weren't able to resolve this file's directory, so don't try again. return _directoryVersions[directory] = null; + } catch (error) { + stderr.writeln('Could not read package configuration for ' + '$displayPath:\n$error'); + stderr.writeln('To avoid searching for a package configuration, ' + 'specify a language version using "--language-version".'); + return null; } finally { Profile.end('look up package config'); } diff --git a/test/cli_test.dart b/test/cli_test.dart index 0ad70c34..610aa1db 100644 --- a/test/cli_test.dart +++ b/test/cli_test.dart @@ -390,10 +390,64 @@ void main() { }); group('--stdin-name', () { - test('errors if given path', () async { + test('errors if also given path', () async { var process = await runFormatter(['--stdin-name=name', 'path']); await process.shouldExit(64); }); + + test('infers language version from surrounding package', () async { + // The package config sets the language version to 3.1, but the switch + // case uses a syntax which is valid in earlier versions of Dart but an + // error in 3.0 and later. Verify that the error is reported. + await d.dir('foo', [ + packageConfig('foo', 2, 19), + ]).create(); + + var process = await runFormatter( + ['--enable-experiment=tall-style', '--stdin-name=foo/main.dart']); + // Write a switch whose syntax is valid in 2.19, but an error in later + // versions. + process.stdin.writeln('main() { switch (o) { case 1 + 2: break; } }'); + await process.stdin.close(); + + expect(await process.stdout.next, 'main() {'); + expect(await process.stdout.next, ' switch (o) {'); + expect(await process.stdout.next, ' case 1 + 2:'); + expect(await process.stdout.next, ' break;'); + expect(await process.stdout.next, ' }'); + expect(await process.stdout.next, '}'); + await process.shouldExit(0); + }); + + test('no package search if language version is specified', () async { + // Put the stdin-name in a directory with a malformed package config. If + // we search for it, we should get an error. + await d.dir('foo', [ + d.dir('.dart_tool', [ + d.file('package_config.json', 'this no good json is bad json'), + ]), + d.file('main.dart', 'main(){ }'), + ]).create(); + + var process = await runFormatter([ + '--language-version=2.19', + '--enable-experiment=tall-style', + '--stdin-name=foo/main.dart' + ]); + + // Write a switch whose syntax is valid in 2.19, but an error in later + // versions. + process.stdin.writeln('main() { switch (o) { case 1 + 2: break; } }'); + await process.stdin.close(); + + expect(await process.stdout.next, 'main() {'); + expect(await process.stdout.next, ' switch (o) {'); + expect(await process.stdout.next, ' case 1 + 2:'); + expect(await process.stdout.next, ' break;'); + expect(await process.stdout.next, ' }'); + expect(await process.stdout.next, '}'); + await process.shouldExit(0); + }); }); group('language version', () { @@ -518,87 +572,86 @@ main() { await process.stderr.cancel(); await process.shouldExit(65); }); + }); - group('package config', () { - // TODO(rnystrom): Remove this test when the experiment ships. - test('no package search if experiment is off', () async { - // Put the file in a directory with a malformed package config. If we - // search for it, we should get an error. - await d.dir('foo', [ - d.dir('.dart_tool', [ - d.file('package_config.json', 'this no good json is bad json'), - ]), - d.file('main.dart', 'main(){ }'), - ]).create(); + group('package config', () { + // TODO(rnystrom): Remove this test when the experiment ships. + test('no package search if experiment is off', () async { + // Put the file in a directory with a malformed package config. If we + // search for it, we should get an error. + await d.dir('foo', [ + d.dir('.dart_tool', [ + d.file('package_config.json', 'this no good json is bad json'), + ]), + d.file('main.dart', 'main(){ }'), + ]).create(); - var process = await runFormatterOnDir(); - await process.shouldExit(0); + var process = await runFormatterOnDir(); + await process.shouldExit(0); - // Should format the file without any error reading the package config. - await d.dir('foo', [d.file('main.dart', 'main() {}\n')]).validate(); - }); + // Should format the file without any error reading the package config. + await d.dir('foo', [d.file('main.dart', 'main() {}\n')]).validate(); + }); - test('no package search if language version is specified', () async { - // Put the file in a directory with a malformed package config. If we - // search for it, we should get an error. - await d.dir('foo', [ - d.dir('.dart_tool', [ - d.file('package_config.json', 'this no good json is bad json'), - ]), - d.file('main.dart', 'main(){ }'), - ]).create(); + test('no package search if language version is specified', () async { + // Put the file in a directory with a malformed package config. If we + // search for it, we should get an error. + await d.dir('foo', [ + d.dir('.dart_tool', [ + d.file('package_config.json', 'this no good json is bad json'), + ]), + d.file('main.dart', 'main(){ }'), + ]).create(); - var process = await runFormatterOnDir( - ['--language-version=latest', '--enable-experiment=tall-style']); - await process.shouldExit(0); + var process = await runFormatterOnDir( + ['--language-version=latest', '--enable-experiment=tall-style']); + await process.shouldExit(0); - // Should format the file without any error reading the package config. - await d.dir('foo', [d.file('main.dart', 'main() {}\n')]).validate(); - }); + // Should format the file without any error reading the package config. + await d.dir('foo', [d.file('main.dart', 'main() {}\n')]).validate(); + }); - test('default to language version of surrounding package', () async { - // The package config sets the language version to 3.1, but the switch - // case uses a syntax which is valid in earlier versions of Dart but an - // error in 3.0 and later. Verify that the error is reported (instead of - // the formatter falling back to try to parse the file at the older - // language version). - await d.dir('foo', [ - packageConfig('foo', 3, 1), - d.file('main.dart', 'main() { switch (o) { case 1 + 2: break; } }'), - ]).create(); + test('default to language version of surrounding package', () async { + // The package config sets the language version to 3.1, but the switch + // case uses a syntax which is valid in earlier versions of Dart but an + // error in 3.0 and later. Verify that the error is reported. + await d.dir('foo', [ + packageConfig('foo', 3, 1), + d.file('main.dart', 'main() { switch (o) { case 1 + 2: break; } }'), + ]).create(); - var path = p.join(d.sandbox, 'foo', 'main.dart'); - // TODO(rnystrom): Remove experiment flag when it ships. - var process = - await runFormatter([path, '--enable-experiment=tall-style']); + var path = p.join(d.sandbox, 'foo', 'main.dart'); + // TODO(rnystrom): Remove experiment flag when it ships. + var process = + await runFormatter([path, '--enable-experiment=tall-style']); - expect(await process.stderr.next, - 'Could not format because the source could not be parsed:'); - expect(await process.stderr.next, ''); - expect(await process.stderr.next, contains('main.dart')); - await process.shouldExit(65); - }); + expect(await process.stderr.next, + 'Could not format because the source could not be parsed:'); + expect(await process.stderr.next, ''); + expect(await process.stderr.next, contains('main.dart')); + await process.shouldExit(65); + }); - test('language version comment overrides package default', () async { - // The package config sets the language version to 3.1, but the switch - // case uses a syntax which is valid in earlier versions of Dart but an - // error in 3.0 and later. Verify that no error is reported since this - // file opts to the older version. - await d.dir('foo', [ - packageConfig('foo', 3, 1), - d.file('main.dart', ''' + test('language version comment overrides package default', () async { + // The package config sets the language version to 3.1, but the switch + // case uses a syntax which is valid in earlier versions of Dart but an + // error in 3.0 and later. Verify that no error is reported since this + // file opts to the older version. + await d.dir('foo', [ + packageConfig('foo', 3, 1), + d.file('main.dart', ''' // @dart=2.19 main() { switch (obj) { case 1 + 2: // Error in 3.1. } } '''), - ]).create(); + ]).create(); - var process = await runFormatterOnDir(); - await process.shouldExit(0); + var process = await runFormatterOnDir(); + await process.shouldExit(0); - // Formats the file. - await d.dir('foo', [ - d.file('main.dart', ''' + // Formats the file. + await d.dir('foo', [ + d.file('main.dart', ''' // @dart=2.19 main() { switch (obj) { @@ -606,28 +659,27 @@ main() { } } ''') - ]).validate(); - }); + ]).validate(); + }); - test('malformed', () async { - await d.dir('foo', [ - d.dir('.dart_tool', [ - d.file('package_config.json', 'this no good json is bad json'), - ]), - d.file('main.dart', 'main() {}'), - ]).create(); + test('malformed', () async { + await d.dir('foo', [ + d.dir('.dart_tool', [ + d.file('package_config.json', 'this no good json is bad json'), + ]), + d.file('main.dart', 'main() {}'), + ]).create(); - var path = p.join(d.sandbox, 'foo', 'main.dart'); - // TODO(rnystrom): Remove experiment flag when it ships. - var process = - await runFormatter([path, '--enable-experiment=tall-style']); + var path = p.join(d.sandbox, 'foo', 'main.dart'); + // TODO(rnystrom): Remove experiment flag when it ships. + var process = + await runFormatter([path, '--enable-experiment=tall-style']); - expect( - await process.stderr.next, - allOf(startsWith('Could not read package configuration for'), - contains(p.join('foo', 'main.dart')))); - await process.shouldExit(65); - }); + expect( + await process.stderr.next, + allOf(startsWith('Could not read package configuration for'), + contains(p.join('foo', 'main.dart')))); + await process.shouldExit(65); }); }); } diff --git a/test/language_version_cache_test.dart b/test/language_version_cache_test.dart index 4ef67489..71f847d5 100644 --- a/test/language_version_cache_test.dart +++ b/test/language_version_cache_test.dart @@ -108,11 +108,11 @@ void main() { Future _expectVersion( LanguageVersionCache cache, String path, int major, int minor) async { - expect(await cache.find(_expectedFile(path)), Version(major, minor, 0)); + expect(await cache.find(_expectedFile(path), path), Version(major, minor, 0)); } Future _expectNullVersion(LanguageVersionCache cache, String path) async { - expect(await cache.find(_expectedFile(path)), null); + expect(await cache.find(_expectedFile(path), path), null); } /// Normalize path separators to the host OS separator since that's what the