Fix sleep and purity lints not counting proc overrides (#214)

Fixes #203.

src/dreamchecker/lib.rs

@@ -358,19 +358,20 @@ fn run_inner(context: &Context, objtree: &ObjectTree, cli: bool) {
 
     cli_println!("Procs analyzed: {}. Errored: {}. Builtins: {}.\n", present, invalid, builtin);
 
-    cli_println!("============================================================");
-    cli_println!("Analyzing proc call tree...\n");
-    analyzer.check_proc_call_tree();
-
     cli_println!("============================================================");
     cli_println!("Analyzing proc override validity...\n");
     objtree.root().recurse(&mut |ty| {
         for proc in ty.iter_self_procs() {
             analyzer.check_kwargs(proc);
+            analyzer.propagate_violations(proc);
         }
     });
 
     analyzer.finish_check_kwargs();
+
+    cli_println!("============================================================");
+    cli_println!("Analyzing proc call tree...\n");
+    analyzer.check_proc_call_tree();
 }
 
 // ----------------------------------------------------------------------------
@@ -489,13 +490,13 @@ impl<'o> CallStack<'o> {
 }
 
 trait DMErrorExt {
-    fn with_callstack(self, stack: CallStack) -> Self;
+    fn with_callstack(self, stack: &CallStack) -> Self;
     fn with_blocking_builtins(self, blockers: &Vec<(String, Location)>) -> Self;
     fn with_impure_operations(self, impures: &Vec<(String, Location)>) -> Self;
 }
 
 impl DMErrorExt for DMError {
-    fn with_callstack(mut self, stack: CallStack) -> DMError {
+    fn with_callstack(mut self, stack: &CallStack) -> DMError {
         for (procref, location, new_context) in stack.call_stack.iter() {
             self.add_note(*location, format!("{}() called here", procref));
         }
@@ -532,6 +533,21 @@ impl<'o> ViolatingProcs<'o> {
     }
 }
 
+#[derive(Default, Debug)]
+pub struct ViolatingOverrides<'o> {
+    overrides: HashMap<ProcRef<'o>, Vec<ProcRef<'o>>>,
+}
+
+impl<'o> ViolatingOverrides<'o> {
+    pub fn insert_override(&mut self, proc: ProcRef<'o>, child: ProcRef<'o>) {
+        self.overrides.entry(proc).or_default().push(child);
+    }
+
+    pub fn get_override_violators(&self, proc: ProcRef<'o>) -> Option<&Vec<ProcRef<'o>>> {
+        self.overrides.get(&proc)
+    }
+}
+
 /// A deeper analysis of an ObjectTree
 pub struct AnalyzeObjectTree<'o> {
     context: &'o Context,
@@ -554,6 +570,9 @@ pub struct AnalyzeObjectTree<'o> {
     sleeping_procs: ViolatingProcs<'o>,
     impure_procs: ViolatingProcs<'o>,
     waitfor_procs: HashSet<ProcRef<'o>>,
+
+    sleeping_overrides: ViolatingOverrides<'o>,
+    impure_overrides: ViolatingOverrides<'o>,
 }
 
 impl<'o> AnalyzeObjectTree<'o> {
@@ -578,6 +597,8 @@ impl<'o> AnalyzeObjectTree<'o> {
             sleeping_procs: Default::default(),
             impure_procs: Default::default(),
             waitfor_procs: Default::default(),
+            sleeping_overrides: Default::default(),
+            impure_overrides: Default::default(),
         }
     }
 
@@ -665,9 +686,21 @@ impl<'o> AnalyzeObjectTree<'o> {
                     error(procref.get().location, format!("{} sets SpacemanDMM_should_not_sleep but calls blocking proc {}", procref, nextproc))
                         .with_note(location, "SpacemanDMM_should_not_sleep set here")
                         .with_errortype("must_not_sleep")
-                        .with_callstack(callstack)
+                        .with_callstack(&callstack)
                         .with_blocking_builtins(sleepvec)
                         .register(self.context)
+                } else if let Some(overridesleep) = self.sleeping_overrides.get_override_violators(nextproc) {
+                    for child_violator in overridesleep {
+                        if procref.ty().is_subtype_of(&nextproc.ty()) && !child_violator.ty().is_subtype_of(&procref.ty()) {
+                            continue
+                        }
+                        error(procref.get().location, format!("{} calls {} which has override child proc that sleeps {}", procref, nextproc, child_violator))
+                            .with_note(location, "SpacemanDMM_should_not_sleep set here")
+                            .with_errortype("must_not_sleep")
+                            .with_callstack(&callstack)
+                            .with_blocking_builtins(self.sleeping_procs.get_violators(*child_violator).unwrap())
+                            .register(self.context)
+                    }
                 } else if let Some(calledvec) = self.call_tree.get(&nextproc) {
                     for (proccalled, location, new_context) in calledvec.iter() {
                         let mut newstack = callstack.clone();
@@ -703,9 +736,21 @@ impl<'o> AnalyzeObjectTree<'o> {
                     error(procref.get().location, format!("{} sets SpacemanDMM_should_be_pure but calls a {} that does impure operations", procref, nextproc))
                         .with_note(*location, "SpacemanDMM_should_be_pure set here")
                         .with_errortype("must_be_pure")
-                        .with_callstack(callstack)
+                        .with_callstack(&callstack)
                         .with_impure_operations(impurevec)
                         .register(self.context)
+                } else if let Some(overrideimpure) = self.impure_overrides.get_override_violators(nextproc) {
+                    for child_violator in overrideimpure {
+                        if procref.ty().is_subtype_of(&nextproc.ty()) && !child_violator.ty().is_subtype_of(&procref.ty()) {
+                            continue
+                        }
+                        error(procref.get().location, format!("{} calls {} which has override child proc that does impure operations {}", procref, nextproc, child_violator))
+                            .with_note(*location, "SpacemanDMM_should_not_pure set here")
+                            .with_errortype("must_be_pure")
+                            .with_callstack(&callstack)
+                            .with_blocking_builtins(self.impure_procs.get_violators(*child_violator).unwrap())
+                            .register(self.context)
+                    }
                 } else if let Some(calledvec) = self.call_tree.get(&nextproc) {
                     for (proccalled, location, new_context) in calledvec.iter() {
                         let mut newstack = callstack.clone();
@@ -786,6 +831,27 @@ impl<'o> AnalyzeObjectTree<'o> {
         }
     }
 
+    /// Propagate violations make up the inheritence graph
+    pub fn propagate_violations(&mut self, proc: ProcRef<'o>) {
+        if proc.name() == "New" { // New() propogates via ..() and causes weirdness
+            return;
+        }
+        if self.sleeping_procs.get_violators(proc).is_some() {
+            let mut next = proc.parent_proc();
+            while let Some(current) = next {
+                self.sleeping_overrides.insert_override(current, proc);
+                next = current.parent_proc();
+            }
+        }
+        if self.impure_procs.get_violators(proc).is_some() {
+            let mut next = proc.parent_proc();
+            while let Some(current) = next {
+                self.impure_overrides.insert_override(current, proc);
+                next = current.parent_proc();
+            }
+        }
+    }
+
     /// Check and build a list of bad overrides of kwargs for a ProcRef
     pub fn check_kwargs(&mut self, proc: ProcRef) {
         let param_names: HashSet<&String> = proc.parameters.iter().map(|p| &p.name).collect();

src/dreamchecker/test_helpers.rs

@@ -1,8 +1,7 @@
-use dm::objtree::Code;
 use dm::Context;
 use std::borrow::Cow;
 
-use crate::{check_var_defs, AnalyzeObjectTree};
+use crate::{run_inner};
 
 pub const NO_ERRORS: &[(u32, u16, &str)] = &[];
 
@@ -17,41 +16,7 @@ pub fn parse_a_file_for_test<S: Into<Cow<'static, str>>>(buffer: S) -> Context {
     parser.enable_procs();
     let tree = parser.parse_object_tree();
 
-    check_var_defs(&tree, &context);
-
-    let mut analyzer = AnalyzeObjectTree::new(&context, &tree);
-
-    tree.root().recurse(&mut |ty| {
-        for proc in ty.iter_self_procs() {
-            if let Code::Present(ref code) = proc.get().code {
-                analyzer.gather_settings(proc, code);
-            }
-        }
-    });
-
-    tree.root().recurse(&mut |ty| {
-        for proc in ty.iter_self_procs() {
-            match proc.get().code {
-                Code::Present(ref code) => {
-                    analyzer.check_proc(proc, code);
-                }
-                Code::Invalid(_) => {}
-                Code::Builtin => {}
-                Code::Disabled => {
-                    panic!("proc parsing was enabled, but also disabled. this is a bug")
-                }
-            }
-        }
-    });
-
-    analyzer.check_proc_call_tree();
-
-    tree.root().recurse(&mut |ty| {
-        for proc in ty.iter_self_procs() {
-            analyzer.check_kwargs(proc);
-        }
-    });
-    analyzer.finish_check_kwargs();
+    run_inner(&context, &tree, false);
 
     context
 }

src/dreamchecker/tests/sleep_pure_tests.rs

@@ -45,6 +45,40 @@ fn sleep() {
     check_errors_match(code, SLEEP_ERRORS);
 }
 
+pub const SLEEP_ERRORS2: &[(u32, u16, &str)] = &[
+    (8, 21, "/mob/living/proc/bar calls /mob/living/proc/foo which has override child proc that sleeps /mob/living/carbon/proc/foo"),
+];
+
+#[test]
+fn sleep2() {
+    let code = r##"
+/mob/New()
+/mob/proc/foo()
+    sleep(1)
+/mob/living/foo()
+    return TRUE
+/mob/living/carbon/foo()
+    sleep(1)
+/mob/living/proc/bar()
+    set SpacemanDMM_should_not_sleep = TRUE
+    foo()
+    thing()
+    new /mob/living()
+/mob/living/New()
+    . = ..()
+/mob/living/carbon/human/foo()
+    . = ..()
+/mob/dead/New()
+    sleep(1)
+/mob/proc/thing()
+/mob/dead/thing()
+    sleep(1)
+/mob/living/thing()
+    . = ..()
+"##.trim();
+    check_errors_match(code, SLEEP_ERRORS2);
+}
+
 pub const PURE_ERRORS: &[(u32, u16, &str)] = &[
     (12, 16, "/mob/proc/test2 sets SpacemanDMM_should_be_pure but calls a /proc/impure that does impure operations"),
 ];