Deprecate --packages flag and add --package (dart-archive/coverage#370)

* Migrate tests to package_config.json

* Swap --packages flag for --package

* Fix analysis errors

* Keep the --packages flag, but delete support for .package in resolver

* Make separate private constructor for Resolver

* Improve resolver test coverage

* Switch readme example commands from `pub` to `dart pub`

* 1.2.0 is published, so move changes to 1.3.0-dev

Author: liamappelbe

pkgs/coverage/CHANGELOG.md

@@ -1,3 +1,9 @@
+## 1.3.0-dev
+* Add a `--package` flag, which takes the package's root directory, instead of
+  the .package file. Deprecate the `--packages` flag.
+* Deprecate the packagesPath parameter and add packagePath instead, in
+  `HitMap.parseJson`, `HitMap.parseFiles`, `createHitmap`, and `parseCoverage`.
+
 ## 1.2.0
 
 * Support branch level coverage information, when running tests in the Dart VM.

pkgs/coverage/README.md

@@ -16,18 +16,18 @@ Tools
 
     dart pub global activate coverage
 
-Consider adding the `pub global run` executables directory to your path.
+Consider adding the `dart pub global run` executables directory to your path.
 See [Running a script from your PATH](https://dart.dev/tools/pub/cmd/pub-global#running-a-script-from-your-path)
 for more details.
 
 #### Collecting coverage from the VM
 
 ```
 dart --pause-isolates-on-exit --disable-service-auth-codes --enable-vm-service=NNNN script.dart
-pub global run coverage:collect_coverage --uri=http://... -o coverage.json --resume-isolates
+dart pub global run coverage:collect_coverage --uri=http://... -o coverage.json --resume-isolates
 ```
 
-or if the `pub global run` executables are on your PATH,
+or if the `dart pub global run` executables are on your PATH,
 
 ```
 collect_coverage --uri=http://... -o coverage.json --resume-isolates
@@ -44,17 +44,18 @@ all isolates are paused before collecting coverage.
 #### Formatting coverage data
 
 ```
-pub global run coverage:format_coverage --packages=app_package/.packages -i coverage.json
+dart pub global run coverage:format_coverage --package=app_package -i coverage.json
 ```
 
-or if the `pub global run` exectuables are on your PATH,
+or if the `dart pub global run` exectuables are on your PATH,
 
 ```
-format_coverage --packages=app_package/.packages -i coverage.json
+format_coverage --package=app_package -i coverage.json
 ```
 
 where `app_package` is the path to the package whose coverage is being
-collected. If `--sdk-root` is set, Dart SDK coverage will also be output.
+collected (defaults to the current working directory). If `--sdk-root` is set,
+Dart SDK coverage will also be output.
 
 #### Ignore lines from coverage
 
@@ -69,15 +70,15 @@ collect_coverage:
 
 ```
 dart --pause-isolates-on-exit --disable-service-auth-codes --enable-vm-service=NNNN script.dart
-pub global run coverage:collect_coverage --uri=http://... -o coverage.json --resume-isolates --function-coverage
+dart pub global run coverage:collect_coverage --uri=http://... -o coverage.json --resume-isolates --function-coverage
 ```
 
 To gather branch level coverage information, pass `--branch-coverage` to *both*
 collect_coverage and the Dart command you're gathering coverage from:
 
 ```
 dart --pause-isolates-on-exit --disable-service-auth-codes --enable-vm-service=NNNN --branch-coverage script.dart
-pub global run coverage:collect_coverage --uri=http://... -o coverage.json --resume-isolates --branch-coverage
+dart pub global run coverage:collect_coverage --uri=http://... -o coverage.json --resume-isolates --branch-coverage
 ```
 
 Branch coverage requires Dart VM 2.17.0, with service API v3.56. Function,
@@ -86,5 +87,5 @@ those flags:
 
 ```
 dart --pause-isolates-on-exit --disable-service-auth-codes --enable-vm-service=NNNN --branch-coverage script.dart
-pub global run coverage:collect_coverage --uri=http://... -o coverage.json --resume-isolates --function-coverage --branch-coverage
+dart pub global run coverage:collect_coverage --uri=http://... -o coverage.json --resume-isolates --function-coverage --branch-coverage
 ```

pkgs/coverage/bin/format_coverage.dart

@@ -19,6 +19,7 @@ class Environment {
     required this.lcov,
     required this.output,
     required this.packagesPath,
+    required this.packagePath,
     required this.prettyPrint,
     required this.prettyPrintFunc,
     required this.prettyPrintBranch,
@@ -36,6 +37,7 @@ class Environment {
   bool lcov;
   IOSink output;
   String? packagesPath;
+  String packagePath;
   bool prettyPrint;
   bool prettyPrintFunc;
   bool prettyPrintBranch;
@@ -54,7 +56,8 @@ Future<void> main(List<String> arguments) async {
     print('  # files: ${files.length}');
     print('  # workers: ${env.workers}');
     print('  sdk-root: ${env.sdkRoot}');
-    print('  package-spec: ${env.packagesPath}');
+    print('  package-path: ${env.packagePath}');
+    print('  packages-path: ${env.packagesPath}');
     print('  report-on: ${env.reportOn}');
     print('  check-ignore: ${env.checkIgnore}');
   }
@@ -63,7 +66,9 @@ Future<void> main(List<String> arguments) async {
   final hitmap = await HitMap.parseFiles(
     files,
     checkIgnoredLines: env.checkIgnore,
+    // ignore: deprecated_member_use_from_same_package
     packagesPath: env.packagesPath,
+    packagePath: env.packagePath,
   );
 
   // All workers are done. Process the data.
@@ -74,7 +79,11 @@ Future<void> main(List<String> arguments) async {
   String output;
   final resolver = env.bazel
       ? BazelResolver(workspacePath: env.bazelWorkspace)
-      : Resolver(packagesPath: env.packagesPath, sdkRoot: env.sdkRoot);
+      : await Resolver.create(
+          packagesPath: env.packagesPath,
+          packagePath: env.packagePath,
+          sdkRoot: env.sdkRoot,
+        );
   final loader = Loader();
   if (env.prettyPrint) {
     output = await hitmap.prettyPrint(resolver, loader,
@@ -116,7 +125,10 @@ Environment parseArgs(List<String> arguments) {
   final parser = ArgParser();
 
   parser.addOption('sdk-root', abbr: 's', help: 'path to the SDK root');
-  parser.addOption('packages', help: 'path to the package spec file');
+  parser.addOption('packages',
+      help: '[DEPRECATED] path to the package spec file');
+  parser.addOption('package',
+      help: 'root directory of the package', defaultsTo: '.');
   parser.addOption('in', abbr: 'i', help: 'input(s): may be file or directory');
   parser.addOption('out',
       abbr: 'o', defaultsTo: 'stdout', help: 'output: may be file or stdout');
@@ -191,6 +203,11 @@ Environment parseArgs(List<String> arguments) {
     }
   }
 
+  final packagePath = args['package'] as String;
+  if (!FileSystemEntity.isDirectorySync(packagePath)) {
+    fail('Package spec "${args["package"]}" not found, or not a directory.');
+  }
+
   if (args['in'] == null) fail('No input files given.');
   final input = p.absolute(p.normalize(args['in'] as String));
   if (!FileSystemEntity.isDirectorySync(input) &&
@@ -254,6 +271,7 @@ Environment parseArgs(List<String> arguments) {
       lcov: lcov,
       output: output,
       packagesPath: packagesPath,
+      packagePath: packagePath,
       prettyPrint: prettyPrint,
       prettyPrintFunc: prettyPrintFunc,
       prettyPrintBranch: prettyPrintBranch,

pkgs/coverage/lib/src/hitmap.dart

@@ -39,9 +39,11 @@ class HitMap {
   static Future<Map<String, HitMap>> parseJson(
     List<Map<String, dynamic>> jsonResult, {
     bool checkIgnoredLines = false,
-    String? packagesPath,
+    @Deprecated('Use packagePath') String? packagesPath,
+    String? packagePath,
   }) async {
-    final resolver = Resolver(packagesPath: packagesPath);
+    final resolver = await Resolver.create(
+        packagesPath: packagesPath, packagePath: packagePath);
     final loader = Loader();
 
     // Map of source file to map of line to hit count for that line.
@@ -159,7 +161,8 @@ class HitMap {
   static Future<Map<String, HitMap>> parseFiles(
     Iterable<File> files, {
     bool checkIgnoredLines = false,
-    String? packagesPath,
+    @Deprecated('Use packagePath') String? packagesPath,
+    String? packagePath,
   }) async {
     final globalHitmap = <String, HitMap>{};
     for (var file in files) {
@@ -170,7 +173,9 @@ class HitMap {
         globalHitmap.merge(await HitMap.parseJson(
           jsonResult.cast<Map<String, dynamic>>(),
           checkIgnoredLines: checkIgnoredLines,
+          // ignore: deprecated_member_use_from_same_package
           packagesPath: packagesPath,
+          packagePath: packagePath,
         ));
       }
     }
@@ -239,12 +244,14 @@ class _HitInfo {
 Future<Map<String, Map<int, int>>> createHitmap(
   List<Map<String, dynamic>> jsonResult, {
   bool checkIgnoredLines = false,
-  String? packagesPath,
+  @Deprecated('Use packagePath') String? packagesPath,
+  String? packagePath,
 }) async {
   final result = await HitMap.parseJson(
     jsonResult,
     checkIgnoredLines: checkIgnoredLines,
     packagesPath: packagesPath,
+    packagePath: packagePath,
   );
   return result.map((key, value) => MapEntry(key, value.lineHits));
 }
@@ -276,10 +283,13 @@ Future<Map<String, Map<int, int>>> parseCoverage(
   Iterable<File> files,
   int _, {
   bool checkIgnoredLines = false,
-  String? packagesPath,
+  @Deprecated('Use packagePath') String? packagesPath,
+  String? packagePath,
 }) async {
   final result = await HitMap.parseFiles(files,
-      checkIgnoredLines: checkIgnoredLines, packagesPath: packagesPath);
+      checkIgnoredLines: checkIgnoredLines,
+      packagesPath: packagesPath,
+      packagePath: packagePath);
   return result.map((key, value) => MapEntry(key, value.lineHits));
 }
 

pkgs/coverage/lib/src/resolver.dart

@@ -2,18 +2,42 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
-import 'dart:convert';
 import 'dart:io';
 
 import 'package:package_config/package_config.dart';
 import 'package:path/path.dart' as p;
 
 /// [Resolver] resolves imports with respect to a given environment.
 class Resolver {
+  @Deprecated('Use Resolver.create')
   Resolver({this.packagesPath, this.sdkRoot})
-      : _packages = packagesPath != null ? _parsePackages(packagesPath) : null;
+      : _packages = packagesPath != null ? _parsePackages(packagesPath) : null,
+        packagePath = null;
+
+  Resolver._(
+      {this.packagesPath,
+      this.packagePath,
+      this.sdkRoot,
+      Map<String, Uri>? packages})
+      : _packages = packages;
+
+  static Future<Resolver> create({
+    String? packagesPath,
+    String? packagePath,
+    String? sdkRoot,
+  }) async {
+    return Resolver._(
+      packagesPath: packagesPath,
+      packagePath: packagePath,
+      sdkRoot: sdkRoot,
+      packages: packagesPath != null
+          ? _parsePackages(packagesPath)
+          : (packagePath != null ? await _parsePackage(packagePath) : null),
+    );
+  }
 
   final String? packagesPath;
+  final String? packagePath;
   final String? sdkRoot;
   final List<String> failed = [];
   final Map<String, Uri>? _packages;
@@ -86,32 +110,20 @@ class Resolver {
 
   static Map<String, Uri> _parsePackages(String packagesPath) {
     final content = File(packagesPath).readAsStringSync();
-    try {
-      final packagesUri = p.toUri(packagesPath);
-      final parsed =
-          PackageConfig.parseString(content, Uri.base.resolveUri(packagesUri));
-      return {
-        for (var package in parsed.packages)
-          package.name: package.packageUriRoot
-      };
-    } on FormatException catch (_) {
-      // It was probably an old style .packages file
-      final lines = LineSplitter.split(content);
-      final packageMap = <String, Uri>{};
-      for (var line in lines) {
-        if (line.startsWith('#')) continue;
-        final firstColon = line.indexOf(':');
-        if (firstColon == -1) {
-          throw FormatException(
-              'Unexpected package config format, expected an old style '
-              '.packages file or new style package_config.json file.',
-              content);
-        }
-        packageMap[line.substring(0, firstColon)] =
-            Uri.parse(line.substring(firstColon + 1, line.length));
-      }
-      return packageMap;
-    }
+    final packagesUri = p.toUri(packagesPath);
+    final parsed =
+        PackageConfig.parseString(content, Uri.base.resolveUri(packagesUri));
+    return {
+      for (var package in parsed.packages) package.name: package.packageUriRoot
+    };
+  }
+
+  static Future<Map<String, Uri>?> _parsePackage(String packagePath) async {
+    final parsed = await findPackageConfig(Directory(packagePath));
+    if (parsed == null) return null;
+    return {
+      for (var package in parsed.packages) package.name: package.packageUriRoot
+    };
   }
 }
 

pkgs/coverage/pubspec.yaml

@@ -1,5 +1,5 @@
 name: coverage
-version: 1.2.0
+version: 1.3.0-dev
 description: Coverage data manipulation and formatting
 homepage: https://github.com/dart-lang/coverage
 

pkgs/coverage/test/collect_coverage_test.dart

@@ -299,8 +299,7 @@ void main() {
       await outputFile.writeAsString(coverageResults, flush: true);
 
       final parsedResult = await HitMap.parseFiles([outputFile],
-          packagesPath: '.dart_tool/package_config.json',
-          checkIgnoredLines: true);
+          packagePath: '.', checkIgnoredLines: true);
 
       // This file has ignore:coverage-file.
       expect(parsedResult, isNot(contains(_sampleAppFileUri)));