feat: Validate Array Ranges (PLC-lang#1195)

This commit adds a validation for the ranges of arrays, in which the start values must be less than the end values. For example something like `... ARRAY[1..-5] OF ...` is invalid because `-5` < `1`. Previously this would either panic (debug mode) or generate huge arrays (release mode) due to overflows.

Author: volsa

compiler/plc_diagnostics/src/diagnostics.rs

@@ -1,11 +1,12 @@
 use std::fmt::Display;
 
+use serde::{Deserialize, Serialize};
+
 use plc_ast::ast::AstNode;
 use plc_source::{
     source_location::{SourceLocation, SourceLocationFactory},
     SourceCode,
 };
-use serde::{Deserialize, Serialize};
 
 pub mod diagnostics_registry;
 

compiler/plc_diagnostics/src/diagnostics/diagnostics_registry.rs

@@ -388,6 +388,7 @@ lazy_static! {
     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
 );
 }
 

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

@@ -0,0 +1,4 @@
+# Invalid Array Range
+
+Ranges such as `ARRAY [0..-1]` are invalid in ST because end values of ranges must be greater than their start values.
+A valid range for the given statement would have been `ARRAY[-1..0]`.

src/typesystem.rs

@@ -692,6 +692,23 @@ impl DataTypeInformation {
         }
     }
 
+    /// Recursively retrieves all type names for nested arrays.
+    ///
+    /// This is needed because a nested array such as `foo : ARRAY[1..5] OF ARRAY[5..10] OF DINT`
+    /// provides range information for `[1..5]` and `[5..10]` in two different types stored in
+    /// the index.
+    pub fn get_inner_array_types<'a>(&'a self, types: &mut Vec<&'a DataTypeInformation>, index: &'a Index) {
+        if let DataTypeInformation::Array { name, inner_type_name, .. } = self {
+            if name != inner_type_name {
+                types.push(self);
+
+                if let Some(ty) = index.find_type(inner_type_name).map(DataType::get_type_information) {
+                    ty.get_inner_array_types(types, index);
+                }
+            }
+        }
+    }
+
     pub fn get_inner_pointer_type_name(&self) -> Option<&str> {
         match self {
             DataTypeInformation::Pointer { inner_type_name, .. } => Some(inner_type_name),

src/validation/tests/array_validation_test.rs

@@ -346,3 +346,52 @@ fn array_assignment_function_call() {
 
     assert_snapshot!(diagnostics);
 }
+
+#[test]
+fn validate_ranges() {
+    let diagnostics = parse_and_validate_buffered(
+        r"
+        FUNCTION foo
+            VAR
+                arr_A : ARRAY[1..-5] OF DINT;
+                arr_B : ARRAY[1..5] OF ARRAY[1..-10] OF DINT;
+                arr_C : ARRAY[1..-5] OF ARRAY[1..-10] OF DINT;
+                arr_D : ARRAY[1..5, 1..-5] OF DINT;
+            END_VAR
+        END_FUNCTION
+        ",
+    );
+
+    assert_snapshot!(diagnostics, @r###"
+    error[E097]: Invalid range `1..-5`, did you mean `-5..1`?
+      ┌─ <internal>:4:17
+      │
+    4 │                 arr_A : ARRAY[1..-5] OF DINT;
+      │                 ^^^^^ Invalid range `1..-5`, did you mean `-5..1`?
+
+    error[E097]: Invalid range `1..-10`, did you mean `-10..1`?
+      ┌─ <internal>:5:17
+      │
+    5 │                 arr_B : ARRAY[1..5] OF ARRAY[1..-10] OF DINT;
+      │                 ^^^^^ Invalid range `1..-10`, did you mean `-10..1`?
+
+    error[E097]: Invalid range `1..-5`, did you mean `-5..1`?
+      ┌─ <internal>:6:17
+      │
+    6 │                 arr_C : ARRAY[1..-5] OF ARRAY[1..-10] OF DINT;
+      │                 ^^^^^ Invalid range `1..-5`, did you mean `-5..1`?
+
+    error[E097]: Invalid range `1..-10`, did you mean `-10..1`?
+      ┌─ <internal>:6:17
+      │
+    6 │                 arr_C : ARRAY[1..-5] OF ARRAY[1..-10] OF DINT;
+      │                 ^^^^^ Invalid range `1..-10`, did you mean `-10..1`?
+
+    error[E097]: Invalid range `1..-5`, did you mean `-5..1`?
+      ┌─ <internal>:7:17
+      │
+    7 │                 arr_D : ARRAY[1..5, 1..-5] OF DINT;
+      │                 ^^^^^ Invalid range `1..-5`, did you mean `-5..1`?
+
+    "###);
+}

src/validation/variable.rs

@@ -1,6 +1,7 @@
 use plc_ast::ast::{ArgumentProperty, Pou, PouType, Variable, VariableBlock, VariableBlockType};
 use plc_diagnostics::diagnostics::Diagnostic;
 
+use crate::typesystem::DataTypeInformation;
 use crate::{index::const_expressions::ConstExpression, resolver::AnnotationMap};
 
 use super::{
@@ -101,16 +102,47 @@ fn validate_vla(validator: &mut Validator, pou: Option<&Pou>, block: &VariableBl
     }
 }
 
+fn validate_array_ranges<T>(validator: &mut Validator, variable: &Variable, context: &ValidationContext<T>)
+where
+    T: AnnotationMap,
+{
+    let ty_name = variable.data_type_declaration.get_name().unwrap_or_default();
+    let ty_info = context.index.get_effective_type_or_void_by_name(ty_name).get_type_information();
+
+    if ty_info.is_array() {
+        let mut types = vec![];
+        ty_info.get_inner_array_types(&mut types, context.index);
+
+        for ty in types {
+            let DataTypeInformation::Array { dimensions, .. } = ty else {
+                unreachable!("`get_inner_types()` only operates on Arrays");
+            };
+
+            for dimension in dimensions.iter().filter_map(|dim| dim.get_range(context.index).ok()) {
+                let std::ops::Range { start, end } = dimension;
+
+                if start > end {
+                    validator.push_diagnostic(
+                        Diagnostic::new(format!(
+                            "Invalid range `{start}..{end}`, did you mean `{end}..{start}`?"
+                        ))
+                        .with_location(variable.location.clone())
+                        .with_error_code("E097"),
+                    );
+                }
+            }
+        }
+    }
+}
+
 fn validate_variable<T: AnnotationMap>(
     validator: &mut Validator,
     variable: &Variable,
     context: &ValidationContext<T>,
 ) {
-    if let Some(v_entry) = context
-        .qualifier
-        .and_then(|qualifier| context.index.find_member(qualifier, variable.name.as_str()))
-        .or_else(|| context.index.find_global_variable(variable.name.as_str()))
-    {
+    validate_array_ranges(validator, variable, context);
+
+    if let Some(v_entry) = context.index.find_variable(context.qualifier, &[&variable.name]) {
         if let Some(initializer) = &variable.initializer {
             // Assume `foo : ARRAY[1..5] OF DINT := [...]`, here the first function call validates the
             // assignment as a whole whereas the second function call (`visit_statement`) validates the