From 0bd8c743feafee5ddeca330b4d90d47c1b2f5617 Mon Sep 17 00:00:00 2001 From: RobertoRoos Date: Thu, 28 Nov 2024 11:03:49 +0100 Subject: [PATCH 1/4] Clarified error handling slightly, made sure to include a syntax error on purpose --- src/plcdoc/interpreter.py | 2 ++ src/plcdoc/st_declaration.tx | 5 +++-- tests/plc_code/T_ALIAS.txt | 2 ++ .../test-plc-project/src_plc/DUTs/E_Error.TcDUT | 15 +++++++++++++++ .../src_plc/DUTs/ST_MyStruct.TcDUT | 12 ++++++++++++ .../test-plc-project/src_plc/DUTs/T_ALIAS.TcDUT | 8 ++++++++ .../roots/test-plc-project/src_plc/MyPLC.plcproj | 12 ++++++++++++ .../src_plc/POUs/F_SyntaxError.TcPOU | 12 ++++++++++++ tests/test_plc_project.py | 6 +++++- tests/test_st_grammar.py | 7 ++++--- 10 files changed, 75 insertions(+), 6 deletions(-) create mode 100644 tests/plc_code/T_ALIAS.txt create mode 100644 tests/roots/test-plc-project/src_plc/DUTs/E_Error.TcDUT create mode 100644 tests/roots/test-plc-project/src_plc/DUTs/ST_MyStruct.TcDUT create mode 100644 tests/roots/test-plc-project/src_plc/DUTs/T_ALIAS.TcDUT create mode 100644 tests/roots/test-plc-project/src_plc/POUs/F_SyntaxError.TcPOU diff --git a/src/plcdoc/interpreter.py b/src/plcdoc/interpreter.py index 7786fdf..38fa716 100644 --- a/src/plcdoc/interpreter.py +++ b/src/plcdoc/interpreter.py @@ -132,6 +132,7 @@ def _parse_file(self, filepath) -> bool: plc_item = item.tag # I.e. "POU" if plc_item not in self.XML_TYPES: + logger.warning(f"Skipping file with XML tag {plc_item}") continue # Name is repeated inside the declaration, use it from there instead @@ -139,6 +140,7 @@ def _parse_file(self, filepath) -> bool: object_model = self._parse_declaration(item) if object_model is None: + # Log entry is made inside _parse_declaration() already continue obj = PlcDeclaration(object_model, filepath) diff --git a/src/plcdoc/st_declaration.tx b/src/plcdoc/st_declaration.tx index 2e2727f..ec27ebe 100644 --- a/src/plcdoc/st_declaration.tx +++ b/src/plcdoc/st_declaration.tx @@ -8,12 +8,13 @@ Note: `FUNCTION*` does not have a closing call for some reason! There are a lot of `comments*=CommentAny`. This will offer a list of comments, the relevant docblocks need to be extracted later. There seems no better way to do this in TextX. -Comment captures should be move down (= more basic elements) as much as possible to limit their usage. +Comment captures should be moved down (= more basic elements) as much as possible to limit their usage. There can be dynamic expressions in variable declarations, which are very tricky to parse. Therefore expected -expressions are parsed greedily as whole string. As a consequence, blocks like argument lists will result in a long +expressions are parsed greedily as whole strings. As a consequence, blocks like argument lists will result in a long string including the parentheses and commas. */ + Declaration: types*=TypeDef properties*=Property diff --git a/tests/plc_code/T_ALIAS.txt b/tests/plc_code/T_ALIAS.txt new file mode 100644 index 0000000..4407e01 --- /dev/null +++ b/tests/plc_code/T_ALIAS.txt @@ -0,0 +1,2 @@ +TYPE T_INTERLOCK : WORD; +END_TYPE diff --git a/tests/roots/test-plc-project/src_plc/DUTs/E_Error.TcDUT b/tests/roots/test-plc-project/src_plc/DUTs/E_Error.TcDUT new file mode 100644 index 0000000..0abf316 --- /dev/null +++ b/tests/roots/test-plc-project/src_plc/DUTs/E_Error.TcDUT @@ -0,0 +1,15 @@ + + + + + + \ No newline at end of file diff --git a/tests/roots/test-plc-project/src_plc/DUTs/ST_MyStruct.TcDUT b/tests/roots/test-plc-project/src_plc/DUTs/ST_MyStruct.TcDUT new file mode 100644 index 0000000..1418ba1 --- /dev/null +++ b/tests/roots/test-plc-project/src_plc/DUTs/ST_MyStruct.TcDUT @@ -0,0 +1,12 @@ + + + + + + \ No newline at end of file diff --git a/tests/roots/test-plc-project/src_plc/DUTs/T_ALIAS.TcDUT b/tests/roots/test-plc-project/src_plc/DUTs/T_ALIAS.TcDUT new file mode 100644 index 0000000..a5a354b --- /dev/null +++ b/tests/roots/test-plc-project/src_plc/DUTs/T_ALIAS.TcDUT @@ -0,0 +1,8 @@ + + + + + + \ No newline at end of file diff --git a/tests/roots/test-plc-project/src_plc/MyPLC.plcproj b/tests/roots/test-plc-project/src_plc/MyPLC.plcproj index e9ab387..a5e6a0d 100644 --- a/tests/roots/test-plc-project/src_plc/MyPLC.plcproj +++ b/tests/roots/test-plc-project/src_plc/MyPLC.plcproj @@ -1,5 +1,17 @@ + + Code + + + Code + + + Code + + + Code + Code diff --git a/tests/roots/test-plc-project/src_plc/POUs/F_SyntaxError.TcPOU b/tests/roots/test-plc-project/src_plc/POUs/F_SyntaxError.TcPOU new file mode 100644 index 0000000..ff0f605 --- /dev/null +++ b/tests/roots/test-plc-project/src_plc/POUs/F_SyntaxError.TcPOU @@ -0,0 +1,12 @@ + + + + + + + + + \ No newline at end of file diff --git a/tests/test_plc_project.py b/tests/test_plc_project.py index 8214f95..4527f01 100644 --- a/tests/test_plc_project.py +++ b/tests/test_plc_project.py @@ -17,6 +17,8 @@ def test_project_interpret(app, status, warning): "functionblock": ["FB_MyBlock", "FB_SecondBlock", "PlainFunctionBlock"], "function": ["PlainFunction", "RegularFunction"], "program": ["MAIN"], + "enum": ["E_Error"], + "struct": ["ST_MyStruct"], } for objtype, objects in expected.items(): @@ -29,6 +31,8 @@ def test_project_interpret(app, status, warning): @pytest.mark.sphinx("dummy", testroot="plc-project") -def test_project_build(app, status, warning): +def test_project_build(app, status, warning, caplog): """Test building a document loading a project.""" app.builder.build_all() + # Project contains a function with an outright syntax error, but the project + # completes nonetheless. diff --git a/tests/test_st_grammar.py b/tests/test_st_grammar.py index f6569f9..66b447b 100644 --- a/tests/test_st_grammar.py +++ b/tests/test_st_grammar.py @@ -5,7 +5,7 @@ import pytest import os -from textx import metamodel_from_file +from textx import metamodel_from_file, TextXSyntaxError import re @@ -30,6 +30,7 @@ def meta_model(): "E_Filter.txt", "Properties.txt", "Unions.txt", + "T_ALIAS.txt", "GlobalVariableList.txt", "Main.txt", ] @@ -41,8 +42,8 @@ def test_grammar_on_files(meta_model, file): filepath = os.path.realpath(tests_dir + "/plc_code/" + file) try: model = meta_model.model_from_file(filepath) - except: - pytest.fail(f"Error when analyzing the file `{file}`") + except TextXSyntaxError as err: # noqa + pytest.fail(f"Error when analyzing the file `{file}`: {err}") else: assert model is not None assert ( From 1e2788672a49feb86aca71617290ea72d75210a7 Mon Sep 17 00:00:00 2001 From: RobertoRoos Date: Thu, 28 Nov 2024 11:15:24 +0100 Subject: [PATCH 2/4] Added grammar support for alias types --- src/plcdoc/interpreter.py | 2 ++ src/plcdoc/st_declaration.tx | 9 ++++++++- tests/plc_code/T_ALIAS.txt | 4 +++- tests/test_plc_project.py | 1 + 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/plcdoc/interpreter.py b/src/plcdoc/interpreter.py index 38fa716..87b9fc4 100644 --- a/src/plcdoc/interpreter.py +++ b/src/plcdoc/interpreter.py @@ -289,6 +289,8 @@ def __init__(self, meta_model: TextXMetaClass, file=None): self._objtype = "struct" elif "Union" in type_str: self._objtype = "union" + elif "Alias" in type_str: + self._objtype = "alias" else: raise ValueError(f"Could not categorize type `{type_str}`") diff --git a/src/plcdoc/st_declaration.tx b/src/plcdoc/st_declaration.tx index ec27ebe..6a7ef3f 100644 --- a/src/plcdoc/st_declaration.tx +++ b/src/plcdoc/st_declaration.tx @@ -42,7 +42,7 @@ TypeDef: ; AnyType: - TypeStruct | TypeUnion | TypeEnum + TypeStruct | TypeUnion | TypeEnum | TypeAlias ; TypeStruct: @@ -78,6 +78,13 @@ EnumOption: (comment=CommentLine)? ; +TypeAlias: + base=VariableType + CommentAny* + // Catch trailing comments here, not at the end of `Variable` + ';' +; + /* --------------------------------------------------- */ diff --git a/tests/plc_code/T_ALIAS.txt b/tests/plc_code/T_ALIAS.txt index 4407e01..188edcf 100644 --- a/tests/plc_code/T_ALIAS.txt +++ b/tests/plc_code/T_ALIAS.txt @@ -1,2 +1,4 @@ -TYPE T_INTERLOCK : WORD; +TYPE T_INTERLOCK : WORD; END_TYPE + +TYPE T_Message : STRING[50]; END_TYPE diff --git a/tests/test_plc_project.py b/tests/test_plc_project.py index 4527f01..51490b4 100644 --- a/tests/test_plc_project.py +++ b/tests/test_plc_project.py @@ -19,6 +19,7 @@ def test_project_interpret(app, status, warning): "program": ["MAIN"], "enum": ["E_Error"], "struct": ["ST_MyStruct"], + "alias": ["T_ALIAS"], } for objtype, objects in expected.items(): From f2402790a77cd7b812328f5e5805e7ab70b5a9f4 Mon Sep 17 00:00:00 2001 From: RobertoRoos Date: Thu, 28 Nov 2024 11:29:59 +0100 Subject: [PATCH 3/4] Added grammar support to enum defaults --- src/plcdoc/st_declaration.tx | 7 +++++++ tests/plc_code/E_Filter.txt | 8 ++++++++ tests/plc_code/E_Options.txt | 10 +++++++++- 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/plcdoc/st_declaration.tx b/src/plcdoc/st_declaration.tx index 6a7ef3f..7b4a5a1 100644 --- a/src/plcdoc/st_declaration.tx +++ b/src/plcdoc/st_declaration.tx @@ -67,6 +67,7 @@ TypeEnum: CommentAny* ')' (base_type=Fqn)? + (default=EnumDefault)? ';' ; @@ -78,6 +79,12 @@ EnumOption: (comment=CommentLine)? ; +EnumDefault: + ':=' + option=ID + // Enum default must be a literal field, it cannot be e.g. an integer +; + TypeAlias: base=VariableType CommentAny* diff --git a/tests/plc_code/E_Filter.txt b/tests/plc_code/E_Filter.txt index 1090563..80843db 100644 --- a/tests/plc_code/E_Filter.txt +++ b/tests/plc_code/E_Filter.txt @@ -14,3 +14,11 @@ TYPE E_Filter : Filter697nm := 6 // Note: non-consecutive number! ) USINT; // Specify base type so API is explicit END_TYPE + +TYPE E_FilterDefault : +( + NoFilter := 0, + Filter434nm, + Filter697nm +) USINT := NoFilter; +END_TYPE diff --git a/tests/plc_code/E_Options.txt b/tests/plc_code/E_Options.txt index cb9ff7f..5927810 100644 --- a/tests/plc_code/E_Options.txt +++ b/tests/plc_code/E_Options.txt @@ -1,7 +1,15 @@ TYPE E_Options : ( - Default := 0, + Default := 0, Option1, Option2 ); END_TYPE + +TYPE E_OptionsDefault : +( + Default := 0, + Option1, + Option2 +) := DEFAULT; +END_TYPE From 393247080ee4ede0777ac5b308add4f6d6ce37bf Mon Sep 17 00:00:00 2001 From: RobertoRoos Date: Thu, 28 Nov 2024 11:43:45 +0100 Subject: [PATCH 4/4] Added support for duplicated semicolons --- src/plcdoc/st_declaration.tx | 15 +++++++++++---- tests/plc_code/FB_Variables.txt | 2 ++ tests/test_st_grammar.py | 7 ++++--- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/plcdoc/st_declaration.tx b/src/plcdoc/st_declaration.tx index 7b4a5a1..059761f 100644 --- a/src/plcdoc/st_declaration.tx +++ b/src/plcdoc/st_declaration.tx @@ -68,7 +68,7 @@ TypeEnum: ')' (base_type=Fqn)? (default=EnumDefault)? - ';' + SemiColon ; EnumOption: @@ -89,7 +89,7 @@ TypeAlias: base=VariableType CommentAny* // Catch trailing comments here, not at the end of `Variable` - ';' + SemiColon ; /* @@ -106,7 +106,7 @@ Function: ('EXTENDS' extends=Fqn)? ('IMPLEMENTS' implements=Fqn)? (':' return=VariableType (arglist=ArgList)?)? - (';')? + (SemiColon)? lists*=VariableList ; @@ -167,7 +167,7 @@ Variable: type=VariableType (arglist=ArgList)? (AssignmentSymbol value=AssignmentValue)? - ';' + SemiColon comment=CommentLine? ; @@ -235,6 +235,13 @@ Fqn[noskipws]: /\s/*- ; +/* +Semi-colons may be repeated in valid code +*/ +SemiColon: + ';'+ +; + /* Anything that is considered a value: a literal, a variable, or e.g. a sum */ diff --git a/tests/plc_code/FB_Variables.txt b/tests/plc_code/FB_Variables.txt index 490c5eb..8028425 100644 --- a/tests/plc_code/FB_Variables.txt +++ b/tests/plc_code/FB_Variables.txt @@ -54,6 +54,8 @@ myfloat_no_ws:REAL; mypointer2 : REFERENCE TO UDINT; mypointer3 : REFERENCE TO FB_Motor REF= _motor; + extra_semicolons : INT := 7;;;;;;;;;; + timeout1 : TIME := T#2S; timeout2 : TIME := T#12m13s14ms; diff --git a/tests/test_st_grammar.py b/tests/test_st_grammar.py index 66b447b..f829f9a 100644 --- a/tests/test_st_grammar.py +++ b/tests/test_st_grammar.py @@ -80,8 +80,8 @@ def test_grammar_variables(meta_model): filepath = os.path.realpath(tests_dir + "/plc_code/" + filename) try: model = meta_model.model_from_file(filepath) - except: - pytest.fail(f"Error when analyzing the file `{filename}`") + except TextXSyntaxError as err: + pytest.fail(f"Error when analyzing the file `{filename}`: {err}") else: assert model.functions fb = model.functions[0] @@ -145,12 +145,13 @@ def test_grammar_variables(meta_model): ("mypointer1", "UDINT", None, None, None, "POINTER"), ("mypointer2", "UDINT", None, None, None, "REFERENCE"), ("mypointer3", "FB_Motor", "_motor", None, None, "REFERENCE"), + ("extra_semicolons", "INT", "7", None, None, None), ("timeout1", "TIME", "T#2S", None, None, None), ("timeout2", "TIME", "T#12m13s14ms", None, None, None), ("inline1", "INT", None, None, None, None), ] - assert len(variables) == 44 + assert len(variables) == 45 for i, expected in enumerate(expected_list): assert_variable(variables[i], expected)