feat: Validate VAR_CONFIG and template variables (#1303)

* feat: Validate VAR_CONFIG and template variables

Introduces a validation for template variables (e.g. `foo AT %I* : BOOL`) and their configuration
as specified in VAR_CONFIG blocks. Specifically the validation checks if
(1) the instances defined in VAR_CONFIG blocks have a valid reference to some variable defined elsewhere
(2) the instances defined in VAR_CONFIG blocks are complete (i.e. no `*` in their hardware address) and
(3) the datatypes defined in VAR_CONFIG blocks match with the reference variables datatype

* refactor: Some polishing

* chore: Add error codes

* refactor: PR feedback

Author: volsa

compiler/plc_ast/src/ast.rs

@@ -935,17 +935,11 @@ impl AstNode {
         }
     }
 
-    pub fn get_flat_base_reference_name(&self) -> Option<&str> {
-        match &self.stmt {
-            AstStatement::ReferenceExpr(ReferenceExpr { base: Some(base), .. }, ..) => {
-                base.as_ref().get_flat_base_reference_name()
-            }
-            AstStatement::ReferenceExpr(
-                ReferenceExpr { access: ReferenceAccess::Member(reference), base: None },
-                ..,
-            ) => reference.as_ref().get_flat_base_reference_name(),
-            AstStatement::Identifier(name, ..) => Some(name),
-            _ => None,
+    pub fn get_parent_name_of_reference(&self) -> Option<&str> {
+        if let AstStatement::ReferenceExpr(ReferenceExpr { base: Some(base), .. }, ..) = &self.stmt {
+            base.as_ref().get_flat_reference_name()
+        } else {
+            None
         }
     }
 
@@ -1089,6 +1083,13 @@ impl AstNode {
         let location = self.get_location();
         AstFactory::create_not_expression(self, location, id_provider.next_id())
     }
+
+    pub fn is_template(&self) -> bool {
+        matches!(
+            self.stmt,
+            AstStatement::HardwareAccess(HardwareAccess { access: DirectAccessType::Template, .. })
+        )
+    }
 }
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]

compiler/plc_diagnostics/src/diagnostics/diagnostics_registry.rs

@@ -196,12 +196,16 @@ lazy_static! {
         E092,   Info,       include_str!("./error_codes/E092.md"),
         E093,   Warning,    include_str!("./error_codes/E093.md"),
         E094,   Error,      include_str!("./error_codes/E094.md"),
-        E095,   Error,      include_str!("./error_codes/E095.md"), // Action call without `()`
-        E096,   Warning,    include_str!("./error_codes/E096.md"), // Integer Condition
-        E097,   Error,      include_str!("./error_codes/E097.md"), // Invalid Array Range
-        E098,   Error,      include_str!("./error_codes/E098.md"), // Invalid `REF=` assignment
-        E099,   Error,      include_str!("./error_codes/E099.md"), // Invalid `REFERENCE TO` declaration
-        E100,   Error,      include_str!("./error_codes/E100.md"), // Immutable variable address
+        E095,   Error,      include_str!("./error_codes/E095.md"),  // Action call without `()`
+        E096,   Warning,    include_str!("./error_codes/E096.md"),  // Integer Condition
+        E097,   Error,      include_str!("./error_codes/E097.md"),  // Invalid Array Range
+        E098,   Error,      include_str!("./error_codes/E098.md"),  // Invalid `REF=` assignment
+        E099,   Error,      include_str!("./error_codes/E099.md"),  // Invalid `REFERENCE TO` declaration
+        E100,   Error,      include_str!("./error_codes/E100.md"),  // Immutable variable address
+        E101,   Error,      include_str!("./error_codes/E101.md"),  // Invalid VAR_CONFIG / Template Variable Declaration
+        E102,   Error,      include_str!("./error_codes/E102.md"),  // Template variable without hardware binding
+        E103,   Error,      include_str!("./error_codes/E103.md"),  // Immutable Hardware Binding
+        E104,   Error,      include_str!("./error_codes/E104.md"),  // Config Variable With Incomplete Address
     );
 }
 

compiler/plc_diagnostics/src/diagnostics/error_codes/E101.md

@@ -0,0 +1,25 @@
+# Template variable does not exist
+
+A variable was configured in a `VAR_CONFIG` block, but the variable can not be found in the code.
+
+Erroneous code example:
+```
+VAR_CONFIG
+    main.foo.bar AT %IX1.0 : BOOL;
+END_VAR
+
+PROGRAM main
+    VAR
+        foo : foo_fb;
+    END_VAR
+END_PROGRAM
+
+FUNCTION_BLOCK foo_fb
+    VAR
+        qux AT %I* : BOOL;
+    END_VAR
+END_FUNCTION_BLOCK
+```
+
+In this example a variable named `bar` is configured, however the function block `foo_fb` does not contain
+a `bar` variable. The could should have been `main.foo.qux AT %IX1.0 : BOOL` instead for it to be valid.

compiler/plc_diagnostics/src/diagnostics/error_codes/E102.md

@@ -0,0 +1,27 @@
+# Template variable without hardware binding
+
+A template variable must contain a hardware binding.
+
+
+Erroneous code example:
+```
+VAR_CONFIG
+    main.foo.bar AT %IX1.0 : BOOL;
+END_VAR
+
+PROGRAM main
+    VAR
+        foo : foo_fb;
+    END_VAR
+END_PROGRAM
+
+FUNCTION_BLOCK foo_fb
+    VAR
+        bar : BOOL;
+    END_VAR
+END_FUNCTION_BLOCK
+```
+
+In this example the `VAR_CONFIG` block declares the `bar` variable inside `foo_fb` as a
+template variable. However `bar` does not have a hardware binding. For the example to be
+considered valid, `bar` should have been declared as e.g. `bar AT %I* : BOOL`.

compiler/plc_diagnostics/src/diagnostics/error_codes/E103.md

@@ -0,0 +1,27 @@
+# Immutable Hardware Binding
+
+Variables configured in a `VAR_CONFIG` block can not override their hardware binding.
+
+Erroneous code example:
+```
+VAR_CONFIG
+    main.foo.bar AT %IX1.0 : BOOL;
+END_VAR
+
+PROGRAM main
+    VAR
+        foo : foo_fb;
+    END_VAR
+END_PROGRAM
+
+FUNCTION_BLOCK foo_fb
+    VAR
+        bar AT IX1.5: BOOL;
+    END_VAR
+END_FUNCTION_BLOCK
+```
+
+In this example the `VAR_CONFIG` block configures `bar` to have a hardware adress `IX1.0`.
+However, at the same time the `bar` inside the POU `foo_fb` assigns a hardware address `IX1.5`.
+
+For the code to be considered valid, `bar` should have been declared as `bar AT %I* : BOOL`.

compiler/plc_diagnostics/src/diagnostics/error_codes/E104.md

@@ -0,0 +1,13 @@
+# Config Variable With Incomplete Address
+
+Variables defined in a `VAR_CONFIG` block, i.e. config variables, must specify a complete address.
+
+Erroneous code example:
+```
+VAR_CONFIG
+    main.foo.bar AT %I* : BOOL;
+END_VAR
+```
+
+In this example `main.foo.bar` has specified a placeholder hardware address. 
+For the example to be considered valid, a specific address such as `%IX1.0` should have been declared.

src/index.rs

@@ -273,6 +273,10 @@ impl VariableIndexEntry {
         self.binding.is_some()
     }
 
+    pub fn is_template(&self) -> bool {
+        matches!(self.binding, Some(HardwareBinding { access: DirectAccessType::Template, .. }))
+    }
+
     pub fn get_hardware_binding(&self) -> Option<&HardwareBinding> {
         self.binding.as_ref()
     }

src/parser/tests/parse_errors/parse_error_messages_test.rs

@@ -165,5 +165,11 @@ fn test_incomplete_var_config_block() {
        │ ╰───────────────────────^ Unexpected token: expected KeywordEndVar but found 'AT %IX3.1;
                     %IX3.1;'
 
+    error[E101]: Template variable `bar` does not exist
+      ┌─ <internal>:7:17
+      │
+    7 │                 instance4.bar AT %IX3.1 : BOOL;
+      │                 ^^^^^^^^^^^^^ Template variable `bar` does not exist
+
     "###);
 }

src/validation.rs

@@ -2,6 +2,7 @@ use plc_ast::ast::{AstNode, CompilationUnit};
 use plc_derive::Validators;
 use plc_diagnostics::diagnostics::Diagnostic;
 use plc_index::GlobalContext;
+use variable::visit_config_variable;
 
 use crate::{
     index::{
@@ -168,22 +169,27 @@ impl<'a> Validator<'a> {
 
     pub fn visit_unit<T: AnnotationMap>(&mut self, annotations: &T, index: &Index, unit: &CompilationUnit) {
         let context = ValidationContext { annotations, index, qualifier: None, is_call: false };
-        // validate POU and declared Variables
+        // Validate POU and declared Variables
         for pou in &unit.units {
             visit_pou(self, pou, &context.with_qualifier(pou.name.as_str()));
         }
 
-        // validate user declared types
+        // Validate user declared types
         for t in &unit.user_types {
             visit_user_type_declaration(self, t, &context);
         }
 
-        // validate global variables
+        // Validate config variables (VAR_CONFIG)
+        for variable in &unit.var_config {
+            visit_config_variable(self, variable, &context);
+        }
+
+        // Validate global variables
         for gv in &unit.global_vars {
             visit_variable_block(self, None, gv, &context);
         }
 
-        // validate implementations
+        // Validate implementations
         for implementation in &unit.implementations {
             visit_implementation(self, implementation, &context);
         }

src/validation/tests/variable_validation_tests.rs

@@ -459,3 +459,172 @@ fn date_invalid_declaration() {
 
     assert_snapshot!(diagnostics, @r###""###);
 }
+
+#[test]
+fn var_conf_template_variable_does_not_exist() {
+    let diagnostics = parse_and_validate_buffered(
+        r#"
+        FUNCTION_BLOCK foo_fb
+            VAR
+                bar AT %I* : BOOL;
+            END_VAR
+        END_FUNCTION_BLOCK
+
+        PROGRAM main
+            VAR
+                foo : foo_fb;
+            END_VAR
+        END_PROGRAM
+
+        VAR_CONFIG
+            main.foo.qux AT %IX1.0 : BOOL;
+        END_VAR
+        "#,
+    );
+
+    assert_snapshot!(diagnostics, @r###"
+    error[E101]: Template variable `qux` does not exist
+       ┌─ <internal>:15:13
+       │
+    15 │             main.foo.qux AT %IX1.0 : BOOL;
+       │             ^^^^^^^^^^^^ Template variable `qux` does not exist
+
+    "###);
+}
+
+#[test]
+fn var_conf_config_and_template_variable_types_differ() {
+    let diagnostics = parse_and_validate_buffered(
+        r#"
+        FUNCTION_BLOCK foo_fb
+            VAR
+                bar AT %I* : DINT;
+            END_VAR
+        END_FUNCTION_BLOCK
+
+        PROGRAM main
+            VAR
+                foo : foo_fb;
+            END_VAR
+        END_PROGRAM
+
+        VAR_CONFIG
+            main.foo.bar AT %IX1.0 : BOOL;
+        END_VAR
+        "#,
+    );
+
+    assert_snapshot!(diagnostics, @r###"
+    error[E001]: Config and Template variable types differ (BOOL and : DINT)
+       ┌─ <internal>:15:13
+       │
+     4 │                 bar AT %I* : DINT;
+       │                 --- see also
+       ·
+    15 │             main.foo.bar AT %IX1.0 : BOOL;
+       │             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Config and Template variable types differ (BOOL and : DINT)
+
+    "###);
+}
+
+#[test]
+fn var_conf_config_variable_has_incomplete_address() {
+    let diagnostics = parse_and_validate_buffered(
+        r#"
+        FUNCTION_BLOCK foo_fb
+            VAR
+                bar AT %I* : BOOL;
+            END_VAR
+        END_FUNCTION_BLOCK
+
+        PROGRAM main
+            VAR
+                foo : foo_fb;
+            END_VAR
+        END_PROGRAM
+
+        VAR_CONFIG
+            main.foo.bar AT %I* : BOOL;
+        END_VAR
+        "#,
+    );
+
+    assert_snapshot!(diagnostics, @r###"
+    error[E104]: Variables defined in a VAR_CONFIG block must have a complete address
+       ┌─ <internal>:15:26
+       │
+    15 │             main.foo.bar AT %I* : BOOL;
+       │                          ^^^^^^ Variables defined in a VAR_CONFIG block must have a complete address
+
+    "###);
+}
+
+#[test]
+fn var_conf_template_address_has_complete_address() {
+    let diagnostics = parse_and_validate_buffered(
+        r#"
+        FUNCTION_BLOCK foo_fb
+            VAR
+                bar AT %IX1.0 : BOOL;
+            END_VAR
+        END_FUNCTION_BLOCK
+
+        PROGRAM main
+            VAR
+                foo : foo_fb;
+            END_VAR
+        END_PROGRAM
+
+        VAR_CONFIG
+            main.foo.bar AT %IX1.0 : BOOL;
+        END_VAR
+        "#,
+    );
+
+    assert_snapshot!(diagnostics, @r###"
+    error[E103]: The configured variable is not a template, overriding non-template hardware addresses is not allowed
+       ┌─ <internal>:15:13
+       │
+     4 │                 bar AT %IX1.0 : BOOL;
+       │                 --- see also
+       ·
+    15 │             main.foo.bar AT %IX1.0 : BOOL;
+       │             ^^^^^^^^^^^^ The configured variable is not a template, overriding non-template hardware addresses is not allowed
+
+    "###);
+}
+
+#[test]
+fn var_conf_template_variable_is_no_template() {
+    let diagnostics = parse_and_validate_buffered(
+        r#"
+        FUNCTION_BLOCK foo_fb
+            VAR
+                bar : BOOL;
+            END_VAR
+        END_FUNCTION_BLOCK
+
+        PROGRAM main
+            VAR
+                foo : foo_fb;
+            END_VAR
+        END_PROGRAM
+
+        VAR_CONFIG
+            main.foo.bar AT %IX1.0 : BOOL;
+        END_VAR
+        "#,
+    );
+
+    assert_snapshot!(diagnostics, @r###"
+    error[E102]: `foo` is missing a hardware binding
+       ┌─ <internal>:4:17
+       │
+     4 │                 bar : BOOL;
+       │                 ^^^ `foo` is missing a hardware binding
+       ·
+    15 │             main.foo.bar AT %IX1.0 : BOOL;
+       │             ----------------------------- see also
+
+    "###);
+}