From 3d0dcbea182f6a81539c12c9d5156cf0f6863b67 Mon Sep 17 00:00:00 2001
From: Laurenz <laurmaedje@gmail.com>
Date: Tue, 5 Oct 2021 19:55:15 +0200
Subject: [PATCH] Error on out-of-range values in `rgb`

---
 src/library/mod.rs          |  2 +-
 src/library/utility.rs      | 44 +++++++++++++++++++++----------------
 tests/typ/utility/color.typ | 11 ++++++++--
 3 files changed, 35 insertions(+), 22 deletions(-)

diff --git a/src/library/mod.rs b/src/library/mod.rs
index 1fcd05814..411755bb7 100644
--- a/src/library/mod.rs
+++ b/src/library/mod.rs
@@ -22,7 +22,7 @@ use crate::eval::{Args, Array, EvalContext, Scope, State, Str, Template, Value};
 use crate::font::{FontFamily, FontStretch, FontStyle, FontWeight, VerticalFontMetric};
 use crate::geom::*;
 use crate::layout::LayoutNode;
-use crate::syntax::Spanned;
+use crate::syntax::{Span, Spanned};
 
 /// Construct a scope containing all standard library definitions.
 pub fn new() -> Scope {
diff --git a/src/library/utility.rs b/src/library/utility.rs
index 3a2f49b7f..ef7adff0b 100644
--- a/src/library/utility.rs
+++ b/src/library/utility.rs
@@ -84,6 +84,31 @@ pub fn str(_: &mut EvalContext, args: &mut Args) -> TypResult<Value> {
     }))
 }
 
+/// `rgb`: Create an RGB(A) color.
+pub fn rgb(_: &mut EvalContext, args: &mut Args) -> TypResult<Value> {
+    Ok(Value::Color(Color::Rgba(
+        if let Some(string) = args.eat::<Spanned<Str>>() {
+            match RgbaColor::from_str(&string.v) {
+                Ok(color) => color,
+                Err(_) => bail!(string.span, "invalid color"),
+            }
+        } else {
+            let r = args.expect("red component")?;
+            let g = args.expect("green component")?;
+            let b = args.expect("blue component")?;
+            let a = args.eat().unwrap_or(Spanned::new(1.0, Span::detached()));
+            let f = |Spanned { v, span }: Spanned<f64>| {
+                if 0.0 <= v && v <= 1.0 {
+                    Ok((v * 255.0).round() as u8)
+                } else {
+                    bail!(span, "value must be between 0.0 and 1.0");
+                }
+            };
+            RgbaColor::new(f(r)?, f(g)?, f(b)?, f(a)?)
+        },
+    )))
+}
+
 /// `abs`: The absolute value of a numeric value.
 pub fn abs(_: &mut EvalContext, args: &mut Args) -> TypResult<Value> {
     let Spanned { v, span } = args.expect("numeric value")?;
@@ -130,25 +155,6 @@ fn minmax(args: &mut Args, goal: Ordering) -> TypResult<Value> {
     Ok(extremum)
 }
 
-/// `rgb`: Create an RGB(A) color.
-pub fn rgb(_: &mut EvalContext, args: &mut Args) -> TypResult<Value> {
-    Ok(Value::Color(Color::Rgba(
-        if let Some(string) = args.eat::<Spanned<Str>>() {
-            match RgbaColor::from_str(&string.v) {
-                Ok(color) => color,
-                Err(_) => bail!(string.span, "invalid color"),
-            }
-        } else {
-            let r = args.expect("red component")?;
-            let g = args.expect("green component")?;
-            let b = args.expect("blue component")?;
-            let a = args.eat().unwrap_or(1.0);
-            let f = |v: f64| (v.clamp(0.0, 1.0) * 255.0).round() as u8;
-            RgbaColor::new(f(r), f(g), f(b), f(a))
-        },
-    )))
-}
-
 /// `lower`: Convert a string to lowercase.
 pub fn lower(_: &mut EvalContext, args: &mut Args) -> TypResult<Value> {
     Ok(args.expect::<Str>("string")?.to_lowercase().into())
diff --git a/tests/typ/utility/color.typ b/tests/typ/utility/color.typ
index 1e16c0a6f..31d3dae87 100644
--- a/tests/typ/utility/color.typ
+++ b/tests/typ/utility/color.typ
@@ -8,8 +8,15 @@
 // Alpha channel.
 #test(rgb(1.0, 0.0, 0.0, 0.5), rgb("ff000080"))
 
-// Clamped.
-#test(rgb(-30, 15.5, 0.5), rgb("00ff80"))
+---
+// Error for values that are out of range.
+// Error: 11-14 value must be between 0.0 and 1.0
+#test(rgb(-30, 15.5, 0.5))
+
+---
+// Error for values that are out of range.
+// Error: 26-30 value must be between 0.0 and 1.0
+#test(rgb(0.1, 0.2, 0.3, -0.1))
 
 ---
 // Error: 6-11 invalid color