Skip to content

Commit

Permalink
Improve error messages for incorrect units in color functions (#1772)
Browse files Browse the repository at this point in the history
Closes #1745
  • Loading branch information
nex3 authored Aug 10, 2022
1 parent ba88dd6 commit 6fd25ae
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 11 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 1.54.4

* Improve error messages when passing incorrect units that are also
out-of-bounds to various color functions.

## 1.54.3

* Release a native ARM64 executable for Mac OS.
Expand Down
14 changes: 9 additions & 5 deletions lib/src/extend/functions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -235,12 +235,13 @@ Iterable<ComplexSelector>? _weaveParents(
var trailingCombinators = _mergeTrailingCombinators(queue1, queue2);
if (trailingCombinators == null) return null;

// Make sure all selectors that are required to be at the root
// Make sure all selectors that are required to be at the root are unified
// with one another.
var rootish1 = _firstIfRootish(queue1);
var rootish2 = _firstIfRootish(queue2);
if (rootish1 != null && rootish2 != null) {
var rootish =
unifyCompound(rootish1.selector.components, rootish2.selector.components);
var rootish = unifyCompound(
rootish1.selector.components, rootish2.selector.components);
if (rootish == null) return null;
queue1.addFirst(ComplexSelectorComponent(rootish, rootish1.combinators));
queue2.addFirst(ComplexSelectorComponent(rootish, rootish2.combinators));
Expand Down Expand Up @@ -299,11 +300,14 @@ Iterable<ComplexSelector>? _weaveParents(
/// If the first element of [queue] has a selector like `:root` that can only
/// appear in a complex selector's first component, removes and returns that
/// element.
ComplexSelectorComponent? _firstIfRootish(Queue<ComplexSelectorComponent> queue) {
ComplexSelectorComponent? _firstIfRootish(
Queue<ComplexSelectorComponent> queue) {
if (queue.isEmpty) return null;
var first = queue.first;
for (var simple in first.selector.components) {
if (simple is PseudoSelector && simple.isClass && _rootishPseudoClasses.contains(simple.normalizedName)) {
if (simple is PseudoSelector &&
simple.isClass &&
_rootishPseudoClasses.contains(simple.normalizedName)) {
queue.removeFirst();
return first;
}
Expand Down
11 changes: 8 additions & 3 deletions lib/src/functions/color.dart
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,10 @@ SassColor _updateComponents(List<Value> arguments,
if (!scale && checkPercent) _checkPercent(number, name);
if (scale || assertPercent) number.assertUnit("%", name);
if (scale) max = 100;
return number.valueInRange(change ? 0 : -max, max, name);
return scale || assertPercent
? number.valueInRange(change ? 0 : -max, max, name)
: number.valueInRangeWithUnit(
change ? 0 : -max, max, name, checkPercent ? '%' : '');
}

var alpha = getParam("alpha", 1);
Expand Down Expand Up @@ -846,7 +849,8 @@ SassColor _opacify(List<Value> arguments) {
var amount = arguments[1].assertNumber("amount");

return color.changeAlpha(
(color.alpha + amount.valueInRange(0, 1, "amount")).clamp(0, 1));
(color.alpha + amount.valueInRangeWithUnit(0, 1, "amount", ""))
.clamp(0, 1));
}

/// The definition of the `transparentize()` and `fade-out()` functions.
Expand All @@ -855,7 +859,8 @@ SassColor _transparentize(List<Value> arguments) {
var amount = arguments[1].assertNumber("amount");

return color.changeAlpha(
(color.alpha - amount.valueInRange(0, 1, "amount")).clamp(0, 1));
(color.alpha - amount.valueInRangeWithUnit(0, 1, "amount", ""))
.clamp(0, 1));
}

/// Like [new BuiltInCallable.function], but always sets the URL to
Expand Down
16 changes: 16 additions & 0 deletions lib/src/value/number.dart
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,22 @@ abstract class SassNumber extends Value {
name);
}

/// Like [valueInRange], but with an explicit unit for the expected upper and
/// lower bounds.
///
/// This exists to solve the confusing error message in sass/dart-sass#1745,
/// and should be removed once sass/sass#3374 fully lands and unitless values
/// are required in these positions.
///
/// @nodoc
@internal
num valueInRangeWithUnit(num min, num max, String name, String unit) {
var result = fuzzyCheckRange(value, min, max);
if (result != null) return result;
throw _exception(
"Expected $this to be within $min$unit and $max$unit.", name);
}

/// Returns whether [this] has [unit] as its only unit (and as a numerator).
bool hasUnit(String unit);

Expand Down
4 changes: 4 additions & 0 deletions pkg/sass_api/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 2.0.4

* No user-visible changes.

## 2.0.3

* No user-visible changes.
Expand Down
4 changes: 2 additions & 2 deletions pkg/sass_api/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ name: sass_api
# Note: Every time we add a new Sass AST node, we need to bump the *major*
# version because it's a breaking change for anyone who's implementing the
# visitor interface(s).
version: 2.0.3
version: 2.0.4
description: Additional APIs for Dart Sass.
homepage: https://github.com/sass/dart-sass

environment:
sdk: ">=2.12.0 <3.0.0"

dependencies:
sass: 1.54.3
sass: 1.54.4

dev_dependencies:
dartdoc: ^5.0.0
Expand Down
2 changes: 1 addition & 1 deletion pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: sass
version: 1.54.3
version: 1.54.4
description: A Sass implementation in Dart.
homepage: https://github.com/sass/dart-sass

Expand Down

0 comments on commit 6fd25ae

Please sign in to comment.