Undefined variables results in the rule just not being outputted

[sunjay]

Thanks for working on this crate!

This is a case where I would expect an error to be produced, but instead I just get nothing at all.

Here's an example program with rsass 0.9.6 and Rust 1.32.0:

fn main() {
    println!("{:?}", rsass::compile_scss(b"h1 { color: #fff; }", rsass::OutputStyle::Expanded));
    println!("{:?}", rsass::compile_scss(b"h1 { color: $does-not-exist; }", rsass::OutputStyle::Expanded));
}

It prints:

Ok([104, 49, 32, 123, 10, 32, 32, 99, 111, 108, 111, 114, 58, 32, 35, 102, 102, 102, 59, 10, 125, 10])
Ok([])

Despite the $does-not-exist variable not being defined, I get no error. The rule just isn't produced.

I would really like it if errors like this were made part of the Error enum so I could handle them properly instead of being surprized when things suddenly don't work because of something like this. If ignoring malformed rules is expected behaviour, it would be nice to at least make that configurable in case that isn't desired.

[kaj]

The "not being printed" part is because sass considers a property being set to a null value being equivalent to a property not being set at all, and an empty rule should not be printed. So that part reflects the specification.

I agree that having an error returned when trying to use an undefined variable would be helpfull, but I'm afraid it would go against the sass specification. Or would it? The first step here should probably be to find an example of an undefined variable in the sass specification test suite, then we "just" have to make sure that rsass passes that test ...

Maybe the proper thing to do would be to print a warning?

[sunjay]

I found a couple of tests here: https://github.com/sass/sass-spec/search?q=undefined+variable

It does seem to be an error case when a variable ends up undefined.

[sunjay]

I also checked with several of the existing implementations and they all appear to produce errors for when a variable is not defined.

[kaj]

Great, then rsass should indeed also produce an error on use of an undefined variable. Do you want to write the changes?

Note: the automatic test-converter spectest.rs won't be able to convert these tests, as it don't handle any tests with expected errors. Instead, a manual test will have to be written.

[sunjay]

I would need some mentoring instructions to get started. :)

If we reach out to the broader community (and we have some instructions), someone may volunteer to help out. I'd be happy to help get the word out. :)

[glebm]

@sunjay I've given it a go at #35