Make error handling optionally more permissive. (dart-lang/package_config#58) * Make error handling optionally more permissive. * Make the LanguageVersion be an object, not a string. * Make it version 3.0.0-dev
diff --git a/pkgs/package_config/CHANGELOG.md b/pkgs/package_config/CHANGELOG.md index 84384fc..3171840 100644 --- a/pkgs/package_config/CHANGELOG.md +++ b/pkgs/package_config/CHANGELOG.md
@@ -1,9 +1,22 @@ +## 3.0.0 + +- Make the language version be represented as a `LanguageVersion` class + instead of a string. +- Made error handling interceptable. Passing an `onError` handler + makes the parsers attempt to do a best-effort error correction after + detecting an error. +- Do not require root URIs to have paths starting with `/`. That + only makes sense for `file` or `http`, and they enforce it anyway. +- Fixed bug in language version validation not accepting the digit `9`. + ## 2.0.0 - - Based on new JSON file format with more content. + +- Based on new JSON file format with more content. ## 1.2.0 - - Added support for writing default-package entries. - - Fixed bug when writing `Uri`s containing a fragment. + +- Added support for writing default-package entries. +- Fixed bug when writing `Uri`s containing a fragment. ## 1.1.0
diff --git a/pkgs/package_config/README.md b/pkgs/package_config/README.md index 073a1c7..45791aa 100644 --- a/pkgs/package_config/README.md +++ b/pkgs/package_config/README.md
@@ -3,11 +3,7 @@ Support for working with **Package Configuration** files as described in the Package Configuration v2 [design document](https://github.com/dart-lang/language/blob/master/accepted/future-releases/language-versioning/package-config-file-v2.md). -This version of the `package_config` package is also available as `package_config_2`, -which can be used by packages which transitively depend on a version of `package_config` -with a version <2.0.0. - -[](https://travis-ci.org/dart-lang/package_config) [](https://pub.dartlang.org/packages/package_config) +[](https://travis-ci.org/dart-lang/package_config) [](https://pub.dartlang.org/packages/package_config) ## Features and bugs
diff --git a/pkgs/package_config/lib/package_config.dart b/pkgs/package_config/lib/package_config.dart index 77e8d61..98b31f1 100644 --- a/pkgs/package_config/lib/package_config.dart +++ b/pkgs/package_config/lib/package_config.dart
@@ -8,11 +8,14 @@ import "dart:io" show File, Directory; import "dart:typed_data" show Uint8List; + import "src/discovery.dart" as discover; +import "src/errors.dart" show throwError; import "src/package_config.dart"; import "src/package_config_json.dart"; -export "src/package_config.dart" show PackageConfig, Package; +export "src/package_config.dart" + show PackageConfig, Package, LanguageVersion, InvalidLanguageVersion; export "src/errors.dart" show PackageConfigError; /// Reads a specific package configuration file. @@ -30,9 +33,15 @@ /// is loaded even if there is an available `package_config.json` file. /// The caller can determine this from the [PackageConfig.version] /// being 1 and look for a `package_config.json` file themselves. +/// +/// If [onError] is provided, the configuration file parsing will report errors +/// by calling that function, and then try to recover. +/// The returned package configuration is a *best effort* attempt to create +/// a valid configuration from the invalid configuration file. +/// If no [onError] is provided, errors are thrown immediately. Future<PackageConfig> loadPackageConfig(File file, - {bool preferNewest = true}) => - readAnyConfigFile(file, preferNewest); + {bool preferNewest = true, void onError(Object error)}) => + readAnyConfigFile(file, preferNewest, onError ?? throwError); /// Reads a specific package configuration URI. /// @@ -68,10 +77,17 @@ /// of an I/O issue, as long as the location URIs are valid. /// As such, it does not distinguish between a file not existing, /// and it being temporarily locked or unreachable. +/// +/// If [onError] is provided, the configuration file parsing will report errors +/// by calling that function, and then try to recover. +/// The returned package configuration is a *best effort* attempt to create +/// a valid configuration from the invalid configuration file. +/// If no [onError] is provided, errors are thrown immediately. Future<PackageConfig> loadPackageConfigUri(Uri file, {Future<Uint8List /*?*/ > loader(Uri uri) /*?*/, - bool preferNewest = true}) => - readAnyConfigFileUri(file, loader, preferNewest); + bool preferNewest = true, + void onError(Object error)}) => + readAnyConfigFileUri(file, loader, onError ?? throwError, preferNewest); /// Finds a package configuration relative to [directory]. /// @@ -86,10 +102,16 @@ /// If [recurse] is set to [false], this parent directory check is not /// performed. /// +/// If [onError] is provided, the configuration file parsing will report errors +/// by calling that function, and then try to recover. +/// The returned package configuration is a *best effort* attempt to create +/// a valid configuration from the invalid configuration file. +/// If no [onError] is provided, errors are thrown immediately. +/// /// Returns `null` if no configuration file is found. Future<PackageConfig> findPackageConfig(Directory directory, - {bool recurse = true}) => - discover.findPackageConfig(directory, recurse); + {bool recurse = true, void onError(Object error)}) => + discover.findPackageConfig(directory, recurse, onError ?? throwError); /// Finds a package configuration relative to [location]. /// @@ -124,10 +146,19 @@ /// As such, it does not distinguish between a file not existing, /// and it being temporarily locked or unreachable. /// +/// If [onError] is provided, the configuration file parsing will report errors +/// by calling that function, and then try to recover. +/// The returned package configuration is a *best effort* attempt to create +/// a valid configuration from the invalid configuration file. +/// If no [onError] is provided, errors are thrown immediately. +/// /// Returns `null` if no configuration file is found. Future<PackageConfig> findPackageConfigUri(Uri location, - {bool recurse = true, Future<Uint8List /*?*/ > loader(Uri uri)}) => - discover.findPackageConfigUri(location, loader, recurse); + {bool recurse = true, + Future<Uint8List /*?*/ > loader(Uri uri), + void onError(Object error)}) => + discover.findPackageConfigUri( + location, loader, onError ?? throwError, recurse); /// Writes a package configuration to the provided directory. ///
diff --git a/pkgs/package_config/lib/src/discovery.dart b/pkgs/package_config/lib/src/discovery.dart index 5ad6ac5..14033ed 100644 --- a/pkgs/package_config/lib/src/discovery.dart +++ b/pkgs/package_config/lib/src/discovery.dart
@@ -5,13 +5,11 @@ import "dart:io"; import 'dart:typed_data'; -import "package:path/path.dart" as p; - import "errors.dart"; import "package_config_impl.dart"; import "package_config_json.dart"; import "packages_file.dart" as packages_file; -import "util.dart" show defaultLoader; +import "util.dart" show defaultLoader, pathJoin; final Uri packageConfigJsonPath = Uri(path: ".dart_tool/package_config.json"); final Uri dotPackagesPath = Uri(path: ".packages"); @@ -33,7 +31,7 @@ /// Returns `null` if no configuration was found. If a configuration /// is needed, then the caller can supply [PackageConfig.empty]. Future<PackageConfig /*?*/ > findPackageConfig( - Directory baseDirectory, bool recursive) async { + Directory baseDirectory, bool recursive, void onError(Object error)) async { var directory = baseDirectory; if (!directory.isAbsolute) directory = directory.absolute; if (!await directory.exists()) { @@ -41,7 +39,7 @@ } do { // Check for $cwd/.packages - var packageConfig = await findPackagConfigInDirectory(directory); + var packageConfig = await findPackagConfigInDirectory(directory, onError); if (packageConfig != null) return packageConfig; if (!recursive) break; // Check in parent directories. @@ -53,16 +51,22 @@ } /// Similar to [findPackageConfig] but based on a URI. -Future<PackageConfig /*?*/ > findPackageConfigUri(Uri location, - Future<Uint8List /*?*/ > loader(Uri uri) /*?*/, bool recursive) async { +Future<PackageConfig /*?*/ > findPackageConfigUri( + Uri location, + Future<Uint8List /*?*/ > loader(Uri uri) /*?*/, + void onError(Object error) /*?*/, + bool recursive) async { if (location.isScheme("package")) { - throw PackageConfigArgumentError( - location, "location", "Must not be a package: URI"); + onError(PackageConfigArgumentError( + location, "location", "Must not be a package: URI")); + return null; } if (loader == null) { if (location.isScheme("file")) { return findPackageConfig( - Directory.fromUri(location.resolveUri(currentPath)), recursive); + Directory.fromUri(location.resolveUri(currentPath)), + recursive, + onError); } loader = defaultLoader; } @@ -71,12 +75,12 @@ var file = location.resolveUri(packageConfigJsonPath); var bytes = await loader(file); if (bytes != null) { - return parsePackageConfigBytes(bytes, file); + return parsePackageConfigBytes(bytes, file, onError); } file = location.resolveUri(dotPackagesPath); bytes = await loader(file); if (bytes != null) { - return packages_file.parse(bytes, file); + return packages_file.parse(bytes, file, onError); } if (!recursive) break; var parent = location.resolveUri(parentPath); @@ -90,33 +94,35 @@ /// /// Loads the file, if it is there, and returns the resulting [PackageConfig]. /// Returns `null` if the file isn't there. -/// Throws [FormatException] if a file is there but is not valid. +/// Reports a [FormatException] if a file is there but the content is not valid. +/// If the file exists, but fails to be read, the file system error is reported. /// -/// If [extraData] is supplied and the `package_config.json` contains extra -/// entries in the top JSON object, those extra entries are stored into -/// [extraData]. +/// If [onError] is supplied, parsing errors are reported using that, and +/// a best-effort attempt is made to return a package configuration. +/// This may be the empty package configuration. Future<PackageConfig /*?*/ > findPackagConfigInDirectory( - Directory directory) async { + Directory directory, void onError(Object error)) async { var packageConfigFile = await checkForPackageConfigJsonFile(directory); if (packageConfigFile != null) { - return await readPackageConfigJsonFile(packageConfigFile); + return await readPackageConfigJsonFile(packageConfigFile, onError); } packageConfigFile = await checkForDotPackagesFile(directory); if (packageConfigFile != null) { - return await readDotPackagesFile(packageConfigFile); + return await readDotPackagesFile(packageConfigFile, onError); } return null; } Future<File> /*?*/ checkForPackageConfigJsonFile(Directory directory) async { assert(directory.isAbsolute); - var file = File(p.join(directory.path, ".dart_tool", "package_config.json")); + var file = + File(pathJoin(directory.path, ".dart_tool", "package_config.json")); if (await file.exists()) return file; return null; } Future<File /*?*/ > checkForDotPackagesFile(Directory directory) async { - var file = File(p.join(directory.path, ".packages")); + var file = File(pathJoin(directory.path, ".packages")); if (await file.exists()) return file; return null; }
diff --git a/pkgs/package_config/lib/src/errors.dart b/pkgs/package_config/lib/src/errors.dart index 6c31cce..c973617 100644 --- a/pkgs/package_config/lib/src/errors.dart +++ b/pkgs/package_config/lib/src/errors.dart
@@ -1,24 +1,33 @@ -// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file -// 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. - -/// General superclass of most errors and exceptions thrown by this package. -/// -/// Only covers errors thrown while parsing package configuration files. -/// Programming errors and I/O exceptions are not covered. -abstract class PackageConfigError { - PackageConfigError._(); -} - -class PackageConfigArgumentError extends ArgumentError - implements PackageConfigError { - PackageConfigArgumentError(Object /*?*/ value, String name, String message) - : super.value(value, name, message); -} - -class PackageConfigFormatException extends FormatException - implements PackageConfigError { - PackageConfigFormatException(String message, Object /*?*/ value, - [int /*?*/ index]) - : super(message, value, index); -} +// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file +// 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. + +/// General superclass of most errors and exceptions thrown by this package. +/// +/// Only covers errors thrown while parsing package configuration files. +/// Programming errors and I/O exceptions are not covered. +abstract class PackageConfigError { + PackageConfigError._(); +} + +class PackageConfigArgumentError extends ArgumentError + implements PackageConfigError { + PackageConfigArgumentError(Object /*?*/ value, String name, String message) + : super.value(value, name, message); + + PackageConfigArgumentError.from(ArgumentError error) + : super.value(error.invalidValue, error.name, error.message); +} + +class PackageConfigFormatException extends FormatException + implements PackageConfigError { + PackageConfigFormatException(String message, Object /*?*/ source, + [int /*?*/ offset]) + : super(message, source, offset); + + PackageConfigFormatException.from(FormatException exception) + : super(exception.message, exception.source, exception.offset); +} + +/// The default `onError` handler. +void /*Never*/ throwError(Object error) => throw error;
diff --git a/pkgs/package_config/lib/src/package_config.dart b/pkgs/package_config/lib/src/package_config.dart index f7b96b8..08c4a96 100644 --- a/pkgs/package_config/lib/src/package_config.dart +++ b/pkgs/package_config/lib/src/package_config.dart
@@ -2,6 +2,7 @@ // 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 'errors.dart'; import "package_config_impl.dart"; /// A package configuration. @@ -34,7 +35,7 @@ /// /// The version of the resulting configuration is always [maxVersion]. factory PackageConfig(Iterable<Package> packages, {dynamic extraData}) => - SimplePackageConfig(maxVersion, packages); + SimplePackageConfig(maxVersion, packages, extraData); /// The configuration version number. /// @@ -124,9 +125,10 @@ /// [Package.extraData] of the created package. factory Package(String name, Uri root, {Uri /*?*/ packageUriRoot, - String /*?*/ languageVersion, + LanguageVersion /*?*/ languageVersion, dynamic extraData}) => - SimplePackage(name, root, packageUriRoot, languageVersion, extraData); + SimplePackage.validate( + name, root, packageUriRoot, languageVersion, extraData, throwError); /// The package-name of the package. String get name; @@ -156,16 +158,9 @@ /// Each package may have a default language version associated, /// which is the language version used to parse and compile /// Dart files in the package. - /// A package version is always of the form: - /// - /// * A numeral consisting of one or more decimal digits, - /// with no leading zero unless the entire numeral is a single zero digit. - /// * Followed by a `.` character. - /// * Followed by another numeral of the same form. - /// - /// There is no whitespace allowed around the numerals. - /// Valid version numbers include `2.5`, `3.0`, and `1234.5678`. - String /*?*/ get languageVersion; + /// A package version is defined by two non-negative numbers, + /// the *major* and *minor* version numbers. + LanguageVersion /*?*/ get languageVersion; /// Extra data associated with the specific package. /// @@ -174,3 +169,104 @@ /// JSON-like list/map data structures. dynamic get extraData; } + +/// A language version. +/// +/// A language version is represented by two non-negative integers, +/// the [major] and [minor] version numbers. +/// +/// If errors during parsing are handled using an `onError` handler, +/// then an *invalid* language version may be represented by an +/// [InvalidLanguageVersion] object. +abstract class LanguageVersion implements Comparable<LanguageVersion> { + /// The maximal value allowed by [major] and [minor] values; + static const int maxValue = 0x7FFFFFFF; + factory LanguageVersion(int major, int minor) { + RangeError.checkValueInInterval(major, 0, maxValue, "major"); + RangeError.checkValueInInterval(minor, 0, maxValue, "major"); + return SimpleLanguageVersion(major, minor, null); + } + + /// Parses a language version string. + /// + /// A valid language version string has the form + /// + /// > *decimalNumber* `.` *decimalNumber* + /// + /// where a *decimalNumber* is a non-empty sequence of decimal digits + /// with no unnecessary leading zeros (the decimal number only starts + /// with a zero digit if that digit is the entire number). + /// No spaces are allowed in the string. + /// + /// If the [source] is valid then it is parsed into a valid + /// [LanguageVersion] object. + /// If not, then the [onError] is called with a [FormatException]. + /// If [onError] is not supplied, it defaults to throwing the exception. + /// If the call does not throw, then an [InvalidLanguageVersion] is returned + /// containing the original [source]. + static LanguageVersion parse(String source, {void onError(Object error)}) => + parseLanguageVersion(source, onError ?? throwError); + + /// The major language version. + /// + /// A non-negative integer less than 2<sup>31</sup>. + /// + /// The value is negative for objects representing *invalid* language + /// versions ([InvalidLanguageVersion]). + int get major; + + /// The minor language version. + /// + /// A non-negative integer less than 2<sup>31</sup>. + /// + /// The value is negative for objects representing *invalid* language + /// versions ([InvalidLanguageVersion]). + int get minor; + + /// Compares language versions. + /// + /// Two language versions are considered equal if they have the + /// same major and minor version numbers. + /// + /// A language version is greater then another if the former's major version + /// is greater than the latter's major version, or if they have + /// the same major version and the former's minor version is greater than + /// the latter's. + int compareTo(LanguageVersion other); + + /// Valid language versions with the same [major] and [minor] values are + /// equal. + /// + /// Invalid language versions ([InvalidLanguageVersion]) are not equal to + /// any other object. + bool operator ==(Object other); + + int get hashCode; + + /// A string representation of the language version. + /// + /// A valid language version is represented as + /// `"${version.major}.${version.minor}"`. + String toString(); +} + +/// An *invalid* language version. +/// +/// Stored in a [Package] when the orginal language version string +/// was invalid and a `onError` handler was passed to the parser +/// which did not throw on an error. +abstract class InvalidLanguageVersion implements LanguageVersion { + /// The value -1 for an invalid language version. + int get major; + + /// The value -1 for an invalid language version. + int get minor; + + /// An invalid language version is only equal to itself. + bool operator ==(Object other); + + int get hashCode; + + /// The original invalid version string. + String toString(); +}
diff --git a/pkgs/package_config/lib/src/package_config_impl.dart b/pkgs/package_config/lib/src/package_config_impl.dart index 6257961..39633fe 100644 --- a/pkgs/package_config/lib/src/package_config_impl.dart +++ b/pkgs/package_config/lib/src/package_config_impl.dart
@@ -14,16 +14,18 @@ final PackageTree _packageTree; final dynamic extraData; - SimplePackageConfig(int version, Iterable<Package> packages, - [dynamic extraData]) - : this._(_validateVersion(version), packages, - [...packages]..sort(_compareRoot), extraData); + factory SimplePackageConfig(int version, Iterable<Package> packages, + [dynamic extraData, void onError(Object error)]) { + onError ??= throwError; + var validVersion = _validateVersion(version, onError); + var sortedPackages = [...packages]..sort(_compareRoot); + var packageTree = _validatePackages(packages, sortedPackages, onError); + return SimplePackageConfig._(validVersion, packageTree, + {for (var p in packageTree.allPackages) p.name: p}, extraData); + } - /// Expects a list of [packages] sorted on root path. - SimplePackageConfig._(this.version, Iterable<Package> originalPackages, - List<Package> packages, this.extraData) - : _packageTree = _validatePackages(originalPackages, packages), - _packages = {for (var p in packages) p.name: p}; + SimplePackageConfig._( + this.version, this._packageTree, this._packages, this.extraData); /// Creates empty configuration. /// @@ -35,66 +37,78 @@ _packages = const <String, Package>{}, extraData = null; - static int _validateVersion(int version) { + static int _validateVersion(int version, void onError(Object error)) { if (version < 0 || version > PackageConfig.maxVersion) { - throw PackageConfigArgumentError(version, "version", - "Must be in the range 1 to ${PackageConfig.maxVersion}"); + onError(PackageConfigArgumentError(version, "version", + "Must be in the range 1 to ${PackageConfig.maxVersion}")); + return 2; // The minimal version supporting a SimplePackageConfig. } return version; } - static PackageTree _validatePackages( - Iterable<Package> originalPackages, List<Package> packages) { - // Assumes packages are sorted. - Map<String, Package> result = {}; + static PackageTree _validatePackages(Iterable<Package> originalPackages, + List<Package> packages, void onError(Object error)) { + var packageNames = <String>{}; var tree = MutablePackageTree(); - SimplePackage package; for (var originalPackage in packages) { + if (originalPackage == null) { + onError(ArgumentError.notNull("element of packages")); + continue; + } + SimplePackage package; if (originalPackage is! SimplePackage) { // SimplePackage validates these properties. - try { - package = SimplePackage( - originalPackage.name, - originalPackage.root, - originalPackage.packageUriRoot, - originalPackage.languageVersion, - originalPackage.extraData); - } catch (e) { - throw PackageConfigArgumentError( - packages, "packages", "Package ${package.name}: ${e.message}"); - } + package = SimplePackage.validate( + originalPackage.name, + originalPackage.root, + originalPackage.packageUriRoot, + originalPackage.languageVersion, + originalPackage.extraData, (error) { + if (error is PackageConfigArgumentError) { + onError(PackageConfigArgumentError(packages, "packages", + "Package ${package.name}: ${error.message}")); + } else { + onError(error); + } + }); + if (package == null) continue; } else { package = originalPackage; } var name = package.name; - if (result.containsKey(name)) { - throw PackageConfigArgumentError( - name, "packages", "Duplicate package name"); + if (packageNames.contains(name)) { + onError(PackageConfigArgumentError( + name, "packages", "Duplicate package name")); + continue; } - result[name] = package; - try { - tree.add(0, package); - } on ConflictException catch (e) { - // There is a conflict with an existing package. - var existingPackage = e.existingPackage; - if (e.isRootConflict) { - throw PackageConfigArgumentError( - originalPackages, - "packages", - "Packages ${package.name} and ${existingPackage.name}" - "have the same root directory: ${package.root}.\n"); + packageNames.add(name); + tree.add(0, package, (error) { + if (error is ConflictException) { + // There is a conflict with an existing package. + var existingPackage = error.existingPackage; + if (error.isRootConflict) { + onError(PackageConfigArgumentError( + originalPackages, + "packages", + "Packages ${package.name} and ${existingPackage.name}" + "have the same root directory: ${package.root}.\n")); + } else { + assert(error.isPackageRootConflict); + // Package is inside the package URI root of the existing package. + onError(PackageConfigArgumentError( + originalPackages, + "packages", + "Package ${package.name} is inside the package URI root of " + "package ${existingPackage.name}.\n" + "${existingPackage.name} URI root: " + "${existingPackage.packageUriRoot}\n" + "${package.name} root: ${package.root}\n")); + } + } else { + // Any other error. + onError(error); } - assert(e.isPackageRootConflict); - // Or package is inside the package URI root of the existing package. - throw PackageConfigArgumentError( - originalPackages, - "packages", - "Package ${package.name} is inside the package URI root of " - "package ${existingPackage.name}.\n" - "${existingPackage.name} URI root: " - "${existingPackage.packageUriRoot}\n" - "${package.name} root: ${package.root}\n"); - } + }); } return tree; } @@ -145,7 +159,7 @@ final String name; final Uri root; final Uri packageUriRoot; - final String /*?*/ languageVersion; + final LanguageVersion /*?*/ languageVersion; final dynamic extraData; SimplePackage._(this.name, this.root, this.packageUriRoot, @@ -154,45 +168,175 @@ /// Creates a [SimplePackage] with the provided content. /// /// The provided arguments must be valid. - factory SimplePackage(String name, Uri root, Uri packageUriRoot, - String /*?*/ languageVersion, dynamic extraData) { - _validatePackageData(name, root, packageUriRoot, languageVersion); + /// + /// If the arguments are invalid then the error is reported by + /// calling [onError], then the erroneous entry is ignored. + /// + /// If [onError] is provided, the user is expected to be able to handle + /// errors themselves. An invalid [languageVersion] string + /// will be replaced with the string `"invalid"`. This allows + /// users to detect the difference between an absent version and + /// an invalid one. + /// + /// Returns `null` if the input is invalid and an approximately valid package + /// cannot be salvaged from the input. + static SimplePackage /*?*/ validate( + String name, + Uri root, + Uri packageUriRoot, + LanguageVersion /*?*/ languageVersion, + dynamic extraData, + void onError(Object error)) { + bool fatalError = false; + var invalidIndex = checkPackageName(name); + if (invalidIndex >= 0) { + onError(PackageConfigFormatException( + "Not a valid package name", name, invalidIndex)); + fatalError = true; + } + if (root.isScheme("package")) { + onError(PackageConfigArgumentError( + "$root", "root", "Must not be a package URI")); + fatalError = true; + } else if (!isAbsoluteDirectoryUri(root)) { + onError(PackageConfigArgumentError( + "$root", + "root", + "In package $name: Not an absolute URI with no query or fragment " + "with a path ending in /")); + // Try to recover. If the URI has a scheme, + // then ensure that the path ends with `/`. + if (!root.hasScheme) { + fatalError = true; + } else if (!root.path.endsWith("/")) { + root = root.replace(path: root.path + "/"); + } + } + if (!fatalError) { + if (!isAbsoluteDirectoryUri(packageUriRoot)) { + onError(PackageConfigArgumentError( + packageUriRoot, + "packageUriRoot", + "In package $name: Not an absolute URI with no query or fragment " + "with a path ending in /")); + packageUriRoot = root; + } else if (!isUriPrefix(root, packageUriRoot)) { + onError(PackageConfigArgumentError(packageUriRoot, "packageUriRoot", + "The package URI root is not below the package root")); + packageUriRoot = root; + } + } + if (fatalError) return null; return SimplePackage._( name, root, packageUriRoot, languageVersion, extraData); } } -void _validatePackageData( - String name, Uri root, Uri packageUriRoot, String /*?*/ languageVersion) { - if (!isValidPackageName(name)) { - throw PackageConfigArgumentError(name, "name", "Not a valid package name"); +/// Checks whether [version] is a valid Dart language version string. +/// +/// The format is (as RegExp) `^(0|[1-9]\d+)\.(0|[1-9]\d+)$`. +/// +/// Reports a format exception on [onError] if not, or if the numbers +/// are too large (at most 32-bit signed integers). +LanguageVersion parseLanguageVersion( + String source, void onError(Object error)) { + var index = 0; + // Reads a positive decimal numeral. Returns the value of the numeral, + // or a negative number in case of an error. + // Starts at [index] and increments the index to the position after + // the numeral. + // It is an error if the numeral value is greater than 0x7FFFFFFFF. + // It is a recoverable error if the numeral starts with leading zeros. + int readNumeral() { + const maxValue = 0x7FFFFFFF; + if (index == source.length) { + onError(PackageConfigFormatException("Missing number", source, index)); + return -1; + } + var start = index; + + var char = source.codeUnitAt(index); + var digit = char ^ 0x30; + if (digit > 9) { + onError(PackageConfigFormatException("Missing number", source, index)); + return -1; + } + var firstDigit = digit; + var value = 0; + do { + value = value * 10 + digit; + if (value > maxValue) { + onError( + PackageConfigFormatException("Number too large", source, start)); + return -1; + } + index++; + if (index == source.length) break; + char = source.codeUnitAt(index); + digit = char ^ 0x30; + } while (digit <= 9); + if (firstDigit == 0 && index > start + 1) { + onError(PackageConfigFormatException( + "Leading zero not allowed", source, start)); + } + return value; } - if (!isAbsoluteDirectoryUri(root)) { - throw PackageConfigArgumentError( - "$root", - "root", - "Not an absolute URI with no query or fragment " - "with a path ending in /"); + + var major = readNumeral(); + if (major < 0) { + return SimpleInvalidLanguageVersion(source); } - if (!isAbsoluteDirectoryUri(packageUriRoot)) { - throw PackageConfigArgumentError( - packageUriRoot, - "packageUriRoot", - "Not an absolute URI with no query or fragment " - "with a path ending in /"); + if (index == source.length || source.codeUnitAt(index) != $dot) { + onError(PackageConfigFormatException("Missing '.'", source, index)); + return SimpleInvalidLanguageVersion(source); } - if (!isUriPrefix(root, packageUriRoot)) { - throw PackageConfigArgumentError(packageUriRoot, "packageUriRoot", - "The package URI root is not below the package root"); + index++; + var minor = readNumeral(); + if (minor < 0) { + return SimpleInvalidLanguageVersion(source); } - if (languageVersion != null && - checkValidVersionNumber(languageVersion) >= 0) { - throw PackageConfigArgumentError( - languageVersion, "languageVersion", "Invalid language version format"); + if (index != source.length) { + onError(PackageConfigFormatException( + "Unexpected trailing character", source, index)); + return SimpleInvalidLanguageVersion(source); + } + return SimpleLanguageVersion(major, minor, source); +} + +abstract class _SimpleLanguageVersionBase implements LanguageVersion { + int compareTo(LanguageVersion other) { + int result = major.compareTo(other.major); + if (result != 0) return result; + return minor.compareTo(other.minor); } } +class SimpleLanguageVersion extends _SimpleLanguageVersionBase { + final int major; + final int minor; + String /*?*/ _source; + SimpleLanguageVersion(this.major, this.minor, this._source); + + bool operator ==(Object other) => + other is LanguageVersion && major == other.major && minor == other.minor; + + int get hashCode => (major * 17 ^ minor * 37) & 0x3FFFFFFF; + + String toString() => _source ??= "$major.$minor"; +} + +class SimpleInvalidLanguageVersion extends _SimpleLanguageVersionBase + implements InvalidLanguageVersion { + final String _source; + SimpleInvalidLanguageVersion(this._source); + int get major => -1; + int get minor => -1; + + String toString() => _source; +} + abstract class PackageTree { + Iterable<Package> get allPackages; SimplePackage /*?*/ packageOf(Uri file); } @@ -210,13 +354,24 @@ final List<SimplePackage> packages = []; Map<String, MutablePackageTree /*?*/ > /*?*/ _packageChildren; + Iterable<Package> get allPackages sync* { + for (var package in packages) yield package; + if (_packageChildren != null) { + for (var tree in _packageChildren.values) yield* tree.allPackages; + } + } + /// Tries to (add) `package` to the tree. /// - /// Throws [ConflictException] if the added package conflicts with an + /// Reports a [ConflictException] if the added package conflicts with an /// existing package. /// It conflicts if it has the same root path, or if the new package /// contains the existing package's package root. - void add(int start, SimplePackage package) { + /// + /// If a conflict is detected between [package] and a previous package, + /// then [onError] is called with a [ConflictException] object + /// and the [package] is not added to the tree. + void add(int start, SimplePackage package, void onError(Object error)) { var path = package.root.toString(); for (var childPackage in packages) { var childPath = childPackage.root.toString(); @@ -225,13 +380,15 @@ if (_beginsWith(start, childPath, path)) { var childPathLength = childPath.length; if (path.length == childPathLength) { - throw ConflictException.root(package, childPackage); + onError(ConflictException.root(package, childPackage)); + return; } var childPackageRoot = childPackage.packageUriRoot.toString(); if (_beginsWith(childPathLength, childPackageRoot, path)) { - throw ConflictException.packageRoot(package, childPackage); + onError(ConflictException.packageRoot(package, childPackage)); + return; } - _treeOf(childPackage).add(childPathLength, package); + _treeOf(childPackage).add(childPathLength, package, onError); return; } } @@ -286,6 +443,8 @@ class EmptyPackageTree implements PackageTree { const EmptyPackageTree(); + Iterable<Package> get allPackages => const Iterable<Package>.empty(); + SimplePackage packageOf(Uri file) => null; }
diff --git a/pkgs/package_config/lib/src/package_config_json.dart b/pkgs/package_config/lib/src/package_config_json.dart index 8108102..f56c912 100644 --- a/pkgs/package_config/lib/src/package_config_json.dart +++ b/pkgs/package_config/lib/src/package_config_json.dart
@@ -6,9 +6,6 @@ import "dart:io"; import "dart:typed_data"; -import 'package:charcode/ascii.dart'; -import "package:path/path.dart" as p; - import "discovery.dart" show packageConfigJsonPath; import "errors.dart"; import "package_config_impl.dart"; @@ -45,41 +42,66 @@ /// a `.packages` file and there is an available `package_config.json` file. /// /// The file must exist and be a normal file. -Future<PackageConfig> readAnyConfigFile(File file, bool preferNewest) async { - var bytes = await file.readAsBytes(); +Future<PackageConfig> readAnyConfigFile( + File file, bool preferNewest, void onError(Object error)) async { + Uint8List bytes; + try { + bytes = await file.readAsBytes(); + } catch (e) { + onError(e); + return const SimplePackageConfig.empty(); + } int firstChar = firstNonWhitespaceChar(bytes); if (firstChar != $lbrace) { // Definitely not a JSON object, probably a .packages. if (preferNewest) { var alternateFile = File( - p.join(p.dirname(file.path), ".dart_tool", "package_config.json")); + pathJoin(dirName(file.path), ".dart_tool", "package_config.json")); if (alternateFile.existsSync()) { - return parsePackageConfigBytes( - await alternateFile.readAsBytes(), alternateFile.uri); + Uint8List /*?*/ bytes; + try { + bytes = await alternateFile.readAsBytes(); + } catch (e) { + onError(e); + return const SimplePackageConfig.empty(); + } + if (bytes != null) { + return parsePackageConfigBytes(bytes, alternateFile.uri, onError); + } } } - return packages_file.parse(bytes, file.uri); + return packages_file.parse(bytes, file.uri, onError); } - return parsePackageConfigBytes(bytes, file.uri); + return parsePackageConfigBytes(bytes, file.uri, onError); } /// Like [readAnyConfigFile] but uses a URI and an optional loader. -Future<PackageConfig> readAnyConfigFileUri(Uri file, - Future<Uint8List /*?*/ > loader(Uri uri) /*?*/, bool preferNewest) async { +Future<PackageConfig> readAnyConfigFileUri( + Uri file, + Future<Uint8List /*?*/ > loader(Uri uri) /*?*/, + void onError(Object error), + bool preferNewest) async { if (file.isScheme("package")) { throw PackageConfigArgumentError( file, "file", "Must not be a package: URI"); } if (loader == null) { if (file.isScheme("file")) { - return readAnyConfigFile(File.fromUri(file), preferNewest); + return readAnyConfigFile(File.fromUri(file), preferNewest, onError); } loader = defaultLoader; } - var bytes = await loader(file); + Uint8List bytes; + try { + bytes = await loader(file); + } catch (e) { + onError(e); + return const SimplePackageConfig.empty(); + } if (bytes == null) { - throw PackageConfigArgumentError( - file.toString(), "file", "File cannot be read"); + onError(PackageConfigArgumentError( + file.toString(), "file", "File cannot be read")); + return const SimplePackageConfig.empty(); } int firstChar = firstNonWhitespaceChar(bytes); if (firstChar != $lbrace) { @@ -87,39 +109,59 @@ if (preferNewest) { // Check if there is a package_config.json file. var alternateFile = file.resolveUri(packageConfigJsonPath); - var alternateBytes = await loader(alternateFile); + Uint8List alternateBytes; + try { + alternateBytes = await loader(alternateFile); + } catch (e) { + onError(e); + return const SimplePackageConfig.empty(); + } if (alternateBytes != null) { - return parsePackageConfigBytes(alternateBytes, alternateFile); + return parsePackageConfigBytes(alternateBytes, alternateFile, onError); } } - return packages_file.parse(bytes, file); + return packages_file.parse(bytes, file, onError); } - return parsePackageConfigBytes(bytes, file); + return parsePackageConfigBytes(bytes, file, onError); } -Future<PackageConfig> readPackageConfigJsonFile(File file) async { +Future<PackageConfig> readPackageConfigJsonFile( + File file, void onError(Object error)) async { Uint8List bytes; try { bytes = await file.readAsBytes(); - } catch (_) { - return null; + } catch (error) { + onError(error); + return const SimplePackageConfig.empty(); } - return parsePackageConfigBytes(bytes, file.uri); + return parsePackageConfigBytes(bytes, file.uri, onError); } -Future<PackageConfig> readDotPackagesFile(File file) async { +Future<PackageConfig> readDotPackagesFile( + File file, void onError(Object error)) async { Uint8List bytes; try { bytes = await file.readAsBytes(); - } catch (_) { - return null; + } catch (error) { + onError(error); + return const SimplePackageConfig.empty(); } - return packages_file.parse(bytes, file.uri); + return packages_file.parse(bytes, file.uri, onError); } -PackageConfig parsePackageConfigBytes(Uint8List bytes, Uri file) { +final _jsonUtf8Decoder = json.fuse(utf8).decoder; + +PackageConfig parsePackageConfigBytes( + Uint8List bytes, Uri file, void onError(Object error)) { // TODO(lrn): Make this simpler. Maybe parse directly from bytes. - return parsePackageConfigJson(json.fuse(utf8).decode(bytes), file); + var jsonObject; + try { + jsonObject = _jsonUtf8Decoder.convert(bytes); + } on FormatException catch (e) { + onError(PackageConfigFormatException(e.message, e.source, e.offset)); + return const SimplePackageConfig.empty(); + } + return parsePackageConfigJson(jsonObject, file, onError); } /// Creates a [PackageConfig] from a parsed JSON-like object structure. @@ -144,7 +186,8 @@ /// /// The [baseLocation] is used as base URI to resolve the "rootUri" /// URI referencestring. -PackageConfig parsePackageConfigJson(dynamic json, Uri baseLocation) { +PackageConfig parsePackageConfigJson( + dynamic json, Uri baseLocation, void onError(Object error)) { if (!baseLocation.hasScheme || baseLocation.isScheme("package")) { throw PackageConfigArgumentError(baseLocation.toString(), "baseLocation", "Must be an absolute non-package: URI"); @@ -168,27 +211,34 @@ var message = "$name${packageName != null ? " of package $packageName" : ""}" " is not a JSON ${typeName<T>()}"; - throw PackageConfigFormatException(message, value); + onError(PackageConfigFormatException(message, value)); + return null; } - Package parsePackage(Map<String, dynamic> entry) { + Package /*?*/ parsePackage(Map<String, dynamic> entry) { String /*?*/ name; String /*?*/ rootUri; String /*?*/ packageUri; String /*?*/ languageVersion; Map<String, dynamic> /*?*/ extraData; + bool hasName = false; + bool hasRoot = false; + bool hasVersion = false; entry.forEach((key, value) { switch (key) { case _nameKey: + hasName = true; name = checkType<String>(value, _nameKey); break; case _rootUriKey: + hasRoot = true; rootUri = checkType<String>(value, _rootUriKey, name); break; case _packageUriKey: packageUri = checkType<String>(value, _packageUriKey, name); break; case _languageVersionKey: + hasVersion = true; languageVersion = checkType<String>(value, _languageVersionKey, name); break; default: @@ -196,37 +246,61 @@ break; } }); - if (name == null) { - throw PackageConfigFormatException("Missing name entry", entry); + if (!hasName) { + onError(PackageConfigFormatException("Missing name entry", entry)); } - if (rootUri == null) { - throw PackageConfigFormatException("Missing rootUri entry", entry); + if (!hasRoot) { + onError(PackageConfigFormatException("Missing rootUri entry", entry)); } + if (name == null || rootUri == null) return null; Uri root = baseLocation.resolve(rootUri); - Uri /*?*/ packageRoot = root; + if (!root.path.endsWith("/")) root = root.replace(path: root.path + "/"); + Uri packageRoot = root; if (packageUri != null) packageRoot = root.resolve(packageUri); - try { - return SimplePackage(name, root, packageRoot, languageVersion, extraData); - } on ArgumentError catch (e) { - throw PackageConfigFormatException(e.message, e.invalidValue); + if (!packageRoot.path.endsWith("/")) { + packageRoot = packageRoot.replace(path: packageRoot.path + "/"); } + + LanguageVersion /*?*/ version; + if (languageVersion != null) { + version = parseLanguageVersion(languageVersion, onError); + } else if (hasVersion) { + version = SimpleInvalidLanguageVersion("invalid"); + } + + return SimplePackage.validate(name, root, packageRoot, version, extraData, + (error) { + if (error is ArgumentError) { + onError( + PackageConfigFormatException(error.message, error.invalidValue)); + } else { + onError(error); + } + }); } var map = checkType<Map<String, dynamic>>(json, "value"); + if (map == null) return const SimplePackageConfig.empty(); Map<String, dynamic> /*?*/ extraData = null; List<Package> /*?*/ packageList; int /*?*/ configVersion; map.forEach((key, value) { switch (key) { case _configVersionKey: - configVersion = checkType<int>(value, _configVersionKey); + configVersion = checkType<int>(value, _configVersionKey) ?? 2; break; case _packagesKey: - var packageArray = checkType<List<dynamic>>(value, _packagesKey); + var packageArray = checkType<List<dynamic>>(value, _packagesKey) ?? []; var packages = <Package>[]; for (var package in packageArray) { - packages.add(parsePackage( - checkType<Map<String, dynamic>>(package, "package entry"))); + var packageMap = + checkType<Map<String, dynamic>>(package, "package entry"); + if (packageMap != null) { + var entry = parsePackage(packageMap); + if (entry != null) { + packages.add(entry); + } + } } packageList = packages; break; @@ -236,22 +310,27 @@ } }); if (configVersion == null) { - throw PackageConfigFormatException("Missing configVersion entry", json); + onError(PackageConfigFormatException("Missing configVersion entry", json)); + configVersion = 2; } - if (packageList == null) - throw PackageConfigFormatException("Missing packages list", json); - try { - return SimplePackageConfig(configVersion, packageList, extraData); - } on ArgumentError catch (e) { - throw PackageConfigFormatException(e.message, e.invalidValue); + if (packageList == null) { + onError(PackageConfigFormatException("Missing packages list", json)); + packageList = []; } + return SimplePackageConfig(configVersion, packageList, extraData, (error) { + if (error is ArgumentError) { + onError(PackageConfigFormatException(error.message, error.invalidValue)); + } else { + onError(error); + } + }); } Future<void> writePackageConfigJson( PackageConfig config, Directory targetDirectory) async { // Write .dart_tool/package_config.json first. var file = - File(p.join(targetDirectory.path, ".dart_tool", "package_config.json")); + File(pathJoin(targetDirectory.path, ".dart_tool", "package_config.json")); var baseUri = file.uri; var extraData = config.extraData; var data = <String, dynamic>{ @@ -263,8 +342,9 @@ _rootUriKey: relativizeUri(package.root, baseUri), if (package.root != package.packageUriRoot) _packageUriKey: relativizeUri(package.packageUriRoot, package.root), - if (package.languageVersion != null) - _languageVersionKey: package.languageVersion, + if (package.languageVersion != null && + package.languageVersion is! InvalidLanguageVersion) + _languageVersionKey: package.languageVersion.toString(), ...?_extractExtraData(package.extraData, _packageNames), } ], @@ -283,7 +363,7 @@ "${generated != null ? " on $generated" : ""}."; } } - file = File(p.join(targetDirectory.path, ".packages")); + file = File(pathJoin(targetDirectory.path, ".packages")); baseUri = file.uri; var buffer = StringBuffer(); packages_file.write(buffer, config, baseUri: baseUri, comment: comment);
diff --git a/pkgs/package_config/lib/src/packages_file.dart b/pkgs/package_config/lib/src/packages_file.dart index ac57b4f..475a782 100644 --- a/pkgs/package_config/lib/src/packages_file.dart +++ b/pkgs/package_config/lib/src/packages_file.dart
@@ -3,9 +3,8 @@ // BSD-style license that can be found in the LICENSE file. import "package_config_impl.dart"; -import "package:charcode/ascii.dart"; -import "util.dart" show isValidPackageName, relativizeUri; +import "util.dart"; import "errors.dart"; /// Parses a `.packages` file into a [PackageConfig]. @@ -25,16 +24,18 @@ /// Returns a simple package configuration where each package's /// [Package.packageUriRoot] is the same as its [Package.root] /// and it has no [Package.languageVersion]. -PackageConfig parse(List<int> source, Uri baseLocation) { +PackageConfig parse( + List<int> source, Uri baseLocation, void onError(Object error)) { if (baseLocation.isScheme("package")) { - throw PackageConfigArgumentError( - baseLocation, "baseLocation", "Must not be a package: URI"); + onError(PackageConfigArgumentError( + baseLocation, "baseLocation", "Must not be a package: URI")); + return PackageConfig.empty; } int index = 0; List<Package> packages = []; Set<String> packageNames = {}; while (index < source.length) { - bool isComment = false; + bool ignoreLine = false; int start = index; int separatorIndex = -1; int end = source.length; @@ -43,10 +44,14 @@ continue; } if (char == $colon) { - throw PackageConfigFormatException( - "Missing package name", source, index - 1); + onError(PackageConfigFormatException( + "Missing package name", source, index - 1)); + ignoreLine = true; // Ignore if package name is invalid. + } else { + ignoreLine = char == $hash; // Ignore if comment. } - isComment = char == $hash; + int queryStart = -1; + int fragmentStart = -1; while (index < source.length) { char = source[index++]; if (char == $colon && separatorIndex < 0) { @@ -54,40 +59,70 @@ } else if (char == $cr || char == $lf) { end = index - 1; break; + } else if (char == $question && queryStart < 0 && fragmentStart < 0) { + queryStart = index - 1; + } else if (char == $hash && fragmentStart < 0) { + fragmentStart = index - 1; } } - if (isComment) continue; + if (ignoreLine) continue; if (separatorIndex < 0) { - throw PackageConfigFormatException("No ':' on line", source, index - 1); + onError( + PackageConfigFormatException("No ':' on line", source, index - 1)); + continue; } var packageName = String.fromCharCodes(source, start, separatorIndex); - if (!isValidPackageName(packageName)) { - throw PackageConfigFormatException( - "Not a valid package name", packageName, 0); + int invalidIndex = checkPackageName(packageName); + if (invalidIndex >= 0) { + onError(PackageConfigFormatException( + "Not a valid package name", source, start + invalidIndex)); + continue; + } + if (queryStart >= 0) { + onError(PackageConfigFormatException( + "Location URI must not have query", source, queryStart)); + end = queryStart; + } else if (fragmentStart >= 0) { + onError(PackageConfigFormatException( + "Location URI must not have fragment", source, fragmentStart)); + end = fragmentStart; } var packageValue = String.fromCharCodes(source, separatorIndex + 1, end); - Uri packageLocation = baseLocation.resolve(packageValue); - if (packageLocation.isScheme("package")) { - throw PackageConfigFormatException( - "Package URI as location for package", source, separatorIndex + 1); + Uri packageLocation; + try { + packageLocation = baseLocation.resolve(packageValue); + } on FormatException catch (e) { + onError(PackageConfigFormatException.from(e)); + continue; } - if (packageLocation.hasQuery || packageLocation.hasFragment) { - throw PackageConfigFormatException( - "Location URI must not have query or fragment", source, start); + if (packageLocation.isScheme("package")) { + onError(PackageConfigFormatException( + "Package URI as location for package", source, separatorIndex + 1)); + continue; } if (!packageLocation.path.endsWith('/')) { packageLocation = packageLocation.replace(path: packageLocation.path + "/"); } if (packageNames.contains(packageName)) { - throw PackageConfigFormatException( - "Same package name occured more than once", source, start); + onError(PackageConfigFormatException( + "Same package name occured more than once", source, start)); + continue; } - packages.add(SimplePackage( - packageName, packageLocation, packageLocation, null, null)); - packageNames.add(packageName); + var package = SimplePackage.validate( + packageName, packageLocation, packageLocation, null, null, (error) { + if (error is ArgumentError) { + onError(PackageConfigFormatException(error.message, source)); + } else { + onError(error); + } + }); + if (package != null) { + packages.add(package); + packageNames.add(packageName); + } } - return SimplePackageConfig(1, packages, null); + return SimplePackageConfig(1, packages, null, onError); } /// Writes the configuration to a [StringSink]. @@ -137,7 +172,7 @@ } output.write(packageName); output.write(':'); - // If baseUri provided, make uri relative. + // If baseUri is provided, make the URI relative to baseUri. if (baseUri != null) { uri = relativizeUri(uri, baseUri); }
diff --git a/pkgs/package_config/lib/src/util.dart b/pkgs/package_config/lib/src/util.dart index 25d7b89..2609a2f 100644 --- a/pkgs/package_config/lib/src/util.dart +++ b/pkgs/package_config/lib/src/util.dart
@@ -8,8 +8,6 @@ import 'dart:io'; import 'dart:typed_data'; -import "package:charcode/ascii.dart"; - import "errors.dart"; // All ASCII characters that are valid in a package name, with space @@ -46,6 +44,11 @@ } /// Validate that a [Uri] is a valid `package:` URI. +/// +/// Used to validate user input. +/// +/// Returns the package name extracted from the package URI, +/// which is the path segment between `package:` and the first `/`. String checkValidPackageUri(Uri packageUri, String name) { if (packageUri.scheme != "package") { throw PackageConfigArgumentError(packageUri, name, "Not a package: URI"); @@ -100,65 +103,16 @@ return packageName; } -/// Checks whether [version] is a valid Dart language version string. -/// -/// The format is (as RegExp) `^(0|[1-9]\d+)\.(0|[1-9]\d+)$`. -/// -/// Returns the position of the first invalid character, or -1 if -/// the string is valid. -/// If the string is terminated early, the result is the length of the string. -int checkValidVersionNumber(String version) { - if (version == null) { - return 0; - } - int index = 0; - int dotsSeen = 0; - outer: - for (;;) { - // Check for numeral. - if (index == version.length) return index; - int char = version.codeUnitAt(index++); - int digit = char ^ 0x30; - if (digit != 0) { - if (digit < 9) { - while (index < version.length) { - char = version.codeUnitAt(index++); - digit = char ^ 0x30; - if (digit < 9) continue; - if (char == 0x2e /*.*/) { - if (dotsSeen > 0) return index - 1; - dotsSeen = 1; - continue outer; - } - return index - 1; - } - if (dotsSeen > 0) return -1; - return index; - } - return index - 1; - } - // Leading zero means numeral is over. - if (index >= version.length) { - if (dotsSeen > 0) return -1; - return index; - } - if (dotsSeen > 0) return index; - char = version.codeUnitAt(index++); - if (char != 0x2e /*.*/) return index - 1; - } -} - /// Checks whether URI is just an absolute directory. /// /// * It must have a scheme. /// * It must not have a query or fragment. -/// * The path must start and end with `/`. +/// * The path must end with `/`. bool isAbsoluteDirectoryUri(Uri uri) { if (uri.hasQuery) return false; if (uri.hasFragment) return false; if (!uri.hasScheme) return false; var path = uri.path; - if (!path.startsWith("/")) return false; if (!path.endsWith("/")) return false; return true; } @@ -302,3 +256,63 @@ } return result; } + +/// The file name of a path. +/// +/// The file name is everything after the last occurrence of +/// [Platform.pathSeparator]. +String fileName(String path) { + var separator = Platform.pathSeparator; + int lastSeparator = path.lastIndexOf(separator); + if (lastSeparator < 0) return path; + return path.substring(lastSeparator + separator.length); +} + +/// The file name of a path. +/// +/// The file name is everything before the last occurrence of +/// [Platform.pathSeparator]. +String dirName(String path) { + var separator = Platform.pathSeparator; + int lastSeparator = path.lastIndexOf(separator); + if (lastSeparator < 0) return ""; + return path.substring(0, lastSeparator); +} + +/// Join path parts with the [Platform.pathSeparator]. +String pathJoin(String part1, String part2, [String part3]) { + var separator = Platform.pathSeparator; + if (part3 == null) { + return "$part1$separator$part2"; + } + return "$part1$separator$part2$separator$part3"; +} + +/// Join an unknown number of path parts with [Platform.pathSeparator]. +String pathJoinAll(Iterable<String> parts) => + parts.join(Platform.pathSeparator); + +// Character constants used by this package. +/// "Line feed" control character. +const int $lf = 0x0a; + +/// "Carriage return" control character. +const int $cr = 0x0d; + +/// Space character. +const int $space = 0x20; + +/// Character `#`. +const int $hash = 0x23; + +/// Character `.`. +const int $dot = 0x2e; + +/// Character `:`. +const int $colon = 0x3a; + +/// Character `?`. +const int $question = 0x3f; + +/// Character `{`. +const int $lbrace = 0x7b;
diff --git a/pkgs/package_config/pubspec.yaml b/pkgs/package_config/pubspec.yaml index 66aeb78..7f47d2e 100644 --- a/pkgs/package_config/pubspec.yaml +++ b/pkgs/package_config/pubspec.yaml
@@ -1,5 +1,5 @@ name: package_config -version: 2.0.0 +version: 3.0.0-dev description: Support for working with Package Configuration files. author: Dart Team <misc@dartlang.org> homepage: https://github.com/dart-lang/package_config @@ -7,9 +7,5 @@ environment: sdk: '>=2.7.0 <3.0.0' -dependencies: - charcode: ^1.1.0 - path: ^1.0.0 - dev_dependencies: test: ^1.3.0
diff --git a/pkgs/package_config/test/discovery_test.dart b/pkgs/package_config/test/discovery_test.dart index 3256187..23efc67 100644 --- a/pkgs/package_config/test/discovery_test.dart +++ b/pkgs/package_config/test/discovery_test.dart
@@ -114,36 +114,92 @@ expect(config, null); }); - fileTest("invalid .packages", { - ".packages": "not a .packages file", - }, (Directory directory) { - expect(() => findPackageConfig(directory), - throwsA(TypeMatcher<FormatException>())); + group("throws", () { + fileTest("invalid .packages", { + ".packages": "not a .packages file", + }, (Directory directory) { + expect(findPackageConfig(directory), + throwsA(TypeMatcher<FormatException>())); + }); + + fileTest("invalid .packages as JSON", { + ".packages": packageConfigFile, + }, (Directory directory) { + expect(findPackageConfig(directory), + throwsA(TypeMatcher<FormatException>())); + }); + + fileTest("invalid .packages", { + ".dart_tool": { + "package_config.json": "not a JSON file", + } + }, (Directory directory) { + expect(findPackageConfig(directory), + throwsA(TypeMatcher<FormatException>())); + }); + + fileTest("invalid .packages as INI", { + ".dart_tool": { + "package_config.json": packagesFile, + } + }, (Directory directory) { + expect(findPackageConfig(directory), + throwsA(TypeMatcher<FormatException>())); + }); }); - fileTest("invalid .packages as JSON", { - ".packages": packageConfigFile, - }, (Directory directory) { - expect(() => findPackageConfig(directory), - throwsA(TypeMatcher<FormatException>())); - }); + group("handles error", () { + fileTest("invalid .packages", { + ".packages": "not a .packages file", + }, (Directory directory) async { + bool hadError = false; + await findPackageConfig(directory, + onError: expectAsync1((error) { + hadError = true; + expect(error, isA<FormatException>()); + }, max: -1)); + expect(hadError, true); + }); - fileTest("invalid .packages", { - ".dart_tool": { - "package_config.json": "not a JSON file", - } - }, (Directory directory) { - expect(() => findPackageConfig(directory), - throwsA(TypeMatcher<FormatException>())); - }); + fileTest("invalid .packages as JSON", { + ".packages": packageConfigFile, + }, (Directory directory) async { + bool hadError = false; + await findPackageConfig(directory, + onError: expectAsync1((error) { + hadError = true; + expect(error, isA<FormatException>()); + }, max: -1)); + expect(hadError, true); + }); - fileTest("invalid .packages as INI", { - ".dart_tool": { - "package_config.json": packagesFile, - } - }, (Directory directory) { - expect(() => findPackageConfig(directory), - throwsA(TypeMatcher<FormatException>())); + fileTest("invalid package_config not JSON", { + ".dart_tool": { + "package_config.json": "not a JSON file", + } + }, (Directory directory) async { + bool hadError = false; + await findPackageConfig(directory, + onError: expectAsync1((error) { + hadError = true; + expect(error, isA<FormatException>()); + }, max: -1)); + expect(hadError, true); + }); + + fileTest("invalid package config as INI", { + ".dart_tool": { + "package_config.json": packagesFile, + } + }, (Directory directory) async { + bool hadError = false; + await findPackageConfig(directory, + onError: expectAsync1((error) { + hadError = true; + expect(error, isA<FormatException>()); + }, max: -1)); + expect(hadError, true); + }); }); }); @@ -226,6 +282,17 @@ throwsA(TypeMatcher<FileSystemException>())); }); + fileTest("no config found, handled", {}, (Directory directory) async { + File file = dirFile(directory, "anyname"); + bool hadError = false; + await loadPackageConfig(file, + onError: expectAsync1((error) { + hadError = true; + expect(error, isA<FileSystemException>()); + }, max: -1)); + expect(hadError, true); + }); + fileTest("specified file syntax error", { "anyname": "syntax error", }, (Directory directory) {
diff --git a/pkgs/package_config/test/discovery_uri_test.dart b/pkgs/package_config/test/discovery_uri_test.dart index 8ad428a..081ec60 100644 --- a/pkgs/package_config/test/discovery_uri_test.dart +++ b/pkgs/package_config/test/discovery_uri_test.dart
@@ -4,6 +4,8 @@ library package_config.discovery_test; +import 'dart:io'; + import "package:test/test.dart"; import "package:package_config/package_config.dart"; @@ -219,7 +221,20 @@ loaderTest("no config found", {}, (Uri directory, loader) { Uri file = directory.resolve("anyname"); expect(() => loadPackageConfigUri(file, loader: loader), - throwsArgumentError); + throwsA(isA<ArgumentError>())); + }); + + loaderTest("no config found, handle error", {}, + (Uri directory, loader) async { + Uri file = directory.resolve("anyname"); + bool hadError = false; + await loadPackageConfigUri(file, + loader: loader, + onError: expectAsync1((error) { + hadError = true; + expect(error, isA<ArgumentError>()); + }, max: -1)); + expect(hadError, true); }); loaderTest("specified file syntax error", { @@ -230,6 +245,20 @@ throwsFormatException); }); + loaderTest("specified file syntax error", { + "anyname": "syntax error", + }, (Uri directory, loader) async { + Uri file = directory.resolve("anyname"); + bool hadError = false; + await loadPackageConfigUri(file, + loader: loader, + onError: expectAsync1((error) { + hadError = true; + expect(error, isA<FormatException>()); + }, max: -1)); + expect(hadError, true); + }); + // Find package_config.json in subdir even if initial file syntax error. loaderTest("specified file syntax error", { "anyname": "syntax error", @@ -246,10 +275,16 @@ // A file starting with `{` is a package_config.json file. loaderTest("file syntax error with {", { ".packages": "{syntax error", - }, (Uri directory, loader) { + }, (Uri directory, loader) async { Uri file = directory.resolve(".packages"); - expect(() => loadPackageConfigUri(file, loader: loader), - throwsFormatException); + var hadError = false; + await loadPackageConfigUri(file, + loader: loader, + onError: expectAsync1((error) { + hadError = true; + expect(error, isA<FormatException>()); + }, max: -1)); + expect(hadError, true); }); }); }
diff --git a/pkgs/package_config/test/parse_test.dart b/pkgs/package_config/test/parse_test.dart index 3d1a20e..d943095 100644 --- a/pkgs/package_config/test/parse_test.dart +++ b/pkgs/package_config/test/parse_test.dart
@@ -6,10 +6,13 @@ import "package:test/test.dart"; +import "package:package_config/package_config.dart"; import "package:package_config/src/packages_file.dart" as packages; import "package:package_config/src/package_config_json.dart"; import "src/util.dart"; +void throwError(Object error) => throw error; + void main() { group(".packages", () { test("valid", () { @@ -17,8 +20,8 @@ "foo:file:///foo/lib/\n" "bar:/bar/lib/\n" "baz:lib/\n"; - var result = packages.parse( - utf8.encode(packagesFile), Uri.parse("file:///tmp/file.dart")); + var result = packages.parse(utf8.encode(packagesFile), + Uri.parse("file:///tmp/file.dart"), throwError); expect(result.version, 1); expect({for (var p in result.packages) p.name}, {"foo", "bar", "baz"}); expect(result.resolve(pkg("foo", "foo.dart")), @@ -37,8 +40,8 @@ test("valid empty", () { var packagesFile = "# Generated by pub yadda yadda\n"; - var result = - packages.parse(utf8.encode(packagesFile), Uri.file("/tmp/file.dart")); + var result = packages.parse( + utf8.encode(packagesFile), Uri.file("/tmp/file.dart"), throwError); expect(result.version, 1); expect({for (var p in result.packages) p.name}, <String>{}); }); @@ -47,9 +50,18 @@ var baseFile = Uri.file("/tmp/file.dart"); testThrows(String name, String content) { test(name, () { - expect(() => packages.parse(utf8.encode(content), baseFile), + expect( + () => packages.parse(utf8.encode(content), baseFile, throwError), throwsA(TypeMatcher<FormatException>())); }); + test(name + ", handle error", () { + bool hadError = false; + packages.parse(utf8.encode(content), baseFile, (error) { + hadError = true; + expect(error, isA<FormatException>()); + }); + expect(hadError, true); + }); } testThrows("repeated package name", "foo:lib/\nfoo:lib\n"); @@ -81,12 +93,17 @@ "name": "bar", "rootUri": "/bar/", "packageUri": "lib/", - "languageVersion": "100.100" + "languageVersion": "9999.9999" }, { "name": "baz", "rootUri": "../", "packageUri": "lib/" + }, + { + "name": "noslash", + "rootUri": "../noslash", + "packageUri": "lib" } ], "generator": "pub", @@ -94,9 +111,10 @@ } """; var config = parsePackageConfigBytes(utf8.encode(packageConfigFile), - Uri.parse("file:///tmp/.dart_tool/file.dart")); + Uri.parse("file:///tmp/.dart_tool/file.dart"), throwError); expect(config.version, 2); - expect({for (var p in config.packages) p.name}, {"foo", "bar", "baz"}); + expect({for (var p in config.packages) p.name}, + {"foo", "bar", "baz", "noslash"}); expect(config.resolve(pkg("foo", "foo.dart")), Uri.parse("file:///foo/lib/foo.dart")); @@ -109,14 +127,14 @@ expect(foo, isNotNull); expect(foo.root, Uri.parse("file:///foo/")); expect(foo.packageUriRoot, Uri.parse("file:///foo/lib/")); - expect(foo.languageVersion, "2.5"); + expect(foo.languageVersion, LanguageVersion(2, 5)); expect(foo.extraData, {"nonstandard": true}); var bar = config["bar"]; expect(bar, isNotNull); expect(bar.root, Uri.parse("file:///bar/")); expect(bar.packageUriRoot, Uri.parse("file:///bar/lib/")); - expect(bar.languageVersion, "100.100"); + expect(bar.languageVersion, LanguageVersion(9999, 9999)); expect(bar.extraData, null); var baz = config["baz"]; @@ -125,6 +143,13 @@ expect(baz.packageUriRoot, Uri.parse("file:///tmp/lib/")); expect(baz.languageVersion, null); + // No slash after root or package root. One is inserted. + var noslash = config["noslash"]; + expect(noslash, isNotNull); + expect(noslash.root, Uri.parse("file:///tmp/noslash/")); + expect(noslash.packageUriRoot, Uri.parse("file:///tmp/noslash/lib/")); + expect(noslash.languageVersion, null); + expect(config.extraData, { "generator": "pub", "other": [42] @@ -146,7 +171,7 @@ }, { "packageUri": "lib/", - "languageVersion": "100.100", + "languageVersion": "9999.9999", "rootUri": "/bar/", "name": "bar" }, @@ -160,7 +185,7 @@ } """; var config = parsePackageConfigBytes(utf8.encode(packageConfigFile), - Uri.parse("file:///tmp/.dart_tool/file.dart")); + Uri.parse("file:///tmp/.dart_tool/file.dart"), throwError); expect(config.version, 2); expect({for (var p in config.packages) p.name}, {"foo", "bar", "baz"}); @@ -184,7 +209,7 @@ var root = '"rootUri":"/foo/"'; test("minimal", () { var config = parsePackageConfigBytes(utf8.encode("{$cfg,$pkgs}"), - Uri.parse("file:///tmp/.dart_tool/file.dart")); + Uri.parse("file:///tmp/.dart_tool/file.dart"), throwError); expect(config.version, 2); expect(config.packages, isEmpty); }); @@ -193,7 +218,8 @@ // are optional. var config = parsePackageConfigBytes( utf8.encode('{$cfg,"packages":[{$name,$root}]}'), - Uri.parse("file:///tmp/.dart_tool/file.dart")); + Uri.parse("file:///tmp/.dart_tool/file.dart"), + throwError); expect(config.version, 2); expect(config.packages.first.name, "foo"); }); @@ -208,8 +234,8 @@ {"name": "qux", "rootUri": "/foo/qux/", "packageUri": "lib/"}, ] })); - var config = parsePackageConfigBytes( - configBytes, Uri.parse("file:///tmp/.dart_tool/file.dart")); + var config = parsePackageConfigBytes(configBytes, + Uri.parse("file:///tmp/.dart_tool/file.dart"), throwError); expect(config.version, 2); expect(config.packageOf(Uri.parse("file:///foo/lala/lala.dart")).name, "foo"); @@ -232,12 +258,9 @@ group("invalid", () { testThrows(String name, String source) { test(name, () { - if (name == "inside lib") { - print(name); - } expect( () => parsePackageConfigBytes(utf8.encode(source), - Uri.parse("file:///tmp/.dart_tool/file.dart")), + Uri.parse("file:///tmp/.dart_tool/file.dart"), throwError), throwsA(TypeMatcher<FormatException>())); }); }
diff --git a/pkgs/package_config/test/src/util.dart b/pkgs/package_config/test/src/util.dart index ec4e5e8..95670bd 100644 --- a/pkgs/package_config/test/src/util.dart +++ b/pkgs/package_config/test/src/util.dart
@@ -6,8 +6,8 @@ import "dart:io"; import 'dart:typed_data'; -import "package:path/path.dart" as path; import "package:test/test.dart"; +import "package:package_config/src/util.dart"; /// Creates a directory structure from [description] and runs [fileTest]. /// @@ -46,7 +46,7 @@ // Creates temporary files in the target directory. void _createFiles(Directory target, Map<Object, Object> description) { description.forEach((name, content) { - var entryName = path.join(target.path, "$name"); + var entryName = pathJoin(target.path, "$name"); if (content is Map<Object, Object>) { _createFiles(Directory(entryName)..createSync(), content); } else { @@ -57,11 +57,11 @@ /// Creates a [Directory] for a subdirectory of [parent]. Directory subdir(Directory parent, String dirName) => - Directory(path.joinAll([parent.path, ...dirName.split("/")])); + Directory(pathJoinAll([parent.path, ...dirName.split("/")])); /// Creates a [File] for an entry in the [directory] directory. File dirFile(Directory directory, String fileName) => - File(path.join(directory.path, fileName)); + File(pathJoin(directory.path, fileName)); /// Creates a package: URI. Uri pkg(String packageName, String packagePath) {