Add filter flag checking (#165)

* Add filter flag checking

* trait

* update test

src/dreamchecker/lib.rs

@@ -248,6 +248,18 @@ impl WithFixHint for DMError {
     }
 }
 
+trait WithFilterArgs {
+    fn with_filter_args(self, loc: Location, filtertype: &str) -> Self;
+}
+
+impl WithFilterArgs for DMError {
+    fn with_filter_args(mut self, loc: Location, filtertype: &str) -> Self {
+        // luckily lummox has made the anchor url match the type= value for each filter
+        self.add_note(loc, format!("See: http://www.byond.com/docs/ref/#/{{notes}}/filters/{} for the permitted arguments", filtertype));
+        self
+    }
+}
+
 /// Build an analysis from an assumption set.
 impl<'o> From<AssumptionSet<'o>> for Analysis<'o> {
     fn from(aset: AssumptionSet<'o>) -> Analysis<'o> {
@@ -1701,6 +1713,49 @@ impl<'o, 's> AnalyzeProc<'o, 's> {
         Analysis::empty()
     }
 
+    fn check_filter_flag(&mut self, expr: &'o Expression, can_be_zero: bool, location: Location, typevalue: &str, valid_flags: &[&str], flagfieldname: &str, exclusive: bool) {
+        match expr {
+            Expression::BinaryOp{ op: BinaryOp::BitOr, lhs, rhs } => {
+                if exclusive {
+                    error(location, format!("filter(type=\"{}\") '{}' parameter must have one value, found bitwise OR", typevalue, flagfieldname))
+                        .with_filter_args(location, typevalue)
+                        .register(self.context);
+                    return
+                }
+                // recurse
+                self.check_filter_flag(lhs, can_be_zero, location, typevalue, valid_flags, flagfieldname, exclusive);
+                self.check_filter_flag(rhs, can_be_zero, location, typevalue, valid_flags, flagfieldname, exclusive);
+            },
+            Expression::Base{ unary, term, follow: _ } => {
+                if unary.len() > 0 {
+                    error(location, "filter() flag fields cannot have unary ops")
+                        .register(self.context);
+                    return
+                }
+                match &term.elem {
+                    Term::Ident(flagname) => {
+                        if valid_flags.iter().position(|&x| x == flagname).is_none() {
+                            error(location, format!("filter(type=\"{}\") called with invalid '{}' flag '{}'", typevalue, flagfieldname, flagname))
+                                .with_filter_args(location, typevalue)
+                                .register(self.context);
+                        }
+                    },
+                    Term::Int(0) if can_be_zero => {},
+                    other => {
+                        error(location, format!("filter(type=\"{}\") called with invalid '{}' value '{:?}'", typevalue, flagfieldname, other))
+                            .with_filter_args(location, typevalue)
+                            .register(self.context);
+                    },
+                }
+            },
+            _ => {
+                error(location, format!("filter(type=\"{}\"), extremely invalid value passed to '{}' field", typevalue, flagfieldname))
+                    .with_filter_args(location, typevalue)
+                    .register(self.context);
+            }
+        }
+    }
+
     fn visit_call(&mut self, location: Location, src: TypeRef<'o>, proc: ProcRef<'o>, args: &'o [Expression], is_exact: bool, local_vars: &mut HashMap<String, LocalVar<'o>>) -> Analysis<'o> {
         if let Some(callhashset) = self.env.call_tree.get_mut(&self.proc_ref) {
             callhashset.push((proc, location));
@@ -1718,6 +1773,7 @@ impl<'o, 's> AnalyzeProc<'o, 's> {
         let mut any_kwargs_yet = false;
 
         let mut param_name_map = HashMap::new();
+        let mut param_expr_map = HashMap::new();
         let mut param_idx_map = HashMap::new();
         let mut param_idx = 0;
         let mut arglist_used = false;
@@ -1790,16 +1846,15 @@ impl<'o, 's> AnalyzeProc<'o, 's> {
             let analysis = self.visit_expression(location, argument_value, None, local_vars);
             if let Some(kw) = this_kwarg {
                 param_name_map.insert(kw.as_str(), analysis);
+                param_expr_map.insert(kw.as_str(), argument_value);
             } else {
                 param_idx_map.insert(param_idx, analysis);
                 param_idx += 1;
             }
         }
 
         // filter call checking
-        // TODO: check flags for valid values
-        //  eg "wave" type "flags" param only works with WAVE_SIDEWAYS, WAVE_BOUND
-        // also some filters have limits for their numerical params
+        // TODO: some filters have limits for their numerical params
         //  eg "rays" type "threshold" param defaults to 0.5, can be 0 to 1
         if proc.ty().is_root() && proc.name() == "filter" {
             guard!(let Some(typename) = param_name_map.get("type") else {
@@ -1822,11 +1877,15 @@ impl<'o, 's> AnalyzeProc<'o, 's> {
             for arg in param_name_map.keys() {
                 if *arg != "type" && arglist.iter().position(|&x| x == *arg).is_none() {
                     error(location, format!("filter(type=\"{}\") called with invalid keyword parameter '{}'", typevalue, arg))
-                        // luckily lummox has made the anchor url match the type= value for each filter
-                        .with_note(location, format!("See: http://www.byond.com/docs/ref/#/{{notes}}/filters/{} for the permitted arguments", typevalue))
+                        .with_filter_args(location, typevalue)
                         .register(self.context);
                 }
             }
+            if let Some((flagfieldname, exclusive, can_be_zero, valid_flags)) = VALID_FILTER_FLAGS.get(typevalue.as_str()) {
+                if let Some(flagsvalue) = param_expr_map.get(flagfieldname) {
+                    self.check_filter_flag(flagsvalue, *can_be_zero, location, typevalue, valid_flags, flagfieldname, *exclusive);
+                }
+            }
         }
 
         if proc.ty().is_root() && proc.is_builtin() {

src/dreamchecker/tests/kwargs_tests.rs

@@ -18,6 +18,7 @@ fn after_kwarg() {
 }
 
 pub const FILTER_KWARGS_ERRORS: &[(u32, u16, &str)] = &[
+    (4, 5, "filter(type=\"color\") called with invalid 'space' value 'Null'"),
     (15, 5, "filter(type=\"alpha\") called with invalid keyword parameter 'color'"),
     (16, 5, "filter(type=\"blur\") called with invalid keyword parameter 'x'"),
     (17, 5, "filter() called with invalid type keyword parameter value 'fakename'"),
@@ -47,7 +48,7 @@ fn filter_kwarg() {
     filter(type="blur", x=1)
     filter(type="fakename", x=3)
     filter(x=4)
-    filter("alpha", x=1)
+    filter("alpha", x=1, flags=MASK_INVERSE|MASK_INVERSE|MASK_INVERSE|MASK_INVERSE|MASK_INVERSE|MASK_INVERSE)
     filter(type="wave", color=null)
 "##.trim();
     check_errors_match(code, FILTER_KWARGS_ERRORS);

src/dreammaker/ast.rs

@@ -930,3 +930,13 @@ pub static VALID_FILTER_TYPES: phf::Map<&'static str, &[&str]> = phf_map! {
     "ripple" => &[ "x", "y", "size", "repeat", "radius", "falloff", "flags" ],
     "wave" => &[ "x", "y", "size", "offset", "flags" ],
 };
+
+// filter type => (flag field name, exclusive, can_be_0, valid flag values)
+pub static VALID_FILTER_FLAGS: phf::Map<&'static str, (&str, bool, bool, &[&str])> = phf_map! {
+    "alpha" => ("flags", false, true, &[ "MASK_INVERSE", "MASK_SWAP" ]),
+    "color" => ("space", true, false, &[ "FILTER_COLOR_RGB", "FILTER_COLOR_HSV", "FILTER_COLOR_HSL", "FILTER_COLOR_HCY" ]),
+    "layer" => ("flags", true, true, &[ "FLAG_OVERLAY", "FLAG_UNDERLAY" ]),
+    "rays" => ("flags", false, true, &[ "FLAG_OVERLAY", "FLAG_UNDERLAY" ]),
+    "ripple" => ("flags", false, true, &[ "WAVE_BOUND" ]),
+    "wave" => ("flags", false, true, &[ "WAVE_SIDEWAYS", "WAVE_BOUND" ]),
+};

src/dreammaker/builtins.rs

@@ -158,23 +158,6 @@ pub fn default_defines(defines: &mut DefineMap) {
         DATABASE_ROW_LIST = Int(18);
 
         // 513 stuff
-        // alpha mask filter
-        MASK_INVERSE = Int(1);
-        MASK_SWAP = Int(2);
-
-        // rgb filter
-        FILTER_COLOR_RGB = Int(0);
-        FILTER_COLOR_HSV = Int(1);
-        FILTER_COLOR_HSL = Int(2);
-        FILTER_COLOR_HCY = Int(3);
-
-        // layering / ray filter
-        FILTER_OVERLAY = Int(1);
-        FILTER_UNDERLAY = Int(2);
-
-        // wave filter / ripple filter
-        WAVE_SIDEWAYS = Int(1);
-        WAVE_BOUNDED = Int(2);
 
         // vis_flags
         VIS_INHERIT_ICON = Int(1);
@@ -326,6 +309,24 @@ pub fn register_builtins(tree: &mut ObjectTree) -> Result<(), DMError> {
         // this is just a procstyle syntax wrapper for \ref[foo]
         proc/ref(A);
 
+        // alpha mask filter
+        var/const/MASK_INVERSE = int!(1);
+        var/const/MASK_SWAP = int!(2);
+
+        // rgb filter
+        var/const/FILTER_COLOR_RGB = int!(0);
+        var/const/FILTER_COLOR_HSV = int!(1);
+        var/const/FILTER_COLOR_HSL = int!(2);
+        var/const/FILTER_COLOR_HCY = int!(3);
+
+        // layering / ray filter
+        var/const/FILTER_OVERLAY = int!(1);
+        var/const/FILTER_UNDERLAY = int!(2);
+
+        // wave filter / ripple filter
+        var/const/WAVE_SIDEWAYS = int!(1);
+        var/const/WAVE_BOUNDED = int!(2);
+
         // global procs
         proc/abs(A);
         proc/addtext(Arg1, Arg2/*, ...*/);