diff --git a/src/dreamchecker/lib.rs b/src/dreamchecker/lib.rs index 307e9c46..4e1a5958 100644 --- a/src/dreamchecker/lib.rs +++ b/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, Vec>>, +} + +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>> { + 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>, + + 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(); diff --git a/src/dreamchecker/test_helpers.rs b/src/dreamchecker/test_helpers.rs index 89edd1e0..dd7d14b9 100644 --- a/src/dreamchecker/test_helpers.rs +++ b/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>>(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 } diff --git a/src/dreamchecker/tests/sleep_pure_tests.rs b/src/dreamchecker/tests/sleep_pure_tests.rs index 670403e5..bfb3a92d 100644 --- a/src/dreamchecker/tests/sleep_pure_tests.rs +++ b/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"), ];