Skip to content

Commit

Permalink
Deprecate /-as-division and add replacements
Browse files Browse the repository at this point in the history 
Partially addresses #663
  • Loading branch information
@nex3
nex3 committed May 10, 2021
1 parent dd13738 commit 5f9da83
Show file tree
Hide file tree
Showing 10 changed files with 272 additions and 88 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
## 1.33.0

* Deprecate the use of `/` for division. The new `divide()` function should be
used instead. See [this page][] for details.

[this page]: https://sass-lang.com/documentation/breaking-changes/slash-div

* Add a `slash-list()` function that returns a slash-separated list.

## 1.32.13

* Use the proper parameter names in error messages about `string.slice`
Expand Down
27 changes: 24 additions & 3 deletions lib/src/functions/color.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,25 @@ Object /* SassString | List<Value> */ _parseChannels(
String name, List<String> 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) {
Expand All @@ -720,18 +739,20 @@ Object /* SassString | List<Value> */ _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} "
"${pluralize('was', list.length, plural: '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;
Expand Down
35 changes: 26 additions & 9 deletions lib/src/functions/list.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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'
Expand All @@ -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];
Expand Down Expand Up @@ -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<Value> arguments)) =>
Expand Down
43 changes: 24 additions & 19 deletions lib/src/value.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,27 +195,32 @@ abstract class Value implements ext.Value {
if (list.asList.isEmpty) return null;

var result = <String>[];
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 ? ', ' : ' ');
}
Expand Down
3 changes: 3 additions & 0 deletions lib/src/value/list.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
104 changes: 77 additions & 27 deletions lib/src/visitor/async_evaluate.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(() {
Expand All @@ -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);
Expand Down Expand Up @@ -1761,8 +1761,8 @@ class _EvaluateVisitor
return queries;
}

Future<Value> visitReturnRule(ReturnRule node) =>
node.expression.accept(this);
Future<Value> visitReturnRule(ReturnRule node) async =>
_withoutSlash(await node.expression.accept(this), node.expression);

Future<Value?> visitSilentComment(SilentComment node) async => null;

Expand Down Expand Up @@ -1949,7 +1949,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),
Expand Down Expand Up @@ -2055,6 +2056,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 "divide(${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;
}

Expand Down Expand Up @@ -2199,6 +2223,8 @@ class _EvaluateVisitor
UserDefinedCallable<AsyncEnvironment> callable,
AstNode nodeWithSpan,
Future<V> 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;
Expand All @@ -2216,10 +2242,11 @@ class _EvaluateVisitor
var minLength =
math.min(evaluated.positional.length, declaredArguments.length);
for (var i = 0; i < minLength; i++) {
var nodeForSpan = evaluated.positionalNodes[i];
_environment.setLocalVariable(
declaredArguments[i].name,
evaluated.positional[i].withoutSlash(),
evaluated.positionalNodes[i]);
_withoutSlash(evaluated.positional[i], nodeForSpan),
nodeForSpan);
}

for (var i = evaluated.positional.length;
Expand All @@ -2228,11 +2255,10 @@ class _EvaluateVisitor
var argument = declaredArguments[i];
var value = evaluated.named.remove(argument.name) ??
await argument.defaultValue!.accept<Future<Value>>(this);
var nodeForSpan = evaluated.namedNodes[argument.name] ??
_expressionNode(argument.defaultValue!);
_environment.setLocalVariable(
argument.name,
value.withoutSlash(),
evaluated.namedNodes[argument.name] ??
_expressionNode(argument.defaultValue!));
argument.name, _withoutSlash(value, nodeForSpan), nodeForSpan);
}

SassArgumentList? argumentList;
Expand Down Expand Up @@ -2275,20 +2301,20 @@ class _EvaluateVisitor
Future<Value> _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<AsyncEnvironment>) {
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;
}

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.",
Expand Down Expand Up @@ -2880,20 +2906,15 @@ class _EvaluateVisitor

/// Returns the [AstNode] whose span should be used for [expression].
///
/// If [expression] is a variable reference, [AstNode]'s span will be the span
/// where that variable was originally declared. Otherwise, this will just
/// return [expression].
/// If [expression] is a variable reference and [_sourceMap] is `true`,
/// [AstNode]'s span will be the span where that variable was originally
/// declared. Otherwise, this will just return [expression].
///
/// 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;

if (expression is VariableExpression) {
if (_sourceMap && expression is VariableExpression) {
return _environment.getVariableNode(expression.name,
namespace: expression.namespace) ??
expression;
Expand Down Expand Up @@ -2994,6 +3015,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,
Expand Down

0 comments on commit 5f9da83

Please sign in to comment.