From 2a9eaadefa5583f728aa46c7db1cbd06623f0061 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Wed, 17 Apr 2024 14:33:10 -0700 Subject: [PATCH 01/14] Implement access tracking for containingUrl (#2220) Co-authored-by: Natalie Weizenbaum --- lib/src/async_import_cache.dart | 18 +++---- lib/src/embedded/importer/file.dart | 4 +- lib/src/embedded/importer/host.dart | 4 +- lib/src/import_cache.dart | 20 ++++---- lib/src/importer/async.dart | 15 +++++- lib/src/importer/canonicalize_context.dart | 47 +++++++++++++++++++ lib/src/importer/js_to_dart/async.dart | 8 ++-- lib/src/importer/js_to_dart/async_file.dart | 10 ++-- lib/src/importer/js_to_dart/file.dart | 10 ++-- lib/src/importer/js_to_dart/sync.dart | 8 ++-- lib/src/importer/utils.dart | 38 ++++++++------- lib/src/js.dart | 2 + lib/src/js/importer.dart | 10 +--- lib/src/js/importer/canonicalize_context.dart | 16 +++++++ lib/src/js/value/mixin.dart | 3 +- 15 files changed, 141 insertions(+), 72 deletions(-) create mode 100644 lib/src/importer/canonicalize_context.dart create mode 100644 lib/src/js/importer/canonicalize_context.dart diff --git a/lib/src/async_import_cache.dart b/lib/src/async_import_cache.dart index 576094cb4..8ddc66f3d 100644 --- a/lib/src/async_import_cache.dart +++ b/lib/src/async_import_cache.dart @@ -11,6 +11,7 @@ import 'package:path/path.dart' as p; import 'ast/sass.dart'; import 'deprecation.dart'; import 'importer.dart'; +import 'importer/canonicalize_context.dart'; import 'importer/no_op.dart'; import 'importer/utils.dart'; import 'io.dart'; @@ -206,18 +207,17 @@ final class AsyncImportCache { /// that result is cacheable at all. Future<(AsyncCanonicalizeResult?, bool cacheable)> _canonicalize( AsyncImporter importer, Uri url, Uri? baseUrl, bool forImport) async { - var canonicalize = forImport - ? () => inImportRule(() => importer.canonicalize(url)) - : () => importer.canonicalize(url); - var passContainingUrl = baseUrl != null && (url.scheme == '' || await importer.isNonCanonicalScheme(url.scheme)); - var result = await withContainingUrl( - passContainingUrl ? baseUrl : null, canonicalize); - // TODO(sass/dart-sass#2208): Determine whether the containing URL was - // _actually_ accessed rather than assuming it was. - var cacheable = !passContainingUrl || importer is FilesystemImporter; + var canonicalizeContext = + CanonicalizeContext(passContainingUrl ? baseUrl : null, forImport); + + var result = await withCanonicalizeContext( + canonicalizeContext, () => importer.canonicalize(url)); + + var cacheable = + !passContainingUrl || !canonicalizeContext.wasContainingUrlAccessed; if (result == null) return (null, cacheable); diff --git a/lib/src/embedded/importer/file.dart b/lib/src/embedded/importer/file.dart index 57d97ddf9..7c4d9975c 100644 --- a/lib/src/embedded/importer/file.dart +++ b/lib/src/embedded/importer/file.dart @@ -21,10 +21,12 @@ final class FileImporter extends ImporterBase { ..importerId = _importerId ..url = url.toString() ..fromImport = fromImport; - if (containingUrl case var containingUrl?) { + if (canonicalizeContext.containingUrlWithoutMarking + case var containingUrl?) { request.containingUrl = containingUrl.toString(); } var response = dispatcher.sendFileImportRequest(request); + if (!response.containingUrlUnused) canonicalizeContext.containingUrl; switch (response.whichResult()) { case InboundMessage_FileImportResponse_Result.fileUrl: diff --git a/lib/src/embedded/importer/host.dart b/lib/src/embedded/importer/host.dart index 25245721b..e5342dc31 100644 --- a/lib/src/embedded/importer/host.dart +++ b/lib/src/embedded/importer/host.dart @@ -35,10 +35,12 @@ final class HostImporter extends ImporterBase { ..importerId = _importerId ..url = url.toString() ..fromImport = fromImport; - if (containingUrl case var containingUrl?) { + if (canonicalizeContext.containingUrlWithoutMarking + case var containingUrl?) { request.containingUrl = containingUrl.toString(); } var response = dispatcher.sendCanonicalizeRequest(request); + if (!response.containingUrlUnused) canonicalizeContext.containingUrl; return switch (response.whichResult()) { InboundMessage_CanonicalizeResponse_Result.url => diff --git a/lib/src/import_cache.dart b/lib/src/import_cache.dart index 6204971e4..7e5c941a0 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: 37dd173d676ec6cf201a25b3cca9ac81d92b1433 +// Checksum: 36bc42050cf2eb3a43f36376c4f06c1708eee777 // // ignore_for_file: unused_import @@ -18,6 +18,7 @@ import 'package:path/path.dart' as p; import 'ast/sass.dart'; import 'deprecation.dart'; import 'importer.dart'; +import 'importer/canonicalize_context.dart'; import 'importer/no_op.dart'; import 'importer/utils.dart'; import 'io.dart'; @@ -206,18 +207,17 @@ final class ImportCache { /// that result is cacheable at all. (CanonicalizeResult?, bool cacheable) _canonicalize( Importer importer, Uri url, Uri? baseUrl, bool forImport) { - var canonicalize = forImport - ? () => inImportRule(() => importer.canonicalize(url)) - : () => importer.canonicalize(url); - var passContainingUrl = baseUrl != null && (url.scheme == '' || importer.isNonCanonicalScheme(url.scheme)); - var result = - withContainingUrl(passContainingUrl ? baseUrl : null, canonicalize); - // TODO(sass/dart-sass#2208): Determine whether the containing URL was - // _actually_ accessed rather than assuming it was. - var cacheable = !passContainingUrl || importer is FilesystemImporter; + var canonicalizeContext = + CanonicalizeContext(passContainingUrl ? baseUrl : null, forImport); + + var result = withCanonicalizeContext( + canonicalizeContext, () => importer.canonicalize(url)); + + var cacheable = + !passContainingUrl || !canonicalizeContext.wasContainingUrlAccessed; if (result == null) return (null, cacheable); diff --git a/lib/src/importer/async.dart b/lib/src/importer/async.dart index d7d6951fe..d777d84f5 100644 --- a/lib/src/importer/async.dart +++ b/lib/src/importer/async.dart @@ -6,6 +6,7 @@ import 'dart:async'; import 'package:meta/meta.dart'; +import 'canonicalize_context.dart'; import 'result.dart'; import 'utils.dart' as utils; @@ -54,7 +55,19 @@ abstract class AsyncImporter { /// Outside of that context, its value is undefined and subject to change. @protected @nonVirtual - Uri? get containingUrl => utils.containingUrl; + Uri? get containingUrl => utils.canonicalizeContext.containingUrl; + + /// The canonicalize context of the stylesheet that caused the current + /// [canonicalize] invocation. + /// + /// Subclasses should only access this from within calls to [canonicalize]. + /// Outside of that context, its value is undefined and subject to change. + /// + /// @nodoc + @internal + @protected + @nonVirtual + CanonicalizeContext get canonicalizeContext => utils.canonicalizeContext; /// If [url] is recognized by this importer, returns its canonical format. /// diff --git a/lib/src/importer/canonicalize_context.dart b/lib/src/importer/canonicalize_context.dart new file mode 100644 index 000000000..e28e69e8d --- /dev/null +++ b/lib/src/importer/canonicalize_context.dart @@ -0,0 +1,47 @@ +// Copyright 2024 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +import 'dart:async'; + +import 'package:meta/meta.dart'; + +/// Contextual information used by importers' `canonicalize` method. +@internal +final class CanonicalizeContext { + /// Whether the Sass compiler is currently evaluating an `@import` rule. + bool get fromImport => _fromImport; + bool _fromImport; + + /// The URL of the stylesheet that contains the current load. + Uri? get containingUrl { + _wasContainingUrlAccessed = true; + return _containingUrl; + } + + final Uri? _containingUrl; + + /// Returns the same value as [containingUrl], but doesn't mark it accessed. + Uri? get containingUrlWithoutMarking => _containingUrl; + + /// Whether [containingUrl] has been accessed. + /// + /// This is used to determine whether canonicalize result is cacheable. + bool get wasContainingUrlAccessed => _wasContainingUrlAccessed; + var _wasContainingUrlAccessed = false; + + /// Runs [callback] in a context with specificed [fromImport]. + T withFromImport(bool fromImport, T callback()) { + assert(Zone.current[#_canonicalizeContext] == this); + + var oldFromImport = _fromImport; + _fromImport = fromImport; + try { + return callback(); + } finally { + _fromImport = oldFromImport; + } + } + + CanonicalizeContext(this._containingUrl, this._fromImport); +} diff --git a/lib/src/importer/js_to_dart/async.dart b/lib/src/importer/js_to_dart/async.dart index 11ffbd735..c6d26cbe2 100644 --- a/lib/src/importer/js_to_dart/async.dart +++ b/lib/src/importer/js_to_dart/async.dart @@ -13,6 +13,7 @@ import '../../js/url.dart'; import '../../js/utils.dart'; import '../../util/nullable.dart'; import '../async.dart'; +import '../canonicalize_context.dart'; import '../result.dart'; import 'utils.dart'; @@ -38,11 +39,8 @@ final class JSToDartAsyncImporter extends AsyncImporter { } FutureOr canonicalize(Uri url) async { - var result = wrapJSExceptions(() => _canonicalize( - url.toString(), - CanonicalizeContext( - fromImport: fromImport, - containingUrl: containingUrl.andThen(dartToJSUrl)))); + var result = wrapJSExceptions( + () => _canonicalize(url.toString(), canonicalizeContext)); if (isPromise(result)) result = await promiseToFuture(result as Promise); if (result == null) return null; diff --git a/lib/src/importer/js_to_dart/async_file.dart b/lib/src/importer/js_to_dart/async_file.dart index 7be4b9461..95b2af908 100644 --- a/lib/src/importer/js_to_dart/async_file.dart +++ b/lib/src/importer/js_to_dart/async_file.dart @@ -8,11 +8,10 @@ import 'package:cli_pkg/js.dart'; import 'package:node_interop/js.dart'; import 'package:node_interop/util.dart'; -import '../../js/importer.dart'; import '../../js/url.dart'; import '../../js/utils.dart'; -import '../../util/nullable.dart'; import '../async.dart'; +import '../canonicalize_context.dart'; import '../filesystem.dart'; import '../result.dart'; import '../utils.dart'; @@ -28,11 +27,8 @@ final class JSToDartAsyncFileImporter extends AsyncImporter { FutureOr canonicalize(Uri url) async { if (url.scheme == 'file') return FilesystemImporter.cwd.canonicalize(url); - var result = wrapJSExceptions(() => _findFileUrl( - url.toString(), - CanonicalizeContext( - fromImport: fromImport, - containingUrl: containingUrl.andThen(dartToJSUrl)))); + var result = wrapJSExceptions( + () => _findFileUrl(url.toString(), canonicalizeContext)); if (isPromise(result)) result = await promiseToFuture(result as Promise); if (result == null) return null; if (!isJSUrl(result)) { diff --git a/lib/src/importer/js_to_dart/file.dart b/lib/src/importer/js_to_dart/file.dart index e3302f881..555c9df16 100644 --- a/lib/src/importer/js_to_dart/file.dart +++ b/lib/src/importer/js_to_dart/file.dart @@ -6,10 +6,9 @@ import 'package:cli_pkg/js.dart'; import 'package:node_interop/js.dart'; import '../../importer.dart'; -import '../../js/importer.dart'; import '../../js/url.dart'; import '../../js/utils.dart'; -import '../../util/nullable.dart'; +import '../canonicalize_context.dart'; import '../utils.dart'; /// A wrapper for a potentially-asynchronous JS API file importer that exposes @@ -23,11 +22,8 @@ final class JSToDartFileImporter extends Importer { Uri? canonicalize(Uri url) { if (url.scheme == 'file') return FilesystemImporter.cwd.canonicalize(url); - var result = wrapJSExceptions(() => _findFileUrl( - url.toString(), - CanonicalizeContext( - fromImport: fromImport, - containingUrl: containingUrl.andThen(dartToJSUrl)))); + var result = wrapJSExceptions( + () => _findFileUrl(url.toString(), canonicalizeContext)); if (result == null) return null; if (isPromise(result)) { diff --git a/lib/src/importer/js_to_dart/sync.dart b/lib/src/importer/js_to_dart/sync.dart index f69dafb35..06b91310a 100644 --- a/lib/src/importer/js_to_dart/sync.dart +++ b/lib/src/importer/js_to_dart/sync.dart @@ -10,6 +10,7 @@ import '../../js/importer.dart'; import '../../js/url.dart'; import '../../js/utils.dart'; import '../../util/nullable.dart'; +import '../canonicalize_context.dart'; import 'utils.dart'; /// A wrapper for a synchronous JS API importer that exposes it as a Dart @@ -34,11 +35,8 @@ final class JSToDartImporter extends Importer { } Uri? canonicalize(Uri url) { - var result = wrapJSExceptions(() => _canonicalize( - url.toString(), - CanonicalizeContext( - fromImport: fromImport, - containingUrl: containingUrl.andThen(dartToJSUrl)))); + var result = wrapJSExceptions( + () => _canonicalize(url.toString(), canonicalizeContext)); if (result == null) return null; if (isJSUrl(result)) return jsToDartUrl(result as JSUrl); diff --git a/lib/src/importer/utils.dart b/lib/src/importer/utils.dart index a68ae6f5e..4c5c8106c 100644 --- a/lib/src/importer/utils.dart +++ b/lib/src/importer/utils.dart @@ -7,6 +7,7 @@ import 'dart:async'; import 'package:path/path.dart' as p; import '../io.dart'; +import './canonicalize_context.dart'; /// Whether the Sass compiler is currently evaluating an `@import` rule. /// @@ -15,30 +16,35 @@ import '../io.dart'; /// canonicalization should be identical for `@import` and `@use` rules. It's /// admittedly hacky to set this globally, but `@import` will eventually be /// removed, at which point we can delete this and have one consistent behavior. -bool get fromImport => Zone.current[#_inImportRule] as bool? ?? false; - -/// The URL of the stylesheet that contains the current load. -Uri? get containingUrl => switch (Zone.current[#_containingUrl]) { +bool get fromImport => + ((Zone.current[#_canonicalizeContext] as CanonicalizeContext?) + ?.fromImport ?? + false); + +/// The CanonicalizeContext of the current load. +CanonicalizeContext get canonicalizeContext => + switch (Zone.current[#_canonicalizeContext]) { null => throw StateError( - "containingUrl may only be accessed within a call to canonicalize()."), - #_none => null, - Uri url => url, + "canonicalizeContext may only be accessed within a call to canonicalize()."), + CanonicalizeContext context => context, var value => throw StateError( - "Unexpected Zone.current[#_containingUrl] value $value.") + "Unexpected Zone.current[#_canonicalizeContext] value $value.") }; /// Runs [callback] in a context where [fromImport] returns `true` and /// [resolveImportPath] uses `@import` semantics rather than `@use` semantics. T inImportRule(T callback()) => - runZoned(callback, zoneValues: {#_inImportRule: true}); + switch (Zone.current[#_canonicalizeContext]) { + null => runZoned(callback, + zoneValues: {#_canonicalizeContext: CanonicalizeContext(null, true)}), + CanonicalizeContext context => context.withFromImport(true, callback), + var value => throw StateError( + "Unexpected Zone.current[#_canonicalizeContext] value $value.") + }; -/// Runs [callback] in a context where [containingUrl] returns [url]. -/// -/// If [when] is `false`, runs [callback] without setting [containingUrl]. -T withContainingUrl(Uri? url, T callback()) => - // Use #_none as a sentinel value so we can distinguish a containing URL - // that's set to null from one that's unset at all. - runZoned(callback, zoneValues: {#_containingUrl: url ?? #_none}); +/// Runs [callback] in the given context. +T withCanonicalizeContext(CanonicalizeContext? context, T callback()) => + runZoned(callback, zoneValues: {#_canonicalizeContext: context}); /// Resolves an imported path using the same logic as the filesystem importer. /// diff --git a/lib/src/js.dart b/lib/src/js.dart index 79bf90180..dc0384bc4 100644 --- a/lib/src/js.dart +++ b/lib/src/js.dart @@ -7,6 +7,7 @@ import 'package:js/js_util.dart'; import 'js/exception.dart'; import 'js/deprecations.dart'; import 'js/exports.dart'; +import 'js/importer/canonicalize_context.dart'; import 'js/compile.dart'; import 'js/compiler.dart'; import 'js/legacy.dart'; @@ -64,6 +65,7 @@ void main() { "dart2js\t${const String.fromEnvironment('dart-version')}\t" "(Dart Compiler)\t[Dart]"; + updateCanonicalizeContextPrototype(); updateSourceSpanPrototype(); // Legacy API diff --git a/lib/src/js/importer.dart b/lib/src/js/importer.dart index 0469737ee..09ffcd665 100644 --- a/lib/src/js/importer.dart +++ b/lib/src/js/importer.dart @@ -4,6 +4,7 @@ import 'package:js/js.dart'; +import '../importer/canonicalize_context.dart'; import 'url.dart'; @JS() @@ -15,15 +16,6 @@ class JSImporter { external Object? get nonCanonicalScheme; } -@JS() -@anonymous -class CanonicalizeContext { - external bool get fromImport; - external JSUrl? get containingUrl; - - external factory CanonicalizeContext({bool fromImport, JSUrl? containingUrl}); -} - @JS() @anonymous class JSImporterResult { diff --git a/lib/src/js/importer/canonicalize_context.dart b/lib/src/js/importer/canonicalize_context.dart new file mode 100644 index 000000000..412f21ce8 --- /dev/null +++ b/lib/src/js/importer/canonicalize_context.dart @@ -0,0 +1,16 @@ +// Copyright 2014 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +import '../../importer/canonicalize_context.dart'; +import '../../util/nullable.dart'; +import '../reflection.dart'; +import '../utils.dart'; + +/// Adds JS members to Dart's `CanonicalizeContext` class. +void updateCanonicalizeContextPrototype() => + getJSClass(CanonicalizeContext(null, false)).defineGetters({ + 'fromImport': (CanonicalizeContext self) => self.fromImport, + 'containingUrl': (CanonicalizeContext self) => + self.containingUrl.andThen(dartToJSUrl), + }); diff --git a/lib/src/js/value/mixin.dart b/lib/src/js/value/mixin.dart index a41b394d2..cc55f3eb4 100644 --- a/lib/src/js/value/mixin.dart +++ b/lib/src/js/value/mixin.dart @@ -13,7 +13,8 @@ import '../utils.dart'; final JSClass mixinClass = () { var jsClass = createJSClass('sass.SassMixin', (Object self) { jsThrow(JsError( - 'It is not possible to construct a SassMixin through the JavaScript API')); + 'It is not possible to construct a SassMixin through the JavaScript ' + 'API')); }); getJSClass(SassMixin(Callable('f', '', (_) => sassNull))) From b97f26f71f8078c3a2c4efb954c21733165ba051 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Wed, 17 Apr 2024 17:14:04 -0700 Subject: [PATCH 02/14] Add a per-importer cache for loads that aren't cacheable en masse (#2219) Inspired by comments on #2215 --- lib/src/async_import_cache.dart | 93 ++++++++++++++++++++------------- lib/src/import_cache.dart | 93 ++++++++++++++++++++------------- lib/src/util/map.dart | 6 +++ lib/src/util/option.dart | 12 +++++ 4 files changed, 133 insertions(+), 71 deletions(-) create mode 100644 lib/src/util/option.dart diff --git a/lib/src/async_import_cache.dart b/lib/src/async_import_cache.dart index 8ddc66f3d..6d6e4fa8c 100644 --- a/lib/src/async_import_cache.dart +++ b/lib/src/async_import_cache.dart @@ -16,6 +16,7 @@ import 'importer/no_op.dart'; import 'importer/utils.dart'; import 'io.dart'; import 'logger.dart'; +import 'util/map.dart'; import 'util/nullable.dart'; import 'utils.dart'; @@ -44,30 +45,28 @@ final class AsyncImportCache { /// The `forImport` in each key is true when this canonicalization is for an /// `@import` rule. Otherwise, it's for a `@use` or `@forward` rule. /// - /// This cache isn't used for relative imports, because they depend on the - /// specific base importer. That's stored separately in - /// [_relativeCanonicalizeCache]. + /// This cache covers loads that go through the entire chain of [_importers], + /// but it doesn't cover individual loads or loads in which any importer + /// accesses `containingUrl`. See also [_perImporterCanonicalizeCache]. final _canonicalizeCache = <(Uri, {bool forImport}), AsyncCanonicalizeResult?>{}; - /// The canonicalized URLs for each non-canonical URL that's resolved using a - /// relative importer. + /// Like [_canonicalizeCache] but also includes the specific importer in the + /// key. /// - /// The map's keys have four parts: + /// This is used to cache both relative imports from the base importer and + /// individual importer results in the case where some other component of the + /// importer chain isn't cacheable. + final _perImporterCanonicalizeCache = + <(AsyncImporter, Uri, {bool forImport}), AsyncCanonicalizeResult?>{}; + + /// A map from the keys in [_perImporterCanonicalizeCache] that are generated + /// for relative URL loads agains the base importer to the original relative + /// URLs what were loaded. /// - /// 1. The URL passed to [canonicalize] (the same as in [_canonicalizeCache]). - /// 2. Whether the canonicalization is for an `@import` rule. - /// 3. The `baseImporter` passed to [canonicalize]. - /// 4. The `baseUrl` passed to [canonicalize]. - /// - /// The map's values are the same as the return value of [canonicalize]. - final _relativeCanonicalizeCache = <( - Uri, { - bool forImport, - AsyncImporter baseImporter, - Uri? baseUrl - }), - AsyncCanonicalizeResult?>{}; + /// This is used to invalidate the cache when files are changed. + final _nonCanonicalRelativeUrls = + <(AsyncImporter, Uri, {bool forImport}), Uri>{}; /// The parsed stylesheets for each canonicalized import URL. final _importCache = {}; @@ -155,18 +154,17 @@ final class AsyncImportCache { } if (baseImporter != null && url.scheme == '') { - var relativeResult = await putIfAbsentAsync(_relativeCanonicalizeCache, ( - url, - forImport: forImport, - baseImporter: baseImporter, - baseUrl: baseUrl - ), () async { - var (result, cacheable) = await _canonicalize( - baseImporter, baseUrl?.resolveUri(url) ?? url, baseUrl, forImport); + var resolvedUrl = baseUrl?.resolveUri(url) ?? url; + var key = (baseImporter, resolvedUrl, forImport: forImport); + var relativeResult = + await putIfAbsentAsync(_perImporterCanonicalizeCache, key, () async { + var (result, cacheable) = + await _canonicalize(baseImporter, resolvedUrl, baseUrl, forImport); assert( cacheable, "Relative loads should always be cacheable because they never " "provide access to the containing URL."); + if (baseUrl != null) _nonCanonicalRelativeUrls[key] = url; return result; }); if (relativeResult != null) return relativeResult; @@ -182,17 +180,41 @@ final class AsyncImportCache { // `canonicalize()` calls we've attempted are cacheable. Only if they are do // we store the result in the cache. var cacheable = true; - for (var importer in _importers) { + for (var i = 0; i < _importers.length; i++) { + var importer = _importers[i]; + var perImporterKey = (importer, url, forImport: forImport); + switch (_perImporterCanonicalizeCache.getOption(perImporterKey)) { + case (var result?,): + return result; + case (null,): + continue; + } + switch (await _canonicalize(importer, url, baseUrl, forImport)) { case (var result?, true) when cacheable: _canonicalizeCache[key] = result; return result; - case (var result?, _): - return result; - - case (_, false): - cacheable = false; + case (var result, true) when !cacheable: + _perImporterCanonicalizeCache[perImporterKey] = result; + if (result != null) return result; + + case (var result, false): + if (cacheable) { + // If this is the first uncacheable result, add all previous results + // to the per-importer cache so we don't have to re-run them for + // future uses of this importer. + for (var j = 0; j < i; j++) { + _perImporterCanonicalizeCache[( + _importers[j], + url, + forImport: forImport + )] = null; + } + cacheable = false; + } + + if (result != null) return result; } } @@ -315,7 +337,7 @@ final class AsyncImportCache { Uri sourceMapUrl(Uri canonicalUrl) => _resultsCache[canonicalUrl]?.sourceMapUrl ?? canonicalUrl; - /// Clears the cached canonical version of the given [url]. + /// Clears the cached canonical version of the given non-canonical [url]. /// /// Has no effect if the canonical version of [url] has not been cached. /// @@ -324,7 +346,8 @@ final class AsyncImportCache { void clearCanonicalize(Uri url) { _canonicalizeCache.remove((url, forImport: false)); _canonicalizeCache.remove((url, forImport: true)); - _relativeCanonicalizeCache.removeWhere((key, _) => key.$1 == url); + _perImporterCanonicalizeCache.removeWhere( + (key, _) => key.$2 == url || _nonCanonicalRelativeUrls[key] == url); } /// Clears the cached parse tree for the stylesheet with the given diff --git a/lib/src/import_cache.dart b/lib/src/import_cache.dart index 7e5c941a0..9590b0e5a 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: 36bc42050cf2eb3a43f36376c4f06c1708eee777 +// Checksum: 4362e28e5cd425786c235d2a6a2bb60539403799 // // ignore_for_file: unused_import @@ -23,6 +23,7 @@ import 'importer/no_op.dart'; import 'importer/utils.dart'; import 'io.dart'; import 'logger.dart'; +import 'util/map.dart'; import 'util/nullable.dart'; import 'utils.dart'; @@ -47,29 +48,26 @@ final class ImportCache { /// The `forImport` in each key is true when this canonicalization is for an /// `@import` rule. Otherwise, it's for a `@use` or `@forward` rule. /// - /// This cache isn't used for relative imports, because they depend on the - /// specific base importer. That's stored separately in - /// [_relativeCanonicalizeCache]. + /// This cache covers loads that go through the entire chain of [_importers], + /// but it doesn't cover individual loads or loads in which any importer + /// accesses `containingUrl`. See also [_perImporterCanonicalizeCache]. final _canonicalizeCache = <(Uri, {bool forImport}), CanonicalizeResult?>{}; - /// The canonicalized URLs for each non-canonical URL that's resolved using a - /// relative importer. + /// Like [_canonicalizeCache] but also includes the specific importer in the + /// key. /// - /// The map's keys have four parts: + /// This is used to cache both relative imports from the base importer and + /// individual importer results in the case where some other component of the + /// importer chain isn't cacheable. + final _perImporterCanonicalizeCache = + <(Importer, Uri, {bool forImport}), CanonicalizeResult?>{}; + + /// A map from the keys in [_perImporterCanonicalizeCache] that are generated + /// for relative URL loads agains the base importer to the original relative + /// URLs what were loaded. /// - /// 1. The URL passed to [canonicalize] (the same as in [_canonicalizeCache]). - /// 2. Whether the canonicalization is for an `@import` rule. - /// 3. The `baseImporter` passed to [canonicalize]. - /// 4. The `baseUrl` passed to [canonicalize]. - /// - /// The map's values are the same as the return value of [canonicalize]. - final _relativeCanonicalizeCache = <( - Uri, { - bool forImport, - Importer baseImporter, - Uri? baseUrl - }), - CanonicalizeResult?>{}; + /// This is used to invalidate the cache when files are changed. + final _nonCanonicalRelativeUrls = <(Importer, Uri, {bool forImport}), Uri>{}; /// The parsed stylesheets for each canonicalized import URL. final _importCache = {}; @@ -155,18 +153,16 @@ final class ImportCache { } if (baseImporter != null && url.scheme == '') { - var relativeResult = _relativeCanonicalizeCache.putIfAbsent(( - url, - forImport: forImport, - baseImporter: baseImporter, - baseUrl: baseUrl - ), () { - var (result, cacheable) = _canonicalize( - baseImporter, baseUrl?.resolveUri(url) ?? url, baseUrl, forImport); + var resolvedUrl = baseUrl?.resolveUri(url) ?? url; + var key = (baseImporter, resolvedUrl, forImport: forImport); + var relativeResult = _perImporterCanonicalizeCache.putIfAbsent(key, () { + var (result, cacheable) = + _canonicalize(baseImporter, resolvedUrl, baseUrl, forImport); assert( cacheable, "Relative loads should always be cacheable because they never " "provide access to the containing URL."); + if (baseUrl != null) _nonCanonicalRelativeUrls[key] = url; return result; }); if (relativeResult != null) return relativeResult; @@ -182,17 +178,41 @@ final class ImportCache { // `canonicalize()` calls we've attempted are cacheable. Only if they are do // we store the result in the cache. var cacheable = true; - for (var importer in _importers) { + for (var i = 0; i < _importers.length; i++) { + var importer = _importers[i]; + var perImporterKey = (importer, url, forImport: forImport); + switch (_perImporterCanonicalizeCache.getOption(perImporterKey)) { + case (var result?,): + return result; + case (null,): + continue; + } + switch (_canonicalize(importer, url, baseUrl, forImport)) { case (var result?, true) when cacheable: _canonicalizeCache[key] = result; return result; - case (var result?, _): - return result; - - case (_, false): - cacheable = false; + case (var result, true) when !cacheable: + _perImporterCanonicalizeCache[perImporterKey] = result; + if (result != null) return result; + + case (var result, false): + if (cacheable) { + // If this is the first uncacheable result, add all previous results + // to the per-importer cache so we don't have to re-run them for + // future uses of this importer. + for (var j = 0; j < i; j++) { + _perImporterCanonicalizeCache[( + _importers[j], + url, + forImport: forImport + )] = null; + } + cacheable = false; + } + + if (result != null) return result; } } @@ -312,7 +332,7 @@ final class ImportCache { Uri sourceMapUrl(Uri canonicalUrl) => _resultsCache[canonicalUrl]?.sourceMapUrl ?? canonicalUrl; - /// Clears the cached canonical version of the given [url]. + /// Clears the cached canonical version of the given non-canonical [url]. /// /// Has no effect if the canonical version of [url] has not been cached. /// @@ -321,7 +341,8 @@ final class ImportCache { void clearCanonicalize(Uri url) { _canonicalizeCache.remove((url, forImport: false)); _canonicalizeCache.remove((url, forImport: true)); - _relativeCanonicalizeCache.removeWhere((key, _) => key.$1 == url); + _perImporterCanonicalizeCache.removeWhere( + (key, _) => key.$2 == url || _nonCanonicalRelativeUrls[key] == url); } /// Clears the cached parse tree for the stylesheet with the given diff --git a/lib/src/util/map.dart b/lib/src/util/map.dart index 70037fd64..46982a79c 100644 --- a/lib/src/util/map.dart +++ b/lib/src/util/map.dart @@ -2,6 +2,8 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. +import 'option.dart'; + extension MapExtensions on Map { /// If [this] doesn't contain the given [key], sets that key to [value] and /// returns it. @@ -16,4 +18,8 @@ extension MapExtensions on Map { // TODO(nweiz): Remove this once dart-lang/collection#289 is released. /// Like [Map.entries], but returns each entry as a record. Iterable<(K, V)> get pairs => entries.map((e) => (e.key, e.value)); + + /// Returns an option that contains the value at [key] if one exists and null + /// otherwise. + Option getOption(K key) => containsKey(key) ? (this[key]!,) : null; } diff --git a/lib/src/util/option.dart b/lib/src/util/option.dart new file mode 100644 index 000000000..84d296a80 --- /dev/null +++ b/lib/src/util/option.dart @@ -0,0 +1,12 @@ +// Copyright 2024 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +/// A type that represents either the presence of a value of type `T` or its +/// absence. +/// +/// When the option is present, this will be a single-element tuple that +/// contains the value. If it's absent, it will be null. This allows callers to +/// distinguish between a present null value and a value that's absent +/// altogether. +typedef Option = (T,)?; From eafc279ae77382314ff2fd0e005263d50eccfa7c Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Tue, 23 Apr 2024 16:23:33 -0700 Subject: [PATCH 03/14] Explicitly add a breaking change exemption for invalid CSS output (#2225) This was always implicitly the case (as pointed out in the new section), but this makes it explicit. --- README.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/README.md b/README.md index 844cdf0fc..fbc545065 100644 --- a/README.md +++ b/README.md @@ -42,6 +42,7 @@ A [Dart][dart] implementation of [Sass][sass]. **Sass makes CSS fun**. * [Compatibility Policy](#compatibility-policy) * [Browser Compatibility](#browser-compatibility) * [Node.js Compatibility](#nodejs-compatibility) + * [Invalid CSS](#invalid-css) * [Embedded Dart Sass](#embedded-dart-sass) * [Usage](#usage) * [Behavioral Differences from Ruby Sass](#behavioral-differences-from-ruby-sass) @@ -405,6 +406,18 @@ considers itself free to break support if necessary. [the Node.js release page]: https://nodejs.org/en/about/previous-releases +### Invalid CSS + +Changes to the behavior of Sass stylesheets that produce invalid CSS output are +_not_ considered breaking changes. Such changes are almost always necessary when +adding support for new CSS features, and delaying all such features until a new +major version would be unduly burdensome for most users. + +For example, when Sass began parsing `calc()` expressions, the invalid +expression `calc(1 +)` became a Sass error where before it was passed through +as-is. This was not considered a breaking change, because `calc(1 +)` was never +valid CSS to begin with. + ## Embedded Dart Sass Dart Sass includes an implementation of the compiler side of the [Embedded Sass From f145e1c11b394870c53d58f2fd2f51553283a4db Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Tue, 23 Apr 2024 18:02:48 -0700 Subject: [PATCH 04/14] Throw errors for misplaced statements in keyframe blocks (#2226) --- CHANGELOG.md | 4 ++++ lib/src/visitor/async_evaluate.dart | 21 +++++++++++++++++++++ lib/src/visitor/evaluate.dart | 23 ++++++++++++++++++++++- pkg/sass_api/CHANGELOG.md | 4 ++++ pkg/sass_api/pubspec.yaml | 4 ++-- pubspec.yaml | 2 +- 6 files changed, 54 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae92aca14..f908395f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## 1.75.1 + +* Throw errors for misplaced statements in keyframe blocks. + ## 1.75.0 * Fix a bug in which stylesheet canonicalization could be cached incorrectly diff --git a/lib/src/visitor/async_evaluate.dart b/lib/src/visitor/async_evaluate.dart index ce4105bda..8343a9131 100644 --- a/lib/src/visitor/async_evaluate.dart +++ b/lib/src/visitor/async_evaluate.dart @@ -1324,6 +1324,9 @@ final class _EvaluateVisitor if (_declarationName != null) { throw _exception( "At-rules may not be used within nested declarations.", node.span); + } else if (_inKeyframes && _parent is CssKeyframeBlock) { + throw _exception( + "At-rules may not be used within keyframe blocks.", node.span); } var name = await _interpolationToValue(node.name); @@ -1895,6 +1898,9 @@ final class _EvaluateVisitor if (_declarationName != null) { throw _exception( "Media rules may not be used within nested declarations.", node.span); + } else if (_inKeyframes && _parent is CssKeyframeBlock) { + throw _exception( + "At-rules may not be used within keyframe blocks.", node.span); } var queries = await _visitMediaQueries(node.query); @@ -1985,6 +1991,9 @@ final class _EvaluateVisitor if (_declarationName != null) { throw _exception( "Style rules may not be used within nested declarations.", node.span); + } else if (_inKeyframes && _parent is CssKeyframeBlock) { + throw _exception( + "Style rules may not be used within keyframe blocks.", node.span); } var (selectorText, selectorMap) = @@ -2112,6 +2121,9 @@ final class _EvaluateVisitor throw _exception( "Supports rules may not be used within nested declarations.", node.span); + } else if (_inKeyframes && _parent is CssKeyframeBlock) { + throw _exception( + "At-rules may not be used within keyframe blocks.", node.span); } var condition = CssValue( @@ -3270,6 +3282,9 @@ final class _EvaluateVisitor if (_declarationName != null) { throw _exception( "At-rules may not be used within nested declarations.", node.span); + } else if (_inKeyframes && _parent is CssKeyframeBlock) { + throw _exception( + "At-rules may not be used within keyframe blocks.", node.span); } if (node.isChildless) { @@ -3353,6 +3368,9 @@ final class _EvaluateVisitor if (_declarationName != null) { throw _exception( "Media rules may not be used within nested declarations.", node.span); + } else if (_inKeyframes && _parent is CssKeyframeBlock) { + throw _exception( + "At-rules may not be used within keyframe blocks.", node.span); } var mergedQueries = _mediaQueries.andThen( @@ -3401,6 +3419,9 @@ final class _EvaluateVisitor if (_declarationName != null) { throw _exception( "Style rules may not be used within nested declarations.", node.span); + } else if (_inKeyframes && _parent is CssKeyframeBlock) { + throw _exception( + "Style rules may not be used within keyframe blocks.", node.span); } var styleRule = _styleRule; diff --git a/lib/src/visitor/evaluate.dart b/lib/src/visitor/evaluate.dart index 32c4e2764..bbf334ad1 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: 135bf44f65efcbebb4a55b38ada86c754fcdb86b // // ignore_for_file: unused_import @@ -1321,6 +1321,9 @@ final class _EvaluateVisitor if (_declarationName != null) { throw _exception( "At-rules may not be used within nested declarations.", node.span); + } else if (_inKeyframes && _parent is CssKeyframeBlock) { + throw _exception( + "At-rules may not be used within keyframe blocks.", node.span); } var name = _interpolationToValue(node.name); @@ -1887,6 +1890,9 @@ final class _EvaluateVisitor if (_declarationName != null) { throw _exception( "Media rules may not be used within nested declarations.", node.span); + } else if (_inKeyframes && _parent is CssKeyframeBlock) { + throw _exception( + "At-rules may not be used within keyframe blocks.", node.span); } var queries = _visitMediaQueries(node.query); @@ -1975,6 +1981,9 @@ final class _EvaluateVisitor if (_declarationName != null) { throw _exception( "Style rules may not be used within nested declarations.", node.span); + } else if (_inKeyframes && _parent is CssKeyframeBlock) { + throw _exception( + "Style rules may not be used within keyframe blocks.", node.span); } var (selectorText, selectorMap) = @@ -2102,6 +2111,9 @@ final class _EvaluateVisitor throw _exception( "Supports rules may not be used within nested declarations.", node.span); + } else if (_inKeyframes && _parent is CssKeyframeBlock) { + throw _exception( + "At-rules may not be used within keyframe blocks.", node.span); } var condition = @@ -3240,6 +3252,9 @@ final class _EvaluateVisitor if (_declarationName != null) { throw _exception( "At-rules may not be used within nested declarations.", node.span); + } else if (_inKeyframes && _parent is CssKeyframeBlock) { + throw _exception( + "At-rules may not be used within keyframe blocks.", node.span); } if (node.isChildless) { @@ -3323,6 +3338,9 @@ final class _EvaluateVisitor if (_declarationName != null) { throw _exception( "Media rules may not be used within nested declarations.", node.span); + } else if (_inKeyframes && _parent is CssKeyframeBlock) { + throw _exception( + "At-rules may not be used within keyframe blocks.", node.span); } var mergedQueries = _mediaQueries.andThen( @@ -3369,6 +3387,9 @@ final class _EvaluateVisitor if (_declarationName != null) { throw _exception( "Style rules may not be used within nested declarations.", node.span); + } else if (_inKeyframes && _parent is CssKeyframeBlock) { + throw _exception( + "Style rules may not be used within keyframe blocks.", node.span); } var styleRule = _styleRule; diff --git a/pkg/sass_api/CHANGELOG.md b/pkg/sass_api/CHANGELOG.md index 77d3aaa74..028cd0796 100644 --- a/pkg/sass_api/CHANGELOG.md +++ b/pkg/sass_api/CHANGELOG.md @@ -1,3 +1,7 @@ +## 10.2.1 + +* No user-visible changes. + ## 10.2.0 * No user-visible changes. diff --git a/pkg/sass_api/pubspec.yaml b/pkg/sass_api/pubspec.yaml index ff9a9b383..5f7a9be18 100644 --- a/pkg/sass_api/pubspec.yaml +++ b/pkg/sass_api/pubspec.yaml @@ -2,7 +2,7 @@ name: sass_api # Note: Every time we add a new Sass AST node, we need to bump the *major* # version because it's a breaking change for anyone who's implementing the # visitor interface(s). -version: 10.2.0 +version: 10.2.1-dev description: Additional APIs for Dart Sass. homepage: https://github.com/sass/dart-sass @@ -10,7 +10,7 @@ environment: sdk: ">=3.0.0 <4.0.0" dependencies: - sass: 1.75.0 + sass: 1.75.1 dev_dependencies: dartdoc: ^6.0.0 diff --git a/pubspec.yaml b/pubspec.yaml index 54602aa9a..c0f363e2b 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass -version: 1.75.0 +version: 1.75.1-dev description: A Sass implementation in Dart. homepage: https://github.com/sass/dart-sass From 264b2d58b056b0fa65dcbcec1b18cf8f3567bbe0 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Tue, 30 Apr 2024 14:19:59 -0700 Subject: [PATCH 05/14] Deprecate function and mixin names beginning with `--` (#2230) See #2197 --- CHANGELOG.md | 6 +++++- lib/src/deprecation.dart | 4 ++++ lib/src/parse/stylesheet.dart | 24 ++++++++++++++++++++++++ pkg/sass_api/CHANGELOG.md | 4 ++++ pkg/sass_api/pubspec.yaml | 4 ++-- pubspec.yaml | 2 +- 6 files changed, 40 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f908395f9..adfce2e9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,11 @@ -## 1.75.1 +## 1.76.0 * Throw errors for misplaced statements in keyframe blocks. +* Mixins and functions whose names begin with `--` are now deprecated for + forwards-compatibility with the in-progress CSS functions and mixins spec. + This deprecation is named `css-function-mixin`. + ## 1.75.0 * Fix a bug in which stylesheet canonicalization could be cached incorrectly diff --git a/lib/src/deprecation.dart b/lib/src/deprecation.dart index a7412e2ce..b13180e10 100644 --- a/lib/src/deprecation.dart +++ b/lib/src/deprecation.dart @@ -74,6 +74,10 @@ enum Deprecation { description: 'Using the current working directory as an implicit load path.'), + cssFunctionMixin('css-function-mixin', + deprecatedIn: '1.76.0', + description: 'Function and mixin names beginning with --.'), + @Deprecated('This deprecation name was never actually used.') calcInterp('calc-interp', deprecatedIn: null), diff --git a/lib/src/parse/stylesheet.dart b/lib/src/parse/stylesheet.dart index 19662c192..fb1f1b614 100644 --- a/lib/src/parse/stylesheet.dart +++ b/lib/src/parse/stylesheet.dart @@ -846,7 +846,19 @@ abstract class StylesheetParser extends Parser { FunctionRule _functionRule(LineScannerState start) { var precedingComment = lastSilentComment; lastSilentComment = null; + var beforeName = scanner.state; var name = identifier(normalize: true); + + if (name.startsWith('--')) { + logger.warnForDeprecation( + Deprecation.cssFunctionMixin, + 'Sass @function names beginning with -- are deprecated for forward-' + 'compatibility with plain CSS mixins.\n' + '\n' + 'For details, see https://sass-lang.com/d/css-function-mixin', + span: scanner.spanFrom(beforeName)); + } + whitespace(); var arguments = _argumentDeclaration(); @@ -1261,7 +1273,19 @@ abstract class StylesheetParser extends Parser { MixinRule _mixinRule(LineScannerState start) { var precedingComment = lastSilentComment; lastSilentComment = null; + var beforeName = scanner.state; var name = identifier(normalize: true); + + if (name.startsWith('--')) { + logger.warnForDeprecation( + Deprecation.cssFunctionMixin, + 'Sass @mixin names beginning with -- are deprecated for forward-' + 'compatibility with plain CSS mixins.\n' + '\n' + 'For details, see https://sass-lang.com/d/css-function-mixin', + span: scanner.spanFrom(beforeName)); + } + whitespace(); var arguments = scanner.peekChar() == $lparen ? _argumentDeclaration() diff --git a/pkg/sass_api/CHANGELOG.md b/pkg/sass_api/CHANGELOG.md index 028cd0796..d72b35469 100644 --- a/pkg/sass_api/CHANGELOG.md +++ b/pkg/sass_api/CHANGELOG.md @@ -1,3 +1,7 @@ +## 10.3.0 + +* No user-visible changes. + ## 10.2.1 * No user-visible changes. diff --git a/pkg/sass_api/pubspec.yaml b/pkg/sass_api/pubspec.yaml index 5f7a9be18..3d8c3388a 100644 --- a/pkg/sass_api/pubspec.yaml +++ b/pkg/sass_api/pubspec.yaml @@ -2,7 +2,7 @@ name: sass_api # Note: Every time we add a new Sass AST node, we need to bump the *major* # version because it's a breaking change for anyone who's implementing the # visitor interface(s). -version: 10.2.1-dev +version: 10.3.0 description: Additional APIs for Dart Sass. homepage: https://github.com/sass/dart-sass @@ -10,7 +10,7 @@ environment: sdk: ">=3.0.0 <4.0.0" dependencies: - sass: 1.75.1 + sass: 1.76.0 dev_dependencies: dartdoc: ^6.0.0 diff --git a/pubspec.yaml b/pubspec.yaml index c0f363e2b..e78af1daa 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass -version: 1.75.1-dev +version: 1.76.0 description: A Sass implementation in Dart. homepage: https://github.com/sass/dart-sass From 85f39d5ad7056e0afeab4ad649fc34eaed8c89a5 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Mon, 6 May 2024 14:48:24 -0700 Subject: [PATCH 06/14] Allow at-rules in `@keyframes` blocks (#2236) See sass/sass#3859 See sass/sass#3861 See sass/sass-spec#1987 --- CHANGELOG.md | 4 ++++ lib/src/visitor/async_evaluate.dart | 15 --------------- lib/src/visitor/evaluate.dart | 17 +---------------- pkg/sass_api/CHANGELOG.md | 4 ++++ pkg/sass_api/pubspec.yaml | 4 ++-- pubspec.yaml | 2 +- 6 files changed, 12 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index adfce2e9b..52adaf61a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## 1.77.0 + +* *Don't* throw errors for at-rules in keyframe blocks. + ## 1.76.0 * Throw errors for misplaced statements in keyframe blocks. diff --git a/lib/src/visitor/async_evaluate.dart b/lib/src/visitor/async_evaluate.dart index 8343a9131..b564b8323 100644 --- a/lib/src/visitor/async_evaluate.dart +++ b/lib/src/visitor/async_evaluate.dart @@ -1324,9 +1324,6 @@ final class _EvaluateVisitor if (_declarationName != null) { throw _exception( "At-rules may not be used within nested declarations.", node.span); - } else if (_inKeyframes && _parent is CssKeyframeBlock) { - throw _exception( - "At-rules may not be used within keyframe blocks.", node.span); } var name = await _interpolationToValue(node.name); @@ -1898,9 +1895,6 @@ final class _EvaluateVisitor if (_declarationName != null) { throw _exception( "Media rules may not be used within nested declarations.", node.span); - } else if (_inKeyframes && _parent is CssKeyframeBlock) { - throw _exception( - "At-rules may not be used within keyframe blocks.", node.span); } var queries = await _visitMediaQueries(node.query); @@ -2121,9 +2115,6 @@ final class _EvaluateVisitor throw _exception( "Supports rules may not be used within nested declarations.", node.span); - } else if (_inKeyframes && _parent is CssKeyframeBlock) { - throw _exception( - "At-rules may not be used within keyframe blocks.", node.span); } var condition = CssValue( @@ -3282,9 +3273,6 @@ final class _EvaluateVisitor if (_declarationName != null) { throw _exception( "At-rules may not be used within nested declarations.", node.span); - } else if (_inKeyframes && _parent is CssKeyframeBlock) { - throw _exception( - "At-rules may not be used within keyframe blocks.", node.span); } if (node.isChildless) { @@ -3368,9 +3356,6 @@ final class _EvaluateVisitor if (_declarationName != null) { throw _exception( "Media rules may not be used within nested declarations.", node.span); - } else if (_inKeyframes && _parent is CssKeyframeBlock) { - throw _exception( - "At-rules may not be used within keyframe blocks.", node.span); } var mergedQueries = _mediaQueries.andThen( diff --git a/lib/src/visitor/evaluate.dart b/lib/src/visitor/evaluate.dart index bbf334ad1..473728c1c 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: 135bf44f65efcbebb4a55b38ada86c754fcdb86b +// Checksum: 7788c21fd8c721992490ac01d0ef4783dddf3f1f // // ignore_for_file: unused_import @@ -1321,9 +1321,6 @@ final class _EvaluateVisitor if (_declarationName != null) { throw _exception( "At-rules may not be used within nested declarations.", node.span); - } else if (_inKeyframes && _parent is CssKeyframeBlock) { - throw _exception( - "At-rules may not be used within keyframe blocks.", node.span); } var name = _interpolationToValue(node.name); @@ -1890,9 +1887,6 @@ final class _EvaluateVisitor if (_declarationName != null) { throw _exception( "Media rules may not be used within nested declarations.", node.span); - } else if (_inKeyframes && _parent is CssKeyframeBlock) { - throw _exception( - "At-rules may not be used within keyframe blocks.", node.span); } var queries = _visitMediaQueries(node.query); @@ -2111,9 +2105,6 @@ final class _EvaluateVisitor throw _exception( "Supports rules may not be used within nested declarations.", node.span); - } else if (_inKeyframes && _parent is CssKeyframeBlock) { - throw _exception( - "At-rules may not be used within keyframe blocks.", node.span); } var condition = @@ -3252,9 +3243,6 @@ final class _EvaluateVisitor if (_declarationName != null) { throw _exception( "At-rules may not be used within nested declarations.", node.span); - } else if (_inKeyframes && _parent is CssKeyframeBlock) { - throw _exception( - "At-rules may not be used within keyframe blocks.", node.span); } if (node.isChildless) { @@ -3338,9 +3326,6 @@ final class _EvaluateVisitor if (_declarationName != null) { throw _exception( "Media rules may not be used within nested declarations.", node.span); - } else if (_inKeyframes && _parent is CssKeyframeBlock) { - throw _exception( - "At-rules may not be used within keyframe blocks.", node.span); } var mergedQueries = _mediaQueries.andThen( diff --git a/pkg/sass_api/CHANGELOG.md b/pkg/sass_api/CHANGELOG.md index d72b35469..1c068a43f 100644 --- a/pkg/sass_api/CHANGELOG.md +++ b/pkg/sass_api/CHANGELOG.md @@ -1,3 +1,7 @@ +## 10.4.0 + +* No user-visible changes. + ## 10.3.0 * No user-visible changes. diff --git a/pkg/sass_api/pubspec.yaml b/pkg/sass_api/pubspec.yaml index 3d8c3388a..aa5fda388 100644 --- a/pkg/sass_api/pubspec.yaml +++ b/pkg/sass_api/pubspec.yaml @@ -2,7 +2,7 @@ name: sass_api # Note: Every time we add a new Sass AST node, we need to bump the *major* # version because it's a breaking change for anyone who's implementing the # visitor interface(s). -version: 10.3.0 +version: 10.4.0 description: Additional APIs for Dart Sass. homepage: https://github.com/sass/dart-sass @@ -10,7 +10,7 @@ environment: sdk: ">=3.0.0 <4.0.0" dependencies: - sass: 1.76.0 + sass: 1.77.0 dev_dependencies: dartdoc: ^6.0.0 diff --git a/pubspec.yaml b/pubspec.yaml index e78af1daa..8bc82a811 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass -version: 1.76.0 +version: 1.77.0 description: A Sass implementation in Dart. homepage: https://github.com/sass/dart-sass From 372f15cc24973314c08c77b9e29b31f9f801faf3 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Fri, 10 May 2024 15:49:58 -0700 Subject: [PATCH 07/14] Fix `MapExtensions.getOption()` for nullable types (#2241) Closes #2239 --- CHANGELOG.md | 4 ++++ lib/src/util/map.dart | 2 +- pkg/sass_api/CHANGELOG.md | 4 ++++ pkg/sass_api/pubspec.yaml | 4 ++-- pubspec.yaml | 2 +- 5 files changed, 12 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 52adaf61a..0c306b1ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## 1.77.1 + +* Fix a crash that could come up with importers in certain contexts. + ## 1.77.0 * *Don't* throw errors for at-rules in keyframe blocks. diff --git a/lib/src/util/map.dart b/lib/src/util/map.dart index 46982a79c..a61c151df 100644 --- a/lib/src/util/map.dart +++ b/lib/src/util/map.dart @@ -21,5 +21,5 @@ extension MapExtensions on Map { /// Returns an option that contains the value at [key] if one exists and null /// otherwise. - Option getOption(K key) => containsKey(key) ? (this[key]!,) : null; + Option getOption(K key) => containsKey(key) ? (this[key] as V,) : null; } diff --git a/pkg/sass_api/CHANGELOG.md b/pkg/sass_api/CHANGELOG.md index 1c068a43f..63e2b58f8 100644 --- a/pkg/sass_api/CHANGELOG.md +++ b/pkg/sass_api/CHANGELOG.md @@ -1,3 +1,7 @@ +## 10.4.1 + +* No user-visible changes. + ## 10.4.0 * No user-visible changes. diff --git a/pkg/sass_api/pubspec.yaml b/pkg/sass_api/pubspec.yaml index aa5fda388..0db04d6b3 100644 --- a/pkg/sass_api/pubspec.yaml +++ b/pkg/sass_api/pubspec.yaml @@ -2,7 +2,7 @@ name: sass_api # Note: Every time we add a new Sass AST node, we need to bump the *major* # version because it's a breaking change for anyone who's implementing the # visitor interface(s). -version: 10.4.0 +version: 10.4.1 description: Additional APIs for Dart Sass. homepage: https://github.com/sass/dart-sass @@ -10,7 +10,7 @@ environment: sdk: ">=3.0.0 <4.0.0" dependencies: - sass: 1.77.0 + sass: 1.77.1 dev_dependencies: dartdoc: ^6.0.0 diff --git a/pubspec.yaml b/pubspec.yaml index 8bc82a811..4ecb69a9a 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass -version: 1.77.0 +version: 1.77.1 description: A Sass implementation in Dart. homepage: https://github.com/sass/dart-sass From 54a6decfc8dbb7b66bf3f149fd15401007af5891 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 13 May 2024 14:18:05 -0700 Subject: [PATCH 08/14] Bump lints from 3.0.0 to 4.0.0 (#2242) Bumps [lints](https://github.com/dart-lang/lints) from 3.0.0 to 4.0.0. - [Release notes](https://github.com/dart-lang/lints/releases) - [Changelog](https://github.com/dart-lang/lints/blob/main/CHANGELOG.md) - [Commits](https://github.com/dart-lang/lints/compare/v3.0.0...v4.0.0) --- updated-dependencies: - dependency-name: lints dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- pubspec.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pubspec.yaml b/pubspec.yaml index 4ecb69a9a..2bbf7a4ed 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -45,7 +45,7 @@ dev_dependencies: dartdoc: ">=6.0.0 <9.0.0" grinder: ^0.9.0 node_preamble: ^2.0.2 - lints: ">=2.0.0 <4.0.0" + lints: ">=2.0.0 <5.0.0" protoc_plugin: ">=20.0.0 <22.0.0" pub_api_client: ^2.1.1 pubspec_parse: ^1.0.0 From c9f0d3f62317101776a55b828f4cf8cd67cf24e8 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Wed, 15 May 2024 12:49:27 -0700 Subject: [PATCH 09/14] Fix deprecation flags in the CLI and add tests Closes #2244 --- CHANGELOG.md | 9 + lib/src/executable/compile_stylesheet.dart | 4 + lib/src/executable/repl.dart | 12 +- pkg/sass_api/CHANGELOG.md | 4 + pkg/sass_api/pubspec.yaml | 4 +- pubspec.yaml | 2 +- test/cli/dart/deprecations_test.dart | 15 + test/cli/node/deprecations_test.dart | 17 + test/cli/shared/deprecations.dart | 497 +++++++++++++++++++++ 9 files changed, 558 insertions(+), 6 deletions(-) create mode 100644 test/cli/dart/deprecations_test.dart create mode 100644 test/cli/node/deprecations_test.dart create mode 100644 test/cli/shared/deprecations.dart diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c306b1ac..c932688b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,12 @@ +## 1.77.2 + +### Command-Line Interface + +* Properly handle the `--silence-deprecation` flag. + +* Handle the `--fatal-deprecation` and `--future-deprecation` flags for + `--interactive` mode. + ## 1.77.1 * Fix a crash that could come up with importers in certain contexts. diff --git a/lib/src/executable/compile_stylesheet.dart b/lib/src/executable/compile_stylesheet.dart index cd121a6f5..3dbf3dbe0 100644 --- a/lib/src/executable/compile_stylesheet.dart +++ b/lib/src/executable/compile_stylesheet.dart @@ -110,6 +110,7 @@ Future _compileStylesheetWithoutErrorHandling(ExecutableOptions options, verbose: options.verbose, sourceMap: options.emitSourceMap, charset: options.charset, + silenceDeprecations: options.silenceDeprecations, fatalDeprecations: options.fatalDeprecations, futureDeprecations: options.futureDeprecations) : await compileAsync(source, @@ -121,6 +122,7 @@ Future _compileStylesheetWithoutErrorHandling(ExecutableOptions options, verbose: options.verbose, sourceMap: options.emitSourceMap, charset: options.charset, + silenceDeprecations: options.silenceDeprecations, fatalDeprecations: options.fatalDeprecations, futureDeprecations: options.futureDeprecations); } else { @@ -135,6 +137,7 @@ Future _compileStylesheetWithoutErrorHandling(ExecutableOptions options, verbose: options.verbose, sourceMap: options.emitSourceMap, charset: options.charset, + silenceDeprecations: options.silenceDeprecations, fatalDeprecations: options.fatalDeprecations, futureDeprecations: options.futureDeprecations) : compile(source, @@ -146,6 +149,7 @@ Future _compileStylesheetWithoutErrorHandling(ExecutableOptions options, verbose: options.verbose, sourceMap: options.emitSourceMap, charset: options.charset, + silenceDeprecations: options.silenceDeprecations, fatalDeprecations: options.fatalDeprecations, futureDeprecations: options.futureDeprecations); } diff --git a/lib/src/executable/repl.dart b/lib/src/executable/repl.dart index e2e858a26..f79e2de33 100644 --- a/lib/src/executable/repl.dart +++ b/lib/src/executable/repl.dart @@ -12,6 +12,7 @@ import '../exception.dart'; import '../executable/options.dart'; import '../import_cache.dart'; import '../importer/filesystem.dart'; +import '../logger/deprecation_processing.dart'; import '../logger/tracking.dart'; import '../parse/parser.dart'; import '../utils.dart'; @@ -20,7 +21,12 @@ import '../visitor/evaluate.dart'; /// Runs an interactive SassScript shell according to [options]. Future repl(ExecutableOptions options) async { var repl = Repl(prompt: '>> '); - var logger = TrackingLogger(options.logger); + var trackingLogger = TrackingLogger(options.logger); + var logger = DeprecationProcessingLogger(trackingLogger, + silenceDeprecations: options.silenceDeprecations, + fatalDeprecations: options.fatalDeprecations, + futureDeprecations: options.futureDeprecations, + limitRepetition: !options.verbose); var evaluator = Evaluator( importer: FilesystemImporter.cwd, importCache: ImportCache( @@ -46,8 +52,8 @@ Future repl(ExecutableOptions options) async { print(evaluator.evaluate(Expression.parse(line, logger: logger))); } } on SassException catch (error, stackTrace) { - _logError( - error, getTrace(error) ?? stackTrace, line, repl, options, logger); + _logError(error, getTrace(error) ?? stackTrace, line, repl, options, + trackingLogger); } } } diff --git a/pkg/sass_api/CHANGELOG.md b/pkg/sass_api/CHANGELOG.md index 63e2b58f8..c0839df75 100644 --- a/pkg/sass_api/CHANGELOG.md +++ b/pkg/sass_api/CHANGELOG.md @@ -1,3 +1,7 @@ +## 10.4.2 + +* No user-visible changes. + ## 10.4.1 * No user-visible changes. diff --git a/pkg/sass_api/pubspec.yaml b/pkg/sass_api/pubspec.yaml index 0db04d6b3..08eda2c49 100644 --- a/pkg/sass_api/pubspec.yaml +++ b/pkg/sass_api/pubspec.yaml @@ -2,7 +2,7 @@ name: sass_api # Note: Every time we add a new Sass AST node, we need to bump the *major* # version because it's a breaking change for anyone who's implementing the # visitor interface(s). -version: 10.4.1 +version: 10.4.2 description: Additional APIs for Dart Sass. homepage: https://github.com/sass/dart-sass @@ -10,7 +10,7 @@ environment: sdk: ">=3.0.0 <4.0.0" dependencies: - sass: 1.77.1 + sass: 1.77.2 dev_dependencies: dartdoc: ^6.0.0 diff --git a/pubspec.yaml b/pubspec.yaml index 4ecb69a9a..47cec4e93 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass -version: 1.77.1 +version: 1.77.2 description: A Sass implementation in Dart. homepage: https://github.com/sass/dart-sass diff --git a/test/cli/dart/deprecations_test.dart b/test/cli/dart/deprecations_test.dart new file mode 100644 index 000000000..4b4a4244f --- /dev/null +++ b/test/cli/dart/deprecations_test.dart @@ -0,0 +1,15 @@ +// Copyright 2024 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +@TestOn('vm') + +import 'package:test/test.dart'; + +import '../dart_test.dart'; +import '../shared/deprecations.dart'; + +void main() { + setUpAll(ensureSnapshotUpToDate); + sharedTests(runSass); +} diff --git a/test/cli/node/deprecations_test.dart b/test/cli/node/deprecations_test.dart new file mode 100644 index 000000000..88e383d2b --- /dev/null +++ b/test/cli/node/deprecations_test.dart @@ -0,0 +1,17 @@ +// Copyright 2024 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +@TestOn('vm') +@Tags(['node']) + +import 'package:test/test.dart'; + +import '../../ensure_npm_package.dart'; +import '../node_test.dart'; +import '../shared/deprecations.dart'; + +void main() { + setUpAll(ensureNpmPackage); + sharedTests(runSass); +} diff --git a/test/cli/shared/deprecations.dart b/test/cli/shared/deprecations.dart new file mode 100644 index 000000000..03f1b4dc2 --- /dev/null +++ b/test/cli/shared/deprecations.dart @@ -0,0 +1,497 @@ +// Copyright 2024 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +import 'package:test/test.dart'; +import 'package:test_descriptor/test_descriptor.dart' as d; +import 'package:test_process/test_process.dart'; + +/// Defines test that are shared between the Dart and Node.js CLI test suites. +void sharedTests(Future runSass(Iterable arguments)) { + // Test complaining about invalid deprecations, combinations, etc + + group("--silence-deprecation", () { + group("prints a warning", () { + setUp(() => d.file("test.scss", "").create()); + + test("for user-authored", () async { + var sass = + await runSass(["--silence-deprecation=user-authored", "test.scss"]); + expect(sass.stderr, emits(contains("User-authored deprecations"))); + await sass.shouldExit(0); + }); + + test("for an obsolete deprecation", () async { + // TODO: test this when a deprecation is obsoleted + }); + + test("for an inactive future deprecation", () async { + var sass = await runSass(["--silence-deprecation=import", "test.scss"]); + expect(sass.stderr, emits(contains("Future import deprecation"))); + await sass.shouldExit(0); + }); + + test("for an active future deprecation", () async { + var sass = await runSass([ + "--future-deprecation=import", + "--silence-deprecation=import", + "test.scss" + ]); + expect(sass.stderr, emits(contains("Conflicting options for future"))); + await sass.shouldExit(0); + }); + + test("in watch mode", () async { + var sass = await runSass([ + "--watch", + "--poll", + "--silence-deprecation=user-authored", + "test.scss:out.css" + ]); + expect(sass.stderr, emits(contains("User-authored deprecations"))); + + await expectLater(sass.stdout, + emitsThrough(endsWith('Compiled test.scss to out.css.'))); + await sass.kill(); + }); + + test("in repl mode", () async { + var sass = await runSass( + ["--interactive", "--silence-deprecation=user-authored"]); + await expectLater( + sass.stderr, emits(contains("User-authored deprecations"))); + await sass.kill(); + }); + }); + + group("throws an error for an unknown deprecation", () { + setUp(() => d.file("test.scss", "").create()); + + test("in immediate mode", () async { + var sass = + await runSass(["--silence-deprecation=unknown", "test.scss"]); + expect(sass.stdout, emits(contains('Invalid deprecation "unknown".'))); + await sass.shouldExit(64); + }); + + test("in watch mode", () async { + var sass = await runSass([ + "--watch", + "--poll", + "--silence-deprecation=unknown", + "test.scss:out.css" + ]); + expect(sass.stdout, emits(contains('Invalid deprecation "unknown".'))); + await sass.shouldExit(64); + }); + + test("in repl mode", () async { + var sass = + await runSass(["--interactive", "--silence-deprecation=unknown"]); + expect(sass.stdout, emits(contains('Invalid deprecation "unknown".'))); + await sass.shouldExit(64); + }); + }); + + group("silences", () { + group("a parse-time deprecation", () { + setUp( + () => d.file("test.scss", "@if true {} @elseif false {}").create()); + + test("in immediate mode", () async { + var sass = + await runSass(["--silence-deprecation=elseif", "test.scss"]); + expect(sass.stderr, emitsDone); + await sass.shouldExit(0); + }); + + test("in watch mode", () async { + var sass = await runSass([ + "--watch", + "--poll", + "--silence-deprecation=elseif", + "test.scss:out.css" + ]); + expect(sass.stderr, emitsDone); + + await expectLater(sass.stdout, + emitsThrough(endsWith('Compiled test.scss to out.css.'))); + await sass.kill(); + }); + + test("in repl mode", () async { + var sass = await runSass( + ["--interactive", "--silence-deprecation=strict-unary"]); + expect(sass.stderr, emitsDone); + sass.stdin.writeln("4 -(5)"); + await expectLater(sass.stdout, emitsInOrder([">> 4 -(5)", "-1"])); + await sass.kill(); + }); + }); + + group("an evaluation-time deprecation", () { + setUp(() => d.file("test.scss", """ + @use 'sass:math'; + a {b: math.random(1px)} + """).create()); + + test("in immediate mode", () async { + var sass = await runSass( + ["--silence-deprecation=function-units", "test.scss"]); + expect(sass.stderr, emitsDone); + await sass.shouldExit(0); + }); + + test("in watch mode", () async { + var sass = await runSass([ + "--watch", + "--poll", + "--silence-deprecation=function-units", + "test.scss:out.css" + ]); + expect(sass.stderr, emitsDone); + + await expectLater(sass.stdout, + emitsThrough(endsWith('Compiled test.scss to out.css.'))); + await sass.kill(); + }); + + test("in repl mode", () async { + var sass = await runSass( + ["--interactive", "--silence-deprecation=function-units"]); + expect(sass.stderr, emitsDone); + sass.stdin.writeln("@use 'sass:math'"); + await expectLater(sass.stdout, emits(">> @use 'sass:math'")); + sass.stdin.writeln("math.random(1px)"); + await expectLater( + sass.stdout, emitsInOrder([">> math.random(1px)", "1"])); + await sass.kill(); + }); + }); + }); + }); + + group("--fatal-deprecation", () { + group("prints a warning", () { + setUp(() => d.file("test.scss", "").create()); + + test("for an obsolete deprecation", () async { + // TODO: test this when a deprecation is obsoleted + }); + + test("for an inactive future deprecation", () async { + var sass = await runSass(["--fatal-deprecation=import", "test.scss"]); + expect(sass.stderr, emits(contains("Future import deprecation"))); + await sass.shouldExit(0); + }); + + test("for a silent deprecation", () async { + var sass = await runSass([ + "--fatal-deprecation=elseif", + "--silence-deprecation=elseif", + "test.scss" + ]); + expect(sass.stderr, emits(contains("Ignoring setting to silence"))); + await sass.shouldExit(0); + }); + + test("in watch mode", () async { + var sass = await runSass([ + "--watch", + "--poll", + "--fatal-deprecation=elseif", + "--silence-deprecation=elseif", + "test.scss:out.css" + ]); + expect(sass.stderr, emits(contains("Ignoring setting to silence"))); + + await expectLater(sass.stdout, + emitsThrough(endsWith('Compiled test.scss to out.css.'))); + await sass.kill(); + }); + + test("in repl mode", () async { + var sass = await runSass([ + "--interactive", + "--fatal-deprecation=elseif", + "--silence-deprecation=elseif" + ]); + await expectLater( + sass.stderr, emits(contains("Ignoring setting to silence"))); + await sass.kill(); + }); + }); + + group("throws an error for", () { + group("an unknown deprecation", () { + setUp(() => d.file("test.scss", "").create()); + + test("in immediate mode", () async { + var sass = + await runSass(["--fatal-deprecation=unknown", "test.scss"]); + expect( + sass.stdout, emits(contains('Invalid deprecation "unknown".'))); + await sass.shouldExit(64); + }); + + test("in watch mode", () async { + var sass = await runSass([ + "--watch", + "--poll", + "--fatal-deprecation=unknown", + "test.scss:out.css" + ]); + expect( + sass.stdout, emits(contains('Invalid deprecation "unknown".'))); + await sass.shouldExit(64); + }); + + test("in repl mode", () async { + var sass = + await runSass(["--interactive", "--fatal-deprecation=unknown"]); + expect( + sass.stdout, emits(contains('Invalid deprecation "unknown".'))); + await sass.shouldExit(64); + }); + }); + + group("a parse-time deprecation", () { + setUp( + () => d.file("test.scss", "@if true {} @elseif false {}").create()); + + test("in immediate mode", () async { + var sass = await runSass(["--fatal-deprecation=elseif", "test.scss"]); + expect(sass.stderr, emits(startsWith("Error: "))); + await sass.shouldExit(65); + }); + + test("in watch mode", () async { + var sass = await runSass([ + "--watch", + "--poll", + "--fatal-deprecation=elseif", + "test.scss:out.css" + ]); + await expectLater(sass.stderr, emits(startsWith("Error: "))); + await expectLater( + sass.stdout, + emitsInOrder( + ["Sass is watching for changes. Press Ctrl-C to stop.", ""])); + await sass.kill(); + }); + + test("in repl mode", () async { + var sass = await runSass( + ["--interactive", "--fatal-deprecation=strict-unary"]); + sass.stdin.writeln("4 -(5)"); + await expectLater( + sass.stdout, + emitsInOrder([ + ">> 4 -(5)", + emitsThrough(startsWith("Error: ")), + emitsThrough(contains("Remove this setting")) + ])); + + // Verify that there's no output written for the previous line. + sass.stdin.writeln("1"); + await expectLater(sass.stdout, emitsInOrder([">> 1", "1"])); + await sass.kill(); + }); + }); + + group("an evaluation-time deprecation", () { + setUp(() => d.file("test.scss", """ + @use 'sass:math'; + a {b: math.random(1px)} + """).create()); + + test("in immediate mode", () async { + var sass = await runSass( + ["--fatal-deprecation=function-units", "test.scss"]); + expect(sass.stderr, emits(startsWith("Error: "))); + await sass.shouldExit(65); + }); + + test("in watch mode", () async { + var sass = await runSass([ + "--watch", + "--poll", + "--fatal-deprecation=function-units", + "test.scss:out.css" + ]); + await expectLater(sass.stderr, emits(startsWith("Error: "))); + await expectLater( + sass.stdout, + emitsInOrder( + ["Sass is watching for changes. Press Ctrl-C to stop.", ""])); + await sass.kill(); + }); + + test("in repl mode", () async { + var sass = await runSass( + ["--interactive", "--fatal-deprecation=function-units"]); + sass.stdin.writeln("@use 'sass:math'"); + await expectLater(sass.stdout, emits(">> @use 'sass:math'")); + sass.stdin.writeln("math.random(1px)"); + await expectLater( + sass.stdout, + emitsInOrder([ + ">> math.random(1px)", + emitsThrough(startsWith("Error: ")), + emitsThrough(contains("Remove this setting")) + ])); + + // Verify that there's no output written for the previous line. + sass.stdin.writeln("1"); + await expectLater(sass.stdout, emitsInOrder([">> 1", "1"])); + await sass.kill(); + }); + }); + }); + }); + + group("--future-deprecation", () { + group("prints a warning for", () { + group("an active deprecation", () { + setUp(() => d.file("test.scss", "").create()); + + test("in immediate mode", () async { + var sass = await runSass( + ["--future-deprecation=function-units", "test.scss"]); + expect(sass.stderr, + emits(contains("function-units is not a future deprecation"))); + await sass.shouldExit(0); + }); + + test("in watch mode", () async { + var sass = await runSass([ + "--watch", + "--poll", + "--future-deprecation=function-units", + "test.scss:out.css" + ]); + expect(sass.stderr, + emits(contains("function-units is not a future deprecation"))); + + await expectLater(sass.stdout, + emitsThrough(endsWith('Compiled test.scss to out.css.'))); + await sass.kill(); + }); + + test("in repl mode", () async { + // TODO: test this when there's an expression-level future deprecation + }); + }); + + group("an obsolete deprecation", () { + // TODO: test this when there are obsolete deprecations + }); + + group("a parse-time deprecation", () { + setUp(() async { + await d.file("test.scss", "@import 'other';").create(); + await d.file("_other.scss", "").create(); + }); + + test("in immediate mode", () async { + var sass = + await runSass(["--future-deprecation=import", "test.scss"]); + expect(sass.stderr, emits(startsWith("DEPRECATION WARNING"))); + await sass.shouldExit(0); + }); + + test("in watch mode", () async { + var sass = await runSass([ + "--watch", + "--poll", + "--future-deprecation=import", + "test.scss:out.css" + ]); + + await expectLater( + sass.stderr, emits(startsWith("DEPRECATION WARNING"))); + await sass.kill(); + }); + + test("in repl mode", () async { + // TODO: test this when there's an expression-level future deprecation + }); + }); + + group("an evaluation-time deprecation", () { + // TODO: test this when there's an evaluation-time future deprecation + }); + }); + + group("throws an error for", () { + group("an unknown deprecation", () { + setUp(() => d.file("test.scss", "").create()); + + test("in immediate mode", () async { + var sass = + await runSass(["--future-deprecation=unknown", "test.scss"]); + expect( + sass.stdout, emits(contains('Invalid deprecation "unknown".'))); + await sass.shouldExit(64); + }); + + test("in watch mode", () async { + var sass = await runSass([ + "--watch", + "--poll", + "--future-deprecation=unknown", + "test.scss:out.css" + ]); + expect( + sass.stdout, emits(contains('Invalid deprecation "unknown".'))); + await sass.shouldExit(64); + }); + + test("in repl mode", () async { + var sass = + await runSass(["--interactive", "--future-deprecation=unknown"]); + expect( + sass.stdout, emits(contains('Invalid deprecation "unknown".'))); + await sass.shouldExit(64); + }); + }); + + group("a fatal deprecation", () { + setUp(() async { + await d.file("test.scss", "@import 'other';").create(); + await d.file("_other.scss", "").create(); + }); + + test("in immediate mode", () async { + var sass = await runSass([ + "--fatal-deprecation=import", + "--future-deprecation=import", + "test.scss" + ]); + expect(sass.stderr, emits(startsWith("Error: "))); + await sass.shouldExit(65); + }); + + test("in watch mode", () async { + var sass = await runSass([ + "--watch", + "--poll", + "--fatal-deprecation=import", + "--future-deprecation=import", + "test.scss:out.css" + ]); + await expectLater(sass.stderr, emits(startsWith("Error: "))); + await expectLater( + sass.stdout, + emitsInOrder( + ["Sass is watching for changes. Press Ctrl-C to stop.", ""])); + await sass.kill(); + }); + + test("in repl mode", () async { + // TODO: test this when there's an expression-level future deprecation + }); + }); + }); + }); +} From 8c48a013617f96a6582b9c0b069bdab81a83a929 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Wed, 15 May 2024 14:16:31 -0700 Subject: [PATCH 10/14] Expand dartdoc range for sass_api to match sass --- pkg/sass_api/pubspec.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/sass_api/pubspec.yaml b/pkg/sass_api/pubspec.yaml index 08eda2c49..e11f303a5 100644 --- a/pkg/sass_api/pubspec.yaml +++ b/pkg/sass_api/pubspec.yaml @@ -13,7 +13,7 @@ dependencies: sass: 1.77.2 dev_dependencies: - dartdoc: ^6.0.0 + dartdoc: ">=6.0.0 <9.0.0" dependency_overrides: sass: { path: ../.. } From 9a9e48342cccd8f40acc36980a22ffa1c62af3a7 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Wed, 15 May 2024 14:34:21 -0700 Subject: [PATCH 11/14] Add a test to ensure the sass_api dartdoc version matches sass --- test/double_check_test.dart | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/double_check_test.dart b/test/double_check_test.dart index 5cc2a2411..ee87b922d 100644 --- a/test/double_check_test.dart +++ b/test/double_check_test.dart @@ -114,6 +114,17 @@ void main() { expect(pkgPubspec.environment!["sdk"], equals(sassPubspec.environment!["sdk"])); }); + + test("matches dartdoc version", () { + // TODO(nweiz): Just use equals() once dart-lang/pubspec_parse#127 lands + // and is released. + var sassDep = sassPubspec.devDependencies["dartdoc"]; + var pkgDep = pkgPubspec.devDependencies["dartdoc"]; + expect(pkgDep, isA()); + expect(sassDep, isA()); + expect((pkgDep as HostedDependency).version, + equals((sassDep as HostedDependency).version)); + }); }); } } From 5121eb195dea513a0c59c5d9874bebeccf90069c Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Thu, 16 May 2024 15:04:08 -0700 Subject: [PATCH 12/14] Don't treat underscores as hyphens for the purpose of error checking (#2247) --- CHANGELOG.md | 5 ++++ lib/src/ast/sass/expression/function.dart | 15 ++++++----- .../sass/statement/callable_declaration.dart | 10 ++++--- lib/src/ast/sass/statement/include_rule.dart | 9 +++++-- lib/src/parse/stylesheet.dart | 8 +++--- lib/src/visitor/async_evaluate.dart | 25 +++++++++++++++++ lib/src/visitor/evaluate.dart | 27 ++++++++++++++++++- 7 files changed, 81 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c932688b4..9d64f38c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ ## 1.77.2 +* Don't emit deprecation warnings for functions and mixins beginning with `__`. + +* Allow user-defined functions whose names begin with `_` and otherwise look + like vendor-prefixed functions with special CSS syntax. + ### Command-Line Interface * Properly handle the `--silence-deprecation` flag. diff --git a/lib/src/ast/sass/expression/function.dart b/lib/src/ast/sass/expression/function.dart index 398a2ff03..64c508c19 100644 --- a/lib/src/ast/sass/expression/function.dart +++ b/lib/src/ast/sass/expression/function.dart @@ -23,6 +23,12 @@ final class FunctionExpression /// without a namespace. final String? namespace; + /// The name of the function being invoked, with underscores converted to + /// hyphens. + /// + /// If this function is a plain CSS function, use [originalName] instead. + final String name; + /// The name of the function being invoked, with underscores left as-is. final String originalName; @@ -31,12 +37,6 @@ final class FunctionExpression final FileSpan span; - /// The name of the function being invoked, with underscores converted to - /// hyphens. - /// - /// If this function is a plain CSS function, use [originalName] instead. - String get name => originalName.replaceAll('_', '-'); - FileSpan get nameSpan { if (namespace == null) return span.initialIdentifier(); return span.withoutNamespace().initialIdentifier(); @@ -46,7 +46,8 @@ final class FunctionExpression namespace == null ? null : span.initialIdentifier(); FunctionExpression(this.originalName, this.arguments, this.span, - {this.namespace}); + {this.namespace}) + : name = originalName.replaceAll('_', '-'); T accept(ExpressionVisitor visitor) => visitor.visitFunctionExpression(this); diff --git a/lib/src/ast/sass/statement/callable_declaration.dart b/lib/src/ast/sass/statement/callable_declaration.dart index e39b9c035..3ce0ec9a0 100644 --- a/lib/src/ast/sass/statement/callable_declaration.dart +++ b/lib/src/ast/sass/statement/callable_declaration.dart @@ -18,6 +18,9 @@ abstract base class CallableDeclaration /// The name of this callable, with underscores converted to hyphens. final String name; + /// The callable's original name, without underscores converted to hyphens. + final String originalName; + /// The comment immediately preceding this declaration. final SilentComment? comment; @@ -26,8 +29,9 @@ abstract base class CallableDeclaration final FileSpan span; - CallableDeclaration( - this.name, this.arguments, Iterable children, this.span, + CallableDeclaration(this.originalName, this.arguments, + Iterable children, this.span, {this.comment}) - : super(List.unmodifiable(children)); + : name = originalName.replaceAll('_', '-'), + super(List.unmodifiable(children)); } diff --git a/lib/src/ast/sass/statement/include_rule.dart b/lib/src/ast/sass/statement/include_rule.dart index d3c9ceba6..940716cac 100644 --- a/lib/src/ast/sass/statement/include_rule.dart +++ b/lib/src/ast/sass/statement/include_rule.dart @@ -25,6 +25,10 @@ final class IncludeRule /// hyphens. final String name; + /// The original name of the mixin being invoked, without underscores + /// converted to hyphens. + final String originalName; + /// The arguments to pass to the mixin. final ArgumentInvocation arguments; @@ -55,8 +59,9 @@ final class IncludeRule return startSpan.initialIdentifier(); } - IncludeRule(this.name, this.arguments, this.span, - {this.namespace, this.content}); + IncludeRule(this.originalName, this.arguments, this.span, + {this.namespace, this.content}) + : name = originalName.replaceAll('_', '-'); T accept(StatementVisitor visitor) => visitor.visitIncludeRule(this); diff --git a/lib/src/parse/stylesheet.dart b/lib/src/parse/stylesheet.dart index fb1f1b614..e72e2527b 100644 --- a/lib/src/parse/stylesheet.dart +++ b/lib/src/parse/stylesheet.dart @@ -847,7 +847,7 @@ abstract class StylesheetParser extends Parser { var precedingComment = lastSilentComment; lastSilentComment = null; var beforeName = scanner.state; - var name = identifier(normalize: true); + var name = identifier(); if (name.startsWith('--')) { logger.warnForDeprecation( @@ -1221,8 +1221,6 @@ abstract class StylesheetParser extends Parser { if (scanner.scanChar($dot)) { namespace = name; name = _publicIdentifier(); - } else { - name = name.replaceAll("_", "-"); } whitespace(); @@ -1274,7 +1272,7 @@ abstract class StylesheetParser extends Parser { var precedingComment = lastSilentComment; lastSilentComment = null; var beforeName = scanner.state; - var name = identifier(normalize: true); + var name = identifier(); if (name.startsWith('--')) { logger.warnForDeprecation( @@ -3457,7 +3455,7 @@ abstract class StylesheetParser extends Parser { /// Like [identifier], but rejects identifiers that begin with `_` or `-`. String _publicIdentifier() { var start = scanner.state; - var result = identifier(normalize: true); + var result = identifier(); _assertPublic(result, () => scanner.spanFrom(start)); return result; } diff --git a/lib/src/visitor/async_evaluate.dart b/lib/src/visitor/async_evaluate.dart index b564b8323..b8434ff47 100644 --- a/lib/src/visitor/async_evaluate.dart +++ b/lib/src/visitor/async_evaluate.dart @@ -1853,6 +1853,18 @@ final class _EvaluateVisitor Future visitIncludeRule(IncludeRule node) async { var mixin = _addExceptionSpan(node, () => _environment.getMixin(node.name, namespace: node.namespace)); + if (node.originalName.startsWith('--') && + mixin is UserDefinedCallable && + !mixin.declaration.originalName.startsWith('--')) { + _warn( + 'Sass @mixin names beginning with -- are deprecated for forward-' + 'compatibility with plain CSS mixins.\n' + '\n' + 'For details, see https://sass-lang.com/d/css-function-mixin', + node.nameSpan, + Deprecation.cssFunctionMixin); + } + var contentCallable = node.content.andThen((content) => UserDefinedCallable( content, _environment.closure(), inDependency: _inDependency)); @@ -2510,6 +2522,19 @@ final class _EvaluateVisitor PlainCssCallable(node.originalName); } + if (node.originalName.startsWith('--') && + function is UserDefinedCallable && + !function.declaration.originalName.startsWith('--')) { + _warn( + 'Sass @function names beginning with -- are deprecated for forward-' + 'compatibility with plain CSS functions.\n' + '\n' + 'For details, see https://sass-lang.com/d/css-function-mixin', + node.nameSpan, + Deprecation.cssFunctionMixin, + ); + } + var oldInFunction = _inFunction; _inFunction = true; var result = await _addErrorSpan( diff --git a/lib/src/visitor/evaluate.dart b/lib/src/visitor/evaluate.dart index 473728c1c..1680be88e 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: 7788c21fd8c721992490ac01d0ef4783dddf3f1f +// Checksum: 116b8079719577ac6e4dad4aebe403282136e611 // // ignore_for_file: unused_import @@ -1845,6 +1845,18 @@ final class _EvaluateVisitor Value? visitIncludeRule(IncludeRule node) { var mixin = _addExceptionSpan(node, () => _environment.getMixin(node.name, namespace: node.namespace)); + if (node.originalName.startsWith('--') && + mixin is UserDefinedCallable && + !mixin.declaration.originalName.startsWith('--')) { + _warn( + 'Sass @mixin names beginning with -- are deprecated for forward-' + 'compatibility with plain CSS mixins.\n' + '\n' + 'For details, see https://sass-lang.com/d/css-function-mixin', + node.nameSpan, + Deprecation.cssFunctionMixin); + } + var contentCallable = node.content.andThen((content) => UserDefinedCallable( content, _environment.closure(), inDependency: _inDependency)); @@ -2486,6 +2498,19 @@ final class _EvaluateVisitor PlainCssCallable(node.originalName); } + if (node.originalName.startsWith('--') && + function is UserDefinedCallable && + !function.declaration.originalName.startsWith('--')) { + _warn( + 'Sass @function names beginning with -- are deprecated for forward-' + 'compatibility with plain CSS functions.\n' + '\n' + 'For details, see https://sass-lang.com/d/css-function-mixin', + node.nameSpan, + Deprecation.cssFunctionMixin, + ); + } + var oldInFunction = _inFunction; _inFunction = true; var result = _addErrorSpan( From fc24fec7e97148780180f5d8030c3f827240ca83 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Tue, 28 May 2024 17:21:03 -0700 Subject: [PATCH 13/14] Use `pubspec_parse` dependency equality (#2254) --- pubspec.yaml | 2 +- test/double_check_test.dart | 10 ++-------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/pubspec.yaml b/pubspec.yaml index bb285238e..c4acb1234 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -48,7 +48,7 @@ dev_dependencies: lints: ">=2.0.0 <5.0.0" protoc_plugin: ">=20.0.0 <22.0.0" pub_api_client: ^2.1.1 - pubspec_parse: ^1.0.0 + pubspec_parse: ^1.3.0 test: ^1.16.7 test_descriptor: ^2.0.0 test_process: ^2.0.0 diff --git a/test/double_check_test.dart b/test/double_check_test.dart index ee87b922d..1da209885 100644 --- a/test/double_check_test.dart +++ b/test/double_check_test.dart @@ -116,14 +116,8 @@ void main() { }); test("matches dartdoc version", () { - // TODO(nweiz): Just use equals() once dart-lang/pubspec_parse#127 lands - // and is released. - var sassDep = sassPubspec.devDependencies["dartdoc"]; - var pkgDep = pkgPubspec.devDependencies["dartdoc"]; - expect(pkgDep, isA()); - expect(sassDep, isA()); - expect((pkgDep as HostedDependency).version, - equals((sassDep as HostedDependency).version)); + expect(sassPubspec.devDependencies["dartdoc"], + equals(pkgPubspec.devDependencies["dartdoc"])); }); }); } From 1073c7b4121dc292887a3be60fbbdde3af0413f5 Mon Sep 17 00:00:00 2001 From: Jennifer Thakar Date: Wed, 29 May 2024 14:23:43 -0700 Subject: [PATCH 14/14] Generate deprecations list from the language repo (#2253) This updates the Deprecation enum to be generated from spec/deprecations.yaml in the language repo. --- .pubignore | 2 +- CHANGELOG.md | 8 +++ lib/src/deprecation.dart | 63 ++++++++++++-------- lib/src/parse/stylesheet.dart | 4 +- pkg/sass_api/CHANGELOG.md | 4 ++ pkg/sass_api/pubspec.yaml | 4 +- pubspec.yaml | 2 +- test/double_check_test.dart | 30 +++++++--- tool/grind.dart | 30 +++------- tool/grind/double_check.dart | 5 +- tool/grind/generate_deprecations.dart | 82 +++++++++++++++++++++++++++ tool/grind/utils.dart | 17 ++++++ 12 files changed, 189 insertions(+), 62 deletions(-) create mode 100644 tool/grind/generate_deprecations.dart diff --git a/.pubignore b/.pubignore index 2fbab300a..08992ce75 100644 --- a/.pubignore +++ b/.pubignore @@ -1,5 +1,5 @@ # This should be identical to .gitignore except that it doesn't exclude -# generated protobuf files. +# generated Dart files. .buildlog .DS_Store diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d64f38c1..d06b712fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +## 1.77.3 + +### Dart API + +* `Deprecation.duplicateVariableFlags` has been deprecated and replaced with + `Deprecation.duplicateVarFlags` to make it consistent with the + `duplicate-var-flags` name used on the command line and in the JS API. + ## 1.77.2 * Don't emit deprecation warnings for functions and mixins beginning with `__`. diff --git a/lib/src/deprecation.dart b/lib/src/deprecation.dart index b13180e10..cec976714 100644 --- a/lib/src/deprecation.dart +++ b/lib/src/deprecation.dart @@ -1,4 +1,4 @@ -// Copyright 2022 Google LLC. Use of this source code is governed by an +// Copyright 2024 Google LLC. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. @@ -10,34 +10,42 @@ import 'util/nullable.dart'; /// A deprecated feature in the language. enum Deprecation { - /// Deprecation for passing a string to `call` instead of `get-function`. + // START AUTOGENERATED CODE + // + // DO NOT EDIT. This section was generated from the language repo. + // See tool/grind/generate_deprecations.dart for details. + // + // Checksum: 22d9bdbe92eb39b3c0d6d64ebe1879a431c0037e + + /// Deprecation for passing a string directly to meta.call(). callString('call-string', deprecatedIn: '0.0.0', description: 'Passing a string directly to meta.call().'), - /// Deprecation for `@elseif`. + /// Deprecation for @elseif. elseif('elseif', deprecatedIn: '1.3.2', description: '@elseif.'), - /// Deprecation for parsing `@-moz-document`. + /// Deprecation for @-moz-document. mozDocument('moz-document', deprecatedIn: '1.7.2', description: '@-moz-document.'), - /// Deprecation for importers using relative canonical URLs. - relativeCanonical('relative-canonical', deprecatedIn: '1.14.2'), + /// Deprecation for imports using relative canonical URLs. + relativeCanonical('relative-canonical', + deprecatedIn: '1.14.2', + description: 'Imports using relative canonical URLs.'), - /// Deprecation for declaring new variables with `!global`. + /// Deprecation for declaring new variables with !global. newGlobal('new-global', deprecatedIn: '1.17.2', description: 'Declaring new variables with !global.'), - /// Deprecation for certain functions in the color module matching the - /// behavior of their global counterparts for compatiblity reasons. + /// Deprecation for using color module functions in place of plain CSS functions. colorModuleCompat('color-module-compat', deprecatedIn: '1.23.0', description: 'Using color module functions in place of plain CSS functions.'), - /// Deprecation for treating `/` as division. + /// Deprecation for / operator for division. slashDiv('slash-div', deprecatedIn: '1.33.0', description: '/ operator for division.'), @@ -46,46 +54,55 @@ enum Deprecation { deprecatedIn: '1.54.0', description: 'Leading, trailing, and repeated combinators.'), - /// Deprecation for ambiguous `+` and `-` operators. + /// Deprecation for ambiguous + and - operators. strictUnary('strict-unary', deprecatedIn: '1.55.0', description: 'Ambiguous + and - operators.'), - /// Deprecation for passing invalid units to certain built-in functions. + /// Deprecation for passing invalid units to built-in functions. functionUnits('function-units', deprecatedIn: '1.56.0', description: 'Passing invalid units to built-in functions.'), - /// Deprecation for passing percentages to the Sass abs() function. - absPercent('abs-percent', - deprecatedIn: '1.65.0', - description: 'Passing percentages to the Sass abs() function.'), - - duplicateVariableFlags('duplicate-var-flags', + /// Deprecation for using !default or !global multiple times for one variable. + duplicateVarFlags('duplicate-var-flags', deprecatedIn: '1.62.0', description: 'Using !default or !global multiple times for one variable.'), + /// Deprecation for passing null as alpha in the ${isJS ? 'JS': 'Dart'} API. nullAlpha('null-alpha', deprecatedIn: '1.62.3', description: 'Passing null as alpha in the ${isJS ? 'JS' : 'Dart'} API.'), + /// Deprecation for passing percentages to the Sass abs() function. + absPercent('abs-percent', + deprecatedIn: '1.65.0', + description: 'Passing percentages to the Sass abs() function.'), + + /// Deprecation for using the current working directory as an implicit load path. fsImporterCwd('fs-importer-cwd', deprecatedIn: '1.73.0', description: 'Using the current working directory as an implicit load path.'), + /// Deprecation for function and mixin names beginning with --. cssFunctionMixin('css-function-mixin', deprecatedIn: '1.76.0', description: 'Function and mixin names beginning with --.'), - @Deprecated('This deprecation name was never actually used.') - calcInterp('calc-interp', deprecatedIn: null), - - /// Deprecation for `@import` rules. + /// Deprecation for @import rules. import.future('import', description: '@import rules.'), + // END AUTOGENERATED CODE + /// Used for deprecations coming from user-authored code. - userAuthored('user-authored', deprecatedIn: null); + userAuthored('user-authored', deprecatedIn: null), + + @Deprecated('This deprecation name was never actually used.') + calcInterp('calc-interp', deprecatedIn: null); + + @Deprecated('Use duplicateVarFlags instead.') + static const duplicateVariableFlags = duplicateVarFlags; /// A unique ID for this deprecation in kebab case. /// diff --git a/lib/src/parse/stylesheet.dart b/lib/src/parse/stylesheet.dart index e72e2527b..e372e3912 100644 --- a/lib/src/parse/stylesheet.dart +++ b/lib/src/parse/stylesheet.dart @@ -235,7 +235,7 @@ abstract class StylesheetParser extends Parser { case 'default': if (guarded) { logger.warnForDeprecation( - Deprecation.duplicateVariableFlags, + Deprecation.duplicateVarFlags, '!default should only be written once for each variable.\n' 'This will be an error in Dart Sass 2.0.0.', span: scanner.spanFrom(flagStart)); @@ -248,7 +248,7 @@ abstract class StylesheetParser extends Parser { scanner.spanFrom(flagStart)); } else if (global) { logger.warnForDeprecation( - Deprecation.duplicateVariableFlags, + Deprecation.duplicateVarFlags, '!global should only be written once for each variable.\n' 'This will be an error in Dart Sass 2.0.0.', span: scanner.spanFrom(flagStart)); diff --git a/pkg/sass_api/CHANGELOG.md b/pkg/sass_api/CHANGELOG.md index c0839df75..e82b07c57 100644 --- a/pkg/sass_api/CHANGELOG.md +++ b/pkg/sass_api/CHANGELOG.md @@ -1,3 +1,7 @@ +## 10.4.3 + +* No user-visible changes. + ## 10.4.2 * No user-visible changes. diff --git a/pkg/sass_api/pubspec.yaml b/pkg/sass_api/pubspec.yaml index e11f303a5..0943de732 100644 --- a/pkg/sass_api/pubspec.yaml +++ b/pkg/sass_api/pubspec.yaml @@ -2,7 +2,7 @@ name: sass_api # Note: Every time we add a new Sass AST node, we need to bump the *major* # version because it's a breaking change for anyone who's implementing the # visitor interface(s). -version: 10.4.2 +version: 10.4.3 description: Additional APIs for Dart Sass. homepage: https://github.com/sass/dart-sass @@ -10,7 +10,7 @@ environment: sdk: ">=3.0.0 <4.0.0" dependencies: - sass: 1.77.2 + sass: 1.77.3 dev_dependencies: dartdoc: ">=6.0.0 <9.0.0" diff --git a/pubspec.yaml b/pubspec.yaml index c4acb1234..5260eba17 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass -version: 1.77.2 +version: 1.77.3 description: A Sass implementation in Dart. homepage: https://github.com/sass/dart-sass diff --git a/test/double_check_test.dart b/test/double_check_test.dart index 1da209885..44def85ae 100644 --- a/test/double_check_test.dart +++ b/test/double_check_test.dart @@ -7,25 +7,39 @@ import 'dart:io'; import 'dart:convert'; +import 'package:crypto/crypto.dart'; import 'package:path/path.dart' as p; import 'package:pub_semver/pub_semver.dart'; import 'package:pubspec_parse/pubspec_parse.dart'; import 'package:test/test.dart'; +import '../tool/grind/generate_deprecations.dart' as deprecations; import '../tool/grind/synchronize.dart' as synchronize; /// Tests that double-check that everything in the repo looks sensible. void main() { - group("synchronized file is up-to-date:", () { - synchronize.sources.forEach((sourcePath, targetPath) { - test(targetPath, () { - if (File(targetPath).readAsStringSync() != - synchronize.synchronizeFile(sourcePath)) { - fail("$targetPath is out-of-date.\n" - "Run `dart pub run grinder` to update it."); - } + group("up-to-date generated", () { + group("synchronized file:", () { + synchronize.sources.forEach((sourcePath, targetPath) { + test(targetPath, () { + if (File(targetPath).readAsStringSync() != + synchronize.synchronizeFile(sourcePath)) { + fail("$targetPath is out-of-date.\n" + "Run `dart run grinder` to update it."); + } + }); }); }); + + test("deprecations", () { + var inputText = File(deprecations.yamlPath).readAsStringSync(); + var outputText = File(deprecations.dartPath).readAsStringSync(); + var checksum = sha1.convert(utf8.encode(inputText)); + if (!outputText.contains('// Checksum: $checksum')) { + fail('${deprecations.dartPath} is out-of-date.\n' + 'Run `dart run grinder` to update it.'); + } + }); }, // Windows sees different bytes than other OSes, possibly because of // newline normalization issues. diff --git a/tool/grind.dart b/tool/grind.dart index 631bdebdf..7af45b438 100644 --- a/tool/grind.dart +++ b/tool/grind.dart @@ -11,6 +11,7 @@ import 'package:grinder/grinder.dart'; import 'package:path/path.dart' as p; import 'package:source_span/source_span.dart'; +import 'grind/generate_deprecations.dart'; import 'grind/synchronize.dart'; import 'grind/utils.dart'; @@ -18,8 +19,10 @@ export 'grind/bazel.dart'; export 'grind/benchmark.dart'; export 'grind/double_check.dart'; export 'grind/frameworks.dart'; +export 'grind/generate_deprecations.dart'; export 'grind/subpackages.dart'; export 'grind/synchronize.dart'; +export 'grind/utils.dart'; void main(List args) { pkg.humanName.value = "Dart Sass"; @@ -127,7 +130,7 @@ void main(List args) { } @DefaultTask('Compile async code and reformat.') -@Depends(format, synchronize) +@Depends(format, synchronize, deprecations) void all() {} @Task('Run the Dart formatter.') @@ -140,7 +143,7 @@ void npmInstall() => run(Platform.isWindows ? "npm.cmd" : "npm", arguments: ["install"]); @Task('Runs the tasks that are required for running tests.') -@Depends(format, synchronize, protobuf, "pkg-npm-dev", npmInstall, +@Depends(format, synchronize, protobuf, deprecations, "pkg-npm-dev", npmInstall, "pkg-standalone-dev") void beforeTest() {} @@ -213,9 +216,9 @@ String _readAndResolveMarkdown(String path) => File(path) /// Returns a map from JS type declaration file names to their contnets. Map _fetchJSTypes() { - var languageRepo = _updateLanguageRepo(); + updateLanguageRepo(); - var typeRoot = p.join(languageRepo, 'js-api-doc'); + var typeRoot = p.join('build/language', 'js-api-doc'); return { for (var entry in Directory(typeRoot).listSync(recursive: true)) if (entry is File && entry.path.endsWith('.d.ts')) @@ -231,6 +234,7 @@ void _matchError(Match match, String message, {Object? url}) { } @Task('Compile the protocol buffer definition to a Dart library.') +@Depends(updateLanguageRepo) Future protobuf() async { Directory('build').createSync(recursive: true); @@ -250,8 +254,6 @@ dart run protoc_plugin "\$@" run('chmod', arguments: ['a+x', 'build/protoc-gen-dart']); } - _updateLanguageRepo(); - await runAsync("buf", arguments: ["generate"], runOptions: RunOptions(environment: { @@ -321,19 +323,3 @@ String _updateHomebrewLanguageRevision(String formula) { match.group(0)!.replaceFirst(match.group(1)!, languageRepoRevision) + formula.substring(match.end); } - -/// Clones the main branch of `github.com/sass/sass` and returns the path to the -/// clone. -/// -/// If the `UPDATE_SASS_SASS_REPO` environment variable is `false`, this instead -/// assumes the repo that already exists at `build/language/sass`. -/// `UPDATE_SASS_PROTOCOL` is also checked as a deprecated alias for -/// `UPDATE_SASS_SASS_REPO`. -String _updateLanguageRepo() => - // UPDATE_SASS_PROTOCOL is considered deprecated, because it doesn't apply as - // generically to other tasks. - Platform.environment['UPDATE_SASS_SASS_REPO'] != 'false' && - Platform.environment['UPDATE_SASS_PROTOCOL'] != 'false' - ? cloneOrCheckout("https://github.com/sass/sass.git", "main", - name: 'language') - : 'build/language'; diff --git a/tool/grind/double_check.dart b/tool/grind/double_check.dart index 4ec0cd8e7..8b1dca18f 100644 --- a/tool/grind/double_check.dart +++ b/tool/grind/double_check.dart @@ -5,13 +5,12 @@ import 'dart:io'; import 'package:cli_pkg/cli_pkg.dart' as pkg; +import 'package:collection/collection.dart'; import 'package:grinder/grinder.dart'; import 'package:path/path.dart' as p; import 'package:pub_api_client/pub_api_client.dart'; import 'package:pubspec_parse/pubspec_parse.dart'; -import 'package:sass/src/utils.dart'; - import 'utils.dart'; @Task('Verify that the package is in a good state to release.') @@ -21,7 +20,7 @@ Future doubleCheckBeforeRelease() async { fail("GITHUB_REF $ref is different than pubspec version ${pkg.version}."); } - if (listEquals(pkg.version.preRelease, ["dev"])) { + if (const ListEquality().equals(pkg.version.preRelease, ["dev"])) { fail("${pkg.version} is a dev release."); } diff --git a/tool/grind/generate_deprecations.dart b/tool/grind/generate_deprecations.dart new file mode 100644 index 000000000..21ce6f58e --- /dev/null +++ b/tool/grind/generate_deprecations.dart @@ -0,0 +1,82 @@ +// Copyright 2024 Google LLC. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +import 'dart:convert'; +import 'dart:io'; + +import 'package:crypto/crypto.dart'; +import 'package:dart_style/dart_style.dart'; +import 'package:grinder/grinder.dart'; +import 'package:yaml/yaml.dart'; + +import 'utils.dart'; + +const yamlPath = 'build/language/spec/deprecations.yaml'; +const dartPath = 'lib/src/deprecation.dart'; + +final _blockRegex = + RegExp(r'// START AUTOGENERATED CODE[\s\S]*?// END AUTOGENERATED CODE'); + +@Task('Generate deprecation.g.dart from the list in the language repo.') +@Depends(updateLanguageRepo) +void deprecations() { + var yamlFile = File(yamlPath); + var dartFile = File(dartPath); + var yamlText = yamlFile.readAsStringSync(); + var data = loadYaml(yamlText, sourceUrl: yamlFile.uri) as Map; + var dartText = dartFile.readAsStringSync(); + var buffer = StringBuffer('''// START AUTOGENERATED CODE + // + // DO NOT EDIT. This section was generated from the language repo. + // See tool/grind/generate_deprecations.dart for details. + // + // Checksum: ${sha1.convert(utf8.encode(yamlText))} + +'''); + for (var MapEntry(:String key, :value) in data.entries) { + var camelCase = key.replaceAllMapped( + RegExp(r'-(.)'), (match) => match.group(1)!.toUpperCase()); + var (description, deprecatedIn, obsoleteIn) = switch (value) { + { + 'description': String description, + 'dart-sass': {'status': 'future'}, + } => + (description, null, null), + { + 'description': String description, + 'dart-sass': {'status': 'active', 'deprecated': String deprecatedIn}, + } => + (description, deprecatedIn, null), + { + 'description': String description, + 'dart-sass': { + 'status': 'obsolete', + 'deprecated': String deprecatedIn, + 'obsolete': String obsoleteIn + }, + } => + (description, deprecatedIn, obsoleteIn), + _ => throw Exception('Invalid deprecation $key: $value') + }; + description = + description.replaceAll(r'$PLATFORM', r"${isJS ? 'JS': 'Dart'}"); + var constructorName = deprecatedIn == null ? '.future' : ''; + var deprecatedClause = + deprecatedIn == null ? '' : "deprecatedIn: '$deprecatedIn', "; + var obsoleteClause = + obsoleteIn == null ? '' : "obsoleteIn: '$obsoleteIn', "; + var comment = 'Deprecation for ${description.substring(0, 1).toLowerCase()}' + '${description.substring(1)}'; + buffer.writeln('/// $comment'); + buffer.writeln( + "$camelCase$constructorName('$key', $deprecatedClause$obsoleteClause" + "description: '$description'),"); + } + buffer.write('\n // END AUTOGENERATED CODE'); + if (!dartText.contains(_blockRegex)) { + fail("Couldn't find block for generated code in lib/src/deprecation.dart"); + } + var newCode = dartText.replaceFirst(_blockRegex, buffer.toString()); + dartFile.writeAsStringSync(DartFormatter().format(newCode)); +} diff --git a/tool/grind/utils.dart b/tool/grind/utils.dart index 21d9f66c8..86ea7ed96 100644 --- a/tool/grind/utils.dart +++ b/tool/grind/utils.dart @@ -100,3 +100,20 @@ void afterTask(String taskName, FutureOr callback()) { await callback(); }); } + +/// Clones the main branch of `github.com/sass/sass`. +/// +/// If the `UPDATE_SASS_SASS_REPO` environment variable is `false`, this instead +/// assumes the repo that already exists at `build/language/sass`. +/// `UPDATE_SASS_PROTOCOL` is also checked as a deprecated alias for +/// `UPDATE_SASS_SASS_REPO`. +@Task('Clones the main branch of `github.com/sass/sass` if necessary.') +void updateLanguageRepo() { + // UPDATE_SASS_PROTOCOL is considered deprecated, because it doesn't apply as + // generically to other tasks. + if (Platform.environment['UPDATE_SASS_SASS_REPO'] != 'false' && + Platform.environment['UPDATE_SASS_PROTOCOL'] != 'false') { + cloneOrCheckout("https://github.com/sass/sass.git", "main", + name: 'language'); + } +}