Skip to content

Commit db6a197

Browse files
kajkaj
committed
Make it possible to set a variable in a module.
This means setting a variable may now fail.
1 parent 4e8afe2 commit db6a197

File tree

24 files changed

+181
-93
lines changed

24 files changed

+181
-93
lines changed

Diff for: ‎CHANGELOG.md

+4-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ project adheres to
1212
### Breaking changes
1313

1414
* Changes in `Error` representation. Many errors are now constructed like
15-
`Invalid::Something.at(pos)` (PR #145).
15+
`Invalid::SomeVariant.at(pos)` (PR #145).
1616
* `@extend` is still unsupported, but now some uses of it (e.g. in control
1717
structures) will result in an error instead of wrong output.
1818
* Some `sass::Item` alternatives now contain a `Callable`, combining
@@ -30,11 +30,14 @@ project adheres to
3030
* Added a variant to `ScopeError`.
3131
* The sass `Value::Variable` and `Item::VariableDeclaration` variants
3232
now holds a `Name` rather than just a `String` for the variable name.
33+
Also, both now holds a `SourcePos`.
3334

3435
### Improvements
3536

3637
* The `@content` can have arguments when declaring and calling a mixin
3738
(PR #146).
39+
* Variable declartions can be scoped (like `module.$var = value`). Some
40+
error reporting improvements (PR #148).
3841
* Allow interpolation in css min and max function arguments.
3942
* The url for `@use` and `@forward` must be quoted.
4043
* Some `@` rules are now forbidden in some places as they should (PR #145).

Diff for: ‎src/error.rs

+5
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,8 @@ pub enum Invalid {
161161
UndefinedVariable,
162162
/// Undefined function.
163163
UndefinedFunction,
164+
/// Cannot modify built-in variable.
165+
ModifiedBuiltinModule,
164166
/// Attemt to use an undefined module.
165167
UndefModule(String),
166168
/// Tried to declare a function with a forbidden name.
@@ -197,6 +199,9 @@ impl fmt::Display for Invalid {
197199
name
198200
)
199201
}
202+
Invalid::ModifiedBuiltinModule => {
203+
"Cannot modify built-in variable.".fmt(out)
204+
}
200205
Invalid::FunctionName => "Invalid function name.".fmt(out),
201206
Invalid::AtRule => "This at-rule is not allowed here.".fmt(out),
202207
Invalid::MixinInMixin => {

Diff for: ‎src/output/transform.rs

+8-5
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ fn handle_item(
9898
value.do_evaluate(scope.clone(), true)
9999
})?;
100100
if module.get_or_none(name).is_none() {
101-
module.define(name.clone(), value);
101+
module.define(name.clone(), value)?;
102102
} else {
103103
return Err(Error::error(
104104
"The same variable may only be configured once.",
@@ -154,7 +154,7 @@ fn handle_item(
154154
value.do_evaluate(scope.clone(), true)
155155
})?;
156156
if module.get_or_none(name).is_none() {
157-
module.define(name.clone(), value);
157+
module.define(name.clone(), value)?;
158158
} else {
159159
return Err(Error::error(
160160
"The same variable may only be configured once.",
@@ -305,9 +305,12 @@ fn handle_item(
305305
ref val,
306306
default,
307307
global,
308+
ref pos,
308309
} => {
309310
let val = val.do_evaluate(scope.clone(), true)?;
310-
scope.set_variable(name.clone(), val, *default, *global);
311+
scope
312+
.set_variable(name.clone(), val, *default, *global)
313+
.map_err(|e| e.at(pos.clone()))?;
311314
}
312315
Item::FunctionDeclaration(ref name, ref body) => {
313316
if name == "calc"
@@ -394,7 +397,7 @@ fn handle_item(
394397
let mut rule = rule;
395398
let pushed = scope.store_local_values(names);
396399
for value in values.evaluate(scope.clone())?.iter_items() {
397-
scope.define_multi(names, value);
400+
scope.define_multi(names, value)?;
398401
handle_body(
399402
body,
400403
head,
@@ -422,7 +425,7 @@ fn handle_item(
422425
let mut rule = rule;
423426
for value in range {
424427
let scope = ScopeRef::sub(scope.clone());
425-
scope.define(name.clone(), value);
428+
scope.define(name.clone(), value)?;
426429
handle_body(
427430
body,
428431
head,

Diff for: ‎src/parser/mod.rs

+79-38
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ fn top_level_item(input: Span) -> PResult<Item> {
203203
b"$" => variable_declaration2(input),
204204
b"/*" => comment_item(input),
205205
b"@" => at_rule2(input),
206-
b"" => rule(input),
206+
b"" => alt((variable_declaration_mod, rule))(input),
207207
_ => unreachable!(),
208208
}
209209
}
@@ -623,16 +623,37 @@ fn body_block2(input: Span) -> PResult<Vec<Item>> {
623623
Ok((input, v))
624624
}
625625

626-
#[cfg(test)] // TODO: Or remove this?
627626
fn variable_declaration(input: Span) -> PResult<Item> {
628627
preceded(tag("$"), variable_declaration2)(input)
629628
}
630629

631-
fn variable_declaration2(input: Span) -> PResult<Item> {
630+
fn variable_declaration_mod(input: Span) -> PResult<Item> {
631+
map(
632+
pair(terminated(name, tag(".")), variable_declaration),
633+
|(module, decl)| match decl {
634+
Item::VariableDeclaration {
635+
name,
636+
val,
637+
default,
638+
global,
639+
pos,
640+
} => Item::VariableDeclaration {
641+
name: format!("{}.{}", module, name).into(),
642+
val,
643+
default,
644+
global,
645+
pos: pos.opt_back(&format!("{}.", module)),
646+
},
647+
_ => unreachable!(),
648+
},
649+
)(input)
650+
}
651+
652+
fn variable_declaration2(input0: Span) -> PResult<Item> {
632653
let (input, name) = terminated(
633654
map(name, Name::from),
634655
delimited(opt_spacelike, tag(":"), opt_spacelike),
635-
)(input)?;
656+
)(input0)?;
636657
let (input, val) = terminated(value_expression, opt_spacelike)(input)?;
637658
let (input, (default, global)) = fold_many0(
638659
terminated(
@@ -645,14 +666,15 @@ fn variable_declaration2(input: Span) -> PResult<Item> {
645666
|| (false, false),
646667
|(default, global), (d, g)| (default || d, global || g),
647668
)(input)?;
648-
let (input, _) = semi_or_end(input)?;
669+
let (trail, _) = semi_or_end(input)?;
649670
Ok((
650-
input,
671+
trail,
651672
Item::VariableDeclaration {
652673
name,
653674
val,
654675
default,
655676
global,
677+
pos: SourcePos::from_to(input0, input).opt_back("$"),
656678
},
657679
))
658680
}
@@ -721,47 +743,66 @@ fn test_property_2() {
721743

722744
#[test]
723745
fn test_variable_declaration_simple() {
724-
assert_eq!(
725-
check_parse!(variable_declaration, b"$foo: bar;"),
746+
match check_parse!(variable_declaration, b"$foo: bar;") {
726747
Item::VariableDeclaration {
727-
name: "foo".into(),
728-
val: string("bar"),
729-
default: false,
730-
global: false,
731-
},
732-
)
748+
name,
749+
val,
750+
default,
751+
global,
752+
pos: _,
753+
} => {
754+
assert_eq!(
755+
(name, val, default, global),
756+
("foo".into(), string("bar"), false, false)
757+
)
758+
}
759+
_ => panic!(),
760+
}
733761
}
734762

735763
#[test]
736764
fn test_variable_declaration_global() {
737-
assert_eq!(
738-
check_parse!(variable_declaration, b"$y: some value !global;"),
765+
match check_parse!(variable_declaration, b"$y: some value !global;") {
739766
Item::VariableDeclaration {
740-
name: "y".into(),
741-
val: Value::List(
742-
vec![string("some"), string("value")],
743-
Some(ListSeparator::Space),
744-
false,
745-
),
746-
default: false,
747-
global: true,
748-
},
749-
)
767+
name,
768+
val,
769+
default,
770+
global,
771+
pos: _,
772+
} => {
773+
assert_eq!(
774+
(name, val, default, global),
775+
(
776+
"y".into(),
777+
Value::List(
778+
vec![string("some"), string("value")],
779+
Some(ListSeparator::Space),
780+
false,
781+
),
782+
false,
783+
true,
784+
)
785+
)
786+
}
787+
_ => panic!(),
788+
}
750789
}
751790

752791
#[test]
753792
fn test_variable_declaration_default() {
754-
assert_eq!(
755-
check_parse!(variable_declaration, b"$y: some value !default;"),
793+
match check_parse!(variable_declaration, b"$y: value !default;") {
756794
Item::VariableDeclaration {
757-
name: "y".into(),
758-
val: Value::List(
759-
vec![string("some"), string("value")],
760-
Some(ListSeparator::Space),
761-
false,
762-
),
763-
default: true,
764-
global: false,
765-
},
766-
)
795+
name,
796+
val,
797+
default,
798+
global,
799+
pos: _,
800+
} => {
801+
assert_eq!(
802+
(name, val, default, global),
803+
("y".into(), string("value"), true, false,)
804+
)
805+
}
806+
_ => panic!(),
807+
}
767808
}

Diff for: ‎src/sass/formal_args.rs

+10-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use super::{Call, Name, Value};
2-
use crate::{css, Error, ScopeRef, SourcePos};
2+
use crate::{css, Error, ScopeError, ScopeRef, SourcePos};
33
use std::fmt;
44

55
/// The declared arguments of a mixin or function declaration.
@@ -69,17 +69,17 @@ impl FormalArgs {
6969
}
7070
let positional = args.take_positional(self.0.len());
7171
for ((name, _default), value) in self.0.iter().zip(&positional) {
72-
argscope.define(name.clone(), value.clone());
72+
argscope.define(name.clone(), value.clone())?;
7373
}
7474
if self.0.len() > positional.len() {
7575
for (name, default) in &self.0[positional.len()..] {
7676
if let Some(v) = args.named.remove(name) {
77-
argscope.define(name.clone(), v);
77+
argscope.define(name.clone(), v)?;
7878
} else if let Some(default) = default {
7979
argscope.define(
8080
name.clone(),
8181
default.do_evaluate(argscope.clone(), true)?,
82-
);
82+
)?;
8383
} else {
8484
return Err(ArgsError::Missing(name.clone()));
8585
}
@@ -89,7 +89,7 @@ impl FormalArgs {
8989
argscope.define(
9090
va_name.clone(),
9191
args.only_named(va_name).unwrap_or_else(|| args.into()),
92-
);
92+
)?;
9393
} else {
9494
args.check_no_named()?;
9595
}
@@ -189,6 +189,11 @@ impl From<Error> for ArgsError {
189189
ArgsError::Eval(Box::new(e))
190190
}
191191
}
192+
impl From<ScopeError> for ArgsError {
193+
fn from(e: ScopeError) -> ArgsError {
194+
Error::from(e).into()
195+
}
196+
}
192197

193198
// Note: this is only for some special cases, normally the "context"
194199
// of a function declaration pos is required.

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

+4-2
Original file line numberDiff line numberDiff line change
@@ -195,8 +195,10 @@ pub fn create_module() -> Scope {
195195
}
196196
});
197197

198-
f.set_variable(name!(pi), Value::scalar(PI), false, false);
199-
f.set_variable(name!(e), Value::scalar(E), false, false);
198+
f.set_variable(name!(pi), Value::scalar(PI), false, false)
199+
.unwrap();
200+
f.set_variable(name!(e), Value::scalar(E), false, false)
201+
.unwrap();
200202
f
201203
}
202204

Diff for: ‎src/sass/item.rs

+2
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ pub enum Item {
1818
default: bool,
1919
/// true if this is a `!global` variable.
2020
global: bool,
21+
/// The source location of this variable declaration.
22+
pos: SourcePos,
2123
},
2224

2325
/// An `@at-root` directive.

Diff for: ‎src/sass/mixin.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ impl MixinDecl {
107107
let scope = ScopeRef::sub(scope);
108108
if let Some(with) = with {
109109
for (key, value) in with.into_iter() {
110-
scope.define(key.into(), value);
110+
scope.define(key.into(), value)?;
111111
}
112112
}
113113
Ok(Mixin {

0 commit comments

Comments
 (0)