Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error messages for incorrect units in color functions #1772

Merged
merged 1 commit into from Aug 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
@@ -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
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
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
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
@@ -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
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
@@ -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