From c84b2d288ea143a327ea7bb991907b0aaec5cf81 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Mon, 8 Apr 2024 16:57:10 -0700 Subject: [PATCH] *ImportCache.canonicalize: Deprecate base importer without URL See #2208 --- CHANGELOG.md | 20 ++++++ lib/src/async_compile.dart | 10 +++ lib/src/async_import_cache.dart | 20 +++--- lib/src/compile.dart | 12 +++- lib/src/deprecation.dart | 5 ++ lib/src/embedded/logger.dart | 5 +- lib/src/executable/compile_stylesheet.dart | 7 ++- lib/src/executable/repl.dart | 3 +- lib/src/import_cache.dart | 22 +++---- lib/src/logger/stderr.dart | 5 +- lib/src/stylesheet_graph.dart | 18 ++---- lib/src/syntax.dart | 11 ++++ lib/src/util/source_map_buffer.dart | 8 ++- lib/src/utils.dart | 20 ++++-- lib/src/visitor/async_evaluate.dart | 41 ++++++------- lib/src/visitor/evaluate.dart | 49 ++++++--------- pubspec.yaml | 2 +- test/deprecations_test.dart | 71 +++++++++++++++++++--- test/embedded/file_importer_test.dart | 3 + test/embedded/importer_test.dart | 11 +++- test/embedded/utils.dart | 10 +++ 21 files changed, 236 insertions(+), 117 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1979177f2..4d9f92e11 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/lib/src/async_compile.dart b/lib/src/async_compile.dart index 063f3d2dc..d0c554602 100644 --- a/lib/src/async_compile.dart +++ b/lib/src/async_compile.dart @@ -118,6 +118,16 @@ Future 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); diff --git a/lib/src/async_import_cache.dart b/lib/src/async_import_cache.dart index 0deb6285f..c9118da48 100644 --- a/lib/src/async_import_cache.dart +++ b/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 _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. diff --git a/lib/src/compile.dart b/lib/src/compile.dart index 5a7fe54f2..defa5d70a 100644 --- a/lib/src/compile.dart +++ b/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); diff --git a/lib/src/deprecation.dart b/lib/src/deprecation.dart index a7412e2ce..6ecb924dc 100644 --- a/lib/src/deprecation.dart +++ b/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), diff --git a/lib/src/embedded/logger.dart b/lib/src/embedded/logger.dart index dd1f2a223..456ef30cb 100644 --- a/lib/src/embedded/logger.dart +++ b/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'); diff --git a/lib/src/executable/compile_stylesheet.dart b/lib/src/executable/compile_stylesheet.dart index cd121a6f5..995f6d821 100644 --- a/lib/src/executable/compile_stylesheet.dart +++ b/lib/src/executable/compile_stylesheet.dart @@ -68,13 +68,12 @@ Future<(int, String, String?)?> compileStylesheet(ExecutableOptions options, Future _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 _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 _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, diff --git a/lib/src/executable/repl.dart b/lib/src/executable/repl.dart index e2e858a26..72ce8e2e4 100644 --- a/lib/src/executable/repl.dart +++ b/lib/src/executable/repl.dart @@ -22,9 +22,8 @@ Future 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); diff --git a/lib/src/import_cache.dart b/lib/src/import_cache.dart index e34f0a7ee..ce4f9b02d 100644 --- a/lib/src/import_cache.dart +++ b/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. diff --git a/lib/src/logger/stderr.dart b/lib/src/logger/stderr.dart index fc001008f..2c29f2d7e 100644 --- a/lib/src/logger/stderr.dart +++ b/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'); diff --git a/lib/src/stylesheet_graph.dart b/lib/src/stylesheet_graph.dart index 3109fc5f0..4573a61af 100644 --- a/lib/src/stylesheet_graph.dart +++ b/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,7 +59,7 @@ class StylesheetGraph { }); } - var node = _add(url, baseImporter, baseUrl); + var node = _add(url); if (node == null) return true; return transitiveModificationTime(node).isAfter(since); } @@ -71,14 +67,10 @@ class StylesheetGraph { /// 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 { diff --git a/lib/src/syntax.dart b/lib/src/syntax.dart index ab9fc9557..0ee702ca6 100644 --- a/lib/src/syntax.dart +++ b/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; diff --git a/lib/src/util/source_map_buffer.dart b/lib/src/util/source_map_buffer.dart index e1026f3c4..c39e44f2c 100644 --- a/lib/src/util/source_map_buffer.dart +++ b/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() => diff --git a/lib/src/utils.dart b/lib/src/utils.dart index abd7834cc..ce8a7a65d 100644 --- a/lib/src/utils.dart +++ b/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 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. /// diff --git a/lib/src/visitor/async_evaluate.dart b/lib/src/visitor/async_evaluate.dart index ce4105bda..3ad36e6a4 100644 --- a/lib/src/visitor/async_evaluate.dart +++ b/lib/src/visitor/async_evaluate.dart @@ -85,6 +85,8 @@ Future evaluateAsync(Stylesheet stylesheet, Logger? logger, bool quietDeps = false, bool sourceMap = false}) => + // TODO(sass/sass#3831): Throw an ArgumentError here if importer is passed + // but stylesheet doesn't have a URL. _EvaluateVisitor( importCache: importCache, nodeImporter: nodeImporter, @@ -100,28 +102,23 @@ final class AsyncEvaluator { /// The visitor that evaluates each expression and statement. final _EvaluateVisitor _visitor; - /// The importer to use to resolve `@use` rules in [_visitor]. - final AsyncImporter? _importer; - /// Creates an evaluator. /// /// Arguments are the same as for [evaluateAsync]. AsyncEvaluator( {AsyncImportCache? importCache, - AsyncImporter? importer, Iterable? functions, Logger? logger}) : _visitor = _EvaluateVisitor( - importCache: importCache, functions: functions, logger: logger), - _importer = importer; + importCache: importCache, functions: functions, logger: logger); - Future use(UseRule use) => _visitor.runStatement(_importer, use); + Future use(UseRule use) => _visitor.runStatement(use); Future evaluate(Expression expression) => - _visitor.runExpression(_importer, expression); + _visitor.runExpression(expression); Future setVariable(VariableDeclaration declaration) => - _visitor.runStatement(_importer, declaration); + _visitor.runStatement(declaration); } /// A visitor that executes Sass code to produce a CSS tree. @@ -605,17 +602,15 @@ final class _EvaluateVisitor } } - Future runExpression(AsyncImporter? importer, Expression expression) => - withEvaluationContext( - _EvaluationContext(this, expression), - () => _withFakeStylesheet(importer, expression, - () => _addExceptionTrace(() => expression.accept(this)))); + Future runExpression(Expression expression) => withEvaluationContext( + _EvaluationContext(this, expression), + () => _withFakeStylesheet( + expression, () => _addExceptionTrace(() => expression.accept(this)))); - Future runStatement(AsyncImporter? importer, Statement statement) => - withEvaluationContext( - _EvaluationContext(this, statement), - () => _withFakeStylesheet(importer, statement, - () => _addExceptionTrace(() => statement.accept(this)))); + Future runStatement(Statement statement) => withEvaluationContext( + _EvaluationContext(this, statement), + () => _withFakeStylesheet( + statement, () => _addExceptionTrace(() => statement.accept(this)))); /// Asserts that [value] is not `null` and returns it. /// @@ -627,12 +622,12 @@ final class _EvaluateVisitor throw StateError("Can't access $name outside of a module."); } - /// Runs [callback] with [importer] as [_importer] and a fake [_stylesheet] - /// with [nodeWithSpan]'s source span. - Future _withFakeStylesheet(AsyncImporter? importer, + /// Runs [callback] with a fake [_stylesheet] with [nodeWithSpan]'s source + /// span. + Future _withFakeStylesheet( AstNode nodeWithSpan, FutureOr callback()) async { var oldImporter = _importer; - _importer = importer; + _importer = null; assert(__stylesheet == null); _stylesheet = Stylesheet(const [], nodeWithSpan.span); diff --git a/lib/src/visitor/evaluate.dart b/lib/src/visitor/evaluate.dart index 32c4e2764..178ab3ab2 100644 --- a/lib/src/visitor/evaluate.dart +++ b/lib/src/visitor/evaluate.dart @@ -5,7 +5,7 @@ // DO NOT EDIT. This file was generated from async_evaluate.dart. // See tool/grind/synchronize.dart for details. // -// Checksum: 05cb957cd0c7698d8ad648f31d862dc91f0daa7b +// Checksum: 5775f1fcfd2f4d0b22285daebdaa7441b567a593 // // ignore_for_file: unused_import @@ -93,6 +93,8 @@ EvaluateResult evaluate(Stylesheet stylesheet, Logger? logger, bool quietDeps = false, bool sourceMap = false}) => + // TODO(sass/sass#3831): Throw an ArgumentError here if importer is passed + // but stylesheet doesn't have a URL. _EvaluateVisitor( importCache: importCache, nodeImporter: nodeImporter, @@ -108,28 +110,20 @@ final class Evaluator { /// The visitor that evaluates each expression and statement. final _EvaluateVisitor _visitor; - /// The importer to use to resolve `@use` rules in [_visitor]. - final Importer? _importer; - /// Creates an evaluator. /// /// Arguments are the same as for [evaluate]. Evaluator( - {ImportCache? importCache, - Importer? importer, - Iterable? functions, - Logger? logger}) + {ImportCache? importCache, Iterable? functions, Logger? logger}) : _visitor = _EvaluateVisitor( - importCache: importCache, functions: functions, logger: logger), - _importer = importer; + importCache: importCache, functions: functions, logger: logger); - void use(UseRule use) => _visitor.runStatement(_importer, use); + void use(UseRule use) => _visitor.runStatement(use); - Value evaluate(Expression expression) => - _visitor.runExpression(_importer, expression); + Value evaluate(Expression expression) => _visitor.runExpression(expression); void setVariable(VariableDeclaration declaration) => - _visitor.runStatement(_importer, declaration); + _visitor.runStatement(declaration); } /// A visitor that executes Sass code to produce a CSS tree. @@ -606,17 +600,15 @@ final class _EvaluateVisitor } } - Value runExpression(Importer? importer, Expression expression) => - withEvaluationContext( - _EvaluationContext(this, expression), - () => _withFakeStylesheet(importer, expression, - () => _addExceptionTrace(() => expression.accept(this)))); + Value runExpression(Expression expression) => withEvaluationContext( + _EvaluationContext(this, expression), + () => _withFakeStylesheet( + expression, () => _addExceptionTrace(() => expression.accept(this)))); - void runStatement(Importer? importer, Statement statement) => - withEvaluationContext( - _EvaluationContext(this, statement), - () => _withFakeStylesheet(importer, statement, - () => _addExceptionTrace(() => statement.accept(this)))); + void runStatement(Statement statement) => withEvaluationContext( + _EvaluationContext(this, statement), + () => _withFakeStylesheet( + statement, () => _addExceptionTrace(() => statement.accept(this)))); /// Asserts that [value] is not `null` and returns it. /// @@ -628,12 +620,11 @@ final class _EvaluateVisitor throw StateError("Can't access $name outside of a module."); } - /// Runs [callback] with [importer] as [_importer] and a fake [_stylesheet] - /// with [nodeWithSpan]'s source span. - T _withFakeStylesheet( - Importer? importer, AstNode nodeWithSpan, T callback()) { + /// Runs [callback] with a fake [_stylesheet] with [nodeWithSpan]'s source + /// span. + T _withFakeStylesheet(AstNode nodeWithSpan, T callback()) { var oldImporter = _importer; - _importer = importer; + _importer = null; assert(__stylesheet == null); _stylesheet = Stylesheet(const [], nodeWithSpan.span); diff --git a/pubspec.yaml b/pubspec.yaml index e37160085..a30a7ea43 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass -version: 1.74.1 +version: 1.75.0-dev description: A Sass implementation in Dart. homepage: https://github.com/sass/dart-sass diff --git a/test/deprecations_test.dart b/test/deprecations_test.dart index 28cd1e2fd..0c4b62b0e 100644 --- a/test/deprecations_test.dart +++ b/test/deprecations_test.dart @@ -4,9 +4,12 @@ @TestOn('vm') +import 'package:source_span/source_span.dart'; +import 'package:stack_trace/stack_trace.dart'; import 'package:test/test.dart'; import 'package:sass/sass.dart'; +import 'package:sass/src/logger.dart'; void main() { // Deprecated in all version of Dart Sass @@ -16,7 +19,7 @@ void main() { // Deprecated in 1.3.2 test("elseIf is violated by using @elseif instead of @else if", () { - _expectDeprecation("@if false {} @elseif {}", Deprecation.elseif); + _expectDeprecation("@if false {} @elseif false {}", Deprecation.elseif); }); // Deprecated in 1.7.2 @@ -125,15 +128,65 @@ void main() { Deprecation.functionUnits); }); }); + + // Deprecated in 1.75.0 + group( + "importerWithoutUrl is violated by passing an importer without a base URL to", + () { + test("compileString", () { + var logger = _DeprecationTrackingLogger(); + compileString("", importer: FilesystemImporter('.'), logger: logger); + expect(logger.deprecations, equals({Deprecation.importerWithoutUrl})); + }); + + test("compileStringAsync", () async { + var logger = _DeprecationTrackingLogger(); + await compileStringAsync("", + importer: FilesystemImporter('.'), logger: logger); + expect(logger.deprecations, equals({Deprecation.importerWithoutUrl})); + }); + + test("compileStringToResult", () { + var logger = _DeprecationTrackingLogger(); + compileStringToResult("", + importer: FilesystemImporter('.'), logger: logger); + expect(logger.deprecations, equals({Deprecation.importerWithoutUrl})); + }); + + test("compileStringToResultAsync", () async { + var logger = _DeprecationTrackingLogger(); + await compileStringToResultAsync("", + importer: FilesystemImporter('.'), logger: logger); + expect(logger.deprecations, equals({Deprecation.importerWithoutUrl})); + }); + }); } -/// Confirms that [source] will error if [deprecation] is fatal. -void _expectDeprecation(String source, Deprecation deprecation) { - try { - compileStringToResult(source, fatalDeprecations: {deprecation}); - } catch (e) { - if (e.toString().contains("$deprecation deprecation to be fatal")) return; - fail('Unexpected error: $e'); +/// A logger that tracks which deprecations were emitted by a Sass compilation. +class _DeprecationTrackingLogger extends LoggerWithDeprecationType { + /// The deprecations encountered by this logger during the compilation. + final deprecations = {}; + + /// The internal logger to which to delegate non-deprecation messages. + final _inner = Logger.stderr(); + + @override + void internalWarn(String message, + {FileSpan? span, Trace? trace, Deprecation? deprecation}) { + if (deprecation != null) { + deprecations.add(deprecation); + } else { + _inner.warn(message, span: span, trace: trace); + } } - fail("No error for violating $deprecation."); + + @override + void debug(String message, SourceSpan span) => _inner.debug(message, span); +} + +/// Verifies that [source] emits [deprecation] when compiled. +void _expectDeprecation(String source, Deprecation deprecation) { + var logger = _DeprecationTrackingLogger(); + compileStringToResult(source, logger: logger); + expect(logger.deprecations, equals({deprecation})); } diff --git a/test/embedded/file_importer_test.dart b/test/embedded/file_importer_test.dart index 8d5bf4ec2..f8f11a447 100644 --- a/test/embedded/file_importer_test.dart +++ b/test/embedded/file_importer_test.dart @@ -8,6 +8,7 @@ import 'package:path/path.dart' as p; import 'package:test/test.dart'; import 'package:test_descriptor/test_descriptor.dart' as d; +import 'package:sass/src/deprecation.dart'; import 'package:sass/src/embedded/embedded_sass.pb.dart'; import 'package:sass/src/embedded/utils.dart'; @@ -254,6 +255,8 @@ void main() { importer: InboundMessage_CompileRequest_Importer() ..fileImporterId = 1)); + await expectDeprecationMessage(process, Deprecation.importerWithoutUrl); + var request = await getFileImportRequest(process); expect(request.url, equals("other")); diff --git a/test/embedded/importer_test.dart b/test/embedded/importer_test.dart index fdfc904d2..3a23dc431 100644 --- a/test/embedded/importer_test.dart +++ b/test/embedded/importer_test.dart @@ -8,6 +8,7 @@ import 'package:source_maps/source_maps.dart' as source_maps; import 'package:test/test.dart'; import 'package:test_descriptor/test_descriptor.dart' as d; +import 'package:sass/src/deprecation.dart'; import 'package:sass/src/embedded/embedded_sass.pb.dart'; import 'package:sass/src/embedded/utils.dart'; @@ -106,6 +107,13 @@ void main() { }); }); + test("emits a deprecation for an importer without a base URL", () async { + process.send(compileString('', + importer: InboundMessage_CompileRequest_Importer()..importerId = 1)); + await expectDeprecationMessage(process, Deprecation.importerWithoutUrl); + await process.close(); + }); + group("canonicalization", () { group("emits a compile failure", () { test("for a canonicalize response with an empty URL", () async { @@ -522,7 +530,8 @@ void main() { test("handles an importer for a string compile request", () async { process.send(compileString("@import 'other'", - importer: InboundMessage_CompileRequest_Importer()..importerId = 1)); + importer: InboundMessage_CompileRequest_Importer()..importerId = 1, + url: "x:foo/bar.scss")); await _canonicalize(process); var request = await getImportRequest(process); diff --git a/test/embedded/utils.dart b/test/embedded/utils.dart index 7741706d6..a676364e5 100644 --- a/test/embedded/utils.dart +++ b/test/embedded/utils.dart @@ -5,6 +5,7 @@ import 'package:path/path.dart' as p; import 'package:test/test.dart'; +import 'package:sass/src/deprecation.dart'; import 'package:sass/src/embedded/embedded_sass.pb.dart'; import 'package:sass/src/embedded/utils.dart'; import 'package:sass/src/util/nullable.dart'; @@ -180,6 +181,15 @@ Future getLogEvent(EmbeddedProcess process) async { return message.logEvent; } +/// Asserts that [process] emits a deprecation warning of the given type. +Future expectDeprecationMessage( + EmbeddedProcess process, Deprecation deprecation) async { + var event = await getLogEvent(process); + expect(event.type, equals(LogEventType.DEPRECATION_WARNING), + reason: "Expected a deprecation warning."); + expect(event.deprecationType, equals('importer-without-url')); +} + /// Asserts that [process] emits a `CompileResponse` with CSS that matches /// [css], with a source map that matches [sourceMap] (if passed). ///