Enhancement: support access specifiers for function block declarations

[engineerjoe440]

Changes:

• Enhances grammar to qualify function blocks with more broad access specifiers, including:
  • ABSTRACT (pre-existing)
  • PUBLIC
  • PRIVATE
  • PROTECTED
  • INTERNAL
  • FINAL
• Adds tests to validate 61131 source directly, without the use of *.TcPOU files.

Closing Thoughts:

Please let me know if there are any questions or concerns with the changes! I have a few other enhancements I would like to contribute, and I want to make sure that my contributions are effective, and in-line with the direction you want to maintain.

Thank you so much!

[klauer]

Thanks for the help here! Overall, this is looking good. This is an important missing/mistaken piece in the current grammar.

There will be a few more things - if you're comfortable poking around, I'd be happy for your contributions. If not, we can merge and I can do a follow-up PR.

1. blark.transform.FunctionBlock will require some changes since it's no longer just "ABSTRACT" which we support.

   • https://github.com/engineerjoe440/blark/blob/be1e969d2fdc830318f996c42022840f520458d1/blark/transform.py#L1791
   • This will need to be switched to Optional[MethodAccess]
   • Annotations below will need to be updated as well

2. The transform classes support round-tripping of source code (modulo spacing information, since this is an abstract syntax tree and not a full syntax tree)

   • So to that end, the __str__ method will need updating to reflect the change in access.
   • Take a look at how the Method or Property ones work, since this is pretty similar. The short of it is that the _FlagHelper base class takes care of creating the space-delimited strings.
   • https://github.com/engineerjoe440/blark/blob/be1e969d2fdc830318f996c42022840f520458d1/blark/transform.py#L1818

3. I'd like to get some more test coverage in test_transform to ensure the above are working sufficiently

   • The test_parsing suite is more tailored to "will the source be parsed without erroring out?" - smoke tests of sorts
   • The test_transform suite is "can it be parsed, transformed, and round-tripped back to source code?"
   • You'll see a lot more test parametrization and coverage there as opposed to the lackluster test_parsing

4. I'd be inclined to rename MethodAccess to AccessSpecifier to be more general. It started out as just for methods, then I realized properties used the same, and now function blocks.

[engineerjoe440]

Thank you so much for your input and suggestions! I'm about to add some additional tests to test_transform to get some more coverage there. I'll try to keep that to one commit more for ease of review.

Thank you again for your willingness to guide and share your work!

[engineerjoe440]

Thank you again for letting me muddle my way through this, and become familiar with your code-base. I really appreciate it! I think with this most recent set of commits, this PR should be ready for your review. 😄

[klauer]

This all looks great to me!

One last question about the vscode settings (above) and then I'll be happy to merge this in.

[klauer]

As mostly a vim user, I'm not too familiar with how vscode settings are distributed with source packages. Is version-controlling settings.json generally good practice?

[engineerjoe440]

This is an excellent question!

In earnest, I think it's really at your discretion...

I like to keep the .vscode settings in the repo so that they'll be picked up across platforms and by any other developers using VSCode, that said, there's no inherent NEED for them, and could just as easily be lobbed into the .gitignore file. I find it generally helpful to keep them around just like any other development tooling files (like ..gitignore, or .pre-comit-config.yaml, or anything else).

[klauer]

Gotcha. I'm OK with leaving it in for now, at least.