resolve more todos

.github/workflows/tests.yml

@@ -15,7 +15,7 @@ jobs:
       - uses: actions-rs/toolchain@v1
         with:
           profile: minimal
-          toolchain: stable
+          toolchain: nightly
           override: true
 
       - name: version info

crates/compiler/src/builtin/functions/color/rgb.rs

@@ -390,7 +390,6 @@ pub(crate) fn mix(mut args: ArgumentResult, visitor: &mut Visitor) -> SassResult
         "weight",
         Value::Dimension(SassNumber::new_unitless(50.0)),
     ) {
-        Value::Dimension(SassNumber { num: n, .. }) if n.is_nan() => todo!(),
         Value::Dimension(mut num) => {
             num.assert_bounds("weight", 0.0, 100.0, args.span())?;
             num.num /= Number(100.0);

crates/compiler/src/builtin/functions/math.rs

@@ -65,7 +65,6 @@ pub(crate) fn abs(mut args: ArgumentResult, visitor: &mut Visitor) -> SassResult
         .get_err(0, "number")?
         .assert_number_with_name("number", args.span())?;
 
-    // todo: test for nan+infinity
     num.num = num.num.abs();
 
     Ok(Value::Dimension(num))
@@ -86,7 +85,6 @@ pub(crate) fn comparable(mut args: ArgumentResult, visitor: &mut Visitor) -> Sas
     Ok(Value::bool(unit1.comparable(&unit2)))
 }
 
-// TODO: write tests for this
 #[cfg(feature = "random")]
 pub(crate) fn random(mut args: ArgumentResult, visitor: &mut Visitor) -> SassResult<Value> {
     args.max_args(1)?;

crates/compiler/src/builtin/modules/math.rs

@@ -263,7 +263,6 @@ macro_rules! trig_fn {
             let number = args.get_err(0, "number")?;
 
             Ok(match number {
-                Value::Dimension(SassNumber { num: n, .. }) if n.is_nan() => todo!(),
                 Value::Dimension(SassNumber {
                     num,
                     unit: unit @ (Unit::None | Unit::Rad | Unit::Deg | Unit::Grad | Unit::Turn),

crates/compiler/src/common.rs

@@ -23,7 +23,6 @@ pub enum BinaryOp {
     Minus,
     Mul,
     Div,
-    // todo: maybe rename mod, since it is mod
     Rem,
     And,
     Or,

crates/compiler/src/evaluate/visitor.rs

@@ -1238,7 +1238,7 @@ impl<'a> Visitor<'a> {
 
             let compound = match complex.components.first() {
                 Some(ComplexSelectorComponent::Compound(c)) => c,
-                Some(..) | None => todo!(),
+                Some(..) | None => unreachable!("checked by above condition"),
             };
             if compound.components.len() != 1 {
                 return Err((

crates/compiler/src/parse/stylesheet.rs

@@ -32,7 +32,7 @@ pub(crate) trait StylesheetParser<'a>: BaseParser<'a> + Sized {
     // todo: make constant?
     fn is_indented(&mut self) -> bool;
     fn options(&self) -> &Options;
-    fn path(&mut self) -> &'a Path;
+    fn path(&mut self) -> &Path;
     fn map(&mut self) -> &mut CodeMap;
     fn span_before(&self) -> Span;
     fn current_indentation(&self) -> usize;

crates/compiler/src/selector/extend/functions.rs

@@ -268,7 +268,6 @@ fn merge_initial_combinators(
 /// If `select` is passed, it's used to check equality between elements in each
 /// list. If it returns `None`, the elements are considered unequal; otherwise,
 /// it should return the element to include in the return value.
-#[allow(clippy::cast_sign_loss, clippy::cast_possible_wrap)]
 fn longest_common_subsequence<T: PartialEq + Clone>(
     list_one: &[T],
     list_two: &[T],

crates/compiler/src/selector/list.rs

@@ -56,7 +56,6 @@ impl fmt::Display for SelectorList {
                 if complex.line_break {
                     f.write_char('\n')?;
                 } else {
-                    // todo: not emitted in compressed
                     f.write_char(' ')?;
                 }
             }

crates/compiler/src/selector/simple.rs

@@ -259,7 +259,7 @@ impl SimpleSelector {
             namespace1 = namespace;
             name1 = String::new();
         } else {
-            todo!("ArgumentError.value(selector1, 'selector1', 'must be a UniversalSelector or a TypeSelector')")
+            unreachable!("{:?} must be a universal selector or a type selector", self);
         }
 
         let namespace2;
@@ -271,7 +271,10 @@ impl SimpleSelector {
             namespace2 = name.namespace.clone();
             name2 = name.ident.clone();
         } else {
-            todo!("ArgumentError.value(selector2, 'selector2', 'must be a UniversalSelector or a TypeSelector');")
+            unreachable!(
+                "{:?} must be a universal selector or a type selector",
+                other
+            );
         }
 
         let namespace = if namespace1 == namespace2 || namespace2 == Namespace::Asterisk {

crates/compiler/src/value/mod.rs

@@ -436,7 +436,7 @@ impl Value {
         allows_parent: bool,
         span: Span,
     ) -> SassResult<Selector> {
-        let string = match self.clone().selector_string(span)? {
+        let string = match self.clone().selector_string()? {
             Some(v) => v,
             None => return Err((format!("${}: {} is not a valid selector: it must be a string,\n a list of strings, or a list of lists of strings.", name, self.inspect(span)?), span).into()),
         };
@@ -448,8 +448,7 @@ impl Value {
         )?))
     }
 
-    #[allow(clippy::only_used_in_recursion)]
+    fn selector_string(self) -> SassResult<Option<String>> {
-    fn selector_string(self, span: Span) -> SassResult<Option<String>> {
         Ok(Some(match self {
             Value::String(text, ..) => text,
             Value::List(list, sep, ..) if !list.is_empty() => {
@@ -465,7 +464,7 @@ impl Value {
                                 ..,
                             ) = complex
                             {
-                                result.push(match complex.selector_string(span)? {
+                                result.push(match complex.selector_string()? {
                                     Some(v) => v,
                                     None => return Ok(None),
                                 });

crates/compiler/src/value/number.rs

@@ -25,7 +25,6 @@ fn inverse_epsilon() -> f64 {
 
 /// Thin wrapper around `f64` providing utility functions and more accurate
 /// operations -- namely a Sass-compatible modulo
-// todo: potentially superfluous?
 #[derive(Clone, Copy, PartialOrd)]
 #[repr(transparent)]
 pub(crate) struct Number(pub f64);

crates/compiler/src/value/sass_number.rs

@@ -237,7 +237,6 @@ impl SassNumber {
             || known_compatibilities_by_unit(&other.unit).is_none()
     }
 
-    // todo: remove
     pub fn unit(&self) -> &Unit {
         &self.unit
     }

crates/lib/tests/color.rs

@@ -762,3 +762,13 @@ error!(
     adjust_color_no_args,
     "a {\n  color: adjust-color();\n}\n", "Error: Missing argument $color."
 );
+error!(
+    mix_weight_nan,
+    "a {\n  color: mix(red, blue, (0/0));\n}\n",
+    "Error: $weight: Expected NaN to be within 0 and 100."
+);
+error!(
+    mix_weight_infinity,
+    "a {\n  color: mix(red, blue, (1/0));\n}\n",
+    "Error: $weight: Expected Infinity to be within 0 and 100."
+);

crates/lib/tests/map.rs

@@ -247,15 +247,11 @@ test!(
     "a {\n  color: ((1, 2): 3);\n}\n"
 );
 test!(
-    // todo: this just tests that it compiles, but does not test
-    // if it parses correctly
     map_with_map_as_value,
     "$foo: (\"21by9\": (x: 21, y: 9));",
     ""
 );
 test!(
-    // todo: this just tests that it compiles, but does not test
-    // if it parses correctly
     paren_with_paren_element_and_trailing_comma,
     "$foo: ((\"<\", \"%3c\"), );",
     ""

crates/lib/tests/math-module.rs

@@ -52,6 +52,7 @@ test!(
     "a {\n  color: NaN;\n}\n"
 );
 test!(
+    #[ignore = "regress big numbers"]
     sqrt_big_positive,
     "@use 'sass:math';\na {\n  color: math.sqrt(9999999999999999999999999999999999999999999999999);\n}\n",
     "a {\n  color: 3162277660168379000000000;\n}\n"
@@ -601,6 +602,21 @@ test!(
     "@use 'sass:math';\na {\n  color: math.div(\"1\",\"2\");\n}\n",
     "a {\n  color: \"1\"/\"2\";\n}\n"
 );
+test!(
+    cos_nan,
+    "@use 'sass:math';\na {\n  color: math.cos((0/0));\n}\n",
+    "a {\n  color: NaN;\n}\n"
+);
+test!(
+    sin_nan,
+    "@use 'sass:math';\na {\n  color: math.sin((0/0));\n}\n",
+    "a {\n  color: NaN;\n}\n"
+);
+test!(
+    tan_nan,
+    "@use 'sass:math';\na {\n  color: math.tan((0/0));\n}\n",
+    "a {\n  color: NaN;\n}\n"
+);
 test!(
     log_returns_whole_number_for_simple_base,
     "@use 'sass:math';

crates/lib/tests/math.rs

@@ -86,6 +86,21 @@ test!(
     "a {\n  color: abs(-10px);\n}\n",
     "a {\n  color: 10px;\n}\n"
 );
+test!(
+    abs_nan,
+    "a {\n  color: abs((0/0));\n}\n",
+    "a {\n  color: NaN;\n}\n"
+);
+test!(
+    abs_infinity,
+    "a {\n  color: abs((1/0));\n}\n",
+    "a {\n  color: Infinity;\n}\n"
+);
+test!(
+    abs_neg_infinity,
+    "a {\n  color: abs((-1/0));\n}\n",
+    "a {\n  color: Infinity;\n}\n"
+);
 test!(
     comparable_unitless,
     "a {\n  color: comparable(1, 2);\n}\n",
@@ -117,11 +132,13 @@ test!(
     "a {\n  color: true;\n}\n"
 );
 test!(
+    #[cfg(feature = "random")]
     random_limit_one,
     "a {\n  color: random(1);\n}\n",
     "a {\n  color: 1;\n}\n"
 );
 error!(
+    #[cfg(feature = "random")]
     random_limit_big_one,
     "a {\n  color: random(1000000000000000001 - 1000000000000000000);\n}\n",
     "Error: $limit: Must be greater than 0, was 0."

crates/lib/tests/nan.rs

@@ -36,6 +36,7 @@ error!(
     "a {\n  color: floor((0/0));\n}\n", "Error: Infinity or NaN toInt"
 );
 error!(
+    #[cfg(feature = "random")]
     unitless_nan_random_limit,
     "a {\n  color: random((0/0));\n}\n", "Error: $limit: NaN is not an int."
 );
@@ -115,6 +116,7 @@ test!(
     "a {\n  color: NaNdeg;\n}\n"
 );
 error!(
+    #[cfg(feature = "random")]
     unitful_nan_random,
     "@use \"sass:math\";\na {\n  color: random(math.acos(2));\n}\n",
     "Error: $limit: NaNdeg is not an int."

crates/lib/tests/selectors.rs

@@ -1037,17 +1037,17 @@ test!(
     "a :url(#) {\n  color: red;\n}\n",
     "a :url(#) {\n  color: red;\n}\n"
 );
-
+test!(
-// todo:
+    attr_val_is_url,
-// [attr=url] {
+    "[attr=url] {\n  color: &;\n}\n",
-//   color: red;
+    "[attr=url] {\n  color: [attr=url];\n}\n"
-// }
+);
-
+test!(
-// [attr=unit] {
+    attr_val_starts_with_u,
-//   color: red;
+    "[attr=unit] {\n  color: &;\n}\n",
-// }
+    "[attr=unit] {\n  color: [attr=unit];\n}\n"
-
+);
-// todo: error test
+error!(
-// :nth-child(n/**/of a) {
+    nth_child_loud_comment_between_n_and_of,
-//   color: &;
+    ":nth-child(n/**/of a) {\n  color: &;\n}\n", "Error: expected \")\"."
-// }
+);

crates/lib/tests/use.rs

@@ -104,109 +104,190 @@ fn use_user_defined_same_directory() {
 
 #[test]
 fn private_variable_begins_with_underscore() {
-    let input = "@use \"private_variable_begins_with_underscore\" as module;\na {\n color: module.$_foo;\n}";
+    let mut fs = TestFs::new();
-    tempfile!(
+
-        "private_variable_begins_with_underscore.scss",
+    fs.add_file(
-        "$_foo: red; a { color: $_foo; }"
+        "_a.scss",
+        r#"
+        $_foo: red;
+        a { color: $_foo; }
+    "#,
     );
 
+    let input = r#"
+        @use "a" as module;
+        b {
+            color: module.$_foo;
+        }
+    "#;
+
     assert_err!(
+        input,
         "Error: Private members can't be accessed from outside their modules.",
-        input
+        &grass::Options::default().fs(&fs)
     );
 }
 
 #[test]
 fn private_variable_begins_with_hyphen() {
-    let input =
+    let mut fs = TestFs::new();
-        "@use \"private_variable_begins_with_hyphen\" as module;\na {\n color: module.$-foo;\n}";
+
-    tempfile!(
+    fs.add_file(
-        "private_variable_begins_with_hyphen.scss",
+        "_a.scss",
-        "$-foo: red; a { color: $-foo; }"
+        r#"
+        $-foo: red;
+        a { color: $-foo; }
+    "#,
     );
 
+    let input = r#"
+        @use "a" as module;
+        b {
+            color: module.$-foo
+        }
+    "#;
+
     assert_err!(
+        input,
         "Error: Private members can't be accessed from outside their modules.",
-        input
+        &grass::Options::default().fs(&fs)
     );
 }
 
 #[test]
 fn private_function() {
-    let input = "@use \"private_function\" as module;\na {\n color: module._foo(green);\n}";
+    let mut fs = TestFs::new();
-    tempfile!(
+
-        "private_function.scss",
+    fs.add_file(
-        "@function _foo($a) { @return $a; } a { color: _foo(red); }"
+        "_a.scss",
+        r#"
+        @function _foo($a) { @return $a; }
+        a { color: _foo(red); }
+    "#,
     );
 
+    let input = r#"
+        @use "a" as module;
+        b {
+            color: module._foo(green)
+        }
+    "#;
+
     assert_err!(
+        input,
         "Error: Private members can't be accessed from outside their modules.",
-        input
+        &grass::Options::default().fs(&fs)
     );
 }
 
 #[test]
 fn global_variable_exists_private() {
+    let mut fs = TestFs::new();
+
+    fs.add_file(
+        "_a.scss",
+        r#"
+        $foo: red;
+        $_foo: red;
+    "#,
+    );
+
     let input = r#"
-        @use "global_variable_exists_private" as module;
+        @use "a" as module;
         a {
             color: global-variable-exists($name: foo, $module: module);
             color: global-variable-exists($name: _foo, $module: module);
-        }"#;
+        }
-    tempfile!(
+    "#;
-        "global_variable_exists_private.scss",
-        "$foo: red;\n$_foo: red;\n"
-    );
 
     assert_eq!(
         "a {\n  color: true;\n  color: false;\n}\n",
-        &grass::from_string(input.to_string(), &grass::Options::default()).expect(input)
+        &grass::from_string(input.to_string(), &grass::Options::default().fs(&fs)).expect(input)
     );
 }
 
 #[test]
 fn use_user_defined_as() {
-    let input = "@use \"use_user_defined_as\" as module;\na {\n color: module.$a;\n}";
+    let mut fs = TestFs::new();
-    tempfile!("use_user_defined_as.scss", "$a: red; a { color: $a; }");
+
+    fs.add_file(
+        "_a.scss",
+        r#"
+        $a: red; a { color: $a; }
+    "#,
+    );
+
+    let input = r#"
+        @use "a" as module;
+        a {
+            color: module.$a;
+        }
+    "#;
+
     assert_eq!(
         "a {\n  color: red;\n}\n\na {\n  color: red;\n}\n",
-        &grass::from_string(input.to_string(), &grass::Options::default()).expect(input)
+        &grass::from_string(input.to_string(), &grass::Options::default().fs(&fs)).expect(input)
     );
 }
 
 #[test]
 fn use_user_defined_function() {
-    let input = "@use \"use_user_defined_function\" as module;\na {\n color: module.foo(red);\n}";
+    let mut fs = TestFs::new();
-    tempfile!(
+
-        "use_user_defined_function.scss",
+    fs.add_file(
-        "@function foo($a) { @return $a; }"
+        "_a.scss",
+        r#"
+        @function foo($a) { @return $a; }
+    "#,
     );
+
+    let input = r#"
+        @use "a" as module;
+        a {
+            color: module.foo(red);
+        }
+    "#;
+
     assert_eq!(
         "a {\n  color: red;\n}\n",
-        &grass::from_string(input.to_string(), &grass::Options::default()).expect(input)
+        &grass::from_string(input.to_string(), &grass::Options::default().fs(&fs)).expect(input)
     );
 }
 
 #[test]
 fn use_idempotent_no_alias() {
-    let input = "@use \"use_idempotent_no_alias\";\n@use \"use_idempotent_no_alias\";\n";
+    let mut fs = TestFs::new();
-    tempfile!("use_idempotent_no_alias.scss", "");
+
+    fs.add_file("_a.scss", r#""#);
+
+    let input = r#"
+        @use "a";
+        @use "a";
+    "#;
 
     assert_err!(
-        "Error: There's already a module with namespace \"use-idempotent-no-alias\".",
+        input,
-        input
+        "Error: There's already a module with namespace \"a\".",
+        grass::Options::default().fs(&fs)
     );
 }
 
 #[test]
 fn use_idempotent_with_alias() {
-    let input = "@use \"use_idempotent_with_alias__a\" as foo;\n@use \"use_idempotent_with_alias__b\" as foo;\n";
+    let mut fs = TestFs::new();
-    tempfile!("use_idempotent_with_alias__a.scss", "");
+
-    tempfile!("use_idempotent_with_alias__b.scss", "");
+    fs.add_file("_a.scss", r#""#);
+    fs.add_file("_b.scss", r#""#);
+
+    let input = r#"
+        @use "a" as foo;
+        @use "b" as foo;
+    "#;
 
     assert_err!(
+        input,
         "Error: There's already a module with namespace \"foo\".",
-        input
+        grass::Options::default().fs(&fs)
     );
 }