From 58cbab42e63500f4269bc3f5ed01f7ee311cc4d3 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Mon, 21 Aug 2023 16:47:18 -0700 Subject: [PATCH 01/30] Stop working around dart-lang/linter#4381 (#2071) --- lib/src/ast/sass/expression/binary_operation.dart | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/src/ast/sass/expression/binary_operation.dart b/lib/src/ast/sass/expression/binary_operation.dart index dfaf87d16..4e87fe8de 100644 --- a/lib/src/ast/sass/expression/binary_operation.dart +++ b/lib/src/ast/sass/expression/binary_operation.dart @@ -82,10 +82,7 @@ final class BinaryOperationExpression implements Expression { var right = this.right; // Hack to make analysis work. var rightNeedsParens = switch (right) { BinaryOperationExpression(:var operator) => - // dart-lang/linter#4381 - // ignore: unnecessary_this operator.precedence <= this.operator.precedence && - // ignore: unnecessary_this !(operator == this.operator && operator.isAssociative), ListExpression(hasBrackets: false, contents: [_, _, ...]) => true, _ => false From af0118ad6409bbe54bc5d0f8347458ea68703946 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Fri, 1 Sep 2023 16:09:06 -0700 Subject: [PATCH 02/30] Improve `sass --embedded` performance (#2013) Co-authored-by: Natalie Weizenbaum --- CHANGELOG.md | 8 + lib/src/embedded/dispatcher.dart | 221 +++++++++--------- lib/src/embedded/host_callable.dart | 6 +- lib/src/embedded/importer/file.dart | 48 ++-- lib/src/embedded/importer/host.dart | 64 +++-- lib/src/embedded/isolate_dispatcher.dart | 153 ++++-------- lib/src/embedded/reusable_isolate.dart | 142 +++++++++++ .../util/explicit_close_transformer.dart | 38 --- pubspec.yaml | 3 +- 9 files changed, 363 insertions(+), 320 deletions(-) create mode 100644 lib/src/embedded/reusable_isolate.dart delete mode 100644 lib/src/embedded/util/explicit_close_transformer.dart diff --git a/CHANGELOG.md b/CHANGELOG.md index 49739cdc2..1666f753f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +## 1.66.2 + +### Embedded Sass + +* Substantially improve the embedded compiler's performance when compiling many + files or files that require many importer or function call round-trips with + the embedded host. + ## 1.66.1 ### JS API diff --git a/lib/src/embedded/dispatcher.dart b/lib/src/embedded/dispatcher.dart index fae22b458..22df57fca 100644 --- a/lib/src/embedded/dispatcher.dart +++ b/lib/src/embedded/dispatcher.dart @@ -2,15 +2,15 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. -import 'dart:async'; import 'dart:convert'; import 'dart:io'; +import 'dart:isolate'; import 'dart:typed_data'; +import 'package:native_synchronization/mailbox.dart'; import 'package:path/path.dart' as p; import 'package:protobuf/protobuf.dart'; import 'package:sass/sass.dart' as sass; -import 'package:stream_channel/stream_channel.dart'; import 'embedded_sass.pb.dart'; import 'function_registry.dart'; @@ -30,83 +30,69 @@ final _outboundRequestId = 0; /// A class that dispatches messages to and from the host for a single /// compilation. final class Dispatcher { - /// The channel of encoded protocol buffers, connected to the host. - final StreamChannel _channel; + /// The mailbox for receiving messages from the host. + final Mailbox _mailbox; + + /// The send port for sending messages to the host. + final SendPort _sendPort; /// The compilation ID for which this dispatcher is running. /// - /// This is added to outgoing messages but is _not_ parsed from incoming - /// messages, since that's already handled by the [IsolateDispatcher]. - final int _compilationId; + /// This is used in error messages. + late int _compilationId; /// [_compilationId], serialized as a varint. - final Uint8List _compilationIdVarint; - - /// Whether this dispatcher has received its compile request. - var _compiling = false; + /// + /// This is used in outgoing messages. + late Uint8List _compilationIdVarint; - /// A completer awaiting a response to an outbound request. + /// Whether we detected a [ProtocolError] while parsing an incoming response. /// - /// Since each [Dispatcher] is only running a single-threaded compilation, it - /// can only ever have one request outstanding. - Completer? _outstandingRequest; + /// If we have, we don't want to send the final compilation result because + /// it'll just be a wrapper around the error. + var _requestError = false; - /// Creates a [Dispatcher] that sends and receives encoded protocol buffers - /// over [channel]. - Dispatcher(this._channel, this._compilationId) - : _compilationIdVarint = serializeVarint(_compilationId); + /// Creates a [Dispatcher] that receives encoded protocol buffers through + /// [_mailbox] and sends them through [_sendPort]. + Dispatcher(this._mailbox, this._sendPort); /// Listens for incoming `CompileRequests` and runs their compilations. - /// - /// This may only be called once. Returns whether or not the compilation - /// succeeded. - Future listen() async { - var success = false; - await _channel.stream.listen((binaryMessage) async { - // Wait a single microtask tick so that we're running in a separate - // microtask from the initial request dispatch. Otherwise, [waitFor] will - // deadlock the event loop fiber that would otherwise be checking stdin - // for new input. - await Future.value(); + void listen() { + do { + var packet = _mailbox.take(); + if (packet.isEmpty) break; try { - InboundMessage? message; + var (compilationId, messageBuffer) = parsePacket(packet); + + _compilationId = compilationId; + _compilationIdVarint = serializeVarint(compilationId); + + InboundMessage message; try { - message = InboundMessage.fromBuffer(binaryMessage); + message = InboundMessage.fromBuffer(messageBuffer); } on InvalidProtocolBufferException catch (error) { throw parseError(error.message); } switch (message.whichMessage()) { - case InboundMessage_Message.versionRequest: - throw paramsError("VersionRequest must have compilation ID 0."); - case InboundMessage_Message.compileRequest: - if (_compiling) { - throw paramsError( - "A CompileRequest with compilation ID $_compilationId is " - "already active."); - } - _compiling = true; - var request = message.compileRequest; - var response = await _compile(request); - _send(OutboundMessage()..compileResponse = response); - success = true; - // Each Dispatcher runs a single compilation and then closes. - _channel.sink.close(); - - case InboundMessage_Message.canonicalizeResponse: - _dispatchResponse(message.id, message.canonicalizeResponse); - - case InboundMessage_Message.importResponse: - _dispatchResponse(message.id, message.importResponse); + var response = _compile(request); + if (!_requestError) { + _send(OutboundMessage()..compileResponse = response); + } - case InboundMessage_Message.fileImportResponse: - _dispatchResponse(message.id, message.fileImportResponse); + case InboundMessage_Message.versionRequest: + throw paramsError("VersionRequest must have compilation ID 0."); - case InboundMessage_Message.functionCallResponse: - _dispatchResponse(message.id, message.functionCallResponse); + case InboundMessage_Message.canonicalizeResponse || + InboundMessage_Message.importResponse || + InboundMessage_Message.fileImportResponse || + InboundMessage_Message.functionCallResponse: + throw paramsError( + "Response ID ${message.id} doesn't match any outstanding requests" + " in compilation $_compilationId."); case InboundMessage_Message.notSet: throw parseError("InboundMessage.message is not set."); @@ -115,16 +101,14 @@ final class Dispatcher { throw parseError( "Unknown message type: ${message.toDebugString()}"); } - } on ProtocolError catch (error, stackTrace) { - sendError(handleError(error, stackTrace)); - _channel.sink.close(); + } catch (error, stackTrace) { + _handleError(error, stackTrace); } - }).asFuture(); - return success; + } while (!_requestError); } - Future _compile( - InboundMessage_CompileRequest request) async { + OutboundMessage_CompileResponse _compile( + InboundMessage_CompileRequest request) { var functions = FunctionRegistry(); var style = request.style == OutputStyle.COMPRESSED @@ -159,7 +143,6 @@ final class Dispatcher { verbose: request.verbose, sourceMap: request.sourceMap, charset: request.charset); - break; case InboundMessage_CompileRequest_Input.path: if (request.path.isEmpty) { @@ -188,7 +171,6 @@ final class Dispatcher { ..end = SourceSpan_SourceLocation() ..url = p.toUri(request.path).toString())); } - break; case InboundMessage_CompileRequest_Input.notSet: throw mandatoryError("CompileRequest.input"); @@ -245,59 +227,86 @@ final class Dispatcher { void sendError(ProtocolError error) => _send(OutboundMessage()..error = error); - Future sendCanonicalizeRequest( + InboundMessage_CanonicalizeResponse sendCanonicalizeRequest( OutboundMessage_CanonicalizeRequest request) => _sendRequest( OutboundMessage()..canonicalizeRequest = request); - Future sendImportRequest( + InboundMessage_ImportResponse sendImportRequest( OutboundMessage_ImportRequest request) => _sendRequest( OutboundMessage()..importRequest = request); - Future sendFileImportRequest( + InboundMessage_FileImportResponse sendFileImportRequest( OutboundMessage_FileImportRequest request) => _sendRequest( OutboundMessage()..fileImportRequest = request); - Future sendFunctionCallRequest( + InboundMessage_FunctionCallResponse sendFunctionCallRequest( OutboundMessage_FunctionCallRequest request) => _sendRequest( OutboundMessage()..functionCallRequest = request); /// Sends [request] to the host and returns the message sent in response. - Future _sendRequest( - OutboundMessage request) async { - request.id = _outboundRequestId; - _send(request); - - if (_outstandingRequest != null) { - throw StateError( - "Dispatcher.sendRequest() can't be called when another request is " - "active."); - } + T _sendRequest(OutboundMessage message) { + message.id = _outboundRequestId; + _send(message); - return (_outstandingRequest = Completer()).future; - } + var packet = _mailbox.take(); - /// Dispatches [response] to the appropriate outstanding request. - /// - /// Throws an error if there's no outstanding request with the given [id] or - /// if that request is expecting a different type of response. - void _dispatchResponse(int? id, T response) { - var completer = _outstandingRequest; - _outstandingRequest = null; - if (completer == null || id != _outboundRequestId) { - throw paramsError( - "Response ID $id doesn't match any outstanding requests in " - "compilation $_compilationId."); - } else if (completer is! Completer) { - throw paramsError( - "Request ID $id doesn't match response type ${response.runtimeType} " - "in compilation $_compilationId."); + try { + var messageBuffer = + Uint8List.sublistView(packet, _compilationIdVarint.length); + + InboundMessage message; + try { + message = InboundMessage.fromBuffer(messageBuffer); + } on InvalidProtocolBufferException catch (error) { + throw parseError(error.message); + } + + var response = switch (message.whichMessage()) { + InboundMessage_Message.canonicalizeResponse => + message.canonicalizeResponse, + InboundMessage_Message.importResponse => message.importResponse, + InboundMessage_Message.fileImportResponse => message.fileImportResponse, + InboundMessage_Message.functionCallResponse => + message.functionCallResponse, + InboundMessage_Message.compileRequest => throw paramsError( + "A CompileRequest with compilation ID $_compilationId is already " + "active."), + InboundMessage_Message.versionRequest => + throw paramsError("VersionRequest must have compilation ID 0."), + InboundMessage_Message.notSet => + throw parseError("InboundMessage.message is not set."), + _ => + throw parseError("Unknown message type: ${message.toDebugString()}") + }; + + if (message.id != _outboundRequestId) { + throw paramsError( + "Response ID ${message.id} doesn't match any outstanding requests " + "in compilation $_compilationId."); + } else if (response is! T) { + throw paramsError( + "Request ID $_outboundRequestId doesn't match response type " + "${response.runtimeType} in compilation $_compilationId."); + } + + return response; + } catch (error, stackTrace) { + _handleError(error, stackTrace); + _requestError = true; + rethrow; } + } - completer.complete(response); + /// Handles an error thrown by the dispatcher or code it dispatches to. + /// + /// The [messageId] indicate the IDs of the message being responded to, if + /// available. + void _handleError(Object error, StackTrace stackTrace, {int? messageId}) { + sendError(handleError(error, stackTrace, messageId: messageId)); } /// Sends [message] to the host with the given [wireId]. @@ -306,16 +315,18 @@ final class Dispatcher { message.writeToCodedBufferWriter(protobufWriter); // Add one additional byte to the beginning to indicate whether or not the - // compilation is finished, so the [IsolateDispatcher] knows whether to - // treat this isolate as inactive. + // compilation has finished (1) or encountered a fatal error (2), so the + // [IsolateDispatcher] knows whether to treat this isolate as inactive or + // close out entirely. var packet = Uint8List( 1 + _compilationIdVarint.length + protobufWriter.lengthInBytes); - packet[0] = - message.whichMessage() == OutboundMessage_Message.compileResponse - ? 1 - : 0; + packet[0] = switch (message.whichMessage()) { + OutboundMessage_Message.compileResponse => 1, + OutboundMessage_Message.error => 2, + _ => 0 + }; packet.setAll(1, _compilationIdVarint); protobufWriter.writeTo(packet, 1 + _compilationIdVarint.length); - _channel.sink.add(packet); + _sendPort.send(packet); } } diff --git a/lib/src/embedded/host_callable.dart b/lib/src/embedded/host_callable.dart index bb1770ea4..448cce217 100644 --- a/lib/src/embedded/host_callable.dart +++ b/lib/src/embedded/host_callable.dart @@ -2,9 +2,6 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. -// ignore: deprecated_member_use -import 'dart:cli'; - import '../callable.dart'; import '../exception.dart'; import 'dispatcher.dart'; @@ -37,8 +34,7 @@ Callable hostCallable( request.name = callable.name; } - // ignore: deprecated_member_use - var response = waitFor(dispatcher.sendFunctionCallRequest(request)); + var response = dispatcher.sendFunctionCallRequest(request); try { switch (response.whichResult()) { case InboundMessage_FunctionCallResponse_Result.success: diff --git a/lib/src/embedded/importer/file.dart b/lib/src/embedded/importer/file.dart index b945cba2e..8250515eb 100644 --- a/lib/src/embedded/importer/file.dart +++ b/lib/src/embedded/importer/file.dart @@ -2,9 +2,6 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. -// ignore: deprecated_member_use -import 'dart:cli'; - import '../../importer.dart'; import '../dispatcher.dart'; import '../embedded_sass.pb.dart' hide SourceSpan; @@ -27,30 +24,27 @@ final class FileImporter extends ImporterBase { Uri? canonicalize(Uri url) { if (url.scheme == 'file') return _filesystemImporter.canonicalize(url); - // ignore: deprecated_member_use - return waitFor(() async { - var response = await dispatcher - .sendFileImportRequest(OutboundMessage_FileImportRequest() - ..importerId = _importerId - ..url = url.toString() - ..fromImport = fromImport); - - switch (response.whichResult()) { - case InboundMessage_FileImportResponse_Result.fileUrl: - var url = parseAbsoluteUrl("The file importer", response.fileUrl); - if (url.scheme != 'file') { - throw 'The file importer must return a file: URL, was "$url"'; - } - - return _filesystemImporter.canonicalize(url); - - case InboundMessage_FileImportResponse_Result.error: - throw response.error; - - case InboundMessage_FileImportResponse_Result.notSet: - return null; - } - }()); + var response = + dispatcher.sendFileImportRequest(OutboundMessage_FileImportRequest() + ..importerId = _importerId + ..url = url.toString() + ..fromImport = fromImport); + + switch (response.whichResult()) { + case InboundMessage_FileImportResponse_Result.fileUrl: + var url = parseAbsoluteUrl("The file importer", response.fileUrl); + if (url.scheme != 'file') { + throw 'The file importer must return a file: URL, was "$url"'; + } + + return _filesystemImporter.canonicalize(url); + + case InboundMessage_FileImportResponse_Result.error: + throw response.error; + + case InboundMessage_FileImportResponse_Result.notSet: + return null; + } } ImporterResult? load(Uri url) => _filesystemImporter.load(url); diff --git a/lib/src/embedded/importer/host.dart b/lib/src/embedded/importer/host.dart index e4a952100..a1d5eed31 100644 --- a/lib/src/embedded/importer/host.dart +++ b/lib/src/embedded/importer/host.dart @@ -2,9 +2,6 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. -// ignore: deprecated_member_use -import 'dart:cli'; - import '../../importer.dart'; import '../dispatcher.dart'; import '../embedded_sass.pb.dart' hide SourceSpan; @@ -19,44 +16,35 @@ final class HostImporter extends ImporterBase { HostImporter(Dispatcher dispatcher, this._importerId) : super(dispatcher); Uri? canonicalize(Uri url) { - // ignore: deprecated_member_use - return waitFor(() async { - var response = await dispatcher - .sendCanonicalizeRequest(OutboundMessage_CanonicalizeRequest() - ..importerId = _importerId - ..url = url.toString() - ..fromImport = fromImport); - - return switch (response.whichResult()) { - InboundMessage_CanonicalizeResponse_Result.url => - parseAbsoluteUrl("The importer", response.url), - InboundMessage_CanonicalizeResponse_Result.error => - throw response.error, - InboundMessage_CanonicalizeResponse_Result.notSet => null - }; - }()); + var response = + dispatcher.sendCanonicalizeRequest(OutboundMessage_CanonicalizeRequest() + ..importerId = _importerId + ..url = url.toString() + ..fromImport = fromImport); + + return switch (response.whichResult()) { + InboundMessage_CanonicalizeResponse_Result.url => + parseAbsoluteUrl("The importer", response.url), + InboundMessage_CanonicalizeResponse_Result.error => throw response.error, + InboundMessage_CanonicalizeResponse_Result.notSet => null + }; } ImporterResult? load(Uri url) { - // ignore: deprecated_member_use - return waitFor(() async { - var response = - await dispatcher.sendImportRequest(OutboundMessage_ImportRequest() - ..importerId = _importerId - ..url = url.toString()); - - return switch (response.whichResult()) { - InboundMessage_ImportResponse_Result.success => ImporterResult( - response.success.contents, - sourceMapUrl: response.success.sourceMapUrl.isEmpty - ? null - : parseAbsoluteUrl( - "The importer", response.success.sourceMapUrl), - syntax: syntaxToSyntax(response.success.syntax)), - InboundMessage_ImportResponse_Result.error => throw response.error, - InboundMessage_ImportResponse_Result.notSet => null - }; - }()); + var response = dispatcher.sendImportRequest(OutboundMessage_ImportRequest() + ..importerId = _importerId + ..url = url.toString()); + + return switch (response.whichResult()) { + InboundMessage_ImportResponse_Result.success => ImporterResult( + response.success.contents, + sourceMapUrl: response.success.sourceMapUrl.isEmpty + ? null + : parseAbsoluteUrl("The importer", response.success.sourceMapUrl), + syntax: syntaxToSyntax(response.success.syntax)), + InboundMessage_ImportResponse_Result.error => throw response.error, + InboundMessage_ImportResponse_Result.notSet => null + }; } String toString() => "HostImporter"; diff --git a/lib/src/embedded/isolate_dispatcher.dart b/lib/src/embedded/isolate_dispatcher.dart index 78d340997..044e2b9c2 100644 --- a/lib/src/embedded/isolate_dispatcher.dart +++ b/lib/src/embedded/isolate_dispatcher.dart @@ -7,45 +7,33 @@ import 'dart:ffi'; import 'dart:isolate'; import 'dart:typed_data'; +import 'package:native_synchronization/mailbox.dart'; import 'package:pool/pool.dart'; import 'package:protobuf/protobuf.dart'; -import 'package:stream_channel/isolate_channel.dart'; import 'package:stream_channel/stream_channel.dart'; import 'dispatcher.dart'; import 'embedded_sass.pb.dart'; -import 'util/explicit_close_transformer.dart'; +import 'reusable_isolate.dart'; import 'util/proto_extensions.dart'; import 'utils.dart'; -/// The message sent to a previously-inactive isolate to initiate a new -/// compilation session. -/// -/// The [SendPort] is used to establish a dedicated [IsolateChannel] for this -/// compilation and the [int] is the compilation ID to use on the wire. -/// -/// We apply the compilation ID in the isolate for efficiency reasons: it allows -/// us to write the protobuf directly to the same buffer as the wire ID, which -/// saves a copy for each message. -typedef _InitialMessage = (SendPort, int); - /// A class that dispatches messages between the host and various isolates that /// are each running an individual compilation. class IsolateDispatcher { /// The channel of encoded protocol buffers, connected to the host. final StreamChannel _channel; - /// A map from compilation IDs to the sinks for isolates running those - /// compilations. - final _activeIsolates = >{}; - - /// A set of isolates that are _not_ actively running compilations. - final _inactiveIsolates = >{}; - - /// The actual isolate objects that have been spawned. + /// All isolates that have been spawned to dispatch to. /// /// Only used for cleaning up the process when the underlying channel closes. - final _allIsolates = >[]; + final _allIsolates = >[]; + + /// The isolates that aren't currently running compilations + final _inactiveIsolates = {}; + + /// A map from active compilationIds to isolates running those compilations. + final _activeIsolates = {}; /// A pool controlling how many isolates (and thus concurrent compilations) /// may be live at once. @@ -55,10 +43,6 @@ class IsolateDispatcher { /// See https://github.com/sass/dart-sass/pull/2019 final _isolatePool = Pool(sizeOf() <= 4 ? 7 : 15); - /// Whether the underlying channel has closed and the dispatcher is shutting - /// down. - var _closed = false; - IsolateDispatcher(this._channel); void listen() { @@ -70,13 +54,15 @@ class IsolateDispatcher { (compilationId, messageBuffer) = parsePacket(packet); if (compilationId != 0) { - // TODO(nweiz): Consider using the techniques described in - // https://github.com/dart-lang/language/issues/124#issuecomment-988718728 - // or https://github.com/dart-lang/language/issues/3118 for low-cost - // inter-isolate transfers. - (_activeIsolates[compilationId] ?? await _getIsolate(compilationId)) - .add(messageBuffer); - return; + var isolate = _activeIsolates[compilationId] ?? + await _getIsolate(compilationId); + try { + isolate.send(packet); + return; + } on StateError catch (_) { + throw paramsError( + "Received multiple messages for compilation ID $compilationId"); + } } try { @@ -102,20 +88,9 @@ class IsolateDispatcher { }, onError: (Object error, StackTrace stackTrace) { _handleError(error, stackTrace); }, onDone: () async { - _closed = true; for (var isolate in _allIsolates) { (await isolate).kill(); } - - // Killing isolates isn't sufficient to make sure the process closes; we - // also have to close all the [ReceivePort]s we've constructed (by closing - // the [IsolateChannel]s). - for (var sink in _activeIsolates.values) { - sink.close(); - } - for (var channel in _inactiveIsolates) { - channel.sink.close(); - } }); } @@ -123,63 +98,37 @@ class IsolateDispatcher { /// /// This re-uses an existing isolate if possible, and spawns a new one /// otherwise. - Future> _getIsolate(int compilationId) async { + Future _getIsolate(int compilationId) async { var resource = await _isolatePool.request(); + ReusableIsolate isolate; if (_inactiveIsolates.isNotEmpty) { - return _activate(_inactiveIsolates.first, compilationId, resource); + isolate = _inactiveIsolates.first; + _inactiveIsolates.remove(isolate); + } else { + var future = ReusableIsolate.spawn(_isolateMain); + _allIsolates.add(future); + isolate = await future; } - var receivePort = ReceivePort(); - var future = Isolate.spawn(_isolateMain, receivePort.sendPort); - _allIsolates.add(future); - await future; - - var channel = IsolateChannel<_InitialMessage?>.connectReceive(receivePort) - .transform(const ExplicitCloseTransformer()); - channel.stream.listen(null, - onError: (Object error, StackTrace stackTrace) => - _handleError(error, stackTrace), - onDone: _channel.sink.close); - return _activate(channel, compilationId, resource); - } + _activeIsolates[compilationId] = isolate; + isolate.checkOut().listen(_channel.sink.add, + onError: (Object error, StackTrace stackTrace) { + if (error is ProtocolError) { + // Protocol errors have already been through [_handleError] in the child + // isolate, so we just send them as-is and close out the underlying + // channel. + sendError(compilationId, error); + _channel.sink.close(); + } else { + _handleError(error, stackTrace); + } + }, onDone: () { + _activeIsolates.remove(compilationId); + _inactiveIsolates.add(isolate); + resource.release(); + }); - /// Activates [isolate] for a new individual compilation. - /// - /// This pipes all the outputs from the given isolate through to [_channel]. - /// The [resource] is released once the isolate is no longer active. - StreamSink _activate(StreamChannel<_InitialMessage> isolate, - int compilationId, PoolResource resource) { - _inactiveIsolates.remove(isolate); - - // Each individual compilation has its own dedicated [IsolateChannel], which - // closes once the compilation is finished. - var receivePort = ReceivePort(); - isolate.sink.add((receivePort.sendPort, compilationId)); - - var channel = IsolateChannel.connectReceive(receivePort); - channel.stream.listen( - (message) { - // The first byte of messages from isolates indicates whether the - // entire compilation is finished. Sending this as part of the message - // buffer rather than a separate message avoids a race condition where - // the host might send a new compilation request with the same ID as - // one that just finished before the [IsolateDispatcher] receives word - // that the isolate with that ID is done. See sass/dart-sass#2004. - if (message[0] == 1) { - channel.sink.close(); - _activeIsolates.remove(compilationId); - _inactiveIsolates.add(isolate); - resource.release(); - } - _channel.sink.add(Uint8List.sublistView(message, 1)); - }, - onError: (Object error, StackTrace stackTrace) => - _handleError(error, stackTrace), - onDone: () { - if (_closed) isolate.sink.close(); - }); - _activeIsolates[compilationId] = channel.sink; - return channel.sink; + return isolate; } /// Creates a [OutboundMessage_VersionResponse] @@ -211,14 +160,6 @@ class IsolateDispatcher { _send(compilationId, OutboundMessage()..error = error); } -void _isolateMain(SendPort sendPort) { - var channel = IsolateChannel<_InitialMessage?>.connectSend(sendPort) - .transform(const ExplicitCloseTransformer()); - channel.stream.listen((initialMessage) async { - var (compilationSendPort, compilationId) = initialMessage; - var compilationChannel = - IsolateChannel.connectSend(compilationSendPort); - var success = await Dispatcher(compilationChannel, compilationId).listen(); - if (!success) channel.sink.close(); - }); +void _isolateMain(Mailbox mailbox, SendPort sendPort) { + Dispatcher(mailbox, sendPort).listen(); } diff --git a/lib/src/embedded/reusable_isolate.dart b/lib/src/embedded/reusable_isolate.dart new file mode 100644 index 000000000..0ed9eec8c --- /dev/null +++ b/lib/src/embedded/reusable_isolate.dart @@ -0,0 +1,142 @@ +// Copyright 2023 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 'dart:isolate'; +import 'dart:typed_data'; + +import 'package:native_synchronization/mailbox.dart'; +import 'package:native_synchronization/sendable.dart'; +import 'embedded_sass.pb.dart'; +import 'utils.dart'; + +/// The entrypoint for a [ReusableIsolate]. +/// +/// This must be a static global function. It's run when the isolate is spawned, +/// and is passed a [Mailbox] that receives messages from [ReusableIsolate.send] +/// and a [SendPort] that sends messages to the stream returned by +/// [ReusableIsolate.checkOut]. +/// +/// If the [sendPort] sends a message before [ReusableIsolate.checkOut] is +/// called, this will throw an unhandled [StateError]. +typedef ReusableIsolateEntryPoint = FutureOr Function( + Mailbox mailbox, SendPort sink); + +class ReusableIsolate { + /// The wrapped isolate. + final Isolate _isolate; + + /// The mailbox used to send messages to this isolate. + final Mailbox _mailbox; + + /// The [ReceivePort] that receives messages from the wrapped isolate. + final ReceivePort _receivePort; + + /// The subscription to [_port]. + final StreamSubscription _subscription; + + /// Whether [checkOut] has been called and the returned stream has not yet + /// closed. + bool _checkedOut = false; + + ReusableIsolate._(this._isolate, this._mailbox, this._receivePort) + : _subscription = _receivePort.listen(_defaultOnData); + + /// Spawns a [ReusableIsolate] that runs the given [entryPoint]. + static Future spawn( + ReusableIsolateEntryPoint entryPoint) async { + var mailbox = Mailbox(); + var receivePort = ReceivePort(); + var isolate = await Isolate.spawn( + _isolateMain, (entryPoint, mailbox.asSendable, receivePort.sendPort)); + return ReusableIsolate._(isolate, mailbox, receivePort); + } + + /// Checks out this isolate and returns a stream of messages from it. + /// + /// This isolate is considered "checked out" until the returned stream + /// completes. While checked out, messages may be sent to the isolate using + /// [send]. + /// + /// Throws a [StateError] if this is called while the isolate is already + /// checked out. + Stream checkOut() { + if (_checkedOut) { + throw StateError( + "Can't call ResuableIsolate.checkOut until the previous stream has " + "completed."); + } + _checkedOut = true; + + var controller = StreamController(sync: true); + + _subscription.onData((message) { + var fullBuffer = message as Uint8List; + + // The first byte of messages from isolates indicates whether the entire + // compilation is finished (1) or if it encountered an error (2). Sending + // this as part of the message buffer rather than a separate message + // avoids a race condition where the host might send a new compilation + // request with the same ID as one that just finished before the + // [IsolateDispatcher] receives word that the isolate with that ID is + // done. See sass/dart-sass#2004. + var category = fullBuffer[0]; + var packet = Uint8List.sublistView(fullBuffer, 1); + + if (category == 2) { + // Parse out the compilation ID and surface the [ProtocolError] as an + // error. This allows the [IsolateDispatcher] to notice that an error + // has occurred and close out the underlying channel. + var (_, buffer) = parsePacket(packet); + controller.addError(OutboundMessage.fromBuffer(buffer).error); + return; + } + + controller.sink.add(packet); + if (category == 1) { + _checkedOut = false; + _subscription.onData(_defaultOnData); + _subscription.onError(null); + controller.close(); + } + }); + + _subscription.onError(controller.addError); + + return controller.stream; + } + + /// Sends [message] to the isolate. + /// + /// Throws a [StateError] if this is called while the isolate isn't checked + /// out, or if a second message is sent before the isolate has processed the + /// first one. + void send(Uint8List message) { + _mailbox.put(message); + } + + /// Shuts down the isolate. + void kill() { + _isolate.kill(); + _receivePort.close(); + + // If the isolate is blocking on [Mailbox.take], it won't even process a + // kill event, so we send an empty message to make sure it wakes up. + try { + _mailbox.put(Uint8List(0)); + } on StateError catch (_) {} + } +} + +/// The default handler for data events from the wrapped isolate when it's not +/// checked out. +void _defaultOnData(Object? _) { + throw StateError("Shouldn't receive a message before being checked out."); +} + +void _isolateMain( + (ReusableIsolateEntryPoint, Sendable, SendPort) message) { + var (entryPoint, sendableMailbox, sendPort) = message; + entryPoint(sendableMailbox.materialize(), sendPort); +} diff --git a/lib/src/embedded/util/explicit_close_transformer.dart b/lib/src/embedded/util/explicit_close_transformer.dart deleted file mode 100644 index 43a5334dd..000000000 --- a/lib/src/embedded/util/explicit_close_transformer.dart +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright 2023 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:async/async.dart'; -import 'package:stream_channel/stream_channel.dart'; - -/// A [StreamChannelTransformer] that explicitly ensures that when one endpoint -/// closes its sink, the other endpoint will close as well. -/// -/// This must be applied to both ends of the channel, and the underlying channel -/// must reserve `null` for a close event. -class ExplicitCloseTransformer - implements StreamChannelTransformer { - const ExplicitCloseTransformer(); - - StreamChannel bind(StreamChannel channel) { - var closedUnderlyingSink = false; - return StreamChannel.withCloseGuarantee(channel.stream - .transform(StreamTransformer.fromHandlers(handleData: (data, sink) { - if (data == null) { - channel.sink.close(); - closedUnderlyingSink = true; - } else { - sink.add(data); - } - })), channel.sink - .transform(StreamSinkTransformer.fromHandlers(handleDone: (sink) { - if (!closedUnderlyingSink) { - closedUnderlyingSink = true; - sink.add(null); - sink.close(); - } - }))); - } -} diff --git a/pubspec.yaml b/pubspec.yaml index 8c93d7766..c77344546 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass -version: 1.66.1 +version: 1.66.2-dev description: A Sass implementation in Dart. homepage: https://github.com/sass/dart-sass @@ -20,6 +20,7 @@ dependencies: http: "^1.1.0" js: ^0.6.3 meta: ^1.3.0 + native_synchronization: ^0.2.0 node_interop: ^2.1.0 package_config: ^2.0.0 path: ^1.8.0 From fddf421c8f2a12f668c4d1345be746ace7d8567f Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Fri, 8 Sep 2023 16:07:54 -0700 Subject: [PATCH 03/30] Don't try to load absolute URLs from the base importer (#2077) --- CHANGELOG.md | 8 ++++- lib/src/async_import_cache.dart | 2 +- lib/src/import_cache.dart | 4 +-- pubspec.yaml | 2 +- test/dart_api/importer_test.dart | 55 ++++++++++++++++++++++++++++++++ test/dart_api_test.dart | 22 +++++++++++-- 6 files changed, 86 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1666f753f..2894b314a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,10 @@ -## 1.66.2 +## 1.67.0 + +* **Potentially breaking bug fix**: The importer used to load a given file is no + longer used to load absolute URLs that appear in that file. This was + unintented behavior that contradicted the Sass specification. Absolute URLs + will now correctly be loaded only from the global importer list. This applies + to the modern JS API, the Dart API, and the embedded protocol. ### Embedded Sass diff --git a/lib/src/async_import_cache.dart b/lib/src/async_import_cache.dart index c67f77081..019b3414a 100644 --- a/lib/src/async_import_cache.dart +++ b/lib/src/async_import_cache.dart @@ -142,7 +142,7 @@ final class AsyncImportCache { throw "Custom importers are required to load stylesheets when compiling in the browser."; } - if (baseImporter != null) { + if (baseImporter != null && url.scheme == '') { var relativeResult = await putIfAbsentAsync(_relativeCanonicalizeCache, ( url, forImport: forImport, diff --git a/lib/src/import_cache.dart b/lib/src/import_cache.dart index b85c78d07..b9c48a6f8 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: 1b6289e0dd362fcb02f331a16a30fe94050b4e17 +// Checksum: 3e4cae79c03ce2af6626b1822f1468523b401e90 // // ignore_for_file: unused_import @@ -142,7 +142,7 @@ final class ImportCache { throw "Custom importers are required to load stylesheets when compiling in the browser."; } - if (baseImporter != null) { + if (baseImporter != null && url.scheme == '') { var relativeResult = _relativeCanonicalizeCache.putIfAbsent(( url, forImport: forImport, diff --git a/pubspec.yaml b/pubspec.yaml index c77344546..5c2a1698b 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass -version: 1.66.2-dev +version: 1.67.0-dev description: A Sass implementation in Dart. homepage: https://github.com/sass/dart-sass diff --git a/test/dart_api/importer_test.dart b/test/dart_api/importer_test.dart index bf701f2b3..113e9a24b 100644 --- a/test/dart_api/importer_test.dart +++ b/test/dart_api/importer_test.dart @@ -201,6 +201,61 @@ void main() { }))); }); + group("compileString()'s importer option", () { + test("loads relative imports from the entrypoint", () { + var css = compileString('@import "orange";', + importer: TestImporter((url) => Uri.parse("u:$url"), (url) { + var color = url.path; + return ImporterResult('.$color {color: $color}', indented: false); + })); + + expect(css, equals(".orange {\n color: orange;\n}")); + }); + + test("loads imports relative to the entrypoint's URL", () { + var css = compileString('@import "baz/qux";', + importer: TestImporter((url) => url.resolve("bang"), (url) { + return ImporterResult('a {result: "${url.path}"}', indented: false); + }), + url: Uri.parse("u:foo/bar")); + + expect(css, equals('a {\n result: "foo/baz/bang";\n}')); + }); + + test("doesn't load absolute imports", () { + var css = compileString('@import "u:orange";', + importer: TestImporter((_) => throw "Should not be called", + (_) => throw "Should not be called"), + importers: [ + TestImporter((url) => url, (url) { + var color = url.path; + return ImporterResult('.$color {color: $color}', indented: false); + }) + ]); + + expect(css, equals(".orange {\n color: orange;\n}")); + }); + + test("doesn't load from other importers", () { + var css = compileString('@import "u:midstream";', + importer: TestImporter((_) => throw "Should not be called", + (_) => throw "Should not be called"), + importers: [ + TestImporter((url) => url, (url) { + if (url.path == "midstream") { + return ImporterResult("@import 'orange';", indented: false); + } else { + var color = url.path; + return ImporterResult('.$color {color: $color}', + indented: false); + } + }) + ]); + + expect(css, equals(".orange {\n color: orange;\n}")); + }); + }); + group("currentLoadFromImport is", () { test("true from an @import", () { compileString('@import "foo"', importers: [FromImportImporter(true)]); diff --git a/test/dart_api_test.dart b/test/dart_api_test.dart index 60691aad3..2fd8d6737 100644 --- a/test/dart_api_test.dart +++ b/test/dart_api_test.dart @@ -12,6 +12,8 @@ import 'package:test_descriptor/test_descriptor.dart' as d; import 'package:sass/sass.dart'; import 'package:sass/src/exception.dart'; +import 'dart_api/test_importer.dart'; + void main() { // TODO(nweiz): test SASS_PATH when dart-lang/sdk#28160 is fixed. @@ -139,8 +141,9 @@ void main() { expect(css, equals("a {\n b: from-relative;\n}")); }); - test("the original importer takes precedence over other importers", - () async { + test( + "the original importer takes precedence over other importers for " + "relative imports", () async { await d.dir( "original", [d.file("other.scss", "a {b: from-original}")]).create(); await d @@ -153,6 +156,21 @@ void main() { expect(css, equals("a {\n b: from-original;\n}")); }); + test("importer order is preserved for absolute imports", () { + var css = compileString('@import "second:other";', importers: [ + TestImporter((url) => url.scheme == 'first' ? url : null, + (url) => ImporterResult('a {from: first}', indented: false)), + // This importer should only be invoked once, because when the + // "first:other" import is resolved it should be passed to the first + // importer first despite being in the second importer's file. + TestImporter( + expectAsync1((url) => url.scheme == 'second' ? url : null, + count: 1), + (url) => ImporterResult('@import "first:other";', indented: false)), + ]); + expect(css, equals("a {\n from: first;\n}")); + }); + test("importers take precedence over load paths", () async { await d.dir("load-path", [d.file("other.scss", "a {b: from-load-path}")]).create(); From 77e208c2044dc91ab5aa75c6a96531922592b049 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Fri, 8 Sep 2023 18:29:09 -0700 Subject: [PATCH 04/30] Run cli compilations in parallel dart isolates (#2078) Co-authored-by: Natalie Weizenbaum --- bin/sass.dart | 77 ++----------- lib/src/executable/compile_stylesheet.dart | 64 ++++++++++- lib/src/executable/concurrent.dart | 66 ++++++++++++ lib/src/executable/concurrent/js.dart | 10 ++ lib/src/executable/concurrent/vm.dart | 33 ++++++ lib/src/executable/watch.dart | 119 +++++++-------------- lib/src/io/interface.dart | 8 ++ lib/src/io/js.dart | 8 ++ lib/src/io/vm.dart | 15 ++- lib/src/parse/parser.dart | 5 +- test/cli/shared/colon_args.dart | 1 - test/cli/shared/update.dart | 23 +++- test/cli/shared/watch.dart | 4 +- 13 files changed, 273 insertions(+), 160 deletions(-) create mode 100644 lib/src/executable/concurrent.dart create mode 100644 lib/src/executable/concurrent/js.dart create mode 100644 lib/src/executable/concurrent/vm.dart diff --git a/bin/sass.dart b/bin/sass.dart index 986ff9bcb..ad23649d4 100644 --- a/bin/sass.dart +++ b/bin/sass.dart @@ -8,17 +8,14 @@ import 'package:path/path.dart' as p; import 'package:stack_trace/stack_trace.dart'; import 'package:term_glyph/term_glyph.dart' as term_glyph; -import 'package:sass/src/exception.dart'; -import 'package:sass/src/executable/compile_stylesheet.dart'; +import 'package:sass/src/executable/concurrent.dart'; import 'package:sass/src/executable/options.dart'; import 'package:sass/src/executable/repl.dart'; import 'package:sass/src/executable/watch.dart'; import 'package:sass/src/import_cache.dart'; import 'package:sass/src/io.dart'; -import 'package:sass/src/io.dart' as io; import 'package:sass/src/logger/deprecation_handling.dart'; import 'package:sass/src/stylesheet_graph.dart'; -import 'package:sass/src/util/map.dart'; import 'package:sass/src/utils.dart'; import 'package:sass/src/embedded/executable.dart' // Never load the embedded protocol when compiling to JS. @@ -26,27 +23,6 @@ import 'package:sass/src/embedded/executable.dart' as embedded; Future main(List args) async { - var printedError = false; - - // Prints [error] to stderr, along with a preceding newline if anything else - // has been printed to stderr. - // - // If [trace] is passed, its terse representation is printed after the error. - void printError(String error, StackTrace? stackTrace) { - var buffer = StringBuffer(); - if (printedError) buffer.writeln(); - printedError = true; - buffer.write(error); - - if (stackTrace != null) { - buffer.writeln(); - buffer.writeln(); - buffer.write(Trace.from(stackTrace).terse.toString().trimRight()); - } - - io.printError(buffer); - } - if (args case ['--embedded', ...var rest]) { embedded.main(rest); return; @@ -84,37 +60,8 @@ Future main(List args) async { return; } - for (var (source, destination) in options.sourcesToDestinations.pairs) { - try { - await compileStylesheet(options, graph, source, destination, - ifModified: options.update); - } on SassException catch (error, stackTrace) { - if (destination != null && !options.emitErrorCss) { - _tryDelete(destination); - } - - printError(error.toString(color: options.color), - options.trace ? getTrace(error) ?? stackTrace : null); - - // Exit code 65 indicates invalid data per - // https://www.freebsd.org/cgi/man.cgi?query=sysexits. - // - // We let exitCode 66 take precedence for deterministic behavior. - if (exitCode != 66) exitCode = 65; - if (options.stopOnError) return; - } on FileSystemException catch (error, stackTrace) { - var path = error.path; - printError( - path == null - ? error.message - : "Error reading ${p.relative(path)}: ${error.message}.", - options.trace ? getTrace(error) ?? stackTrace : null); - - // Error 66 indicates no input. - exitCode = 66; - if (options.stopOnError) return; - } - } + await compileStylesheets(options, graph, options.sourcesToDestinations, + ifModified: options.update); } on UsageException catch (error) { print("${error.message}\n"); print("Usage: sass [output.css]\n" @@ -128,8 +75,11 @@ Future main(List args) async { if (options?.color ?? false) buffer.write('\u001b[0m'); buffer.writeln(); buffer.writeln(error); - - printError(buffer.toString(), getTrace(error) ?? stackTrace); + buffer.writeln(); + buffer.writeln(); + buffer.write( + Trace.from(getTrace(error) ?? stackTrace).terse.toString().trimRight()); + printError(buffer); exitCode = 255; } } @@ -154,14 +104,3 @@ Future _loadVersion() async { .split(" ") .last; } - -/// Delete [path] if it exists and do nothing otherwise. -/// -/// This is a separate function to work around dart-lang/sdk#53082. -void _tryDelete(String path) { - try { - deleteFile(path); - } on FileSystemException { - // If the file doesn't exist, that's fine. - } -} diff --git a/lib/src/executable/compile_stylesheet.dart b/lib/src/executable/compile_stylesheet.dart index 8c24ee494..70b52ba10 100644 --- a/lib/src/executable/compile_stylesheet.dart +++ b/lib/src/executable/compile_stylesheet.dart @@ -6,6 +6,7 @@ import 'dart:convert'; import 'package:path/path.dart' as p; import 'package:source_maps/source_maps.dart'; +import 'package:stack_trace/stack_trace.dart'; import '../async_import_cache.dart'; import '../compile.dart'; @@ -30,8 +31,42 @@ import 'options.dart'; /// If [ifModified] is `true`, only recompiles if [source]'s modification time /// or that of a file it imports is more recent than [destination]'s /// modification time. Note that these modification times are cached by [graph]. -Future compileStylesheet(ExecutableOptions options, StylesheetGraph graph, - String? source, String? destination, +/// +/// Returns `(exitCode, error, stackTrace)` when an error occurs. +Future<(int, String, String?)?> compileStylesheet(ExecutableOptions options, + StylesheetGraph graph, String? source, String? destination, + {bool ifModified = false}) async { + try { + await _compileStylesheetWithoutErrorHandling( + options, graph, source, destination, + ifModified: ifModified); + } on SassException catch (error, stackTrace) { + if (destination != null && !options.emitErrorCss) { + _tryDelete(destination); + } + var message = error.toString(color: options.color); + + // Exit code 65 indicates invalid data per + // https://www.freebsd.org/cgi/man.cgi?query=sysexits. + return _getErrorWithStackTrace( + 65, message, options.trace ? getTrace(error) ?? stackTrace : null); + } on FileSystemException catch (error, stackTrace) { + var path = error.path; + var message = path == null + ? error.message + : "Error reading ${p.relative(path)}: ${error.message}."; + + // Exit code 66 indicates no input. + return _getErrorWithStackTrace( + 66, message, options.trace ? getTrace(error) ?? stackTrace : null); + } + return null; +} + +/// Like [compileStylesheet], but throws errors instead of handling them +/// internally. +Future _compileStylesheetWithoutErrorHandling(ExecutableOptions options, + StylesheetGraph graph, String? source, String? destination, {bool ifModified = false}) async { var importer = FilesystemImporter('.'); if (ifModified) { @@ -150,7 +185,7 @@ Future compileStylesheet(ExecutableOptions options, StylesheetGraph graph, buffer.write('Compiled $sourceName to $destinationName.'); if (options.color) buffer.write('\u001b[0m'); - print(buffer); + safePrint(buffer); } /// Writes the source map given by [mapping] to disk (if necessary) according to @@ -195,3 +230,26 @@ String _writeSourceMap( return (options.style == OutputStyle.compressed ? '' : '\n\n') + '/*# sourceMappingURL=$escapedUrl */'; } + +/// Delete [path] if it exists and do nothing otherwise. +/// +/// This is a separate function to work around dart-lang/sdk#53082. +void _tryDelete(String path) { + try { + deleteFile(path); + } on FileSystemException { + // If the file doesn't exist, that's fine. + } +} + +/// Return a Record of `(exitCode, error, stackTrace)` for the given error. +(int, String, String?) _getErrorWithStackTrace( + int exitCode, String error, StackTrace? stackTrace) { + return ( + exitCode, + error, + stackTrace != null + ? Trace.from(stackTrace).terse.toString().trimRight() + : null + ); +} diff --git a/lib/src/executable/concurrent.dart b/lib/src/executable/concurrent.dart new file mode 100644 index 000000000..f3020f679 --- /dev/null +++ b/lib/src/executable/concurrent.dart @@ -0,0 +1,66 @@ +// Copyright 2023 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:math' as math; + +import '../io.dart'; +import '../stylesheet_graph.dart'; +import '../util/map.dart'; +import 'compile_stylesheet.dart'; +import 'concurrent/vm.dart' + // Never load the isolate library when compiling to JS. + if (dart.library.js) 'concurrent/js.dart'; +import 'options.dart'; + +/// Compiles the stylesheets concurrently and returns whether all stylesheets are compiled +/// successfully. +Future compileStylesheets(ExecutableOptions options, + StylesheetGraph graph, Map sourcesToDestinations, + {bool ifModified = false}) async { + var errorsWithStackTraces = switch ([...sourcesToDestinations.pairs]) { + // Concurrency does add some overhead, so avoid it in the common case of + // compiling a single stylesheet. + [(var source, var destination)] => [ + await compileStylesheet(options, graph, source, destination, + ifModified: ifModified) + ], + var pairs => await Future.wait([ + for (var (source, destination) in pairs) + compileStylesheetConcurrently(options, graph, source, destination, + ifModified: ifModified) + ], eagerError: options.stopOnError) + }; + + var printedError = false; + + // Print all errors in deterministic order. + for (var errorWithStackTrace in errorsWithStackTraces) { + if (errorWithStackTrace == null) continue; + var (code, error, stackTrace) = errorWithStackTrace; + + // We let the highest exitCode take precedence for deterministic behavior. + exitCode = math.max(exitCode, code); + + _printError(error, stackTrace, printedError); + printedError = true; + } + + return !printedError; +} + +// Prints [error] to stderr, along with a preceding newline if anything else +// has been printed to stderr. +// +// If [stackTrace] is passed, it is printed after the error. +void _printError(String error, String? stackTrace, bool printedError) { + var buffer = StringBuffer(); + if (printedError) buffer.writeln(); + buffer.write(error); + if (stackTrace != null) { + buffer.writeln(); + buffer.writeln(); + buffer.write(stackTrace); + } + printError(buffer); +} diff --git a/lib/src/executable/concurrent/js.dart b/lib/src/executable/concurrent/js.dart new file mode 100644 index 000000000..11cdcb1c1 --- /dev/null +++ b/lib/src/executable/concurrent/js.dart @@ -0,0 +1,10 @@ +// Copyright 2023 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 '../compile_stylesheet.dart'; + +/// We don't currently support concurrent compilation in JS. +/// +/// In the future, we could add support using web workers. +final compileStylesheetConcurrently = compileStylesheet; diff --git a/lib/src/executable/concurrent/vm.dart b/lib/src/executable/concurrent/vm.dart new file mode 100644 index 000000000..b063b52f0 --- /dev/null +++ b/lib/src/executable/concurrent/vm.dart @@ -0,0 +1,33 @@ +// Copyright 2023 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:isolate'; + +import 'package:term_glyph/term_glyph.dart' as term_glyph; + +import '../options.dart'; +import '../../stylesheet_graph.dart'; +import '../compile_stylesheet.dart'; + +/// Compiles the stylesheet at [source] to [destination]. +/// +/// Runs in a new Dart Isolate, unless [source] is `null`. +Future<(int, String, String?)?> compileStylesheetConcurrently( + ExecutableOptions options, + StylesheetGraph graph, + String? source, + String? destination, + {bool ifModified = false}) { + // Reading from stdin does not work properly in dart isolate. + if (source == null) { + return compileStylesheet(options, graph, source, destination, + ifModified: ifModified); + } + + return Isolate.run(() { + term_glyph.ascii = !options.unicode; + return compileStylesheet(options, graph, source, destination, + ifModified: ifModified); + }); +} diff --git a/lib/src/executable/watch.dart b/lib/src/executable/watch.dart index 1fc8d9f37..c8a222b0b 100644 --- a/lib/src/executable/watch.dart +++ b/lib/src/executable/watch.dart @@ -2,21 +2,16 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. -import 'dart:collection'; - import 'package:path/path.dart' as p; -import 'package:stack_trace/stack_trace.dart'; import 'package:stream_transform/stream_transform.dart'; import 'package:watcher/watcher.dart'; -import '../exception.dart'; import '../importer/filesystem.dart'; import '../io.dart'; import '../stylesheet_graph.dart'; import '../util/map.dart'; import '../util/multi_dir_watcher.dart'; -import '../utils.dart'; -import 'compile_stylesheet.dart'; +import 'concurrent.dart'; import 'options.dart'; /// Watches all the files in [graph] for changes and updates them as necessary. @@ -41,15 +36,17 @@ Future watch(ExecutableOptions options, StylesheetGraph graph) async { // they currently exist. This ensures that changes that come in update a // known-good state. var watcher = _Watcher(options, graph); - for (var (source, destination) in _sourcesToDestinations(options).pairs) { + var sourcesToDestinations = _sourcesToDestinations(options); + for (var source in sourcesToDestinations.keys) { graph.addCanonical( FilesystemImporter('.'), p.toUri(canonicalize(source)), p.toUri(source), recanonicalize: false); - var success = await watcher.compile(source, destination, ifModified: true); - if (!success && options.stopOnError) { - dirWatcher.events.listen(null).cancel(); - return; - } + } + var success = await compileStylesheets(options, graph, sourcesToDestinations, + ifModified: true); + if (!success && options.stopOnError) { + dirWatcher.events.listen(null).cancel(); + return; } print("Sass is watching for changes. Press Ctrl-C to stop.\n"); @@ -67,34 +64,6 @@ final class _Watcher { _Watcher(this._options, this._graph); - /// Compiles the stylesheet at [source] to [destination], and prints any - /// errors that occur. - /// - /// Returns whether or not compilation succeeded. - Future compile(String source, String destination, - {bool ifModified = false}) async { - try { - await compileStylesheet(_options, _graph, source, destination, - ifModified: ifModified); - return true; - } on SassException catch (error, stackTrace) { - if (!_options.emitErrorCss) _delete(destination); - _printError( - error.toString(color: _options.color), getTrace(error) ?? stackTrace); - exitCode = 65; - return false; - } on FileSystemException catch (error, stackTrace) { - var path = error.path; - _printError( - path == null - ? error.message - : "Error reading ${p.relative(path)}: ${error.message}.", - getTrace(error) ?? stackTrace); - exitCode = 66; - return false; - } - } - /// Deletes the file at [path] and prints a message about it. void _delete(String path) { try { @@ -109,21 +78,6 @@ final class _Watcher { } } - /// Prints [message] to standard error, with [stackTrace] if [_options.trace] - /// is set. - void _printError(String message, StackTrace stackTrace) { - var buffer = StringBuffer(message); - - if (_options.trace) { - buffer.writeln(); - buffer.writeln(); - buffer.write(Trace.from(stackTrace).terse.toString().trimRight()); - } - - if (!_options.stopOnError) buffer.writeln(); - printError(buffer); - } - /// Listens to `watcher.events` and updates the filesystem accordingly. /// /// Returns a future that will only complete if an unexpected error occurs. @@ -172,8 +126,9 @@ final class _Watcher { /// Returns whether all necessary recompilations succeeded. Future _handleAdd(String path) async { var destination = _destinationFor(path); - - var success = destination == null || await compile(path, destination); + var success = destination == null || + await compileStylesheets(_options, _graph, {path: destination}, + ifModified: true); var downstream = _graph.addCanonical( FilesystemImporter('.'), _canonicalize(path), p.toUri(path)); return await _recompileDownstream(downstream) && success; @@ -226,34 +181,42 @@ final class _Watcher { /// Returns whether all recompilations succeeded. Future _recompileDownstream(Iterable nodes) async { var seen = {}; - var toRecompile = Queue.of(nodes); - var allSucceeded = true; - while (toRecompile.isNotEmpty) { - var node = toRecompile.removeFirst(); - if (!seen.add(node)) continue; + while (nodes.isNotEmpty) { + nodes = [ + for (var node in nodes) + if (seen.add(node)) node + ]; - var success = await _compileIfEntrypoint(node.canonicalUrl); - allSucceeded = allSucceeded && success; - if (!success && _options.stopOnError) return false; + var sourcesToDestinations = _sourceEntrypointsToDestinations(nodes); + if (sourcesToDestinations.isNotEmpty) { + var success = await compileStylesheets( + _options, _graph, sourcesToDestinations, + ifModified: true); + if (!success && _options.stopOnError) return false; - toRecompile.addAll(node.downstream); + allSucceeded = allSucceeded && success; + } + + nodes = [for (var node in nodes) ...node.downstream]; } return allSucceeded; } - /// Compiles the stylesheet at [url] to CSS if it's an entrypoint that's being - /// watched. - /// - /// Returns `false` if compilation failed, `true` otherwise. - Future _compileIfEntrypoint(Uri url) async { - if (url.scheme != 'file') return true; - - var source = p.fromUri(url); - return switch (_destinationFor(source)) { - var destination? => await compile(source, destination), - _ => true - }; + /// Returns a sourcesToDestinations mapping for nodes that are entrypoints. + Map _sourceEntrypointsToDestinations( + Iterable nodes) { + var entrypoints = {}; + for (var node in nodes) { + var url = node.canonicalUrl; + if (url.scheme != 'file') continue; + + var source = p.fromUri(url); + if (_destinationFor(source) case var destination?) { + entrypoints[source] = destination; + } + } + return entrypoints; } /// If a Sass file at [source] should be compiled to CSS, returns the path to diff --git a/lib/src/io/interface.dart b/lib/src/io/interface.dart index 618c3ff6e..be0ec574c 100644 --- a/lib/src/io/interface.dart +++ b/lib/src/io/interface.dart @@ -32,8 +32,16 @@ bool get isBrowser => throw ''; /// sequences. bool get supportsAnsiEscapes => throw ''; +/// Prints [message] (followed by a newline) to standard output or the +/// equivalent. +/// +/// This method is thread-safe. +void safePrint(Object? message) => throw ''; + /// Prints [message] (followed by a newline) to standard error or the /// equivalent. +/// +/// This method is thread-safe. void printError(Object? message) => throw ''; /// Reads the file at [path] as a UTF-8 encoded string. diff --git a/lib/src/io/js.dart b/lib/src/io/js.dart index efc1955e4..c806dc181 100644 --- a/lib/src/io/js.dart +++ b/lib/src/io/js.dart @@ -28,6 +28,14 @@ class FileSystemException { String toString() => "${p.prettyUri(p.toUri(path))}: $message"; } +void safePrint(Object? message) { + if (process case var process?) { + process.stdout.write("${message ?? ''}\n"); + } else { + console.log(message ?? ''); + } +} + void printError(Object? message) { if (process case var process?) { process.stderr.write("${message ?? ''}\n"); diff --git a/lib/src/io/vm.dart b/lib/src/io/vm.dart index 0174e09b4..cba88e40f 100644 --- a/lib/src/io/vm.dart +++ b/lib/src/io/vm.dart @@ -37,8 +37,21 @@ bool get supportsAnsiEscapes { return io.stdout.supportsAnsiEscapes; } +void safePrint(Object? message) { + _threadSafeWriteLn(io.stdout, message); +} + void printError(Object? message) { - io.stderr.writeln(message); + _threadSafeWriteLn(io.stderr, message); +} + +void _threadSafeWriteLn(io.IOSink sink, Object? message) { + // This does have performance penality of copying buffer + // if message is already a StringBuffer. + // https://github.com/dart-lang/sdk/issues/53471. + var buffer = StringBuffer(message.toString()); + buffer.writeln(); + sink.write(buffer); } String readFile(String path) { diff --git a/lib/src/parse/parser.dart b/lib/src/parse/parser.dart index dba0bef99..6dd17c80d 100644 --- a/lib/src/parse/parser.dart +++ b/lib/src/parse/parser.dart @@ -9,6 +9,7 @@ import 'package:string_scanner/string_scanner.dart'; import '../exception.dart'; import '../interpolation_map.dart'; +import '../io.dart'; import '../logger.dart'; import '../util/character.dart'; import '../util/lazy_file_span.dart'; @@ -696,9 +697,9 @@ class Parser { @protected void debug([Object? message]) { if (message == null) { - print(scanner.emptySpan.highlight(color: true)); + safePrint(scanner.emptySpan.highlight(color: true)); } else { - print(scanner.emptySpan.message(message.toString(), color: true)); + safePrint(scanner.emptySpan.message(message.toString(), color: true)); } } diff --git a/test/cli/shared/colon_args.dart b/test/cli/shared/colon_args.dart index 88ca8c642..a5ccf314f 100644 --- a/test/cli/shared/colon_args.dart +++ b/test/cli/shared/colon_args.dart @@ -97,7 +97,6 @@ void sharedTests(Future runSass(Iterable arguments)) { await sass.shouldExit(65); await d.file("out1.css", contains(message)).validate(); - await d.nothing("out2.css").validate(); }); group("with a directory argument", () { diff --git a/test/cli/shared/update.dart b/test/cli/shared/update.dart index 01aa73949..4fc7bfbc1 100644 --- a/test/cli/shared/update.dart +++ b/test/cli/shared/update.dart @@ -66,16 +66,31 @@ void sharedTests(Future runSass(Iterable arguments)) { await d.file("test2.scss", r"$var: 2; @import 'other'").create(); var sass = await update(["test1.scss:out1.css", "test2.scss:out2.css"]); - expect(sass.stdout, emits(endsWith('Compiled test1.scss to out1.css.'))); - expect(sass.stdout, emits(endsWith('Compiled test2.scss to out2.css.'))); + expect( + sass.stdout, + emitsInAnyOrder([ + endsWith('Compiled test1.scss to out1.css.'), + endsWith('Compiled test2.scss to out2.css.') + ])); await sass.shouldExit(0); + await d + .file("out1.css", equalsIgnoringWhitespace("a { b: 1; }")) + .validate(); + await d + .file("out2.css", equalsIgnoringWhitespace("a { b: 2; }")) + .validate(); + await tick; await d.file("other.scss", r"x {y: $var}").create(); sass = await update(["test1.scss:out1.css", "test2.scss:out2.css"]); - expect(sass.stdout, emits(endsWith('Compiled test1.scss to out1.css.'))); - expect(sass.stdout, emits(endsWith('Compiled test2.scss to out2.css.'))); + expect( + sass.stdout, + emitsInAnyOrder([ + endsWith('Compiled test1.scss to out1.css.'), + endsWith('Compiled test2.scss to out2.css.') + ])); await sass.shouldExit(0); await d diff --git a/test/cli/shared/watch.dart b/test/cli/shared/watch.dart index b46fd97a0..b80b33e64 100644 --- a/test/cli/shared/watch.dart +++ b/test/cli/shared/watch.dart @@ -3,6 +3,7 @@ // https://opensource.org/licenses/MIT. import 'package:path/path.dart' as p; +import 'package:sass/src/io.dart'; import 'package:test/test.dart'; import 'package:test_descriptor/test_descriptor.dart' as d; import 'package:test_process/test_process.dart'; @@ -29,7 +30,7 @@ void sharedTests(Future runSass(Iterable arguments)) { /// Modifying a file very quickly after it was processed can go /// unrecognized, especially on Windows where filesystem operations can have /// very high delays. - Future tickIfPoll() => poll ? tick : Future.value(); + Future tickIfPoll() => poll || isWindows ? tick : Future.value(); group("${poll ? 'with' : 'without'} --poll", () { group("when started", () { @@ -128,7 +129,6 @@ void sharedTests(Future runSass(Iterable arguments)) { await sass.shouldExit(65); await d.file("out1.css", contains(message)).validate(); - await d.nothing("out2.css").validate(); }); }); From bdb145f0396446dd1771229592bb31aea9c4383a Mon Sep 17 00:00:00 2001 From: Daniil Gentili Date: Tue, 12 Sep 2023 22:15:09 +0200 Subject: [PATCH 05/30] Fix example (#2074) --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index e22309bd4..64bbb851e 100644 --- a/README.md +++ b/README.md @@ -305,8 +305,8 @@ commands: ```Dockerfile # Dart stage -FROM dart:stable AS dart FROM bufbuild/buf AS buf +FROM dart:stable AS dart # Add your scss files COPY --from=another_stage /app /app From 5c31d1f245c274ff90eded7d7ae4437c664798b9 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Wed, 13 Sep 2023 17:23:05 -0700 Subject: [PATCH 06/30] Re-enable new calculation functions (#2080) --- CHANGELOG.md | 26 + lib/src/ast/sass.dart | 1 - lib/src/ast/sass/expression.dart | 87 ++- .../ast/sass/expression/binary_operation.dart | 12 + lib/src/ast/sass/expression/calculation.dart | 108 ---- lib/src/ast/sass/interpolation.dart | 3 + lib/src/deprecation.dart | 5 + lib/src/embedded/protofier.dart | 4 +- lib/src/functions/math.dart | 88 +-- lib/src/js/value/calculation.dart | 4 +- lib/src/parse/css.dart | 25 +- lib/src/parse/stylesheet.dart | 253 +------- lib/src/util/nullable.dart | 7 + lib/src/util/number.dart | 78 +++ lib/src/value/calculation.dart | 596 +++++++++++++++++- lib/src/value/number.dart | 2 +- lib/src/value/number/unitless.dart | 2 +- lib/src/visitor/ast_search.dart | 185 ++++++ lib/src/visitor/async_evaluate.dart | 407 +++++++++--- lib/src/visitor/evaluate.dart | 404 +++++++++--- lib/src/visitor/expression_to_calc.dart | 12 +- lib/src/visitor/interface/expression.dart | 1 - lib/src/visitor/recursive_ast.dart | 12 +- lib/src/visitor/replace_expression.dart | 4 - lib/src/visitor/serialize.dart | 11 +- pkg/sass_api/CHANGELOG.md | 8 + pkg/sass_api/lib/sass_api.dart | 1 + pkg/sass_api/pubspec.yaml | 4 +- pubspec.yaml | 2 +- test/dart_api/value/calculation_test.dart | 7 + test/embedded/function_test.dart | 4 +- 31 files changed, 1677 insertions(+), 686 deletions(-) delete mode 100644 lib/src/ast/sass/expression/calculation.dart create mode 100644 lib/src/visitor/ast_search.dart diff --git a/CHANGELOG.md b/CHANGELOG.md index 2894b314a..e828616a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,31 @@ ## 1.67.0 +* All functions defined in CSS Values and Units 4 are now once again parsed as + calculation objects: `round()`, `mod()`, `rem()`, `sin()`, `cos()`, `tan()`, + `asin()`, `acos()`, `atan()`, `atan2()`, `pow()`, `sqrt()`, `hypot()`, + `log()`, `exp()`, `abs()`, and `sign()`. + + Unlike in 1.65.0, function calls are _not_ locked into being parsed as + calculations or plain Sass functions at parse-time. This means that + user-defined functions will take precedence over CSS calculations of the same + name. Although the function names `calc()` and `clamp()` are still forbidden, + users may continue to freely define functions whose names overlap with other + CSS calculations (including `abs()`, `min()`, `max()`, and `round()` whose + names overlap with global Sass functions). + +* As a consequence of the change in calculation parsing described above, + calculation functions containing interpolation are now parsed more strictly + than before. However, all interpolations that would have produced valid CSS + will continue to work, so this is not considered a breaking change. + +* Interpolations in calculation functions that aren't used in a position that + could also have a normal calculation value are now deprecated. For example, + `calc(1px #{"+ 2px"})` is deprecated, but `calc(1px + #{"2px"})` is still + allowed. This deprecation is named `calc-interp`. See [the Sass website] for + more information. + + [the Sass website]: https://sass-lang.com/d/calc-interp + * **Potentially breaking bug fix**: The importer used to load a given file is no longer used to load absolute URLs that appear in that file. This was unintented behavior that contradicted the Sass specification. Absolute URLs diff --git a/lib/src/ast/sass.dart b/lib/src/ast/sass.dart index aa5ebbb79..149641670 100644 --- a/lib/src/ast/sass.dart +++ b/lib/src/ast/sass.dart @@ -13,7 +13,6 @@ export 'sass/dependency.dart'; export 'sass/expression.dart'; export 'sass/expression/binary_operation.dart'; export 'sass/expression/boolean.dart'; -export 'sass/expression/calculation.dart'; export 'sass/expression/color.dart'; export 'sass/expression/function.dart'; export 'sass/expression/if.dart'; diff --git a/lib/src/ast/sass/expression.dart b/lib/src/ast/sass/expression.dart index a5682411e..051e1c269 100644 --- a/lib/src/ast/sass/expression.dart +++ b/lib/src/ast/sass/expression.dart @@ -2,13 +2,16 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. +import 'package:charcode/charcode.dart'; import 'package:meta/meta.dart'; import '../../exception.dart'; import '../../logger.dart'; import '../../parse/scss.dart'; +import '../../util/nullable.dart'; +import '../../value.dart'; import '../../visitor/interface/expression.dart'; -import 'node.dart'; +import '../sass.dart'; /// A SassScript expression in a Sass syntax tree. /// @@ -27,3 +30,85 @@ abstract interface class Expression implements SassNode { factory Expression.parse(String contents, {Object? url, Logger? logger}) => ScssParser(contents, url: url, logger: logger).parseExpression(); } + +// Use an extension class rather than a method so we don't have to make +// [Expression] a concrete base class for something we'll get rid of anyway once +// we remove the global math functions that make this necessary. +extension ExpressionExtensions on Expression { + /// Whether this expression can be used in a calculation context. + /// + /// @nodoc + @internal + bool get isCalculationSafe => accept(_IsCalculationSafeVisitor()); +} + +// We could use [AstSearchVisitor] to implement this more tersely, but that +// would default to returning `true` if we added a new expression type and +// forgot to update this class. +class _IsCalculationSafeVisitor implements ExpressionVisitor { + const _IsCalculationSafeVisitor(); + + bool visitBinaryOperationExpression(BinaryOperationExpression node) => + (const { + BinaryOperator.times, + BinaryOperator.dividedBy, + BinaryOperator.plus, + BinaryOperator.minus + }).contains(node.operator) && + (node.left.accept(this) || node.right.accept(this)); + + bool visitBooleanExpression(BooleanExpression node) => false; + + bool visitColorExpression(ColorExpression node) => false; + + bool visitFunctionExpression(FunctionExpression node) => true; + + bool visitInterpolatedFunctionExpression( + InterpolatedFunctionExpression node) => + true; + + bool visitIfExpression(IfExpression node) => true; + + bool visitListExpression(ListExpression node) => + node.separator == ListSeparator.space && + !node.hasBrackets && + node.contents.length > 1 && + node.contents.every((expression) => expression.accept(this)); + + bool visitMapExpression(MapExpression node) => false; + + bool visitNullExpression(NullExpression node) => false; + + bool visitNumberExpression(NumberExpression node) => true; + + bool visitParenthesizedExpression(ParenthesizedExpression node) => + node.expression.accept(this); + + bool visitSelectorExpression(SelectorExpression node) => false; + + bool visitStringExpression(StringExpression node) { + if (node.hasQuotes) return false; + + // Exclude non-identifier constructs that are parsed as [StringExpression]s. + // We could just check if they parse as valid identifiers, but this is + // cheaper. + var text = node.text.initialPlain; + return + // !important + !text.startsWith("!") && + // ID-style identifiers + !text.startsWith("#") && + // Unicode ranges + text.codeUnitAtOrNull(1) != $plus && + // url() + text.codeUnitAtOrNull(3) != $lparen; + } + + bool visitSupportsExpression(SupportsExpression node) => false; + + bool visitUnaryOperationExpression(UnaryOperationExpression node) => false; + + bool visitValueExpression(ValueExpression node) => false; + + bool visitVariableExpression(VariableExpression node) => true; +} diff --git a/lib/src/ast/sass/expression/binary_operation.dart b/lib/src/ast/sass/expression/binary_operation.dart index 4e87fe8de..dc750900a 100644 --- a/lib/src/ast/sass/expression/binary_operation.dart +++ b/lib/src/ast/sass/expression/binary_operation.dart @@ -6,6 +6,7 @@ import 'package:charcode/charcode.dart'; import 'package:meta/meta.dart'; import 'package:source_span/source_span.dart'; +import '../../../util/span.dart'; import '../../../visitor/interface/expression.dart'; import '../expression.dart'; import 'list.dart'; @@ -45,6 +46,17 @@ final class BinaryOperationExpression implements Expression { return left.span.expand(right.span); } + /// Returns the span that covers only [operator]. + /// + /// @nodoc + @internal + FileSpan get operatorSpan => left.span.file == right.span.file && + left.span.end.offset < right.span.start.offset + ? left.span.file + .span(left.span.end.offset, right.span.start.offset) + .trim() + : span; + BinaryOperationExpression(this.operator, this.left, this.right) : allowsSlash = false; diff --git a/lib/src/ast/sass/expression/calculation.dart b/lib/src/ast/sass/expression/calculation.dart deleted file mode 100644 index 38c25ed14..000000000 --- a/lib/src/ast/sass/expression/calculation.dart +++ /dev/null @@ -1,108 +0,0 @@ -// Copyright 2021 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:meta/meta.dart'; -import 'package:source_span/source_span.dart'; - -import '../../../visitor/interface/expression.dart'; -import '../expression.dart'; -import 'binary_operation.dart'; -import 'function.dart'; -import 'if.dart'; -import 'number.dart'; -import 'parenthesized.dart'; -import 'string.dart'; -import 'variable.dart'; - -/// A calculation literal. -/// -/// {@category AST} -final class CalculationExpression implements Expression { - /// This calculation's name. - final String name; - - /// The arguments for the calculation. - final List arguments; - - final FileSpan span; - - /// Returns a `calc()` calculation expression. - CalculationExpression.calc(Expression argument, FileSpan span) - : this("calc", [argument], span); - - /// Returns a `min()` calculation expression. - CalculationExpression.min(Iterable arguments, this.span) - : name = "min", - arguments = _verifyArguments(arguments) { - if (this.arguments.isEmpty) { - throw ArgumentError("min() requires at least one argument."); - } - } - - /// Returns a `max()` calculation expression. - CalculationExpression.max(Iterable arguments, this.span) - : name = "max", - arguments = _verifyArguments(arguments) { - if (this.arguments.isEmpty) { - throw ArgumentError("max() requires at least one argument."); - } - } - - /// Returns a `clamp()` calculation expression. - CalculationExpression.clamp( - Expression min, Expression value, Expression max, FileSpan span) - : this("clamp", [min, max, value], span); - - /// Returns a calculation expression with the given name and arguments. - /// - /// Unlike the other constructors, this doesn't verify that the arguments are - /// valid for the name. - @internal - CalculationExpression(this.name, Iterable arguments, this.span) - : arguments = _verifyArguments(arguments); - - /// Throws an [ArgumentError] if [arguments] aren't valid calculation - /// arguments, and returns them as an unmodifiable list if they are. - static List _verifyArguments(Iterable arguments) => - List.unmodifiable(arguments.map((arg) { - _verify(arg); - return arg; - })); - - /// Throws an [ArgumentError] if [expression] isn't a valid calculation - /// argument. - static void _verify(Expression expression) { - switch (expression) { - case NumberExpression() || - CalculationExpression() || - VariableExpression() || - FunctionExpression() || - IfExpression() || - StringExpression(hasQuotes: false): - break; - - case ParenthesizedExpression(:var expression): - _verify(expression); - - case BinaryOperationExpression( - :var left, - :var right, - operator: BinaryOperator.plus || - BinaryOperator.minus || - BinaryOperator.times || - BinaryOperator.dividedBy - ): - _verify(left); - _verify(right); - - case _: - throw ArgumentError("Invalid calculation argument $expression."); - } - } - - T accept(ExpressionVisitor visitor) => - visitor.visitCalculationExpression(this); - - String toString() => "$name(${arguments.join(', ')})"; -} diff --git a/lib/src/ast/sass/interpolation.dart b/lib/src/ast/sass/interpolation.dart index 578394e83..075b3344f 100644 --- a/lib/src/ast/sass/interpolation.dart +++ b/lib/src/ast/sass/interpolation.dart @@ -21,6 +21,9 @@ final class Interpolation implements SassNode { final FileSpan span; + /// Returns whether this contains no interpolated expressions. + bool get isPlain => asPlain != null; + /// If this contains no interpolated expressions, returns its text contents. /// /// Otherwise, returns `null`. diff --git a/lib/src/deprecation.dart b/lib/src/deprecation.dart index 5ea363008..007fbb152 100644 --- a/lib/src/deprecation.dart +++ b/lib/src/deprecation.dart @@ -69,6 +69,11 @@ enum Deprecation { deprecatedIn: '1.62.3', description: 'Passing null as alpha in the ${isJS ? 'JS' : 'Dart'} API.'), + calcInterp('calc-interp', + deprecatedIn: '1.67.0', + description: 'Using interpolation in a calculation outside a value ' + 'position.'), + /// Deprecation for `@import` rules. import.future('import', description: '@import rules.'), diff --git a/lib/src/embedded/protofier.dart b/lib/src/embedded/protofier.dart index 3a1a792b0..6ad083ca4 100644 --- a/lib/src/embedded/protofier.dart +++ b/lib/src/embedded/protofier.dart @@ -134,8 +134,6 @@ final class Protofier { ..operator = _protofyCalculationOperator(value.operator) ..left = _protofyCalculationValue(value.left) ..right = _protofyCalculationValue(value.right); - case CalculationInterpolation(): - result.interpolation = value.value; case _: throw "Unknown calculation value $value"; } @@ -352,7 +350,7 @@ final class Protofier { _deprotofyCalculationValue(value.operation.left), _deprotofyCalculationValue(value.operation.right)), Value_Calculation_CalculationValue_Value.interpolation => - CalculationInterpolation(value.interpolation), + SassString('(${value.interpolation})', quotes: false), Value_Calculation_CalculationValue_Value.notSet => throw mandatoryError("Value.Calculation.value") }; diff --git a/lib/src/functions/math.dart b/lib/src/functions/math.dart index a85e5b1a4..bca609d0d 100644 --- a/lib/src/functions/math.dart +++ b/lib/src/functions/math.dart @@ -12,6 +12,7 @@ import '../deprecation.dart'; import '../evaluation_context.dart'; import '../exception.dart'; import '../module/built_in.dart'; +import '../util/number.dart'; import '../value.dart'; /// The global definitions of Sass math functions. @@ -149,87 +150,32 @@ final _log = _function("log", r"$number, $base: null", (arguments) { final _pow = _function("pow", r"$base, $exponent", (arguments) { var base = arguments[0].assertNumber("base"); var exponent = arguments[1].assertNumber("exponent"); - if (base.hasUnits) { - throw SassScriptException("\$base: Expected $base to have no units."); - } else if (exponent.hasUnits) { - throw SassScriptException( - "\$exponent: Expected $exponent to have no units."); - } else { - return SassNumber(math.pow(base.value, exponent.value)); - } + return pow(base, exponent); }); -final _sqrt = _function("sqrt", r"$number", (arguments) { - var number = arguments[0].assertNumber("number"); - if (number.hasUnits) { - throw SassScriptException("\$number: Expected $number to have no units."); - } else { - return SassNumber(math.sqrt(number.value)); - } -}); +final _sqrt = _singleArgumentMathFunc("sqrt", sqrt); /// /// Trigonometric functions /// -final _acos = _function("acos", r"$number", (arguments) { - var number = arguments[0].assertNumber("number"); - if (number.hasUnits) { - throw SassScriptException("\$number: Expected $number to have no units."); - } else { - return SassNumber.withUnits(math.acos(number.value) * 180 / math.pi, - numeratorUnits: ['deg']); - } -}); +final _acos = _singleArgumentMathFunc("acos", acos); -final _asin = _function("asin", r"$number", (arguments) { - var number = arguments[0].assertNumber("number"); - if (number.hasUnits) { - throw SassScriptException("\$number: Expected $number to have no units."); - } else { - return SassNumber.withUnits(math.asin(number.value) * 180 / math.pi, - numeratorUnits: ['deg']); - } -}); +final _asin = _singleArgumentMathFunc("asin", asin); -final _atan = _function("atan", r"$number", (arguments) { - var number = arguments[0].assertNumber("number"); - if (number.hasUnits) { - throw SassScriptException("\$number: Expected $number to have no units."); - } else { - return SassNumber.withUnits(math.atan(number.value) * 180 / math.pi, - numeratorUnits: ['deg']); - } -}); +final _atan = _singleArgumentMathFunc("atan", atan); final _atan2 = _function("atan2", r"$y, $x", (arguments) { var y = arguments[0].assertNumber("y"); var x = arguments[1].assertNumber("x"); - return SassNumber.withUnits( - math.atan2(y.value, x.convertValueToMatch(y, 'x', 'y')) * 180 / math.pi, - numeratorUnits: ['deg']); + return atan2(y, x); }); -final _cos = _function( - "cos", - r"$number", - (arguments) => SassNumber(math.cos(arguments[0] - .assertNumber("number") - .coerceValueToUnit("rad", "number")))); - -final _sin = _function( - "sin", - r"$number", - (arguments) => SassNumber(math.sin(arguments[0] - .assertNumber("number") - .coerceValueToUnit("rad", "number")))); - -final _tan = _function( - "tan", - r"$number", - (arguments) => SassNumber(math.tan(arguments[0] - .assertNumber("number") - .coerceValueToUnit("rad", "number")))); +final _cos = _singleArgumentMathFunc("cos", cos); + +final _sin = _singleArgumentMathFunc("sin", sin); + +final _tan = _singleArgumentMathFunc("tan", tan); /// /// Unit functions @@ -305,6 +251,16 @@ final _div = _function("div", r"$number1, $number2", (arguments) { /// Helpers /// +/// Returns a [Callable] named [name] that calls a single argument +/// math function. +BuiltInCallable _singleArgumentMathFunc( + String name, SassNumber mathFunc(SassNumber value)) { + return _function(name, r"$number", (arguments) { + var number = arguments[0].assertNumber("number"); + return mathFunc(number); + }); +} + /// Returns a [Callable] named [name] that transforms a number's value /// using [transform] and preserves its units. BuiltInCallable _numberFunction(String name, double transform(double value)) { diff --git a/lib/src/js/value/calculation.dart b/lib/src/js/value/calculation.dart index 6154de77b..51dfadae8 100644 --- a/lib/src/js/value/calculation.dart +++ b/lib/src/js/value/calculation.dart @@ -93,7 +93,7 @@ final JSClass calculationOperationClass = () { _assertCalculationValue(left); _assertCalculationValue(right); return SassCalculation.operateInternal(operator, left, right, - inMinMax: false, simplify: false); + inLegacySassFunction: false, simplify: false); }); jsClass.defineMethods({ @@ -109,7 +109,7 @@ final JSClass calculationOperationClass = () { getJSClass(SassCalculation.operateInternal( CalculationOperator.plus, SassNumber(1), SassNumber(1), - inMinMax: false, simplify: false)) + inLegacySassFunction: false, simplify: false)) .injectSuperclass(jsClass); return jsClass; }(); diff --git a/lib/src/parse/css.dart b/lib/src/parse/css.dart index acbb51fe7..6ed9123b7 100644 --- a/lib/src/parse/css.dart +++ b/lib/src/parse/css.dart @@ -22,7 +22,11 @@ final _disallowedFunctionNames = ..remove("invert") ..remove("alpha") ..remove("opacity") - ..remove("saturate"); + ..remove("saturate") + ..remove("min") + ..remove("max") + ..remove("round") + ..remove("abs"); class CssParser extends ScssParser { bool get plainCss => true; @@ -96,6 +100,17 @@ class CssParser extends ScssParser { ], scanner.spanFrom(start)); } + ParenthesizedExpression parentheses() { + // Expressions are only allowed within calculations, but we verify this at + // evaluation time. + var start = scanner.state; + scanner.expectChar($lparen); + whitespace(); + var expression = expressionUntilComma(); + scanner.expectChar($rparen); + return ParenthesizedExpression(expression, scanner.spanFrom(start)); + } + Expression identifierLike() { var start = scanner.state; var identifier = interpolatedIdentifier(); @@ -107,6 +122,8 @@ class CssParser extends ScssParser { } var beforeArguments = scanner.state; + // `namespacedExpression()` is just here to throw a clearer error. + if (scanner.scanChar($dot)) return namespacedExpression(plain, start); if (!scanner.scanChar($lparen)) return StringExpression(identifier); var allowEmptySecondArg = lower == 'var'; @@ -132,10 +149,8 @@ class CssParser extends ScssParser { "This function isn't allowed in plain CSS.", scanner.spanFrom(start)); } - return InterpolatedFunctionExpression( - // Create a fake interpolation to force the function to be interpreted - // as plain CSS, rather than calling a user-defined function. - Interpolation([StringExpression(identifier)], identifier.span), + return FunctionExpression( + plain, ArgumentInvocation( arguments, const {}, scanner.spanFrom(beforeArguments)), scanner.spanFrom(start)); diff --git a/lib/src/parse/stylesheet.dart b/lib/src/parse/stylesheet.dart index 9e9f7b2ed..23c53952e 100644 --- a/lib/src/parse/stylesheet.dart +++ b/lib/src/parse/stylesheet.dart @@ -1821,8 +1821,13 @@ abstract class StylesheetParser extends Parser { void addOperator(BinaryOperator operator) { if (plainCss && - operator != BinaryOperator.dividedBy && - operator != BinaryOperator.singleEquals) { + operator != BinaryOperator.singleEquals && + // These are allowed in calculations, so we have to check them at + // evaluation time. + operator != BinaryOperator.plus && + operator != BinaryOperator.minus && + operator != BinaryOperator.times && + operator != BinaryOperator.dividedBy) { scanner.error("Operators aren't allowed in plain CSS.", position: scanner.position - operator.operator.length, length: operator.operator.length); @@ -1876,7 +1881,7 @@ abstract class StylesheetParser extends Parser { case $lparen: // Parenthesized numbers can't be slash-separated. - addSingleExpression(_parentheses()); + addSingleExpression(parentheses()); case $lbracket: addSingleExpression(_expression(bracketList: true)); @@ -2065,7 +2070,7 @@ abstract class StylesheetParser extends Parser { /// produces a potentially slash-separated number. bool _isSlashOperand(Expression expression) => expression is NumberExpression || - expression is CalculationExpression || + expression is FunctionExpression || (expression is BinaryOperationExpression && expression.allowsSlash); /// Consumes an expression that doesn't contain any top-level whitespace. @@ -2073,7 +2078,7 @@ abstract class StylesheetParser extends Parser { // Note: when adding a new case, make sure it's reflected in // [_lookingAtExpression] and [_expression]. null => scanner.error("Expected expression."), - $lparen => _parentheses(), + $lparen => parentheses(), $slash => _unaryOperation(), $dot => _number(), $lbracket => _expression(bracketList: true), @@ -2101,11 +2106,8 @@ abstract class StylesheetParser extends Parser { }; /// Consumes a parenthesized expression. - Expression _parentheses() { - if (plainCss) { - scanner.error("Parentheses aren't allowed in plain CSS.", length: 1); - } - + @protected + Expression parentheses() { var wasInParentheses = _inParentheses; _inParentheses = true; try { @@ -2601,17 +2603,12 @@ abstract class StylesheetParser extends Parser { /// [name]. @protected Expression? trySpecialFunction(String name, LineScannerState start) { - if (scanner.peekChar() == $lparen) { - if (_tryCalculation(name, start) case var calculation?) { - return calculation; - } - } - var normalized = unvendor(name); InterpolationBuffer buffer; switch (normalized) { - case "calc" || "element" || "expression" when scanner.scanChar($lparen): + case "calc" when normalized != name && scanner.scanChar($lparen): + case "element" || "expression" when scanner.scanChar($lparen): buffer = InterpolationBuffer() ..write(name) ..writeCharCode($lparen); @@ -2643,228 +2640,6 @@ abstract class StylesheetParser extends Parser { return StringExpression(buffer.interpolation(scanner.spanFrom(start))); } - /// If [name] is the name of a calculation expression, parses the - /// corresponding calculation and returns it. - /// - /// Assumes the scanner is positioned immediately before the opening - /// parenthesis of the argument list. - CalculationExpression? _tryCalculation(String name, LineScannerState start) { - assert(scanner.peekChar() == $lparen); - switch (name) { - case "calc": - var arguments = _calculationArguments(1); - return CalculationExpression(name, arguments, scanner.spanFrom(start)); - - case "min" || "max": - // min() and max() are parsed as calculations if possible, and otherwise - // are parsed as normal Sass functions. - var beforeArguments = scanner.state; - List arguments; - try { - arguments = _calculationArguments(); - } on FormatException catch (_) { - scanner.state = beforeArguments; - return null; - } - - return CalculationExpression(name, arguments, scanner.spanFrom(start)); - - case "clamp": - var arguments = _calculationArguments(3); - return CalculationExpression(name, arguments, scanner.spanFrom(start)); - - case _: - return null; - } - } - - /// Consumes and returns arguments for a calculation expression, including the - /// opening and closing parentheses. - /// - /// If [maxArgs] is passed, at most that many arguments are consumed. - /// Otherwise, any number greater than zero are consumed. - List _calculationArguments([int? maxArgs]) { - scanner.expectChar($lparen); - if (_tryCalculationInterpolation() case var interpolation?) { - scanner.expectChar($rparen); - return [interpolation]; - } - - whitespace(); - var arguments = [_calculationSum()]; - while ((maxArgs == null || arguments.length < maxArgs) && - scanner.scanChar($comma)) { - whitespace(); - arguments.add(_calculationSum()); - } - - scanner.expectChar($rparen, - name: arguments.length == maxArgs - ? '"+", "-", "*", "/", or ")"' - : '"+", "-", "*", "/", ",", or ")"'); - - return arguments; - } - - /// Parses a calculation operation or value expression. - Expression _calculationSum() { - var sum = _calculationProduct(); - - while (true) { - var next = scanner.peekChar(); - if (next != $plus && next != $minus) return sum; - - if (!scanner.peekChar(-1).isWhitespace || - !scanner.peekChar(1).isWhitespace) { - scanner.error( - '"+" and "-" must be surrounded by whitespace in calculations.'); - } - - scanner.readChar(); - whitespace(); - sum = BinaryOperationExpression( - next == $plus ? BinaryOperator.plus : BinaryOperator.minus, - sum, - _calculationProduct()); - } - } - - /// Parses a calculation product or value expression. - Expression _calculationProduct() { - var product = _calculationValue(); - - while (true) { - whitespace(); - var next = scanner.peekChar(); - if (next != $asterisk && next != $slash) return product; - - scanner.readChar(); - whitespace(); - product = BinaryOperationExpression( - next == $asterisk ? BinaryOperator.times : BinaryOperator.dividedBy, - product, - _calculationValue()); - } - } - - /// Parses a single calculation value. - Expression _calculationValue() { - switch (scanner.peekChar()) { - case $plus || $dot || int(isDigit: true): - return _number(); - case $dollar: - return _variable(); - case $lparen: - var start = scanner.state; - scanner.readChar(); - - Expression? value = _tryCalculationInterpolation(); - if (value == null) { - whitespace(); - value = _calculationSum(); - } - - whitespace(); - scanner.expectChar($rparen); - return ParenthesizedExpression(value, scanner.spanFrom(start)); - case _ when lookingAtIdentifier(): - var start = scanner.state; - var ident = identifier(); - if (scanner.scanChar($dot)) return namespacedExpression(ident, start); - if (scanner.peekChar() != $lparen) { - return StringExpression( - Interpolation([ident], scanner.spanFrom(start)), - quotes: false); - } - - var lowerCase = ident.toLowerCase(); - if (_tryCalculation(lowerCase, start) case var calculation?) { - return calculation; - } else if (lowerCase == "if") { - return IfExpression(_argumentInvocation(), scanner.spanFrom(start)); - } else { - return FunctionExpression( - ident, _argumentInvocation(), scanner.spanFrom(start)); - } - - // This has to go after [lookingAtIdentifier] because a hyphen can start - // an identifier as well as a number. - case $minus: - return _number(); - - case _: - scanner.error("Expected number, variable, function, or calculation."); - } - } - - /// If the following text up to the next unbalanced `")"`, `"]"`, or `"}"` - /// contains interpolation, parses that interpolation as an unquoted - /// [StringExpression] and returns it. - StringExpression? _tryCalculationInterpolation() => - _containsCalculationInterpolation() - ? StringExpression(_interpolatedDeclarationValue()) - : null; - - /// Returns whether the following text up to the next unbalanced `")"`, `"]"`, - /// or `"}"` contains interpolation. - bool _containsCalculationInterpolation() { - var parens = 0; - var brackets = []; - - var start = scanner.state; - while (!scanner.isDone) { - var next = scanner.peekChar(); - switch (next) { - case $backslash: - scanner.readChar(); - scanner.readChar(); - - case $slash: - if (!scanComment()) scanner.readChar(); - - case $single_quote || $double_quote: - interpolatedString(); - - case $hash: - if (parens == 0 && scanner.peekChar(1) == $lbrace) { - scanner.state = start; - return true; - } - scanner.readChar(); - - case $lparen: - parens++; - continue left; - - left: - case $lbrace: - case $lbracket: - // dart-lang/sdk#45357 - brackets.add(opposite(next!)); - scanner.readChar(); - - case $rparen: - parens--; - continue right; - - right: - case $rbrace: - case $rbracket: - if (brackets.isEmpty || brackets.removeLast() != next) { - scanner.state = start; - return false; - } - scanner.readChar(); - - case _: - scanner.readChar(); - } - } - - scanner.state = start; - return false; - } - /// Like [_urlContents], but returns `null` if the URL fails to parse. /// /// [start] is the position before the beginning of the name. [name] is the diff --git a/lib/src/util/nullable.dart b/lib/src/util/nullable.dart index 125f58d46..ad4a8ba2f 100644 --- a/lib/src/util/nullable.dart +++ b/lib/src/util/nullable.dart @@ -21,3 +21,10 @@ extension SetExtension on Set { return cast(); } } + +extension StringExtension on String { + /// Like [String.codeUnitAt], but returns `null` instead of throwing an error + /// if [index] is past the end of the string. + int? codeUnitAtOrNull(int index) => + index >= length ? null : codeUnitAt(index); +} diff --git a/lib/src/util/number.dart b/lib/src/util/number.dart index 80fd3aaa2..8df7beb1a 100644 --- a/lib/src/util/number.dart +++ b/lib/src/util/number.dart @@ -110,6 +110,11 @@ double fuzzyAssertRange(double number, int min, int max, [String? name]) { /// /// [floored division]: https://en.wikipedia.org/wiki/Modulo_operation#Variants_of_the_definition double moduloLikeSass(double num1, double num2) { + if (num1.isInfinite) return double.nan; + if (num2.isInfinite) { + return num1.signIncludingZero == num2.sign ? num1 : double.nan; + } + if (num2 > 0) return num1 % num2; if (num2 == 0) return double.nan; @@ -118,3 +123,76 @@ double moduloLikeSass(double num1, double num2) { var result = num1 % num2; return result == 0 ? 0 : result + num2; } + +/// Returns the square root of [number]. +SassNumber sqrt(SassNumber number) { + number.assertNoUnits("number"); + return SassNumber(math.sqrt(number.value)); +} + +/// Returns the sine of [number]. +SassNumber sin(SassNumber number) => + SassNumber(math.sin(number.coerceValueToUnit("rad", "number"))); + +/// Returns the cosine of [number]. +SassNumber cos(SassNumber number) => + SassNumber(math.cos(number.coerceValueToUnit("rad", "number"))); + +/// Returns the tangent of [number]. +SassNumber tan(SassNumber number) => + SassNumber(math.tan(number.coerceValueToUnit("rad", "number"))); + +/// Returns the arctangent of [number]. +SassNumber atan(SassNumber number) { + number.assertNoUnits("number"); + return _radiansToDegrees(math.atan(number.value)); +} + +/// Returns the arcsine of [number]. +SassNumber asin(SassNumber number) { + number.assertNoUnits("number"); + return _radiansToDegrees(math.asin(number.value)); +} + +/// Returns the arccosine of [number] +SassNumber acos(SassNumber number) { + number.assertNoUnits("number"); + return _radiansToDegrees(math.acos(number.value)); +} + +/// Returns the absolute value of [number]. +SassNumber abs(SassNumber number) => + SassNumber(number.value.abs()).coerceToMatch(number); + +/// Returns the logarithm of [number] with respect to [base]. +SassNumber log(SassNumber number, SassNumber? base) { + if (base != null) { + return SassNumber(math.log(number.value) / math.log(base.value)); + } + return SassNumber(math.log(number.value)); +} + +/// Returns the value of [base] raised to the power of [exponent]. +SassNumber pow(SassNumber base, SassNumber exponent) { + base.assertNoUnits("base"); + exponent.assertNoUnits("exponent"); + return SassNumber(math.pow(base.value, exponent.value)); +} + +/// Returns the arctangent for [y] and [x]. +SassNumber atan2(SassNumber y, SassNumber x) => + _radiansToDegrees(math.atan2(y.value, x.convertValueToMatch(y, 'x', 'y'))); + +/// Returns [radians] as a [SassNumber] with unit `deg`. +SassNumber _radiansToDegrees(double radians) => + SassNumber.withUnits(radians * (180 / math.pi), numeratorUnits: ['deg']); + +/// Extension methods to get the sign of the double's numerical value, +/// including positive and negative zero. +extension DoubleWithSignedZero on double { + double get signIncludingZero { + if (identical(this, -0.0)) return -1.0; + if (this == 0) return 1.0; + return sign; + } +} diff --git a/lib/src/value/calculation.dart b/lib/src/value/calculation.dart index b39011bf0..cbb8b92e6 100644 --- a/lib/src/value/calculation.dart +++ b/lib/src/value/calculation.dart @@ -2,11 +2,18 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. +import 'dart:math' as math; + +import 'package:charcode/charcode.dart'; import 'package:meta/meta.dart'; +import '../deprecation.dart'; +import '../evaluation_context.dart'; import '../exception.dart'; +import '../callable.dart'; +import '../util/character.dart'; import '../util/nullable.dart'; -import '../util/number.dart'; +import '../util/number.dart' as number_lib; import '../utils.dart'; import '../value.dart'; import '../visitor/interface/value.dart'; @@ -27,7 +34,7 @@ final class SassCalculation extends Value { /// The calculation's arguments. /// /// Each argument is either a [SassNumber], a [SassCalculation], an unquoted - /// [SassString], a [CalculationOperation], or a [CalculationInterpolation]. + /// [SassString], or a [CalculationOperation]. final List arguments; /// @nodoc @@ -44,8 +51,7 @@ final class SassCalculation extends Value { /// Creates a `calc()` calculation with the given [argument]. /// /// The [argument] must be either a [SassNumber], a [SassCalculation], an - /// unquoted [SassString], a [CalculationOperation], or a - /// [CalculationInterpolation]. + /// unquoted [SassString], or a [CalculationOperation]. /// /// This automatically simplifies the calculation, so it may return a /// [SassNumber] rather than a [SassCalculation]. It throws an exception if it @@ -60,8 +66,8 @@ final class SassCalculation extends Value { /// Creates a `min()` calculation with the given [arguments]. /// /// Each argument must be either a [SassNumber], a [SassCalculation], an - /// unquoted [SassString], a [CalculationOperation], or a - /// [CalculationInterpolation]. It must be passed at least one argument. + /// unquoted [SassString], or a [CalculationOperation]. It must be passed at + /// least one argument. /// /// This automatically simplifies the calculation, so it may return a /// [SassNumber] rather than a [SassCalculation]. It throws an exception if it @@ -91,8 +97,8 @@ final class SassCalculation extends Value { /// Creates a `max()` calculation with the given [arguments]. /// /// Each argument must be either a [SassNumber], a [SassCalculation], an - /// unquoted [SassString], a [CalculationOperation], or a - /// [CalculationInterpolation]. It must be passed at least one argument. + /// unquoted [SassString], or a [CalculationOperation]. It must be passed at + /// least one argument. /// /// This automatically simplifies the calculation, so it may return a /// [SassNumber] rather than a [SassCalculation]. It throws an exception if it @@ -119,11 +125,181 @@ final class SassCalculation extends Value { return SassCalculation._("max", args); } + /// Creates a `hypot()` calculation with the given [arguments]. + /// + /// Each argument must be either a [SassNumber], a [SassCalculation], an + /// unquoted [SassString], or a [CalculationOperation]. It must be passed at + /// least one argument. + /// + /// This automatically simplifies the calculation, so it may return a + /// [SassNumber] rather than a [SassCalculation]. It throws an exception if it + /// can determine that the calculation will definitely produce invalid CSS. + static Value hypot(Iterable arguments) { + var args = _simplifyArguments(arguments); + if (args.isEmpty) { + throw ArgumentError("hypot() must have at least one argument."); + } + _verifyCompatibleNumbers(args); + + var subtotal = 0.0; + var first = args.first; + if (first is! SassNumber || first.hasUnit('%')) { + return SassCalculation._("hypot", args); + } + for (var i = 0; i < args.length; i++) { + var number = args.elementAt(i); + if (number is! SassNumber || !number.hasCompatibleUnits(first)) { + return SassCalculation._("hypot", args); + } + var value = + number.convertValueToMatch(first, "numbers[${i + 1}]", "numbers[1]"); + subtotal += value * value; + } + return SassNumber.withUnits(math.sqrt(subtotal), + numeratorUnits: first.numeratorUnits, + denominatorUnits: first.denominatorUnits); + } + + /// Creates a `sqrt()` calculation with the given [argument]. + /// + /// The [argument] must be either a [SassNumber], a [SassCalculation], an + /// unquoted [SassString], or a [CalculationOperation]. + /// + /// This automatically simplifies the calculation, so it may return a + /// [SassNumber] rather than a [SassCalculation]. It throws an exception if it + /// can determine that the calculation will definitely produce invalid CSS. + static Value sqrt(Object argument) => + _singleArgument("sqrt", argument, number_lib.sqrt, forbidUnits: true); + + /// Creates a `sin()` calculation with the given [argument]. + /// + /// The [argument] must be either a [SassNumber], a [SassCalculation], an + /// unquoted [SassString], or a [CalculationOperation]. + /// + /// This automatically simplifies the calculation, so it may return a + /// [SassNumber] rather than a [SassCalculation]. It throws an exception if it + /// can determine that the calculation will definitely produce invalid CSS. + static Value sin(Object argument) => + _singleArgument("sin", argument, number_lib.sin); + + /// Creates a `cos()` calculation with the given [argument]. + /// + /// The [argument] must be either a [SassNumber], a [SassCalculation], an + /// unquoted [SassString], or a [CalculationOperation]. + /// + /// This automatically simplifies the calculation, so it may return a + /// [SassNumber] rather than a [SassCalculation]. It throws an exception if it + /// can determine that the calculation will definitely produce invalid CSS. + static Value cos(Object argument) => + _singleArgument("cos", argument, number_lib.cos); + + /// Creates a `tan()` calculation with the given [argument]. + /// + /// The [argument] must be either a [SassNumber], a [SassCalculation], an + /// unquoted [SassString], or a [CalculationOperation]. + /// + /// This automatically simplifies the calculation, so it may return a + /// [SassNumber] rather than a [SassCalculation]. It throws an exception if it + /// can determine that the calculation will definitely produce invalid CSS. + static Value tan(Object argument) => + _singleArgument("tan", argument, number_lib.tan); + + /// Creates an `atan()` calculation with the given [argument]. + /// + /// The [argument] must be either a [SassNumber], a [SassCalculation], an + /// unquoted [SassString], or a [CalculationOperation]. + /// + /// This automatically simplifies the calculation, so it may return a + /// [SassNumber] rather than a [SassCalculation]. It throws an exception if it + /// can determine that the calculation will definitely produce invalid CSS. + static Value atan(Object argument) => + _singleArgument("atan", argument, number_lib.atan, forbidUnits: true); + + /// Creates an `asin()` calculation with the given [argument]. + /// + /// The [argument] must be either a [SassNumber], a [SassCalculation], an + /// unquoted [SassString], or a [CalculationOperation]. + /// + /// This automatically simplifies the calculation, so it may return a + /// [SassNumber] rather than a [SassCalculation]. It throws an exception if it + /// can determine that the calculation will definitely produce invalid CSS. + static Value asin(Object argument) => + _singleArgument("asin", argument, number_lib.asin, forbidUnits: true); + + /// Creates an `acos()` calculation with the given [argument]. + /// + /// The [argument] must be either a [SassNumber], a [SassCalculation], an + /// unquoted [SassString], or a [CalculationOperation]. + /// + /// This automatically simplifies the calculation, so it may return a + /// [SassNumber] rather than a [SassCalculation]. It throws an exception if it + /// can determine that the calculation will definitely produce invalid CSS. + static Value acos(Object argument) => + _singleArgument("acos", argument, number_lib.acos, forbidUnits: true); + + /// Creates an `abs()` calculation with the given [argument]. + /// + /// The [argument] must be either a [SassNumber], a [SassCalculation], an + /// unquoted [SassString], or a [CalculationOperation]. + /// + /// This automatically simplifies the calculation, so it may return a + /// [SassNumber] rather than a [SassCalculation]. It throws an exception if it + /// can determine that the calculation will definitely produce invalid CSS. + static Value abs(Object argument) { + argument = _simplify(argument); + if (argument is! SassNumber) return SassCalculation._("abs", [argument]); + if (argument.hasUnit("%")) { + warnForDeprecation( + "Passing percentage units to the global abs() function is deprecated.\n" + "In the future, this will emit a CSS abs() function to be resolved by the browser.\n" + "To preserve current behavior: math.abs($argument)" + "\n" + "To emit a CSS abs() now: abs(#{$argument})\n" + "More info: https://sass-lang.com/d/abs-percent", + Deprecation.absPercent); + } + return number_lib.abs(argument); + } + + /// Creates an `exp()` calculation with the given [argument]. + /// + /// The [argument] must be either a [SassNumber], a [SassCalculation], an + /// unquoted [SassString], or a [CalculationOperation]. + /// + /// This automatically simplifies the calculation, so it may return a + /// [SassNumber] rather than a [SassCalculation]. It throws an exception if it + /// can determine that the calculation will definitely produce invalid CSS. + static Value exp(Object argument) { + argument = _simplify(argument); + if (argument is! SassNumber) { + return SassCalculation._("exp", [argument]); + } + argument.assertNoUnits(); + return number_lib.pow(SassNumber(math.e), argument); + } + + /// Creates a `sign()` calculation with the given [argument]. + /// + /// The [argument] must be either a [SassNumber], a [SassCalculation], an + /// unquoted [SassString], or a [CalculationOperation]. + /// + /// This automatically simplifies the calculation, so it may return a + /// [SassNumber] rather than a [SassCalculation]. It throws an exception if it + /// can determine that the calculation will definitely produce invalid CSS. + static Value sign(Object argument) { + argument = _simplify(argument); + return switch (argument) { + SassNumber(value: double(isNaN: true) || 0) => argument, + SassNumber arg when !arg.hasUnit('%') => + SassNumber(arg.value.sign).coerceToMatch(argument), + _ => SassCalculation._("sign", [argument]), + }; + } + /// Creates a `clamp()` calculation with the given [min], [value], and [max]. /// /// Each argument must be either a [SassNumber], a [SassCalculation], an - /// unquoted [SassString], a [CalculationOperation], or a - /// [CalculationInterpolation]. + /// unquoted [SassString], or a [CalculationOperation]. /// /// This automatically simplifies the calculation, so it may return a /// [SassNumber] rather than a [SassCalculation]. It throws an exception if it @@ -157,6 +333,246 @@ final class SassCalculation extends Value { return SassCalculation._("clamp", args); } + /// Creates a `pow()` calculation with the given [base] and [exponent]. + /// + /// Each argument must be either a [SassNumber], a [SassCalculation], an + /// unquoted [SassString], or a [CalculationOperation]. + /// + /// This automatically simplifies the calculation, so it may return a + /// [SassNumber] rather than a [SassCalculation]. It throws an exception if it + /// can determine that the calculation will definitely produce invalid CSS. + /// + /// This may be passed fewer than two arguments, but only if one of the + /// arguments is an unquoted `var()` string. + static Value pow(Object base, Object? exponent) { + var args = [base, if (exponent != null) exponent]; + _verifyLength(args, 2); + base = _simplify(base); + exponent = exponent.andThen(_simplify); + if (base is! SassNumber || exponent is! SassNumber) { + return SassCalculation._("pow", args); + } + base.assertNoUnits(); + exponent.assertNoUnits(); + return number_lib.pow(base, exponent); + } + + /// Creates a `log()` calculation with the given [number] and [base]. + /// + /// Each argument must be either a [SassNumber], a [SassCalculation], an + /// unquoted [SassString], or a [CalculationOperation]. + /// + /// This automatically simplifies the calculation, so it may return a + /// [SassNumber] rather than a [SassCalculation]. It throws an exception if it + /// can determine that the calculation will definitely produce invalid CSS. + /// + /// If arguments contains exactly a single argument, the base is set to + /// `math.e` by default. + static Value log(Object number, Object? base) { + number = _simplify(number); + base = base.andThen(_simplify); + var args = [number, if (base != null) base]; + if (number is! SassNumber || (base != null && base is! SassNumber)) { + return SassCalculation._("log", args); + } + number.assertNoUnits(); + if (base is SassNumber) { + base.assertNoUnits(); + return number_lib.log(number, base); + } + return number_lib.log(number, null); + } + + /// Creates a `atan2()` calculation for [y] and [x]. + /// + /// Each argument must be either a [SassNumber], a [SassCalculation], an + /// unquoted [SassString], or a [CalculationOperation]. + /// + /// This automatically simplifies the calculation, so it may return a + /// [SassNumber] rather than a [SassCalculation]. It throws an exception if it + /// can determine that the calculation will definitely produce invalid CSS. + /// + /// This may be passed fewer than two arguments, but only if one of the + /// arguments is an unquoted `var()` string. + static Value atan2(Object y, Object? x) { + y = _simplify(y); + x = x.andThen(_simplify); + var args = [y, if (x != null) x]; + _verifyLength(args, 2); + _verifyCompatibleNumbers(args); + if (y is! SassNumber || + x is! SassNumber || + y.hasUnit('%') || + x.hasUnit('%') || + !y.hasCompatibleUnits(x)) { + return SassCalculation._("atan2", args); + } + return number_lib.atan2(y, x); + } + + /// Creates a `rem()` calculation with the given [dividend] and [modulus]. + /// + /// Each argument must be either a [SassNumber], a [SassCalculation], an + /// unquoted [SassString], or a [CalculationOperation]. + /// + /// This automatically simplifies the calculation, so it may return a + /// [SassNumber] rather than a [SassCalculation]. It throws an exception if it + /// can determine that the calculation will definitely produce invalid CSS. + /// + /// This may be passed fewer than two arguments, but only if one of the + /// arguments is an unquoted `var()` string. + static Value rem(Object dividend, Object? modulus) { + dividend = _simplify(dividend); + modulus = modulus.andThen(_simplify); + var args = [dividend, if (modulus != null) modulus]; + _verifyLength(args, 2); + _verifyCompatibleNumbers(args); + if (dividend is! SassNumber || + modulus is! SassNumber || + !dividend.hasCompatibleUnits(modulus)) { + return SassCalculation._("rem", args); + } + var result = dividend.modulo(modulus); + if (modulus.value.signIncludingZero != dividend.value.signIncludingZero) { + if (modulus.value.isInfinite) return dividend; + if (result.value == 0) { + return result.unaryMinus(); + } + return result.minus(modulus); + } + return result; + } + + /// Creates a `mod()` calculation with the given [dividend] and [modulus]. + /// + /// Each argument must be either a [SassNumber], a [SassCalculation], an + /// unquoted [SassString], or a [CalculationOperation]. + /// + /// This automatically simplifies the calculation, so it may return a + /// [SassNumber] rather than a [SassCalculation]. It throws an exception if it + /// can determine that the calculation will definitely produce invalid CSS. + /// + /// This may be passed fewer than two arguments, but only if one of the + /// arguments is an unquoted `var()` string. + static Value mod(Object dividend, Object? modulus) { + dividend = _simplify(dividend); + modulus = modulus.andThen(_simplify); + var args = [dividend, if (modulus != null) modulus]; + _verifyLength(args, 2); + _verifyCompatibleNumbers(args); + if (dividend is! SassNumber || + modulus is! SassNumber || + !dividend.hasCompatibleUnits(modulus)) { + return SassCalculation._("mod", args); + } + return dividend.modulo(modulus); + } + + /// Creates a `round()` calculation with the given [strategyOrNumber], + /// [numberOrStep], and [step]. Strategy must be either nearest, up, down or + /// to-zero. + /// + /// Number and step must be either a [SassNumber], a [SassCalculation], an + /// unquoted [SassString], or a [CalculationOperation]. + /// + /// This automatically simplifies the calculation, so it may return a + /// [SassNumber] rather than a [SassCalculation]. It throws an exception if it + /// can determine that the calculation will definitely produce invalid CSS. + /// + /// This may be passed fewer than two arguments, but only if one of the + /// arguments is an unquoted `var()` string. + static Value round(Object strategyOrNumber, + [Object? numberOrStep, Object? step]) { + switch (( + _simplify(strategyOrNumber), + numberOrStep.andThen(_simplify), + step.andThen(_simplify) + )) { + case (SassNumber number, null, null): + return _matchUnits(number.value.round().toDouble(), number); + + case (SassNumber number, SassNumber step, null) + when !number.hasCompatibleUnits(step): + _verifyCompatibleNumbers([number, step]); + return SassCalculation._("round", [number, step]); + + case (SassNumber number, SassNumber step, null): + _verifyCompatibleNumbers([number, step]); + return _roundWithStep('nearest', number, step); + + case ( + SassString(text: 'nearest' || 'up' || 'down' || 'to-zero') && + var strategy, + SassNumber number, + SassNumber step + ) + when !number.hasCompatibleUnits(step): + _verifyCompatibleNumbers([number, step]); + return SassCalculation._("round", [strategy, number, step]); + + case ( + SassString(text: 'nearest' || 'up' || 'down' || 'to-zero') && + var strategy, + SassNumber number, + SassNumber step + ): + _verifyCompatibleNumbers([number, step]); + return _roundWithStep(strategy.text, number, step); + + case ( + SassString(text: 'nearest' || 'up' || 'down' || 'to-zero') && + var strategy, + SassString rest, + null + ): + return SassCalculation._("round", [strategy, rest]); + + case ( + SassString(text: 'nearest' || 'up' || 'down' || 'to-zero'), + _?, + null + ): + throw SassScriptException("If strategy is not null, step is required."); + + case ( + SassString(text: 'nearest' || 'up' || 'down' || 'to-zero'), + null, + null + ): + throw SassScriptException( + "Number to round and step arguments are required."); + + case (SassString rest, null, null): + return SassCalculation._("round", [rest]); + + case (var number, null, null): + throw SassScriptException( + "Single argument $number expected to be simplifiable."); + + case (var number, var step?, null): + return SassCalculation._("round", [number, step]); + + case ( + (SassString(text: 'nearest' || 'up' || 'down' || 'to-zero') || + SassString(isVar: true)) && + var strategy, + var number?, + var step? + ): + return SassCalculation._("round", [strategy, number, step]); + + case (_, _?, _?): + throw SassScriptException( + "$strategyOrNumber must be either nearest, up, down or to-zero."); + + case (_, null, _?): + // TODO(pamelalozano): Get rid of this case once dart-lang/sdk#52908 is solved. + // ignore: unreachable_switch_case + case (_, _, _): + throw SassScriptException("Invalid parameters."); + } + } + /// Creates and simplifies a [CalculationOperation] with the given [operator], /// [left], and [right]. /// @@ -164,15 +580,15 @@ final class SassCalculation extends Value { /// [SassNumber] rather than a [CalculationOperation]. /// /// Each of [left] and [right] must be either a [SassNumber], a - /// [SassCalculation], an unquoted [SassString], a [CalculationOperation], or - /// a [CalculationInterpolation]. + /// [SassCalculation], an unquoted [SassString], or a [CalculationOperation]. static Object operate( CalculationOperator operator, Object left, Object right) => - operateInternal(operator, left, right, inMinMax: false, simplify: true); + operateInternal(operator, left, right, + inLegacySassFunction: false, simplify: true); - /// Like [operate], but with the internal-only [inMinMax] parameter. + /// Like [operate], but with the internal-only [inLegacySassFunction] parameter. /// - /// If [inMinMax] is `true`, this allows unitless numbers to be added and + /// If [inLegacySassFunction] is `true`, this allows unitless numbers to be added and /// subtracted with numbers with units, for backwards-compatibility with the /// old global `min()` and `max()` functions. /// @@ -180,7 +596,7 @@ final class SassCalculation extends Value { @internal static Object operateInternal( CalculationOperator operator, Object left, Object right, - {required bool inMinMax, required bool simplify}) { + {required bool inLegacySassFunction, required bool simplify}) { if (!simplify) return CalculationOperation._(operator, left, right); left = _simplify(left); right = _simplify(right); @@ -188,7 +604,7 @@ final class SassCalculation extends Value { if (operator case CalculationOperator.plus || CalculationOperator.minus) { if (left is SassNumber && right is SassNumber && - (inMinMax + (inLegacySassFunction ? left.isComparableTo(right) : left.hasCompatibleUnits(right))) { return operator == CalculationOperator.plus @@ -198,7 +614,7 @@ final class SassCalculation extends Value { _verifyCompatibleNumbers([left, right]); - if (right is SassNumber && fuzzyLessThan(right.value, 0)) { + if (right is SassNumber && number_lib.fuzzyLessThan(right.value, 0)) { right = right.times(SassNumber(-1)); operator = operator == CalculationOperator.plus ? CalculationOperator.minus @@ -219,19 +635,88 @@ final class SassCalculation extends Value { /// simplification. SassCalculation._(this.name, this.arguments); + // Returns [value] coerced to [number]'s units. + static SassNumber _matchUnits(double value, SassNumber number) => + SassNumber.withUnits(value, + numeratorUnits: number.numeratorUnits, + denominatorUnits: number.denominatorUnits); + + /// Returns a rounded [number] based on a selected rounding [strategy], + /// to the nearest integer multiple of [step]. + static SassNumber _roundWithStep( + String strategy, SassNumber number, SassNumber step) { + if (!{'nearest', 'up', 'down', 'to-zero'}.contains(strategy)) { + throw ArgumentError( + "$strategy must be either nearest, up, down or to-zero."); + } + + if (number.value.isInfinite && step.value.isInfinite || + step.value == 0 || + number.value.isNaN || + step.value.isNaN) { + return _matchUnits(double.nan, number); + } + if (number.value.isInfinite) return number; + + if (step.value.isInfinite) { + return switch ((strategy, number.value)) { + (_, 0) => number, + ('nearest' || 'to-zero', > 0) => _matchUnits(0.0, number), + ('nearest' || 'to-zero', _) => _matchUnits(-0.0, number), + ('up', > 0) => _matchUnits(double.infinity, number), + ('up', _) => _matchUnits(-0.0, number), + ('down', < 0) => _matchUnits(-double.infinity, number), + ('down', _) => _matchUnits(0, number), + (_, _) => throw UnsupportedError("Invalid argument: $strategy.") + }; + } + + var stepWithNumberUnit = step.convertValueToMatch(number); + return switch (strategy) { + 'nearest' => _matchUnits( + (number.value / stepWithNumberUnit).round() * stepWithNumberUnit, + number), + 'up' => _matchUnits( + (step.value < 0 + ? (number.value / stepWithNumberUnit).floor() + : (number.value / stepWithNumberUnit).ceil()) * + stepWithNumberUnit, + number), + 'down' => _matchUnits( + (step.value < 0 + ? (number.value / stepWithNumberUnit).ceil() + : (number.value / stepWithNumberUnit).floor()) * + stepWithNumberUnit, + number), + 'to-zero' => number.value < 0 + ? _matchUnits( + (number.value / stepWithNumberUnit).ceil() * stepWithNumberUnit, + number) + : _matchUnits( + (number.value / stepWithNumberUnit).floor() * stepWithNumberUnit, + number), + _ => _matchUnits(double.nan, number) + }; + } + /// Returns an unmodifiable list of [args], with each argument simplified. static List _simplifyArguments(Iterable args) => List.unmodifiable(args.map(_simplify)); /// Simplifies a calculation argument. static Object _simplify(Object arg) => switch (arg) { - SassNumber() || - CalculationInterpolation() || - CalculationOperation() => - arg, + SassNumber() || CalculationOperation() => arg, + CalculationInterpolation() => + SassString('(${arg.value})', quotes: false), SassString(hasQuotes: false) => arg, SassString() => throw SassScriptException( "Quoted string $arg can't be used in a calculation."), + SassCalculation( + name: 'calc', + arguments: [SassString(hasQuotes: false, :var text)] + ) + when _needsParentheses(text) => + SassString('($text)', quotes: false), SassCalculation(name: 'calc', arguments: [var value]) => value, SassCalculation() => arg, Value() => throw SassScriptException( @@ -239,6 +724,40 @@ final class SassCalculation extends Value { _ => throw ArgumentError("Unexpected calculation argument $arg.") }; + /// Returns whether [text] needs parentheses if it's the contents of a + /// `calc()` being embedded in another calculation. + static bool _needsParentheses(String text) { + var first = text.codeUnitAt(0); + if (_charNeedsParentheses(first)) return true; + var couldBeVar = text.length >= 4 && characterEqualsIgnoreCase(first, $v); + + if (text.length < 2) return false; + var second = text.codeUnitAt(1); + if (_charNeedsParentheses(second)) return true; + couldBeVar = couldBeVar && characterEqualsIgnoreCase(second, $a); + + if (text.length < 3) return false; + var third = text.codeUnitAt(2); + if (_charNeedsParentheses(third)) return true; + couldBeVar = couldBeVar && characterEqualsIgnoreCase(third, $r); + + if (text.length < 4) return false; + var fourth = text.codeUnitAt(3); + if (couldBeVar && fourth == $lparen) return true; + if (_charNeedsParentheses(fourth)) return true; + + for (var i = 4; i < text.length; i++) { + if (_charNeedsParentheses(text.codeUnitAt(i))) return true; + } + return false; + } + + /// Returns whether [character] intrinsically needs parentheses if it appears + /// in the unquoted string argument of a `calc()` being embedded in another + /// calculation. + static bool _charNeedsParentheses(int character) => + character.isWhitespace || character == $slash || character == $asterisk; + /// Verifies that all the numbers in [args] aren't known to be incompatible /// with one another, and that they don't have units that are too complex for /// calculations. @@ -267,11 +786,10 @@ final class SassCalculation extends Value { } /// Throws a [SassScriptException] if [args] isn't [expectedLength] *and* - /// doesn't contain either a [SassString] or a [CalculationInterpolation]. + /// doesn't contain a [SassString]. static void _verifyLength(List args, int expectedLength) { if (args.length == expectedLength) return; - if (args - .any((arg) => arg is SassString || arg is CalculationInterpolation)) { + if (args.any((arg) => arg is SassString)) { return; } throw SassScriptException( @@ -279,6 +797,21 @@ final class SassCalculation extends Value { "${pluralize('was', args.length, plural: 'were')} passed."); } + /// Returns a [Callable] named [name] that calls a single argument + /// math function. + /// + /// If [forbidUnits] is `true` it will throw an error if [argument] has units. + static Value _singleArgument( + String name, Object argument, SassNumber mathFunc(SassNumber value), + {bool forbidUnits = false}) { + argument = _simplify(argument); + if (argument is! SassNumber) { + return SassCalculation._(name, [argument]); + } + if (forbidUnits) argument.assertNoUnits(); + return mathFunc(argument); + } + /// @nodoc @internal T accept(ValueVisitor visitor) => visitor.visitCalculation(this); @@ -329,14 +862,14 @@ final class CalculationOperation { /// The left-hand operand. /// /// This is either a [SassNumber], a [SassCalculation], an unquoted - /// [SassString], a [CalculationOperation], or a [CalculationInterpolation]. + /// [SassString], or a [CalculationOperation]. Object get left => _left; final Object _left; /// The right-hand operand. /// /// This is either a [SassNumber], a [SassCalculation], an unquoted - /// [SassString], a [CalculationOperation], or a [CalculationInterpolation]. + /// [SassString], or a [CalculationOperation]. Object get right => _right; final Object _right; @@ -392,12 +925,15 @@ enum CalculationOperator { String toString() => name; } -/// A string injected into a [SassCalculation] using interpolation. +/// A deprecated representation of a string injected into a [SassCalculation] +/// using interpolation. /// -/// This is tracked separately from string arguments because it requires -/// additional parentheses when used as an operand of a [CalculationOperation]. +/// This only exists for backwards-compatibility with an older version of Dart +/// Sass. It's now equivalent to creating a `SassString` whose value is wrapped +/// in parentheses. /// /// {@category Value} +@Deprecated("Use SassString instead.") @sealed class CalculationInterpolation { /// We use a getters to allow overriding the logic in the JS API diff --git a/lib/src/value/number.dart b/lib/src/value/number.dart index 410bc8465..a5c90a501 100644 --- a/lib/src/value/number.dart +++ b/lib/src/value/number.dart @@ -710,7 +710,7 @@ abstract class SassNumber extends Value { /// @nodoc @internal - Value modulo(Value other) { + SassNumber modulo(Value other) { if (other is SassNumber) { return withValue(_coerceUnits(other, moduloLikeSass)); } diff --git a/lib/src/value/number/unitless.dart b/lib/src/value/number/unitless.dart index 06b54d39b..7272b7c59 100644 --- a/lib/src/value/number/unitless.dart +++ b/lib/src/value/number/unitless.dart @@ -98,7 +98,7 @@ class UnitlessSassNumber extends SassNumber { return super.lessThanOrEquals(other); } - Value modulo(Value other) { + SassNumber modulo(Value other) { if (other is SassNumber) { return other.withValue(moduloLikeSass(value, other.value)); } diff --git a/lib/src/visitor/ast_search.dart b/lib/src/visitor/ast_search.dart new file mode 100644 index 000000000..d971afd23 --- /dev/null +++ b/lib/src/visitor/ast_search.dart @@ -0,0 +1,185 @@ +// Copyright 2023 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:meta/meta.dart'; + +import '../ast/sass.dart'; +import '../util/iterable.dart'; +import '../util/nullable.dart'; +import 'interface/expression.dart'; +import 'recursive_statement.dart'; +import 'statement_search.dart'; + +/// A visitor that recursively traverses each statement and expression in a Sass +/// AST whose `visit*` methods default to returning `null`, but which returns +/// the first non-`null` value returned by any method. +/// +/// This extends [RecursiveStatementVisitor] to traverse each expression in +/// addition to each statement. It supports the same additional methods as +/// [RecursiveAstVisitor]. +/// +/// {@category Visitor} +mixin AstSearchVisitor on StatementSearchVisitor + implements ExpressionVisitor { + T? visitAtRootRule(AtRootRule node) => + node.query.andThen(visitInterpolation) ?? super.visitAtRootRule(node); + + T? visitAtRule(AtRule node) => + visitInterpolation(node.name) ?? + node.value.andThen(visitInterpolation) ?? + super.visitAtRule(node); + + T? visitContentRule(ContentRule node) => + visitArgumentInvocation(node.arguments); + + T? visitDebugRule(DebugRule node) => visitExpression(node.expression); + + T? visitDeclaration(Declaration node) => + visitInterpolation(node.name) ?? + node.value.andThen(visitExpression) ?? + super.visitDeclaration(node); + + T? visitEachRule(EachRule node) => + visitExpression(node.list) ?? super.visitEachRule(node); + + T? visitErrorRule(ErrorRule node) => visitExpression(node.expression); + + T? visitExtendRule(ExtendRule node) => visitInterpolation(node.selector); + + T? visitForRule(ForRule node) => + visitExpression(node.from) ?? + visitExpression(node.to) ?? + super.visitForRule(node); + + T? visitForwardRule(ForwardRule node) => node.configuration + .search((variable) => visitExpression(variable.expression)); + + T? visitIfRule(IfRule node) => + node.clauses.search((clause) => + visitExpression(clause.expression) ?? + clause.children.search((child) => child.accept(this))) ?? + node.lastClause.andThen((lastClause) => + lastClause.children.search((child) => child.accept(this))); + + T? visitImportRule(ImportRule node) => + node.imports.search((import) => import is StaticImport + ? visitInterpolation(import.url) ?? + import.modifiers.andThen(visitInterpolation) + : null); + + T? visitIncludeRule(IncludeRule node) => + visitArgumentInvocation(node.arguments) ?? super.visitIncludeRule(node); + + T? visitLoudComment(LoudComment node) => visitInterpolation(node.text); + + T? visitMediaRule(MediaRule node) => + visitInterpolation(node.query) ?? super.visitMediaRule(node); + + T? visitReturnRule(ReturnRule node) => visitExpression(node.expression); + + T? visitStyleRule(StyleRule node) => + visitInterpolation(node.selector) ?? super.visitStyleRule(node); + + T? visitSupportsRule(SupportsRule node) => + visitSupportsCondition(node.condition) ?? super.visitSupportsRule(node); + + T? visitUseRule(UseRule node) => node.configuration + .search((variable) => visitExpression(variable.expression)); + + T? visitVariableDeclaration(VariableDeclaration node) => + visitExpression(node.expression); + + T? visitWarnRule(WarnRule node) => visitExpression(node.expression); + + T? visitWhileRule(WhileRule node) => + visitExpression(node.condition) ?? super.visitWhileRule(node); + + T? visitExpression(Expression expression) => expression.accept(this); + + T? visitBinaryOperationExpression(BinaryOperationExpression node) => + node.left.accept(this) ?? node.right.accept(this); + + T? visitBooleanExpression(BooleanExpression node) => null; + + T? visitColorExpression(ColorExpression node) => null; + + T? visitFunctionExpression(FunctionExpression node) => + visitArgumentInvocation(node.arguments); + + T? visitInterpolatedFunctionExpression(InterpolatedFunctionExpression node) => + visitInterpolation(node.name) ?? visitArgumentInvocation(node.arguments); + + T? visitIfExpression(IfExpression node) => + visitArgumentInvocation(node.arguments); + + T? visitListExpression(ListExpression node) => + node.contents.search((item) => item.accept(this)); + + T? visitMapExpression(MapExpression node) => + node.pairs.search((pair) => pair.$1.accept(this) ?? pair.$2.accept(this)); + + T? visitNullExpression(NullExpression node) => null; + + T? visitNumberExpression(NumberExpression node) => null; + + T? visitParenthesizedExpression(ParenthesizedExpression node) => + node.expression.accept(this); + + T? visitSelectorExpression(SelectorExpression node) => null; + + T? visitStringExpression(StringExpression node) => + visitInterpolation(node.text); + + T? visitSupportsExpression(SupportsExpression node) => + visitSupportsCondition(node.condition); + + T? visitUnaryOperationExpression(UnaryOperationExpression node) => + node.operand.accept(this); + + T? visitValueExpression(ValueExpression node) => null; + + T? visitVariableExpression(VariableExpression node) => null; + + @protected + T? visitCallableDeclaration(CallableDeclaration node) => + node.arguments.arguments.search( + (argument) => argument.defaultValue.andThen(visitExpression)) ?? + super.visitCallableDeclaration(node); + + /// Visits each expression in an [invocation]. + /// + /// The default implementation of the visit methods calls this to visit any + /// argument invocation in a statement. + @protected + T? visitArgumentInvocation(ArgumentInvocation invocation) => + invocation.positional + .search((expression) => visitExpression(expression)) ?? + invocation.named.values + .search((expression) => visitExpression(expression)) ?? + invocation.rest.andThen(visitExpression) ?? + invocation.keywordRest.andThen(visitExpression); + + /// Visits each expression in [condition]. + /// + /// The default implementation of the visit methods call this to visit any + /// [SupportsCondition] they encounter. + @protected + T? visitSupportsCondition(SupportsCondition condition) => switch (condition) { + SupportsOperation() => visitSupportsCondition(condition.left) ?? + visitSupportsCondition(condition.right), + SupportsNegation() => visitSupportsCondition(condition.condition), + SupportsInterpolation() => visitExpression(condition.expression), + SupportsDeclaration() => + visitExpression(condition.name) ?? visitExpression(condition.value), + _ => null + }; + + /// Visits each expression in an [interpolation]. + /// + /// The default implementation of the visit methods call this to visit any + /// interpolation in a statement. + @protected + T? visitInterpolation(Interpolation interpolation) => interpolation.contents + .search((node) => node is Expression ? visitExpression(node) : null); +} diff --git a/lib/src/visitor/async_evaluate.dart b/lib/src/visitor/async_evaluate.dart index 3e8dabcd3..b8917ba42 100644 --- a/lib/src/visitor/async_evaluate.dart +++ b/lib/src/visitor/async_evaluate.dart @@ -40,6 +40,7 @@ import '../module/built_in.dart'; import '../parse/keyframe_selector.dart'; import '../syntax.dart'; import '../utils.dart'; +import '../util/character.dart'; import '../util/map.dart'; import '../util/multi_span.dart'; import '../util/nullable.dart'; @@ -430,10 +431,14 @@ final class _EvaluateVisitor return SassFunction(PlainCssCallable(name.text)); } - var callable = _addExceptionSpan( - _callableNode!, - () => _getFunction(name.text.replaceAll("_", "-"), - namespace: module?.text)); + var callable = _addExceptionSpan(_callableNode!, () { + var normalizedName = name.text.replaceAll("_", "-"); + var namespace = module?.text; + var local = + _environment.getFunction(normalizedName, namespace: namespace); + if (local != null || namespace != null) return local; + return _builtInFunctions[normalizedName]; + }); if (callable == null) throw "Function not found: $name"; return SassFunction(callable); @@ -2182,6 +2187,13 @@ final class _EvaluateVisitor // ## Expressions Future visitBinaryOperationExpression(BinaryOperationExpression node) { + if (_stylesheet.plainCss && + node.operator != BinaryOperator.singleEquals && + node.operator != BinaryOperator.dividedBy) { + throw _exception( + "Operators aren't allowed in plain CSS.", node.operatorSpan); + } + return _addExceptionSpanAsync(node, () async { var left = await node.left.accept(this); return switch (node.operator) { @@ -2217,7 +2229,10 @@ final class _EvaluateVisitor Value _slash(Value left, Value right, BinaryOperationExpression node) { var result = left.dividedBy(right); switch ((left, right)) { - case (SassNumber left, SassNumber right) when node.allowsSlash: + case (SassNumber left, SassNumber right) + when node.allowsSlash && + _operandAllowsSlash(node.left) && + _operandAllowsSlash(node.right): return (result as SassNumber).withSlash(left, right); case (SassNumber(), SassNumber()): @@ -2250,6 +2265,20 @@ final class _EvaluateVisitor } } + /// Returns whether [node] can be used as a component of a slash-separated + /// number. + /// + /// Although this logic is mostly resolved at parse-time, we can't tell + /// whether operands will be evaluated as calculations until evaluation-time. + bool _operandAllowsSlash(Expression node) => + node is! FunctionExpression || + (node.namespace == null && + const { + "calc", "clamp", "hypot", "sin", "cos", "tan", "asin", "acos", // + "atan", "sqrt", "exp", "sign", "mod", "rem", "atan2", "pow", "log" + }.contains(node.name.toLowerCase()) && + _environment.getFunction(node.name) == null); + Future visitValueExpression(ValueExpression node) async => node.value; Future visitVariableExpression(VariableExpression node) async { @@ -2294,23 +2323,142 @@ final class _EvaluateVisitor SassNumber(node.value, node.unit); Future visitParenthesizedExpression(ParenthesizedExpression node) => - node.expression.accept(this); + _stylesheet.plainCss + ? throw _exception( + "Parentheses aren't allowed in plain CSS.", node.span) + : node.expression.accept(this); + + Future visitColorExpression(ColorExpression node) async => + node.value; + + Future visitListExpression(ListExpression node) async => SassList( + await mapAsync( + node.contents, (Expression expression) => expression.accept(this)), + node.separator, + brackets: node.hasBrackets); + + Future visitMapExpression(MapExpression node) async { + var map = {}; + var keyNodes = {}; + for (var (key, value) in node.pairs) { + var keyValue = await key.accept(this); + var valueValue = await value.accept(this); - Future visitCalculationExpression(CalculationExpression node) async { + if (map.containsKey(keyValue)) { + var oldValueSpan = keyNodes[keyValue]?.span; + throw MultiSpanSassRuntimeException( + 'Duplicate key.', + key.span, + 'second key', + {if (oldValueSpan != null) oldValueSpan: 'first key'}, + _stackTrace(key.span)); + } + map[keyValue] = valueValue; + keyNodes[keyValue] = key; + } + return SassMap(map); + } + + Future visitFunctionExpression(FunctionExpression node) async { + var function = _stylesheet.plainCss + ? null + : _addExceptionSpan( + node, + () => + _environment.getFunction(node.name, namespace: node.namespace)); + if (function == null) { + if (node.namespace != null) { + throw _exception("Undefined function.", node.span); + } + + switch (node.name.toLowerCase()) { + case "min" || "max" || "round" || "abs" + when node.arguments.named.isEmpty && + node.arguments.rest == null && + node.arguments.positional + .every((argument) => argument.isCalculationSafe): + return await _visitCalculation(node, inLegacySassFunction: true); + + case "calc" || + "clamp" || + "hypot" || + "sin" || + "cos" || + "tan" || + "asin" || + "acos" || + "atan" || + "sqrt" || + "exp" || + "sign" || + "mod" || + "rem" || + "atan2" || + "pow" || + "log": + return await _visitCalculation(node); + } + + function = (_stylesheet.plainCss ? null : _builtInFunctions[node.name]) ?? + PlainCssCallable(node.originalName); + } + + var oldInFunction = _inFunction; + _inFunction = true; + var result = await _addErrorSpan( + node, () => _runFunctionCallable(node.arguments, function, node)); + _inFunction = oldInFunction; + return result; + } + + Future _visitCalculation(FunctionExpression node, + {bool inLegacySassFunction = false}) async { + if (node.arguments.named.isNotEmpty) { + throw _exception( + "Keyword arguments can't be used with calculations.", node.span); + } else if (node.arguments.rest != null) { + throw _exception( + "Rest arguments can't be used with calculations.", node.span); + } + + _checkCalculationArguments(node); var arguments = [ - for (var argument in node.arguments) - await _visitCalculationValue(argument, - inMinMax: node.name == 'min' || node.name == 'max') + for (var argument in node.arguments.positional) + await _visitCalculationExpression(argument, + inLegacySassFunction: inLegacySassFunction) ]; if (_inSupportsDeclaration) { return SassCalculation.unsimplified(node.name, arguments); } try { - return switch (node.name) { + return switch (node.name.toLowerCase()) { "calc" => SassCalculation.calc(arguments[0]), + "sqrt" => SassCalculation.sqrt(arguments[0]), + "sin" => SassCalculation.sin(arguments[0]), + "cos" => SassCalculation.cos(arguments[0]), + "tan" => SassCalculation.tan(arguments[0]), + "asin" => SassCalculation.asin(arguments[0]), + "acos" => SassCalculation.acos(arguments[0]), + "atan" => SassCalculation.atan(arguments[0]), + "abs" => SassCalculation.abs(arguments[0]), + "exp" => SassCalculation.exp(arguments[0]), + "sign" => SassCalculation.sign(arguments[0]), "min" => SassCalculation.min(arguments), "max" => SassCalculation.max(arguments), + "hypot" => SassCalculation.hypot(arguments), + "pow" => + SassCalculation.pow(arguments[0], arguments.elementAtOrNull(1)), + "atan2" => + SassCalculation.atan2(arguments[0], arguments.elementAtOrNull(1)), + "log" => + SassCalculation.log(arguments[0], arguments.elementAtOrNull(1)), + "mod" => + SassCalculation.mod(arguments[0], arguments.elementAtOrNull(1)), + "rem" => + SassCalculation.rem(arguments[0], arguments.elementAtOrNull(1)), + "round" => SassCalculation.round(arguments[0], + arguments.elementAtOrNull(1), arguments.elementAtOrNull(2)), "clamp" => SassCalculation.clamp(arguments[0], arguments.elementAtOrNull(1), arguments.elementAtOrNull(2)), _ => throw UnsupportedError('Unknown calculation name "${node.name}".') @@ -2319,11 +2467,54 @@ final class _EvaluateVisitor // The simplification logic in the [SassCalculation] static methods will // throw an error if the arguments aren't compatible, but we have access // to the original spans so we can throw a more informative error. - _verifyCompatibleNumbers(arguments, node.arguments); + if (error.message.contains("compatible")) { + _verifyCompatibleNumbers(arguments, node.arguments.positional); + } throwWithTrace(_exception(error.message, node.span), error, stackTrace); } } + /// Verifies that the calculation [node] has the correct number of arguments. + void _checkCalculationArguments(FunctionExpression node) { + void check([int? maxArgs]) { + if (node.arguments.positional.isEmpty) { + throw _exception("Missing argument.", node.span); + } else if (maxArgs != null && + node.arguments.positional.length > maxArgs) { + throw _exception( + "Only $maxArgs ${pluralize('argument', maxArgs)} allowed, but " + "${node.arguments.positional.length} " + + pluralize('was', node.arguments.positional.length, + plural: 'were') + + " passed.", + node.span); + } + } + + switch (node.name.toLowerCase()) { + case "calc" || + "sqrt" || + "sin" || + "cos" || + "tan" || + "asin" || + "acos" || + "atan" || + "abs" || + "exp" || + "sign": + check(1); + case "min" || "max" || "hypot": + check(); + case "pow" || "atan2" || "log" || "mod" || "rem": + check(2); + case "round" || "clamp": + check(3); + case _: + throw UnsupportedError('Unknown calculation name "${node.name}".'); + } + } + /// Verifies that [args] all have compatible units that can be used for CSS /// calculations, and throws a [SassException] if not. /// @@ -2361,55 +2552,47 @@ final class _EvaluateVisitor /// Evaluates [node] as a component of a calculation. /// - /// If [inMinMax] is `true`, this allows unitless numbers to be added and + /// If [inLegacySassFunction] is `true`, this allows unitless numbers to be added and /// subtracted with numbers with units, for backwards-compatibility with the - /// old global `min()` and `max()` functions. - Future _visitCalculationValue(Expression node, - {required bool inMinMax}) async { + /// old global `min()`, `max()`, `round()`, and `abs()` functions. + Future _visitCalculationExpression(Expression node, + {required bool inLegacySassFunction}) async { switch (node) { case ParenthesizedExpression(expression: var inner): - var result = await _visitCalculationValue(inner, inMinMax: inMinMax); - return inner is FunctionExpression && - inner.name.toLowerCase() == 'var' && - result is SassString && - !result.hasQuotes + var result = await _visitCalculationExpression(inner, + inLegacySassFunction: inLegacySassFunction); + return result is SassString ? SassString('(${result.text})', quotes: false) : result; - case StringExpression(text: Interpolation(asPlain: var text?)): + case StringExpression() when node.isCalculationSafe: assert(!node.hasQuotes); - return switch (text.toLowerCase()) { + return switch (node.text.asPlain?.toLowerCase()) { 'pi' => SassNumber(math.pi), 'e' => SassNumber(math.e), 'infinity' => SassNumber(double.infinity), '-infinity' => SassNumber(double.negativeInfinity), 'nan' => SassNumber(double.nan), - _ => SassString(text, quotes: false) + _ => SassString(await _performInterpolation(node.text), quotes: false) }; - // If there's actual interpolation, create a CalculationInterpolation. - // Otherwise, create an UnquotedString. The main difference is that - // UnquotedStrings don't get extra defensive parentheses. - case StringExpression(): - assert(!node.hasQuotes); - return CalculationInterpolation(await _performInterpolation(node.text)); - case BinaryOperationExpression(:var operator, :var left, :var right): + _checkWhitespaceAroundCalculationOperator(node); return await _addExceptionSpanAsync( node, () async => SassCalculation.operateInternal( - _binaryOperatorToCalculationOperator(operator), - await _visitCalculationValue(left, inMinMax: inMinMax), - await _visitCalculationValue(right, inMinMax: inMinMax), - inMinMax: inMinMax, + _binaryOperatorToCalculationOperator(operator, node), + await _visitCalculationExpression(left, + inLegacySassFunction: inLegacySassFunction), + await _visitCalculationExpression(right, + inLegacySassFunction: inLegacySassFunction), + inLegacySassFunction: inLegacySassFunction, simplify: !_inSupportsDeclaration)); - case _: - assert(node is NumberExpression || - node is CalculationExpression || - node is VariableExpression || - node is FunctionExpression || - node is IfExpression); + case NumberExpression() || + VariableExpression() || + FunctionExpression() || + IfExpression(): return switch (await node.accept(this)) { SassNumber result => result, SassCalculation result => result, @@ -2417,70 +2600,104 @@ final class _EvaluateVisitor var result => throw _exception( "Value $result can't be used in a calculation.", node.span) }; + + case ListExpression( + hasBrackets: false, + separator: ListSeparator.space, + contents: [_, _, ...] + ): + var elements = [ + for (var element in node.contents) + await _visitCalculationExpression(element, + inLegacySassFunction: inLegacySassFunction) + ]; + + _checkAdjacentCalculationValues(elements, node); + + for (var i = 0; i < elements.length; i++) { + if (elements[i] is CalculationOperation && + node.contents[i] is ParenthesizedExpression) { + elements[i] = SassString("(${elements[i]})", quotes: false); + } + } + + return SassString(elements.join(' '), quotes: false); + + case _: + assert(!node.isCalculationSafe); + throw _exception( + "This expression can't be used in a calculation.", node.span); + } + } + + /// Throws an error if [node] requires whitespace around its operator in a + /// calculation but doesn't have it. + void _checkWhitespaceAroundCalculationOperator( + BinaryOperationExpression node) { + if (node.operator != BinaryOperator.plus && + node.operator != BinaryOperator.minus) { + return; + } + + // We _should_ never be able to violate these conditions since we always + // parse binary operations from a single file, but it's better to be safe + // than have this crash bizarrely. + if (node.left.span.file != node.right.span.file) return; + if (node.left.span.end.offset >= node.right.span.start.offset) return; + + var textBetweenOperands = node.left.span.file + .getText(node.left.span.end.offset, node.right.span.start.offset); + var first = textBetweenOperands.codeUnitAt(0); + var last = textBetweenOperands.codeUnitAt(textBetweenOperands.length - 1); + if (!(first.isWhitespace || first == $slash) || + !(last.isWhitespace || last == $slash)) { + throw _exception( + '"+" and "-" must be surrounded by whitespace in calculations.', + node.operatorSpan); } } /// Returns the [CalculationOperator] that corresponds to [operator]. CalculationOperator _binaryOperatorToCalculationOperator( - BinaryOperator operator) => + BinaryOperator operator, BinaryOperationExpression node) => switch (operator) { BinaryOperator.plus => CalculationOperator.plus, BinaryOperator.minus => CalculationOperator.minus, BinaryOperator.times => CalculationOperator.times, BinaryOperator.dividedBy => CalculationOperator.dividedBy, - _ => throw UnsupportedError("Invalid calculation operator $operator.") + _ => throw _exception( + "This operation can't be used in a calculation.", node.operatorSpan) }; - Future visitColorExpression(ColorExpression node) async => - node.value; - - Future visitListExpression(ListExpression node) async => SassList( - await mapAsync( - node.contents, (Expression expression) => expression.accept(this)), - node.separator, - brackets: node.hasBrackets); - - Future visitMapExpression(MapExpression node) async { - var map = {}; - var keyNodes = {}; - for (var (key, value) in node.pairs) { - var keyValue = await key.accept(this); - var valueValue = await value.accept(this); - - var oldValue = map[keyValue]; - if (oldValue != null) { - var oldValueSpan = keyNodes[keyValue]?.span; - throw MultiSpanSassRuntimeException( - 'Duplicate key.', - key.span, - 'second key', - {if (oldValueSpan != null) oldValueSpan: 'first key'}, - _stackTrace(key.span)); - } - map[keyValue] = valueValue; - keyNodes[keyValue] = key; - } - return SassMap(map); - } - - Future visitFunctionExpression(FunctionExpression node) async { - var function = _addExceptionSpan( - node, () => _getFunction(node.name, namespace: node.namespace)); - - if (function == null) { - if (node.namespace != null) { - throw _exception("Undefined function.", node.span); + /// Throws an error if [elements] contains two adjacent non-string values. + void _checkAdjacentCalculationValues( + List elements, ListExpression node) { + assert(elements.length > 1); + + for (var i = 1; i < elements.length; i++) { + var previous = elements[i - 1]; + var current = elements[i]; + if (previous is SassString || current is SassString) continue; + + var previousNode = node.contents[i - 1]; + var currentNode = node.contents[i]; + if (currentNode + case UnaryOperationExpression( + operator: UnaryOperator.minus || UnaryOperator.plus + ) || + NumberExpression(value: < 0)) { + // `calc(1 -2)` parses as a space-separated list whose second value is a + // unary operator or a negative number, but just saying it's an invalid + // expression doesn't help the user understand what's going wrong. We + // add special case error handling to help clarify the issue. + throw _exception( + '"+" and "-" must be surrounded by whitespace in calculations.', + currentNode.span.subspan(0, 1)); + } else { + throw _exception('Missing math operator.', + previousNode.span.expand(currentNode.span)); } - - function = PlainCssCallable(node.originalName); } - - var oldInFunction = _inFunction; - _inFunction = true; - var result = await _addErrorSpan( - node, () => _runFunctionCallable(node.arguments, function, node)); - _inFunction = oldInFunction; - return result; } Future visitInterpolatedFunctionExpression( @@ -2494,14 +2711,6 @@ final class _EvaluateVisitor return result; } - /// Like `_environment.getFunction`, but also returns built-in - /// globally-available functions. - AsyncCallable? _getFunction(String name, {String? namespace}) { - var local = _environment.getFunction(name, namespace: namespace); - if (local != null || namespace != null) return local; - return _builtInFunctions[name]; - } - /// Evaluates the arguments in [arguments] as applied to [callable], and /// invokes [run] in a scope with those arguments defined. Future _runUserDefinedCallable( diff --git a/lib/src/visitor/evaluate.dart b/lib/src/visitor/evaluate.dart index a8639f4e6..521e6cd02 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: 6eb7f76735562eba91e9460af796b269b3b0aaf7 +// Checksum: ccd4ec1a65cfc2487fccd30481d427086f5c76cc // // ignore_for_file: unused_import @@ -49,6 +49,7 @@ import '../module/built_in.dart'; import '../parse/keyframe_selector.dart'; import '../syntax.dart'; import '../utils.dart'; +import '../util/character.dart'; import '../util/map.dart'; import '../util/multi_span.dart'; import '../util/nullable.dart'; @@ -438,10 +439,14 @@ final class _EvaluateVisitor return SassFunction(PlainCssCallable(name.text)); } - var callable = _addExceptionSpan( - _callableNode!, - () => _getFunction(name.text.replaceAll("_", "-"), - namespace: module?.text)); + var callable = _addExceptionSpan(_callableNode!, () { + var normalizedName = name.text.replaceAll("_", "-"); + var namespace = module?.text; + var local = + _environment.getFunction(normalizedName, namespace: namespace); + if (local != null || namespace != null) return local; + return _builtInFunctions[normalizedName]; + }); if (callable == null) throw "Function not found: $name"; return SassFunction(callable); @@ -2170,6 +2175,13 @@ final class _EvaluateVisitor // ## Expressions Value visitBinaryOperationExpression(BinaryOperationExpression node) { + if (_stylesheet.plainCss && + node.operator != BinaryOperator.singleEquals && + node.operator != BinaryOperator.dividedBy) { + throw _exception( + "Operators aren't allowed in plain CSS.", node.operatorSpan); + } + return _addExceptionSpan(node, () { var left = node.left.accept(this); return switch (node.operator) { @@ -2200,7 +2212,10 @@ final class _EvaluateVisitor Value _slash(Value left, Value right, BinaryOperationExpression node) { var result = left.dividedBy(right); switch ((left, right)) { - case (SassNumber left, SassNumber right) when node.allowsSlash: + case (SassNumber left, SassNumber right) + when node.allowsSlash && + _operandAllowsSlash(node.left) && + _operandAllowsSlash(node.right): return (result as SassNumber).withSlash(left, right); case (SassNumber(), SassNumber()): @@ -2233,6 +2248,20 @@ final class _EvaluateVisitor } } + /// Returns whether [node] can be used as a component of a slash-separated + /// number. + /// + /// Although this logic is mostly resolved at parse-time, we can't tell + /// whether operands will be evaluated as calculations until evaluation-time. + bool _operandAllowsSlash(Expression node) => + node is! FunctionExpression || + (node.namespace == null && + const { + "calc", "clamp", "hypot", "sin", "cos", "tan", "asin", "acos", // + "atan", "sqrt", "exp", "sign", "mod", "rem", "atan2", "pow", "log" + }.contains(node.name.toLowerCase()) && + _environment.getFunction(node.name) == null); + Value visitValueExpression(ValueExpression node) => node.value; Value visitVariableExpression(VariableExpression node) { @@ -2276,23 +2305,140 @@ final class _EvaluateVisitor SassNumber(node.value, node.unit); Value visitParenthesizedExpression(ParenthesizedExpression node) => - node.expression.accept(this); + _stylesheet.plainCss + ? throw _exception( + "Parentheses aren't allowed in plain CSS.", node.span) + : node.expression.accept(this); + + SassColor visitColorExpression(ColorExpression node) => node.value; - Value visitCalculationExpression(CalculationExpression node) { + SassList visitListExpression(ListExpression node) => SassList( + node.contents.map((Expression expression) => expression.accept(this)), + node.separator, + brackets: node.hasBrackets); + + SassMap visitMapExpression(MapExpression node) { + var map = {}; + var keyNodes = {}; + for (var (key, value) in node.pairs) { + var keyValue = key.accept(this); + var valueValue = value.accept(this); + + if (map.containsKey(keyValue)) { + var oldValueSpan = keyNodes[keyValue]?.span; + throw MultiSpanSassRuntimeException( + 'Duplicate key.', + key.span, + 'second key', + {if (oldValueSpan != null) oldValueSpan: 'first key'}, + _stackTrace(key.span)); + } + map[keyValue] = valueValue; + keyNodes[keyValue] = key; + } + return SassMap(map); + } + + Value visitFunctionExpression(FunctionExpression node) { + var function = _stylesheet.plainCss + ? null + : _addExceptionSpan( + node, + () => + _environment.getFunction(node.name, namespace: node.namespace)); + if (function == null) { + if (node.namespace != null) { + throw _exception("Undefined function.", node.span); + } + + switch (node.name.toLowerCase()) { + case "min" || "max" || "round" || "abs" + when node.arguments.named.isEmpty && + node.arguments.rest == null && + node.arguments.positional + .every((argument) => argument.isCalculationSafe): + return _visitCalculation(node, inLegacySassFunction: true); + + case "calc" || + "clamp" || + "hypot" || + "sin" || + "cos" || + "tan" || + "asin" || + "acos" || + "atan" || + "sqrt" || + "exp" || + "sign" || + "mod" || + "rem" || + "atan2" || + "pow" || + "log": + return _visitCalculation(node); + } + + function = (_stylesheet.plainCss ? null : _builtInFunctions[node.name]) ?? + PlainCssCallable(node.originalName); + } + + var oldInFunction = _inFunction; + _inFunction = true; + var result = _addErrorSpan( + node, () => _runFunctionCallable(node.arguments, function, node)); + _inFunction = oldInFunction; + return result; + } + + Value _visitCalculation(FunctionExpression node, + {bool inLegacySassFunction = false}) { + if (node.arguments.named.isNotEmpty) { + throw _exception( + "Keyword arguments can't be used with calculations.", node.span); + } else if (node.arguments.rest != null) { + throw _exception( + "Rest arguments can't be used with calculations.", node.span); + } + + _checkCalculationArguments(node); var arguments = [ - for (var argument in node.arguments) - _visitCalculationValue(argument, - inMinMax: node.name == 'min' || node.name == 'max') + for (var argument in node.arguments.positional) + _visitCalculationExpression(argument, + inLegacySassFunction: inLegacySassFunction) ]; if (_inSupportsDeclaration) { return SassCalculation.unsimplified(node.name, arguments); } try { - return switch (node.name) { + return switch (node.name.toLowerCase()) { "calc" => SassCalculation.calc(arguments[0]), + "sqrt" => SassCalculation.sqrt(arguments[0]), + "sin" => SassCalculation.sin(arguments[0]), + "cos" => SassCalculation.cos(arguments[0]), + "tan" => SassCalculation.tan(arguments[0]), + "asin" => SassCalculation.asin(arguments[0]), + "acos" => SassCalculation.acos(arguments[0]), + "atan" => SassCalculation.atan(arguments[0]), + "abs" => SassCalculation.abs(arguments[0]), + "exp" => SassCalculation.exp(arguments[0]), + "sign" => SassCalculation.sign(arguments[0]), "min" => SassCalculation.min(arguments), "max" => SassCalculation.max(arguments), + "hypot" => SassCalculation.hypot(arguments), + "pow" => + SassCalculation.pow(arguments[0], arguments.elementAtOrNull(1)), + "atan2" => + SassCalculation.atan2(arguments[0], arguments.elementAtOrNull(1)), + "log" => + SassCalculation.log(arguments[0], arguments.elementAtOrNull(1)), + "mod" => + SassCalculation.mod(arguments[0], arguments.elementAtOrNull(1)), + "rem" => + SassCalculation.rem(arguments[0], arguments.elementAtOrNull(1)), + "round" => SassCalculation.round(arguments[0], + arguments.elementAtOrNull(1), arguments.elementAtOrNull(2)), "clamp" => SassCalculation.clamp(arguments[0], arguments.elementAtOrNull(1), arguments.elementAtOrNull(2)), _ => throw UnsupportedError('Unknown calculation name "${node.name}".') @@ -2301,11 +2447,54 @@ final class _EvaluateVisitor // The simplification logic in the [SassCalculation] static methods will // throw an error if the arguments aren't compatible, but we have access // to the original spans so we can throw a more informative error. - _verifyCompatibleNumbers(arguments, node.arguments); + if (error.message.contains("compatible")) { + _verifyCompatibleNumbers(arguments, node.arguments.positional); + } throwWithTrace(_exception(error.message, node.span), error, stackTrace); } } + /// Verifies that the calculation [node] has the correct number of arguments. + void _checkCalculationArguments(FunctionExpression node) { + void check([int? maxArgs]) { + if (node.arguments.positional.isEmpty) { + throw _exception("Missing argument.", node.span); + } else if (maxArgs != null && + node.arguments.positional.length > maxArgs) { + throw _exception( + "Only $maxArgs ${pluralize('argument', maxArgs)} allowed, but " + "${node.arguments.positional.length} " + + pluralize('was', node.arguments.positional.length, + plural: 'were') + + " passed.", + node.span); + } + } + + switch (node.name.toLowerCase()) { + case "calc" || + "sqrt" || + "sin" || + "cos" || + "tan" || + "asin" || + "acos" || + "atan" || + "abs" || + "exp" || + "sign": + check(1); + case "min" || "max" || "hypot": + check(); + case "pow" || "atan2" || "log" || "mod" || "rem": + check(2); + case "round" || "clamp": + check(3); + case _: + throw UnsupportedError('Unknown calculation name "${node.name}".'); + } + } + /// Verifies that [args] all have compatible units that can be used for CSS /// calculations, and throws a [SassException] if not. /// @@ -2343,54 +2532,47 @@ final class _EvaluateVisitor /// Evaluates [node] as a component of a calculation. /// - /// If [inMinMax] is `true`, this allows unitless numbers to be added and + /// If [inLegacySassFunction] is `true`, this allows unitless numbers to be added and /// subtracted with numbers with units, for backwards-compatibility with the - /// old global `min()` and `max()` functions. - Object _visitCalculationValue(Expression node, {required bool inMinMax}) { + /// old global `min()`, `max()`, `round()`, and `abs()` functions. + Object _visitCalculationExpression(Expression node, + {required bool inLegacySassFunction}) { switch (node) { case ParenthesizedExpression(expression: var inner): - var result = _visitCalculationValue(inner, inMinMax: inMinMax); - return inner is FunctionExpression && - inner.name.toLowerCase() == 'var' && - result is SassString && - !result.hasQuotes + var result = _visitCalculationExpression(inner, + inLegacySassFunction: inLegacySassFunction); + return result is SassString ? SassString('(${result.text})', quotes: false) : result; - case StringExpression(text: Interpolation(asPlain: var text?)): + case StringExpression() when node.isCalculationSafe: assert(!node.hasQuotes); - return switch (text.toLowerCase()) { + return switch (node.text.asPlain?.toLowerCase()) { 'pi' => SassNumber(math.pi), 'e' => SassNumber(math.e), 'infinity' => SassNumber(double.infinity), '-infinity' => SassNumber(double.negativeInfinity), 'nan' => SassNumber(double.nan), - _ => SassString(text, quotes: false) + _ => SassString(_performInterpolation(node.text), quotes: false) }; - // If there's actual interpolation, create a CalculationInterpolation. - // Otherwise, create an UnquotedString. The main difference is that - // UnquotedStrings don't get extra defensive parentheses. - case StringExpression(): - assert(!node.hasQuotes); - return CalculationInterpolation(_performInterpolation(node.text)); - case BinaryOperationExpression(:var operator, :var left, :var right): + _checkWhitespaceAroundCalculationOperator(node); return _addExceptionSpan( node, () => SassCalculation.operateInternal( - _binaryOperatorToCalculationOperator(operator), - _visitCalculationValue(left, inMinMax: inMinMax), - _visitCalculationValue(right, inMinMax: inMinMax), - inMinMax: inMinMax, + _binaryOperatorToCalculationOperator(operator, node), + _visitCalculationExpression(left, + inLegacySassFunction: inLegacySassFunction), + _visitCalculationExpression(right, + inLegacySassFunction: inLegacySassFunction), + inLegacySassFunction: inLegacySassFunction, simplify: !_inSupportsDeclaration)); - case _: - assert(node is NumberExpression || - node is CalculationExpression || - node is VariableExpression || - node is FunctionExpression || - node is IfExpression); + case NumberExpression() || + VariableExpression() || + FunctionExpression() || + IfExpression(): return switch (node.accept(this)) { SassNumber result => result, SassCalculation result => result, @@ -2398,68 +2580,104 @@ final class _EvaluateVisitor var result => throw _exception( "Value $result can't be used in a calculation.", node.span) }; + + case ListExpression( + hasBrackets: false, + separator: ListSeparator.space, + contents: [_, _, ...] + ): + var elements = [ + for (var element in node.contents) + _visitCalculationExpression(element, + inLegacySassFunction: inLegacySassFunction) + ]; + + _checkAdjacentCalculationValues(elements, node); + + for (var i = 0; i < elements.length; i++) { + if (elements[i] is CalculationOperation && + node.contents[i] is ParenthesizedExpression) { + elements[i] = SassString("(${elements[i]})", quotes: false); + } + } + + return SassString(elements.join(' '), quotes: false); + + case _: + assert(!node.isCalculationSafe); + throw _exception( + "This expression can't be used in a calculation.", node.span); + } + } + + /// Throws an error if [node] requires whitespace around its operator in a + /// calculation but doesn't have it. + void _checkWhitespaceAroundCalculationOperator( + BinaryOperationExpression node) { + if (node.operator != BinaryOperator.plus && + node.operator != BinaryOperator.minus) { + return; + } + + // We _should_ never be able to violate these conditions since we always + // parse binary operations from a single file, but it's better to be safe + // than have this crash bizarrely. + if (node.left.span.file != node.right.span.file) return; + if (node.left.span.end.offset >= node.right.span.start.offset) return; + + var textBetweenOperands = node.left.span.file + .getText(node.left.span.end.offset, node.right.span.start.offset); + var first = textBetweenOperands.codeUnitAt(0); + var last = textBetweenOperands.codeUnitAt(textBetweenOperands.length - 1); + if (!(first.isWhitespace || first == $slash) || + !(last.isWhitespace || last == $slash)) { + throw _exception( + '"+" and "-" must be surrounded by whitespace in calculations.', + node.operatorSpan); } } /// Returns the [CalculationOperator] that corresponds to [operator]. CalculationOperator _binaryOperatorToCalculationOperator( - BinaryOperator operator) => + BinaryOperator operator, BinaryOperationExpression node) => switch (operator) { BinaryOperator.plus => CalculationOperator.plus, BinaryOperator.minus => CalculationOperator.minus, BinaryOperator.times => CalculationOperator.times, BinaryOperator.dividedBy => CalculationOperator.dividedBy, - _ => throw UnsupportedError("Invalid calculation operator $operator.") + _ => throw _exception( + "This operation can't be used in a calculation.", node.operatorSpan) }; - SassColor visitColorExpression(ColorExpression node) => node.value; - - SassList visitListExpression(ListExpression node) => SassList( - node.contents.map((Expression expression) => expression.accept(this)), - node.separator, - brackets: node.hasBrackets); - - SassMap visitMapExpression(MapExpression node) { - var map = {}; - var keyNodes = {}; - for (var (key, value) in node.pairs) { - var keyValue = key.accept(this); - var valueValue = value.accept(this); - - var oldValue = map[keyValue]; - if (oldValue != null) { - var oldValueSpan = keyNodes[keyValue]?.span; - throw MultiSpanSassRuntimeException( - 'Duplicate key.', - key.span, - 'second key', - {if (oldValueSpan != null) oldValueSpan: 'first key'}, - _stackTrace(key.span)); - } - map[keyValue] = valueValue; - keyNodes[keyValue] = key; - } - return SassMap(map); - } - - Value visitFunctionExpression(FunctionExpression node) { - var function = _addExceptionSpan( - node, () => _getFunction(node.name, namespace: node.namespace)); - - if (function == null) { - if (node.namespace != null) { - throw _exception("Undefined function.", node.span); + /// Throws an error if [elements] contains two adjacent non-string values. + void _checkAdjacentCalculationValues( + List elements, ListExpression node) { + assert(elements.length > 1); + + for (var i = 1; i < elements.length; i++) { + var previous = elements[i - 1]; + var current = elements[i]; + if (previous is SassString || current is SassString) continue; + + var previousNode = node.contents[i - 1]; + var currentNode = node.contents[i]; + if (currentNode + case UnaryOperationExpression( + operator: UnaryOperator.minus || UnaryOperator.plus + ) || + NumberExpression(value: < 0)) { + // `calc(1 -2)` parses as a space-separated list whose second value is a + // unary operator or a negative number, but just saying it's an invalid + // expression doesn't help the user understand what's going wrong. We + // add special case error handling to help clarify the issue. + throw _exception( + '"+" and "-" must be surrounded by whitespace in calculations.', + currentNode.span.subspan(0, 1)); + } else { + throw _exception('Missing math operator.', + previousNode.span.expand(currentNode.span)); } - - function = PlainCssCallable(node.originalName); } - - var oldInFunction = _inFunction; - _inFunction = true; - var result = _addErrorSpan( - node, () => _runFunctionCallable(node.arguments, function, node)); - _inFunction = oldInFunction; - return result; } Value visitInterpolatedFunctionExpression( @@ -2473,14 +2691,6 @@ final class _EvaluateVisitor return result; } - /// Like `_environment.getFunction`, but also returns built-in - /// globally-available functions. - Callable? _getFunction(String name, {String? namespace}) { - var local = _environment.getFunction(name, namespace: namespace); - if (local != null || namespace != null) return local; - return _builtInFunctions[name]; - } - /// Evaluates the arguments in [arguments] as applied to [callable], and /// invokes [run] in a scope with those arguments defined. V _runUserDefinedCallable( diff --git a/lib/src/visitor/expression_to_calc.dart b/lib/src/visitor/expression_to_calc.dart index 961735655..aca554355 100644 --- a/lib/src/visitor/expression_to_calc.dart +++ b/lib/src/visitor/expression_to_calc.dart @@ -10,9 +10,13 @@ import 'replace_expression.dart'; /// This assumes that [expression] already returns a number. It's intended for /// use in end-user messaging, and may not produce directly evaluable /// expressions. -CalculationExpression expressionToCalc(Expression expression) => - CalculationExpression.calc( - expression.accept(const _MakeExpressionCalculationSafe()), +FunctionExpression expressionToCalc(Expression expression) => + FunctionExpression( + "calc", + ArgumentInvocation( + [expression.accept(const _MakeExpressionCalculationSafe())], + const {}, + expression.span), expression.span); /// A visitor that replaces constructs that can't be used in a calculation with @@ -20,8 +24,6 @@ CalculationExpression expressionToCalc(Expression expression) => class _MakeExpressionCalculationSafe with ReplaceExpressionVisitor { const _MakeExpressionCalculationSafe(); - Expression visitCalculationExpression(CalculationExpression node) => node; - Expression visitBinaryOperationExpression(BinaryOperationExpression node) => node .operator == BinaryOperator.modulo diff --git a/lib/src/visitor/interface/expression.dart b/lib/src/visitor/interface/expression.dart index db5f70f32..7d40c87a2 100644 --- a/lib/src/visitor/interface/expression.dart +++ b/lib/src/visitor/interface/expression.dart @@ -12,7 +12,6 @@ import '../../ast/sass.dart'; abstract interface class ExpressionVisitor { T visitBinaryOperationExpression(BinaryOperationExpression node); T visitBooleanExpression(BooleanExpression node); - T visitCalculationExpression(CalculationExpression node); T visitColorExpression(ColorExpression node); T visitInterpolatedFunctionExpression(InterpolatedFunctionExpression node); T visitFunctionExpression(FunctionExpression node); diff --git a/lib/src/visitor/recursive_ast.dart b/lib/src/visitor/recursive_ast.dart index 0b31aafe2..290572697 100644 --- a/lib/src/visitor/recursive_ast.dart +++ b/lib/src/visitor/recursive_ast.dart @@ -33,12 +33,6 @@ mixin RecursiveAstVisitor on RecursiveStatementVisitor super.visitAtRule(node); } - void visitCalculationExpression(CalculationExpression node) { - for (var argument in node.arguments) { - argument.accept(this); - } - } - void visitContentRule(ContentRule node) { visitArgumentInvocation(node.arguments); } @@ -110,8 +104,6 @@ mixin RecursiveAstVisitor on RecursiveStatementVisitor super.visitMediaRule(node); } - void visitMixinRule(MixinRule node) => visitCallableDeclaration(node); - void visitReturnRule(ReturnRule node) { visitExpression(node.expression); } @@ -128,7 +120,7 @@ mixin RecursiveAstVisitor on RecursiveStatementVisitor void visitUseRule(UseRule node) { for (var variable in node.configuration) { - variable.expression.accept(this); + visitExpression(variable.expression); } } @@ -160,7 +152,7 @@ mixin RecursiveAstVisitor on RecursiveStatementVisitor void visitForwardRule(ForwardRule node) { for (var variable in node.configuration) { - variable.expression.accept(this); + visitExpression(variable.expression); } } diff --git a/lib/src/visitor/replace_expression.dart b/lib/src/visitor/replace_expression.dart index b330cfbbf..43d93eebc 100644 --- a/lib/src/visitor/replace_expression.dart +++ b/lib/src/visitor/replace_expression.dart @@ -22,10 +22,6 @@ import 'interface/expression.dart'; /// /// {@category Visitor} mixin ReplaceExpressionVisitor implements ExpressionVisitor { - Expression visitCalculationExpression(CalculationExpression node) => - CalculationExpression(node.name, - node.arguments.map((argument) => argument.accept(this)), node.span); - Expression visitBinaryOperationExpression(BinaryOperationExpression node) => BinaryOperationExpression( node.operator, node.left.accept(this), node.right.accept(this)); diff --git a/lib/src/visitor/serialize.dart b/lib/src/visitor/serialize.dart index c0c071155..f86d6c7fb 100644 --- a/lib/src/visitor/serialize.dart +++ b/lib/src/visitor/serialize.dart @@ -518,13 +518,9 @@ final class _SerializeVisitor case Value(): value.accept(this); - case CalculationInterpolation(): - _buffer.write(value.value); - case CalculationOperation(:var operator, :var left, :var right): - var parenthesizeLeft = left is CalculationInterpolation || - (left is CalculationOperation && - left.operator.precedence < operator.precedence); + var parenthesizeLeft = left is CalculationOperation && + left.operator.precedence < operator.precedence; if (parenthesizeLeft) _buffer.writeCharCode($lparen); _writeCalculationValue(left); if (parenthesizeLeft) _buffer.writeCharCode($rparen); @@ -534,8 +530,7 @@ final class _SerializeVisitor _buffer.write(operator.operator); if (operatorWhitespace) _buffer.writeCharCode($space); - var parenthesizeRight = right is CalculationInterpolation || - (right is CalculationOperation && + var parenthesizeRight = (right is CalculationOperation && _parenthesizeCalculationRhs(operator, right.operator)) || (operator == CalculationOperator.dividedBy && right is SassNumber && diff --git a/pkg/sass_api/CHANGELOG.md b/pkg/sass_api/CHANGELOG.md index 1ba7b431c..d42f28444 100644 --- a/pkg/sass_api/CHANGELOG.md +++ b/pkg/sass_api/CHANGELOG.md @@ -1,3 +1,11 @@ +## 9.0.0 + +* Remove the `CalculationExpression` class and the associated visitor methods. + +* Add an `AstSearchVisitor` helper class. + +* Add an `Interpolation.isPlain` getter. + ## 8.2.1 * No user-visible changes. diff --git a/pkg/sass_api/lib/sass_api.dart b/pkg/sass_api/lib/sass_api.dart index 1f4b076e3..b0369f908 100644 --- a/pkg/sass_api/lib/sass_api.dart +++ b/pkg/sass_api/lib/sass_api.dart @@ -23,6 +23,7 @@ export 'package:sass/src/visitor/find_dependencies.dart'; export 'package:sass/src/visitor/interface/expression.dart'; export 'package:sass/src/visitor/interface/selector.dart'; export 'package:sass/src/visitor/interface/statement.dart'; +export 'package:sass/src/visitor/ast_search.dart'; export 'package:sass/src/visitor/recursive_ast.dart'; export 'package:sass/src/visitor/recursive_selector.dart'; export 'package:sass/src/visitor/recursive_statement.dart'; diff --git a/pkg/sass_api/pubspec.yaml b/pkg/sass_api/pubspec.yaml index 21f333de9..0528454f0 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: 8.2.1 +version: 9.0.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.66.1 + sass: 1.67.0 dev_dependencies: dartdoc: ^6.0.0 diff --git a/pubspec.yaml b/pubspec.yaml index 5c2a1698b..e9cceabb1 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass -version: 1.67.0-dev +version: 1.67.0 description: A Sass implementation in Dart. homepage: https://github.com/sass/dart-sass diff --git a/test/dart_api/value/calculation_test.dart b/test/dart_api/value/calculation_test.dart index 594842cee..ed284db04 100644 --- a/test/dart_api/value/calculation_test.dart +++ b/test/dart_api/value/calculation_test.dart @@ -69,5 +69,12 @@ void main() { .assertNumber(), equals(SassNumber(8.5))); }); + + test('interpolation', () { + var result = SassCalculation.calc(CalculationInterpolation('1 + 2')) + .assertCalculation(); + expect(result.name, equals('calc')); + expect(result.arguments[0], equals(SassString('(1 + 2)'))); + }); }); } diff --git a/test/embedded/function_test.dart b/test/embedded/function_test.dart index af4bd62db..d927a780f 100644 --- a/test/embedded/function_test.dart +++ b/test/embedded/function_test.dart @@ -761,7 +761,7 @@ void main() { equals(Value_Calculation() ..name = "calc" ..arguments.add(Value_Calculation_CalculationValue() - ..interpolation = "var(--foo)"))); + ..string = "var(--foo)"))); }); test("with number arguments", () async { @@ -1429,7 +1429,7 @@ void main() { ..name = "calc" ..arguments.add(Value_Calculation_CalculationValue() ..interpolation = "var(--foo)"))), - "calc(var(--foo))"); + "calc((var(--foo)))"); }); test("with number arguments", () async { From 7370d6a97df58966f72e896f8dd53dfe2ab3c4df Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Fri, 15 Sep 2023 16:54:30 -0700 Subject: [PATCH 07/30] Fix changelog for 1.67.0 (#2085) --- CHANGELOG.md | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e828616a6..7c8055dd7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,18 +13,13 @@ CSS calculations (including `abs()`, `min()`, `max()`, and `round()` whose names overlap with global Sass functions). -* As a consequence of the change in calculation parsing described above, - calculation functions containing interpolation are now parsed more strictly - than before. However, all interpolations that would have produced valid CSS - will continue to work, so this is not considered a breaking change. - -* Interpolations in calculation functions that aren't used in a position that - could also have a normal calculation value are now deprecated. For example, - `calc(1px #{"+ 2px"})` is deprecated, but `calc(1px + #{"2px"})` is still - allowed. This deprecation is named `calc-interp`. See [the Sass website] for - more information. - - [the Sass website]: https://sass-lang.com/d/calc-interp +* **Breaking change**: As a consequence of the change in calculation parsing + described above, calculation functions containing interpolation are now parsed + more strictly than before. However, _almost_ all interpolations that would + have produced valid CSS will continue to work. The only exception is + `#{$variable}%` which is not valid in Sass and is no longer valid in + calculations. Instead of this, either use `$variable` directly and ensure it + already has the `%` unit, or write `($variable * 1%)`. * **Potentially breaking bug fix**: The importer used to load a given file is no longer used to load absolute URLs that appear in that file. This was From 37e0ed54da1bd3ba3d4f4127da3a9575f7a9224e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Fri, 15 Sep 2023 16:55:49 -0700 Subject: [PATCH 08/30] Fix source span for calculation deprecation warnings (#2084) Co-authored-by: Natalie Weizenbaum --- CHANGELOG.md | 4 ++++ lib/src/visitor/async_evaluate.dart | 5 +++++ lib/src/visitor/evaluate.dart | 7 ++++++- pubspec.yaml | 2 +- 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c8055dd7..31dc00661 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## 1.67.1 + +* Fix the source spans associated with the `abs-percent` deprecation. + ## 1.67.0 * All functions defined in CSS Values and Units 4 are now once again parsed as diff --git a/lib/src/visitor/async_evaluate.dart b/lib/src/visitor/async_evaluate.dart index b8917ba42..6e16a71bf 100644 --- a/lib/src/visitor/async_evaluate.dart +++ b/lib/src/visitor/async_evaluate.dart @@ -2431,6 +2431,9 @@ final class _EvaluateVisitor return SassCalculation.unsimplified(node.name, arguments); } + var oldCallableNode = _callableNode; + _callableNode = node; + try { return switch (node.name.toLowerCase()) { "calc" => SassCalculation.calc(arguments[0]), @@ -2471,6 +2474,8 @@ final class _EvaluateVisitor _verifyCompatibleNumbers(arguments, node.arguments.positional); } throwWithTrace(_exception(error.message, node.span), error, stackTrace); + } finally { + _callableNode = oldCallableNode; } } diff --git a/lib/src/visitor/evaluate.dart b/lib/src/visitor/evaluate.dart index 521e6cd02..87f3ef6c0 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: ccd4ec1a65cfc2487fccd30481d427086f5c76cc +// Checksum: 7669de19668af665d1a9a60cf67e53e071bf415e // // ignore_for_file: unused_import @@ -2411,6 +2411,9 @@ final class _EvaluateVisitor return SassCalculation.unsimplified(node.name, arguments); } + var oldCallableNode = _callableNode; + _callableNode = node; + try { return switch (node.name.toLowerCase()) { "calc" => SassCalculation.calc(arguments[0]), @@ -2451,6 +2454,8 @@ final class _EvaluateVisitor _verifyCompatibleNumbers(arguments, node.arguments.positional); } throwWithTrace(_exception(error.message, node.span), error, stackTrace); + } finally { + _callableNode = oldCallableNode; } } diff --git a/pubspec.yaml b/pubspec.yaml index e9cceabb1..3036d6b5e 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass -version: 1.67.0 +version: 1.67.1-dev description: A Sass implementation in Dart. homepage: https://github.com/sass/dart-sass From c22c84d2c5cdd3d0c0333eb1c5b37bedb8b1cc3c Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Mon, 18 Sep 2023 14:06:55 -0700 Subject: [PATCH 09/30] Update changelog for sass/embedded-host-node#248 (#2086) --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 31dc00661..4e5dace4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ * Fix the source spans associated with the `abs-percent` deprecation. +### Embedded Sass + +* Properly include TypeScript types in the `sass-embedded` package. + ## 1.67.0 * All functions defined in CSS Values and Units 4 are now once again parsed as From 69f1847bae60df16fe7d7bdfdfa794da8bf8c3d4 Mon Sep 17 00:00:00 2001 From: Olle Jonsson Date: Mon, 18 Sep 2023 23:09:01 +0200 Subject: [PATCH 10/30] CI: Configure dependabot to update GitHub Actions, too (#2087) --- .github/dependabot.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index d24aa6443..44ca4f044 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -4,3 +4,7 @@ updates: directory: "/" schedule: interval: "weekly" + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "weekly" From 13c9fb3c9d97d4d9582ccd33689111d74024261d Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Mon, 18 Sep 2023 16:00:24 -0700 Subject: [PATCH 11/30] Expose the containing URL to importers under some circumstances (#2083) Closes #1946 --- .github/workflows/ci.yml | 1 + lib/src/async_import_cache.dart | 76 ++++++---- lib/src/embedded/dispatcher.dart | 19 ++- lib/src/embedded/importer/file.dart | 15 +- lib/src/embedded/importer/host.dart | 35 ++++- lib/src/import_cache.dart | 74 +++++---- lib/src/importer.dart | 2 + lib/src/importer/async.dart | 27 ++++ lib/src/importer/js_to_dart/async.dart | 25 +++- 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 | 25 +++- lib/src/importer/js_to_dart/utils.dart | 16 ++ lib/src/importer/utils.dart | 24 +++ lib/src/js/compile.dart | 22 ++- lib/src/js/compile_options.dart | 2 +- lib/src/js/importer.dart | 14 +- lib/src/util/span.dart | 6 +- test/dart_api/importer_test.dart | 93 ++++++++++++ test/dart_api/test_importer.dart | 14 +- test/embedded/importer_test.dart | 158 ++++++++++++++++++++ tool/grind/utils.dart | 4 + 22 files changed, 574 insertions(+), 98 deletions(-) create mode 100644 lib/src/importer/js_to_dart/utils.dart diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2f8fdc4f2..ad796a58c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -137,6 +137,7 @@ jobs: sass_spec_js_embedded: name: 'JS API Tests | Embedded | Node ${{ matrix.node-version }} | ${{ matrix.os }}' runs-on: ${{ matrix.os }}-latest + if: "github.event_name != 'pull_request' || !contains(github.event.pull_request.body, 'skip sass-embedded')" strategy: fail-fast: false diff --git a/lib/src/async_import_cache.dart b/lib/src/async_import_cache.dart index 019b3414a..33fc26cce 100644 --- a/lib/src/async_import_cache.dart +++ b/lib/src/async_import_cache.dart @@ -122,6 +122,9 @@ final class AsyncImportCache { /// Canonicalizes [url] according to one of this cache's importers. /// + /// The [baseUrl] should be the canonical URL of the stylesheet that contains + /// the load, if it exists. + /// /// Returns the importer that was used to canonicalize [url], the canonical /// URL, and the URL that was passed to the importer (which may be resolved /// relative to [baseUrl] if it's passed). @@ -139,33 +142,30 @@ final class AsyncImportCache { if (isBrowser && (baseImporter == null || baseImporter is NoOpImporter) && _importers.isEmpty) { - throw "Custom importers are required to load stylesheets when compiling in the browser."; + throw "Custom importers are required to load stylesheets when compiling " + "in the browser."; } if (baseImporter != null && url.scheme == '') { - var relativeResult = await putIfAbsentAsync(_relativeCanonicalizeCache, ( - url, - forImport: forImport, - baseImporter: baseImporter, - baseUrl: baseUrl - ), () async { - var resolvedUrl = baseUrl?.resolveUri(url) ?? url; - if (await _canonicalize(baseImporter, resolvedUrl, forImport) - case var canonicalUrl?) { - return (baseImporter, canonicalUrl, originalUrl: resolvedUrl); - } else { - return null; - } - }); + var relativeResult = await putIfAbsentAsync( + _relativeCanonicalizeCache, + ( + url, + forImport: forImport, + baseImporter: baseImporter, + baseUrl: baseUrl + ), + () => _canonicalize(baseImporter, baseUrl?.resolveUri(url) ?? url, + baseUrl, forImport)); if (relativeResult != null) return relativeResult; } return await putIfAbsentAsync( _canonicalizeCache, (url, forImport: forImport), () async { for (var importer in _importers) { - if (await _canonicalize(importer, url, forImport) - case var canonicalUrl?) { - return (importer, canonicalUrl, originalUrl: url); + if (await _canonicalize(importer, url, baseUrl, forImport) + case var result?) { + return result; } } @@ -175,18 +175,36 @@ final class AsyncImportCache { /// Calls [importer.canonicalize] and prints a deprecation warning if it /// returns a relative URL. - Future _canonicalize( - AsyncImporter importer, Uri url, bool forImport) async { - var result = await (forImport - ? inImportRule(() => importer.canonicalize(url)) - : importer.canonicalize(url)); - if (result?.scheme == '') { - _logger.warnForDeprecation(Deprecation.relativeCanonical, """ -Importer $importer canonicalized $url to $result. -Relative canonical URLs are deprecated and will eventually be disallowed. -"""); + /// + /// If [resolveUrl] is `true`, this resolves [url] relative to [baseUrl] + /// before passing it to [importer]. + Future _canonicalize( + AsyncImporter importer, Uri url, Uri? baseUrl, bool forImport, + {bool resolveUrl = false}) async { + var resolved = + resolveUrl && baseUrl != null ? baseUrl.resolveUri(url) : url; + var canonicalize = forImport + ? () => inImportRule(() => importer.canonicalize(resolved)) + : () => importer.canonicalize(resolved); + + var passContainingUrl = baseUrl != null && + (url.scheme == '' || await importer.isNonCanonicalScheme(url.scheme)); + var result = await withContainingUrl( + passContainingUrl ? baseUrl : null, canonicalize); + if (result == null) return null; + + if (result.scheme == '') { + _logger.warnForDeprecation( + Deprecation.relativeCanonical, + "Importer $importer canonicalized $resolved to $result.\n" + "Relative canonical URLs are deprecated and will eventually be " + "disallowed."); + } else if (await importer.isNonCanonicalScheme(result.scheme)) { + throw "Importer $importer canonicalized $resolved to $result, which " + "uses a scheme declared as non-canonical."; } - return result; + + return (importer, result, originalUrl: resolved); } /// Tries to import [url] using one of this cache's importers. diff --git a/lib/src/embedded/dispatcher.dart b/lib/src/embedded/dispatcher.dart index 22df57fca..d99098735 100644 --- a/lib/src/embedded/dispatcher.dart +++ b/lib/src/embedded/dispatcher.dart @@ -206,19 +206,32 @@ final class Dispatcher { InboundMessage_CompileRequest_Importer importer) { switch (importer.whichImporter()) { case InboundMessage_CompileRequest_Importer_Importer.path: + _checkNoNonCanonicalScheme(importer); return sass.FilesystemImporter(importer.path); case InboundMessage_CompileRequest_Importer_Importer.importerId: - return HostImporter(this, importer.importerId); + return HostImporter( + this, importer.importerId, importer.nonCanonicalScheme); case InboundMessage_CompileRequest_Importer_Importer.fileImporterId: + _checkNoNonCanonicalScheme(importer); return FileImporter(this, importer.fileImporterId); case InboundMessage_CompileRequest_Importer_Importer.notSet: + _checkNoNonCanonicalScheme(importer); return null; } } + /// Throws a [ProtocolError] if [importer] contains one or more + /// `nonCanonicalScheme`s. + void _checkNoNonCanonicalScheme( + InboundMessage_CompileRequest_Importer importer) { + if (importer.nonCanonicalScheme.isEmpty) return; + throw paramsError("Importer.non_canonical_scheme may only be set along " + "with Importer.importer.importer_id"); + } + /// Sends [event] to the host. void sendLog(OutboundMessage_LogEvent event) => _send(OutboundMessage()..logEvent = event); @@ -278,9 +291,7 @@ final class Dispatcher { InboundMessage_Message.versionRequest => throw paramsError("VersionRequest must have compilation ID 0."), InboundMessage_Message.notSet => - throw parseError("InboundMessage.message is not set."), - _ => - throw parseError("Unknown message type: ${message.toDebugString()}") + throw parseError("InboundMessage.message is not set.") }; if (message.id != _outboundRequestId) { diff --git a/lib/src/embedded/importer/file.dart b/lib/src/embedded/importer/file.dart index 8250515eb..0dbccfed5 100644 --- a/lib/src/embedded/importer/file.dart +++ b/lib/src/embedded/importer/file.dart @@ -24,11 +24,14 @@ final class FileImporter extends ImporterBase { Uri? canonicalize(Uri url) { if (url.scheme == 'file') return _filesystemImporter.canonicalize(url); - var response = - dispatcher.sendFileImportRequest(OutboundMessage_FileImportRequest() - ..importerId = _importerId - ..url = url.toString() - ..fromImport = fromImport); + var request = OutboundMessage_FileImportRequest() + ..importerId = _importerId + ..url = url.toString() + ..fromImport = fromImport; + if (containingUrl case var containingUrl?) { + request.containingUrl = containingUrl.toString(); + } + var response = dispatcher.sendFileImportRequest(request); switch (response.whichResult()) { case InboundMessage_FileImportResponse_Result.fileUrl: @@ -49,5 +52,7 @@ final class FileImporter extends ImporterBase { ImporterResult? load(Uri url) => _filesystemImporter.load(url); + bool isNonCanonicalScheme(String scheme) => scheme != 'file'; + String toString() => "FileImporter"; } diff --git a/lib/src/embedded/importer/host.dart b/lib/src/embedded/importer/host.dart index a1d5eed31..5819be1ec 100644 --- a/lib/src/embedded/importer/host.dart +++ b/lib/src/embedded/importer/host.dart @@ -2,7 +2,10 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. +import '../../exception.dart'; import '../../importer.dart'; +import '../../importer/utils.dart'; +import '../../util/span.dart'; import '../dispatcher.dart'; import '../embedded_sass.pb.dart' hide SourceSpan; import '../utils.dart'; @@ -13,14 +16,31 @@ final class HostImporter extends ImporterBase { /// The host-provided ID of the importer to invoke. final int _importerId; - HostImporter(Dispatcher dispatcher, this._importerId) : super(dispatcher); + /// The set of URL schemes that this importer promises never to return from + /// [canonicalize]. + final Set _nonCanonicalSchemes; + + HostImporter(Dispatcher dispatcher, this._importerId, + Iterable nonCanonicalSchemes) + : _nonCanonicalSchemes = Set.unmodifiable(nonCanonicalSchemes), + super(dispatcher) { + for (var scheme in _nonCanonicalSchemes) { + if (isValidUrlScheme(scheme)) continue; + throw SassException( + '"$scheme" isn\'t a valid URL scheme (for example "file").', + bogusSpan); + } + } Uri? canonicalize(Uri url) { - var response = - dispatcher.sendCanonicalizeRequest(OutboundMessage_CanonicalizeRequest() - ..importerId = _importerId - ..url = url.toString() - ..fromImport = fromImport); + var request = OutboundMessage_CanonicalizeRequest() + ..importerId = _importerId + ..url = url.toString() + ..fromImport = fromImport; + if (containingUrl case var containingUrl?) { + request.containingUrl = containingUrl.toString(); + } + var response = dispatcher.sendCanonicalizeRequest(request); return switch (response.whichResult()) { InboundMessage_CanonicalizeResponse_Result.url => @@ -47,5 +67,8 @@ final class HostImporter extends ImporterBase { }; } + bool isNonCanonicalScheme(String scheme) => + _nonCanonicalSchemes.contains(scheme); + String toString() => "HostImporter"; } diff --git a/lib/src/import_cache.dart b/lib/src/import_cache.dart index b9c48a6f8..bc526d7a3 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: 3e4cae79c03ce2af6626b1822f1468523b401e90 +// Checksum: ff52307a3bc93358ddc46f1e76120894fa3e071f // // ignore_for_file: unused_import @@ -124,6 +124,9 @@ final class ImportCache { /// Canonicalizes [url] according to one of this cache's importers. /// + /// The [baseUrl] should be the canonical URL of the stylesheet that contains + /// the load, if it exists. + /// /// Returns the importer that was used to canonicalize [url], the canonical /// URL, and the URL that was passed to the importer (which may be resolved /// relative to [baseUrl] if it's passed). @@ -139,31 +142,27 @@ final class ImportCache { if (isBrowser && (baseImporter == null || baseImporter is NoOpImporter) && _importers.isEmpty) { - throw "Custom importers are required to load stylesheets when compiling in the browser."; + throw "Custom importers are required to load stylesheets when compiling " + "in the browser."; } if (baseImporter != null && url.scheme == '') { - var relativeResult = _relativeCanonicalizeCache.putIfAbsent(( - url, - forImport: forImport, - baseImporter: baseImporter, - baseUrl: baseUrl - ), () { - var resolvedUrl = baseUrl?.resolveUri(url) ?? url; - if (_canonicalize(baseImporter, resolvedUrl, forImport) - case var canonicalUrl?) { - return (baseImporter, canonicalUrl, originalUrl: resolvedUrl); - } else { - return null; - } - }); + var relativeResult = _relativeCanonicalizeCache.putIfAbsent( + ( + url, + forImport: forImport, + baseImporter: baseImporter, + baseUrl: baseUrl + ), + () => _canonicalize(baseImporter, baseUrl?.resolveUri(url) ?? url, + baseUrl, forImport)); if (relativeResult != null) return relativeResult; } return _canonicalizeCache.putIfAbsent((url, forImport: forImport), () { for (var importer in _importers) { - if (_canonicalize(importer, url, forImport) case var canonicalUrl?) { - return (importer, canonicalUrl, originalUrl: url); + if (_canonicalize(importer, url, baseUrl, forImport) case var result?) { + return result; } } @@ -173,17 +172,36 @@ final class ImportCache { /// Calls [importer.canonicalize] and prints a deprecation warning if it /// returns a relative URL. - Uri? _canonicalize(Importer importer, Uri url, bool forImport) { - var result = (forImport - ? inImportRule(() => importer.canonicalize(url)) - : importer.canonicalize(url)); - if (result?.scheme == '') { - _logger.warnForDeprecation(Deprecation.relativeCanonical, """ -Importer $importer canonicalized $url to $result. -Relative canonical URLs are deprecated and will eventually be disallowed. -"""); + /// + /// If [resolveUrl] is `true`, this resolves [url] relative to [baseUrl] + /// before passing it to [importer]. + CanonicalizeResult? _canonicalize( + Importer importer, Uri url, Uri? baseUrl, bool forImport, + {bool resolveUrl = false}) { + var resolved = + resolveUrl && baseUrl != null ? baseUrl.resolveUri(url) : url; + var canonicalize = forImport + ? () => inImportRule(() => importer.canonicalize(resolved)) + : () => importer.canonicalize(resolved); + + var passContainingUrl = baseUrl != null && + (url.scheme == '' || importer.isNonCanonicalScheme(url.scheme)); + var result = + withContainingUrl(passContainingUrl ? baseUrl : null, canonicalize); + if (result == null) return null; + + if (result.scheme == '') { + _logger.warnForDeprecation( + Deprecation.relativeCanonical, + "Importer $importer canonicalized $resolved to $result.\n" + "Relative canonical URLs are deprecated and will eventually be " + "disallowed."); + } else if (importer.isNonCanonicalScheme(result.scheme)) { + throw "Importer $importer canonicalized $resolved to $result, which " + "uses a scheme declared as non-canonical."; } - return result; + + return (importer, result, originalUrl: resolved); } /// Tries to import [url] using one of this cache's importers. diff --git a/lib/src/importer.dart b/lib/src/importer.dart index 12d626a21..04a08c0f8 100644 --- a/lib/src/importer.dart +++ b/lib/src/importer.dart @@ -40,4 +40,6 @@ abstract class Importer extends AsyncImporter { DateTime modificationTime(Uri url) => DateTime.now(); bool couldCanonicalize(Uri url, Uri canonicalUrl) => true; + + bool isNonCanonicalScheme(String scheme) => false; } diff --git a/lib/src/importer/async.dart b/lib/src/importer/async.dart index 912d05084..d7d6951fe 100644 --- a/lib/src/importer/async.dart +++ b/lib/src/importer/async.dart @@ -41,6 +41,21 @@ abstract class AsyncImporter { @nonVirtual bool get fromImport => utils.fromImport; + /// The canonical URL of the stylesheet that caused the current [canonicalize] + /// invocation. + /// + /// This is only set when the containing stylesheet has a canonical URL, and + /// when the URL being canonicalized is either relative or has a scheme for + /// which [isNonCanonicalScheme] returns `true`. This restriction ensures that + /// canonical URLs are always interpreted the same way regardless of their + /// context. + /// + /// Subclasses should only access this from within calls to [canonicalize]. + /// Outside of that context, its value is undefined and subject to change. + @protected + @nonVirtual + Uri? get containingUrl => utils.containingUrl; + /// If [url] is recognized by this importer, returns its canonical format. /// /// Note that canonical URLs *must* be absolute, including a scheme. Returning @@ -137,4 +152,16 @@ abstract class AsyncImporter { /// [url] would actually resolve to [canonicalUrl]. Subclasses are not allowed /// to return false negatives. FutureOr couldCanonicalize(Uri url, Uri canonicalUrl) => true; + + /// Returns whether the given URL scheme (without `:`) should be considered + /// "non-canonical" for this importer. + /// + /// An importer may not return a URL with a non-canonical scheme from + /// [canonicalize]. In exchange, [containingUrl] is available within + /// [canonicalize] for absolute URLs with non-canonical schemes so that the + /// importer can resolve those URLs differently based on where they're loaded. + /// + /// This must always return the same value for the same [scheme]. It is + /// expected to be very efficient. + FutureOr isNonCanonicalScheme(String scheme) => false; } diff --git a/lib/src/importer/js_to_dart/async.dart b/lib/src/importer/js_to_dart/async.dart index 529789d11..11ffbd735 100644 --- a/lib/src/importer/js_to_dart/async.dart +++ b/lib/src/importer/js_to_dart/async.dart @@ -14,21 +14,35 @@ import '../../js/utils.dart'; import '../../util/nullable.dart'; import '../async.dart'; import '../result.dart'; +import 'utils.dart'; /// A wrapper for a potentially-asynchronous JS API importer that exposes it as /// a Dart [AsyncImporter]. final class JSToDartAsyncImporter extends AsyncImporter { /// The wrapped canonicalize function. - final Object? Function(String, CanonicalizeOptions) _canonicalize; + final Object? Function(String, CanonicalizeContext) _canonicalize; /// The wrapped load function. final Object? Function(JSUrl) _load; - JSToDartAsyncImporter(this._canonicalize, this._load); + /// The set of URL schemes that this importer promises never to return from + /// [canonicalize]. + final Set _nonCanonicalSchemes; + + JSToDartAsyncImporter( + this._canonicalize, this._load, Iterable? nonCanonicalSchemes) + : _nonCanonicalSchemes = nonCanonicalSchemes == null + ? const {} + : Set.unmodifiable(nonCanonicalSchemes) { + _nonCanonicalSchemes.forEach(validateUrlScheme); + } FutureOr canonicalize(Uri url) async { var result = wrapJSExceptions(() => _canonicalize( - url.toString(), CanonicalizeOptions(fromImport: fromImport))); + url.toString(), + CanonicalizeContext( + fromImport: fromImport, + containingUrl: containingUrl.andThen(dartToJSUrl)))); if (isPromise(result)) result = await promiseToFuture(result as Promise); if (result == null) return null; @@ -42,7 +56,7 @@ final class JSToDartAsyncImporter extends AsyncImporter { if (isPromise(result)) result = await promiseToFuture(result as Promise); if (result == null) return null; - result as NodeImporterResult; + result as JSImporterResult; var contents = result.contents; if (!isJsString(contents)) { jsThrow(ArgumentError.value(contents, 'contents', @@ -59,4 +73,7 @@ final class JSToDartAsyncImporter extends AsyncImporter { syntax: parseSyntax(syntax), sourceMapUrl: result.sourceMapUrl.andThen(jsToDartUrl)); } + + bool isNonCanonicalScheme(String scheme) => + _nonCanonicalSchemes.contains(scheme); } diff --git a/lib/src/importer/js_to_dart/async_file.dart b/lib/src/importer/js_to_dart/async_file.dart index 374783f08..e984531dc 100644 --- a/lib/src/importer/js_to_dart/async_file.dart +++ b/lib/src/importer/js_to_dart/async_file.dart @@ -11,6 +11,7 @@ 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 '../filesystem.dart'; import '../result.dart'; @@ -26,7 +27,7 @@ final _filesystemImporter = FilesystemImporter('.'); /// it as a Dart [AsyncImporter]. final class JSToDartAsyncFileImporter extends AsyncImporter { /// The wrapped `findFileUrl` function. - final Object? Function(String, CanonicalizeOptions) _findFileUrl; + final Object? Function(String, CanonicalizeContext) _findFileUrl; JSToDartAsyncFileImporter(this._findFileUrl); @@ -34,7 +35,10 @@ final class JSToDartAsyncFileImporter extends AsyncImporter { if (url.scheme == 'file') return _filesystemImporter.canonicalize(url); var result = wrapJSExceptions(() => _findFileUrl( - url.toString(), CanonicalizeOptions(fromImport: fromImport))); + url.toString(), + CanonicalizeContext( + fromImport: fromImport, + containingUrl: containingUrl.andThen(dartToJSUrl)))); if (isPromise(result)) result = await promiseToFuture(result as Promise); if (result == null) return null; if (!isJSUrl(result)) { @@ -58,4 +62,6 @@ final class JSToDartAsyncFileImporter extends AsyncImporter { bool couldCanonicalize(Uri url, Uri canonicalUrl) => _filesystemImporter.couldCanonicalize(url, canonicalUrl); + + bool isNonCanonicalScheme(String scheme) => scheme != 'file'; } diff --git a/lib/src/importer/js_to_dart/file.dart b/lib/src/importer/js_to_dart/file.dart index 3772e87f5..9ad474d00 100644 --- a/lib/src/importer/js_to_dart/file.dart +++ b/lib/src/importer/js_to_dart/file.dart @@ -9,6 +9,7 @@ import '../../importer.dart'; import '../../js/importer.dart'; import '../../js/url.dart'; import '../../js/utils.dart'; +import '../../util/nullable.dart'; import '../utils.dart'; /// A filesystem importer to use for most implementation details of @@ -21,7 +22,7 @@ final _filesystemImporter = FilesystemImporter('.'); /// it as a Dart [AsyncImporter]. final class JSToDartFileImporter extends Importer { /// The wrapped `findFileUrl` function. - final Object? Function(String, CanonicalizeOptions) _findFileUrl; + final Object? Function(String, CanonicalizeContext) _findFileUrl; JSToDartFileImporter(this._findFileUrl); @@ -29,7 +30,10 @@ final class JSToDartFileImporter extends Importer { if (url.scheme == 'file') return _filesystemImporter.canonicalize(url); var result = wrapJSExceptions(() => _findFileUrl( - url.toString(), CanonicalizeOptions(fromImport: fromImport))); + url.toString(), + CanonicalizeContext( + fromImport: fromImport, + containingUrl: containingUrl.andThen(dartToJSUrl)))); if (result == null) return null; if (isPromise(result)) { @@ -57,4 +61,6 @@ final class JSToDartFileImporter extends Importer { bool couldCanonicalize(Uri url, Uri canonicalUrl) => _filesystemImporter.couldCanonicalize(url, canonicalUrl); + + bool isNonCanonicalScheme(String scheme) => scheme != 'file'; } diff --git a/lib/src/importer/js_to_dart/sync.dart b/lib/src/importer/js_to_dart/sync.dart index da05420ed..f69dafb35 100644 --- a/lib/src/importer/js_to_dart/sync.dart +++ b/lib/src/importer/js_to_dart/sync.dart @@ -10,21 +10,35 @@ import '../../js/importer.dart'; import '../../js/url.dart'; import '../../js/utils.dart'; import '../../util/nullable.dart'; +import 'utils.dart'; /// A wrapper for a synchronous JS API importer that exposes it as a Dart /// [Importer]. final class JSToDartImporter extends Importer { /// The wrapped canonicalize function. - final Object? Function(String, CanonicalizeOptions) _canonicalize; + final Object? Function(String, CanonicalizeContext) _canonicalize; /// The wrapped load function. final Object? Function(JSUrl) _load; - JSToDartImporter(this._canonicalize, this._load); + /// The set of URL schemes that this importer promises never to return from + /// [canonicalize]. + final Set _nonCanonicalSchemes; + + JSToDartImporter( + this._canonicalize, this._load, Iterable? nonCanonicalSchemes) + : _nonCanonicalSchemes = nonCanonicalSchemes == null + ? const {} + : Set.unmodifiable(nonCanonicalSchemes) { + _nonCanonicalSchemes.forEach(validateUrlScheme); + } Uri? canonicalize(Uri url) { var result = wrapJSExceptions(() => _canonicalize( - url.toString(), CanonicalizeOptions(fromImport: fromImport))); + url.toString(), + CanonicalizeContext( + fromImport: fromImport, + containingUrl: containingUrl.andThen(dartToJSUrl)))); if (result == null) return null; if (isJSUrl(result)) return jsToDartUrl(result as JSUrl); @@ -47,7 +61,7 @@ final class JSToDartImporter extends Importer { "functions.")); } - result as NodeImporterResult; + result as JSImporterResult; var contents = result.contents; if (!isJsString(contents)) { jsThrow(ArgumentError.value(contents, 'contents', @@ -64,4 +78,7 @@ final class JSToDartImporter extends Importer { syntax: parseSyntax(syntax), sourceMapUrl: result.sourceMapUrl.andThen(jsToDartUrl)); } + + bool isNonCanonicalScheme(String scheme) => + _nonCanonicalSchemes.contains(scheme); } diff --git a/lib/src/importer/js_to_dart/utils.dart b/lib/src/importer/js_to_dart/utils.dart new file mode 100644 index 000000000..6cd2106f0 --- /dev/null +++ b/lib/src/importer/js_to_dart/utils.dart @@ -0,0 +1,16 @@ +// Copyright 2023 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:node_interop/js.dart'; + +import '../../js/utils.dart'; +import '../utils.dart'; + +/// Throws a JsError if [scheme] isn't a valid URL scheme. +void validateUrlScheme(String scheme) { + if (!isValidUrlScheme(scheme)) { + jsThrow( + JsError('"$scheme" isn\'t a valid URL scheme (for example "file").')); + } +} diff --git a/lib/src/importer/utils.dart b/lib/src/importer/utils.dart index 464a050f8..a68ae6f5e 100644 --- a/lib/src/importer/utils.dart +++ b/lib/src/importer/utils.dart @@ -17,11 +17,29 @@ import '../io.dart'; /// 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]) { + null => throw StateError( + "containingUrl may only be accessed within a call to canonicalize()."), + #_none => null, + Uri url => url, + var value => throw StateError( + "Unexpected Zone.current[#_containingUrl] 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}); +/// 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}); + /// Resolves an imported path using the same logic as the filesystem importer. /// /// This tries to fill in extensions and partial prefixes and check for a @@ -82,3 +100,9 @@ String? _exactlyOne(List paths) => switch (paths) { /// /// Otherwise, returns `null`. T? _ifInImport(T callback()) => fromImport ? callback() : null; + +/// A regular expression matching valid URL schemes. +final _urlSchemeRegExp = RegExp(r"^[a-z0-9+.-]+$"); + +/// Returns whether [scheme] is a valid URL scheme. +bool isValidUrlScheme(String scheme) => _urlSchemeRegExp.hasMatch(scheme); diff --git a/lib/src/js/compile.dart b/lib/src/js/compile.dart index af5702308..0e734dc77 100644 --- a/lib/src/js/compile.dart +++ b/lib/src/js/compile.dart @@ -184,7 +184,7 @@ OutputStyle _parseOutputStyle(String? style) => switch (style) { AsyncImporter _parseAsyncImporter(Object? importer) { if (importer == null) jsThrow(JsError("Importers may not be null.")); - importer as NodeImporter; + importer as JSImporter; var canonicalize = importer.canonicalize; var load = importer.load; if (importer.findFileUrl case var findFileUrl?) { @@ -200,7 +200,8 @@ AsyncImporter _parseAsyncImporter(Object? importer) { "An importer must have either canonicalize and load methods, or a " "findFileUrl method.")); } else { - return JSToDartAsyncImporter(canonicalize, load); + return JSToDartAsyncImporter(canonicalize, load, + _normalizeNonCanonicalSchemes(importer.nonCanonicalScheme)); } } @@ -208,7 +209,7 @@ AsyncImporter _parseAsyncImporter(Object? importer) { Importer _parseImporter(Object? importer) { if (importer == null) jsThrow(JsError("Importers may not be null.")); - importer as NodeImporter; + importer as JSImporter; var canonicalize = importer.canonicalize; var load = importer.load; if (importer.findFileUrl case var findFileUrl?) { @@ -224,10 +225,23 @@ Importer _parseImporter(Object? importer) { "An importer must have either canonicalize and load methods, or a " "findFileUrl method.")); } else { - return JSToDartImporter(canonicalize, load); + return JSToDartImporter(canonicalize, load, + _normalizeNonCanonicalSchemes(importer.nonCanonicalScheme)); } } +/// Converts a JS-style `nonCanonicalScheme` field into the form expected by +/// Dart classes. +List? _normalizeNonCanonicalSchemes(Object? schemes) => + switch (schemes) { + String scheme => [scheme], + List schemes => schemes.cast(), + null => null, + _ => jsThrow( + JsError('nonCanonicalScheme must be a string or list of strings, was ' + '"$schemes"')) + }; + /// Implements the simplification algorithm for custom function return `Value`s. /// {@link https://github.com/sass/sass/blob/main/spec/types/calculation.md#simplifying-a-calculationvalue} Value _simplifyValue(Value value) => switch (value) { diff --git a/lib/src/js/compile_options.dart b/lib/src/js/compile_options.dart index 603adba4e..1789539d5 100644 --- a/lib/src/js/compile_options.dart +++ b/lib/src/js/compile_options.dart @@ -30,5 +30,5 @@ class CompileOptions { class CompileStringOptions extends CompileOptions { external String? get syntax; external JSUrl? get url; - external NodeImporter? get importer; + external JSImporter? get importer; } diff --git a/lib/src/js/importer.dart b/lib/src/js/importer.dart index 3be952951..0469737ee 100644 --- a/lib/src/js/importer.dart +++ b/lib/src/js/importer.dart @@ -8,23 +8,25 @@ import 'url.dart'; @JS() @anonymous -class NodeImporter { - external Object? Function(String, CanonicalizeOptions)? get canonicalize; +class JSImporter { + external Object? Function(String, CanonicalizeContext)? get canonicalize; external Object? Function(JSUrl)? get load; - external Object? Function(String, CanonicalizeOptions)? get findFileUrl; + external Object? Function(String, CanonicalizeContext)? get findFileUrl; + external Object? get nonCanonicalScheme; } @JS() @anonymous -class CanonicalizeOptions { +class CanonicalizeContext { external bool get fromImport; + external JSUrl? get containingUrl; - external factory CanonicalizeOptions({bool fromImport}); + external factory CanonicalizeContext({bool fromImport, JSUrl? containingUrl}); } @JS() @anonymous -class NodeImporterResult { +class JSImporterResult { external String? get contents; external String? get syntax; external JSUrl? get sourceMapUrl; diff --git a/lib/src/util/span.dart b/lib/src/util/span.dart index 46866f60d..bcb9b6165 100644 --- a/lib/src/util/span.dart +++ b/lib/src/util/span.dart @@ -9,8 +9,10 @@ import 'package:string_scanner/string_scanner.dart'; import '../utils.dart'; import 'character.dart'; -/// A span that points nowhere, only used for fake AST nodes that will never be -/// presented to the user. +/// A span that points nowhere. +/// +/// This is used for fake AST nodes that will never be presented to the user, as +/// well as for embedded compilation failures that have no associated spans. final bogusSpan = SourceFile.decoded([]).span(0); extension SpanExtensions on FileSpan { diff --git a/test/dart_api/importer_test.dart b/test/dart_api/importer_test.dart index 113e9a24b..28ec53bee 100644 --- a/test/dart_api/importer_test.dart +++ b/test/dart_api/importer_test.dart @@ -112,6 +112,99 @@ void main() { }); }); + group("the containing URL", () { + test("is null for a potentially canonical scheme", () { + late TestImporter importer; + compileString('@import "u:orange";', + importers: [ + importer = TestImporter(expectAsync1((url) { + expect(importer.publicContainingUrl, isNull); + return url; + }), (_) => ImporterResult('', indented: false)) + ], + url: 'x:original.scss'); + }); + + test("throws an error outside canonicalize", () { + late TestImporter importer; + compileString('@import "orange";', importers: [ + importer = + TestImporter((url) => Uri.parse("u:$url"), expectAsync1((url) { + expect(() => importer.publicContainingUrl, throwsStateError); + return ImporterResult('', indented: false); + })) + ]); + }); + + group("for a non-canonical scheme", () { + test("is set to the original URL", () { + late TestImporter importer; + compileString('@import "u:orange";', + importers: [ + importer = TestImporter(expectAsync1((url) { + expect(importer.publicContainingUrl, + equals(Uri.parse('x:original.scss'))); + return url.replace(scheme: 'x'); + }), (_) => ImporterResult('', indented: false), + nonCanonicalSchemes: {'u'}) + ], + url: 'x:original.scss'); + }); + + test("is null if the original URL is null", () { + late TestImporter importer; + compileString('@import "u:orange";', importers: [ + importer = TestImporter(expectAsync1((url) { + expect(importer.publicContainingUrl, isNull); + return url.replace(scheme: 'x'); + }), (_) => ImporterResult('', indented: false), + nonCanonicalSchemes: {'u'}) + ]); + }); + }); + + group("for a schemeless load", () { + test("is set to the original URL", () { + late TestImporter importer; + compileString('@import "orange";', + importers: [ + importer = TestImporter(expectAsync1((url) { + expect(importer.publicContainingUrl, + equals(Uri.parse('x:original.scss'))); + return Uri.parse("u:$url"); + }), (_) => ImporterResult('', indented: false)) + ], + url: 'x:original.scss'); + }); + + test("is null if the original URL is null", () { + late TestImporter importer; + compileString('@import "orange";', importers: [ + importer = TestImporter(expectAsync1((url) { + expect(importer.publicContainingUrl, isNull); + return Uri.parse("u:$url"); + }), (_) => ImporterResult('', indented: false)) + ]); + }); + }); + }); + + test( + "throws an error if the importer returns a canonical URL with a " + "non-canonical scheme", () { + expect( + () => compileString('@import "orange";', importers: [ + TestImporter(expectAsync1((url) => Uri.parse("u:$url")), + (_) => ImporterResult('', indented: false), + nonCanonicalSchemes: {'u'}) + ]), throwsA(predicate((error) { + expect(error, const TypeMatcher()); + expect(error.toString(), + contains("uses a scheme declared as non-canonical")); + return true; + }))); + }); + test("uses an importer's source map URL", () { var result = compileStringToResult('@import "orange";', importers: [ diff --git a/test/dart_api/test_importer.dart b/test/dart_api/test_importer.dart index 87af315df..fda884c93 100644 --- a/test/dart_api/test_importer.dart +++ b/test/dart_api/test_importer.dart @@ -9,10 +9,22 @@ import 'package:sass/sass.dart'; class TestImporter extends Importer { final Uri? Function(Uri url) _canonicalize; final ImporterResult? Function(Uri url) _load; + final Set _nonCanonicalSchemes; - TestImporter(this._canonicalize, this._load); + /// Public access to the containing URL so that [canonicalize] and [load] + /// implementations can access them. + Uri? get publicContainingUrl => containingUrl; + + TestImporter(this._canonicalize, this._load, + {Iterable? nonCanonicalSchemes}) + : _nonCanonicalSchemes = nonCanonicalSchemes == null + ? const {} + : Set.unmodifiable(nonCanonicalSchemes); Uri? canonicalize(Uri url) => _canonicalize(url); ImporterResult? load(Uri url) => _load(url); + + bool isNonCanonicalScheme(String scheme) => + _nonCanonicalSchemes.contains(scheme); } diff --git a/test/embedded/importer_test.dart b/test/embedded/importer_test.dart index 71574cb4e..fdfc904d2 100644 --- a/test/embedded/importer_test.dart +++ b/test/embedded/importer_test.dart @@ -64,6 +64,46 @@ void main() { process, errorId, "Missing mandatory field Importer.importer"); await process.shouldExit(76); }); + + group("for an importer with nonCanonicalScheme set:", () { + test("path", () async { + process.send(compileString("a {b: c}", importers: [ + InboundMessage_CompileRequest_Importer( + path: "somewhere", nonCanonicalScheme: ["u"]) + ])); + await expectParamsError( + process, + errorId, + "Importer.non_canonical_scheme may only be set along with " + "Importer.importer.importer_id"); + await process.shouldExit(76); + }); + + test("file importer", () async { + process.send(compileString("a {b: c}", importers: [ + InboundMessage_CompileRequest_Importer( + fileImporterId: 1, nonCanonicalScheme: ["u"]) + ])); + await expectParamsError( + process, + errorId, + "Importer.non_canonical_scheme may only be set along with " + "Importer.importer.importer_id"); + await process.shouldExit(76); + }); + + test("unset", () async { + process.send(compileString("a {b: c}", + importer: InboundMessage_CompileRequest_Importer( + nonCanonicalScheme: ["u"]))); + await expectParamsError( + process, + errorId, + "Importer.non_canonical_scheme may only be set along with " + "Importer.importer.importer_id"); + await process.shouldExit(76); + }); + }); }); group("canonicalization", () { @@ -156,6 +196,86 @@ void main() { await process.close(); }); + group("the containing URL", () { + test("is unset for a potentially canonical scheme", () async { + process.send(compileString('@import "u:orange"', importers: [ + InboundMessage_CompileRequest_Importer(importerId: 1) + ])); + + var request = await getCanonicalizeRequest(process); + expect(request.hasContainingUrl(), isFalse); + await process.close(); + }); + + group("for a non-canonical scheme", () { + test("is set to the original URL", () async { + process.send(compileString('@import "u:orange"', + importers: [ + InboundMessage_CompileRequest_Importer( + importerId: 1, nonCanonicalScheme: ["u"]) + ], + url: "x:original.scss")); + + var request = await getCanonicalizeRequest(process); + expect(request.containingUrl, equals("x:original.scss")); + await process.close(); + }); + + test("is unset to the original URL is unset", () async { + process.send(compileString('@import "u:orange"', importers: [ + InboundMessage_CompileRequest_Importer( + importerId: 1, nonCanonicalScheme: ["u"]) + ])); + + var request = await getCanonicalizeRequest(process); + expect(request.hasContainingUrl(), isFalse); + await process.close(); + }); + }); + + group("for a schemeless load", () { + test("is set to the original URL", () async { + process.send(compileString('@import "orange"', + importers: [ + InboundMessage_CompileRequest_Importer(importerId: 1) + ], + url: "x:original.scss")); + + var request = await getCanonicalizeRequest(process); + expect(request.containingUrl, equals("x:original.scss")); + await process.close(); + }); + + test("is unset to the original URL is unset", () async { + process.send(compileString('@import "u:orange"', importers: [ + InboundMessage_CompileRequest_Importer(importerId: 1) + ])); + + var request = await getCanonicalizeRequest(process); + expect(request.hasContainingUrl(), isFalse); + await process.close(); + }); + }); + }); + + test( + "fails if the importer returns a canonical URL with a non-canonical " + "scheme", () async { + process.send(compileString("@import 'other'", importers: [ + InboundMessage_CompileRequest_Importer( + importerId: 1, nonCanonicalScheme: ["u"]) + ])); + + var request = await getCanonicalizeRequest(process); + process.send(InboundMessage( + canonicalizeResponse: InboundMessage_CanonicalizeResponse( + id: request.id, url: "u:other"))); + + await _expectImportError( + process, contains('a scheme declared as non-canonical')); + await process.close(); + }); + test("attempts importers in order", () async { process.send(compileString("@import 'other'", importers: [ for (var i = 0; i < 10; i++) @@ -474,6 +594,44 @@ void main() { await process.close(); }); }); + + group("fails compilation for an invalid scheme:", () { + test("empty", () async { + process.send(compileString("a {b: c}", importers: [ + InboundMessage_CompileRequest_Importer( + importerId: 1, nonCanonicalScheme: [""]) + ])); + + var failure = await getCompileFailure(process); + expect(failure.message, + equals('"" isn\'t a valid URL scheme (for example "file").')); + await process.close(); + }); + + test("uppercase", () async { + process.send(compileString("a {b: c}", importers: [ + InboundMessage_CompileRequest_Importer( + importerId: 1, nonCanonicalScheme: ["U"]) + ])); + + var failure = await getCompileFailure(process); + expect(failure.message, + equals('"U" isn\'t a valid URL scheme (for example "file").')); + await process.close(); + }); + + test("colon", () async { + process.send(compileString("a {b: c}", importers: [ + InboundMessage_CompileRequest_Importer( + importerId: 1, nonCanonicalScheme: ["u:"]) + ])); + + var failure = await getCompileFailure(process); + expect(failure.message, + equals('"u:" isn\'t a valid URL scheme (for example "file").')); + await process.close(); + }); + }); } /// Handles a `CanonicalizeRequest` and returns a response with a generic diff --git a/tool/grind/utils.dart b/tool/grind/utils.dart index 12fd8ed1e..21d9f66c8 100644 --- a/tool/grind/utils.dart +++ b/tool/grind/utils.dart @@ -58,6 +58,10 @@ String cloneOrCheckout(String url, String ref, {String? name}) { } var path = p.join("build", name); + if (Link(path).existsSync()) { + log("$url is symlinked, leaving as-is"); + return path; + } if (!Directory(p.join(path, '.git')).existsSync()) { delete(Directory(path)); From 873e91e5fffefec7b6d9d6d3d79472c4cca3154e Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Wed, 20 Sep 2023 17:18:42 -0700 Subject: [PATCH 12/30] Cut a release (#2090) --- .github/workflows/ci.yml | 5 ++++- CHANGELOG.md | 31 ++++++++++++++++++++++++++++++- pkg/sass_api/CHANGELOG.md | 4 ++++ pkg/sass_api/pubspec.yaml | 4 ++-- pubspec.yaml | 2 +- 5 files changed, 41 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ad796a58c..16c4ec7ec 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -234,7 +234,10 @@ jobs: matrix: os: [ubuntu-latest, windows-latest, macos-latest] dart_channel: [stable] - include: [{os: ubuntu-latest, dart_channel: dev}] + # TODO(nweiz): Re-enable this when + # https://github.com/dart-lang/sdk/issues/52121#issuecomment-1728534228 + # is addressed. + # include: [{os: ubuntu-latest, dart_channel: dev}] steps: - uses: actions/checkout@v3 diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e5dace4d..e56a37ea6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,9 +1,38 @@ -## 1.67.1 +## 1.68.0 * Fix the source spans associated with the `abs-percent` deprecation. +### JS API + +* Non-filesystem importers can now set the `nonCanonicalScheme` field, which + declares that one or more URL schemes (without `:`) will never be used for + URLs returned by the `canonicalize()` method. + +* Add a `containingUrl` field to the `canonicalize()` and `findFileUrl()` + methods of importers, which is set to the canonical URL of the stylesheet that + contains the current load. For filesystem importers, this is always set; for + other importers, it's set only if the current load has no URL scheme, or if + its URL scheme is declared as non-canonical by the importer. + +### Dart API + +* Add `AsyncImporter.isNonCanonicalScheme`, which importers (async or sync) can + use to indicate that a certain URL scheme will never be used for URLs returned + by the `canonicalize()` method. + +* Add `AsyncImporter.containingUrl`, which is set during calls to the + `canonicalize()` method to the canonical URL of the stylesheet that contains + the current load. This is set only if the current load has no URL scheme, or + if its URL scheme is declared as non-canonical by the importer. + ### Embedded Sass +* The `CalculationValue.interpolation` field is deprecated and will be removed + in a future version. It will no longer be set by the compiler, and if the host + sets it it will be treated as equivalent to `CalculationValue.string` except + that `"("` and `")"` will be added to the beginning and end of the string + values. + * Properly include TypeScript types in the `sass-embedded` package. ## 1.67.0 diff --git a/pkg/sass_api/CHANGELOG.md b/pkg/sass_api/CHANGELOG.md index d42f28444..9b9e2c55c 100644 --- a/pkg/sass_api/CHANGELOG.md +++ b/pkg/sass_api/CHANGELOG.md @@ -1,3 +1,7 @@ +## 9.1.0 + +* No user-visible changes. + ## 9.0.0 * Remove the `CalculationExpression` class and the associated visitor methods. diff --git a/pkg/sass_api/pubspec.yaml b/pkg/sass_api/pubspec.yaml index 0528454f0..3fb53a144 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: 9.0.0 +version: 9.1.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.67.0 + sass: 1.68.0 dev_dependencies: dartdoc: ^6.0.0 diff --git a/pubspec.yaml b/pubspec.yaml index 3036d6b5e..021a583c8 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass -version: 1.67.1-dev +version: 1.68.0 description: A Sass implementation in Dart. homepage: https://github.com/sass/dart-sass From de618fa7bd74cc648f3ee62358eed3e237d4644f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Wed, 20 Sep 2023 17:44:50 -0700 Subject: [PATCH 13/30] Fix an error during embedded compiler shutdown (#2092) --- lib/src/embedded/dispatcher.dart | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/src/embedded/dispatcher.dart b/lib/src/embedded/dispatcher.dart index d99098735..e9e723bfc 100644 --- a/lib/src/embedded/dispatcher.dart +++ b/lib/src/embedded/dispatcher.dart @@ -266,6 +266,12 @@ final class Dispatcher { _send(message); var packet = _mailbox.take(); + if (packet.isEmpty) { + // Compiler is shutting down, throw without calling `_handleError` as we + // don't want to report this as an actual error. + _requestError = true; + throw StateError('Compiler is shutting down.'); + } try { var messageBuffer = From 81c0be67e716eac2e9f51253ba2ecac2d4268272 Mon Sep 17 00:00:00 2001 From: Christophe Coevoet Date: Tue, 26 Sep 2023 22:35:35 +0200 Subject: [PATCH 14/30] Avoid useless allocations for interpolations without maps (#2095) --- lib/src/visitor/async_evaluate.dart | 2 +- lib/src/visitor/evaluate.dart | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/src/visitor/async_evaluate.dart b/lib/src/visitor/async_evaluate.dart index 6e16a71bf..13258ed04 100644 --- a/lib/src/visitor/async_evaluate.dart +++ b/lib/src/visitor/async_evaluate.dart @@ -3403,7 +3403,7 @@ final class _EvaluateVisitor Future _performInterpolation(Interpolation interpolation, {bool warnForColor = false}) async { var (result, _) = await _performInterpolationHelper(interpolation, - sourceMap: true, warnForColor: warnForColor); + sourceMap: false, warnForColor: warnForColor); return result; } diff --git a/lib/src/visitor/evaluate.dart b/lib/src/visitor/evaluate.dart index 87f3ef6c0..de45a4c56 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: 7669de19668af665d1a9a60cf67e53e071bf415e +// Checksum: 1c3027293ac9cb8a0d03b18c9ca447d62c2733d7 // // ignore_for_file: unused_import @@ -3372,7 +3372,7 @@ final class _EvaluateVisitor String _performInterpolation(Interpolation interpolation, {bool warnForColor = false}) { var (result, _) = _performInterpolationHelper(interpolation, - sourceMap: true, warnForColor: warnForColor); + sourceMap: false, warnForColor: warnForColor); return result; } From 0b6a1037edce3ba7f7eaf2bb2e6a2dab0d95e905 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Tue, 26 Sep 2023 14:07:12 -0700 Subject: [PATCH 15/30] Deprecate Deprecation.calcInterp (#2096) --- CHANGELOG.md | 7 +++++++ lib/src/deprecation.dart | 6 ++---- pubspec.yaml | 2 +- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e56a37ea6..84aba9723 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +## 1.68.1 + +### Dart API + +* Deprecate `Deprecation.calcInterp` since it was never actually emitted as a + deprecation. + ## 1.68.0 * Fix the source spans associated with the `abs-percent` deprecation. diff --git a/lib/src/deprecation.dart b/lib/src/deprecation.dart index 007fbb152..687e0ac43 100644 --- a/lib/src/deprecation.dart +++ b/lib/src/deprecation.dart @@ -69,10 +69,8 @@ enum Deprecation { deprecatedIn: '1.62.3', description: 'Passing null as alpha in the ${isJS ? 'JS' : 'Dart'} API.'), - calcInterp('calc-interp', - deprecatedIn: '1.67.0', - description: 'Using interpolation in a calculation outside a value ' - 'position.'), + @Deprecated('This deprecation name was never actually used.') + calcInterp('calc-interp', deprecatedIn: null), /// Deprecation for `@import` rules. import.future('import', description: '@import rules.'), diff --git a/pubspec.yaml b/pubspec.yaml index 021a583c8..420ca9d5f 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass -version: 1.68.0 +version: 1.68.1-dev description: A Sass implementation in Dart. homepage: https://github.com/sass/dart-sass From 9177f14d3a37fd07c1dd8ea15acaa957529f0e42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Wed, 27 Sep 2023 13:58:27 -0700 Subject: [PATCH 16/30] Update protocol-version during embedded-host-node release (#2097) --- .github/workflows/ci.yml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 16c4ec7ec..f5eb08c66 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -617,7 +617,9 @@ jobs: - name: Get version id: version - run: echo "version=${GITHUB_REF##*/}" | tee --append "$GITHUB_OUTPUT" + run: | + echo "version=${GITHUB_REF##*/}" | tee --append "$GITHUB_OUTPUT" + echo "protocol_version=$(curl -fsSL -H "Authorization: Bearer ${{ github.token }}" https://raw.githubusercontent.com/sass/sass/HEAD/spec/EMBEDDED_PROTOCOL_VERSION)" | tee --append "$GITHUB_OUTPUT" - name: Update version run: | @@ -632,13 +634,14 @@ jobs: # Update main package version and dependencies on binary packages cat package.json | - jq --arg version ${{ steps.version.outputs.version }} ' + jq --arg version ${{ steps.version.outputs.version }} --arg protocol_version ${{ steps.version.outputs.protocol_version }} ' .version |= $version | ."compiler-version" |= $version | + ."protocol-version" |= $protocol_version | .optionalDependencies = (.optionalDependencies | .[] |= $version) ' > package.json.tmp && mv package.json.tmp package.json - curl https://raw.githubusercontent.com/sass/dart-sass/${{ steps.version.outputs.version }}/CHANGELOG.md > CHANGELOG.md + curl -fsSL -H "Authorization: Bearer ${{ github.token }}" https://raw.githubusercontent.com/sass/dart-sass/${{ steps.version.outputs.version }}/CHANGELOG.md > CHANGELOG.md shell: bash - uses: EndBug/add-and-commit@v9 From 23f01430d6e7f78d39caa3e83b133531b22a5b24 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Wed, 27 Sep 2023 15:14:53 -0700 Subject: [PATCH 17/30] Forbid LLM contributions (#2100) --- CONTRIBUTING.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 934a0576a..8027cad27 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -2,6 +2,7 @@ Want to contribute? Great! First, read this page. * [Before You Contribute](#before-you-contribute) * [The Small Print](#the-small-print) +* [Large Language Models](#large-language-models) * [Development Dependencies](#development-dependencies) * [Writing Code](#writing-code) * [Changing the Language](#changing-the-language) @@ -37,6 +38,17 @@ one above, the [corporate cla]: https://developers.google.com/open-source/cla/corporate +## Large Language Models + +Do not submit any code or prose written or modified by large language models or +"artificial intelligence" such as GitHub Copilot or ChatGPT to this project. +These tools produce code that looks plausible, which means that not only is it +likely to contain bugs those bugs are likely to be difficult to notice on +review. In addition, because these models were trained indiscriminately and +non-consensually on open-source code with a variety of licenses, it's not +obvious that we have the moral or legal right to redistribute code they +generate. + ## Development Dependencies 1. [Install the Dart SDK][]. If you download an archive manually rather than From 00823e056a5c6de183971303c2ba81d7ed766190 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Thu, 28 Sep 2023 12:47:07 -0700 Subject: [PATCH 18/30] Rephrase errors for numbers that must be unitless or % (#2101) --- lib/src/functions/color.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/functions/color.dart b/lib/src/functions/color.dart index 3e6ce871b..9fd40720a 100644 --- a/lib/src/functions/color.dart +++ b/lib/src/functions/color.dart @@ -788,7 +788,7 @@ double _percentageOrUnitless(SassNumber number, num max, String name) { value = max * number.value / 100; } else { throw SassScriptException( - '\$$name: Expected $number to have no units or "%".'); + '\$$name: Expected $number to have unit "%" or no units.'); } return value.clamp(0, max).toDouble(); From ff56fc5e6112846a6279370199308d0a87b0f7da Mon Sep 17 00:00:00 2001 From: Christophe Coevoet Date: Fri, 29 Sep 2023 00:47:17 +0200 Subject: [PATCH 19/30] Implement support for the relative color syntax of CSS Color 5 (#2098) Co-authored-by: Natalie Weizenbaum --- CHANGELOG.md | 6 +++++- lib/src/functions/color.dart | 5 +++++ pubspec.yaml | 2 +- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 84aba9723..c8885e206 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,8 @@ -## 1.68.1 +## 1.69.0 + +* Add support for the relative color syntax from CSS Color 5. This syntax + cannot be used to create Sass color values. It is always emitted as-is in the + CSS output. ### Dart API diff --git a/lib/src/functions/color.dart b/lib/src/functions/color.dart index 9fd40720a..f71c8080f 100644 --- a/lib/src/functions/color.dart +++ b/lib/src/functions/color.dart @@ -736,6 +736,11 @@ Object /* SassString | List */ _parseChannels( } var list = channels.asList; + if (list case [SassString(:var text, hasQuotes: false), _, ...] + when equalsIgnoreCase(text, "from")) { + return _functionString(name, [originalChannels]); + } + if (list.length > 3) { throw SassScriptException("Only 3 elements allowed, but ${list.length} " "were passed."); diff --git a/pubspec.yaml b/pubspec.yaml index 420ca9d5f..b518e7566 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass -version: 1.68.1-dev +version: 1.69.0-dev description: A Sass implementation in Dart. homepage: https://github.com/sass/dart-sass From f66cb47d1488b042e1a8647d5750e4b8bb669be8 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 29 Sep 2023 20:26:39 +0000 Subject: [PATCH 20/30] Bump docker/setup-qemu-action from 2 to 3 (#2089) Bumps [docker/setup-qemu-action](https://github.com/docker/setup-qemu-action) from 2 to 3. - [Release notes](https://github.com/docker/setup-qemu-action/releases) - [Commits](https://github.com/docker/setup-qemu-action/compare/v2...v3) --- updated-dependencies: - dependency-name: docker/setup-qemu-action 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> --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f5eb08c66..c3c5456a9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -441,7 +441,7 @@ jobs: - uses: ./.github/util/initialize with: {github-token: "${{ github.token }}"} - - uses: docker/setup-qemu-action@v2 + - uses: docker/setup-qemu-action@v3 - name: Deploy run: | docker run --rm \ From 507e4399cc79970770e32e818be68d4ac940f9db Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 29 Sep 2023 20:26:47 +0000 Subject: [PATCH 21/30] Bump actions/checkout from 3 to 4 (#2088) Bumps [actions/checkout](https://github.com/actions/checkout) from 3 to 4. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](https://github.com/actions/checkout/compare/v3...v4) --- updated-dependencies: - dependency-name: actions/checkout 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> --- .github/workflows/ci.yml | 52 ++++++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c3c5456a9..5a1979ced 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,7 +21,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: dart-lang/setup-dart@v1 - run: dart format --fix . - run: git diff --exit-code @@ -31,7 +31,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: ./.github/util/initialize with: {github-token: "${{ github.token }}"} @@ -43,7 +43,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: ./.github/util/initialize with: {github-token: "${{ github.token }}"} @@ -72,7 +72,7 @@ jobs: async_args: '--cmd-args --async' steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: ./.github/util/initialize with: dart-sdk: ${{ matrix.dart_channel }} @@ -110,7 +110,7 @@ jobs: node-version: 18 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: ./.github/util/initialize with: dart-sdk: ${{ matrix.dart_channel }} @@ -152,7 +152,7 @@ jobs: node-version: 14 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: ./.github/util/initialize with: github-token: ${{ github.token }} @@ -198,7 +198,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: browser-actions/setup-chrome@v1 - uses: ./.github/util/initialize with: @@ -240,7 +240,7 @@ jobs: # include: [{os: ubuntu-latest, dart_channel: dev}] steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: ./.github/util/initialize with: dart-sdk: ${{ matrix.dart_channel }} @@ -278,7 +278,7 @@ jobs: dart_channel: dev node-version: 18 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: ./.github/util/initialize with: dart-sdk: ${{ matrix.dart_channel }} @@ -299,7 +299,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: browser-actions/setup-chrome@v1 - uses: ./.github/util/initialize with: @@ -329,7 +329,7 @@ jobs: if: "startsWith(github.ref, 'refs/tags/') && github.repository == 'sass/dart-sass'" steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: ./.github/util/initialize with: {github-token: "${{ github.token }}"} @@ -347,7 +347,7 @@ jobs: bootstrap_version: [4, 5] steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: ./.github/util/initialize with: {github-token: "${{ github.token }}"} @@ -362,7 +362,7 @@ jobs: needs: [double_check] steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: ./.github/util/initialize with: {github-token: "${{ github.token }}"} @@ -379,7 +379,7 @@ jobs: needs: [double_check] steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: ./.github/util/initialize with: {github-token: "${{ github.token }}"} @@ -397,7 +397,7 @@ jobs: needs: [double_check] steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: ./.github/util/initialize with: {github-token: "${{ github.token }}"} @@ -413,7 +413,7 @@ jobs: if: "startsWith(github.ref, 'refs/tags/') && github.repository == 'sass/dart-sass'" steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: ./.github/util/initialize with: {github-token: "${{ github.token }}"} @@ -437,7 +437,7 @@ jobs: if: "startsWith(github.ref, 'refs/tags/') && github.repository == 'sass/dart-sass'" steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: ./.github/util/initialize with: {github-token: "${{ github.token }}"} @@ -475,7 +475,7 @@ jobs: architecture: x64 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: ./.github/util/initialize # Workaround for dart-lang/setup-dart#59 with: @@ -495,7 +495,7 @@ jobs: if: "startsWith(github.ref, 'refs/tags/') && github.repository == 'sass/dart-sass'" steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: ./.github/util/initialize with: {github-token: "${{ github.token }}"} @@ -511,7 +511,7 @@ jobs: if: "startsWith(github.ref, 'refs/tags/') && github.repository == 'sass/dart-sass'" steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: ./.github/util/initialize with: {github-token: "${{ github.token }}"} @@ -528,7 +528,7 @@ jobs: if: "startsWith(github.ref, 'refs/tags/') && github.repository == 'sass/dart-sass'" steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: ./.github/util/initialize with: {github-token: "${{ github.token }}"} @@ -543,7 +543,7 @@ jobs: if: "startsWith(github.ref, 'refs/tags/') && github.repository == 'sass/dart-sass'" steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: ./.github/util/initialize with: {github-token: "${{ github.token }}"} @@ -561,7 +561,7 @@ jobs: if: "startsWith(github.ref, 'refs/tags/') && github.repository == 'sass/dart-sass'" steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: dart-lang/setup-dart@v1 - run: dart pub get @@ -578,7 +578,7 @@ jobs: if: "startsWith(github.ref, 'refs/tags/') && github.repository == 'sass/dart-sass'" steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: ./.github/util/initialize with: {github-token: "${{ github.token }}"} @@ -592,7 +592,7 @@ jobs: needs: [bootstrap, bourbon, foundation, bulma] if: "startsWith(github.ref, 'refs/tags/') && github.repository == 'sass/dart-sass'" steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: repository: sass/sass-site token: ${{ secrets.SASS_SITE_TOKEN }} @@ -610,7 +610,7 @@ jobs: needs: [deploy_github_linux, deploy_github_linux_qemu, deploy_github] if: "startsWith(github.ref, 'refs/tags/') && github.repository == 'sass/dart-sass'" steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: repository: sass/embedded-host-node token: ${{ secrets.GH_TOKEN }} From 4255930f52ed4d9aaf483100f8791904fbf08635 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Fri, 29 Sep 2023 17:12:41 -0700 Subject: [PATCH 22/30] Update the version of Sass used by the website on release (#2102) --- .github/workflows/ci.yml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5a1979ced..cc616b22e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -589,7 +589,7 @@ jobs: deploy_website: name: "Deploy sass-lang.com" runs-on: ubuntu-latest - needs: [bootstrap, bourbon, foundation, bulma] + needs: [deploy_npm] if: "startsWith(github.ref, 'refs/tags/') && github.repository == 'sass/dart-sass'" steps: - uses: actions/checkout@v4 @@ -597,6 +597,13 @@ jobs: repository: sass/sass-site token: ${{ secrets.SASS_SITE_TOKEN }} + - name: Get version + id: version + run: echo "version=${GITHUB_REF##*/}" | tee --append "$GITHUB_OUTPUT" + + - name: Update Dart Sass version + run: npm install sass@${{ steps.version.outputs.version }} + - uses: EndBug/add-and-commit@v9 with: author_name: Sass Bot From 16b85120f53bf3f26f77daa143beb0bcf3eabf90 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Tue, 3 Oct 2023 16:23:28 -0700 Subject: [PATCH 23/30] Switch to the GitHub-hosted MacOS ARM64 runner (#2103) --- .github/workflows/ci.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cc616b22e..0e55102c9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -467,7 +467,8 @@ jobs: - runner: macos-latest platform: macos-x64 architecture: x64 - - runner: self-hosted + # https://github.blog/2023-10-02-introducing-the-new-apple-silicon-powered-m1-macos-larger-runner-for-github-actions/ + - runner: macos-latest-xlarge platform: macos-arm64 architecture: arm64 - runner: windows-latest From 310904e217fe52b8663bc3649d1c936a2b1c03f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Wed, 4 Oct 2023 17:24:39 -0700 Subject: [PATCH 24/30] Fix a race condition preventing embedded compiler to shutdown after a protocol error (#2106) Co-authored-by: Natalie Weizenbaum --- CHANGELOG.md | 5 +++++ lib/src/embedded/dispatcher.dart | 14 ++++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c8885e206..abb205336 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,11 @@ * Deprecate `Deprecation.calcInterp` since it was never actually emitted as a deprecation. +### Embedded Sass + +* Fix a rare race condition where the embedded compiler could freeze when a + protocol error was immediately followed by another request. + ## 1.68.0 * Fix the source spans associated with the `abs-percent` deprecation. diff --git a/lib/src/embedded/dispatcher.dart b/lib/src/embedded/dispatcher.dart index e9e723bfc..a1b20b5d4 100644 --- a/lib/src/embedded/dispatcher.dart +++ b/lib/src/embedded/dispatcher.dart @@ -237,7 +237,17 @@ final class Dispatcher { _send(OutboundMessage()..logEvent = event); /// Sends [error] to the host. - void sendError(ProtocolError error) => + /// + /// This is used during compilation by other classes like host callable. + /// Therefore it must set _requestError = true to prevent sending a CompileFailure after + /// sending a ProtocolError. + void sendError(ProtocolError error) { + _sendError(error); + _requestError = true; + } + + /// Sends [error] to the host. + void _sendError(ProtocolError error) => _send(OutboundMessage()..error = error); InboundMessage_CanonicalizeResponse sendCanonicalizeRequest( @@ -323,7 +333,7 @@ final class Dispatcher { /// The [messageId] indicate the IDs of the message being responded to, if /// available. void _handleError(Object error, StackTrace stackTrace, {int? messageId}) { - sendError(handleError(error, stackTrace, messageId: messageId)); + _sendError(handleError(error, stackTrace, messageId: messageId)); } /// Sends [message] to the host with the given [wireId]. From ce545c2e95c7e0d7a445dd7a16af8b73e31814f5 Mon Sep 17 00:00:00 2001 From: Connor Skees <39542938+connorskees@users.noreply.github.com> Date: Thu, 5 Oct 2023 14:50:57 -0700 Subject: [PATCH 25/30] Implement first class mixins (#2073) Co-authored-by: Natalie Weizenbaum --- CHANGELOG.md | 11 +++ lib/src/callable/async_built_in.dart | 10 +- lib/src/callable/built_in.dart | 17 ++-- lib/src/embedded/dispatcher.dart | 9 +- lib/src/embedded/function_registry.dart | 33 ------- lib/src/embedded/host_callable.dart | 11 ++- lib/src/embedded/opaque_registry.dart | 30 ++++++ lib/src/embedded/protofier.dart | 25 ++++- lib/src/functions/meta.dart | 13 +++ lib/src/js.dart | 1 + lib/src/js/exports.dart | 1 + lib/src/js/value.dart | 2 + lib/src/js/value/mixin.dart | 22 +++++ lib/src/value.dart | 9 ++ lib/src/value/mixin.dart | 40 ++++++++ lib/src/visitor/async_evaluate.dart | 122 ++++++++++++++++++++---- lib/src/visitor/evaluate.dart | 119 +++++++++++++++++++---- lib/src/visitor/interface/value.dart | 1 + lib/src/visitor/serialize.dart | 10 ++ tool/grind.dart | 1 + 20 files changed, 396 insertions(+), 91 deletions(-) delete mode 100644 lib/src/embedded/function_registry.dart create mode 100644 lib/src/embedded/opaque_registry.dart create mode 100644 lib/src/js/value/mixin.dart create mode 100644 lib/src/value/mixin.dart diff --git a/CHANGELOG.md b/CHANGELOG.md index abb205336..e5f4fa3ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ ## 1.69.0 +* Add a `meta.get-mixin()` function that returns a mixin as a first-class Sass + value. + +* Add a `meta.apply()` mixin that includes a mixin value. + +* Add a `meta.module-mixins()` function which returns a map from mixin names in + a module to the first-class mixins that belong to those names. + +* Add a `meta.accepts-content()` function which returns whether or not a mixin + value can take a content block. + * Add support for the relative color syntax from CSS Color 5. This syntax cannot be used to create Sass color values. It is always emitted as-is in the CSS output. diff --git a/lib/src/callable/async_built_in.dart b/lib/src/callable/async_built_in.dart index 0132b787f..7dba7e1cd 100644 --- a/lib/src/callable/async_built_in.dart +++ b/lib/src/callable/async_built_in.dart @@ -26,6 +26,11 @@ class AsyncBuiltInCallable implements AsyncCallable { /// The callback to run when executing this callable. final Callback _callback; + /// Whether this callable could potentially accept an `@content` block. + /// + /// This can only be true for mixins. + final bool acceptsContent; + /// Creates a function with a single [arguments] declaration and a single /// [callback]. /// @@ -52,7 +57,7 @@ class AsyncBuiltInCallable implements AsyncCallable { /// defined. AsyncBuiltInCallable.mixin(String name, String arguments, FutureOr callback(List arguments), - {Object? url}) + {Object? url, bool acceptsContent = false}) : this.parsed(name, ArgumentDeclaration.parse('@mixin $name($arguments) {', url: url), (arguments) async { @@ -66,7 +71,8 @@ class AsyncBuiltInCallable implements AsyncCallable { /// Creates a callable with a single [arguments] declaration and a single /// [callback]. - AsyncBuiltInCallable.parsed(this.name, this._arguments, this._callback); + AsyncBuiltInCallable.parsed(this.name, this._arguments, this._callback, + {this.acceptsContent = false}); /// Returns the argument declaration and Dart callback for the given /// positional and named arguments. diff --git a/lib/src/callable/built_in.dart b/lib/src/callable/built_in.dart index 905d11e56..a52662fa0 100644 --- a/lib/src/callable/built_in.dart +++ b/lib/src/callable/built_in.dart @@ -21,6 +21,8 @@ final class BuiltInCallable implements Callable, AsyncBuiltInCallable { /// The overloads declared for this callable. final List<(ArgumentDeclaration, Callback)> _overloads; + final bool acceptsContent; + /// Creates a function with a single [arguments] declaration and a single /// [callback]. /// @@ -48,18 +50,19 @@ final class BuiltInCallable implements Callable, AsyncBuiltInCallable { /// defined. BuiltInCallable.mixin( String name, String arguments, void callback(List arguments), - {Object? url}) + {Object? url, bool acceptsContent = false}) : this.parsed(name, ArgumentDeclaration.parse('@mixin $name($arguments) {', url: url), (arguments) { callback(arguments); return sassNull; - }); + }, acceptsContent: acceptsContent); /// Creates a callable with a single [arguments] declaration and a single /// [callback]. BuiltInCallable.parsed(this.name, ArgumentDeclaration arguments, - Value callback(List arguments)) + Value callback(List arguments), + {this.acceptsContent = false}) : _overloads = [(arguments, callback)]; /// Creates a function with multiple implementations. @@ -79,9 +82,10 @@ final class BuiltInCallable implements Callable, AsyncBuiltInCallable { ArgumentDeclaration.parse('@function $name($args) {', url: url), callback ) - ]; + ], + acceptsContent = false; - BuiltInCallable._(this.name, this._overloads); + BuiltInCallable._(this.name, this._overloads, this.acceptsContent); /// Returns the argument declaration and Dart callback for the given /// positional and named arguments. @@ -117,5 +121,6 @@ final class BuiltInCallable implements Callable, AsyncBuiltInCallable { } /// Returns a copy of this callable with the given [name]. - BuiltInCallable withName(String name) => BuiltInCallable._(name, _overloads); + BuiltInCallable withName(String name) => + BuiltInCallable._(name, _overloads, acceptsContent); } diff --git a/lib/src/embedded/dispatcher.dart b/lib/src/embedded/dispatcher.dart index a1b20b5d4..40e81863d 100644 --- a/lib/src/embedded/dispatcher.dart +++ b/lib/src/embedded/dispatcher.dart @@ -12,8 +12,10 @@ import 'package:path/path.dart' as p; import 'package:protobuf/protobuf.dart'; import 'package:sass/sass.dart' as sass; +import '../value/function.dart'; +import '../value/mixin.dart'; import 'embedded_sass.pb.dart'; -import 'function_registry.dart'; +import 'opaque_registry.dart'; import 'host_callable.dart'; import 'importer/file.dart'; import 'importer/host.dart'; @@ -109,7 +111,8 @@ final class Dispatcher { OutboundMessage_CompileResponse _compile( InboundMessage_CompileRequest request) { - var functions = FunctionRegistry(); + var functions = OpaqueRegistry(); + var mixins = OpaqueRegistry(); var style = request.style == OutputStyle.COMPRESSED ? sass.OutputStyle.compressed @@ -123,7 +126,7 @@ final class Dispatcher { (throw mandatoryError("Importer.importer"))); var globalFunctions = request.globalFunctions - .map((signature) => hostCallable(this, functions, signature)); + .map((signature) => hostCallable(this, functions, mixins, signature)); late sass.CompileResult result; switch (request.whichInput()) { diff --git a/lib/src/embedded/function_registry.dart b/lib/src/embedded/function_registry.dart deleted file mode 100644 index b288fbd8d..000000000 --- a/lib/src/embedded/function_registry.dart +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright 2019 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 '../value/function.dart'; -import 'embedded_sass.pb.dart'; - -/// A registry of [SassFunction]s indexed by ID so that the host can invoke -/// them. -final class FunctionRegistry { - /// First-class functions that have been sent to the host. - /// - /// The functions are located at indexes in the list matching their IDs. - final _functionsById = []; - - /// A reverse map from functions to their indexes in [_functionsById]. - final _idsByFunction = {}; - - /// Converts [function] to a protocol buffer to send to the host. - Value_CompilerFunction protofy(SassFunction function) { - var id = _idsByFunction.putIfAbsent(function, () { - _functionsById.add(function); - return _functionsById.length - 1; - }); - - return Value_CompilerFunction()..id = id; - } - - /// Returns the compiler-side function associated with [id]. - /// - /// If no such function exists, returns `null`. - SassFunction? operator [](int id) => _functionsById[id]; -} diff --git a/lib/src/embedded/host_callable.dart b/lib/src/embedded/host_callable.dart index 448cce217..3518b57e2 100644 --- a/lib/src/embedded/host_callable.dart +++ b/lib/src/embedded/host_callable.dart @@ -4,9 +4,11 @@ import '../callable.dart'; import '../exception.dart'; +import '../value/function.dart'; +import '../value/mixin.dart'; import 'dispatcher.dart'; import 'embedded_sass.pb.dart'; -import 'function_registry.dart'; +import 'opaque_registry.dart'; import 'protofier.dart'; import 'utils.dart'; @@ -19,11 +21,14 @@ import 'utils.dart'; /// /// Throws a [SassException] if [signature] is invalid. Callable hostCallable( - Dispatcher dispatcher, FunctionRegistry functions, String signature, + Dispatcher dispatcher, + OpaqueRegistry functions, + OpaqueRegistry mixins, + String signature, {int? id}) { late Callable callable; callable = Callable.fromSignature(signature, (arguments) { - var protofier = Protofier(dispatcher, functions); + var protofier = Protofier(dispatcher, functions, mixins); var request = OutboundMessage_FunctionCallRequest() ..arguments.addAll( [for (var argument in arguments) protofier.protofy(argument)]); diff --git a/lib/src/embedded/opaque_registry.dart b/lib/src/embedded/opaque_registry.dart new file mode 100644 index 000000000..bf9adaab2 --- /dev/null +++ b/lib/src/embedded/opaque_registry.dart @@ -0,0 +1,30 @@ +// Copyright 2019 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 registry of some `T` indexed by ID so that the host can invoke +/// them. +final class OpaqueRegistry { + /// Instantiations of `T` that have been sent to the host. + /// + /// The values are located at indexes in the list matching their IDs. + final _elementsById = []; + + /// A reverse map from elements to their indexes in [_elementsById]. + final _idsByElement = {}; + + /// Returns the compiler-side id associated with [element]. + int getId(T element) { + var id = _idsByElement.putIfAbsent(element, () { + _elementsById.add(element); + return _elementsById.length - 1; + }); + + return id; + } + + /// Returns the compiler-side element associated with [id]. + /// + /// If no such element exists, returns `null`. + T? operator [](int id) => _elementsById[id]; +} diff --git a/lib/src/embedded/protofier.dart b/lib/src/embedded/protofier.dart index 6ad083ca4..eec7e3d05 100644 --- a/lib/src/embedded/protofier.dart +++ b/lib/src/embedded/protofier.dart @@ -8,8 +8,8 @@ import '../value.dart'; import 'dispatcher.dart'; import 'embedded_sass.pb.dart' as proto; import 'embedded_sass.pb.dart' hide Value, ListSeparator, CalculationOperator; -import 'function_registry.dart'; import 'host_callable.dart'; +import 'opaque_registry.dart'; import 'utils.dart'; /// A class that converts Sass [Value] objects into [Value] protobufs. @@ -21,7 +21,10 @@ final class Protofier { final Dispatcher _dispatcher; /// The IDs of first-class functions. - final FunctionRegistry _functions; + final OpaqueRegistry _functions; + + /// The IDs of first-class mixins. + final OpaqueRegistry _mixins; /// Any argument lists transitively contained in [value]. /// @@ -35,7 +38,10 @@ final class Protofier { /// /// The [functions] tracks the IDs of first-class functions so that the host /// can pass them back to the compiler. - Protofier(this._dispatcher, this._functions); + /// + /// Similarly, the [mixins] tracks the IDs of first-class mixins so that the + /// host can pass them back to the compiler. + Protofier(this._dispatcher, this._functions, this._mixins); /// Converts [value] to its protocol buffer representation. proto.Value protofy(Value value) { @@ -84,7 +90,10 @@ final class Protofier { case SassCalculation(): result.calculation = _protofyCalculation(value); case SassFunction(): - result.compilerFunction = _functions.protofy(value); + result.compilerFunction = + Value_CompilerFunction(id: _functions.getId(value)); + case SassMixin(): + result.compilerMixin = Value_CompilerMixin(id: _mixins.getId(value)); case sassTrue: result.singleton = SingletonValue.TRUE; case sassFalse: @@ -238,9 +247,15 @@ final class Protofier { case Value_Value.hostFunction: return SassFunction(hostCallable( - _dispatcher, _functions, value.hostFunction.signature, + _dispatcher, _functions, _mixins, value.hostFunction.signature, id: value.hostFunction.id)); + case Value_Value.compilerMixin: + var id = value.compilerMixin.id; + if (_mixins[id] case var mixin?) return mixin; + throw paramsError( + "CompilerMixin.id $id doesn't match any known mixins"); + case Value_Value.calculation: return _deprotofyCalculation(value.calculation); diff --git a/lib/src/functions/meta.dart b/lib/src/functions/meta.dart index 41537d55d..a3b683b9a 100644 --- a/lib/src/functions/meta.dart +++ b/lib/src/functions/meta.dart @@ -6,6 +6,7 @@ import 'dart:collection'; import 'package:collection/collection.dart'; +import '../ast/sass/statement/mixin_rule.dart'; import '../callable.dart'; import '../util/map.dart'; import '../value.dart'; @@ -45,6 +46,7 @@ final global = UnmodifiableListView([ sassNull => "null", SassNumber() => "number", SassFunction() => "function", + SassMixin() => "mixin", SassCalculation() => "calculation", SassString() => "string", _ => throw "[BUG] Unknown value type ${arguments[0]}" @@ -77,6 +79,17 @@ final local = UnmodifiableListView([ ? argument : SassString(argument.toString(), quotes: false)), ListSeparator.comma); + }), + _function("accepts-content", r"$mixin", (arguments) { + var mixin = arguments[0].assertMixin("mixin"); + return SassBoolean(switch (mixin.callable) { + AsyncBuiltInCallable(acceptsContent: var acceptsContent) || + BuiltInCallable(acceptsContent: var acceptsContent) => + acceptsContent, + UserDefinedCallable(declaration: MixinRule(hasContent: var hasContent)) => + hasContent, + _ => throw UnsupportedError("Unknown callable type $mixin.") + }); }) ]); diff --git a/lib/src/js.dart b/lib/src/js.dart index cd1480719..9a2c51c06 100644 --- a/lib/src/js.dart +++ b/lib/src/js.dart @@ -32,6 +32,7 @@ void main() { exports.CalculationInterpolation = calculationInterpolationClass; exports.SassColor = colorClass; exports.SassFunction = functionClass; + exports.SassMixin = mixinClass; exports.SassList = listClass; exports.SassMap = mapClass; exports.SassNumber = numberClass; diff --git a/lib/src/js/exports.dart b/lib/src/js/exports.dart index 0dff13698..ee5a74471 100644 --- a/lib/src/js/exports.dart +++ b/lib/src/js/exports.dart @@ -32,6 +32,7 @@ class Exports { external set SassBoolean(JSClass function); external set SassColor(JSClass function); external set SassFunction(JSClass function); + external set SassMixin(JSClass mixin); external set SassList(JSClass function); external set SassMap(JSClass function); external set SassNumber(JSClass function); diff --git a/lib/src/js/value.dart b/lib/src/js/value.dart index f8697efea..c57621c45 100644 --- a/lib/src/js/value.dart +++ b/lib/src/js/value.dart @@ -15,6 +15,7 @@ export 'value/color.dart'; export 'value/function.dart'; export 'value/list.dart'; export 'value/map.dart'; +export 'value/mixin.dart'; export 'value/number.dart'; export 'value/string.dart'; @@ -42,6 +43,7 @@ final JSClass valueClass = () { 'assertColor': (Value self, [String? name]) => self.assertColor(name), 'assertFunction': (Value self, [String? name]) => self.assertFunction(name), 'assertMap': (Value self, [String? name]) => self.assertMap(name), + 'assertMixin': (Value self, [String? name]) => self.assertMixin(name), 'assertNumber': (Value self, [String? name]) => self.assertNumber(name), 'assertString': (Value self, [String? name]) => self.assertString(name), 'tryMap': (Value self) => self.tryMap(), diff --git a/lib/src/js/value/mixin.dart b/lib/src/js/value/mixin.dart new file mode 100644 index 000000000..a41b394d2 --- /dev/null +++ b/lib/src/js/value/mixin.dart @@ -0,0 +1,22 @@ +// Copyright 2021 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:node_interop/js.dart'; + +import '../../callable.dart'; +import '../../value.dart'; +import '../reflection.dart'; +import '../utils.dart'; + +/// The JavaScript `SassMixin` class. +final JSClass mixinClass = () { + var jsClass = createJSClass('sass.SassMixin', (Object self) { + jsThrow(JsError( + 'It is not possible to construct a SassMixin through the JavaScript API')); + }); + + getJSClass(SassMixin(Callable('f', '', (_) => sassNull))) + .injectSuperclass(jsClass); + return jsClass; +}(); diff --git a/lib/src/value.dart b/lib/src/value.dart index 4b21e2434..3149435a0 100644 --- a/lib/src/value.dart +++ b/lib/src/value.dart @@ -15,6 +15,7 @@ import 'value/color.dart'; import 'value/function.dart'; import 'value/list.dart'; import 'value/map.dart'; +import 'value/mixin.dart'; import 'value/number.dart'; import 'value/string.dart'; import 'visitor/interface/value.dart'; @@ -27,6 +28,7 @@ export 'value/color.dart'; export 'value/function.dart'; export 'value/list.dart'; export 'value/map.dart'; +export 'value/mixin.dart'; export 'value/null.dart'; export 'value/number.dart' hide conversionFactor; export 'value/string.dart'; @@ -177,6 +179,13 @@ abstract class Value { SassFunction assertFunction([String? name]) => throw SassScriptException("$this is not a function reference.", name); + /// Throws a [SassScriptException] if [this] isn't a mixin reference. + /// + /// If this came from a function argument, [name] is the argument name + /// (without the `$`). It's used for error reporting. + SassMixin assertMixin([String? name]) => + throw SassScriptException("$this is not a mixin reference.", name); + /// Throws a [SassScriptException] if [this] isn't a map. /// /// If this came from a function argument, [name] is the argument name diff --git a/lib/src/value/mixin.dart b/lib/src/value/mixin.dart new file mode 100644 index 000000000..79091579d --- /dev/null +++ b/lib/src/value/mixin.dart @@ -0,0 +1,40 @@ +// Copyright 2023 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:meta/meta.dart'; + +import '../callable.dart'; +import '../visitor/interface/value.dart'; +import '../value.dart'; + +/// A SassScript mixin reference. +/// +/// A mixin reference captures a mixin from the local environment so that +/// it may be passed between modules. +/// +/// {@category Value} +final class SassMixin extends Value { + /// The callable that this mixin invokes. + /// + /// Note that this is typed as an [AsyncCallable] so that it will work with + /// both synchronous and asynchronous evaluate visitors, but in practice the + /// synchronous evaluate visitor will crash if this isn't a [Callable]. + /// + /// @nodoc + @internal + final AsyncCallable callable; + + SassMixin(this.callable); + + /// @nodoc + @internal + T accept(ValueVisitor visitor) => visitor.visitMixin(this); + + SassMixin assertMixin([String? name]) => this; + + bool operator ==(Object other) => + other is SassMixin && callable == other.callable; + + int get hashCode => callable.hashCode; +} diff --git a/lib/src/visitor/async_evaluate.dart b/lib/src/visitor/async_evaluate.dart index 13258ed04..389c6991a 100644 --- a/lib/src/visitor/async_evaluate.dart +++ b/lib/src/visitor/async_evaluate.dart @@ -418,6 +418,19 @@ final class _EvaluateVisitor }); }, url: "sass:meta"), + BuiltInCallable.function("module-mixins", r"$module", (arguments) { + var namespace = arguments[0].assertString("module"); + var module = _environment.modules[namespace.text]; + if (module == null) { + throw 'There is no module with namespace "${namespace.text}".'; + } + + return SassMap({ + for (var (name, value) in module.mixins.pairs) + SassString(name): SassMixin(value) + }); + }, url: "sass:meta"), + BuiltInCallable.function( "get-function", r"$name, $css: false, $module: null", (arguments) { var name = arguments[0].assertString("name"); @@ -444,6 +457,20 @@ final class _EvaluateVisitor return SassFunction(callable); }, url: "sass:meta"), + BuiltInCallable.function("get-mixin", r"$name, $module: null", + (arguments) { + var name = arguments[0].assertString("name"); + var module = arguments[1].realNull?.assertString("module"); + + var callable = _addExceptionSpan( + _callableNode!, + () => _environment.getMixin(name.text.replaceAll("_", "-"), + namespace: module?.text)); + if (callable == null) throw "Mixin not found: $name"; + + return SassMixin(callable); + }, url: "sass:meta"), + AsyncBuiltInCallable.function("call", r"$function, $args...", (arguments) async { var function = arguments[0]; @@ -517,7 +544,32 @@ final class _EvaluateVisitor configuration: configuration, namesInErrors: true); _assertConfigurationIsEmpty(configuration, nameInError: true); - }, url: "sass:meta") + }, url: "sass:meta"), + BuiltInCallable.mixin("apply", r"$mixin, $args...", (arguments) async { + var mixin = arguments[0]; + var args = arguments[1] as SassArgumentList; + + var callableNode = _callableNode!; + var invocation = ArgumentInvocation( + const [], + const {}, + callableNode.span, + rest: ValueExpression(args, callableNode.span), + ); + + var callable = mixin.assertMixin("mixin").callable; + var content = _environment.content; + + // ignore: unnecessary_type_check + if (callable is AsyncCallable) { + await _applyMixin( + callable, content, invocation, callableNode, callableNode); + } else { + throw SassScriptException( + "The mixin ${callable.name} is asynchronous.\n" + "This is probably caused by a bug in a Sass plugin."); + } + }, url: "sass:meta", acceptsContent: true), ]; var metaModule = BuiltInModule("meta", @@ -1733,41 +1785,57 @@ final class _EvaluateVisitor } } - Future visitIncludeRule(IncludeRule node) async { - var nodeWithSpan = AstNode.fake(() => node.spanWithoutContent); - var mixin = _addExceptionSpan(node, - () => _environment.getMixin(node.name, namespace: node.namespace)); + /// Evaluate a given [mixin] with [arguments] and [contentCallable] + Future _applyMixin( + AsyncCallable? mixin, + UserDefinedCallable? contentCallable, + ArgumentInvocation arguments, + AstNode nodeWithSpan, + AstNode nodeWithSpanWithoutContent) async { switch (mixin) { case null: - throw _exception("Undefined mixin.", node.span); - - case AsyncBuiltInCallable() when node.content != null: - throw _exception("Mixin doesn't accept a content block.", node.span); - + throw _exception("Undefined mixin.", nodeWithSpan.span); + + case AsyncBuiltInCallable(acceptsContent: false) + when contentCallable != null: + { + var evaluated = await _evaluateArguments(arguments); + var (overload, _) = mixin.callbackFor( + evaluated.positional.length, MapKeySet(evaluated.named)); + throw MultiSpanSassRuntimeException( + "Mixin doesn't accept a content block.", + nodeWithSpanWithoutContent.span, + "invocation", + {overload.spanWithName: "declaration"}, + _stackTrace(nodeWithSpanWithoutContent.span)); + } case AsyncBuiltInCallable(): - await _runBuiltInCallable(node.arguments, mixin, nodeWithSpan); + await _environment.withContent(contentCallable, () async { + await _environment.asMixin(() async { + await _runBuiltInCallable( + arguments, mixin, nodeWithSpanWithoutContent); + }); + }); case UserDefinedCallable( declaration: MixinRule(hasContent: false) ) - when node.content != null: + when contentCallable != null: throw MultiSpanSassRuntimeException( "Mixin doesn't accept a content block.", - node.spanWithoutContent, + nodeWithSpanWithoutContent.span, "invocation", {mixin.declaration.arguments.spanWithName: "declaration"}, - _stackTrace(node.spanWithoutContent)); + _stackTrace(nodeWithSpanWithoutContent.span)); case UserDefinedCallable(): - var contentCallable = node.content.andThen((content) => - UserDefinedCallable(content, _environment.closure(), - inDependency: _inDependency)); - await _runUserDefinedCallable(node.arguments, mixin, nodeWithSpan, - () async { + await _runUserDefinedCallable( + arguments, mixin, nodeWithSpanWithoutContent, () async { await _environment.withContent(contentCallable, () async { await _environment.asMixin(() async { for (var statement in mixin.declaration.children) { - await _addErrorSpan(nodeWithSpan, () => statement.accept(this)); + await _addErrorSpan( + nodeWithSpanWithoutContent, () => statement.accept(this)); } }); }); @@ -1776,6 +1844,20 @@ final class _EvaluateVisitor case _: throw UnsupportedError("Unknown callable type $mixin."); } + } + + Future visitIncludeRule(IncludeRule node) async { + var mixin = _addExceptionSpan(node, + () => _environment.getMixin(node.name, namespace: node.namespace)); + var contentCallable = node.content.andThen((content) => UserDefinedCallable( + content, _environment.closure(), + inDependency: _inDependency)); + + var nodeWithSpanWithoutContent = + AstNode.fake(() => node.spanWithoutContent); + + await _applyMixin(mixin, contentCallable, node.arguments, node, + nodeWithSpanWithoutContent); return null; } diff --git a/lib/src/visitor/evaluate.dart b/lib/src/visitor/evaluate.dart index de45a4c56..0aaecec85 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: 1c3027293ac9cb8a0d03b18c9ca447d62c2733d7 +// Checksum: 358960b72c6e4f48d3e2e9d52be3abbe9e8b5a9f // // ignore_for_file: unused_import @@ -426,6 +426,19 @@ final class _EvaluateVisitor }); }, url: "sass:meta"), + BuiltInCallable.function("module-mixins", r"$module", (arguments) { + var namespace = arguments[0].assertString("module"); + var module = _environment.modules[namespace.text]; + if (module == null) { + throw 'There is no module with namespace "${namespace.text}".'; + } + + return SassMap({ + for (var (name, value) in module.mixins.pairs) + SassString(name): SassMixin(value) + }); + }, url: "sass:meta"), + BuiltInCallable.function( "get-function", r"$name, $css: false, $module: null", (arguments) { var name = arguments[0].assertString("name"); @@ -452,6 +465,20 @@ final class _EvaluateVisitor return SassFunction(callable); }, url: "sass:meta"), + BuiltInCallable.function("get-mixin", r"$name, $module: null", + (arguments) { + var name = arguments[0].assertString("name"); + var module = arguments[1].realNull?.assertString("module"); + + var callable = _addExceptionSpan( + _callableNode!, + () => _environment.getMixin(name.text.replaceAll("_", "-"), + namespace: module?.text)); + if (callable == null) throw "Mixin not found: $name"; + + return SassMixin(callable); + }, url: "sass:meta"), + BuiltInCallable.function("call", r"$function, $args...", (arguments) { var function = arguments[0]; var args = arguments[1] as SassArgumentList; @@ -522,7 +549,32 @@ final class _EvaluateVisitor configuration: configuration, namesInErrors: true); _assertConfigurationIsEmpty(configuration, nameInError: true); - }, url: "sass:meta") + }, url: "sass:meta"), + BuiltInCallable.mixin("apply", r"$mixin, $args...", (arguments) { + var mixin = arguments[0]; + var args = arguments[1] as SassArgumentList; + + var callableNode = _callableNode!; + var invocation = ArgumentInvocation( + const [], + const {}, + callableNode.span, + rest: ValueExpression(args, callableNode.span), + ); + + var callable = mixin.assertMixin("mixin").callable; + var content = _environment.content; + + // ignore: unnecessary_type_check + if (callable is Callable) { + _applyMixin( + callable, content, invocation, callableNode, callableNode); + } else { + throw SassScriptException( + "The mixin ${callable.name} is asynchronous.\n" + "This is probably caused by a bug in a Sass plugin."); + } + }, url: "sass:meta", acceptsContent: true), ]; var metaModule = BuiltInModule("meta", @@ -1730,40 +1782,55 @@ final class _EvaluateVisitor } } - Value? visitIncludeRule(IncludeRule node) { - var nodeWithSpan = AstNode.fake(() => node.spanWithoutContent); - var mixin = _addExceptionSpan(node, - () => _environment.getMixin(node.name, namespace: node.namespace)); + /// Evaluate a given [mixin] with [arguments] and [contentCallable] + void _applyMixin( + Callable? mixin, + UserDefinedCallable? contentCallable, + ArgumentInvocation arguments, + AstNode nodeWithSpan, + AstNode nodeWithSpanWithoutContent) { switch (mixin) { case null: - throw _exception("Undefined mixin.", node.span); - - case BuiltInCallable() when node.content != null: - throw _exception("Mixin doesn't accept a content block.", node.span); + throw _exception("Undefined mixin.", nodeWithSpan.span); + case BuiltInCallable(acceptsContent: false) when contentCallable != null: + { + var evaluated = _evaluateArguments(arguments); + var (overload, _) = mixin.callbackFor( + evaluated.positional.length, MapKeySet(evaluated.named)); + throw MultiSpanSassRuntimeException( + "Mixin doesn't accept a content block.", + nodeWithSpanWithoutContent.span, + "invocation", + {overload.spanWithName: "declaration"}, + _stackTrace(nodeWithSpanWithoutContent.span)); + } case BuiltInCallable(): - _runBuiltInCallable(node.arguments, mixin, nodeWithSpan); + _environment.withContent(contentCallable, () { + _environment.asMixin(() { + _runBuiltInCallable(arguments, mixin, nodeWithSpanWithoutContent); + }); + }); case UserDefinedCallable( declaration: MixinRule(hasContent: false) ) - when node.content != null: + when contentCallable != null: throw MultiSpanSassRuntimeException( "Mixin doesn't accept a content block.", - node.spanWithoutContent, + nodeWithSpanWithoutContent.span, "invocation", {mixin.declaration.arguments.spanWithName: "declaration"}, - _stackTrace(node.spanWithoutContent)); + _stackTrace(nodeWithSpanWithoutContent.span)); case UserDefinedCallable(): - var contentCallable = node.content.andThen((content) => - UserDefinedCallable(content, _environment.closure(), - inDependency: _inDependency)); - _runUserDefinedCallable(node.arguments, mixin, nodeWithSpan, () { + _runUserDefinedCallable(arguments, mixin, nodeWithSpanWithoutContent, + () { _environment.withContent(contentCallable, () { _environment.asMixin(() { for (var statement in mixin.declaration.children) { - _addErrorSpan(nodeWithSpan, () => statement.accept(this)); + _addErrorSpan( + nodeWithSpanWithoutContent, () => statement.accept(this)); } }); }); @@ -1772,6 +1839,20 @@ final class _EvaluateVisitor case _: throw UnsupportedError("Unknown callable type $mixin."); } + } + + Value? visitIncludeRule(IncludeRule node) { + var mixin = _addExceptionSpan(node, + () => _environment.getMixin(node.name, namespace: node.namespace)); + var contentCallable = node.content.andThen((content) => UserDefinedCallable( + content, _environment.closure(), + inDependency: _inDependency)); + + var nodeWithSpanWithoutContent = + AstNode.fake(() => node.spanWithoutContent); + + _applyMixin(mixin, contentCallable, node.arguments, node, + nodeWithSpanWithoutContent); return null; } diff --git a/lib/src/visitor/interface/value.dart b/lib/src/visitor/interface/value.dart index db25c86d5..e25f5ba11 100644 --- a/lib/src/visitor/interface/value.dart +++ b/lib/src/visitor/interface/value.dart @@ -12,6 +12,7 @@ abstract interface class ValueVisitor { T visitCalculation(SassCalculation value); T visitColor(SassColor value); T visitFunction(SassFunction value); + T visitMixin(SassMixin value); T visitList(SassList value); T visitMap(SassMap value); T visitNull(); diff --git a/lib/src/visitor/serialize.dart b/lib/src/visitor/serialize.dart index f86d6c7fb..7073c4b4f 100644 --- a/lib/src/visitor/serialize.dart +++ b/lib/src/visitor/serialize.dart @@ -671,6 +671,16 @@ final class _SerializeVisitor _buffer.writeCharCode($rparen); } + void visitMixin(SassMixin mixin) { + if (!_inspect) { + throw SassScriptException("$mixin isn't a valid CSS value."); + } + + _buffer.write("get-mixin("); + _visitQuotedString(mixin.callable.name); + _buffer.writeCharCode($rparen); + } + void visitList(SassList value) { if (value.hasBrackets) { _buffer.writeCharCode($lbracket); diff --git a/tool/grind.dart b/tool/grind.dart index ce65138df..425730c03 100644 --- a/tool/grind.dart +++ b/tool/grind.dart @@ -61,6 +61,7 @@ void main(List args) { 'SassFunction', 'SassList', 'SassMap', + 'SassMixin', 'SassNumber', 'SassString', 'Value', From 8e6a26cc62a61bb0b757d34e8c0c2f2bb706ab16 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Thu, 5 Oct 2023 15:31:13 -0700 Subject: [PATCH 26/30] Cut a release (#2107) --- pkg/sass_api/CHANGELOG.md | 4 ++++ pkg/sass_api/pubspec.yaml | 4 ++-- pubspec.yaml | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/pkg/sass_api/CHANGELOG.md b/pkg/sass_api/CHANGELOG.md index 9b9e2c55c..e3bfcd0a9 100644 --- a/pkg/sass_api/CHANGELOG.md +++ b/pkg/sass_api/CHANGELOG.md @@ -1,3 +1,7 @@ +## 9.2.0 + +* No user-visible changes. + ## 9.1.0 * No user-visible changes. diff --git a/pkg/sass_api/pubspec.yaml b/pkg/sass_api/pubspec.yaml index 3fb53a144..25caba128 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: 9.1.0 +version: 9.2.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.68.0 + sass: 1.69.0 dev_dependencies: dartdoc: ^6.0.0 diff --git a/pubspec.yaml b/pubspec.yaml index b518e7566..ca83d52f0 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass -version: 1.69.0-dev +version: 1.69.0 description: A Sass implementation in Dart. homepage: https://github.com/sass/dart-sass From 2e12e3f3ec05031734d46bfb10c83425ed0c6f9a Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Thu, 5 Oct 2023 18:33:58 -0700 Subject: [PATCH 27/30] Poke CI From 2eef6a2408b7f52bf49802699633e67b3ada450c Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Fri, 6 Oct 2023 14:51:19 -0700 Subject: [PATCH 28/30] Update for `oklab()` tests (#2094) --- lib/src/functions/color.dart | 25 +++++++++++---------- lib/src/value/color.dart | 3 ++- lib/src/value/color/channel.dart | 28 ++++++++++++++++++++--- lib/src/value/color/space/oklab.dart | 2 +- lib/src/value/color/space/oklch.dart | 2 +- lib/src/value/color/space/utils.dart | 3 ++- lib/src/visitor/serialize.dart | 33 ++++++++++------------------ 7 files changed, 57 insertions(+), 39 deletions(-) diff --git a/lib/src/functions/color.dart b/lib/src/functions/color.dart index 713c74a49..7edfc51dc 100644 --- a/lib/src/functions/color.dart +++ b/lib/src/functions/color.dart @@ -476,11 +476,12 @@ final module = BuiltInModule("color", functions: [ var channelInfo = color.space.channels[channelIndex]; var channelValue = color.channels[channelIndex]; + var unit = channelInfo.associatedUnit; + if (unit == '%') { + channelValue = channelValue * 100 / (channelInfo as LinearChannel).max; + } - return channelInfo is LinearChannel - ? SassNumber(channelValue, - channelInfo.min == 0 && channelInfo.max == 100 ? '%' : null) - : SassNumber(channelValue, 'deg'); + return SassNumber(channelValue, unit); }), _function("same", r"$color1, $color2", (arguments) { @@ -1198,7 +1199,8 @@ Value _parseChannels(String functionName, Value input, var channelName = space?.channels[channels.indexOf(channel)].name ?? 'channel'; throw SassScriptException( - 'Expected $channelName $channel to be a number.', name); + 'Expected $channelName channel to be a number, was $channel.', + name); } } @@ -1346,14 +1348,15 @@ SassColor _colorFromChannels(ColorSpace space, SassNumber? channel0, alpha, fromRgbFunction ? ColorFormat.rgbFunction : null); - case ColorSpace.lab: - case ColorSpace.lch: - case ColorSpace.oklab: - case ColorSpace.oklch: + case ColorSpace.lab || + ColorSpace.lch || + ColorSpace.oklab || + ColorSpace.oklch: return SassColor.forSpaceInternal( space, - _channelFromValue(space.channels[0], channel0) - .andThen((lightness) => fuzzyClamp(lightness, 0, 100)), + _channelFromValue(space.channels[0], channel0).andThen((lightness) => + fuzzyClamp( + lightness, 0, (space.channels[0] as LinearChannel).max)), _channelFromValue(space.channels[1], channel1), _channelFromValue(space.channels[2], channel2), alpha); diff --git a/lib/src/value/color.dart b/lib/src/value/color.dart index 96abc5263..6ffdb2bfa 100644 --- a/lib/src/value/color.dart +++ b/lib/src/value/color.dart @@ -531,7 +531,8 @@ class SassColor extends Value { return SassColor.forSpaceInternal( space, clampChannel0 - ? channels[0].andThen((value) => fuzzyClamp(value, 0, 100)) + ? channels[0].andThen((value) => fuzzyClamp( + value, 0, (space.channels[0] as LinearChannel).max)) : channels[0], clampChannel12 ? channels[1].andThen((value) => fuzzyClamp(value, 0, 100)) diff --git a/lib/src/value/color/channel.dart b/lib/src/value/color/channel.dart index 6fe9f9dac..61cd115cf 100644 --- a/lib/src/value/color/channel.dart +++ b/lib/src/value/color/channel.dart @@ -21,9 +21,21 @@ class ColorChannel { /// This is true if and only if this is not a [LinearChannel]. final bool isPolarAngle; + /// The unit that's associated with this channel. + /// + /// Some channels are typically written without units, while others have a + /// specific unit that is conventionally applied to their values. Although any + /// compatible unit or unitless value will work for input¹, this unit is used + /// when the value is serialized or returned from a Sass function. + /// + /// 1: Unless [LinearChannel.requiresPercent] is set, in which case unitless + /// values are not allowed. + final String? associatedUnit; + /// @nodoc @internal - const ColorChannel(this.name, {required this.isPolarAngle}); + const ColorChannel(this.name, + {required this.isPolarAngle, this.associatedUnit}); /// Returns whether this channel is [analogous] to [other]. /// @@ -65,9 +77,19 @@ class LinearChannel extends ColorChannel { /// forbids unitless values. final bool requiresPercent; + /// Creates a linear color channel. + /// + /// By default, [ColorChannel.associatedUnit] is set to `%` if and only if + /// [min] is 0 and [max] is 100. However, if [conventionallyPercent] is + /// true, it's set to `%`, and if it's false, it's set to null. + /// /// @nodoc @internal const LinearChannel(String name, this.min, this.max, - {this.requiresPercent = false}) - : super(name, isPolarAngle: false); + {this.requiresPercent = false, bool? conventionallyPercent}) + : super(name, + isPolarAngle: false, + associatedUnit: (conventionallyPercent ?? (min == 0 && max == 100)) + ? '%' + : null); } diff --git a/lib/src/value/color/space/oklab.dart b/lib/src/value/color/space/oklab.dart index de63b7673..cf8951bbd 100644 --- a/lib/src/value/color/space/oklab.dart +++ b/lib/src/value/color/space/oklab.dart @@ -23,7 +23,7 @@ class OklabColorSpace extends ColorSpace { const OklabColorSpace() : super('oklab', const [ - LinearChannel('lightness', 0, 1), + LinearChannel('lightness', 0, 1, conventionallyPercent: true), LinearChannel('a', -0.4, 0.4), LinearChannel('b', -0.4, 0.4) ]); diff --git a/lib/src/value/color/space/oklch.dart b/lib/src/value/color/space/oklch.dart index 6bd63c736..30887f6e7 100644 --- a/lib/src/value/color/space/oklch.dart +++ b/lib/src/value/color/space/oklch.dart @@ -23,7 +23,7 @@ class OklchColorSpace extends ColorSpace { const OklchColorSpace() : super('oklch', const [ - LinearChannel('lightness', 0, 1), + LinearChannel('lightness', 0, 1, conventionallyPercent: true), LinearChannel('chroma', 0, 0.4), hueChannel ]); diff --git a/lib/src/value/color/space/utils.dart b/lib/src/value/color/space/utils.dart index b1e16f2d9..427da3188 100644 --- a/lib/src/value/color/space/utils.dart +++ b/lib/src/value/color/space/utils.dart @@ -14,7 +14,8 @@ const labKappa = 24389 / 27; // 29^3/3^3; const labEpsilon = 216 / 24389; // 6^3/29^3; /// The hue channel shared across all polar color spaces. -const hueChannel = ColorChannel('hue', isPolarAngle: true); +const hueChannel = + ColorChannel('hue', isPolarAngle: true, associatedUnit: 'deg'); /// The color channels shared across all RGB color spaces (except the legacy RGB space). const rgbChannels = [ diff --git a/lib/src/visitor/serialize.dart b/lib/src/visitor/serialize.dart index a7101a827..8f4569665 100644 --- a/lib/src/visitor/serialize.dart +++ b/lib/src/visitor/serialize.dart @@ -560,38 +560,29 @@ final class _SerializeVisitor case ColorSpace.rgb || ColorSpace.hsl || ColorSpace.hwb: _writeLegacyColor(value); - case ColorSpace.lab || ColorSpace.oklab: + case ColorSpace.lab || + ColorSpace.oklab || + ColorSpace.lch || + ColorSpace.oklch: _buffer ..write(value.space) ..writeCharCode($lparen); - _writeChannel(value.channel0OrNull); - if (!_isCompressed && - value.space == ColorSpace.lab && - !value.isChannel0Missing) { + if (!_isCompressed && !value.isChannel0Missing) { + var max = (value.space.channels[0] as LinearChannel).max; + _writeNumber(value.channel0 * 100 / max); _buffer.writeCharCode($percent); + } else { + _writeChannel(value.channel0OrNull); } _buffer.writeCharCode($space); _writeChannel(value.channel1OrNull); _buffer.writeCharCode($space); _writeChannel(value.channel2OrNull); - _maybeWriteSlashAlpha(value); - _buffer.writeCharCode($rparen); - - case ColorSpace.lch || ColorSpace.oklch: - _buffer - ..write(value.space) - ..writeCharCode($lparen); - _writeChannel(value.channel0OrNull); if (!_isCompressed && - value.space == ColorSpace.lch && - !value.isChannel0Missing) { - _buffer.writeCharCode($percent); + !value.isChannel2Missing && + value.space.channels[2].isPolarAngle) { + _buffer.write('deg'); } - _buffer.writeCharCode($space); - _writeChannel(value.channel1OrNull); - _buffer.writeCharCode($space); - _writeChannel(value.channel2OrNull); - if (!_isCompressed && !value.isChannel2Missing) _buffer.write('deg'); _maybeWriteSlashAlpha(value); _buffer.writeCharCode($rparen); From 5ea5ab0e811a4ba72aaaf3aa53bbeeb0795744d8 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Tue, 10 Oct 2023 13:44:38 -0700 Subject: [PATCH 29/30] Add support for relative color syntax (#2112) See sass/sass#3673 --- lib/src/functions/color.dart | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/src/functions/color.dart b/lib/src/functions/color.dart index 7edfc51dc..ee69e6896 100644 --- a/lib/src/functions/color.dart +++ b/lib/src/functions/color.dart @@ -1165,6 +1165,10 @@ Value _parseChannels(String functionName, Value input, case []: throw SassScriptException('Color component list may not be empty.', name); + case [SassString(:var text, hasQuotes: false), ...] + when text.toLowerCase() == "from": + return _functionString(functionName, [input]); + case _ when components.isVar: channels = [components]; From 5b3de085f20441732176c785e01c3403c6eb4ac9 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Wed, 11 Oct 2023 16:33:33 -0700 Subject: [PATCH 30/30] Update for `lch()` tests (#2108) --- lib/src/functions/color.dart | 2 +- lib/src/value/color.dart | 2 +- lib/src/value/color/space/lch.dart | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/src/functions/color.dart b/lib/src/functions/color.dart index ee69e6896..472006e6b 100644 --- a/lib/src/functions/color.dart +++ b/lib/src/functions/color.dart @@ -1384,7 +1384,7 @@ double? _channelFromValue(ColorChannel channel, SassNumber? value) => 'Expected $value to have unit "%".', channel.name), LinearChannel() => _percentageOrUnitless(value, channel.max, channel.name), - _ => value.coerceValueToUnit('deg', channel.name) + _ => value.coerceValueToUnit('deg', channel.name) % 360 }); /// Returns whether [value] is an unquoted string case-insensitively equal to diff --git a/lib/src/value/color.dart b/lib/src/value/color.dart index 6ffdb2bfa..33b6c2f47 100644 --- a/lib/src/value/color.dart +++ b/lib/src/value/color.dart @@ -249,7 +249,7 @@ class SassColor extends Value { /// This color's hue, between `0` and `360`. @Deprecated('Use channel() instead.') - double get hue => _legacyChannel(ColorSpace.hsl, 'hue') % 360; + double get hue => _legacyChannel(ColorSpace.hsl, 'hue'); /// This color's saturation, a percentage between `0` and `100`. @Deprecated('Use channel() instead.') diff --git a/lib/src/value/color/space/lch.dart b/lib/src/value/color/space/lch.dart index d522e6454..c66baab64 100644 --- a/lib/src/value/color/space/lch.dart +++ b/lib/src/value/color/space/lch.dart @@ -24,7 +24,7 @@ class LchColorSpace extends ColorSpace { const LchColorSpace() : super('lch', const [ LinearChannel('lightness', 0, 100), - LinearChannel('chroma', 0, 100), + LinearChannel('chroma', 0, 150), hueChannel ]);