Properly use ImportResult.sourceMapUrl for source map URLs

This also uses data: URLs to refer to stylesheets from stdin in source
maps.

CHANGELOG.md

@@ -7,6 +7,16 @@
 * Match Ruby Sass's behavior in some edge-cases involving numbers with many
   significant digits.
 
+### Command-Line Interface
+
+* The source map generated for a stylesheet read from standard input now uses a
+  `data:` URL to include that stylesheet's contents in the source map.
+
+### Dart API
+
+* The URL used in a source map to refer to a stylesheet loaded from an importer
+  is now `ImportResult.sourceMapUrl` as documented.
+
 ## 1.15.2
 
 ### Node JS API

lib/src/async_compile.dart

@@ -3,6 +3,7 @@
 // https://opensource.org/licenses/MIT.
 
 import 'dart:async';
+import 'dart:convert';
 
 import 'package:path/path.dart' as p;
 import 'package:source_maps/source_maps.dart';
@@ -17,6 +18,7 @@ import 'io.dart';
 import 'logger.dart';
 import 'sync_package_resolver.dart';
 import 'syntax.dart';
+import 'utils.dart';
 import 'visitor/async_evaluate.dart';
 import 'visitor/serialize.dart';
 
@@ -130,6 +132,18 @@ Future<CompileResult> _compileStylesheet(
       lineFeed: lineFeed,
       sourceMap: sourceMap);
 
+  if (serializeResult.sourceMap != null && importCache != null) {
+    // TODO(nweiz): Don't explicitly use a type parameter when dart-lang/sdk#25490
+    // is fixed.
+    mapInPlace<String>(
+        serializeResult.sourceMap.urls,
+        (url) => url == ''
+            ? Uri.dataFromString(stylesheet.span.file.getText(0),
+                    encoding: utf8)
+                .toString()
+            : importCache.sourceMapUrl(Uri.parse(url)).toString());
+  }
+
   return CompileResult(evaluateResult, serializeResult);
 }
 

lib/src/async_import_cache.dart

@@ -10,6 +10,7 @@ import 'package:tuple/tuple.dart';
 
 import 'ast/sass.dart';
 import 'importer.dart';
+import 'importer/result.dart';
 import 'io.dart';
 import 'logger.dart';
 import 'sync_package_resolver.dart';
@@ -34,6 +35,9 @@ class AsyncImportCache {
   /// The parsed stylesheets for each canonicalized import URL.
   final Map<Uri, Stylesheet> _importCache;
 
+  /// The import results for each canonicalized import URL.
+  final Map<Uri, ImporterResult> _resultsCache;
+
   /// Creates an import cache that resolves imports using [importers].
   ///
   /// Imports are resolved by trying, in order:
@@ -58,14 +62,16 @@ class AsyncImportCache {
       : _importers = _toImporters(importers, loadPaths, packageResolver),
         _logger = logger ?? const Logger.stderr(),
         _canonicalizeCache = {},
-        _importCache = {};
+        _importCache = {},
+        _resultsCache = {};
 
   /// Creates an import cache without any globally-avaiable importers.
   AsyncImportCache.none({Logger logger})
       : _importers = const [],
         _logger = logger ?? const Logger.stderr(),
         _canonicalizeCache = {},
-        _importCache = {};
+        _importCache = {},
+        _resultsCache = {};
 
   /// Converts the user's [importers], [loadPaths], and [packageResolver]
   /// options into a single list of importers.
@@ -96,7 +102,8 @@ class AsyncImportCache {
       : _importers = const [],
         _logger = const Logger.stderr(),
         _canonicalizeCache = const {},
-        _importCache = const {};
+        _importCache = const {},
+        _resultsCache = const {};
 
   /// Canonicalizes [url] according to one of this cache's importers.
   ///
@@ -177,6 +184,8 @@ Relative canonical URLs are deprecated and will eventually be disallowed.
     return await putIfAbsentAsync(_importCache, canonicalUrl, () async {
       var result = await importer.load(canonicalUrl);
       if (result == null) return null;
+
+      _resultsCache[canonicalUrl] = result;
       return Stylesheet.parse(result.contents, result.syntax,
           // For backwards-compatibility, relative canonical URLs are resolved
           // relative to [originalUrl].
@@ -189,8 +198,7 @@ Relative canonical URLs are deprecated and will eventually be disallowed.
 
   /// Return a human-friendly URL for [canonicalUrl] to use in a stack trace.
   ///
-  /// Throws a [StateError] if the stylesheet for [canonicalUrl] hasn't been
-  /// loaded by this cache.
+  /// Returns [canonicalUrl] as-is if it hasn't been loaded by this cache.
   Uri humanize(Uri canonicalUrl) {
     // Display the URL with the shortest path length.
     var url = minBy(
@@ -206,6 +214,12 @@ Relative canonical URLs are deprecated and will eventually be disallowed.
     return url.resolve(p.url.basename(canonicalUrl.path));
   }
 
+  /// Returns the URL to use in the source map to refer to [canonicalUrl].
+  ///
+  /// Returns [canonicalUrl] as-is if it hasn't been loaded by this cache.
+  Uri sourceMapUrl(Uri canonicalUrl) =>
+      _resultsCache[canonicalUrl]?.sourceMapUrl ?? canonicalUrl;
+
   /// Clears the cached canonical version of the given [url].
   ///
   /// Has no effect if the canonical version of [url] has not been cached.
@@ -218,6 +232,7 @@ Relative canonical URLs are deprecated and will eventually be disallowed.
   ///
   /// Has no effect if the imported file at [canonicalUrl] has not been cached.
   void clearImport(Uri canonicalUrl) {
+    _resultsCache.remove(canonicalUrl);
     _importCache.remove(canonicalUrl);
   }
 }

lib/src/compile.dart

@@ -5,13 +5,15 @@
 // DO NOT EDIT. This file was generated from async_compile.dart.
 // See tool/synchronize.dart for details.
 //
-// Checksum: e6de63e581c0f4da96756b9e2904bd81a090dc5b
+// Checksum: 2bb00947655b3add16335253802a82188d730595
 //
 // ignore_for_file: unused_import
 
 import 'async_compile.dart';
 export 'async_compile.dart';
 
+import 'dart:convert';
+
 import 'package:path/path.dart' as p;
 import 'package:source_maps/source_maps.dart';
 import 'package:source_span/source_span.dart';
@@ -25,6 +27,7 @@ import 'io.dart';
 import 'logger.dart';
 import 'sync_package_resolver.dart';
 import 'syntax.dart';
+import 'utils.dart';
 import 'visitor/evaluate.dart';
 import 'visitor/serialize.dart';
 
@@ -138,5 +141,17 @@ CompileResult _compileStylesheet(
       lineFeed: lineFeed,
       sourceMap: sourceMap);
 
+  if (serializeResult.sourceMap != null && importCache != null) {
+    // TODO(nweiz): Don't explicitly use a type parameter when dart-lang/sdk#25490
+    // is fixed.
+    mapInPlace<String>(
+        serializeResult.sourceMap.urls,
+        (url) => url == ''
+            ? Uri.dataFromString(stylesheet.span.file.getText(0),
+                    encoding: utf8)
+                .toString()
+            : importCache.sourceMapUrl(Uri.parse(url)).toString());
+  }
+
   return CompileResult(evaluateResult, serializeResult);
 }

lib/src/executable/compile_stylesheet.dart

@@ -15,6 +15,7 @@ import '../importer/filesystem.dart';
 import '../io.dart';
 import '../stylesheet_graph.dart';
 import '../syntax.dart';
+import '../utils.dart';
 import 'options.dart';
 
 /// Compiles the stylesheet at [source] to [destination].
@@ -126,15 +127,10 @@ String _writeSourceMap(
     sourceMap.targetUrl = p.toUri(p.basename(destination)).toString();
   }
 
-  for (var i = 0; i < sourceMap.urls.length; i++) {
-    var url = sourceMap.urls[i];
-
-    // The special URL "" indicates a file that came from stdin.
-    if (url == "") continue;
-
-    sourceMap.urls[i] =
-        options.sourceMapUrl(Uri.parse(url), destination).toString();
-  }
+  // TODO(nweiz): Don't explicitly use a type parameter when dart-lang/sdk#25490
+  // is fixed.
+  mapInPlace<String>(sourceMap.urls,
+      (url) => options.sourceMapUrl(Uri.parse(url), destination).toString());
   var sourceMapText =
       jsonEncode(sourceMap.toJson(includeSourceContents: options.embedSources));
 

lib/src/executable/options.dart

@@ -402,7 +402,11 @@ class ExecutableOptions {
 
   /// Makes [url] absolute or relative (to the directory containing
   /// [destination]) according to the `source-map-urls` option.
+  ///
+  /// If [url] isn't a `file:` URL, returns it as-is.
   Uri sourceMapUrl(Uri url, String destination) {
+    if (url.scheme.isNotEmpty && url.scheme != 'file') return url;
+
     var path = p.canonicalize(p.fromUri(url));
     return p.toUri(_options['source-map-urls'] == 'relative'
         ? p.relative(path, from: p.dirname(destination))

lib/src/import_cache.dart

@@ -5,7 +5,7 @@
 // DO NOT EDIT. This file was generated from async_import_cache.dart.
 // See tool/synchronize.dart for details.
 //
-// Checksum: 4989811a7b432e181ccc42004e91f4fe54a786ca
+// Checksum: 291afac7e0d5edc6610685b28741786f667f83e8
 //
 // ignore_for_file: unused_import
 
@@ -15,6 +15,7 @@ import 'package:tuple/tuple.dart';
 
 import 'ast/sass.dart';
 import 'importer.dart';
+import 'importer/result.dart';
 import 'io.dart';
 import 'logger.dart';
 import 'sync_package_resolver.dart';
@@ -39,6 +40,9 @@ class ImportCache {
   /// The parsed stylesheets for each canonicalized import URL.
   final Map<Uri, Stylesheet> _importCache;
 
+  /// The import results for each canonicalized import URL.
+  final Map<Uri, ImporterResult> _resultsCache;
+
   /// Creates an import cache that resolves imports using [importers].
   ///
   /// Imports are resolved by trying, in order:
@@ -63,14 +67,16 @@ class ImportCache {
       : _importers = _toImporters(importers, loadPaths, packageResolver),
         _logger = logger ?? const Logger.stderr(),
         _canonicalizeCache = {},
-        _importCache = {};
+        _importCache = {},
+        _resultsCache = {};
 
   /// Creates an import cache without any globally-avaiable importers.
   ImportCache.none({Logger logger})
       : _importers = const [],
         _logger = logger ?? const Logger.stderr(),
         _canonicalizeCache = {},
-        _importCache = {};
+        _importCache = {},
+        _resultsCache = {};
 
   /// Converts the user's [importers], [loadPaths], and [packageResolver]
   /// options into a single list of importers.
@@ -101,7 +107,8 @@ class ImportCache {
       : _importers = const [],
         _logger = const Logger.stderr(),
         _canonicalizeCache = const {},
-        _importCache = const {};
+        _importCache = const {},
+        _resultsCache = const {};
 
   /// Canonicalizes [url] according to one of this cache's importers.
   ///
@@ -181,6 +188,8 @@ Relative canonical URLs are deprecated and will eventually be disallowed.
     return _importCache.putIfAbsent(canonicalUrl, () {
       var result = importer.load(canonicalUrl);
       if (result == null) return null;
+
+      _resultsCache[canonicalUrl] = result;
       return Stylesheet.parse(result.contents, result.syntax,
           // For backwards-compatibility, relative canonical URLs are resolved
           // relative to [originalUrl].
@@ -193,8 +202,7 @@ Relative canonical URLs are deprecated and will eventually be disallowed.
 
   /// Return a human-friendly URL for [canonicalUrl] to use in a stack trace.
   ///
-  /// Throws a [StateError] if the stylesheet for [canonicalUrl] hasn't been
-  /// loaded by this cache.
+  /// Returns [canonicalUrl] as-is if it hasn't been loaded by this cache.
   Uri humanize(Uri canonicalUrl) {
     // Display the URL with the shortest path length.
     var url = minBy(
@@ -210,6 +218,12 @@ Relative canonical URLs are deprecated and will eventually be disallowed.
     return url.resolve(p.url.basename(canonicalUrl.path));
   }
 
+  /// Returns the URL to use in the source map to refer to [canonicalUrl].
+  ///
+  /// Returns [canonicalUrl] as-is if it hasn't been loaded by this cache.
+  Uri sourceMapUrl(Uri canonicalUrl) =>
+      _resultsCache[canonicalUrl]?.sourceMapUrl ?? canonicalUrl;
+
   /// Clears the cached canonical version of the given [url].
   ///
   /// Has no effect if the canonical version of [url] has not been cached.
@@ -222,6 +236,7 @@ Relative canonical URLs are deprecated and will eventually be disallowed.
   ///
   /// Has no effect if the imported file at [canonicalUrl] has not been cached.
   void clearImport(Uri canonicalUrl) {
+    _resultsCache.remove(canonicalUrl);
     _importCache.remove(canonicalUrl);
   }
 }

lib/src/utils.dart

@@ -289,6 +289,13 @@ Map<String, V2> normalizedMapMap<K, V1, V2>(Map<K, V1> map,
   return result;
 }
 
+/// Destructively updates every element of [list] with the result of [function].
+void mapInPlace<T>(List<T> list, T function(T element)) {
+  for (var i = 0; i < list.length; i++) {
+    list[i] = function(list[i]);
+  }
+}
+
 /// Returns the longest common subsequence between [list1] and [list2].
 ///
 /// If there are more than one equally long common subsequence, returns the one

test/cli/shared/source_maps.dart

@@ -116,13 +116,16 @@ void sharedTests(Future<TestProcess> runSass(Iterable<String> arguments)) {
     });
   });
 
-  test("with --stdin uses an empty string", () async {
+  test("with --stdin uses a data: URL", () async {
     var sass = await runSass(["--stdin", "out.css"]);
     sass.stdin.writeln("a {b: c}");
     sass.stdin.close();
     await sass.shouldExit(0);
 
-    expect(_readJson("out.css.map"), containsPair("sources", [""]));
+    expect(
+        _readJson("out.css.map"),
+        containsPair("sources",
+            [Uri.dataFromString("a {b: c}\n", encoding: utf8).toString()]));
   });
 
   group("with --no-source-map,", () {

test/dart_api/importer_test.dart

@@ -4,6 +4,9 @@
 
 @TestOn('vm')
 
+import 'dart:convert';
+
+import 'package:source_maps/source_maps.dart';
 import 'package:test/test.dart';
 
 import 'package:sass/sass.dart';
@@ -86,6 +89,38 @@ main() {
     '''));
   });
 
+  test("uses an importer's source map URL", () {
+    SingleMapping map;
+    compileString('@import "orange";',
+        importers: [
+          _TestImporter((url) => Uri.parse("u:$url"), (url) {
+            var color = url.path;
+            return ImporterResult('.$color {color: $color}',
+                sourceMapUrl: Uri.parse("u:blue"), indented: false);
+          })
+        ],
+        sourceMap: (map_) => map = map_);
+
+    expect(map.urls, contains("u:blue"));
+  });
+
+  test("uses a data: source map URL if the importer doesn't provide one", () {
+    SingleMapping map;
+    compileString('@import "orange";',
+        importers: [
+          _TestImporter((url) => Uri.parse("u:$url"), (url) {
+            var color = url.path;
+            return ImporterResult('.$color {color: $color}', indented: false);
+          })
+        ],
+        sourceMap: (map_) => map = map_);
+
+    expect(
+        map.urls,
+        contains(Uri.dataFromString(".orange {color: orange}", encoding: utf8)
+            .toString()));
+  });
+
   test("wraps an error in canonicalize()", () {
     expect(() {
       compileString('@import "orange";', importers: [