[pigeon] Deduces the package name for dart test file imports. (#2467)
* [pigeon] Deduces the package name for dart test file imports.
* stuart feedback
* stuart feedback 2
diff --git a/packages/pigeon/CHANGELOG.md b/packages/pigeon/CHANGELOG.md
index 0bc6ef8..37b2095 100644
--- a/packages/pigeon/CHANGELOG.md
+++ b/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.
diff --git a/packages/pigeon/lib/dart_generator.dart b/packages/pigeon/lib/dart_generator.dart
index 0fee4ae..a81fe5f 100644
--- a/packages/pigeon/lib/dart_generator.dart
+++ b/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 @@
}
}
-/// 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 @@
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(
diff --git a/packages/pigeon/lib/generator_tools.dart b/packages/pigeon/lib/generator_tools.dart
index 0eb3b13..277806b 100644
--- a/packages/pigeon/lib/generator_tools.dart
+++ b/packages/pigeon/lib/generator_tools.dart
@@ -9,7 +9,7 @@
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() {
diff --git a/packages/pigeon/lib/pigeon_lib.dart b/packages/pigeon/lib/pigeon_lib.dart
index 697e42f..9ac5c68 100644
--- a/packages/pigeon/lib/pigeon_lib.dart
+++ b/packages/pigeon/lib/pigeon_lib.dart
@@ -313,11 +313,6 @@
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 @@
@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!,
);
}
diff --git a/packages/pigeon/pubspec.yaml b/packages/pigeon/pubspec.yaml
index 540ea8a..ad40950 100644
--- a/packages/pigeon/pubspec.yaml
+++ b/packages/pigeon/pubspec.yaml
@@ -2,7 +2,7 @@
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 @@
collection: ^1.15.0
meta: ^1.7.0
path: ^1.8.0
+ yaml: ^3.1.1
dev_dependencies:
test: ^1.11.1
diff --git a/packages/pigeon/test/dart_generator_test.dart b/packages/pigeon/test/dart_generator_test.dart
index ea9c569..e8df05d 100644
--- a/packages/pigeon/test/dart_generator_test.dart
+++ b/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 @@
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 @@
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();
+ }
+ });
}