Update to new syntax. R=het@google.com, pquitslund@google.com Review URL: https://codereview.chromium.org//1178213004.
diff --git a/pkgs/package_config/lib/discovery_analysis.dart b/pkgs/package_config/lib/discovery_analysis.dart index f3bcac6..af4df07 100644 --- a/pkgs/package_config/lib/discovery_analysis.dart +++ b/pkgs/package_config/lib/discovery_analysis.dart
@@ -18,7 +18,6 @@ import "package:path/path.dart" as path; import "packages.dart"; -import "discovery.dart"; import "packages_file.dart" as pkgfile; import "src/packages_impl.dart"; import "src/packages_io_impl.dart";
diff --git a/pkgs/package_config/lib/packages_file.dart b/pkgs/package_config/lib/packages_file.dart index a736ca0..73e6061 100644 --- a/pkgs/package_config/lib/packages_file.dart +++ b/pkgs/package_config/lib/packages_file.dart
@@ -5,7 +5,7 @@ library package_config.packages_file; import "package:charcode/ascii.dart"; -import "src/util.dart" show isIdentifier; +import "src/util.dart" show isValidPackageName; /// Parses a `.packages` file into a map from package name to base URI. /// @@ -28,34 +28,34 @@ while (index < source.length) { bool isComment = false; int start = index; - int eqIndex = -1; + int separatorIndex = -1; int end = source.length; int char = source[index++]; if (char == $cr || char == $lf) { continue; } - if (char == $equal) { + if (char == $colon) { throw new FormatException("Missing package name", source, index - 1); } isComment = char == $hash; while (index < source.length) { char = source[index++]; - if (char == $equal && eqIndex < 0) { - eqIndex = index - 1; + if (char == $colon && separatorIndex < 0) { + separatorIndex = index - 1; } else if (char == $cr || char == $lf) { end = index - 1; break; } } if (isComment) continue; - if (eqIndex < 0) { - throw new FormatException("No '=' on line", source, index - 1); + if (separatorIndex < 0) { + throw new FormatException("No ':' on line", source, index - 1); } - var packageName = new String.fromCharCodes(source, start, eqIndex); - if (!isIdentifier(packageName)) { + var packageName = new String.fromCharCodes(source, start, separatorIndex); + if (!isValidPackageName(packageName)) { throw new FormatException("Not a valid package name", packageName, 0); } - var packageUri = new String.fromCharCodes(source, eqIndex + 1, end); + var packageUri = new String.fromCharCodes(source, separatorIndex + 1, end); var packageLocation = Uri.parse(packageUri); if (!packageLocation.path.endsWith('/')) { packageLocation = @@ -101,11 +101,11 @@ packageMapping.forEach((String packageName, Uri uri) { // Validate packageName. - if (!isIdentifier(packageName)) { + if (!isValidPackageName(packageName)) { throw new ArgumentError('"$packageName" is not a valid package name'); } output.write(packageName); - output.write('='); + output.write(':'); // If baseUri provided, make uri relative. if (baseUri != null) { uri = _relativize(uri, baseUri); @@ -124,7 +124,7 @@ /// but may be relative. /// The `baseUri` must be absolute. Uri _relativize(Uri uri, Uri baseUri) { - assert(!baseUri.isAbsolute); + assert(baseUri.isAbsolute); if (uri.hasQuery || uri.hasFragment) { uri = new Uri( scheme: uri.scheme, @@ -158,6 +158,7 @@ } uri = _normalizePath(uri); List<String> target = uri.pathSegments.toList(); + if (target.isNotEmpty && target.last.isEmpty) target.removeLast(); int index = 0; while (index < base.length && index < target.length) { if (base[index] != target[index]) { @@ -166,6 +167,9 @@ index++; } if (index == base.length) { + if (index == target.length) { + return new Uri(path: "./"); + } return new Uri(path: target.skip(index).join('/')); } else if (index > 0) { return new Uri(
diff --git a/pkgs/package_config/lib/src/util.dart b/pkgs/package_config/lib/src/util.dart index abf1d52..badf640 100644 --- a/pkgs/package_config/lib/src/util.dart +++ b/pkgs/package_config/lib/src/util.dart
@@ -7,26 +7,38 @@ import "package:charcode/ascii.dart"; -/// Tests whether something is a valid Dart identifier/package name. -bool isIdentifier(String string) { - if (string.isEmpty) return false; - int firstChar = string.codeUnitAt(0); - int firstCharLower = firstChar | 0x20; - if (firstCharLower < $a || firstCharLower > $z) { - if (firstChar != $_ && firstChar != $$) return false; - } - for (int i = 1; i < string.length; i++) { - int char = string.codeUnitAt(i); - int charLower = char | 0x20; - if (charLower < $a || charLower > $z) { // Letters. - if ((char ^ 0x30) <= 9) continue; // Digits. - if (char == $_ || char == $$) continue; // $ and _ - if (firstChar != $_ && firstChar != $$) return false; - } - } - return true; +// All ASCII characters that are valid in a package name, with space +// for all the invalid ones (including space). +const String _validPackageNameCharacters = + r" ! $ &'()*+,-. 0123456789 ; = " + r"@ABCDEFGHIJKLMNOPQRSTUVWXYZ _ abcdefghijklmnopqrstuvwxyz ~ "; + +/// Tests whether something is a valid Dart package name. +bool isValidPackageName(String string) { + return _findInvalidCharacter(string) < 0; } +/// Check if a string is a valid package name. +/// +/// Valid package names contain only characters in [_validPackageNameCharacters] +/// and must contain at least one non-'.' character. +/// +/// Returns `-1` if the string is valid. +/// Otherwise returns the index of the first invalid character, +/// or `string.length` if the string contains no non-'.' character. +int _findInvalidCharacter(String string) { + // Becomes non-zero if any non-'.' character is encountered. + int nonDot = 0; + for (int i = 0; i < string.length; i++) { + var c = string.codeUnitAt(i); + if (c > 0x7f || _validPackageNameCharacters.codeUnitAt(c) <= $space) { + return i; + } + nonDot += c ^ $dot; + } + if (nonDot == 0) return string.length; + return -1; +} /// Validate that a Uri is a valid package:URI. String checkValidPackageUri(Uri packageUri) { @@ -61,9 +73,25 @@ "Package URIs must start with the package name followed by a '/'"); } String packageName = packageUri.path.substring(0, firstSlash); - if (!isIdentifier(packageName)) { + int badIndex = _findInvalidCharacter(packageName); + if (badIndex >= 0) { + if (packageName.isEmpty) { + throw new ArgumentError.value(packageUri, "packageUri", + "Package names mus be non-empty"); + } + if (badIndex == packageName.length) { + throw new ArgumentError.value(packageUri, "packageUri", + "Package names must contain at least one non-'.' character"); + } + assert(badIndex < packageName.length); + int badCharCode = packageName.codeUnitAt(badIndex); + var badChar = "U+" + badCharCode.toRadixString(16).padLeft(4, '0'); + if (badCharCode >= 0x20 && badCharCode <= 0x7e) { + // Printable character. + badChar = "'${packageName[badIndex]}' ($badChar)"; + } throw new ArgumentError.value(packageUri, "packageUri", - "Package names must be valid identifiers"); + "Package names must not contain $badChar"); } return packageName; }
diff --git a/pkgs/package_config/pubspec.yaml b/pkgs/package_config/pubspec.yaml index 0b780d9..2d96c90 100644 --- a/pkgs/package_config/pubspec.yaml +++ b/pkgs/package_config/pubspec.yaml
@@ -1,5 +1,5 @@ name: package_config -version: 0.0.4 +version: 0.1.0 description: Support for working with Package Resolution config files. author: Dart Team <misc@dartlang.org> homepage: https://github.com/dart-lang/package_config
diff --git a/pkgs/package_config/test/all.dart b/pkgs/package_config/test/all.dart index 79f5b35..78e6cff 100644 --- a/pkgs/package_config/test/all.dart +++ b/pkgs/package_config/test/all.dart
@@ -2,10 +2,16 @@ // 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 "package:test/test.dart"; + +import "discovery_analysis_test.dart" as discovery_analysis; import "discovery_test.dart" as discovery; import "parse_test.dart" as parse; +import "parse_write_test.dart" as parse_write; main() { - parse.main(); - discovery.main(); + group("parse:", parse.main); + group("discovery:", discovery.main); + group("discovery-analysis:", discovery_analysis.main); + group("parse/write:", parse_write.main); }
diff --git a/pkgs/package_config/test/discovery_analysis_test.dart b/pkgs/package_config/test/discovery_analysis_test.dart index 38599f8..f33a4a8 100644 --- a/pkgs/package_config/test/discovery_analysis_test.dart +++ b/pkgs/package_config/test/discovery_analysis_test.dart
@@ -2,6 +2,8 @@ // 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. +library package_config.discovery_analysis_test; + import "dart:async"; import "dart:io"; @@ -46,9 +48,9 @@ const packagesFile = """ # A comment -foo=file:///dart/packages/foo/ -bar=http://example.com/dart/packages/bar/ -baz=packages/baz/ +foo:file:///dart/packages/foo/ +bar:http://example.com/dart/packages/bar/ +baz:packages/baz/ """; void validatePackagesFile(Packages resolver, Uri location) {
diff --git a/pkgs/package_config/test/discovery_test.dart b/pkgs/package_config/test/discovery_test.dart index d89cc2f..4f780c2 100644 --- a/pkgs/package_config/test/discovery_test.dart +++ b/pkgs/package_config/test/discovery_test.dart
@@ -2,6 +2,8 @@ // 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. +library package_config.discovery_test; + import "dart:async"; import "dart:io"; import "package:test/test.dart"; @@ -11,9 +13,9 @@ const packagesFile = """ # A comment -foo=file:///dart/packages/foo/ -bar=http://example.com/dart/packages/bar/ -baz=packages/baz/ +foo:file:///dart/packages/foo/ +bar:http://example.com/dart/packages/bar/ +baz:packages/baz/ """; void validatePackagesFile(Packages resolver, Uri location) {
diff --git a/pkgs/package_config/test/parse_test.dart b/pkgs/package_config/test/parse_test.dart index ddd8ff6..902d8ce 100644 --- a/pkgs/package_config/test/parse_test.dart +++ b/pkgs/package_config/test/parse_test.dart
@@ -2,7 +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. -library test_all; +library package_config.parse_test; import "package:package_config/packages.dart"; import "package:package_config/packages_file.dart" show parse; @@ -81,27 +81,27 @@ equals(base.resolve("../test/").resolve("bar/baz.dart"))); }); - test("all valid chars", () { + test("all valid chars can be used in URI segment", () { var packages = doParse(allValidCharsSample, base); expect(packages.packages.toList(), equals([allValidChars])); expect(packages.resolve(Uri.parse("package:$allValidChars/bar/baz.dart")), equals(base.resolve("../test/").resolve("bar/baz.dart"))); }); - test("no escapes", () { - expect(() => doParse("x%41x=x", base), throws); + test("no invalid chars accepted", () { + var map = {}; + for (int i = 0; i < allValidChars.length; i++) { + map[allValidChars.codeUnitAt(i)] = true; + } + for (int i = 0; i <= 255; i++) { + if (map[i] == true) continue; + var char = new String.fromCharCode(i); + expect(() => doParse("x${char}x:x"), throws); + } }); - test("not identifiers", () { - expect(() => doParse("1x=x", base), throws); - expect(() => doParse(" x=x", base), throws); - expect(() => doParse("\\x41x=x", base), throws); - expect(() => doParse("x@x=x", base), throws); - expect(() => doParse("x[x=x", base), throws); - expect(() => doParse("x`x=x", base), throws); - expect(() => doParse("x{x=x", base), throws); - expect(() => doParse("x/x=x", base), throws); - expect(() => doParse("x:x=x", base), throws); + test("no escapes", () { + expect(() => doParse("x%41x:x", base), throws); }); test("same name twice", () { @@ -131,26 +131,28 @@ var emptySample = ""; var commentOnlySample = "# comment only\n"; var emptyLinesSample = "\n\n\r\n"; -var singleRelativeSample = "foo=../test/\n"; -var singleRelativeSampleNoSlash = "foo=../test\n"; -var singleRelativeSampleNoNewline = "foo=../test/"; -var singleAbsoluteSample = "foo=http://example.com/some/where/\n"; -var multiRelativeSample = "foo=../test/\nbar=../test2/\n"; +var singleRelativeSample = "foo:../test/\n"; +var singleRelativeSampleNoSlash = "foo:../test\n"; +var singleRelativeSampleNoNewline = "foo:../test/"; +var singleAbsoluteSample = "foo:http://example.com/some/where/\n"; +var multiRelativeSample = "foo:../test/\nbar:../test2/\n"; // All valid path segment characters in an URI. var allValidChars = - r"$0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ_abcdefghijklmnopqrstuvwxyz"; -var allValidCharsSample = "${allValidChars.replaceAll('=', '%3D')}=../test/\n"; -var allUnreservedChars = - "-.0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ_abcdefghijklmnopqrstuvwxyz~"; + r"!$&'()*+,-.0123456789;=" + r"@ABCDEFGHIJKLMNOPQRSTUVWXYZ_abcdefghijklmnopqrstuvwxyz~"; + +var allValidCharsSample = "${allValidChars}:../test/\n"; // Invalid samples. var invalid = [ - "foobar:baz.dart", // no equals - ".=../test/", // dot segment - "..=../test/", // dot-dot segment - "foo/bar=../test/", // - "/foo=../test/", // var multiSegmentSample - "?=../test/", // invalid characters in path segment. - "[=../test/", // invalid characters in path segment. - "x#=../test/", // invalid characters in path segment. + ":baz.dart", // empty. + "foobar=baz.dart", // no colon (but an equals, which is not the same) + ".:../test/", // dot segment + "..:../test/", // dot-dot segment + "...:../test/", // dot-dot-dot segment + "foo/bar:../test/", // slash in name + "/foo:../test/", // slash at start of name + "?:../test/", // invalid characters. + "[:../test/", // invalid characters. + "x#:../test/", // invalid characters. ];
diff --git a/pkgs/package_config/test/parse_write_test.dart b/pkgs/package_config/test/parse_write_test.dart new file mode 100644 index 0000000..fde9616 --- /dev/null +++ b/pkgs/package_config/test/parse_write_test.dart
@@ -0,0 +1,84 @@ +// Copyright (c) 2015, 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. + +library package_config.parse_write_test; + +import "package:package_config/packages.dart"; +import "package:package_config/packages_file.dart"; +import "package:test/test.dart"; + +main() { + testBase(baseDirString) { + var baseDir = Uri.parse(baseDirString); + group("${baseDir.scheme} base", () { + Uri packagesFile = baseDir.resolve(".packages"); + + roundTripTest(String name, Map<String, Uri> map) { + group(name, () { + test("write with no baseUri", () { + var content = writeToString(map).codeUnits; + var resultMap = parse(content, packagesFile); + expect(resultMap, map); + }); + + test("write with base directory", () { + var content = writeToString(map, baseUri: baseDir).codeUnits; + var resultMap = parse(content, packagesFile); + expect(resultMap, map); + }); + + test("write with base .packages file", () { + var content = writeToString(map, baseUri: packagesFile).codeUnits; + var resultMap = parse(content, packagesFile); + expect(resultMap, map); + }); + }); + } + var lowerDir = baseDir.resolve("path3/path4/"); + var higherDir = baseDir.resolve("../"); + var parallelDir = baseDir.resolve("../path3/"); + var rootDir = baseDir.resolve("/"); + var fileDir = Uri.parse("file:///path1/part2/"); + var httpDir = Uri.parse("http://example.com/path1/path2/"); + var otherDir = Uri.parse("other:/path1/path2/"); + + roundTripTest("empty", {}); + roundTripTest("lower directory", {"foo": lowerDir}); + roundTripTest("higher directory", {"foo": higherDir}); + roundTripTest("parallel directory", {"foo": parallelDir}); + roundTripTest("same directory", {"foo": baseDir}); + roundTripTest("root directory", {"foo": rootDir}); + roundTripTest("file directory", {"foo": fileDir}); + roundTripTest("http directory", {"foo": httpDir}); + roundTripTest("other scheme directory", {"foo": otherDir}); + roundTripTest("multiple same-type directories", + {"foo": lowerDir, "bar": higherDir, "baz": parallelDir}); + roundTripTest("multiple scheme directories", + {"foo": fileDir, "bar": httpDir, "baz": otherDir}); + roundTripTest("multiple scheme directories and mutliple same type", + {"foo": fileDir, "bar": httpDir, "baz": otherDir, + "qux": lowerDir, "hip": higherDir, "dep": parallelDir}); + }); + } + + testBase("file:///base1/base2/"); + testBase("http://example.com/base1/base2/"); + testBase("other:/base1/base2/"); + + // Check that writing adds the comment. + test("write preserves comment", () { + var comment = "comment line 1\ncomment line 2\ncomment line 3"; + var result = writeToString({}, comment: comment); + // Comment with "# " before each line and "\n" after last. + var expectedComment = + "# comment line 1\n# comment line 2\n# comment line 3\n"; + expect(result, startsWith(expectedComment)); + }); +} + +String writeToString(Map map, {Uri baseUri, String comment}) { + var buffer = new StringBuffer(); + write(buffer, map, baseUri: baseUri, comment: comment); + return buffer.toString(); +}