From eb1578846f8ab53d9a44c3c411fe3bae93edb2cb Mon Sep 17 00:00:00 2001 From: gaaclarke <30870216+gaaclarke@users.noreply.github.com> Date: Mon, 22 Aug 2022 11:10:17 -0700 Subject: [PATCH] [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 --- packages/pigeon/CHANGELOG.md | 5 ++ packages/pigeon/lib/dart_generator.dart | 87 +++++++++++++++++-- packages/pigeon/lib/generator_tools.dart | 2 +- packages/pigeon/lib/pigeon_lib.dart | 12 +-- packages/pigeon/pubspec.yaml | 3 +- packages/pigeon/test/dart_generator_test.dart | 37 +++++++- 6 files changed, 124 insertions(+), 22 deletions(-) diff --git a/packages/pigeon/CHANGELOG.md b/packages/pigeon/CHANGELOG.md index 0bc6ef863225..37b2095be426 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 0fee4aef8a20..a81fe5f9d663 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 @@ 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 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)['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( diff --git a/packages/pigeon/lib/generator_tools.dart b/packages/pigeon/lib/generator_tools.dart index 0eb3b13feb12..277806b1f715 100644 --- a/packages/pigeon/lib/generator_tools.dart +++ b/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() { diff --git a/packages/pigeon/lib/pigeon_lib.dart b/packages/pigeon/lib/pigeon_lib.dart index 697e42f3eb7d..9ac5c6842dff 100644 --- a/packages/pigeon/lib/pigeon_lib.dart +++ b/packages/pigeon/lib/pigeon_lib.dart @@ -313,11 +313,6 @@ class ParseResults { final Map? pigeonOptions; } -String _posixify(String input) { - final path.Context context = path.Context(style: path.Style.posix); - return context.fromUri(path.toUri(path.absolute(input))); -} - Iterable _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!, ); } diff --git a/packages/pigeon/pubspec.yaml b/packages/pigeon/pubspec.yaml index 540ea8af7636..ad409502aa5f 100644 --- a/packages/pigeon/pubspec.yaml +++ b/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 diff --git a/packages/pigeon/test/dart_generator_test.dart b/packages/pigeon/test/dart_generator_test.dart index ea9c569bf9d6..e8df05d7708f 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 @@ void main() { expect(mainCode, isNot(contains('.ApiMock.doSomething'))); expect(mainCode, isNot(contains('\'${Keys.result}\': output'))); expect(mainCode, isNot(contains('return {};'))); - 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: [], apis: [], enums: []); + 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(); + } + }); }