Skip to content

Commit

Permalink
refactor: Update error codes, add description
Browse files Browse the repository at this point in the history
  • Loading branch information
volsa committed Jan 30, 2025
1 parent 4c1aadc commit ddcfaed
Show file tree
Hide file tree
Showing 7 changed files with 85 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ lazy_static! {
E087, Error, include_str!("./error_codes/E087.md"),
E088, Error, include_str!("./error_codes/E088.md"),
E089, Error, include_str!("./error_codes/E089.md"),
E090, Warning, include_str!("./error_codes/E090.md"), // Incompatible reference Assignment
E090, Warning, include_str!("./error_codes/E090.md"), // Incompatible reference Assignment
E091, Warning, include_str!("./error_codes/E091.md"),
E092, Info, include_str!("./error_codes/E092.md"),
E093, Warning, include_str!("./error_codes/E093.md"),
Expand All @@ -215,6 +215,9 @@ lazy_static! {
E111, Error, include_str!("./error_codes/E111.md"), // Duplicate interface methods with different signatures
E112, Error, include_str!("./error_codes/E112.md"), // Incomplete interface implementation
E113, Warning, include_str!("./error_codes/E113.md"), // Interface default method implementation
E114, Error, include_str!("./error_codes/E114.md"), // Property in unsupported POU type
E115, Error, include_str!("./error_codes/E115.md"), // Property defined in non-supported variable block
E116, Error, include_str!("./error_codes/E116.md"), // Property with invalid number of GET and/or SET blocks
);
}

Expand Down
17 changes: 17 additions & 0 deletions compiler/plc_diagnostics/src/diagnostics/error_codes/E114.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Property defined in non-stateful POU type

Properties may only be defined in stateful POUs such as a `PROGRAM`,`CLASS` or `FUNCTION_BLOCK`.
Defining the property in any other POU results in this error.


Errouneus code example:
```
FUNCTION foo
// Invalid definition
PROPERTY bar : DINT
GET
bar := 42;
END_GET
END_PROPERTY
END_FUNCTION
```
16 changes: 16 additions & 0 deletions compiler/plc_diagnostics/src/diagnostics/error_codes/E115.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Property defined in non-supported variable block

Properties only allow for variable blocks of type `VAR`. Using something like `VAR_INPUT` is considered
invalid and will result in this error.

Errouneus code example:
```
FUNCTION foo
PROPERTY bar : DINT
GET
VAR /* ... */ END_VAR
VAR_INPUT /* ... */ END_VAR // Invalid
END_GET
END_PROPERTY
END_FUNCTION
```
24 changes: 24 additions & 0 deletions compiler/plc_diagnostics/src/diagnostics/error_codes/E116.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Property with invalid number of GET and/or SET blocks

Properties must be non empty and at most contain one block of type GET or SET.

Errouneus code example:
```
FUNCTION foo
PROPERTY bar : DINT
// Invalid, empty (no GET or SET block)
END_PROPERTY
PROPERTY baz : DINT
// Invalid, two GET blocks
GET END_GET
GET END_GET
END_PROPERTY
PROPERTY qux : DINT
// Invalid, two SET blocks
SET END_SET
SET END_SET
END_PROPERTY
END_FUNCTION
```
4 changes: 2 additions & 2 deletions compiler/plc_driver/src/pipelines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use plc::{
codegen::{CodegenContext, GeneratedModule},
index::{indexer, FxIndexSet, Index},
linker::LinkerType,
lowering::{property::PropertyLowerer, InitVisitor},
lowering::{property::PropertyLowerer, validator::ParticipantValidator, InitVisitor},
output::FormatOption,
parser::parse_file,
resolver::{
Expand Down Expand Up @@ -258,7 +258,7 @@ impl<T: SourceContainer> BuildPipeline<T> {
InitParticipant::new(&self.project.get_init_symbol_name(), self.context.provider());
self.register_mut_participant(Box::new(init_participant));

// self.register_mut_participant(Box::new(ParticipantValidator::new())); // XXX: I think this has to run first, because the PropertyLowerer will drain the `properties` field resulting in no validation
self.register_mut_participant(Box::new(ParticipantValidator::new())); // XXX: I think this has to run first, because the PropertyLowerer will drain the `properties` field resulting in no validation
self.register_mut_participant(Box::new(PropertyLowerer::new(self.context.provider())));

let aggregate_return_participant = AggregateTypeLowerer::new(self.context.provider());
Expand Down
24 changes: 11 additions & 13 deletions src/lowering/validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,17 @@ impl ParticipantValidator {
}

pub fn validate_properties(&mut self, properties: &Vec<Property>) {
// TODO: [x] validate number of set blocks
// [x] validate number of get blocks
// [x] at least one setter/getter is required
// [x] pou has to be FUNCTION_BLOCK or PROGRAM
// [x] only VAR blocks are allowed by user
for property in properties {
let mut get_blocks = vec![];
let mut set_blocks = vec![];
if !property.kind_parent.is_stateful_pou() {
self.diagnostics.push(
Diagnostic::new("Only FUNCTION_BLOCK or PROGRAM are allowed as parent for properties")
.with_location(property.name_parent_location.clone())
.with_error_code("E001"),
Diagnostic::new(format!(
"Property `{name}` must be defined in a stateful POU type (PROGRAM, CLASS or FUNCTION_BLOCK)",
name = property.name
))
.with_location(property.name_parent_location.clone())
.with_error_code("E114"),
);
}
for implementation in &property.implementations {
Expand All @@ -38,10 +36,10 @@ impl ParticipantValidator {
plc_ast::ast::VariableBlockType::Local => {}
_ => {
self.diagnostics.push(
Diagnostic::new("Only VAR blocks are allowed for properties")
Diagnostic::new("Properties only allow variable blocks of type VAR")
.with_secondary_location(variable.location.clone())
.with_location(property.name_location.clone())
.with_error_code("E001"),
.with_error_code("E115"),
);
}
}
Expand All @@ -60,7 +58,7 @@ impl ParticipantValidator {
self.diagnostics.push(
Diagnostic::new("Property has no GET or SET block")
.with_location(property.name_location.clone())
.with_error_code("E001"),
.with_error_code("E116"),
);
continue;
}
Expand All @@ -70,15 +68,15 @@ impl ParticipantValidator {
Diagnostic::new("Property has more than one GET block")
.with_location(property.name_location.clone())
.with_secondary_locations(get_blocks)
.with_error_code("E001"),
.with_error_code("E116"),
);
}
if set_blocks.len() > 1 {
self.diagnostics.push(
Diagnostic::new("Property has more than one SET block")
.with_location(property.name_location.clone())
.with_secondary_locations(set_blocks)
.with_error_code("E001"),
.with_error_code("E116"),
);
}
}
Expand Down
22 changes: 11 additions & 11 deletions src/validation/tests/pou_validation_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,19 +280,19 @@ fn property_within_function_pou() {
",
);

assert_snapshot!(diagnostics, @r"
assert_snapshot!(diagnostics, @r###"
error[E001]: Methods cannot be declared in a POU of type 'Function'.
┌─ <internal>:2:24
2 │ FUNCTION foo : DINT
│ ^^^^ Methods cannot be declared in a POU of type 'Function'.
error[E001]: Only FUNCTION_BLOCK or PROGRAM are allowed as parent for properties
error[E114]: Property `prop` must be defined in a stateful POU type (PROGRAM, CLASS or FUNCTION_BLOCK)
┌─ <internal>:2:18
2 │ FUNCTION foo : DINT
│ ^^^ Only FUNCTION_BLOCK or PROGRAM are allowed as parent for properties
");
│ ^^^ Property `prop` must be defined in a stateful POU type (PROGRAM, CLASS or FUNCTION_BLOCK)
"###);
}

#[test]
Expand All @@ -307,8 +307,8 @@ fn property_with_more_than_one_get_block() {
END_FUNCTION_BLOCK
",
);
assert_snapshot!(diagnostics, @r"
error[E001]: Property has more than one GET block
assert_snapshot!(diagnostics, @r###"
error[E116]: Property has more than one GET block
┌─ <internal>:3:22
3 │ PROPERTY prop : DINT
Expand All @@ -317,7 +317,7 @@ fn property_with_more_than_one_get_block() {
│ --- see also
5 │ GET END_GET
│ --- see also
");
"###);
}

#[test]
Expand All @@ -336,16 +336,16 @@ fn property_with_var_output_in_get_block() {
",
);

assert_snapshot!(diagnostics, @r"
error[E001]: Only VAR blocks are allowed for properties
assert_snapshot!(diagnostics, @r###"
error[E115]: Properties only allow variable blocks of type VAR
┌─ <internal>:3:22
3 │ PROPERTY prop : DINT
│ ^^^^ Only VAR blocks are allowed for properties
│ ^^^^ Properties only allow variable blocks of type VAR
4 │ GET
5 │ VAR_OUTPUT
│ ---------- see also
");
"###);
}

#[test]
Expand Down

0 comments on commit ddcfaed

Please sign in to comment.