refactor: Introducing ref_to_value_visitor

`ref_to_value_visitor` visits a reference
and returns a new (owned) value.

Adapted the parameter resolution part of `pre_linter/main`
to use the `RefToValueVisitor` trait,
resulting in less boilerplate code around
collections and `Position`.

Author: ngeor

rusty_linter/src/converter/dim_rules/validation.rs

@@ -6,7 +6,7 @@ use crate::names::ManyNamesTrait;
 use rusty_common::{AtPos, Position, Positioned};
 use rusty_parser::{DimVar, Parameter, TypedName, VarType};
 
-pub fn validate<T>(
+pub fn validate<T: VarType>(
     var_name: &TypedName<T>,
     ctx: &Context,
     pos: Position,
@@ -90,7 +90,7 @@ impl CannotClashWithFunctions for Parameter {
     }
 }
 
-fn user_defined_type_must_exist<T>(
+fn user_defined_type_must_exist<T: VarType>(
     var_name: &TypedName<T>,
     ctx: &Context,
 ) -> Result<(), LintErrorPos>

rusty_linter/src/core/mod.rs

@@ -3,6 +3,7 @@ mod const_value_resolver;
 mod error;
 mod qb_casting;
 mod qualify_variant;
+mod ref_to_value_visitor;
 mod string_length;
 mod traits;
 mod type_resolver;
@@ -15,6 +16,7 @@ pub use self::const_value_resolver::*;
 pub use self::error::*;
 pub use self::qb_casting::*;
 pub use self::qualify_variant::*;
+pub use self::ref_to_value_visitor::*;
 pub use self::string_length::*;
 pub use self::traits::*;
 pub use self::type_resolver::*;

rusty_linter/src/core/ref_to_value_visitor.rs

@@ -0,0 +1,35 @@
+use rusty_common::{AtPos, Positioned};
+
+/// Visits an element by ref and returns a new value.
+pub trait RefToValueVisitor<I, O, E> {
+    fn ref_to_value_visit(&mut self, element: &I) -> Result<O, E>;
+}
+
+// Blanket implementation for Vec
+
+impl<P, I, O, E> RefToValueVisitor<Vec<I>, Vec<O>, E> for P
+where
+    P: RefToValueVisitor<I, O, E>,
+{
+    fn ref_to_value_visit(&mut self, element: &Vec<I>) -> Result<Vec<O>, E> {
+        element.iter().map(|p| self.ref_to_value_visit(p)).collect()
+    }
+}
+
+// Blanket implementation for Positioned
+
+impl<P, I, O, E> RefToValueVisitor<Positioned<I>, Positioned<O>, Positioned<E>> for P
+where
+    P: RefToValueVisitor<I, O, E>,
+{
+    fn ref_to_value_visit(
+        &mut self,
+        element: &Positioned<I>,
+    ) -> Result<Positioned<O>, Positioned<E>> {
+        let Positioned { element, pos } = element;
+        match self.ref_to_value_visit(element) {
+            Ok(ok) => Ok(ok.at_pos(*pos)),
+            Err(err) => Err(err.at_pos(*pos)),
+        }
+    }
+}

rusty_linter/src/pre_linter/main.rs

@@ -51,7 +51,7 @@ impl Visitor<FunctionDeclaration> for MainContext {
             name: Positioned { element: name, .. },
             parameters: params,
         } = f;
-        let param_types: ResolvedParamTypes = self.on_parameters(params)?;
+        let param_types: ResolvedParamTypes = self.resolve_parameters(params)?;
         let bare_name = name.bare_name();
         let q = name.qualify(&self.resolver);
         let signature = Signature::new_function(q, param_types);
@@ -67,7 +67,7 @@ impl Visitor<FunctionImplementation> for MainContext {
             params,
             ..
         } = f;
-        let param_types: ResolvedParamTypes = self.on_parameters(params)?;
+        let param_types: ResolvedParamTypes = self.resolve_parameters(params)?;
         let bare_name = name.bare_name();
         let q = name.qualify(&self.resolver);
         let signature = Signature::new_function(q, param_types);
@@ -84,7 +84,7 @@ impl Visitor<SubDeclaration> for MainContext {
             },
             parameters: params,
         } = s;
-        let param_types: ResolvedParamTypes = self.on_parameters(params)?;
+        let param_types: ResolvedParamTypes = self.resolve_parameters(params)?;
         let signature = Signature::new_sub(param_types);
         self.subs
             .add_declaration(bare_name.clone(), signature.at_pos(self.declaration_pos))
@@ -100,7 +100,7 @@ impl Visitor<SubImplementation> for MainContext {
             params,
             ..
         } = s;
-        let param_types: ResolvedParamTypes = self.on_parameters(params)?;
+        let param_types: ResolvedParamTypes = self.resolve_parameters(params)?;
         let signature = Signature::new_sub(param_types);
         self.subs
             .add_implementation(bare_name.clone(), signature.at_pos(self.declaration_pos))
@@ -131,33 +131,36 @@ impl MainContext {
         )
     }
 
-    // TODO the remaining logic regarding parameters is a different kind of Visitor
-    // with a possible signature visit(&mut self, element: &T) -> Result<U, LintErrorPos>
-
-    fn on_parameters(&self, parameters: &Parameters) -> Result<ResolvedParamTypes, LintErrorPos> {
-        parameters
-            .iter()
-            .map(|p| self.on_parameter_pos(p))
-            .collect()
+    fn resolve_parameters(
+        &mut self,
+        parameters: &Parameters,
+    ) -> Result<ResolvedParamTypes, LintErrorPos> {
+        self.ref_to_value_visit(parameters).map(|v| v.no_pos())
     }
 
-    fn on_parameter_pos(
-        &self,
-        parameter_pos: &ParameterPos,
-    ) -> Result<ResolvedParamType, LintErrorPos> {
-        self.on_parameter(&parameter_pos.element)
-            .with_err_at(parameter_pos)
+    fn post_visit_functions(&self) -> Result<(), LintErrorPos> {
+        self.functions.ensure_declarations_are_implemented()?;
+        self.functions
+            .ensure_does_not_clash_with_built_in(|name| BuiltInFunction::try_parse(name).is_some())
     }
 
-    fn on_parameter(&self, parameter: &Parameter) -> Result<ResolvedParamType, LintError> {
-        self.resolve_param_type(parameter.bare_name(), parameter.var_type())
+    fn post_visit_subs(&self) -> Result<(), LintErrorPos> {
+        // not checking if declarations are present, because in MONEY.BAS there
+        // are two SUBs declared but not implemented (and not called either)
+        self.subs.ensure_does_not_clash_with_built_in(|name| {
+            BuiltInSub::parse_non_keyword_sub(name.as_ref()).is_some()
+        })
     }
+}
+
+impl<T> RefToValueVisitor<T, ResolvedParamType, LintError> for MainContext
+where
+    T: AsRef<BareName> + AsRef<ParamType>,
+{
+    fn ref_to_value_visit(&mut self, element: &T) -> Result<ResolvedParamType, LintError> {
+        let bare_name: &BareName = element.as_ref();
+        let param_type: &ParamType = element.as_ref();
 
-    fn resolve_param_type(
-        &self,
-        bare_name: &BareName,
-        param_type: &ParamType,
-    ) -> Result<ResolvedParamType, LintError> {
         match param_type {
             ParamType::Bare => {
                 let q = bare_name.qualify(&self.resolver);
@@ -175,24 +178,28 @@ impl MainContext {
                 }
             }
             ParamType::Array(element_type) => {
-                let element_param_type =
-                    self.resolve_param_type(bare_name, element_type.as_ref())?;
+                let temp = RefParamName(bare_name, element_type);
+                let element_param_type = self.ref_to_value_visit(&temp)?;
                 Ok(ResolvedParamType::Array(Box::new(element_param_type)))
             }
         }
     }
+}
 
-    fn post_visit_functions(&self) -> Result<(), LintErrorPos> {
-        self.functions.ensure_declarations_are_implemented()?;
-        self.functions
-            .ensure_does_not_clash_with_built_in(|name| BuiltInFunction::try_parse(name).is_some())
+/// This is the same as [Parameter],
+/// but the memebers are references.
+/// It's needed due to the recursive implementation of `RefToValueVisitor`
+/// and the recursive (`Box`) implementation of `ParamType::Array`.
+struct RefParamName<'a>(&'a BareName, &'a ParamType);
+
+impl<'a> AsRef<BareName> for RefParamName<'a> {
+    fn as_ref(&self) -> &BareName {
+        self.0
     }
+}
 
-    fn post_visit_subs(&self) -> Result<(), LintErrorPos> {
-        // not checking if declarations are present, because in MONEY.BAS there
-        // are two SUBs declared but not implemented (and not called either)
-        self.subs.ensure_does_not_clash_with_built_in(|name| {
-            BuiltInSub::parse_non_keyword_sub(name.as_ref()).is_some()
-        })
+impl<'a> AsRef<ParamType> for RefParamName<'a> {
+    fn as_ref(&self) -> &ParamType {
+        self.1
     }
 }