*ImportCache.canonicalize: Deprecate base importer without URL

See #2208

CHANGELOG.md

@@ -1,3 +1,23 @@
+## 1.75.0
+
+### JS API
+
+* Passing an `importer` argument without a corresponding canonical `url` to
+  `compileString()` and `compileStringAsync()` will now emit a deprecation named
+  `importer-without-url`. This was always forbidden by the TypeScript types, and
+  so was officially undefined behavior, but the previous behavior (passing
+  relative URLs as-is to the `importer` before passing them to the `importers`
+  list) will be preserved until Dart Sass 2.0.0. See
+  https://sass-lang.com/d/importer-without-url for more information.
+
+### Dart API
+
+* Passing an `importer` argument without a corresponding canonical `url` to
+  `compileStringToResult()`, `compileStringToResultAsync()`, `compileString()`,
+  and `compileStringAsync()` will now emit a deprecation named
+  `importer-without-url`. See https://sass-lang.com/d/importer-without-url for
+  more information.
+
 ## 1.74.1
 
 * No user-visible changes.

lib/src/async_compile.dart

@@ -118,6 +118,16 @@ Future<CompileResult> compileStringAsync(String source,
           futureDeprecations: {...?futureDeprecations},
           limitRepetition: !verbose);
 
+  // Allow NoOpImporter because various first-party callers use that to opt out
+  // of the also-deprecated FilesystemImporter.cwd behavior.
+  if (importer != null && importer is! NoOpImporter && url == null) {
+    logger.warnForDeprecation(
+        Deprecation.importerWithoutUrl,
+        "Passing an importer to compileString* without a canonical URL is "
+        "deprecated and will be an error in future versions of Sass. Use the "
+        "importers argument for non-relative loads.");
+  }
+
   var stylesheet =
       Stylesheet.parse(source, syntax ?? Syntax.scss, url: url, logger: logger);
 

lib/src/async_import_cache.dart

@@ -154,6 +154,8 @@ final class AsyncImportCache {
     }
 
     if (baseImporter != null && url.scheme == '') {
+      // TODO(sass/sass#3831): Throw an ArgumentError here if baseImporter is
+      // passed without a baseUrl as well.
       var relativeResult = await putIfAbsentAsync(
           _relativeCanonicalizeCache,
           (
@@ -182,17 +184,11 @@ final class AsyncImportCache {
 
   /// Calls [importer.canonicalize] and prints a deprecation warning if it
   /// returns a relative URL.
-  ///
-  /// If [resolveUrl] is `true`, this resolves [url] relative to [baseUrl]
-  /// before passing it to [importer].
   Future<AsyncCanonicalizeResult?> _canonicalize(
-      AsyncImporter importer, Uri url, Uri? baseUrl, bool forImport,
-      {bool resolveUrl = false}) async {
-    var resolved =
-        resolveUrl && baseUrl != null ? baseUrl.resolveUri(url) : url;
+      AsyncImporter importer, Uri url, Uri? baseUrl, bool forImport) async {
     var canonicalize = forImport
-        ? () => inImportRule(() => importer.canonicalize(resolved))
-        : () => importer.canonicalize(resolved);
+        ? () => inImportRule(() => importer.canonicalize(url))
+        : () => importer.canonicalize(url);
 
     var passContainingUrl = baseUrl != null &&
         (url.scheme == '' || await importer.isNonCanonicalScheme(url.scheme));
@@ -203,15 +199,15 @@ final class AsyncImportCache {
     if (result.scheme == '') {
       _logger.warnForDeprecation(
           Deprecation.relativeCanonical,
-          "Importer $importer canonicalized $resolved to $result.\n"
+          "Importer $importer canonicalized $url to $result.\n"
           "Relative canonical URLs are deprecated and will eventually be "
           "disallowed.");
     } else if (await importer.isNonCanonicalScheme(result.scheme)) {
-      throw "Importer $importer canonicalized $resolved to $result, which "
+      throw "Importer $importer canonicalized $url to $result, which "
           "uses a scheme declared as non-canonical.";
     }
 
-    return (importer, result, originalUrl: resolved);
+    return (importer, result, originalUrl: url);
   }
 
   /// Tries to import [url] using one of this cache's importers.

lib/src/compile.dart

@@ -5,7 +5,7 @@
 // DO NOT EDIT. This file was generated from async_compile.dart.
 // See tool/grind/synchronize.dart for details.
 //
-// Checksum: ab2c6fa2588988a86abdbe87512134098e01b39e
+// Checksum: 7e80ff5e991b0815e71b91b23a869222f12cf90b
 //
 // ignore_for_file: unused_import
 
@@ -127,6 +127,16 @@ CompileResult compileString(String source,
           futureDeprecations: {...?futureDeprecations},
           limitRepetition: !verbose);
 
+  // Allow NoOpImporter because various first-party callers use that to opt out
+  // of the also-deprecated FilesystemImporter.cwd behavior.
+  if (importer != null && importer is! NoOpImporter && url == null) {
+    logger.warnForDeprecation(
+        Deprecation.importerWithoutUrl,
+        "Passing an importer to compileString* without a canonical URL is "
+        "deprecated and will be an error in future versions of Sass. Use the "
+        "importers argument for non-relative loads.");
+  }
+
   var stylesheet =
       Stylesheet.parse(source, syntax ?? Syntax.scss, url: url, logger: logger);
 

lib/src/deprecation.dart

@@ -74,6 +74,11 @@ enum Deprecation {
       description:
           'Using the current working directory as an implicit load path.'),
 
+  importerWithoutUrl('importer-without-url',
+      deprecatedIn: '1.75.0',
+      description:
+          'Passing a base importer without a base URL to compileString*.'),
+
   @Deprecated('This deprecation name was never actually used.')
   calcInterp('calc-interp', deprecatedIn: null),
 

lib/src/embedded/logger.dart

@@ -8,7 +8,6 @@ import 'package:stack_trace/stack_trace.dart';
 
 import '../deprecation.dart';
 import '../logger.dart';
-import '../util/nullable.dart';
 import '../utils.dart';
 import 'compilation_dispatcher.dart';
 import 'embedded_sass.pb.dart' hide SourceSpan;
@@ -34,7 +33,9 @@ final class EmbeddedLogger extends LoggerWithDeprecationType {
       ..type = LogEventType.DEBUG
       ..message = message
       ..span = protofySpan(span)
-      ..formatted = (span.start.sourceUrl.andThen(p.prettyUri) ?? '-') +
+      ..formatted = (isRealUrl(span.start.sourceUrl)
+              ? p.prettyUri(span.start.sourceUrl)
+              : '-') +
           ':${span.start.line + 1} ' +
           (_color ? '\u001b[1mDebug\u001b[0m' : 'DEBUG') +
           ': $message\n');

lib/src/executable/compile_stylesheet.dart

@@ -68,13 +68,12 @@ Future<(int, String, String?)?> compileStylesheet(ExecutableOptions options,
 Future<void> _compileStylesheetWithoutErrorHandling(ExecutableOptions options,
     StylesheetGraph graph, String? source, String? destination,
     {bool ifModified = false}) async {
-  var importer = FilesystemImporter.cwd;
   if (ifModified) {
     try {
       if (source != null &&
           destination != null &&
-          !graph.modifiedSince(p.toUri(p.absolute(source)),
-              modificationTime(destination), importer)) {
+          !graph.modifiedSince(
+              p.toUri(p.absolute(source)), modificationTime(destination))) {
         return;
       }
     } on FileSystemException catch (_) {
@@ -105,6 +104,7 @@ Future<void> _compileStylesheetWithoutErrorHandling(ExecutableOptions options,
               logger: options.logger,
               importCache: importCache,
               importer: FilesystemImporter.cwd,
+              url: p.toUri(p.absolute('(stdin)${syntax.extension}')),
               style: options.style,
               quietDeps: options.quietDeps,
               verbose: options.verbose,
@@ -130,6 +130,7 @@ Future<void> _compileStylesheetWithoutErrorHandling(ExecutableOptions options,
               logger: options.logger,
               importCache: graph.importCache,
               importer: FilesystemImporter.cwd,
+              url: p.toUri(p.absolute('(stdin)${syntax.extension}')),
               style: options.style,
               quietDeps: options.quietDeps,
               verbose: options.verbose,

lib/src/executable/repl.dart

@@ -22,9 +22,8 @@ Future<void> repl(ExecutableOptions options) async {
   var repl = Repl(prompt: '>> ');
   var logger = TrackingLogger(options.logger);
   var evaluator = Evaluator(
-      importer: FilesystemImporter.cwd,
       importCache: ImportCache(
-          importers: options.pkgImporters,
+          importers: [...options.pkgImporters, FilesystemImporter('.')],
           loadPaths: options.loadPaths,
           logger: logger),
       logger: logger);

lib/src/import_cache.dart

@@ -5,7 +5,7 @@
 // DO NOT EDIT. This file was generated from async_import_cache.dart.
 // See tool/grind/synchronize.dart for details.
 //
-// Checksum: d157b83599dbc07a80ac6cb5ffdf5dde03b60376
+// Checksum: 01bc58f71fe5862d20d4c9ab7d8c7ba1eb612b7f
 //
 // ignore_for_file: unused_import
 
@@ -154,6 +154,8 @@ final class ImportCache {
     }
 
     if (baseImporter != null && url.scheme == '') {
+      // TODO(sass/sass#3831): Throw an ArgumentError here if baseImporter is
+      // passed without a baseUrl as well.
       var relativeResult = _relativeCanonicalizeCache.putIfAbsent(
           (
             url,
@@ -179,17 +181,11 @@ final class ImportCache {
 
   /// Calls [importer.canonicalize] and prints a deprecation warning if it
   /// returns a relative URL.
-  ///
-  /// If [resolveUrl] is `true`, this resolves [url] relative to [baseUrl]
-  /// before passing it to [importer].
   CanonicalizeResult? _canonicalize(
-      Importer importer, Uri url, Uri? baseUrl, bool forImport,
-      {bool resolveUrl = false}) {
-    var resolved =
-        resolveUrl && baseUrl != null ? baseUrl.resolveUri(url) : url;
+      Importer importer, Uri url, Uri? baseUrl, bool forImport) {
     var canonicalize = forImport
-        ? () => inImportRule(() => importer.canonicalize(resolved))
-        : () => importer.canonicalize(resolved);
+        ? () => inImportRule(() => importer.canonicalize(url))
+        : () => importer.canonicalize(url);
 
     var passContainingUrl = baseUrl != null &&
         (url.scheme == '' || importer.isNonCanonicalScheme(url.scheme));
@@ -200,15 +196,15 @@ final class ImportCache {
     if (result.scheme == '') {
       _logger.warnForDeprecation(
           Deprecation.relativeCanonical,
-          "Importer $importer canonicalized $resolved to $result.\n"
+          "Importer $importer canonicalized $url to $result.\n"
           "Relative canonical URLs are deprecated and will eventually be "
           "disallowed.");
     } else if (importer.isNonCanonicalScheme(result.scheme)) {
-      throw "Importer $importer canonicalized $resolved to $result, which "
+      throw "Importer $importer canonicalized $url to $result, which "
           "uses a scheme declared as non-canonical.";
     }
 
-    return (importer, result, originalUrl: resolved);
+    return (importer, result, originalUrl: url);
   }
 
   /// Tries to import [url] using one of this cache's importers.

lib/src/logger/stderr.dart

@@ -47,8 +47,9 @@ final class StderrLogger implements Logger {
 
   void debug(String message, SourceSpan span) {
     var result = StringBuffer();
-    var url =
-        span.start.sourceUrl == null ? '-' : p.prettyUri(span.start.sourceUrl);
+    var url = isRealUrl(span.start.sourceUrl)
+        ? p.prettyUri(span.start.sourceUrl)
+        : '-';
     result.write('$url:${span.start.line + 1} ');
     result.write(color ? '\u001b[1mDebug\u001b[0m' : 'DEBUG');
     result.write(': $message');

lib/src/stylesheet_graph.dart

@@ -41,12 +41,8 @@ class StylesheetGraph {
   /// Returns whether the stylesheet at [url] or any of the stylesheets it
   /// imports were modified since [since].
   ///
-  /// If [baseImporter] is non-`null`, this first tries to use [baseImporter] to
-  /// import [url] (resolved relative to [baseUrl] if it's passed).
-  ///
   /// Returns `true` if the import cache can't find a stylesheet at [url].
-  bool modifiedSince(Uri url, DateTime since,
-      [Importer? baseImporter, Uri? baseUrl]) {
+  bool modifiedSince(Uri url, DateTime since) {
     DateTime transitiveModificationTime(StylesheetNode node) {
       return _transitiveModificationTimes.putIfAbsent(node.canonicalUrl, () {
         var latest = node.importer.modificationTime(node.canonicalUrl);
@@ -63,22 +59,18 @@ class StylesheetGraph {
       });
     }
 
-    var node = _add(url, baseImporter, baseUrl);
+    var node = _add(url);
     if (node == null) return true;
     return transitiveModificationTime(node).isAfter(since);
   }
 
   /// Adds the stylesheet at [url] and all the stylesheets it imports to this
   /// graph and returns its node.
   ///
-  /// If [baseImporter] is non-`null`, this first tries to use [baseImporter] to
-  /// import [url] (resolved relative to [baseUrl] if it's passed).
-  ///
   /// Returns `null` if the import cache can't find a stylesheet at [url].
-  StylesheetNode? _add(Uri url, [Importer? baseImporter, Uri? baseUrl]) {
-    var result = _ignoreErrors(() => importCache.canonicalize(url,
-        baseImporter: baseImporter, baseUrl: baseUrl));
-    if (result case (var importer, var canonicalUrl, :var originalUrl)) {
+  StylesheetNode? _add(Uri url) {
+    if (_ignoreErrors(() => importCache.canonicalize(url))
+        case (var importer, var canonicalUrl, :var originalUrl)) {
       addCanonical(importer, canonicalUrl, originalUrl);
       return nodes[canonicalUrl];
     } else {

lib/src/syntax.dart

@@ -2,6 +2,7 @@
 // MIT-style license that can be found in the LICENSE file or at
 // https://opensource.org/licenses/MIT.
 
+import 'package:meta/meta.dart';
 import 'package:path/path.dart' as p;
 
 /// An enum of syntaxes that Sass can parse.
@@ -27,6 +28,16 @@ enum Syntax {
   /// The name of the syntax.
   final String _name;
 
+  /// The file extension used for this syntax, including the period.
+  ///
+  /// :nodoc:
+  @internal
+  String get extension => switch (this) {
+        Syntax.sass => '.sass',
+        Syntax.scss => '.scss',
+        Syntax.css => 'css'
+      };
+
   const Syntax(this._name);
 
   String toString() => _name;

lib/src/util/source_map_buffer.dart

@@ -74,7 +74,13 @@ class SourceMapBuffer implements StringBuffer {
       if (entry.target.offset == target.offset) return;
     }
 
-    _entries.add(Entry(source, target, null));
+    _entries.add(Entry(
+        isRealUrl(source.sourceUrl)
+            ? source
+            : SourceLocation(source.offset,
+                sourceUrl: null, line: source.line, column: source.column),
+        target,
+        null));
   }
 
   void clear() =>

lib/src/utils.dart

@@ -6,6 +6,7 @@ import 'dart:math' as math;
 
 import 'package:charcode/charcode.dart';
 import 'package:collection/collection.dart';
+import 'package:path/path.dart' as p;
 import 'package:source_span/source_span.dart';
 import 'package:stack_trace/stack_trace.dart';
 import 'package:string_scanner/string_scanner.dart';
@@ -207,11 +208,11 @@ int mapHash(Map<Object, Object> map) =>
 ///
 /// By default, the frame's URL is set to `span.sourceUrl`. However, if [url] is
 /// passed, it's used instead.
-Frame frameForSpan(SourceSpan span, String member, {Uri? url}) => Frame(
-    url ?? span.sourceUrl ?? _noSourceUrl,
-    span.start.line + 1,
-    span.start.column + 1,
-    member);
+Frame frameForSpan(SourceSpan span, String member, {Uri? url}) {
+  url ??= span.sourceUrl;
+  return Frame(isRealUrl(url) ? url! : _noSourceUrl, span.start.line + 1,
+      span.start.column + 1, member);
+}
 
 /// Returns the variable name (including the leading `$`) from a [span] that
 /// covers a variable declaration, which includes the variable name as well as
@@ -443,6 +444,15 @@ void attachTrace(Object error, StackTrace trace) {
 StackTrace? getTrace(Object error) =>
     error is String || error is num || error is bool ? null : _traces[error];
 
+/// Returns whether [url] is both non-null and not a fake URL to represent
+/// standard input for the Sass CLI.
+bool isRealUrl(Uri? url) =>
+    url != null &&
+    url != _noSourceUrl &&
+    !(url.scheme == 'file' &&
+        const {'(stdin).sass', '(stdin).scss', '(stdin).css'}
+            .contains(p.url.basename(url.path)));
+
 /// Parses a function signature of the format allowed by Node Sass's functions
 /// option and returns its name and declaration.
 ///