diff --git a/CHANGELOG.md b/CHANGELOG.md index 1696968b6..39ec4ff4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,20 @@ ## 1.33.0 +* Deprecate the use of `/` for division. The new `math.div()` function should be + used instead. See [this page][] for details. + + [this page]: https://sass-lang.com/documentation/breaking-changes/slash-div + +* Add a `list.slash()` function that returns a slash-separated list. + +* **Potentially breaking bug fix:** The heuristics around when potentially + slash-separated numbers are converted to slash-free numbers—for example, when + `1/2` will be printed as `0.5` rather than `1/2`—have been slightly expanded. + Previously, a number would be made slash-free if it was passed as an argument + to a *user-defined function*, but not to a *built-in function*. Now it will be + made slash-free in both cases. This is a behavioral change, but it's unlikely + to affect any real-world stylesheets. + ### JS API * The `this` context for importers now has a `fromImport` field, which is `true` diff --git a/lib/src/async_environment.dart b/lib/src/async_environment.dart index 5c1b2d33b..dedf2a402 100644 --- a/lib/src/async_environment.dart +++ b/lib/src/async_environment.dart @@ -25,6 +25,11 @@ import 'utils.dart'; import 'value.dart'; import 'visitor/clone_css.dart'; +// TODO(nweiz): This used to avoid tracking source spans for variables if source +// map generation was disabled. We always have to track them now to produce +// better warnings for /-as-division, but once those warnings are gone we should +// go back to tracking conditionally. + /// The lexical environment in which Sass is executed. /// /// This tracks lexically-scoped information, such as variables, functions, and @@ -77,12 +82,10 @@ class AsyncEnvironment { /// The nodes where each variable in [_variables] was defined. /// - /// This is `null` if source mapping is disabled. - /// /// This stores [AstNode]s rather than [FileSpan]s so it can avoid calling /// [AstNode.span] if the span isn't required, since some nodes need to do /// real work to manufacture a source span. - final List>? _variableNodes; + final List> _variableNodes; /// A map of variable names to their indices in [_variables]. /// @@ -145,7 +148,7 @@ class AsyncEnvironment { /// Creates an [AsyncEnvironment]. /// /// If [sourceMap] is `true`, this tracks variables' source locations - AsyncEnvironment({bool sourceMap = false}) + AsyncEnvironment() : _modules = {}, _namespaceNodes = {}, _globalModules = {}, @@ -155,7 +158,7 @@ class AsyncEnvironment { _nestedForwardedModules = null, _allModules = [], _variables = [{}], - _variableNodes = sourceMap ? [{}] : null, + _variableNodes = [{}], _variableIndices = {}, _functions = [{}], _functionIndices = {}, @@ -199,7 +202,7 @@ class AsyncEnvironment { _nestedForwardedModules, _allModules, _variables.toList(), - _variableNodes?.toList(), + _variableNodes.toList(), _functions.toList(), _mixins.toList(), _content); @@ -218,7 +221,7 @@ class AsyncEnvironment { null, [], _variables.toList(), - _variableNodes?.toList(), + _variableNodes.toList(), _functions.toList(), _mixins.toList(), _content); @@ -406,7 +409,7 @@ class AsyncEnvironment { for (var variable in forwardedVariableNames) { _variableIndices.remove(variable); _variables.last.remove(variable); - _variableNodes?.last.remove(variable); + _variableNodes.last.remove(variable); } for (var function in forwardedFunctionNames) { _functionIndices.remove(function); @@ -468,17 +471,10 @@ class AsyncEnvironment { /// required, since some nodes need to do real work to manufacture a source /// span. AstNode? getVariableNode(String name, {String? namespace}) { - var variableNodes = _variableNodes; - if (variableNodes == null) { - throw StateError( - "getVariableNodes() should only be called if sourceMap = true was " - "passed in."); - } - - if (namespace != null) return _getModule(namespace).variableNodes![name]; + if (namespace != null) return _getModule(namespace).variableNodes[name]; if (_lastVariableName == name) { - return variableNodes[_lastVariableIndex!][name] ?? + return _variableNodes[_lastVariableIndex!][name] ?? _getVariableNodeFromGlobalModule(name); } @@ -486,7 +482,7 @@ class AsyncEnvironment { if (index != null) { _lastVariableName = name; _lastVariableIndex = index; - return variableNodes[index][name] ?? + return _variableNodes[index][name] ?? _getVariableNodeFromGlobalModule(name); } @@ -496,7 +492,8 @@ class AsyncEnvironment { _lastVariableName = name; _lastVariableIndex = index; _variableIndices[name] = index; - return variableNodes[index][name] ?? _getVariableNodeFromGlobalModule(name); + return _variableNodes[index][name] ?? + _getVariableNodeFromGlobalModule(name); } /// Returns the node for the variable named [name] from a namespaceless @@ -511,7 +508,7 @@ class AsyncEnvironment { // We don't need to worry about multiple modules defining the same variable, // because that's already been checked by [getVariable]. for (var module in _globalModules) { - var value = module.variableNodes![name]; + var value = module.variableNodes[name]; if (value != null) return value; } return null; @@ -559,7 +556,7 @@ class AsyncEnvironment { /// defined with the given namespace, if no variable with the given [name] is /// defined in module with the given namespace, or if no [namespace] is passed /// and multiple global modules define variables named [name]. - void setVariable(String name, Value value, AstNode? nodeWithSpan, + void setVariable(String name, Value value, AstNode nodeWithSpan, {String? namespace, bool global = false}) { if (namespace != null) { _getModule(namespace).setVariable(name, value, nodeWithSpan); @@ -587,7 +584,7 @@ class AsyncEnvironment { } _variables.first[name] = value; - if (nodeWithSpan != null) _variableNodes?.first[name] = nodeWithSpan; + _variableNodes.first[name] = nodeWithSpan; return; } @@ -617,7 +614,7 @@ class AsyncEnvironment { _lastVariableName = name; _lastVariableIndex = index; _variables[index][name] = value; - _variableNodes?[index][name] = nodeWithSpan!; + _variableNodes[index][name] = nodeWithSpan; } /// Sets the variable named [name] to [value], associated with @@ -629,15 +626,13 @@ class AsyncEnvironment { /// This takes an [AstNode] rather than a [FileSpan] so it can avoid calling /// [AstNode.span] if the span isn't required, since some nodes need to do /// real work to manufacture a source span. - void setLocalVariable(String name, Value value, AstNode? nodeWithSpan) { + void setLocalVariable(String name, Value value, AstNode nodeWithSpan) { var index = _variables.length - 1; _lastVariableName = name; _lastVariableIndex = index; _variableIndices[name] = index; _variables[index][name] = value; - if (nodeWithSpan != null) { - _variableNodes?[index][name] = nodeWithSpan; - } + _variableNodes[index][name] = nodeWithSpan; } /// Returns the value of the function named [name], optionally with the given @@ -789,7 +784,7 @@ class AsyncEnvironment { _inSemiGlobalScope = semiGlobal; _variables.add({}); - _variableNodes?.add({}); + _variableNodes.add({}); _functions.add({}); _mixins.add({}); _nestedForwardedModules?.add([]); @@ -818,12 +813,12 @@ class AsyncEnvironment { var configuration = {}; for (var i = 0; i < _variables.length; i++) { var values = _variables[i]; - var nodes = _variableNodes?[i] ?? {}; + var nodes = _variableNodes[i]; for (var entry in values.entries) { // Implicit configurations are never invalid, making [configurationSpan] // unnecessary, so we pass null here to avoid having to compute it. configuration[entry.key] = - ConfiguredValue.implicit(entry.value, nodes[entry.key]); + ConfiguredValue.implicit(entry.value, nodes[entry.key]!); } } return Configuration.implicit(configuration); @@ -921,7 +916,7 @@ class _EnvironmentModule implements Module { final List upstream; final Map variables; - final Map? variableNodes; + final Map variableNodes; final Map functions; final Map mixins; final ExtensionStore extensionStore; @@ -951,10 +946,8 @@ class _EnvironmentModule implements Module { _makeModulesByVariable(forwarded), _memberMap(environment._variables.first, forwarded.map((module) => module.variables)), - environment._variableNodes.andThen((nodes) => _memberMap( - nodes.first, - // dart-lang/sdk#45348 - forwarded!.map((module) => module.variableNodes!))), + _memberMap(environment._variableNodes.first, + forwarded.map((module) => module.variableNodes)), _memberMap(environment._functions.first, forwarded.map((module) => module.functions)), _memberMap(environment._mixins.first, @@ -1017,7 +1010,7 @@ class _EnvironmentModule implements Module { required this.transitivelyContainsExtensions}) : upstream = _environment._allModules; - void setVariable(String name, Value value, AstNode? nodeWithSpan) { + void setVariable(String name, Value value, AstNode nodeWithSpan) { var module = _modulesByVariable[name]; if (module != null) { module.setVariable(name, value, nodeWithSpan); @@ -1029,9 +1022,7 @@ class _EnvironmentModule implements Module { } _environment._variables.first[name] = value; - if (nodeWithSpan != null) { - _environment._variableNodes?.first[name] = nodeWithSpan; - } + _environment._variableNodes.first[name] = nodeWithSpan; return; } diff --git a/lib/src/configured_value.dart b/lib/src/configured_value.dart index 01d46b3c4..79da5306f 100644 --- a/lib/src/configured_value.dart +++ b/lib/src/configured_value.dart @@ -17,9 +17,7 @@ class ConfiguredValue { final FileSpan? configurationSpan; /// The [AstNode] where the variable's value originated. - /// - /// This is used to generate source maps. - final AstNode? assignmentNode; + final AstNode assignmentNode; /// Creates a variable value that's been configured explicitly with a `with` /// clause. diff --git a/lib/src/environment.dart b/lib/src/environment.dart index da4910951..5bde2aeb8 100644 --- a/lib/src/environment.dart +++ b/lib/src/environment.dart @@ -5,7 +5,7 @@ // DO NOT EDIT. This file was generated from async_environment.dart. // See tool/grind/synchronize.dart for details. // -// Checksum: bb0b47fc04e32f36a0f87dc73bdfe3f89dc51aa4 +// Checksum: 588f0864bb1f889586178c799d91696341ecf218 // // ignore_for_file: unused_import @@ -32,6 +32,11 @@ import 'utils.dart'; import 'value.dart'; import 'visitor/clone_css.dart'; +// TODO(nweiz): This used to avoid tracking source spans for variables if source +// map generation was disabled. We always have to track them now to produce +// better warnings for /-as-division, but once those warnings are gone we should +// go back to tracking conditionally. + /// The lexical environment in which Sass is executed. /// /// This tracks lexically-scoped information, such as variables, functions, and @@ -84,12 +89,10 @@ class Environment { /// The nodes where each variable in [_variables] was defined. /// - /// This is `null` if source mapping is disabled. - /// /// This stores [AstNode]s rather than [FileSpan]s so it can avoid calling /// [AstNode.span] if the span isn't required, since some nodes need to do /// real work to manufacture a source span. - final List>? _variableNodes; + final List> _variableNodes; /// A map of variable names to their indices in [_variables]. /// @@ -152,7 +155,7 @@ class Environment { /// Creates an [Environment]. /// /// If [sourceMap] is `true`, this tracks variables' source locations - Environment({bool sourceMap = false}) + Environment() : _modules = {}, _namespaceNodes = {}, _globalModules = {}, @@ -162,7 +165,7 @@ class Environment { _nestedForwardedModules = null, _allModules = [], _variables = [{}], - _variableNodes = sourceMap ? [{}] : null, + _variableNodes = [{}], _variableIndices = {}, _functions = [{}], _functionIndices = {}, @@ -206,7 +209,7 @@ class Environment { _nestedForwardedModules, _allModules, _variables.toList(), - _variableNodes?.toList(), + _variableNodes.toList(), _functions.toList(), _mixins.toList(), _content); @@ -225,7 +228,7 @@ class Environment { null, [], _variables.toList(), - _variableNodes?.toList(), + _variableNodes.toList(), _functions.toList(), _mixins.toList(), _content); @@ -414,7 +417,7 @@ class Environment { for (var variable in forwardedVariableNames) { _variableIndices.remove(variable); _variables.last.remove(variable); - _variableNodes?.last.remove(variable); + _variableNodes.last.remove(variable); } for (var function in forwardedFunctionNames) { _functionIndices.remove(function); @@ -476,17 +479,10 @@ class Environment { /// required, since some nodes need to do real work to manufacture a source /// span. AstNode? getVariableNode(String name, {String? namespace}) { - var variableNodes = _variableNodes; - if (variableNodes == null) { - throw StateError( - "getVariableNodes() should only be called if sourceMap = true was " - "passed in."); - } - - if (namespace != null) return _getModule(namespace).variableNodes![name]; + if (namespace != null) return _getModule(namespace).variableNodes[name]; if (_lastVariableName == name) { - return variableNodes[_lastVariableIndex!][name] ?? + return _variableNodes[_lastVariableIndex!][name] ?? _getVariableNodeFromGlobalModule(name); } @@ -494,7 +490,7 @@ class Environment { if (index != null) { _lastVariableName = name; _lastVariableIndex = index; - return variableNodes[index][name] ?? + return _variableNodes[index][name] ?? _getVariableNodeFromGlobalModule(name); } @@ -504,7 +500,8 @@ class Environment { _lastVariableName = name; _lastVariableIndex = index; _variableIndices[name] = index; - return variableNodes[index][name] ?? _getVariableNodeFromGlobalModule(name); + return _variableNodes[index][name] ?? + _getVariableNodeFromGlobalModule(name); } /// Returns the node for the variable named [name] from a namespaceless @@ -519,7 +516,7 @@ class Environment { // We don't need to worry about multiple modules defining the same variable, // because that's already been checked by [getVariable]. for (var module in _globalModules) { - var value = module.variableNodes![name]; + var value = module.variableNodes[name]; if (value != null) return value; } return null; @@ -567,7 +564,7 @@ class Environment { /// defined with the given namespace, if no variable with the given [name] is /// defined in module with the given namespace, or if no [namespace] is passed /// and multiple global modules define variables named [name]. - void setVariable(String name, Value value, AstNode? nodeWithSpan, + void setVariable(String name, Value value, AstNode nodeWithSpan, {String? namespace, bool global = false}) { if (namespace != null) { _getModule(namespace).setVariable(name, value, nodeWithSpan); @@ -595,7 +592,7 @@ class Environment { } _variables.first[name] = value; - if (nodeWithSpan != null) _variableNodes?.first[name] = nodeWithSpan; + _variableNodes.first[name] = nodeWithSpan; return; } @@ -625,7 +622,7 @@ class Environment { _lastVariableName = name; _lastVariableIndex = index; _variables[index][name] = value; - _variableNodes?[index][name] = nodeWithSpan!; + _variableNodes[index][name] = nodeWithSpan; } /// Sets the variable named [name] to [value], associated with @@ -637,15 +634,13 @@ class Environment { /// This takes an [AstNode] rather than a [FileSpan] so it can avoid calling /// [AstNode.span] if the span isn't required, since some nodes need to do /// real work to manufacture a source span. - void setLocalVariable(String name, Value value, AstNode? nodeWithSpan) { + void setLocalVariable(String name, Value value, AstNode nodeWithSpan) { var index = _variables.length - 1; _lastVariableName = name; _lastVariableIndex = index; _variableIndices[name] = index; _variables[index][name] = value; - if (nodeWithSpan != null) { - _variableNodes?[index][name] = nodeWithSpan; - } + _variableNodes[index][name] = nodeWithSpan; } /// Returns the value of the function named [name], optionally with the given @@ -795,7 +790,7 @@ class Environment { _inSemiGlobalScope = semiGlobal; _variables.add({}); - _variableNodes?.add({}); + _variableNodes.add({}); _functions.add({}); _mixins.add({}); _nestedForwardedModules?.add([]); @@ -824,12 +819,12 @@ class Environment { var configuration = {}; for (var i = 0; i < _variables.length; i++) { var values = _variables[i]; - var nodes = _variableNodes?[i] ?? {}; + var nodes = _variableNodes[i]; for (var entry in values.entries) { // Implicit configurations are never invalid, making [configurationSpan] // unnecessary, so we pass null here to avoid having to compute it. configuration[entry.key] = - ConfiguredValue.implicit(entry.value, nodes[entry.key]); + ConfiguredValue.implicit(entry.value, nodes[entry.key]!); } } return Configuration.implicit(configuration); @@ -928,7 +923,7 @@ class _EnvironmentModule implements Module { final List> upstream; final Map variables; - final Map? variableNodes; + final Map variableNodes; final Map functions; final Map mixins; final ExtensionStore extensionStore; @@ -958,10 +953,8 @@ class _EnvironmentModule implements Module { _makeModulesByVariable(forwarded), _memberMap(environment._variables.first, forwarded.map((module) => module.variables)), - environment._variableNodes.andThen((nodes) => _memberMap( - nodes.first, - // dart-lang/sdk#45348 - forwarded!.map((module) => module.variableNodes!))), + _memberMap(environment._variableNodes.first, + forwarded.map((module) => module.variableNodes)), _memberMap(environment._functions.first, forwarded.map((module) => module.functions)), _memberMap(environment._mixins.first, @@ -1025,7 +1018,7 @@ class _EnvironmentModule implements Module { required this.transitivelyContainsExtensions}) : upstream = _environment._allModules; - void setVariable(String name, Value value, AstNode? nodeWithSpan) { + void setVariable(String name, Value value, AstNode nodeWithSpan) { var module = _modulesByVariable[name]; if (module != null) { module.setVariable(name, value, nodeWithSpan); @@ -1037,9 +1030,7 @@ class _EnvironmentModule implements Module { } _environment._variables.first[name] = value; - if (nodeWithSpan != null) { - _environment._variableNodes?.first[name] = nodeWithSpan; - } + _environment._variableNodes.first[name] = nodeWithSpan; return; } diff --git a/lib/src/functions/color.dart b/lib/src/functions/color.dart index 40f6c654a..9b70540c9 100644 --- a/lib/src/functions/color.dart +++ b/lib/src/functions/color.dart @@ -705,6 +705,25 @@ Object /* SassString | List */ _parseChannels( String name, List argumentNames, Value channels) { if (channels.isVar) return _functionString(name, [channels]); + var originalChannels = channels; + Value? alphaFromSlashList; + if (channels.separator == ListSeparator.slash) { + var list = channels.asList; + if (list.length != 2) { + throw SassScriptException( + "Only 2 slash-separated elements allowed, but ${list.length} " + "${pluralize('was', list.length, plural: 'were')} passed."); + } + + channels = list[0]; + + alphaFromSlashList = list[1]; + if (!alphaFromSlashList.isSpecialNumber) { + alphaFromSlashList.assertNumber("alpha"); + } + if (list[0].isVar) return _functionString(name, [originalChannels]); + } + var isCommaSeparated = channels.separator == ListSeparator.comma; var isBracketed = channels.hasBrackets; if (isCommaSeparated || isBracketed) { @@ -720,18 +739,20 @@ Object /* SassString | List */ _parseChannels( var list = channels.asList; if (list.length > 3) { - throw SassScriptException( - "Only 3 elements allowed, but ${list.length} were passed."); + throw SassScriptException("Only 3 elements allowed, but ${list.length} " + "were passed."); } else if (list.length < 3) { if (list.any((value) => value.isVar) || (list.isNotEmpty && _isVarSlash(list.last))) { - return _functionString(name, [channels]); + return _functionString(name, [originalChannels]); } else { var argument = argumentNames[list.length]; throw SassScriptException("Missing element $argument."); } } + if (alphaFromSlashList != null) return [...list, alphaFromSlashList]; + var maybeSlashSeparated = list[2]; if (maybeSlashSeparated is SassNumber) { var slash = maybeSlashSeparated.asSlash; diff --git a/lib/src/functions/list.dart b/lib/src/functions/list.dart index eb8e1a462..83f85317d 100644 --- a/lib/src/functions/list.dart +++ b/lib/src/functions/list.dart @@ -20,7 +20,7 @@ final global = UnmodifiableListView([ /// The Sass list module. final module = BuiltInModule("list", functions: [ _length, _nth, _setNth, _join, _append, _zip, _index, _isBracketed, // - _separator + _separator, _slash ]); final _length = _function( @@ -61,9 +61,11 @@ final _join = _function( separator = ListSeparator.space; } else if (separatorParam.text == "comma") { separator = ListSeparator.comma; + } else if (separatorParam.text == "slash") { + separator = ListSeparator.slash; } else { throw SassScriptException( - '\$separator: Must be "space", "comma", or "auto".'); + '\$separator: Must be "space", "comma", "slash", or "auto".'); } var bracketed = bracketedParam is SassString && bracketedParam.text == 'auto' @@ -89,9 +91,11 @@ final _append = separator = ListSeparator.space; } else if (separatorParam.text == "comma") { separator = ListSeparator.comma; + } else if (separatorParam.text == "slash") { + separator = ListSeparator.slash; } else { throw SassScriptException( - '\$separator: Must be "space", "comma", or "auto".'); + '\$separator: Must be "space", "comma", "slash", or "auto".'); } var newList = [...list.asList, value]; @@ -121,16 +125,29 @@ final _index = _function("index", r"$list, $value", (arguments) { return index == -1 ? sassNull : SassNumber(index + 1); }); -final _separator = _function( - "separator", - r"$list", - (arguments) => arguments[0].separator == ListSeparator.comma - ? SassString("comma", quotes: false) - : SassString("space", quotes: false)); +final _separator = _function("separator", r"$list", (arguments) { + switch (arguments[0].separator) { + case ListSeparator.comma: + return SassString("comma", quotes: false); + case ListSeparator.slash: + return SassString("slash", quotes: false); + default: + return SassString("space", quotes: false); + } +}); final _isBracketed = _function("is-bracketed", r"$list", (arguments) => SassBoolean(arguments[0].hasBrackets)); +final _slash = _function("slash", r"$elements...", (arguments) { + var list = arguments[0].asList; + if (list.length < 2) { + throw SassScriptException("At least two elements are required."); + } + + return SassList(list, ListSeparator.slash); +}); + /// Like [new BuiltInCallable.function], but always sets the URL to `sass:list`. BuiltInCallable _function( String name, String arguments, Value callback(List arguments)) => diff --git a/lib/src/functions/math.dart b/lib/src/functions/math.dart index fbfbb4302..ff60d7636 100644 --- a/lib/src/functions/math.dart +++ b/lib/src/functions/math.dart @@ -12,6 +12,7 @@ import '../exception.dart'; import '../module/built_in.dart'; import '../util/number.dart'; import '../value.dart'; +import '../warn.dart'; /// The global definitions of Sass math functions. final global = UnmodifiableListView([ @@ -25,7 +26,7 @@ final global = UnmodifiableListView([ final module = BuiltInModule("math", functions: [ _abs, _acos, _asin, _atan, _atan2, _ceil, _clamp, _cos, _compatible, // _floor, _hypot, _isUnitless, _log, _max, _min, _percentage, _pow, // - _randomFunction, _round, _sin, _sqrt, _tan, _unit, + _randomFunction, _round, _sin, _sqrt, _tan, _unit, _div ], variables: { "e": SassNumber(math.e), "pi": SassNumber(math.pi), @@ -295,6 +296,18 @@ final _randomFunction = _function("random", r"$limit: null", (arguments) { return SassNumber(_random.nextInt(limit) + 1); }); +final _div = _function("div", r"$number1, $number2", (arguments) { + var number1 = arguments[0]; + var number2 = arguments[1]; + + if (number1 is! SassNumber || number2 is! SassNumber) { + warn("math.div() will only support number arguments in a future release.\n" + "Use list.slash() instead for a slash separator."); + } + + return number1.dividedBy(number2); +}); + /// /// Helpers /// diff --git a/lib/src/module.dart b/lib/src/module.dart index cbd2d32c5..8d570ade6 100644 --- a/lib/src/module.dart +++ b/lib/src/module.dart @@ -26,15 +26,13 @@ abstract class Module { /// The nodes where each variable in [_variables] was defined. /// - /// This is `null` if source mapping is disabled. - /// /// This stores [AstNode]s rather than [FileSpan]s so it can avoid calling /// [AstNode.span] if the span isn't required, since some nodes need to do /// real work to manufacture a source span. /// /// Implementations must ensure that this has the same keys as [variables] if /// it's not `null`. - Map? get variableNodes; + Map get variableNodes; /// The module's functions. /// @@ -71,7 +69,7 @@ abstract class Module { /// /// Throws a [SassScriptException] if this module doesn't define a variable /// named [name]. - void setVariable(String name, Value value, AstNode? nodeWithSpan); + void setVariable(String name, Value value, AstNode nodeWithSpan); /// Returns an opaque object that will be equal to another /// `variableIdentity()` return value for the same name in another module if diff --git a/lib/src/module/built_in.dart b/lib/src/module/built_in.dart index ae1d2d86b..1c5a56ed1 100644 --- a/lib/src/module/built_in.dart +++ b/lib/src/module/built_in.dart @@ -44,7 +44,7 @@ class BuiltInModule implements Module { : UnmodifiableMapView( {for (var callable in callables) callable.name: callable})); - void setVariable(String name, Value value, AstNode? nodeWithSpan) { + void setVariable(String name, Value value, AstNode nodeWithSpan) { if (!variables.containsKey(name)) { throw SassScriptException("Undefined variable."); } diff --git a/lib/src/module/forwarded_view.dart b/lib/src/module/forwarded_view.dart index 14874e82a..4540702a9 100644 --- a/lib/src/module/forwarded_view.dart +++ b/lib/src/module/forwarded_view.dart @@ -10,7 +10,6 @@ import '../exception.dart'; import '../extend/extension_store.dart'; import '../module.dart'; import '../util/limited_map_view.dart'; -import '../util/nullable.dart'; import '../util/prefixed_map_view.dart'; import '../value.dart'; @@ -31,7 +30,7 @@ class ForwardedModuleView implements Module { _inner.transitivelyContainsExtensions; final Map variables; - final Map? variableNodes; + final Map variableNodes; final Map functions; final Map mixins; @@ -53,8 +52,8 @@ class ForwardedModuleView implements Module { ForwardedModuleView(this._inner, this._rule) : variables = _forwardedMap(_inner.variables, _rule.prefix, _rule.shownVariables, _rule.hiddenVariables), - variableNodes = _inner.variableNodes.andThen((inner) => _forwardedMap( - inner, _rule.prefix, _rule.shownVariables, _rule.hiddenVariables)), + variableNodes = _forwardedMap(_inner.variableNodes, _rule.prefix, + _rule.shownVariables, _rule.hiddenVariables), functions = _forwardedMap(_inner.functions, _rule.prefix, _rule.shownMixinsAndFunctions, _rule.hiddenMixinsAndFunctions), mixins = _forwardedMap(_inner.mixins, _rule.prefix, @@ -86,7 +85,7 @@ class ForwardedModuleView implements Module { return map; } - void setVariable(String name, Value value, AstNode? nodeWithSpan) { + void setVariable(String name, Value value, AstNode nodeWithSpan) { var shownVariables = _rule.shownVariables; var hiddenVariables = _rule.hiddenVariables; if (shownVariables != null && !shownVariables.contains(name)) { diff --git a/lib/src/module/shadowed_view.dart b/lib/src/module/shadowed_view.dart index f9f5516e4..cee4ef0d7 100644 --- a/lib/src/module/shadowed_view.dart +++ b/lib/src/module/shadowed_view.dart @@ -9,7 +9,6 @@ import '../exception.dart'; import '../extend/extension_store.dart'; import '../module.dart'; import '../util/limited_map_view.dart'; -import '../util/nullable.dart'; import '../utils.dart'; import '../value.dart'; @@ -28,7 +27,7 @@ class ShadowedModuleView implements Module { _inner.transitivelyContainsExtensions; final Map variables; - final Map? variableNodes; + final Map variableNodes; final Map functions; final Map mixins; @@ -57,8 +56,7 @@ class ShadowedModuleView implements Module { ShadowedModuleView(this._inner, {Set? variables, Set? functions, Set? mixins}) : variables = _shadowedMap(_inner.variables, variables), - variableNodes = - _inner.variableNodes.andThen((map) => _shadowedMap(map, variables)), + variableNodes = _shadowedMap(_inner.variableNodes, variables), functions = _shadowedMap(_inner.functions, functions), mixins = _shadowedMap(_inner.mixins, mixins); @@ -77,7 +75,7 @@ class ShadowedModuleView implements Module { Map map, Set? blocklist) => blocklist != null && map.isNotEmpty && blocklist.any(map.containsKey); - void setVariable(String name, Value value, AstNode? nodeWithSpan) { + void setVariable(String name, Value value, AstNode nodeWithSpan) { if (!variables.containsKey(name)) { throw SassScriptException("Undefined variable."); } else { diff --git a/lib/src/value.dart b/lib/src/value.dart index cff8293ce..2e5759700 100644 --- a/lib/src/value.dart +++ b/lib/src/value.dart @@ -195,27 +195,32 @@ abstract class Value implements ext.Value { if (list.asList.isEmpty) return null; var result = []; - if (list.separator == ListSeparator.comma) { - for (var complex in list.asList) { - if (complex is SassString) { - result.add(complex.text); - } else if (complex is SassList && - complex.separator == ListSeparator.space) { - var string = complex._selectorStringOrNull(); - if (string == null) return null; - result.add(string); - } else { - return null; + switch (list.separator) { + case ListSeparator.comma: + for (var complex in list.asList) { + if (complex is SassString) { + result.add(complex.text); + } else if (complex is SassList && + complex.separator == ListSeparator.space) { + var string = complex._selectorStringOrNull(); + if (string == null) return null; + result.add(string); + } else { + return null; + } } - } - } else { - for (var compound in list.asList) { - if (compound is SassString) { - result.add(compound.text); - } else { - return null; + break; + case ListSeparator.slash: + return null; + default: + for (var compound in list.asList) { + if (compound is SassString) { + result.add(compound.text); + } else { + return null; + } } - } + break; } return result.join(list.separator == ListSeparator.comma ? ', ' : ' '); } diff --git a/lib/src/value/list.dart b/lib/src/value/list.dart index 79bc1feb3..7ed8124e2 100644 --- a/lib/src/value/list.dart +++ b/lib/src/value/list.dart @@ -67,6 +67,9 @@ class ListSeparator { /// A comma-separated list. static const comma = ListSeparator._("comma", ","); + /// A slash-separated list. + static const slash = ListSeparator._("slash", "/"); + /// A separator that hasn't yet been determined. /// /// Singleton lists and empty lists don't have separators defined. This means diff --git a/lib/src/visitor/async_evaluate.dart b/lib/src/visitor/async_evaluate.dart index 1c7fe64d5..4f60ef35f 100644 --- a/lib/src/visitor/async_evaluate.dart +++ b/lib/src/visitor/async_evaluate.dart @@ -298,7 +298,7 @@ class _EvaluateVisitor _sourceMap = sourceMap, // The default environment is overridden in [_execute] for full // stylesheets, but for [AsyncEvaluator] this environment is used. - _environment = AsyncEnvironment(sourceMap: sourceMap) { + _environment = AsyncEnvironment() { var metaFunctions = [ // These functions are defined in the context of the evaluator because // they need access to the [_environment] or other local state. @@ -677,7 +677,7 @@ class _EvaluateVisitor return alreadyLoaded; } - var environment = AsyncEnvironment(sourceMap: _sourceMap); + var environment = AsyncEnvironment(); late CssStylesheet css; var extensionStore = ExtensionStore(); await _withEnvironment(environment, () async { @@ -1135,8 +1135,8 @@ class _EvaluateVisitor var list = await node.list.accept(this); var nodeWithSpan = _expressionNode(node.list); var setVariables = node.variables.length == 1 - ? (Value value) => _environment.setLocalVariable( - node.variables.first, value.withoutSlash(), nodeWithSpan) + ? (Value value) => _environment.setLocalVariable(node.variables.first, + _withoutSlash(value, nodeWithSpan), nodeWithSpan) : (Value value) => _setMultipleVariables(node.variables, value, nodeWithSpan); return _environment.scope(() { @@ -1156,7 +1156,7 @@ class _EvaluateVisitor var minLength = math.min(variables.length, list.length); for (var i = 0; i < minLength; i++) { _environment.setLocalVariable( - variables[i], list[i].withoutSlash(), nodeWithSpan); + variables[i], _withoutSlash(list[i], nodeWithSpan), nodeWithSpan); } for (var i = minLength; i < variables.length; i++) { _environment.setLocalVariable(variables[i], sassNull, nodeWithSpan); @@ -1346,10 +1346,12 @@ class _EvaluateVisitor } } + var variableNodeWithSpan = _expressionNode(variable.expression); newValues[variable.name] = ConfiguredValue.explicit( - (await variable.expression.accept(this)).withoutSlash(), + _withoutSlash( + await variable.expression.accept(this), variableNodeWithSpan), variable.span, - _expressionNode(variable.expression)); + variableNodeWithSpan); } if (configuration is ExplicitConfiguration || configuration.isEmpty) { @@ -1761,8 +1763,8 @@ class _EvaluateVisitor return queries; } - Future visitReturnRule(ReturnRule node) => - node.expression.accept(this); + Future visitReturnRule(ReturnRule node) async => + _withoutSlash(await node.expression.accept(this), node.expression); Future visitSilentComment(SilentComment node) async => null; @@ -1949,7 +1951,8 @@ class _EvaluateVisitor deprecation: true); } - var value = (await node.expression.accept(this)).withoutSlash(); + var value = + _withoutSlash(await node.expression.accept(this), node.expression); _addExceptionSpan(node, () { _environment.setVariable( node.name, value, _expressionNode(node.expression), @@ -1959,15 +1962,19 @@ class _EvaluateVisitor } Future visitUseRule(UseRule node) async { - var configuration = node.configuration.isEmpty - ? const Configuration.empty() - : ExplicitConfiguration({ - for (var variable in node.configuration) - variable.name: ConfiguredValue.explicit( - (await variable.expression.accept(this)).withoutSlash(), - variable.span, - _expressionNode(variable.expression)) - }, node); + var configuration = const Configuration.empty(); + if (node.configuration.isNotEmpty) { + var values = {}; + for (var variable in node.configuration) { + var variableNodeWithSpan = _expressionNode(variable.expression); + values[variable.name] = ConfiguredValue.explicit( + _withoutSlash( + await variable.expression.accept(this), variableNodeWithSpan), + variable.span, + variableNodeWithSpan); + } + configuration = ExplicitConfiguration(values, node); + } await _loadModule(node.url, "@use", node, (module) { _environment.addModule(module, node, namespace: node.namespace); @@ -2055,6 +2062,29 @@ class _EvaluateVisitor if (node.allowsSlash && left is SassNumber && right is SassNumber) { return (result as SassNumber).withSlash(left, right); } else { + if (left is SassNumber && right is SassNumber) { + String recommendation(Expression expression) { + if (expression is BinaryOperationExpression && + expression.operator == BinaryOperator.dividedBy) { + return "math.div(${recommendation(expression.left)}, " + "${recommendation(expression.right)})"; + } else { + return expression.toString(); + } + } + + _warn( + "Using / for division is deprecated and will be removed in " + "Dart Sass 2.0.0.\n" + "\n" + "Recommendation: ${recommendation(node)}\n" + "\n" + "More info and automated migrator: " + "https://sass-lang.com/d/slash-div", + node.span, + deprecation: true); + } + return result; } @@ -2109,8 +2139,8 @@ class _EvaluateVisitor var ifTrue = positional.length > 1 ? positional[1] : named["if-true"]!; var ifFalse = positional.length > 2 ? positional[2] : named["if-false"]!; - return await ((await condition.accept(this)).isTruthy ? ifTrue : ifFalse) - .accept(this); + var result = (await condition.accept(this)).isTruthy ? ifTrue : ifFalse; + return _withoutSlash(await result.accept(this), _expressionNode(result)); } Future visitNullExpression(NullExpression node) async => sassNull; @@ -2199,6 +2229,8 @@ class _EvaluateVisitor UserDefinedCallable callable, AstNode nodeWithSpan, Future run()) async { + // TODO(nweiz): Set [trackSpans] to `null` once we're no longer emitting + // deprecation warnings for /-as-division. var evaluated = await _evaluateArguments(arguments); var name = callable.name; @@ -2216,10 +2248,8 @@ class _EvaluateVisitor var minLength = math.min(evaluated.positional.length, declaredArguments.length); for (var i = 0; i < minLength; i++) { - _environment.setLocalVariable( - declaredArguments[i].name, - evaluated.positional[i].withoutSlash(), - evaluated.positionalNodes?[i]); + _environment.setLocalVariable(declaredArguments[i].name, + evaluated.positional[i], evaluated.positionalNodes[i]); } for (var i = evaluated.positional.length; @@ -2227,12 +2257,14 @@ class _EvaluateVisitor i++) { var argument = declaredArguments[i]; var value = evaluated.named.remove(argument.name) ?? - await argument.defaultValue!.accept>(this); + _withoutSlash( + await argument.defaultValue!.accept>(this), + _expressionNode(argument.defaultValue!)); _environment.setLocalVariable( argument.name, - value.withoutSlash(), - evaluated.namedNodes?[argument.name] ?? - argument.defaultValue.andThen(_expressionNode)); + value, + evaluated.namedNodes[argument.name] ?? + _expressionNode(argument.defaultValue!)); } SassArgumentList? argumentList; @@ -2275,11 +2307,12 @@ class _EvaluateVisitor Future _runFunctionCallable(ArgumentInvocation arguments, AsyncCallable? callable, AstNode nodeWithSpan) async { if (callable is AsyncBuiltInCallable) { - return (await _runBuiltInCallable(arguments, callable, nodeWithSpan)) - .withoutSlash(); + return _withoutSlash( + await _runBuiltInCallable(arguments, callable, nodeWithSpan), + nodeWithSpan); } else if (callable is UserDefinedCallable) { - return (await _runUserDefinedCallable(arguments, callable, nodeWithSpan, - () async { + return await _runUserDefinedCallable(arguments, callable, nodeWithSpan, + () async { for (var statement in callable.declaration.children) { var returnValue = await statement.accept(this); if (returnValue is Value) return returnValue; @@ -2287,8 +2320,7 @@ class _EvaluateVisitor throw _exception( "Function finished without @return.", callable.declaration.span); - })) - .withoutSlash(); + }); } else if (callable is PlainCssCallable) { if (arguments.named.isNotEmpty || arguments.keywordRest != null) { throw _exception("Plain CSS functions don't support keyword arguments.", @@ -2325,7 +2357,7 @@ class _EvaluateVisitor /// body. Future _runBuiltInCallable(ArgumentInvocation arguments, AsyncBuiltInCallable callable, AstNode nodeWithSpan) async { - var evaluated = await _evaluateArguments(arguments, trackSpans: false); + var evaluated = await _evaluateArguments(arguments); var oldCallableNode = _callableNode; _callableNode = nodeWithSpan; @@ -2343,7 +2375,8 @@ class _EvaluateVisitor i++) { var argument = declaredArguments[i]; evaluated.positional.add(evaluated.named.remove(argument.name) ?? - await argument.defaultValue!.accept(this)); + _withoutSlash(await argument.defaultValue!.accept(this), + argument.defaultValue!)); } SassArgumentList? argumentList; @@ -2405,38 +2438,35 @@ class _EvaluateVisitor } /// Returns the evaluated values of the given [arguments]. - /// - /// If [trackSpans] is `true`, this tracks the source spans of the arguments - /// being passed in. It defaults to [_sourceMap]. - Future<_ArgumentResults> _evaluateArguments(ArgumentInvocation arguments, - {bool? trackSpans}) async { - trackSpans ??= _sourceMap; - - var positional = [ - for (var expression in arguments.positional) await expression.accept(this) - ]; - var named = { - for (var entry in arguments.named.entries) - entry.key: await entry.value.accept(this) - }; - - var positionalNodes = trackSpans - ? [ - for (var expression in arguments.positional) - _expressionNode(expression) - ] - : null; - var namedNodes = trackSpans - ? { - for (var entry in arguments.named.entries) - entry.key: _expressionNode(entry.value) - } - : null; + Future<_ArgumentResults> _evaluateArguments( + ArgumentInvocation arguments) async { + // TODO(nweiz): This used to avoid tracking source spans for arguments if + // [_sourceMap]s was false or it was being called from + // [_runBuiltInCallable]. We always have to track them now to produce better + // warnings for /-as-division, but once those warnings are gone we should go + // back to tracking conditionally. + + var positional = []; + var positionalNodes = []; + for (var expression in arguments.positional) { + var nodeForSpan = _expressionNode(expression); + positional.add(_withoutSlash(await expression.accept(this), nodeForSpan)); + positionalNodes.add(nodeForSpan); + } + + var named = {}; + var namedNodes = {}; + for (var entry in arguments.named.entries) { + var nodeForSpan = _expressionNode(entry.value); + named[entry.key] = + _withoutSlash(await entry.value.accept(this), nodeForSpan); + namedNodes[entry.key] = nodeForSpan; + } var restArgs = arguments.rest; if (restArgs == null) { - return _ArgumentResults(positional, named, ListSeparator.undecided, - positionalNodes: positionalNodes, namedNodes: namedNodes); + return _ArgumentResults(positional, positionalNodes, named, namedNodes, + ListSeparator.undecided); } var rest = await restArgs.accept(this); @@ -2444,42 +2474,43 @@ class _EvaluateVisitor var separator = ListSeparator.undecided; if (rest is SassMap) { _addRestMap(named, rest, restArgs, (value) => value); - namedNodes?.addAll({ + namedNodes.addAll({ for (var key in rest.contents.keys) (key as SassString).text: restNodeForSpan }); } else if (rest is SassList) { - positional.addAll(rest.asList); - positionalNodes?.addAll(List.filled(rest.lengthAsList, restNodeForSpan)); + positional.addAll( + rest.asList.map((value) => _withoutSlash(value, restNodeForSpan))); + positionalNodes.addAll(List.filled(rest.lengthAsList, restNodeForSpan)); separator = rest.separator; if (rest is SassArgumentList) { rest.keywords.forEach((key, value) { - named[key] = value; - if (namedNodes != null) namedNodes[key] = restNodeForSpan; + named[key] = _withoutSlash(value, restNodeForSpan); + namedNodes[key] = restNodeForSpan; }); } } else { - positional.add(rest); - positionalNodes?.add(restNodeForSpan); + positional.add(_withoutSlash(rest, restNodeForSpan)); + positionalNodes.add(restNodeForSpan); } var keywordRestArgs = arguments.keywordRest; if (keywordRestArgs == null) { - return _ArgumentResults(positional, named, separator, - positionalNodes: positionalNodes, namedNodes: namedNodes); + return _ArgumentResults( + positional, positionalNodes, named, namedNodes, separator); } var keywordRest = await keywordRestArgs.accept(this); var keywordRestNodeForSpan = _expressionNode(keywordRestArgs); if (keywordRest is SassMap) { _addRestMap(named, keywordRest, keywordRestArgs, (value) => value); - namedNodes?.addAll({ + namedNodes.addAll({ for (var key in keywordRest.contents.keys) (key as SassString).text: keywordRestNodeForSpan }); - return _ArgumentResults(positional, named, separator, - positionalNodes: positionalNodes, namedNodes: namedNodes); + return _ArgumentResults( + positional, positionalNodes, named, namedNodes, separator); } else { throw _exception( "Variable keyword arguments must be a map (was $keywordRest).", @@ -2504,19 +2535,22 @@ class _EvaluateVisitor var positional = invocation.arguments.positional.toList(); var named = Map.of(invocation.arguments.named); var rest = await restArgs.accept(this); + var restNodeForSpan = _expressionNode(restArgs); if (rest is SassMap) { _addRestMap(named, rest, invocation, (value) => ValueExpression(value, restArgs.span)); } else if (rest is SassList) { - positional.addAll( - rest.asList.map((value) => ValueExpression(value, restArgs.span))); + positional.addAll(rest.asList.map((value) => ValueExpression( + _withoutSlash(value, restNodeForSpan), restArgs.span))); if (rest is SassArgumentList) { rest.keywords.forEach((key, value) { - named[key] = ValueExpression(value, restArgs.span); + named[key] = ValueExpression( + _withoutSlash(value, restNodeForSpan), restArgs.span); }); } } else { - positional.add(ValueExpression(rest, restArgs.span)); + positional.add( + ValueExpression(_withoutSlash(rest, restNodeForSpan), restArgs.span)); } var keywordRestArgs_ = invocation.arguments.keywordRest; @@ -2524,9 +2558,15 @@ class _EvaluateVisitor var keywordRestArgs = keywordRestArgs_; // dart-lang/sdk#45348 var keywordRest = await keywordRestArgs.accept(this); + var keywordRestNodeForSpan = _expressionNode(keywordRestArgs); if (keywordRest is SassMap) { - _addRestMap(named, keywordRest, invocation, - (value) => ValueExpression(value, keywordRestArgs.span)); + _addRestMap( + named, + keywordRest, + invocation, + (value) => ValueExpression( + _withoutSlash(value, keywordRestNodeForSpan), + keywordRestArgs.span)); return Tuple2(positional, named); } else { throw _exception( @@ -2548,9 +2588,10 @@ class _EvaluateVisitor /// real work to manufacture a source span. void _addRestMap(Map values, SassMap map, AstNode nodeWithSpan, T convert(Value value)) { + var expressionNode = _expressionNode(nodeWithSpan); map.contents.forEach((key, value) { if (key is SassString) { - values[key.text] = convert(value); + values[key.text] = convert(_withoutSlash(value, expressionNode)); } else { throw _exception( "Variable keyword argument map must have string keys.\n" @@ -2891,15 +2932,17 @@ class _EvaluateVisitor /// This returns an [AstNode] rather than a [FileSpan] so we can avoid calling /// [AstNode.span] if the span isn't required, since some nodes need to do /// real work to manufacture a source span. - AstNode _expressionNode(Expression expression) { - // If we aren't making a source map this doesn't matter, but we still return - // the expression so we don't have to make the type (and everything - // downstream of it) nullable. - if (!_sourceMap) return expression; + AstNode _expressionNode(AstNode expression) { + // TODO(nweiz): This used to return [expression] as-is if source map + // generation was disabled. We always have to track the original location + // now to produce better warnings for /-as-division, but once those warnings + // are gone we should go back to short-circuiting. if (expression is VariableExpression) { - return _environment.getVariableNode(expression.name, - namespace: expression.namespace) ?? + return _addExceptionSpan( + expression, + () => _environment.getVariableNode(expression.name, + namespace: expression.namespace)) ?? expression; } else { return expression; @@ -2998,6 +3041,35 @@ class _EvaluateVisitor return result; } + /// Like [Value.withoutSlash], but produces a deprecation warning if [value] + /// was a slash-separated number. + Value _withoutSlash(Value value, AstNode nodeForSpan) { + if (value is SassNumber && value.asSlash != null) { + String recommendation(SassNumber number) { + var asSlash = number.asSlash; + if (asSlash != null) { + return "math.div(${recommendation(asSlash.item1)}, " + "${recommendation(asSlash.item2)})"; + } else { + return number.toString(); + } + } + + _warn( + "Using / for division is deprecated and will be removed in Dart Sass " + "2.0.0.\n" + "\n" + "Recommendation: ${recommendation(value)}\n" + "\n" + "More info and automated migrator: " + "https://sass-lang.com/d/slash-div", + nodeForSpan.span, + deprecation: true); + } + + return value.withoutSlash(); + } + /// Creates a new stack frame with location information from [member] and /// [span]. Frame _stackFrame(String member, FileSpan span) => frameForSpan(span, member, @@ -3205,28 +3277,26 @@ class _ArgumentResults { /// Arguments passed by position. final List positional; - /// The [AstNode]s that hold the spans for each [positional] argument, or - /// `null` if source span tracking is disabled. + /// The [AstNode]s that hold the spans for each [positional] argument. /// /// This stores [AstNode]s rather than [FileSpan]s so it can avoid calling /// [AstNode.span] if the span isn't required, since some nodes need to do /// real work to manufacture a source span. - final List? positionalNodes; + final List positionalNodes; /// Arguments passed by name. final Map named; - /// The [AstNode]s that hold the spans for each [named] argument, or `null` if - /// source span tracking is disabled. + /// The [AstNode]s that hold the spans for each [named] argument. /// /// This stores [AstNode]s rather than [FileSpan]s so it can avoid calling /// [AstNode.span] if the span isn't required, since some nodes need to do /// real work to manufacture a source span. - final Map? namedNodes; + final Map namedNodes; /// The separator used for the rest argument list, if any. final ListSeparator separator; - _ArgumentResults(this.positional, this.named, this.separator, - {this.positionalNodes, this.namedNodes}); + _ArgumentResults(this.positional, this.positionalNodes, this.named, + this.namedNodes, this.separator); } diff --git a/lib/src/visitor/evaluate.dart b/lib/src/visitor/evaluate.dart index fe2618329..c478d374f 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: 648f9a2041613a8f11c46986b4b4d4ccbaa0c489 +// Checksum: 1fc6b9e6018eba3ac520464abb56e686f4cb9886 // // ignore_for_file: unused_import @@ -306,7 +306,7 @@ class _EvaluateVisitor _sourceMap = sourceMap, // The default environment is overridden in [_execute] for full // stylesheets, but for [AsyncEvaluator] this environment is used. - _environment = Environment(sourceMap: sourceMap) { + _environment = Environment() { var metaFunctions = [ // These functions are defined in the context of the evaluator because // they need access to the [_environment] or other local state. @@ -682,7 +682,7 @@ class _EvaluateVisitor return alreadyLoaded; } - var environment = Environment(sourceMap: _sourceMap); + var environment = Environment(); late CssStylesheet css; var extensionStore = ExtensionStore(); _withEnvironment(environment, () { @@ -1139,8 +1139,8 @@ class _EvaluateVisitor var list = node.list.accept(this); var nodeWithSpan = _expressionNode(node.list); var setVariables = node.variables.length == 1 - ? (Value value) => _environment.setLocalVariable( - node.variables.first, value.withoutSlash(), nodeWithSpan) + ? (Value value) => _environment.setLocalVariable(node.variables.first, + _withoutSlash(value, nodeWithSpan), nodeWithSpan) : (Value value) => _setMultipleVariables(node.variables, value, nodeWithSpan); return _environment.scope(() { @@ -1160,7 +1160,7 @@ class _EvaluateVisitor var minLength = math.min(variables.length, list.length); for (var i = 0; i < minLength; i++) { _environment.setLocalVariable( - variables[i], list[i].withoutSlash(), nodeWithSpan); + variables[i], _withoutSlash(list[i], nodeWithSpan), nodeWithSpan); } for (var i = minLength; i < variables.length; i++) { _environment.setLocalVariable(variables[i], sassNull, nodeWithSpan); @@ -1347,10 +1347,11 @@ class _EvaluateVisitor } } + var variableNodeWithSpan = _expressionNode(variable.expression); newValues[variable.name] = ConfiguredValue.explicit( - variable.expression.accept(this).withoutSlash(), + _withoutSlash(variable.expression.accept(this), variableNodeWithSpan), variable.span, - _expressionNode(variable.expression)); + variableNodeWithSpan); } if (configuration is ExplicitConfiguration || configuration.isEmpty) { @@ -1756,7 +1757,8 @@ class _EvaluateVisitor return queries; } - Value visitReturnRule(ReturnRule node) => node.expression.accept(this); + Value visitReturnRule(ReturnRule node) => + _withoutSlash(node.expression.accept(this), node.expression); Value? visitSilentComment(SilentComment node) => null; @@ -1941,7 +1943,7 @@ class _EvaluateVisitor deprecation: true); } - var value = node.expression.accept(this).withoutSlash(); + var value = _withoutSlash(node.expression.accept(this), node.expression); _addExceptionSpan(node, () { _environment.setVariable( node.name, value, _expressionNode(node.expression), @@ -1951,15 +1953,19 @@ class _EvaluateVisitor } Value? visitUseRule(UseRule node) { - var configuration = node.configuration.isEmpty - ? const Configuration.empty() - : ExplicitConfiguration({ - for (var variable in node.configuration) - variable.name: ConfiguredValue.explicit( - variable.expression.accept(this).withoutSlash(), - variable.span, - _expressionNode(variable.expression)) - }, node); + var configuration = const Configuration.empty(); + if (node.configuration.isNotEmpty) { + var values = {}; + for (var variable in node.configuration) { + var variableNodeWithSpan = _expressionNode(variable.expression); + values[variable.name] = ConfiguredValue.explicit( + _withoutSlash( + variable.expression.accept(this), variableNodeWithSpan), + variable.span, + variableNodeWithSpan); + } + configuration = ExplicitConfiguration(values, node); + } _loadModule(node.url, "@use", node, (module) { _environment.addModule(module, node, namespace: node.namespace); @@ -2046,6 +2052,29 @@ class _EvaluateVisitor if (node.allowsSlash && left is SassNumber && right is SassNumber) { return (result as SassNumber).withSlash(left, right); } else { + if (left is SassNumber && right is SassNumber) { + String recommendation(Expression expression) { + if (expression is BinaryOperationExpression && + expression.operator == BinaryOperator.dividedBy) { + return "math.div(${recommendation(expression.left)}, " + "${recommendation(expression.right)})"; + } else { + return expression.toString(); + } + } + + _warn( + "Using / for division is deprecated and will be removed in " + "Dart Sass 2.0.0.\n" + "\n" + "Recommendation: ${recommendation(node)}\n" + "\n" + "More info and automated migrator: " + "https://sass-lang.com/d/slash-div", + node.span, + deprecation: true); + } + return result; } @@ -2099,7 +2128,8 @@ class _EvaluateVisitor var ifTrue = positional.length > 1 ? positional[1] : named["if-true"]!; var ifFalse = positional.length > 2 ? positional[2] : named["if-false"]!; - return (condition.accept(this).isTruthy ? ifTrue : ifFalse).accept(this); + var result = condition.accept(this).isTruthy ? ifTrue : ifFalse; + return _withoutSlash(result.accept(this), _expressionNode(result)); } SassNull visitNullExpression(NullExpression node) => sassNull; @@ -2186,6 +2216,8 @@ class _EvaluateVisitor UserDefinedCallable callable, AstNode nodeWithSpan, V run()) { + // TODO(nweiz): Set [trackSpans] to `null` once we're no longer emitting + // deprecation warnings for /-as-division. var evaluated = _evaluateArguments(arguments); var name = callable.name; @@ -2203,10 +2235,8 @@ class _EvaluateVisitor var minLength = math.min(evaluated.positional.length, declaredArguments.length); for (var i = 0; i < minLength; i++) { - _environment.setLocalVariable( - declaredArguments[i].name, - evaluated.positional[i].withoutSlash(), - evaluated.positionalNodes?[i]); + _environment.setLocalVariable(declaredArguments[i].name, + evaluated.positional[i], evaluated.positionalNodes[i]); } for (var i = evaluated.positional.length; @@ -2214,12 +2244,13 @@ class _EvaluateVisitor i++) { var argument = declaredArguments[i]; var value = evaluated.named.remove(argument.name) ?? - argument.defaultValue!.accept(this); + _withoutSlash(argument.defaultValue!.accept(this), + _expressionNode(argument.defaultValue!)); _environment.setLocalVariable( argument.name, - value.withoutSlash(), - evaluated.namedNodes?[argument.name] ?? - argument.defaultValue.andThen(_expressionNode)); + value, + evaluated.namedNodes[argument.name] ?? + _expressionNode(argument.defaultValue!)); } SassArgumentList? argumentList; @@ -2262,8 +2293,8 @@ class _EvaluateVisitor Value _runFunctionCallable( ArgumentInvocation arguments, Callable? callable, AstNode nodeWithSpan) { if (callable is BuiltInCallable) { - return _runBuiltInCallable(arguments, callable, nodeWithSpan) - .withoutSlash(); + return _withoutSlash( + _runBuiltInCallable(arguments, callable, nodeWithSpan), nodeWithSpan); } else if (callable is UserDefinedCallable) { return _runUserDefinedCallable(arguments, callable, nodeWithSpan, () { for (var statement in callable.declaration.children) { @@ -2273,7 +2304,7 @@ class _EvaluateVisitor throw _exception( "Function finished without @return.", callable.declaration.span); - }).withoutSlash(); + }); } else if (callable is PlainCssCallable) { if (arguments.named.isNotEmpty || arguments.keywordRest != null) { throw _exception("Plain CSS functions don't support keyword arguments.", @@ -2310,7 +2341,7 @@ class _EvaluateVisitor /// body. Value _runBuiltInCallable(ArgumentInvocation arguments, BuiltInCallable callable, AstNode nodeWithSpan) { - var evaluated = _evaluateArguments(arguments, trackSpans: false); + var evaluated = _evaluateArguments(arguments); var oldCallableNode = _callableNode; _callableNode = nodeWithSpan; @@ -2328,7 +2359,8 @@ class _EvaluateVisitor i++) { var argument = declaredArguments[i]; evaluated.positional.add(evaluated.named.remove(argument.name) ?? - argument.defaultValue!.accept(this)); + _withoutSlash( + argument.defaultValue!.accept(this), argument.defaultValue!)); } SassArgumentList? argumentList; @@ -2390,38 +2422,33 @@ class _EvaluateVisitor } /// Returns the evaluated values of the given [arguments]. - /// - /// If [trackSpans] is `true`, this tracks the source spans of the arguments - /// being passed in. It defaults to [_sourceMap]. - _ArgumentResults _evaluateArguments(ArgumentInvocation arguments, - {bool? trackSpans}) { - trackSpans ??= _sourceMap; - - var positional = [ - for (var expression in arguments.positional) expression.accept(this) - ]; - var named = { - for (var entry in arguments.named.entries) - entry.key: entry.value.accept(this) - }; + _ArgumentResults _evaluateArguments(ArgumentInvocation arguments) { + // TODO(nweiz): This used to avoid tracking source spans for arguments if + // [_sourceMap]s was false or it was being called from + // [_runBuiltInCallable]. We always have to track them now to produce better + // warnings for /-as-division, but once those warnings are gone we should go + // back to tracking conditionally. - var positionalNodes = trackSpans - ? [ - for (var expression in arguments.positional) - _expressionNode(expression) - ] - : null; - var namedNodes = trackSpans - ? { - for (var entry in arguments.named.entries) - entry.key: _expressionNode(entry.value) - } - : null; + var positional = []; + var positionalNodes = []; + for (var expression in arguments.positional) { + var nodeForSpan = _expressionNode(expression); + positional.add(_withoutSlash(expression.accept(this), nodeForSpan)); + positionalNodes.add(nodeForSpan); + } + + var named = {}; + var namedNodes = {}; + for (var entry in arguments.named.entries) { + var nodeForSpan = _expressionNode(entry.value); + named[entry.key] = _withoutSlash(entry.value.accept(this), nodeForSpan); + namedNodes[entry.key] = nodeForSpan; + } var restArgs = arguments.rest; if (restArgs == null) { - return _ArgumentResults(positional, named, ListSeparator.undecided, - positionalNodes: positionalNodes, namedNodes: namedNodes); + return _ArgumentResults(positional, positionalNodes, named, namedNodes, + ListSeparator.undecided); } var rest = restArgs.accept(this); @@ -2429,42 +2456,43 @@ class _EvaluateVisitor var separator = ListSeparator.undecided; if (rest is SassMap) { _addRestMap(named, rest, restArgs, (value) => value); - namedNodes?.addAll({ + namedNodes.addAll({ for (var key in rest.contents.keys) (key as SassString).text: restNodeForSpan }); } else if (rest is SassList) { - positional.addAll(rest.asList); - positionalNodes?.addAll(List.filled(rest.lengthAsList, restNodeForSpan)); + positional.addAll( + rest.asList.map((value) => _withoutSlash(value, restNodeForSpan))); + positionalNodes.addAll(List.filled(rest.lengthAsList, restNodeForSpan)); separator = rest.separator; if (rest is SassArgumentList) { rest.keywords.forEach((key, value) { - named[key] = value; - if (namedNodes != null) namedNodes[key] = restNodeForSpan; + named[key] = _withoutSlash(value, restNodeForSpan); + namedNodes[key] = restNodeForSpan; }); } } else { - positional.add(rest); - positionalNodes?.add(restNodeForSpan); + positional.add(_withoutSlash(rest, restNodeForSpan)); + positionalNodes.add(restNodeForSpan); } var keywordRestArgs = arguments.keywordRest; if (keywordRestArgs == null) { - return _ArgumentResults(positional, named, separator, - positionalNodes: positionalNodes, namedNodes: namedNodes); + return _ArgumentResults( + positional, positionalNodes, named, namedNodes, separator); } var keywordRest = keywordRestArgs.accept(this); var keywordRestNodeForSpan = _expressionNode(keywordRestArgs); if (keywordRest is SassMap) { _addRestMap(named, keywordRest, keywordRestArgs, (value) => value); - namedNodes?.addAll({ + namedNodes.addAll({ for (var key in keywordRest.contents.keys) (key as SassString).text: keywordRestNodeForSpan }); - return _ArgumentResults(positional, named, separator, - positionalNodes: positionalNodes, namedNodes: namedNodes); + return _ArgumentResults( + positional, positionalNodes, named, namedNodes, separator); } else { throw _exception( "Variable keyword arguments must be a map (was $keywordRest).", @@ -2489,19 +2517,22 @@ class _EvaluateVisitor var positional = invocation.arguments.positional.toList(); var named = Map.of(invocation.arguments.named); var rest = restArgs.accept(this); + var restNodeForSpan = _expressionNode(restArgs); if (rest is SassMap) { _addRestMap(named, rest, invocation, (value) => ValueExpression(value, restArgs.span)); } else if (rest is SassList) { - positional.addAll( - rest.asList.map((value) => ValueExpression(value, restArgs.span))); + positional.addAll(rest.asList.map((value) => ValueExpression( + _withoutSlash(value, restNodeForSpan), restArgs.span))); if (rest is SassArgumentList) { rest.keywords.forEach((key, value) { - named[key] = ValueExpression(value, restArgs.span); + named[key] = ValueExpression( + _withoutSlash(value, restNodeForSpan), restArgs.span); }); } } else { - positional.add(ValueExpression(rest, restArgs.span)); + positional.add( + ValueExpression(_withoutSlash(rest, restNodeForSpan), restArgs.span)); } var keywordRestArgs_ = invocation.arguments.keywordRest; @@ -2509,9 +2540,15 @@ class _EvaluateVisitor var keywordRestArgs = keywordRestArgs_; // dart-lang/sdk#45348 var keywordRest = keywordRestArgs.accept(this); + var keywordRestNodeForSpan = _expressionNode(keywordRestArgs); if (keywordRest is SassMap) { - _addRestMap(named, keywordRest, invocation, - (value) => ValueExpression(value, keywordRestArgs.span)); + _addRestMap( + named, + keywordRest, + invocation, + (value) => ValueExpression( + _withoutSlash(value, keywordRestNodeForSpan), + keywordRestArgs.span)); return Tuple2(positional, named); } else { throw _exception( @@ -2533,9 +2570,10 @@ class _EvaluateVisitor /// real work to manufacture a source span. void _addRestMap(Map values, SassMap map, AstNode nodeWithSpan, T convert(Value value)) { + var expressionNode = _expressionNode(nodeWithSpan); map.contents.forEach((key, value) { if (key is SassString) { - values[key.text] = convert(value); + values[key.text] = convert(_withoutSlash(value, expressionNode)); } else { throw _exception( "Variable keyword argument map must have string keys.\n" @@ -2869,15 +2907,17 @@ class _EvaluateVisitor /// This returns an [AstNode] rather than a [FileSpan] so we can avoid calling /// [AstNode.span] if the span isn't required, since some nodes need to do /// real work to manufacture a source span. - AstNode _expressionNode(Expression expression) { - // If we aren't making a source map this doesn't matter, but we still return - // the expression so we don't have to make the type (and everything - // downstream of it) nullable. - if (!_sourceMap) return expression; + AstNode _expressionNode(AstNode expression) { + // TODO(nweiz): This used to return [expression] as-is if source map + // generation was disabled. We always have to track the original location + // now to produce better warnings for /-as-division, but once those warnings + // are gone we should go back to short-circuiting. if (expression is VariableExpression) { - return _environment.getVariableNode(expression.name, - namespace: expression.namespace) ?? + return _addExceptionSpan( + expression, + () => _environment.getVariableNode(expression.name, + namespace: expression.namespace)) ?? expression; } else { return expression; @@ -2972,6 +3012,35 @@ class _EvaluateVisitor return result; } + /// Like [Value.withoutSlash], but produces a deprecation warning if [value] + /// was a slash-separated number. + Value _withoutSlash(Value value, AstNode nodeForSpan) { + if (value is SassNumber && value.asSlash != null) { + String recommendation(SassNumber number) { + var asSlash = number.asSlash; + if (asSlash != null) { + return "math.div(${recommendation(asSlash.item1)}, " + "${recommendation(asSlash.item2)})"; + } else { + return number.toString(); + } + } + + _warn( + "Using / for division is deprecated and will be removed in Dart Sass " + "2.0.0.\n" + "\n" + "Recommendation: ${recommendation(value)}\n" + "\n" + "More info and automated migrator: " + "https://sass-lang.com/d/slash-div", + nodeForSpan.span, + deprecation: true); + } + + return value.withoutSlash(); + } + /// Creates a new stack frame with location information from [member] and /// [span]. Frame _stackFrame(String member, FileSpan span) => frameForSpan(span, member, @@ -3146,28 +3215,26 @@ class _ArgumentResults { /// Arguments passed by position. final List positional; - /// The [AstNode]s that hold the spans for each [positional] argument, or - /// `null` if source span tracking is disabled. + /// The [AstNode]s that hold the spans for each [positional] argument. /// /// This stores [AstNode]s rather than [FileSpan]s so it can avoid calling /// [AstNode.span] if the span isn't required, since some nodes need to do /// real work to manufacture a source span. - final List? positionalNodes; + final List positionalNodes; /// Arguments passed by name. final Map named; - /// The [AstNode]s that hold the spans for each [named] argument, or `null` if - /// source span tracking is disabled. + /// The [AstNode]s that hold the spans for each [named] argument. /// /// This stores [AstNode]s rather than [FileSpan]s so it can avoid calling /// [AstNode.span] if the span isn't required, since some nodes need to do /// real work to manufacture a source span. - final Map? namedNodes; + final Map namedNodes; /// The separator used for the rest argument list, if any. final ListSeparator separator; - _ArgumentResults(this.positional, this.named, this.separator, - {this.positionalNodes, this.namedNodes}); + _ArgumentResults(this.positional, this.positionalNodes, this.named, + this.namedNodes, this.separator); } diff --git a/lib/src/visitor/serialize.dart b/lib/src/visitor/serialize.dart index 8e0245f9f..3171dcc45 100644 --- a/lib/src/visitor/serialize.dart +++ b/lib/src/visitor/serialize.dart @@ -559,14 +559,15 @@ class _SerializeVisitor var singleton = _inspect && value.asList.length == 1 && - value.separator == ListSeparator.comma; + (value.separator == ListSeparator.comma || + value.separator == ListSeparator.slash); if (singleton && !value.hasBrackets) _buffer.writeCharCode($lparen); _writeBetween( _inspect ? value.asList : value.asList.where((element) => !element.isBlank), - value.separator == ListSeparator.space ? " " : _commaSeparator, + _separatorString(value.separator), _inspect ? (element) { var needsParens = _elementNeedsParens(value.separator, element); @@ -579,22 +580,45 @@ class _SerializeVisitor }); if (singleton) { - _buffer.writeCharCode($comma); + _buffer.write(value.separator.separator); if (!value.hasBrackets) _buffer.writeCharCode($rparen); } if (value.hasBrackets) _buffer.writeCharCode($rbracket); } + /// Returns the string to use to separate list items for lists with the given [separator]. + String _separatorString(ListSeparator separator) { + switch (separator) { + case ListSeparator.comma: + return _commaSeparator; + case ListSeparator.slash: + return _isCompressed ? "/" : " / "; + case ListSeparator.space: + return " "; + default: + // This should never be used, but it may still be returned since + // [_separatorString] is invoked eagerly by [writeList] even for lists + // with only one elements. + return ""; + } + } + /// Returns whether [value] needs parentheses as an element in a list with the /// given [separator]. bool _elementNeedsParens(ListSeparator separator, Value value) { if (value is SassList) { if (value.asList.length < 2) return false; if (value.hasBrackets) return false; - return separator == ListSeparator.comma - ? value.separator == ListSeparator.comma - : value.separator != ListSeparator.undecided; + switch (separator) { + case ListSeparator.comma: + return value.separator == ListSeparator.comma; + case ListSeparator.slash: + return value.separator == ListSeparator.comma || + value.separator == ListSeparator.slash; + default: + return value.separator != ListSeparator.undecided; + } } return false; } diff --git a/test/compressed_test.dart b/test/compressed_test.dart index 27c73ea59..0ca281635 100644 --- a/test/compressed_test.dart +++ b/test/compressed_test.dart @@ -109,6 +109,13 @@ void main() { expect(_compile("a {b: x, y, z}"), equals("a{b:x,y,z}")); }); + test("don't include spaces around slashes", () { + expect(_compile(""" + @use "sass:list"; + a {b: list.slash(x, y, z)} + """), equals("a{b:x/y/z}")); + }); + test("do include spaces when space-separated", () { expect(_compile("a {b: x y z}"), equals("a{b:x y z}")); }); diff --git a/test/dart_api/value/list_test.dart b/test/dart_api/value/list_test.dart index b0ddcaa20..6e9a12dea 100644 --- a/test/dart_api/value/list_test.dart +++ b/test/dart_api/value/list_test.dart @@ -116,6 +116,11 @@ void main() { }); }); + test("a slash-separated list is slash-separated", () { + expect(parseValue("list.slash(a, b, c)").separator, + equals(ListSeparator.slash)); + }); + test("a space-separated list is space-separated", () { expect(parseValue("a, b, c").separator, equals(ListSeparator.comma)); }); diff --git a/test/dart_api/value/utils.dart b/test/dart_api/value/utils.dart index ae2435de3..fd48cf42a 100644 --- a/test/dart_api/value/utils.dart +++ b/test/dart_api/value/utils.dart @@ -10,7 +10,11 @@ import 'package:sass/src/exception.dart'; /// Parses [source] by way of a function call. Value parseValue(String source) { late Value value; - compileString("a {b: foo(($source))}", functions: [ + compileString(""" + @use "sass:list"; + + a {b: foo(($source))} + """, functions: [ Callable("foo", r"$arg", expectAsync1((arguments) { expect(arguments, hasLength(1)); value = arguments.first;