Refactor URI processing of native extensions directive #import("dart-ext:foo").

runtime/bin/builtin.dart

 // Copyright (c) 2012, 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("builtin");
 
 void print(arg) {
   _Logger._printString(arg.toString());
 }
 
@@ skipping 49 matching lines @@
   var base = new Uri(scheme: "file", path: cwd.endsWith("/") ? cwd : "$cwd/");
   _entrypoint = base.resolve(scriptName);
   _logResolution("# Resolved script to: $_entrypoint");
 
   return _entrypoint.toString();
 }
 
 String _resolveUri(String base, String userString) {
   var baseUri = new Uri.fromString(base);
   _logResolution("# Resolving: $userString from $base");
-  var resolved = baseUri.resolve(userString);
+
+  // Relative URIs with scheme dart-ext should be resolved as if with no scheme.
+  var uri = new Uri.fromString(userString);
+  var resolved;
+  if ('dart-ext' == uri.scheme) {
+    resolved = baseUri.resolve(uri.path);
+    resolved = new Uri(scheme: "dart-ext", path: resolved.path);
+    // resolved.scheme = 'foo';  // Causes a segfault.  Verify this is expected.

[Ivan Posva, 2012/04/17 18:50:06]

Since the bug is filed now, you can remove this commented out code.

[Bill Hesse, 2012/04/18 09:56:39]

This line will be removed.  A separate bug and reproduction
(http://code.google.com/p/dart/issues/detail?id=2621) has been filed.

+  } else {
+    resolved = baseUri.resolve(userString);
+  }
   _logResolution("# Resolved to: $resolved");
   return resolved.toString();
 }
 
-String _resolveExtensionUri(String base, String userString) {
-  var uri = new Uri.fromString(userString);
-  if ("dart-ext" != uri.scheme) {
-    throw "Not a Dart extension uri: $uri";
-  }
-  var schemelessUri = new Uri(path: uri.path);
-  var baseUri = new Uri.fromString(base);
-  _logResolution("# Resolving: $userString from $base");
-  var resolved = baseUri.resolveUri(schemelessUri);
-  resolved = new Uri(scheme: 'dart-ext', path: resolved.path);
-  _logResolution("# Resolved to: $resolved");
-  return resolved.toString();
-}
 
 String _filePathFromUri(String userUri) {
   var uri = new Uri.fromString(userUri);
   _logResolution("# Getting file path from: $uri");
 
   var path;
   switch (uri.scheme) {
-  case 'file':    path = _filePathFromFileUri(uri); break;
+  case 'file':
+  case 'dart-ext': path = _filePathFromFileUri(uri); break;

[Ivan Posva, 2012/04/17 18:50:06]

Since dart-ext is not a file URI I am expecting this code to break once we
introduce an assert inside _filePathFromFileUri.

How about a remapping step before getting into this code converting the dart-ext
URI into the switch, such as this?

if (uri.scheme == "dart-ext") {
  uri = new Uri(scheme: "file", path: uri.path);
}

I can imagine that we could add other file-like URIs in the future.

[Bill Hesse, 2012/04/18 09:56:39]

Fixed by calling _filePathFromOtherUri.

I wrote code that did such a remapping, but replaced it because "Get a file path
from a URI, ignoring the scheme" is a clear, consistent functionality that is
needed, and implementing it by faking a file: scheme just so an added check can
pass seems like the wrong solution.

Adding the check would catch places where we accidentally call it with a
non-file: scheme, but we know that we are intentionally calling it with a
non-file: scheme in places.

As a temporary fix, I cloned the function _filePathFromFileUri, and called it
_filePathFromOtherUri.
I think a good refactoring in the future would be that the common code could
actually go into _filePathFromUri, and only package: would call an external
helper.

On 2012/04/17 18:50:06, Ivan Posva wrote:

   case 'package': path = _filePathFromPackageUri(uri); break;
 
   default:
-    // Only handling file and package URIs in standalone binary.
-    _logResolution("# Not a file or package URI.");
+    // Only handling file, dart-ext and package URIs in standalone binary.
+    _logResolution("# Not a file, dart-ext, or package URI.");
     throw "Not a known scheme: $uri";
   }
 
   if (_is_windows) {
     // Drop the leading / before the drive letter.
     path = path.substring(1);
     _logResolution("# path: $path");
   }
 
   return path;
@@ skipping 15 matching lines @@
     var wrong = 'package://$path';
 
     throw "URIs using the 'package:' scheme should look like " +
           "'$right', not '$wrong'.";
   }
 
   var path = _entrypoint.resolve('packages/${uri.path}').path;
   _logResolution("# Package: $path");
   return path;
 }