From 2fc5210c6478bfc57954ef8bfe92320b069012a8 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Mon, 8 Aug 2022 18:45:32 -0700 Subject: [PATCH] Improve error messages for incorrect units in color functions Closes #1745 --- CHANGELOG.md | 5 +++++ lib/src/extend/functions.dart | 14 +++++++++----- lib/src/functions/color.dart | 11 ++++++++--- lib/src/value/number.dart | 16 ++++++++++++++++ pkg/sass_api/CHANGELOG.md | 4 ++++ pkg/sass_api/pubspec.yaml | 4 ++-- pubspec.yaml | 2 +- 7 files changed, 45 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fad8863bc..4b10c2333 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/lib/src/extend/functions.dart b/lib/src/extend/functions.dart index b6c433d3b..ffd0f10db 100644 --- a/lib/src/extend/functions.dart +++ b/lib/src/extend/functions.dart @@ -235,12 +235,13 @@ Iterable? _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)); @@ -299,11 +300,14 @@ Iterable? _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 queue) { +ComplexSelectorComponent? _firstIfRootish( + Queue 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; } diff --git a/lib/src/functions/color.dart b/lib/src/functions/color.dart index a2fb78194..646b1e0b6 100644 --- a/lib/src/functions/color.dart +++ b/lib/src/functions/color.dart @@ -459,7 +459,10 @@ SassColor _updateComponents(List 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); @@ -846,7 +849,8 @@ SassColor _opacify(List 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. @@ -855,7 +859,8 @@ SassColor _transparentize(List 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 diff --git a/lib/src/value/number.dart b/lib/src/value/number.dart index 7a2418a59..8caa42041 100644 --- a/lib/src/value/number.dart +++ b/lib/src/value/number.dart @@ -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); diff --git a/pkg/sass_api/CHANGELOG.md b/pkg/sass_api/CHANGELOG.md index 785decf21..629e3bebc 100644 --- a/pkg/sass_api/CHANGELOG.md +++ b/pkg/sass_api/CHANGELOG.md @@ -1,3 +1,7 @@ +## 2.0.4 + +* No user-visible changes. + ## 2.0.3 * No user-visible changes. diff --git a/pkg/sass_api/pubspec.yaml b/pkg/sass_api/pubspec.yaml index 378b063dd..779ff9803 100644 --- a/pkg/sass_api/pubspec.yaml +++ b/pkg/sass_api/pubspec.yaml @@ -2,7 +2,7 @@ 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 @@ -10,7 +10,7 @@ environment: sdk: ">=2.12.0 <3.0.0" dependencies: - sass: 1.54.3 + sass: 1.54.4 dev_dependencies: dartdoc: ^5.0.0 diff --git a/pubspec.yaml b/pubspec.yaml index ac9b92691..3b7735a18 100644 --- a/pubspec.yaml +++ b/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