Use the latest language version if we can't find a package. (#1573)

Use the latest language version if we can't find a package.

In the new tall-style behavior where we look for a surrounding package
config to infer the language version of each file, we have to decide
what to do when that look-up process fails (either because there is no
package config, or it's malformed).

Initially, I had it error out on that file. That's consistent with the
library API where the formatter *requires* a language version before it
will do anything.

But it's not consistent with the language spec and our other tools. For
them, if there is no valid surrounding package config, the file should
be treated as the latest language version.

This PR does that.

It also fixes the catastrophic UX that the current code has which is
that when a file can't have its language version inferred... it is
silently skipped without telling the user anything. Oops. Now it just
formats the file as expected.

Author: munificent

lib/src/io.dart

@@ -35,12 +35,10 @@ Future<void> formatStdin(
   if (languageVersion == null && path != null && cache != null) {
     // We have a stdin-name, so look for a surrounding package config.
     languageVersion = await cache.findLanguageVersion(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.
+  // If they didn't specify a version or a path, or couldn't find a package
+  // surrounding the path, then default to the latest version.
   languageVersion ??= DartFormatter.latestLanguageVersion;
 
   // Determine the page width.
@@ -164,14 +162,12 @@ Future<bool> _processFile(
   var languageVersion = options.languageVersion;
   if (languageVersion == null && cache != null) {
     languageVersion = await cache.findLanguageVersion(file, displayPath);
-
-    // If the lookup failed, don't try to parse the file.
-    if (languageVersion == null) return false;
-  } else {
-    // If they didn't specify a version or a path, default to the latest.
-    languageVersion ??= DartFormatter.latestLanguageVersion;
   }
 
+  // If they didn't specify a version and we couldn't find a surrounding
+  // package, then default to the latest version.
+  languageVersion ??= DartFormatter.latestLanguageVersion;
+
   // Determine the page width.
   var pageWidth = options.pageWidth;
   if (pageWidth == null && cache != null) {

lib/src/testing/test_file_system.dart

@@ -18,27 +18,29 @@ final class TestFileSystem implements FileSystem {
   }
 
   @override
-  Future<bool> fileExists(FileSystemPath path) async =>
-      _files.containsKey(path.testPath);
+  Future<bool> fileExists(covariant TestFileSystemPath path) async =>
+      _files.containsKey(path._path);
 
   @override
-  Future<FileSystemPath> join(FileSystemPath from, String to) async {
+  Future<FileSystemPath> join(
+      covariant TestFileSystemPath from, String to) async {
     // If it's an absolute path, discard [from].
     if (to.startsWith('|')) return TestFileSystemPath(to);
-    return TestFileSystemPath('${from.testPath}|$to');
+    return TestFileSystemPath('${from._path}|$to');
   }
 
   @override
-  Future<FileSystemPath?> parentDirectory(FileSystemPath path) async {
-    var parts = path.testPath.split('|');
+  Future<FileSystemPath?> parentDirectory(
+      covariant TestFileSystemPath path) async {
+    var parts = path._path.split('|');
     if (parts.length == 1) return null;
 
     return TestFileSystemPath(parts.sublist(0, parts.length - 1).join('|'));
   }
 
   @override
-  Future<String> readFile(FileSystemPath path) async {
-    if (_files[path.testPath] case var contents?) return contents;
+  Future<String> readFile(covariant TestFileSystemPath path) async {
+    if (_files[path._path] case var contents?) return contents;
     throw Exception('No file at "$path".');
   }
 }
@@ -51,7 +53,3 @@ final class TestFileSystemPath implements FileSystemPath {
   @override
   String toString() => _path;
 }
-
-extension on FileSystemPath {
-  String get testPath => (this as TestFileSystemPath)._path;
-}

test/cli/language_version_test.dart

@@ -26,10 +26,12 @@ extension type Meters(int value) {
 }
 ''';
 
-    test('defaults to latest language version if omitted', () async {
+    test('uses latest language version if no surrounding package', () async {
       await d.dir('code', [d.file('a.dart', extensionTypeBefore)]).create();
 
-      var process = await runFormatterOnDir();
+      // Enable the experiment to be sure that we're opting in to the new
+      // package config search.
+      var process = await runFormatterOnDir(['--enable-experiment=tall-style']);
       await process.shouldExit(0);
 
       await d.dir('code', [d.file('a.dart', extensionTypeAfter)]).validate();
@@ -160,24 +162,26 @@ main() {
       ]).validate();
     });
 
-    test('malformed', () async {
+    test('use the latest version if the package config is 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() {}'),
+        d.file('main.dart', 'main() {var (a,b)=(1,2);}'),
       ]).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 process = await runFormatterOnDir(['--enable-experiment=tall-style']);
+      await process.shouldExit(0);
 
-      expect(
-          await process.stderr.next,
-          allOf(startsWith('Could not read package configuration for'),
-              contains(p.join('foo', 'main.dart'))));
-      await process.shouldExit(65);
+      // Formats the file.
+      await d.dir('foo', [
+        d.file('main.dart', '''
+main() {
+  var (a, b) = (1, 2);
+}
+''')
+      ]).validate();
     });
   });
 
@@ -235,5 +239,20 @@ main() {
       expect(await process.stdout.next, '}');
       await process.shouldExit(0);
     });
+
+    test('use latest language version if no surrounding package', () async {
+      await d.dir('foo', []).create();
+
+      var process = await runFormatter(
+          ['--enable-experiment=tall-style', '--stdin-name=foo/main.dart']);
+      // Use some relatively recent syntax.
+      process.stdin.writeln('main() {var (a,b)=(1,2);}');
+      await process.stdin.close();
+
+      expect(await process.stdout.next, 'main() {');
+      expect(await process.stdout.next, '  var (a, b) = (1, 2);');
+      expect(await process.stdout.next, '}');
+      await process.shouldExit(0);
+    });
   });
 }