feat: Improved error messages using slices (PLC-lang#1061)

This PR improves error messages by using slices to reference the actual datatype instead of mangled names. For example an invalid assignment validation error message may now use ``ARRAY[1..5] OF DINT` instead of `__main_arr`.

compiler/plc_diagnostics/src/diagnostics.rs

@@ -674,7 +674,7 @@ impl Diagnostic {
 
     pub fn array_size(name: &str, len_lhs: usize, len_rhs: usize, range: SourceLocation) -> Diagnostic {
         Diagnostic::SemanticError {
-            message: format!("Array {name} has a size of {len_lhs}, but {len_rhs} elements were provided"),
+            message: format!("Array `{name}` has a size of {len_lhs}, but {len_rhs} elements were provided"),
             range: vec![range],
             err_no: ErrNo::arr__invalid_array_assignment,
         }

compiler/plc_index/src/lib.rs

@@ -7,7 +7,11 @@ use std::collections::HashMap;
 
 #[derive(Debug, Default)]
 pub struct GlobalContext {
+    /// HashMap containing all read, i.e. parsed, sources where the key represents
+    /// the relative file path and the value some [`SourceCode`]
     sources: HashMap<&'static str, SourceCode>,
+
+    /// [`IdProvider`] used during the parsing session
     provider: IdProvider,
     // TODO: The following would be also nice, to have a cleaner API i.e. instead of working with different structs such
     //       as the index or the diagnostics one could instead ONLY use the `GlobalContext` with methods like
@@ -23,17 +27,7 @@ impl GlobalContext {
         Self { sources: HashMap::new(), provider: IdProvider::default() }
     }
 
-    pub fn with_source<S>(mut self, sources: &[S], enc: Option<&'static Encoding>) -> Result<Self, Diagnostic>
-    where
-        S: SourceContainer,
-    {
-        for source in sources {
-            self.insert(source, enc)?;
-        }
-
-        Ok(self)
-    }
-
+    /// Inserts a single [`SourceCode`] to the internal source map
     pub fn insert<S>(&mut self, container: &S, encoding: Option<&'static Encoding>) -> Result<(), Diagnostic>
     where
         S: SourceContainer,
@@ -46,16 +40,39 @@ impl GlobalContext {
         Ok(())
     }
 
+    /// Inserts multiple [`SourceCode`]s to the internal source map
+    pub fn with_source<S>(mut self, sources: &[S], enc: Option<&'static Encoding>) -> Result<Self, Diagnostic>
+    where
+        S: SourceContainer,
+    {
+        for source in sources {
+            self.insert(source, enc)?;
+        }
+
+        Ok(self)
+    }
+
+    /// Returns some [`SourceCode`] based on the given key
     pub fn get(&self, key: &str) -> Option<&SourceCode> {
         self.sources.get(key)
     }
 
+    /// Returns a cloned [`IdProvider`]
     pub fn provider(&self) -> IdProvider {
         self.provider.clone()
     }
 
     // TODO: `impl Into<SourceLocation>` would be nice here, but adding `plc_ast` as a dep in `plc_source` yields a circular dep so not possible right now
-    pub fn slice(&self, location: &SourceLocation) -> &str {
+    /// Returns a (whitespace) trimmed slice representing the specified location of the source code.
+    /// For example if the location represents `ARRAY[1..5]\n\nOF\t  DINT` the slice `ARRAY[1..5] OF DINT` will be returned instead.
+    pub fn slice(&self, location: &SourceLocation) -> String {
+        let slice = self.slice_original(location);
+        slice.split_whitespace().collect::<Vec<_>>().join(" ")
+    }
+
+    /// Returns a slice representing the specified location of the source code.
+    /// If the location, i.e. file path, does not exist an empty string will be returned.
+    pub fn slice_original(&self, location: &SourceLocation) -> &str {
         let path = location.get_file_name().unwrap_or("<internal>");
 
         let Some(code) = self.sources.get(path) else { return "" };
@@ -83,3 +100,26 @@ impl GlobalContext {
     //     ctxt
     // }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::GlobalContext;
+    use plc_source::source_location::SourceLocationFactory;
+    use plc_source::SourceCode;
+
+    #[test]
+    fn slice() {
+        let input = SourceCode::from("ARRAY[1..5]   \n\n\n\nOF  \n\n\n    \t                 DINT");
+        let mut ctxt = GlobalContext::new();
+        ctxt.insert(&input, None).unwrap();
+
+        let factory = SourceLocationFactory::default();
+        let location = factory.create_range(0..input.source.len());
+
+        assert_eq!(ctxt.slice(&location), "ARRAY[1..5] OF DINT");
+        assert_eq!(
+            ctxt.slice_original(&location),
+            "ARRAY[1..5]   \n\n\n\nOF  \n\n\n    \t                 DINT"
+        );
+    }
+}

src/validation/array.rs

@@ -14,6 +14,7 @@ use plc_ast::{
     literals::AstLiteral,
 };
 use plc_diagnostics::diagnostics::Diagnostic;
+use plc_index::GlobalContext;
 
 use crate::{resolver::AnnotationMap, typesystem::DataTypeInformation};
 
@@ -46,20 +47,23 @@ pub(super) fn validate_array_assignment<'a, T, S>(
     T: AnnotationMap,
     S: Into<StatementWrapper<'a>> + Copy,
 {
-    let Some(rhs_stmt) = statement.into().rhs_statement() else { return };
-    let Some(lhs_info) = statement.into().lhs_info(context) else { return };
+    let statement = statement.into();
+
+    let Some(rhs_stmt) = statement.rhs_statement() else { return };
+    let Some(lhs_info) = statement.lhs_info(context) else { return };
 
     if !lhs_info.is_array() {
         return;
     }
 
-    validate_array(validator, context, lhs_info, rhs_stmt);
+    validate_array(validator, context, &statement, lhs_info, rhs_stmt);
     validate_array_of_structs(validator, context, lhs_info, rhs_stmt);
 }
 
 fn validate_array<T: AnnotationMap>(
     validator: &mut Validator,
     context: &ValidationContext<T>,
+    statement: &StatementWrapper,
     lhs_type: &DataTypeInformation,
     rhs_stmt: &AstNode,
 ) {
@@ -77,9 +81,9 @@ fn validate_array<T: AnnotationMap>(
     }
 
     if len_lhs < len_rhs {
-        let name = lhs_type.get_name();
+        let name = statement.lhs_name(validator.context);
         let location = stmt_rhs.get_location();
-        validator.push_diagnostic(Diagnostic::array_size(name, len_lhs, len_rhs, location));
+        validator.push_diagnostic(Diagnostic::array_size(&name, len_lhs, len_rhs, location));
     }
 }
 
@@ -148,13 +152,23 @@ fn statement_to_array_length<T: AnnotationMap>(context: &ValidationContext<T>, s
 }
 
 impl<'a> StatementWrapper<'a> {
+    fn lhs_name(&self, context: &GlobalContext) -> String {
+        match self {
+            StatementWrapper::Variable(variable) => variable.name.clone(),
+            StatementWrapper::Statement(statement) => {
+                let AstStatement::Assignment(data) = &statement.stmt else { return "".to_string() };
+                context.slice(&data.left.location)
+            }
+        }
+    }
+
     fn rhs_statement(&self) -> Option<&'a AstNode> {
         match self {
-            StatementWrapper::Statement(AstNode { stmt: AstStatement::Assignment(data), .. }) => {
+            StatementWrapper::Variable(variable) => variable.initializer.as_ref(),
+            StatementWrapper::Statement(statement) => {
+                let AstStatement::Assignment(data) = &statement.stmt else { return None };
                 Some(&data.right)
             }
-            StatementWrapper::Variable(variable) => variable.initializer.as_ref(),
-            _ => None,
         }
     }
 
@@ -164,9 +178,7 @@ impl<'a> StatementWrapper<'a> {
     {
         match self {
             StatementWrapper::Statement(statement) => {
-                let AstNode { stmt: AstStatement::Assignment(data), .. } = statement else {
-                    return None;
-                };
+                let AstNode { stmt: AstStatement::Assignment(data), .. } = statement else { return None };
                 context.annotations.get_type(&data.left, context.index).map(|it| it.get_type_information())
             }
 

src/validation/statement.rs

@@ -12,6 +12,7 @@ use plc_diagnostics::diagnostics::Diagnostic;
 use plc_source::source_location::SourceLocation;
 
 use super::{array::validate_array_assignment, ValidationContext, Validator, Validators};
+use crate::validation::statement::helper::get_datatype_name_or_slice;
 use crate::{
     builtins::{self, BuiltIn},
     codegen::generators::expression_generator::get_implicit_call_parameter,
@@ -527,7 +528,7 @@ fn visit_binary_expression<T: AnnotationMap>(
                 let slice_rhs = validator.context.slice(&right.location);
 
                 validator.push_diagnostic(Diagnostic::assignment_instead_of_equal(
-                    slice_lhs, slice_rhs, statement,
+                    &slice_lhs, &slice_rhs, statement,
                 ));
             }
 
@@ -726,8 +727,8 @@ fn validate_assignment<T: AnnotationMap>(
             && is_valid_assignment(left_type, right_type, right, context.index, location, validator))
         {
             validator.push_diagnostic(Diagnostic::invalid_assignment(
-                right_type.get_type_information().get_name(),
-                left_type.get_type_information().get_name(),
+                &get_datatype_name_or_slice(validator.context, right_type),
+                &get_datatype_name_or_slice(validator.context, left_type),
                 location.clone(),
             ));
         } else if right.is_literal() {
@@ -767,8 +768,8 @@ fn validate_variable_length_array_assignment<T: AnnotationMap>(
 
     if left_dt != right_dt || left_dims != right_dims {
         validator.push_diagnostic(Diagnostic::invalid_assignment(
-            right_type.get_type_information().get_name(),
-            left_type.get_type_information().get_name(),
+            &get_datatype_name_or_slice(validator.context, right_type),
+            &get_datatype_name_or_slice(validator.context, left_type),
             location.clone(),
         ));
     }
@@ -1113,7 +1114,9 @@ mod helper {
     use std::ops::Range;
 
     use plc_ast::ast::DirectAccessType;
+    use plc_index::GlobalContext;
 
+    use crate::typesystem::DataType;
     use crate::{index::Index, typesystem::DataTypeInformation};
 
     /// Returns true if the current index is in the range for the given type
@@ -1139,4 +1142,12 @@ mod helper {
     pub fn is_compatible(access: &DirectAccessType, data_type: &DataTypeInformation, index: &Index) -> bool {
         data_type.get_semantic_size(index) as u64 > access.get_bit_width()
     }
+
+    pub fn get_datatype_name_or_slice(context: &GlobalContext, dt: &DataType) -> String {
+        if dt.is_internal() {
+            return dt.get_type_information().get_name().to_string();
+        }
+
+        context.slice(&dt.location)
+    }
 }

src/validation/tests/assignment_validation_tests.rs

@@ -982,5 +982,5 @@ fn string_type_alias_assignment_can_be_validated() {
         ",
     );
 
-    assert_validation_snapshot!(diagnostics)
+    assert_validation_snapshot!(diagnostics);
 }

src/validation/tests/snapshots/rusty__validation__tests__array_validation_test__array_assignment_function_call.snap

@@ -2,16 +2,16 @@
 source: src/validation/tests/array_validation_test.rs
 expression: diagnostics
 ---
-error: Invalid assignment: cannot assign '__foo_return' to '__main_arr_incorrect_size'
+error: Invalid assignment: cannot assign 'ARRAY [0..3] OF USINT' to 'ARRAY[0..1] OF USINT'
    ┌─ <internal>:16:13
    │
 16 │             arr_incorrect_size  := foo(); // We want a invalid size array error here
-   │             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Invalid assignment: cannot assign '__foo_return' to '__main_arr_incorrect_size'
+   │             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Invalid assignment: cannot assign 'ARRAY [0..3] OF USINT' to 'ARRAY[0..1] OF USINT'
 
-error: Array __main_arr_incorrect_size has a size of 2, but 4 elements were provided
+error: Array `arr_incorrect_size` has a size of 2, but 4 elements were provided
    ┌─ <internal>:16:36
    │
 16 │             arr_incorrect_size  := foo(); // We want a invalid size array error here
-   │                                    ^^^^^^ Array __main_arr_incorrect_size has a size of 2, but 4 elements were provided
+   │                                    ^^^^^^ Array `arr_incorrect_size` has a size of 2, but 4 elements were provided
 
 

src/validation/tests/snapshots/rusty__validation__tests__array_validation_test__assignment_1d.snap

@@ -32,11 +32,11 @@ error: Unexpected token: expected KeywordParensClose but found ';'
 16 │             arr := (1, 2, 3, 4, 5];
    │                                   ^ Unexpected token: expected KeywordParensClose but found ';'
 
-error: Array __main_arr has a size of 5, but 6 elements were provided
+error: Array `arr` has a size of 5, but 6 elements were provided
   ┌─ <internal>:4:54
   │
 4 │                 arr         : ARRAY[1..5] OF DINT := [1, 2, 3, 4, 5, 6];
-  │                                                      ^^^^^^^^^^^^^^^^^^ Array __main_arr has a size of 5, but 6 elements were provided
+  │                                                      ^^^^^^^^^^^^^^^^^^ Array `arr` has a size of 5, but 6 elements were provided
 
 error: Array assignments must be surrounded with `[]`
   ┌─ <internal>:5:55
@@ -62,11 +62,11 @@ error: Array assignments must be surrounded with `[]`
 13 │             arr := (1, 2, 3, 4, 5, 6);
    │                     ^^^^^^^^^^^^^^^^ Array assignments must be surrounded with `[]`
 
-error: Array __main_arr has a size of 5, but 6 elements were provided
+error: Array `arr` has a size of 5, but 6 elements were provided
    ┌─ <internal>:14:20
    │
 14 │             arr := [1, 2, 3, 4, 5, 6];
-   │                    ^^^^^^^^^^^^^^^^^^ Array __main_arr has a size of 5, but 6 elements were provided
+   │                    ^^^^^^^^^^^^^^^^^^ Array `arr` has a size of 5, but 6 elements were provided
 
 error: Array assignments must be surrounded with `[]`
    ┌─ <internal>:15:34

src/validation/tests/snapshots/rusty__validation__tests__array_validation_test__assignment_2d.snap

@@ -2,23 +2,23 @@
 source: src/validation/tests/array_validation_test.rs
 expression: diagnostics
 ---
-error: Array __main_arr has a size of 10, but 11 elements were provided
+error: Array `arr` has a size of 10, but 11 elements were provided
   ┌─ <internal>:4:64
   │
 4 │                 arr             : ARRAY[1..2, 1..5] OF DINT := [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11];
-  │                                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Array __main_arr has a size of 10, but 11 elements were provided
+  │                                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Array `arr` has a size of 10, but 11 elements were provided
 
 error: Array assignments must be surrounded with `[]`
   ┌─ <internal>:5:65
   │
 5 │                 arr_alt         : ARRAY[1..2, 1..5] OF DINT := (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11);
   │                                                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Array assignments must be surrounded with `[]`
 
-error: Array __main_arr_nested has a size of 10, but 15 elements were provided
+error: Array `arr_nested` has a size of 10, but 15 elements were provided
   ┌─ <internal>:7:73
   │
 7 │                 arr_nested      : ARRAY[1..2] OF ARRAY[1..5] OF DINT := [ [1, 2, 3, 4, 5], [6, 7, 8, 9, 10], [11, 12, 13, 14, 15] ];
-  │                                                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Array __main_arr_nested has a size of 10, but 15 elements were provided
+  │                                                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Array `arr_nested` has a size of 10, but 15 elements were provided
 
 error: Array assignments must be surrounded with `[]`
   ┌─ <internal>:8:75
@@ -44,17 +44,17 @@ error: Array assignments must be surrounded with `[]`
 16 │             arr := (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11);
    │                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Array assignments must be surrounded with `[]`
 
-error: Array __main_arr has a size of 10, but 11 elements were provided
+error: Array `arr` has a size of 10, but 11 elements were provided
    ┌─ <internal>:17:20
    │
 17 │             arr := [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11];
-   │                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Array __main_arr has a size of 10, but 11 elements were provided
+   │                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Array `arr` has a size of 10, but 11 elements were provided
 
-error: Array __main_arr_nested has a size of 10, but 15 elements were provided
+error: Array `arr_nested` has a size of 10, but 15 elements were provided
    ┌─ <internal>:20:32
    │
 20 │             arr_nested      := [ [1, 2, 3, 4, 5], [6, 7, 8, 9, 10], [11, 12, 13, 14, 15] ];
-   │                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Array __main_arr_nested has a size of 10, but 15 elements were provided
+   │                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Array `arr_nested` has a size of 10, but 15 elements were provided
 
 error: Array assignments must be surrounded with `[]`
    ┌─ <internal>:21:34