[pigeon] Deduces the package name for dart test file imports. (flutte…

…r#2467)

* [pigeon] Deduces the package name for dart test file imports.

* stuart feedback

* stuart feedback 2

packages/pigeon/CHANGELOG.md

@@ -1,3 +1,8 @@
+## 3.2.8
+
+* [dart] Deduces the correct import statement for Dart test files made with
+  `dartHostTestHandler` instead of relying on relative imports.
+
 ## 3.2.7
 
 * Requires `analyzer 4.4.0`, and replaces use of deprecated APIs.

packages/pigeon/lib/dart_generator.dart

@@ -2,6 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'dart:io' show Directory, File, FileSystemEntity;
+
+import 'package:path/path.dart' as path;
+import 'package:yaml/yaml.dart' as yaml;
+
 import 'ast.dart';
 import 'functional.dart';
 import 'generator_tools.dart';
@@ -588,14 +593,66 @@ pigeonMap['${field.name}'] != null
   }
 }
 
-/// Generates Dart source code for test support libraries based on the
-/// given AST represented by [root], outputting the code to [sink].
+/// Crawls up the path of [dartFilePath] until it finds a pubspec.yaml in a
+/// parent directory and returns its path.
+String? _findPubspecPath(String dartFilePath) {
+  try {
+    Directory dir = File(dartFilePath).parent;
+    String? pubspecPath;
+    while (pubspecPath == null) {
+      if (dir.existsSync()) {
+        final Iterable<String> pubspecPaths = dir
+            .listSync()
+            .map((FileSystemEntity e) => e.path)
+            .where((String path) => path.endsWith('pubspec.yaml'));
+        if (pubspecPaths.isNotEmpty) {
+          pubspecPath = pubspecPaths.first;
+        } else {
+          dir = dir.parent;
+        }
+      } else {
+        break;
+      }
+    }
+    return pubspecPath;
+  } catch (ex) {
+    return null;
+  }
+}
+
+/// Given the path of a Dart file, [mainDartFile], the name of the package will
+/// be deduced by locating and parsing its associated pubspec.yaml.
+String? _deducePackageName(String mainDartFile) {
+  final String? pubspecPath = _findPubspecPath(mainDartFile);
+  if (pubspecPath == null) {
+    return null;
+  }
+
+  try {
+    final String text = File(pubspecPath).readAsStringSync();
+    return (yaml.loadYaml(text) as Map<dynamic, dynamic>)['name'];
+  } catch (_) {
+    return null;
+  }
+}
+
+/// Converts [inputPath] to a posix absolute path.
+String _posixify(String inputPath) {
+  final path.Context context = path.Context(style: path.Style.posix);
+  return context.fromUri(path.toUri(path.absolute(inputPath)));
+}
+
+/// Generates Dart source code for test support libraries based on the given AST
+/// represented by [root], outputting the code to [sink]. [dartOutPath] is the
+/// path of the generated dart code to be tested. [testOutPath] is where the
+/// test code will be generated.
 void generateTestDart(
   DartOptions opt,
   Root root,
-  StringSink sink,
-  String mainDartFile,
-) {
+  StringSink sink, {
+  required String dartOutPath,
+  required String testOutPath,
+}) {
   final Indent indent = Indent(sink);
   if (opt.copyrightHeader != null) {
     addLines(indent, opt.copyrightHeader!, linePrefix: '// ');
@@ -615,11 +672,23 @@ void generateTestDart(
   indent.writeln('import \'package:flutter/services.dart\';');
   indent.writeln('import \'package:flutter_test/flutter_test.dart\';');
   indent.writeln('');
-  // TODO(gaaclarke): Switch from relative paths to URIs. This fails in new versions of Dart,
-  // https://github.com/flutter/flutter/issues/97744.
-  indent.writeln(
-    'import \'${_escapeForDartSingleQuotedString(mainDartFile)}\';',
+  final String relativeDartPath =
+      path.Context(style: path.Style.posix).relative(
+    _posixify(dartOutPath),
+    from: _posixify(path.dirname(testOutPath)),
   );
+  late final String? packageName = _deducePackageName(dartOutPath);
+  if (!relativeDartPath.contains('/lib/') || packageName == null) {
+    // If we can't figure out the package name or the relative path doesn't
+    // include a 'lib' directory, try relative path import which only works in
+    // certain (older) versions of Dart.
+    // TODO(gaaclarke): We should add a command-line parameter to override this import.
+    indent.writeln(
+        'import \'${_escapeForDartSingleQuotedString(relativeDartPath)}\';');
+  } else {
+    final String path = relativeDartPath.replaceFirst(RegExp(r'^.*/lib/'), '');
+    indent.writeln("import 'package:$packageName/$path';");
+  }
   for (final Api api in root.apis) {
     if (api.location == ApiLocation.host && api.dartHostTestHandler != null) {
       final Api mockApi = Api(

packages/pigeon/lib/generator_tools.dart

@@ -9,7 +9,7 @@ import 'dart:mirrors';
 import 'ast.dart';
 
 /// The current version of pigeon. This must match the version in pubspec.yaml.
-const String pigeonVersion = '3.2.7';
+const String pigeonVersion = '3.2.8';
 
 /// Read all the content from [stdin] to a String.
 String readStdin() {

packages/pigeon/lib/pigeon_lib.dart

@@ -313,11 +313,6 @@ class ParseResults {
   final Map<String, Object>? pigeonOptions;
 }
 
-String _posixify(String input) {
-  final path.Context context = path.Context(style: path.Style.posix);
-  return context.fromUri(path.toUri(path.absolute(input)));
-}
-
 Iterable<String> _lineReader(String path) sync* {
   final String contents = File(path).readAsStringSync();
   const LineSplitter lineSplitter = LineSplitter();
@@ -408,17 +403,14 @@ class DartTestGenerator implements Generator {
 
   @override
   void generate(StringSink sink, PigeonOptions options, Root root) {
-    final String mainPath = path.context.relative(
-      _posixify(options.dartOut!),
-      from: _posixify(path.dirname(options.dartTestOut!)),
-    );
     final DartOptions dartOptionsWithHeader = _dartOptionsWithCopyrightHeader(
         options.dartOptions, options.copyrightHeader);
     generateTestDart(
       dartOptionsWithHeader,
       root,
       sink,
-      mainPath,
+      dartOutPath: options.dartOut!,
+      testOutPath: options.dartTestOut!,
     );
   }
 

packages/pigeon/pubspec.yaml

@@ -2,7 +2,7 @@ name: pigeon
 description: Code generator tool to make communication between Flutter and the host platform type-safe and easier.
 repository: https://github.com/flutter/packages/tree/main/packages/pigeon
 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3Apigeon
-version: 3.2.7 # This must match the version in lib/generator_tools.dart
+version: 3.2.8 # This must match the version in lib/generator_tools.dart
 
 environment:
   sdk: ">=2.12.0 <3.0.0"
@@ -14,6 +14,7 @@ dependencies:
   collection: ^1.15.0
   meta: ^1.7.0
   path: ^1.8.0
+  yaml: ^3.1.1
 
 dev_dependencies:
   test: ^1.11.1

packages/pigeon/test/dart_generator_test.dart

@@ -2,6 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'dart:io' show Directory, File;
+
+import 'package:path/path.dart' as path;
 import 'package:pigeon/ast.dart';
 import 'package:pigeon/dart_generator.dart';
 import 'package:pigeon/generator_tools.dart';
@@ -609,7 +612,13 @@ void main() {
     expect(mainCode, isNot(contains('.ApiMock.doSomething')));
     expect(mainCode, isNot(contains('\'${Keys.result}\': output')));
     expect(mainCode, isNot(contains('return <Object, Object>{};')));
-    generateTestDart(const DartOptions(), root, testCodeSink, "fo'o.dart");
+    generateTestDart(
+      const DartOptions(),
+      root,
+      testCodeSink,
+      dartOutPath: "fo'o.dart",
+      testOutPath: 'test.dart',
+    );
     final String testCode = testCodeSink.toString();
     expect(testCode, contains('import \'fo\\\'o.dart\';'));
     expect(testCode, isNot(contains('class Api {')));
@@ -1181,4 +1190,30 @@ void main() {
     final String code = sink.toString();
     expect(code, contains('void doit(int? foo);'));
   });
+
+  test('deduces package name', () {
+    final Directory tempDir = Directory.systemTemp.createTempSync('pigeon');
+    try {
+      final Directory foo = Directory(path.join(tempDir.path, 'lib', 'foo'));
+      foo.createSync(recursive: true);
+      final File pubspecFile = File(path.join(tempDir.path, 'pubspec.yaml'));
+      pubspecFile.writeAsStringSync('''
+name: foobar
+''');
+      final Root root =
+          Root(classes: <Class>[], apis: <Api>[], enums: <Enum>[]);
+      final StringBuffer sink = StringBuffer();
+      generateTestDart(
+        const DartOptions(),
+        root,
+        sink,
+        dartOutPath: path.join(foo.path, 'bar.dart'),
+        testOutPath: path.join(tempDir.path, 'test', 'bar_test.dart'),
+      );
+      final String code = sink.toString();
+      expect(code, contains("import 'package:foobar/foo/bar.dart';"));
+    } finally {
+      tempDir.delete();
+    }
+  });
 }