Change resolve for package:URIs to not remove package name.

A "package URI" is defined as one with a `package` scheme,
no authority, a first path segment terminated by `/` which
contains no escapes and is not all `.` characters.
This is the definition of package names accepted by .packages
as well: Valid path characters and not all dots.

Change-Id: I9a161d47732e8bf873d278774315c72a4a928823
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/117542
Commit-Queue: Lasse R.H. Nielsen <lrn@google.com>
Reviewed-by: Nate Bosch <nbosch@google.com>

pkg/analysis_server/test/src/services/correction/assist/convert_to_package_import_test.dart

@@ -85,6 +85,7 @@ import 'package:test/foo/bar.dart';
 ''');
   }
 
+  @FailingTest(issue: 'http://dartbug.com/44871')
   Future<void> test_relativeImport_noAssistWithLint() async {
     createAnalysisOptionsFile(lints: [LintNames.avoid_relative_lib_imports]);
     verifyNoTestUnitErrors = false;

sdk/lib/core/uri.dart

@@ -2486,6 +2486,23 @@ class _Uri implements Uri {
     return resolveUri(Uri.parse(reference));
   }
 
+  // Returns the index of the `/` after the package name of a package URI.
+  //
+  // Returns negative if the URI is not a valid package URI:
+  // * Scheme must be "package".
+  // * No authority.
+  // * Path starts with "something"/
+  // * where "something" is not all "." characters,
+  // * and contains no escapes or colons.
+  //
+  // The characters are necessarily valid path characters.
+  static int _packageNameEnd(Uri uri, String path) {
+    if (uri.isScheme("package") && !uri.hasAuthority) {
+      return _skipPackageNameChars(path, 0, path.length);
+    }
+    return -1;
+  }
+
   Uri resolveUri(Uri reference) {
     // From RFC 3986.
     String targetScheme;
@@ -2526,7 +2543,22 @@ class _Uri implements Uri {
             targetQuery = this._query;
           }
         } else {
-          if (reference.hasAbsolutePath) {
+          String basePath = this.path;
+          int packageNameEnd = _packageNameEnd(this, basePath);
+          if (packageNameEnd > 0) {
+            assert(targetScheme == "package");
+            assert(!this.hasAuthority);
+            assert(!this.hasEmptyPath);
+            // Merging a path into a package URI.
+            String packageName = basePath.substring(0, packageNameEnd);
+            if (reference.hasAbsolutePath) {
+              targetPath = packageName + _removeDotSegments(reference.path);
+            } else {
+              targetPath = packageName +
+                  _removeDotSegments(_mergePaths(
+                      basePath.substring(packageName.length), reference.path));
+            }
+          } else if (reference.hasAbsolutePath) {
             targetPath = _removeDotSegments(reference.path);
           } else {
             // This is the RFC 3986 behavior for merging.
@@ -4278,6 +4310,25 @@ class _SimpleUri implements Uri {
     return _toNonSimple().resolveUri(reference);
   }
 
+  // Returns the index of the `/` after the package name of a package URI.
+  //
+  // Returns negative if the URI is not a valid package URI:
+  // * Scheme must be "package".
+  // * No authority.
+  // * Path starts with "something"/
+  // * where "something" is not all "." characters,
+  // * and contains no escapes or colons.
+  //
+  // The characters are necessarily valid path characters.
+  static int _packageNameEnd(_SimpleUri uri) {
+    if (uri._isPackage && !uri.hasAuthority) {
+      // Becomes Non zero if seeing any non-dot character.
+      // Also guards against empty package names.
+      return _skipPackageNameChars(uri._uri, uri._pathStart, uri._queryStart);
+    }
+    return -1;
+  }
+
   // Merge two simple URIs. This should always result in a prefix of
   // one concatenated with a suffix of the other, possibly with a `/` in
   // the middle of two merged paths, which is again simple.
@@ -4345,8 +4396,11 @@ class _SimpleUri implements Uri {
       return base.removeFragment();
     }
     if (ref.hasAbsolutePath) {
-      var delta = base._pathStart - ref._pathStart;
-      var newUri = base._uri.substring(0, base._pathStart) +
+      int basePathStart = base._pathStart;
+      int packageNameEnd = _packageNameEnd(this);
+      if (packageNameEnd > 0) basePathStart = packageNameEnd;
+      var delta = basePathStart - ref._pathStart;
+      var newUri = base._uri.substring(0, basePathStart) +
           ref._uri.substring(ref._pathStart);
       return _SimpleUri(
           newUri,
@@ -4393,7 +4447,12 @@ class _SimpleUri implements Uri {
     String refUri = ref._uri;
     int baseStart = base._pathStart;
     int baseEnd = base._queryStart;
-    while (baseUri.startsWith("../", baseStart)) baseStart += 3;
+    int packageNameEnd = _packageNameEnd(this);
+    if (packageNameEnd >= 0) {
+      baseStart = packageNameEnd; // At the `/` after the first package name.
+    } else {
+      while (baseUri.startsWith("../", baseStart)) baseStart += 3;
+    }
     int refStart = ref._pathStart;
     int refEnd = ref._queryStart;
 
@@ -4544,3 +4603,25 @@ int _stringOrNullLength(String? s) => (s == null) ? 0 : s.length;
 
 List<String> _toUnmodifiableStringList(String key, List<String> list) =>
     List<String>.unmodifiable(list);
+
+/// Counts valid package name characters in [source].
+///
+/// If [source] starts at [start] with a valid package name,
+/// followed by a `/`, no later than [end],
+/// then the position of the `/` is returned.
+/// If not, a negative value is returned.
+/// (Assumes source characters are valid path characters.)
+/// A name only consisting of `.` characters is not a valid
+/// package name.
+int _skipPackageNameChars(String source, int start, int end) {
+  // Becomes non-zero when seeing a non-dot character.
+  // Also guards against empty package names.
+  var dots = 0;
+  for (var i = start; i < end; i++) {
+    var char = source.codeUnitAt(i);
+    if (char == _SLASH) return (dots != 0) ? i : -1;
+    if (char == _PERCENT || char == _COLON) return -1;
+    dots |= char ^ _DOT;
+  }
+  return -1;
+}

tests/corelib/uri_test.dart

@@ -474,6 +474,105 @@ void testReplace() {
   Expect.listEquals(["43", "38"], params["y"]!);
 }
 
+void testPackageUris() {
+  // A URI is recognized as a package URI if it has:
+  // * "package" as scheme
+  // * no authority
+  // * a first path segment
+  //     * containing no `%`,
+  //     * which is not all "." characters,
+  //     * and which ends with a `/`.
+  //
+  // If so, the package name is unaffected by path resolution.
+  var uri = Uri.parse("package:foo/bar/baz"); // Simple base URI.
+
+  Expect.stringEquals("package:foo/qux", // Resolve simple URI.
+      uri.resolve("../../qux").toString());
+
+  Expect.stringEquals("package:foo/qux",
+      uri.resolve("/qux").toString());
+
+  Expect.stringEquals("package:foo/qux?%2F", // Resolve non-simple URI.
+      uri.resolve("../../qux?%2F").toString());
+
+  Expect.stringEquals("package:foo/qux?%2F",
+      uri.resolve("/qux?%2F").toString());
+
+  uri = Uri.parse("package:foo/%62ar/baz"); // Non-simple base URI.
+
+  Expect.stringEquals("package:foo/qux", // Resolve simple URI.
+      uri.resolve("../../qux").toString());
+
+  Expect.stringEquals("package:foo/qux",
+      uri.resolve("/qux").toString());
+
+  Expect.stringEquals("package:foo/qux?%2F", // Resolve non-simple URI.
+      uri.resolve("../../qux?%2F").toString());
+
+  Expect.stringEquals("package:foo/qux?%2F",
+      uri.resolve("/qux?%2F").toString());
+
+  // The following base URIs are not recognized as package URIs:
+  uri = Uri.parse("puckage:foo/bar/baz"); // Not "package" scheme.
+
+  Expect.stringEquals("puckage:/qux",
+      uri.resolve("../../qux").toString());
+
+  Expect.stringEquals("puckage:/qux",
+      uri.resolve("/qux").toString());
+
+  uri = Uri.parse("package://foo/bar/baz"); // Has authority.
+
+  Expect.stringEquals("package://foo/qux",
+      uri.resolve("../../qux").toString());
+
+  Expect.stringEquals("package://foo/qux",
+      uri.resolve("/qux").toString());
+
+  uri = Uri.parse("package:/foo/bar/baz"); // Has empty package name.
+
+  Expect.stringEquals("package:/qux",
+      uri.resolve("../../qux").toString());
+
+  Expect.stringEquals("package:/qux",
+      uri.resolve("/qux").toString());
+
+  uri = Uri.parse("package:f%2fo/bar/baz"); // Has escape in package name.
+
+  Expect.stringEquals("package:/qux",
+      uri.resolve("../../qux").toString());
+
+  Expect.stringEquals("package:/qux",
+      uri.resolve("/qux").toString());
+
+  uri = Uri.parse("package:f:o/bar/baz"); // Has colon in package name.
+
+  Expect.stringEquals("package:/qux",
+      uri.resolve("../../qux").toString());
+
+  Expect.stringEquals("package:/qux",
+      uri.resolve("/qux").toString());
+
+  uri = Uri.parse("package:.../bar/baz"); // Has only '.' in package name.
+
+  Expect.stringEquals("package:/qux",
+      uri.resolve("../../qux").toString());
+
+  Expect.stringEquals("package:/qux",
+      uri.resolve("/qux").toString());
+
+  uri = Uri.parse("package:foo?/"); // Has no `/` after package name.
+
+  // Resolving relative against non-absolute path gives
+  // a non-absolute path again.
+  // TODO(lrn): Is this a bug?
+  Expect.stringEquals("package:qux",
+      uri.resolve("../../qux").toString());
+
+  Expect.stringEquals("package:/qux",
+      uri.resolve("/qux").toString());
+}
+
 main() {
   testUri("http:", true);
   testUri("file:///", true);
@@ -625,6 +724,7 @@ main() {
   testInvalidUrls();
   testNormalization();
   testReplace();
+  testPackageUris();
 }
 
 String dump(Uri uri) {

tests/corelib_2/uri_test.dart

@@ -474,6 +474,105 @@ void testReplace() {
   Expect.listEquals(["43", "38"], params["y"]);
 }
 
+void testPackageUris() {
+  // A URI is recognized as a package URI if it has:
+  // * "package" as scheme
+  // * no authority
+  // * a first path segment
+  //     * containing no `%`,
+  //     * which is not all "." characters,
+  //     * and which ends with a `/`.
+  //
+  // If so, the package name is unaffected by path resolution.
+  var uri = Uri.parse("package:foo/bar/baz"); // Simple base URI.
+
+  Expect.stringEquals("package:foo/qux", // Resolve simple URI.
+      uri.resolve("../../qux").toString());
+
+  Expect.stringEquals("package:foo/qux",
+      uri.resolve("/qux").toString());
+
+  Expect.stringEquals("package:foo/qux?%2F", // Resolve non-simple URI.
+      uri.resolve("../../qux?%2F").toString());
+
+  Expect.stringEquals("package:foo/qux?%2F",
+      uri.resolve("/qux?%2F").toString());
+
+  uri = Uri.parse("package:foo/%62ar/baz"); // Non-simple base URI.
+
+  Expect.stringEquals("package:foo/qux", // Resolve simple URI.
+      uri.resolve("../../qux").toString());
+
+  Expect.stringEquals("package:foo/qux",
+      uri.resolve("/qux").toString());
+
+  Expect.stringEquals("package:foo/qux?%2F", // Resolve non-simple URI.
+      uri.resolve("../../qux?%2F").toString());
+
+  Expect.stringEquals("package:foo/qux?%2F",
+      uri.resolve("/qux?%2F").toString());
+
+  // The following base URIs are not recognized as package URIs:
+  uri = Uri.parse("puckage:foo/bar/baz"); // Not "package" scheme.
+
+  Expect.stringEquals("puckage:/qux",
+      uri.resolve("../../qux").toString());
+
+  Expect.stringEquals("puckage:/qux",
+      uri.resolve("/qux").toString());
+
+  uri = Uri.parse("package://foo/bar/baz"); // Has authority.
+
+  Expect.stringEquals("package://foo/qux",
+      uri.resolve("../../qux").toString());
+
+  Expect.stringEquals("package://foo/qux",
+      uri.resolve("/qux").toString());
+
+  uri = Uri.parse("package:/foo/bar/baz"); // Has empty package name.
+
+  Expect.stringEquals("package:/qux",
+      uri.resolve("../../qux").toString());
+
+  Expect.stringEquals("package:/qux",
+      uri.resolve("/qux").toString());
+
+  uri = Uri.parse("package:f%2fo/bar/baz"); // Has escape in package name.
+
+  Expect.stringEquals("package:/qux",
+      uri.resolve("../../qux").toString());
+
+  Expect.stringEquals("package:/qux",
+      uri.resolve("/qux").toString());
+
+  uri = Uri.parse("package:f:o/bar/baz"); // Has colon in package name.
+
+  Expect.stringEquals("package:/qux",
+      uri.resolve("../../qux").toString());
+
+  Expect.stringEquals("package:/qux",
+      uri.resolve("/qux").toString());
+
+  uri = Uri.parse("package:.../bar/baz"); // Has only '.' in package name.
+
+  Expect.stringEquals("package:/qux",
+      uri.resolve("../../qux").toString());
+
+  Expect.stringEquals("package:/qux",
+      uri.resolve("/qux").toString());
+
+  uri = Uri.parse("package:foo?/"); // Has no `/` after package name.
+
+  // Resolving relative against non-absolute path gives
+  // a non-absolute path again.
+  // TODO(lrn): Is this a bug?
+  Expect.stringEquals("package:qux",
+      uri.resolve("../../qux").toString());
+
+  Expect.stringEquals("package:/qux",
+      uri.resolve("/qux").toString());
+}
+
 main() {
   testUri("http:", true);
   testUri("file:///", true);
@@ -625,6 +724,7 @@ main() {
   testInvalidUrls();
   testNormalization();
   testReplace();
+  testPackageUris();
 }
 
 String dump(Uri uri) {