From 8a53cbd7ea2cfa5c90f9001a956b0105ae456f6d Mon Sep 17 00:00:00 2001 From: spookydonut Date: Sun, 11 Oct 2020 23:23:29 +0800 Subject: [PATCH 1/2] unused vars --- src/dreamchecker/lib.rs | 152 ++++++++++++++++++++++---------------- src/dreammaker/ast.rs | 2 +- src/dreammaker/error.rs | 2 +- src/dreammaker/objtree.rs | 2 +- 4 files changed, 93 insertions(+), 65 deletions(-) diff --git a/src/dreamchecker/lib.rs b/src/dreamchecker/lib.rs index 21c7565e..74cd73c4 100644 --- a/src/dreamchecker/lib.rs +++ b/src/dreamchecker/lib.rs @@ -5,11 +5,12 @@ extern crate dreammaker as dm; use dm::{Context, DMError, Location, Severity}; -use dm::objtree::{ObjectTree, TypeRef, ProcRef, Code}; +use dm::objtree::{ObjectTree, TypeRef, ProcRef, Code, VarDeclaration}; use dm::constants::{Constant, ConstFn}; use dm::ast::*; use std::collections::{BTreeMap, HashMap, HashSet, VecDeque}; +use std::borrow::Borrow; mod type_expr; use type_expr::TypeExpr; @@ -322,10 +323,10 @@ fn run_inner(context: &Context, objtree: &ObjectTree, cli: bool) { cli_println!("============================================================"); cli_println!("Analyzing variables...\n"); - check_var_defs(&objtree, &context); - let mut analyzer = AnalyzeObjectTree::new(context, objtree); + analyzer.check_var_defs(); + let mut present = 0; let mut invalid = 0; let mut builtin = 0; @@ -372,6 +373,11 @@ fn run_inner(context: &Context, objtree: &ObjectTree, cli: bool) { cli_println!("============================================================"); cli_println!("Analyzing proc call tree...\n"); analyzer.check_proc_call_tree(); + + cli_println!("============================================================"); + cli_println!("Listing unused var declarations...\n"); + + analyzer.check_unused_typevars(); } // ---------------------------------------------------------------------------- @@ -553,6 +559,8 @@ pub struct AnalyzeObjectTree<'o> { context: &'o Context, objtree: &'o ObjectTree, + unused_typevars: HashSet<&'o VarDeclaration>, + return_type: HashMap, TypeExpr<'o>>, must_call_parent: ProcDirective<'o>, must_not_override: ProcDirective<'o>, @@ -584,6 +592,7 @@ impl<'o> AnalyzeObjectTree<'o> { context, objtree, return_type, + unused_typevars: Default::default(), must_call_parent: ProcDirective::new("SpacemanDMM_should_call_parent", true, false, false), must_not_override: ProcDirective::new("SpacemanDMM_should_not_override", false, false, false), private: ProcDirective::new("SpacemanDMM_private_proc", false, true, false), @@ -602,6 +611,70 @@ impl<'o> AnalyzeObjectTree<'o> { } } + // ---------------------------------------------------------------------------- + // Variable analyzer + + /// Examines an ObjectTree for var definitions that are invalid + pub fn check_var_defs(&mut self) { + for typeref in self.objtree.iter_types() { + let path = &typeref.path; + + for (varname, typevar) in typeref.vars.iter() { + if let Some(decl) = typeref.get_var_declaration(varname) { + self.unused_typevars.insert(decl.borrow()); + } + } + + for parent in typeref.iter_parent_types() { + if parent.is_root() { + break; + } + + if &parent.path == path { + continue; + } + + for (varname, typevar) in typeref.vars.iter() { + if varname == "vars" { + continue; + } + if path == "/client" && varname == "parent_type" { + continue; + } + + guard!(let Some(parentvar) = parent.vars.get(varname) + else { continue }); + + guard!(let Some(decl) = &parentvar.declaration + else { continue }); + + if let Some(mydecl) = &typevar.declaration { + if typevar.value.location.is_builtins() { + continue; + } + DMError::new(mydecl.location, format!("{} redeclares var {:?}", path, varname)) + .with_note(decl.location, format!("declared on {} here", parent.path)) + .register(self.context); + } + + if decl.var_type.flags.is_final() { + DMError::new(typevar.value.location, format!("{} overrides final var {:?}", path, varname)) + .with_errortype("final_var") + .with_note(decl.location, format!("declared final on {} here", parent.path)) + .register(self.context); + } + + if decl.var_type.flags.is_private() { + DMError::new(typevar.value.location, format!("{} overrides private var {:?}", path, varname)) + .with_errortype("private_var") + .with_note(decl.location, format!("declared private on {} here", parent.path)) + .register(self.context); + } + } + } + } + } + /// Analyze a specific proc pub fn check_proc(&mut self, proc: ProcRef<'o>, code: &'o [Spanned]) { self.must_not_sleep.try_copy_from_parent(proc); @@ -651,6 +724,18 @@ impl<'o> AnalyzeObjectTree<'o> { } } + pub fn check_unused_typevars(&mut self) { + for decl in self.unused_typevars.iter() { + if decl.location.is_builtins() { + continue + } + error(decl.location, "Unused var declaration") + .with_errortype("unused_var_declaration") + .set_severity(Severity::Info) + .register(self.context); + } + } + pub fn check_proc_call_tree(&mut self) { for (procref, &(_, location)) in self.must_not_sleep.directive.iter() { if let Some(sleepvec) = self.sleeping_procs.get_violators(*procref) { @@ -955,64 +1040,6 @@ fn error>(location: Location, desc: S) -> DMError { DMError::new(location, desc).with_component(dm::Component::DreamChecker) } -// ---------------------------------------------------------------------------- -// Variable analyzer - -/// Examines an ObjectTree for var definitions that are invalid -pub fn check_var_defs(objtree: &ObjectTree, context: &Context) { - for typeref in objtree.iter_types() { - let path = &typeref.path; - - for parent in typeref.iter_parent_types() { - if parent.is_root() { - break; - } - - if &parent.path == path { - continue; - } - - for (varname, typevar) in typeref.vars.iter() { - if varname == "vars" { - continue; - } - if path == "/client" && varname == "parent_type" { - continue; - } - - guard!(let Some(parentvar) = parent.vars.get(varname) - else { continue }); - - guard!(let Some(decl) = &parentvar.declaration - else { continue }); - - if let Some(mydecl) = &typevar.declaration { - if typevar.value.location.is_builtins() { - continue; - } - DMError::new(mydecl.location, format!("{} redeclares var {:?}", path, varname)) - .with_note(decl.location, format!("declared on {} here", parent.path)) - .register(context); - } - - if decl.var_type.flags.is_final() { - DMError::new(typevar.value.location, format!("{} overrides final var {:?}", path, varname)) - .with_errortype("final_var") - .with_note(decl.location, format!("declared final on {} here", parent.path)) - .register(context); - } - - if decl.var_type.flags.is_private() { - DMError::new(typevar.value.location, format!("{} overrides private var {:?}", path, varname)) - .with_errortype("private_var") - .with_note(decl.location, format!("declared private on {} here", parent.path)) - .register(context); - } - } - } - } -} - // ---------------------------------------------------------------------------- // Procedure analyzer #[derive(Debug)] @@ -1662,7 +1689,7 @@ impl<'o, 's> AnalyzeProc<'o, 's> { .with_fix_hint(var.location, "add additional type info here") } if let Some(decl) = self.ty.get_var_declaration(unscoped_name) { - //println!("found type var"); + self.env.unused_typevars.remove(decl); let mut ana = self.static_type(location, &decl.var_type.type_path) .with_fix_hint(decl.location, "add additional type info here"); ana.is_impure = Some(true); @@ -1906,6 +1933,7 @@ impl<'o, 's> AnalyzeProc<'o, 's> { Follow::Field(kind, name) => { if let Some(ty) = lhs.static_ty.basic_type() { if let Some(decl) = ty.get_var_declaration(name) { + self.env.unused_typevars.remove(decl); if ty != self.ty && decl.var_type.flags.is_private() { error(location, format!("field {:?} on {} is declared as private", name, ty)) .with_errortype("private_var") diff --git a/src/dreammaker/ast.rs b/src/dreammaker/ast.rs index ef8b62b4..9610ed0c 100644 --- a/src/dreammaker/ast.rs +++ b/src/dreammaker/ast.rs @@ -922,7 +922,7 @@ impl fmt::Display for VarTypeFlags { } /// A type which may be ascribed to a `var`. -#[derive(Debug, Clone, PartialEq, Default)] +#[derive(Debug, Clone, PartialEq, Default, Eq, Hash)] pub struct VarType { pub flags: VarTypeFlags, pub type_path: TreePath, diff --git a/src/dreammaker/error.rs b/src/dreammaker/error.rs index dec80b97..396f3767 100644 --- a/src/dreammaker/error.rs +++ b/src/dreammaker/error.rs @@ -255,7 +255,7 @@ impl Context { // Location handling /// File, line, and column information for an error. -#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Default)] +#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Default, Hash)] pub struct Location { /// The index into the file table. pub file: FileId, diff --git a/src/dreammaker/objtree.rs b/src/dreammaker/objtree.rs index 09c57a77..20c0e6fa 100644 --- a/src/dreammaker/objtree.rs +++ b/src/dreammaker/objtree.rs @@ -46,7 +46,7 @@ impl SymbolIdSource { pub type Vars = LinkedHashMap; -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Eq, Hash, PartialEq)] pub struct VarDeclaration { pub var_type: VarType, pub location: Location, From 591b75070063c3c54fe698fa8e6ff801a07b7357 Mon Sep 17 00:00:00 2001 From: spookydonut Date: Wed, 14 Oct 2020 13:07:35 +0800 Subject: [PATCH 2/2] default to off --- src/dreamchecker/lib.rs | 2 +- src/dreammaker/config.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/dreamchecker/lib.rs b/src/dreamchecker/lib.rs index 74cd73c4..7bcebdb0 100644 --- a/src/dreamchecker/lib.rs +++ b/src/dreamchecker/lib.rs @@ -725,13 +725,13 @@ impl<'o> AnalyzeObjectTree<'o> { } pub fn check_unused_typevars(&mut self) { + if !self.context.config().code_standards.disallow_unused_var_definitions { return } for decl in self.unused_typevars.iter() { if decl.location.is_builtins() { continue } error(decl.location, "Unused var declaration") .with_errortype("unused_var_declaration") - .set_severity(Severity::Info) .register(self.context); } } diff --git a/src/dreammaker/config.rs b/src/dreammaker/config.rs index bb8785c8..a81aa9fa 100644 --- a/src/dreammaker/config.rs +++ b/src/dreammaker/config.rs @@ -45,6 +45,7 @@ pub struct Langserver { pub struct CodeStandards { pub disallow_relative_proc_definitions: bool, pub disallow_relative_type_definitions: bool, + pub disallow_unused_var_definitions: bool, } /// DMDoc config options