From ddcfaed971afa9a543b8d867a0d8561831183082 Mon Sep 17 00:00:00 2001 From: Volkan Sagcan Date: Thu, 30 Jan 2025 10:46:15 +0100 Subject: [PATCH] refactor: Update error codes, add description --- .../src/diagnostics/diagnostics_registry.rs | 5 +++- .../src/diagnostics/error_codes/E114.md | 17 +++++++++++++ .../src/diagnostics/error_codes/E115.md | 16 +++++++++++++ .../src/diagnostics/error_codes/E116.md | 24 +++++++++++++++++++ compiler/plc_driver/src/pipelines.rs | 4 ++-- src/lowering/validator.rs | 24 +++++++++---------- src/validation/tests/pou_validation_tests.rs | 22 ++++++++--------- 7 files changed, 85 insertions(+), 27 deletions(-) create mode 100644 compiler/plc_diagnostics/src/diagnostics/error_codes/E114.md create mode 100644 compiler/plc_diagnostics/src/diagnostics/error_codes/E115.md create mode 100644 compiler/plc_diagnostics/src/diagnostics/error_codes/E116.md diff --git a/compiler/plc_diagnostics/src/diagnostics/diagnostics_registry.rs b/compiler/plc_diagnostics/src/diagnostics/diagnostics_registry.rs index f8a9f510b6..9511bcfc93 100644 --- a/compiler/plc_diagnostics/src/diagnostics/diagnostics_registry.rs +++ b/compiler/plc_diagnostics/src/diagnostics/diagnostics_registry.rs @@ -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"), @@ -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 ); } diff --git a/compiler/plc_diagnostics/src/diagnostics/error_codes/E114.md b/compiler/plc_diagnostics/src/diagnostics/error_codes/E114.md new file mode 100644 index 0000000000..0834e18b84 --- /dev/null +++ b/compiler/plc_diagnostics/src/diagnostics/error_codes/E114.md @@ -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 +``` \ No newline at end of file diff --git a/compiler/plc_diagnostics/src/diagnostics/error_codes/E115.md b/compiler/plc_diagnostics/src/diagnostics/error_codes/E115.md new file mode 100644 index 0000000000..7c90cf8fa1 --- /dev/null +++ b/compiler/plc_diagnostics/src/diagnostics/error_codes/E115.md @@ -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 +``` \ No newline at end of file diff --git a/compiler/plc_diagnostics/src/diagnostics/error_codes/E116.md b/compiler/plc_diagnostics/src/diagnostics/error_codes/E116.md new file mode 100644 index 0000000000..82a4a4d1e0 --- /dev/null +++ b/compiler/plc_diagnostics/src/diagnostics/error_codes/E116.md @@ -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 +``` \ No newline at end of file diff --git a/compiler/plc_driver/src/pipelines.rs b/compiler/plc_driver/src/pipelines.rs index c8082d1723..eacb4e60bc 100644 --- a/compiler/plc_driver/src/pipelines.rs +++ b/compiler/plc_driver/src/pipelines.rs @@ -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::{ @@ -258,7 +258,7 @@ impl BuildPipeline { 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()); diff --git a/src/lowering/validator.rs b/src/lowering/validator.rs index c096dc9d01..37a6a8e6e8 100644 --- a/src/lowering/validator.rs +++ b/src/lowering/validator.rs @@ -16,19 +16,17 @@ impl ParticipantValidator { } pub fn validate_properties(&mut self, properties: &Vec) { - // 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 { @@ -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"), ); } } @@ -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; } @@ -70,7 +68,7 @@ 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 { @@ -78,7 +76,7 @@ impl ParticipantValidator { 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"), ); } } diff --git a/src/validation/tests/pou_validation_tests.rs b/src/validation/tests/pou_validation_tests.rs index a9bb7c3655..9c247f89de 100644 --- a/src/validation/tests/pou_validation_tests.rs +++ b/src/validation/tests/pou_validation_tests.rs @@ -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'. ┌─ :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) ┌─ :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] @@ -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 ┌─ :3:22 │ 3 │ PROPERTY prop : DINT @@ -317,7 +317,7 @@ fn property_with_more_than_one_get_block() { │ --- see also 5 │ GET END_GET │ --- see also - "); + "###); } #[test] @@ -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 ┌─ :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]