refactor: Adapt for_next_counter_match_linter and print_linter to the Visitor pattern

Author: ngeor

rusty_linter/src/core/visitor.rs

@@ -1,6 +1,7 @@
 use rusty_common::{Position, Positioned};
 use rusty_parser::*;
 
+use crate::delegate_visitor;
 use crate::LintErrorPos;
 
 /// The result of a visitor.
@@ -19,6 +20,20 @@ pub trait SetPosition {
     fn set_position(&mut self, pos: Position);
 }
 
+// Blanket implementation for Option
+
+impl<P, T> Visitor<Option<T>> for P
+where
+    P: Visitor<T>,
+{
+    fn visit(&mut self, element: &Option<T>) -> VisitResult {
+        match element {
+            Some(t) => self.visit(t),
+            _ => Ok(()),
+        }
+    }
+}
+
 // Blanket implementation for Vec
 
 impl<P, T> Visitor<Vec<T>> for P
@@ -51,34 +66,14 @@ pub trait DelegateVisitor<T> {
     fn delegate(&mut self) -> impl Visitor<T>;
 }
 
-/// A visitor that can visit global statements
-/// but does not enter the implementations of functions or subs.
-/// Actual visiting logic is handled by the delegate.
-pub struct ShallowVisitor<P> {
-    delegate: P,
-}
-
-impl<P> ShallowVisitor<P> {
-    pub fn new(delegate: P) -> Self {
-        Self { delegate }
-    }
+delegate_visitor!(
+    /// A visitor that can visit global statements
+    /// but does not enter the implementations of functions or subs.
+    /// Actual visiting logic is handled by the delegate.
+    GlobalVisitor
+);
 
-    /// Returns the delegate back.
-    pub fn delegate(self) -> P {
-        self.delegate
-    }
-}
-
-impl<P> SetPosition for ShallowVisitor<P>
-where
-    P: SetPosition,
-{
-    fn set_position(&mut self, pos: Position) {
-        self.delegate.set_position(pos);
-    }
-}
-
-impl<P> Visitor<GlobalStatement> for ShallowVisitor<P>
+impl<P> Visitor<GlobalStatement> for GlobalVisitor<P>
 where
     P: Visitor<DefType>
         + Visitor<FunctionDeclaration>
@@ -102,3 +97,180 @@ where
         }
     }
 }
+
+delegate_visitor!(
+    /// A visitor that can visit statements
+    /// and also enters the implementations of functions or subs,
+    /// as well as nested block statements (e.g. inside IF statements).
+    /// Actual visiting logic is handled by the delegate.
+    DeepStatementVisitor
+);
+
+impl<P> Visitor<GlobalStatement> for DeepStatementVisitor<P>
+where
+    P: Visitor<DefType>
+        + Visitor<FunctionDeclaration>
+        + Visitor<FunctionImplementation>
+        + Visitor<SubDeclaration>
+        + Visitor<SubImplementation>
+        + Visitor<UserDefinedType>
+        + Visitor<Statement>
+        + SetPosition,
+{
+    fn visit(&mut self, element: &GlobalStatement) -> VisitResult {
+        match element {
+            GlobalStatement::DefType(def_type) => self.delegate.visit(def_type),
+            GlobalStatement::FunctionDeclaration(f) => self.delegate.visit(f),
+            GlobalStatement::FunctionImplementation(f) => {
+                // notify first the delegate about the FUNCTION implementation
+                self.delegate.visit(f)?;
+                // then visit the body (this will go into the statements of the FUNCTION)
+                self.visit(&f.body)
+            }
+            GlobalStatement::Statement(statement) => self.visit(statement),
+            GlobalStatement::SubDeclaration(s) => self.delegate.visit(s),
+            GlobalStatement::SubImplementation(s) => {
+                // notify first the delegate about the SUB implementation
+                self.delegate.visit(s)?;
+                // then visit the body (this will go into the statements of the SUB)
+                self.visit(&s.body)
+            }
+            GlobalStatement::UserDefinedType(user_defined_type) => {
+                self.delegate.visit(user_defined_type)
+            }
+        }
+    }
+}
+
+impl<P> Visitor<Statement> for DeepStatementVisitor<P>
+where
+    P: Visitor<Statement> + SetPosition,
+{
+    fn visit(&mut self, element: &Statement) -> VisitResult {
+        // first visit the delegate
+        self.delegate.visit(element)?;
+
+        // then dive into the statement to for cases of blocks
+        match element {
+            Statement::IfBlock(if_block) => self.visit(if_block),
+            Statement::SelectCase(select_case) => self.visit(select_case),
+            Statement::ForLoop(for_loop) => self.visit(for_loop),
+            Statement::While(conditional_block) => self.visit(conditional_block),
+            Statement::DoLoop(do_loop) => self.visit(do_loop),
+            _ => Ok(()),
+        }
+    }
+}
+
+impl<P> Visitor<ForLoop> for DeepStatementVisitor<P>
+where
+    P: Visitor<Statement> + SetPosition,
+{
+    fn visit(&mut self, element: &ForLoop) -> VisitResult {
+        self.visit(&element.statements)
+    }
+}
+
+impl<P> Visitor<SelectCase> for DeepStatementVisitor<P>
+where
+    P: Visitor<Statement> + SetPosition,
+{
+    fn visit(&mut self, element: &SelectCase) -> VisitResult {
+        self.visit(&element.case_blocks)?;
+        self.visit(&element.else_block)
+    }
+}
+
+impl<P> Visitor<CaseBlock> for DeepStatementVisitor<P>
+where
+    P: Visitor<Statement> + SetPosition,
+{
+    fn visit(&mut self, element: &CaseBlock) -> VisitResult {
+        self.visit(&element.statements)
+    }
+}
+
+impl<P> Visitor<IfBlock> for DeepStatementVisitor<P>
+where
+    P: Visitor<Statement> + SetPosition,
+{
+    fn visit(&mut self, element: &IfBlock) -> VisitResult {
+        self.visit(&element.if_block)?;
+        self.visit(&element.else_if_blocks)?;
+        self.visit(&element.else_block)
+    }
+}
+
+impl<P> Visitor<DoLoop> for DeepStatementVisitor<P>
+where
+    P: Visitor<Statement> + SetPosition,
+{
+    fn visit(&mut self, element: &DoLoop) -> VisitResult {
+        self.visit(&element.statements)
+    }
+}
+
+impl<P> Visitor<ConditionalBlock> for DeepStatementVisitor<P>
+where
+    P: Visitor<Statement> + SetPosition,
+{
+    fn visit(&mut self, element: &ConditionalBlock) -> VisitResult {
+        self.visit(&element.statements)
+    }
+}
+
+/// Creates a no-op visitor implementation
+/// for the given types.
+#[macro_export]
+macro_rules! no_op_visitor {
+    ($visitor_name: ident: $($types:tt),+) => {
+        $(
+            impl Visitor<$types> for $visitor_name {
+                fn visit(&mut self, _element: &$types) -> VisitResult {
+                    Ok(())
+                }
+            }
+        )+
+    };
+}
+
+#[macro_export]
+macro_rules! no_pos_visitor {
+    ($visitor_name: ident) => {
+        impl SetPosition for $visitor_name {
+            fn set_position(&mut self, _pos: Position) {}
+        }
+    };
+}
+
+/// Creates a visitor that delegates to another.
+#[macro_export]
+macro_rules! delegate_visitor {
+    ($(#[$($attrss:tt)*])* $name: ident) => {
+        $(#[$($attrss)*])*
+        pub struct $name<P> {
+            delegate: P
+        }
+
+        impl<P> $name<P> {
+            pub fn new(delegate: P) -> Self {
+                Self { delegate }
+            }
+
+            /// Returns the delegate back.
+            #[allow(unused)]
+            pub fn delegate(self) -> P {
+                self.delegate
+            }
+        }
+
+        impl<P> SetPosition for $name<P>
+        where
+            P: SetPosition,
+        {
+            fn set_position(&mut self, pos: Position) {
+                self.delegate.set_position(pos);
+            }
+        }
+    };
+}

rusty_linter/src/post_linter/for_next_counter_match_linter.rs

@@ -1,11 +1,35 @@
-use crate::core::{LintError, LintErrorPos};
+use crate::core::*;
+use crate::{no_op_visitor, no_pos_visitor};
 use rusty_common::*;
-use rusty_parser::{Expression, ExpressionType, ForLoop, TypeQualifier, VariableInfo};
-
-use super::post_conversion_linter::*;
+use rusty_parser::*;
 
 pub struct ForNextCounterMatch;
 
+no_op_visitor!(ForNextCounterMatch: DefType, FunctionDeclaration, FunctionImplementation, SubDeclaration, SubImplementation, UserDefinedType);
+no_pos_visitor!(ForNextCounterMatch);
+
+impl ForNextCounterMatch {
+    pub fn visitor() -> impl Visitor<Program> + SetPosition {
+        DeepStatementVisitor::new(Self)
+    }
+}
+
+impl Visitor<Statement> for ForNextCounterMatch {
+    fn visit(&mut self, element: &Statement) -> VisitResult {
+        match element {
+            Statement::ForLoop(f) => self.visit(f),
+            _ => Ok(()),
+        }
+    }
+}
+
+impl Visitor<ForLoop> for ForNextCounterMatch {
+    fn visit(&mut self, f: &ForLoop) -> crate::core::VisitResult {
+        self.ensure_numeric_variable(f)?;
+        self.ensure_for_next_counter_match(f)
+    }
+}
+
 impl ForNextCounterMatch {
     fn ensure_numeric_variable(&self, f: &ForLoop) -> Result<(), LintErrorPos> {
         let Positioned {
@@ -26,7 +50,7 @@ impl ForNextCounterMatch {
                 ExpressionType::BuiltIn(_) => Ok(()),
                 _ => Err(LintError::TypeMismatch.at_pos(*pos)),
             },
-            _ => unimplemented!(),
+            _ => panic!("It should not be possible for the FOR variable to be something othe than a variable"),
         }
     }
 
@@ -58,11 +82,3 @@ impl ForNextCounterMatch {
         }
     }
 }
-
-impl PostConversionLinter for ForNextCounterMatch {
-    fn visit_for_loop(&mut self, f: &ForLoop) -> Result<(), LintErrorPos> {
-        self.visit_statements(&f.statements)?;
-        self.ensure_numeric_variable(f)?;
-        self.ensure_for_next_counter_match(f)
-    }
-}

rusty_linter/src/post_linter/post_linter.rs

@@ -1,5 +1,6 @@
 use crate::core::HasSubprograms;
 use crate::core::LintErrorPos;
+use crate::core::Visitor;
 use crate::post_linter::expression_reducer::ExpressionReducer;
 use crate::post_linter::post_conversion_linter::PostConversionLinter;
 use crate::post_linter::{
@@ -24,17 +25,17 @@ fn apply_linters(
     result: &Program,
     linter_context: &impl HasSubprograms,
 ) -> Result<(), LintErrorPos> {
-    let mut linter = for_next_counter_match_linter::ForNextCounterMatch {};
-    linter.visit_program(result)?;
+    let mut linter = for_next_counter_match_linter::ForNextCounterMatch::visitor();
+    linter.visit(result)?;
 
     let mut linter = dots_linter::DotsLinter::default();
     linter.visit_program(result)?;
 
     let mut linter = built_in_linter::BuiltInLinter::new();
     linter.visit_program(result)?;
 
-    let mut linter = print_linter::PrintLinter {};
-    linter.visit_program(result)?;
+    let mut linter = print_linter::PrintLinter::visitor();
+    linter.visit(result)?;
 
     let mut linter = user_defined_function_linter::UserDefinedFunctionLinter { linter_context };
     linter.visit_program(result)?;

rusty_linter/src/post_linter/print_linter.rs

@@ -1,12 +1,30 @@
-use super::post_conversion_linter::PostConversionLinter;
-use crate::core::{LintError, LintErrorPos};
-use rusty_common::AtPos;
-use rusty_parser::{ExpressionType, HasExpressionType, Print, PrintArg, TypeQualifier};
+use crate::core::*;
+use crate::{no_op_visitor, no_pos_visitor};
+use rusty_common::*;
+use rusty_parser::*;
 
 pub struct PrintLinter;
 
-impl PostConversionLinter for PrintLinter {
-    fn visit_print(&mut self, print: &Print) -> Result<(), LintErrorPos> {
+no_op_visitor!(PrintLinter: DefType, FunctionDeclaration, FunctionImplementation, SubDeclaration, SubImplementation, UserDefinedType);
+no_pos_visitor!(PrintLinter);
+
+impl PrintLinter {
+    pub fn visitor() -> impl Visitor<Program> + SetPosition {
+        DeepStatementVisitor::new(Self)
+    }
+}
+
+impl Visitor<Statement> for PrintLinter {
+    fn visit(&mut self, element: &Statement) -> VisitResult {
+        match element {
+            Statement::Print(p) => self.visit(p),
+            _ => Ok(()),
+        }
+    }
+}
+
+impl Visitor<Print> for PrintLinter {
+    fn visit(&mut self, print: &Print) -> VisitResult {
         if let Some(f) = &print.format_string {
             if f.expression_type() != ExpressionType::BuiltIn(TypeQualifier::DollarString) {
                 return Err(LintError::TypeMismatch.at(f));

rusty_linter/src/pre_linter/main.rs

@@ -20,7 +20,7 @@ struct MainContext {
 }
 
 pub fn pre_lint_program(program: &Program) -> Result<PreLinterResult, LintErrorPos> {
-    let mut visitor = ShallowVisitor::new(MainContext::default());
+    let mut visitor = GlobalVisitor::new(MainContext::default());
     visitor.visit(program)?;
     let ctx = visitor.delegate();
     ctx.post_visit_functions()?;