klauer / blark

Beckhoff TwinCAT ST (IEC 61131-3) code parsing in Python using Lark (Earley)
https://klauer.github.io/blark/
GNU General Public License v2.0
42 stars 5 forks source link

Variables Containing the word "Return" Incorrectly Identified as `RETURN` Statements. #67

Closed engineerjoe440 closed 1 year ago

engineerjoe440 commented 1 year ago

Found this one while testing some more "in the wild" code.

It seems that if a variable's name begins with the word "Return" blark will incorrectly identify that word as a RETURN statement instead of part of the variable. I will need to confirm, but I suspect this may also be the case with CONTINUE and BREAK.

Here's an example I'm adding as a test-case that will hopefully lead to an improvement pull-request.

            FUNCTION_BLOCK fbName
                iValue := 1;
                IF 1 THEN
                    iValue := 1;
                    IF 1 THEN
                        iValue := 1;
                    END_IF
                END_IF
                Method();
                ReturnStatus := mReturnStatus;
            END_FUNCTION_BLOCK

blark-parsed results:

FUNCTION_BLOCK fbName
    iValue := 1;
    IF 1 THEN
        iValue := 1;
        IF 1 THEN
            iValue := 1;
        END_IF
    END_IF
    Method();
    RETURN;              //  <-- Here's the offender
    Status := mReturnStatus;
END_FUNCTION_BLOCK
Parse Tree ``` The parse tree is: function_block_type_declaration FUNCTION_BLOCK None fbName None None statement_list assignment_statement variable_name iValue None expression assignment_expression or_else_expression and_then_expression xor_expression and_expression comparison_expression equality_expression add_expression expression_term power_expression unary_expression None constant integer_literal None integer 1 if_statement expression assignment_expression or_else_expression and_then_expression xor_expression and_expression comparison_expression equality_expression add_expression expression_term power_expression unary_expression None constant integer_literal None integer 1 statement_list assignment_statement variable_name iValue None expression assignment_expression or_else_expression and_then_expression xor_expression and_expression comparison_expression equality_expression add_expression expression_term power_expression unary_expression None constant integer_literal None integer 1 if_statement expression assignment_expression or_else_expression and_then_expression xor_expression and_expression comparison_expression equality_expression add_expression expression_term power_expression unary_expression None constant integer_literal None integer 1 statement_list assignment_statement variable_name iValue None expression assignment_expression or_else_expression and_then_expression xor_expression and_expression comparison_expression equality_expression add_expression expression_term power_expression unary_expression None constant integer_literal None integer 1 None None function_call_statement function_call variable_name Method None None return_statement assignment_statement variable_name Status None expression assignment_expression or_else_expression and_then_expression xor_expression and_expression comparison_expression equality_expression add_expression expression_term power_expression unary_expression None variable_name mReturnStatus None END_FUNCTION_BLOCK ```
engineerjoe440 commented 1 year ago

Update:

Looks like CONTINUE statements also get incorrectly identified:

FUNCTION_BLOCK fbName
    iValue := 1;
    IF 1 THEN
        iValue := 1;
        IF 1 THEN
            iValue := 1;
        END_IF
    END_IF
    Method();
    CONTINUE;    // <-- how did that get there?
    Working := somethingElse;   //  Should have been `ContinueWorking := somethingElse;`
END_FUNCTION_BLOCK
klauer commented 1 year ago

Let's sweep this mistake under the rug... 😁

Closed by #68 - thanks, Joe!

engineerjoe440 commented 1 year ago

Happy to close it out! Especially since I'm pretty sure it was my hand in the pie for some of these issues. 😕 Haha! 😆

Thank you, Ken!