Skip to content

Commit

Permalink
Merge pull request #1175 from sass/angle-units
Browse files Browse the repository at this point in the history 
Deprecate incorrect HSL units
  • Loading branch information
@nex3
nex3 committed Dec 29, 2020
2 parents 02c92aa + bae2968 commit 723dc69
Show file tree
Hide file tree
Showing 11 changed files with 562 additions and 109 deletions.
39 changes: 39 additions & 0 deletions CHANGELOG.md
View file
@@ -1,3 +1,42 @@
## 1.32.0

* Deprecate passing non-`%` numbers as lightness and saturation to `hsl()`,
`hsla()`, `color.adjust()`, and `color.change()`. This matches the CSS
specification, which also requires `%` for all lightness and saturation
parameters. See [the Sass website][color-units] for more details.

* Deprecate passing numbers with units other than `deg` as the hue to `hsl()`,
`hsla()`, `adjust-hue()`, `color.adjust()`, and `color.change()`. Unitless
numbers *are* still allowed here, since they're allowed by CSS. See [the Sass
website][color-units] for more details.

* Improve error messages about incompatible units.

* Properly mark some warnings emitted by `sass:color` functions as deprecation
warnings.

### Dart API

* Rename `SassNumber.valueInUnits()` to `SassNumber.coerceValue()`. The old name
remains, but is now deprecated.

* Rename `SassNumber.coerceValueToUnit()`, a shorthand for
`SassNumber.coerceValue()` that takes a single numerator unit.

* Add `SassNumber.coerceToMatch()` and `SassNumber.coerceValueToMatch()`, which
work like `SassNumber.coerce()` and `SassNumber.coerceValue()` but take a
`SassNumber` whose units should be matched rather than taking the units
explicitly. These generate better error messages than `SassNumber.coerce()`
and `SassNumber.coerceValue()`.

* Add `SassNumber.convertToMatch()` and `SassNumber.convertValueToMatch()`,
which work like `SassNumber.coerceToMatch()` and
`SassNumber.coerceValueToMatch()` except they throw exceptions when converting
unitless values to or from units.

* Add `SassNumber.compatibleWithUnit()`, which returns whether the number can be
coerced to a single numerator unit.

## 1.31.0

* Add support for parsing `clamp()` as a special math function, the same way
Expand Down
99 changes: 85 additions & 14 deletions lib/src/functions/color.dart
View file
Expand Up @@ -120,6 +120,7 @@ final global = UnmodifiableListView([
_function("adjust-hue", r"$color, $degrees", (arguments) {
var color = arguments[0].assertColor("color");
var degrees = arguments[1].assertNumber("degrees");
_checkAngle(degrees);
return color.changeHsl(hue: color.hue + degrees.value);
}),

Expand Down Expand Up @@ -231,9 +232,11 @@ final module = BuiltInModule("color", functions: [
}

var result = _functionString("invert", arguments.take(1));
warn("Passing a number to color.invert() is deprecated.\n"
warn(
"Passing a number to color.invert() is deprecated.\n"
"\n"
"Recommendation: $result");
"Recommendation: $result",
deprecation: true);
return result;
}

Expand All @@ -255,9 +258,11 @@ final module = BuiltInModule("color", functions: [
_function("grayscale", r"$color", (arguments) {
if (arguments[0] is SassNumber) {
var result = _functionString("grayscale", arguments.take(1));
warn("Passing a number to color.grayscale() is deprecated.\n"
warn(
"Passing a number to color.grayscale() is deprecated.\n"
"\n"
"Recommendation: $result");
"Recommendation: $result",
deprecation: true);
return result;
}

Expand Down Expand Up @@ -306,9 +311,11 @@ final module = BuiltInModule("color", functions: [
!argument.hasQuotes &&
argument.text.contains(_microsoftFilterStart)) {
var result = _functionString("alpha", arguments);
warn("Using color.alpha() for a Microsoft filter is deprecated.\n"
warn(
"Using color.alpha() for a Microsoft filter is deprecated.\n"
"\n"
"Recommendation: $result");
"Recommendation: $result",
deprecation: true);
return result;
}

Expand All @@ -322,9 +329,11 @@ final module = BuiltInModule("color", functions: [
argument.text.contains(_microsoftFilterStart))) {
// Support the proprietary Microsoft alpha() function.
var result = _functionString("alpha", arguments);
warn("Using color.alpha() for a Microsoft filter is deprecated.\n"
warn(
"Using color.alpha() for a Microsoft filter is deprecated.\n"
"\n"
"Recommendation: $result");
"Recommendation: $result",
deprecation: true);
return result;
}

Expand All @@ -337,9 +346,11 @@ final module = BuiltInModule("color", functions: [
_function("opacity", r"$color", (arguments) {
if (arguments[0] is SassNumber) {
var result = _functionString("opacity", arguments);
warn("Passing a number to color.opacity() is deprecated.\n"
warn(
"Passing a number to color.opacity() is deprecated.\n"
"\n"
"Recommendation: $result");
"Recommendation: $result",
deprecation: true);
return result;
}

Expand Down Expand Up @@ -437,9 +448,11 @@ SassColor _updateComponents(List<Value> arguments,
///
/// [max] should be 255 for RGB channels, 1 for the alpha channel, and 100
/// for saturation, lightness, whiteness, and blackness.
num getParam(String name, num max, {bool assertPercent = false}) {
num getParam(String name, num max,
{bool checkPercent = false, bool assertPercent = false}) {
var number = keywords.remove(name)?.assertNumber(name);
if (number == null) return null;
if (!scale && checkPercent) _checkPercent(number, name);
if (scale || assertPercent) number.assertUnit("%", name);
if (scale) max = 100;
return number.valueInRange(change ? 0 : -max, max, name);
Expand All @@ -449,9 +462,13 @@ SassColor _updateComponents(List<Value> arguments,
var red = getParam("red", 255);
var green = getParam("green", 255);
var blue = getParam("blue", 255);
var hue = scale ? null : keywords.remove("hue")?.assertNumber("hue")?.value;
var saturation = getParam("saturation", 100);
var lightness = getParam("lightness", 100);

var hueNumber = scale ? null : keywords.remove("hue")?.assertNumber("hue");
if (hueNumber != null) _checkAngle(hueNumber, "hue");
var hue = hueNumber?.value;

var saturation = getParam("saturation", 100, checkPercent: true);
var lightness = getParam("lightness", 100, checkPercent: true);
var whiteness = getParam("whiteness", 100, assertPercent: true);
var blackness = getParam("blackness", 100, assertPercent: true);

Expand Down Expand Up @@ -600,6 +617,10 @@ Value _hsl(String name, List<Value> arguments) {
var saturation = arguments[1].assertNumber("saturation");
var lightness = arguments[2].assertNumber("lightness");

_checkAngle(hue, "hue");
_checkPercent(saturation, "saturation");
_checkPercent(lightness, "lightness");

return SassColor.hsl(
hue.value,
saturation.value.clamp(0, 100),
Expand All @@ -609,6 +630,56 @@ Value _hsl(String name, List<Value> arguments) {
: _percentageOrUnitless(alpha.assertNumber("alpha"), 1, "alpha"));
}

/// Prints a deprecation warning if [hue] has a unit other than `deg`.
void _checkAngle(SassNumber angle, [String name]) {
if (!angle.hasUnits || angle.hasUnit('deg')) return;

var message = StringBuffer()
..writeln("\$$name: Passing a unit other than deg is deprecated.")
..writeln();

if (angle.compatibleWithUnit('deg')) {
message
..writeln(
"You're passing $angle, which is currently (incorrectly) converted "
"to ${SassNumber(angle.value, 'deg')}.")
..writeln("Soon, it will instead be correctly converted to "
"${angle.coerce(['deg'], [])}.")
..writeln();

var actualUnit = angle.numeratorUnits.first;
message
..writeln("To preserve current behavior: \$$name * 1deg/1$actualUnit")
..writeln("To migrate to new behavior: 0deg + \$$name")
..writeln();
} else {
message
..writeln("To preserve current behavior: \$$name${_removeUnits(angle)}")
..writeln();
}

message.write("See https://sass-lang.com/d/color-units");
warn(message.toString(), deprecation: true);
}

/// Prints a deprecation warning if [number] doesn't have unit `%`.
void _checkPercent(SassNumber number, String name) {
if (number.hasUnit('%')) return;

warn("\$$name: Passing a number without unit % is deprecated.\n"
"\n"
"To preserve current behavior: \$$name${_removeUnits(number)} * 1%",
deprecation: true);
}

/// Returns the right-hand side of an expression that would remove all units
/// from `$number` but leaves the value the same.
///
/// Used for constructing deprecation messages.
String _removeUnits(SassNumber number) =>
number.denominatorUnits.map((unit) => " * 1$unit").join() +
number.numeratorUnits.map((unit) => " / 1$unit").join();

/// Create an HWB color from the given [arguments].
Value _hwb(List<Value> arguments) {
var alpha = arguments.length > 3 ? arguments[3] : null;
Expand Down
83 changes: 27 additions & 56 deletions lib/src/functions/math.dart
View file
Expand Up @@ -41,20 +41,17 @@ final _clamp = _function("clamp", r"$min, $number, $max", (arguments) {
var min = arguments[0].assertNumber("min");
var number = arguments[1].assertNumber("number");
var max = arguments[2].assertNumber("max");
if (min.hasUnits == number.hasUnits && number.hasUnits == max.hasUnits) {
if (min.greaterThanOrEquals(max).isTruthy) return min;
if (min.greaterThanOrEquals(number).isTruthy) return min;
if (number.greaterThanOrEquals(max).isTruthy) return max;
return number;
}

var arg2 = min.hasUnits != number.hasUnits ? number : max;
var arg2Name = min.hasUnits != number.hasUnits ? "\$number" : "\$max";
var unit1 = min.hasUnits ? "has unit ${min.unitString}" : "is unitless";
var unit2 = arg2.hasUnits ? "has unit ${arg2.unitString}" : "is unitless";
throw SassScriptException(
"\$min $unit1 but $arg2Name $unit2. Arguments must all have units or all "
"be unitless.");
// Even though we don't use the resulting values, `convertValueToMatch`
// generates more user-friendly exceptions than [greaterThanOrEquals] since it
// has more context about parameter names.
number.convertValueToMatch(min, "number", "min");
max.convertValueToMatch(min, "max", "min");

if (min.greaterThanOrEquals(max).isTruthy) return min;
if (min.greaterThanOrEquals(number).isTruthy) return min;
if (number.greaterThanOrEquals(max).isTruthy) return max;
return number;
});

final _floor = _numberFunction("floor", (value) => value.floor());
Expand Down Expand Up @@ -94,27 +91,16 @@ final _hypot = _function("hypot", r"$numbers...", (arguments) {
throw SassScriptException("At least one argument must be passed.");
}

var numeratorUnits = numbers[0].numeratorUnits;
var denominatorUnits = numbers[0].denominatorUnits;
var subtotal = 0.0;
for (var i = 0; i < numbers.length; i++) {
var number = numbers[i];
if (number.hasUnits == numbers[0].hasUnits) {
number = number.coerce(numeratorUnits, denominatorUnits);
subtotal += math.pow(number.value, 2);
} else {
var unit1 = numbers[0].hasUnits
? "has unit ${numbers[0].unitString}"
: "is unitless";
var unit2 =
number.hasUnits ? "has unit ${number.unitString}" : "is unitless";
throw SassScriptException(
"Argument 1 $unit1 but argument ${i + 1} $unit2. Arguments must all "
"have units or all be unitless.");
}
var value = number.convertValueToMatch(
numbers[0], "numbers[${i + 1}]", "numbers[1]");
subtotal += math.pow(value, 2);
}
return SassNumber.withUnits(math.sqrt(subtotal),
numeratorUnits: numeratorUnits, denominatorUnits: denominatorUnits);
numeratorUnits: numbers[0].numeratorUnits,
denominatorUnits: numbers[0].denominatorUnits);
});

///
Expand Down Expand Up @@ -232,42 +218,36 @@ final _atan = _function("atan", r"$number", (arguments) {
final _atan2 = _function("atan2", r"$y, $x", (arguments) {
var y = arguments[0].assertNumber("y");
var x = arguments[1].assertNumber("x");
if (y.hasUnits != x.hasUnits) {
var unit1 = y.hasUnits ? "has unit ${y.unitString}" : "is unitless";
var unit2 = x.hasUnits ? "has unit ${x.unitString}" : "is unitless";
throw SassScriptException(
"\$y $unit1 but \$x $unit2. Arguments must all have units or all be "
"unitless.");
}

x = x.coerce(y.numeratorUnits, y.denominatorUnits);
var xValue = _fuzzyRoundIfZero(x.value);
var xValue = _fuzzyRoundIfZero(x.convertValueToMatch(y, 'x', 'y'));
var yValue = _fuzzyRoundIfZero(y.value);
var atan2 = math.atan2(yValue, xValue) * 180 / math.pi;
return SassNumber.withUnits(atan2, numeratorUnits: ['deg']);
});

final _cos = _function("cos", r"$number", (arguments) {
var number = _coerceToRad(arguments[0].assertNumber("number"));
return SassNumber(math.cos(number.value));
var value =
arguments[0].assertNumber("number").coerceValueToUnit("rad", "number");
return SassNumber(math.cos(value));
});

final _sin = _function("sin", r"$number", (arguments) {
var number = _coerceToRad(arguments[0].assertNumber("number"));
var numberValue = _fuzzyRoundIfZero(number.value);
return SassNumber(math.sin(numberValue));
var value = _fuzzyRoundIfZero(
arguments[0].assertNumber("number").coerceValueToUnit("rad", "number"));
return SassNumber(math.sin(value));
});

final _tan = _function("tan", r"$number", (arguments) {
var number = _coerceToRad(arguments[0].assertNumber("number"));
var value =
arguments[0].assertNumber("number").coerceValueToUnit("rad", "number");
var asymptoteInterval = 0.5 * math.pi;
var tanPeriod = 2 * math.pi;
if (fuzzyEquals((number.value - asymptoteInterval) % tanPeriod, 0)) {
if (fuzzyEquals((value - asymptoteInterval) % tanPeriod, 0)) {
return SassNumber(double.infinity);
} else if (fuzzyEquals((number.value + asymptoteInterval) % tanPeriod, 0)) {
} else if (fuzzyEquals((value + asymptoteInterval) % tanPeriod, 0)) {
return SassNumber(double.negativeInfinity);
} else {
var numberValue = _fuzzyRoundIfZero(number.value);
var numberValue = _fuzzyRoundIfZero(value);
return SassNumber(math.tan(numberValue));
}
});
Expand Down Expand Up @@ -322,15 +302,6 @@ num _fuzzyRoundIfZero(num number) {
return number.isNegative ? -0.0 : 0;
}

SassNumber _coerceToRad(SassNumber number) {
try {
return number.coerce(['rad'], []);
} on SassScriptException catch (error) {
if (!error.message.startsWith('Incompatible units')) rethrow;
throw SassScriptException('\$number: Expected ${number} to be an angle.');
}
}

/// Returns a [Callable] named [name] that transforms a number's value
/// using [transform] and preserves its units.
BuiltInCallable _numberFunction(String name, num transform(num value)) {
Expand Down
5 changes: 5 additions & 0 deletions lib/src/utils.dart
View file
Expand Up @@ -37,6 +37,11 @@ String pluralize(String name, int number, {String plural}) {
return '${name}s';
}

/// Returns `a $word` or `an $word` depending on whether [word] starts with a
/// vowel.
String a(String word) =>
[$a, $e, $i, $o, $u].contains(word.codeUnitAt(0)) ? "an $word" : "a $word";

/// Returns a bulleted list of items in [bullets].
String bulletedList(Iterable<String> bullets) {
return bullets.map((element) {
Expand Down

0 comments on commit 723dc69

Please sign in to comment.