Skip to content

Commit

Permalink
Force parentheses for single assignment statements (PLC-lang#1025)
Browse files Browse the repository at this point in the history
Add a validation for single assignment statements in array of struct initializer, e.g. foo : ARRAY[...] OF MyStruct := [myField := 0] returns a codegen error ("Some initial values were not generated") with no validation error. This PR introduces a validation for that edge-case.
  • Loading branch information
volsa authored Nov 23, 2023
1 parent 477598f commit 5c729d8
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 10 deletions.
33 changes: 23 additions & 10 deletions src/validation/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ pub(super) fn validate_array_assignment<T: AnnotationMap>(
context: &ValidationContext<T>,
wrapper: Wrapper,
) {
let Some(lhs_type) = wrapper.datatype_info_lhs(context) else { return };
let Some(rhs_stmt) = wrapper.get_rhs() else { return };
let Some(lhs_type) = wrapper.datatype_info_lhs(context) else { return; };
let Some(rhs_stmt) = wrapper.get_rhs() else { return; };

if !lhs_type.is_array() {
return;
Expand Down Expand Up @@ -71,16 +71,29 @@ fn validate_array_of_structs<T: AnnotationMap>(
lhs_type: &DataTypeInformation,
rhs_stmt: &AstNode,
) {
let Some(array_type_name) = lhs_type.get_inner_array_type_name() else { return };
let Some(dti) = context.index.find_effective_type_by_name(array_type_name) else { return };
let Some(array_type_name) = lhs_type.get_inner_array_type_name() else { return; };
let Some(dti) = context.index.find_effective_type_by_name(array_type_name) else { return; };

if dti.is_struct() {
let AstStatement::Literal(AstLiteral::Array(array)) = rhs_stmt.get_stmt() else { return };
let Some(AstStatement::ExpressionList(expressions)) = array.elements().map(AstNode::get_stmt) else { return };
if !dti.is_struct() {
return;
}

for invalid in expressions.iter().filter(|it| !it.is_paren()) {
validator.push_diagnostic(Diagnostic::array_struct_assignment(invalid.get_location()));
let AstStatement::Literal(AstLiteral::Array(array)) = rhs_stmt.get_stmt() else { return; };
let Some(elements) = array.elements().map(AstNode::get_stmt) else { return; };

match elements {
AstStatement::ExpressionList(expressions) => {
for invalid in expressions.iter().filter(|it| !it.is_paren()) {
validator.push_diagnostic(Diagnostic::array_struct_assignment(invalid.get_location()));
}
}

// arr := [foo := 0]
AstStatement::Assignment(..) => {
validator.push_diagnostic(Diagnostic::array_struct_assignment(rhs_stmt.get_location()));
}

_ => (),
}
}

Expand Down Expand Up @@ -126,7 +139,7 @@ impl<'a> Wrapper<'a> {
{
match self {
Wrapper::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())
}

Expand Down
1 change: 1 addition & 0 deletions src/validation/tests/array_validation_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@ fn parenthesized_struct_initializers() {
foo_invalid_a : ARRAY[1..2] OF foo := [idx := 0, val := 0, idx := 1, val := 1]; // Both initializers missing parens
foo_invalid_b : ARRAY[1..2] OF foo := [idx := 0, val := 0, (idx := 1, val := 1)]; // First initializer missing parens
foo_invalid_c : ARRAY[1..2] OF foo := [(idx := 0, val := 0), idx := 1, val := 1]; // Second initializer missing parens
foo_invalid_d : ARRAY[1..2] OF foo := [idx := 0];
END_VAR
END_FUNCTION
",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,10 @@ error: Struct initializers within arrays have to be wrapped by `()`
12 │ foo_invalid_c : ARRAY[1..2] OF foo := [(idx := 0, val := 0), idx := 1, val := 1]; // Second initializer missing parens
│ ^^^^^^^^ Struct initializers within arrays have to be wrapped by `()`

error: Struct initializers within arrays have to be wrapped by `()`
┌─ <internal>:13:55
13 │ foo_invalid_d : ARRAY[1..2] OF foo := [idx := 0];
│ ^^^^^^^^^^ Struct initializers within arrays have to be wrapped by `()`


0 comments on commit 5c729d8

Please sign in to comment.