Skip to content

Commit e957c23

Browse files
kajkaj
committed
Add some checks for calc.
When the `calc` function evaluates partially (to a `calc(...)` call), that call should be a valid css calc call.
1 parent 08a9256 commit e957c23

File tree

29 files changed

+109
-262
lines changed

29 files changed

+109
-262
lines changed

Diff for: ‎CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ project adheres to
99

1010
## Unreleased
1111

12+
* Make the `calc(...)` function signal an error when args are known to
13+
be invalid css (PR #138).
1214
* Minor fix in whitespace around at-rules.
1315
* Fix a typo in LICENSE (Issue #136).
1416
* Update sass-spec test suite to 2022-04-07.

Diff for: ‎src/parser/pos.rs

+17
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,23 @@ impl SourcePos {
187187
p.length += len;
188188
}
189189
}
190+
/// If the position is `calc(some-arg)`, change to only `some-arg`.
191+
///
192+
/// This is only used to make errors from rsass more similar to
193+
/// dart-sass errors.
194+
pub(crate) fn opt_in_calc(mut self) -> Self {
195+
let p: &mut SourcePosImpl = Arc::make_mut(&mut self.p);
196+
let s = "calc(";
197+
let part = &p.line[p.line_pos - 1..];
198+
if part.starts_with(s) && part.chars().nth(p.length - 1) == Some(')')
199+
{
200+
let len = s.chars().count();
201+
p.line_pos += len;
202+
p.length -= len;
203+
p.length -= 1;
204+
}
205+
self
206+
}
190207

191208
/// True if this is the position of something built-in.
192209
pub fn is_builtin(&self) -> bool {

Diff for: ‎src/parser/unit.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ pub fn unit(input: Span) -> PResult<Unit> {
1919
"vmax" => Unit::Vmax,
2020
"cm" => Unit::Cm,
2121
"mm" => Unit::Mm,
22-
"q" => Unit::Q,
22+
"Q" | "q" => Unit::Q,
2323
"in" => Unit::In,
2424
"pt" => Unit::Pt,
2525
"pc" => Unit::Pc,

Diff for: ‎src/sass/functions/mod.rs

+63-22
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use crate::error::Error;
33
use crate::output::{Format, Formatted};
44
use crate::parser::SourcePos;
55
use crate::sass::{FormalArgs, Name};
6-
use crate::value::Numeric;
6+
use crate::value::{CssDimension, Numeric, Operator, Quotes};
77
use crate::{sass, Scope, ScopeRef};
88
use lazy_static::lazy_static;
99
use std::collections::BTreeMap;
@@ -240,40 +240,81 @@ lazy_static! {
240240
_ => false,
241241
}
242242
}
243-
fn in_calc(v: Value) -> Value {
243+
// Note: None here is for unknown, e.g. the dimension of something that is not a number.
244+
fn css_dim(v: &Value) -> Option<Vec<(CssDimension, i8)>> {
244245
match v {
245-
Value::Literal(s) => {
246-
if s.quotes() == crate::value::Quotes::None
247-
&& s.value().ends_with(')')
248-
&& s.value().starts_with("calc(")
246+
// TODO: Handle BinOp recursively (again) (or let in_calc return (Value, CssDimension)?)
247+
Value::Numeric(num, _) => {
248+
let u = &num.unit;
249+
if u.is_known() && !u.is_percent() {
250+
Some(u.css_dimension())
251+
} else {
252+
None
253+
}
254+
}
255+
_ => None,
256+
}
257+
}
258+
fn in_calc(v: Value) -> Result<Value, Error> {
259+
match v {
260+
Value::Literal(s) if s.quotes() == Quotes::None => {
261+
if let Some(arg) = s
262+
.value()
263+
.strip_prefix("calc(")
264+
.and_then(|s| s.strip_suffix(')'))
249265
{
250-
Value::Paren(Box::new(
251-
s.value()[5..s.value().len() - 1].into(),
252-
))
266+
Ok(Value::Paren(Box::new(arg.into())))
253267
} else {
254-
s.into()
268+
Ok(s.into())
255269
}
256270
}
257271
Value::Call(name, args) => {
258272
if name == "calc"
259273
&& args.check_no_named().is_ok()
260274
&& args.positional.len() == 1
261275
{
262-
Value::Paren(Box::new(
276+
Ok(Value::Paren(Box::new(
263277
args.positional.into_iter().next().unwrap(),
264-
))
278+
)))
279+
} else {
280+
Ok(Value::Call(name, args))
281+
}
282+
}
283+
Value::BinOp(a, _, op, _, b) => {
284+
let a = in_calc(*a)?;
285+
let b = in_calc(*b)?;
286+
if let (Some(adim), Some(bdim)) = (css_dim(&a), css_dim(&b)) {
287+
if (op == Operator::Plus || op == Operator::Minus) && adim != bdim {
288+
return Err(Error::error(format!(
289+
"{} and {} are incompatible.",
290+
a.format(Format::introspect()),
291+
b.format(Format::introspect()),
292+
)))
293+
}
294+
}
295+
Ok(Value::BinOp(
296+
Box::new(a),
297+
true,
298+
op,
299+
true,
300+
Box::new(b),
301+
))
302+
}
303+
Value::Numeric(num, c) => {
304+
if num.unit.valid_in_css() {
305+
Ok(Value::Numeric(num, c))
265306
} else {
266-
Value::Call(name, args)
307+
Err(Error::error(format!(
308+
"Number {} isn't compatible with CSS calculations.",
309+
num.format(Format::introspect())
310+
)))
267311
}
268312
}
269-
Value::BinOp(a, _, op, _, b) => Value::BinOp(
270-
Box::new(in_calc(*a)),
271-
true,
272-
op,
273-
true,
274-
Box::new(in_calc(*b)),
275-
),
276-
v => v,
313+
v @ Value::Paren(..) => Ok(v),
314+
v => Err(Error::error(format!(
315+
"Value {} can't be used in a calculation.",
316+
v.format(Format::introspect())
317+
))),
277318
}
278319
}
279320
let v = s.get(&name!(expr))?;
@@ -282,7 +323,7 @@ lazy_static! {
282323
} else {
283324
Ok(Value::Call(
284325
"calc".into(),
285-
CallArgs::from_value(in_calc(v))?,
326+
CallArgs::from_value(in_calc(v)?)?,
286327
))
287328
}
288329
});

Diff for: ‎src/sass/value.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,10 @@ impl Value {
156156
let msg = format!("Error: {}", msg);
157157
Error::BadCall(msg, pos.clone(), None)
158158
}
159-
e => Error::BadCall(e.to_string(), pos.clone(), None),
159+
e => {
160+
let pos = pos.clone().opt_in_calc();
161+
Error::BadCall(e.to_string(), pos, None)
162+
}
160163
};
161164
let name = name.into();
162165
if let Some(f) = scope

Diff for: ‎src/value/unit.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ pub enum Unit {
3030
Cm,
3131
/// `mm` unit, absolute length.
3232
Mm,
33-
/// `q` unit, absolute length (4q == 1mm).
33+
/// `q` unit, absolute length (4Q == 1mm).
3434
Q,
3535
/// `in` unit, absolute length in inch.
3636
In,
@@ -216,7 +216,7 @@ impl fmt::Display for Unit {
216216
Unit::Vmax => write!(out, "vmax"),
217217
Unit::Cm => write!(out, "cm"),
218218
Unit::Mm => write!(out, "mm"),
219-
Unit::Q => write!(out, "q"),
219+
Unit::Q => write!(out, "Q"),
220220
Unit::In => write!(out, "in"),
221221
Unit::Pt => write!(out, "pt"),
222222
Unit::Pc => write!(out, "pc"),

Diff for: ‎src/value/unitset.rs

+20-1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,13 @@ impl UnitSet {
2323
pub fn is_none(&self) -> bool {
2424
self.units.iter().all(|(u, _)| *u == Unit::None)
2525
}
26+
/// Check if this UnitSet conains only known units.
27+
pub fn is_known(&self) -> bool {
28+
!self
29+
.units
30+
.iter()
31+
.any(|(u, _)| matches!(u, Unit::Unknown(_)))
32+
}
2633
/// Check if this UnitSet is the percent unit.
2734
pub fn is_percent(&self) -> bool {
2835
self.units == [(Unit::Percent, 1)]
@@ -66,6 +73,14 @@ impl UnitSet {
6673
.filter(|(_d, power)| *power != 0)
6774
.collect::<Vec<_>>()
6875
}
76+
pub(crate) fn valid_in_css(&self) -> bool {
77+
let dim = self.css_dimension();
78+
match &dim[..] {
79+
[] => true,
80+
[(_d, p)] => *p == 1,
81+
_ => false,
82+
}
83+
}
6984

7085
/// Get a scaling factor to convert this unit to another unit.
7186
///
@@ -219,7 +234,11 @@ impl Display for UnitSet {
219234

220235
fn write_one(out: &mut fmt::Formatter, u: &Unit, p: i8) -> fmt::Result {
221236
u.fmt(out)?;
222-
if p != 1 {
237+
if (0..=3).contains(&p) {
238+
for _ in 1..p {
239+
write!(out, "*{}", u)?;
240+
}
241+
} else {
223242
write!(out, "^{}", p)?;
224243
}
225244
Ok(())

Diff for: ‎tests/spec/values/calculation/calc/error/complex_units.rs

-6
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ mod denominator {
1010
use super::runner;
1111

1212
#[test]
13-
#[ignore] // missing error
1413
fn from_variable() {
1514
assert_eq!(
1615
runner().err(
@@ -27,7 +26,6 @@ mod denominator {
2726
);
2827
}
2928
#[test]
30-
#[ignore] // missing error
3129
fn within_calc() {
3230
assert_eq!(
3331
runner().err("a {b: calc(1% + 1 / 2px)}\n"),
@@ -45,7 +43,6 @@ mod multiple_numerator {
4543
use super::runner;
4644

4745
#[test]
48-
#[ignore] // missing error
4946
fn from_variable() {
5047
assert_eq!(
5148
runner().err(
@@ -61,7 +58,6 @@ mod multiple_numerator {
6158
);
6259
}
6360
#[test]
64-
#[ignore] // missing error
6561
fn within_calc() {
6662
assert_eq!(
6763
runner().err("a {b: calc(1% + 1px * 2px)}\n"),
@@ -79,7 +75,6 @@ mod numerator_and_denominator {
7975
use super::runner;
8076

8177
#[test]
82-
#[ignore] // missing error
8378
fn from_variable() {
8479
assert_eq!(
8580
runner().err(
@@ -96,7 +91,6 @@ mod numerator_and_denominator {
9691
);
9792
}
9893
#[test]
99-
#[ignore] // missing error
10094
fn within_calc() {
10195
assert_eq!(
10296
runner().err("a {b: calc(1% + 1s / 2px)}\n"),

0 commit comments

Comments
 (0)