Allow invalid variant in rules

Author: msmans

src/feafea/__init__.py

@@ -824,15 +824,11 @@ def from_dict(c: DictConfig) -> CompiledConfig:
 
             referenced_variants = set((flag, variant) for flag, variant in r.get("variants", {}).items())
 
-            # Remove any referenced variants that are not defined in the config.
-            referenced_variants = {rv for rv in referenced_variants if rv[0] in flags}
-
-            if referenced_variants - all_variants:
-                raise ValueError(f"unknown flag/variant in rule {rule_name}")
+            valid_variants = referenced_variants.intersection(all_variants)
 
             # Rule compilation.
 
-            py_flag = {f: [] for f in set(flag for flag, _ in referenced_variants)}
+            py_flag = {f: [] for f in set(flag for flag, _ in valid_variants)}
             py_common = []
 
             globals: dict[str, Any] = {
@@ -869,7 +865,8 @@ def from_dict(c: DictConfig) -> CompiledConfig:
             # Compile the flag dependent rules.
 
             for flag, variant in r.get("variants", {}).items():
-                if flag in py_flag:  # Only process flags that exist in py_flag dict
+                # Only process flag/variant combo that's valid
+                if (flag, variant) in valid_variants:
                     py_flag[flag].append(f"return {variant!r}")
 
             for flag, py_lines in py_flag.items():

tests/test_configs.py

@@ -139,6 +139,9 @@ def setUpClass(cls):
                     "gg": {
                         "alias": "g",
                     },
+                    "hh": {
+                        "default": False,
+                    }
                 },
                 "filters": {
                     "f1": "attr:at1 in [5,6,7]",
@@ -207,13 +210,25 @@ def setUpClass(cls):
                         "variants": {
                             "nonexistent": "apply",
                         }
+                    },
+                    "r11": {
+                        "filter": "attr:nonexistent",
+                        "variants": {
+                            "hh": True,
+                        },
+                    },
+                    "r12": {
+                        "filter": "attr:be_bad",
+                        "variants": {
+                            "hh": "badvariant",
+                        }
                     }
                 },
             },
         )
 
     def test_valid_config_high_level(self):
-        self.assertSetEqual(set(self._valid_config.flags), {"a", "b", "c", "cc", "d", "e", "f", "g", "h", "gg"})
+        self.assertSetEqual(set(self._valid_config.flags), {"a", "b", "c", "cc", "d", "e", "f", "g", "h", "gg", "hh"})
         self.assertDictEqual(self._valid_config.flags["a"].metadata, {"category": "simple", "deprecated": False, "revision": 22})
         self.assertEqual(self._valid_config.flags["a"].name, "a")
         self.assertTrue(self._valid_config.flags["a"].type is bool)
@@ -246,14 +261,15 @@ def test_valid_const_config(self):
             ("cc", {"at1": 5}, "const_rule", "r5", "med"),  # alias of c
             ("gg", {}, "const_rule", "r9", 9),
             ("g", {}, "const_rule", "r9", 9),  # setting rule against alias sets it against the original flag
+            ("hh", {"be_bad": True}, "default", "", False), # rule r11 has invalid variant
         ]
 
         for flag, attrs, reason, rule, variant in cases:
             with self.subTest(f"flag={flag}, attrs={attrs}"):
                 ev = self._valid_config.flags[flag].eval(attrs)
-                self.assertEqual(ev.variant, variant)
-                self.assertEqual(ev.reason, reason)
-                self.assertEqual(ev.rule, rule)
+                self.assertEqual(ev.variant, variant, msg=f"flag={flag}, attrs={attrs}")
+                self.assertEqual(ev.reason, reason, msg=f"flag={flag}, attrs={attrs}")
+                self.assertEqual(ev.rule, rule, msg=f"flag={flag}, attrs={attrs}")
 
 
 class TestInvalidConfigs(unittest.TestCase):
@@ -318,7 +334,6 @@ def test_invalid_rules_def(self):
 
         cases = [
             ({}, ValidationError, "."),
-            ({"variants": {"a": 4}}, ValueError, "unknown flag/variant"),  # non-existing variant
             ({"split_group": 12}, ValidationError, "."),
             ({"variants": {"a": 4}, "split_group": 12}, ValidationError, "."),  # cannot combine split_group with variants
             ({"metadata": "abc"}, ValidationError, "."),