Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add dlang codegen #623

Merged
merged 19 commits into from Nov 12, 2022
Merged

Add dlang codegen #623

merged 19 commits into from Nov 12, 2022

Conversation

tom-tan
Copy link
Member

@tom-tan tom-tan commented Nov 8, 2022

This request adds a new codegen for D programming language (dlang).
It only generates a declaration for each data structure and the parser is automatically generated from the declaration with schema-salad-d.
My personal goal is to replace my hand-written CWL parser with auto-generated one.

Todo (needed to be merged):

  • Add tests!
    • What kind of tests are needed to make it ready to be merged?
    • Will be added to cwl-d-auto
  • Make tests passed
    • CWL v1.0
    • CWL v1.1
    • CWL v1.2
      • Except the backend YAML parser issues
  • documents for usage

Future works of codegened parser:

  • support extension fields
  • support subScope
  • Support YAML v1.2 spec (currently it only works in YAML v1.1 due to its backend YAML parser)
  • Add workaround for https://issues.dlang.org/show_bug.cgi?id=20443
    • Generated parser may not be compiled with the latest D compiler in the case that the schema contains mutually recursive structure

Here is a list of known issues

  • Support secondaryFilesDSL
    • need fix in both of dlang codegen in schema-salad and schema-salad-d
  • cannot parse YAML v1.2 -> Added to README
    • example:
      hints:
        DockerRequirement: {dockerPull: debian:stretch-slim}
    • It is due to the backend parser. I leave it as is and add the limitation to README.

@lgtm-com
Copy link

lgtm-com bot commented Nov 8, 2022

This pull request introduces 1 alert when merging b33356a into 3059ff0 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@tom-tan
Copy link
Member Author

tom-tan commented Nov 9, 2022

I updated schema-salad-d to make generated parser with cleaner interface.

Also, I consider to remove or rename hand-written parser in schema-salad-d because it occupies cwl module name.
IMO it is better for users if cwl-d-auto can provide parsers as cwl modules.

@tom-tan
Copy link
Member Author

tom-tan commented Nov 9, 2022

I updated schema-salad-d and codegen to correctly handle Expression.
I guess other improvements can be done without modifying codegen.

I will add a function to add unittest for given examples to make this request ready to merge.

@tom-tan
Copy link
Member Author

tom-tan commented Nov 9, 2022

Now it can support --codegen-examples!

It adds unittest block with an annotation in the end of the code.
It makes the generated code self-contained and also make the unittest controllable with third party testing framework such as silly.

@tom-tan
Copy link
Member Author

tom-tan commented Nov 10, 2022

Now it mostly passes all the tests for CWL v1.0, v1.1 and v1.2!

The rest of the failed tests are due to the limitation of the backend YAML parser that only supports v1.1 spec.
I will add this limitation to README.

@tom-tan
Copy link
Member Author

tom-tan commented Nov 10, 2022

Code cleanup is needed but it is mostly ready to review!
I updated README in common-workflow-lab/cwl-d-auto to show main features, how the parsers generated, how to load and store CWL documents and future plan.

@tom-tan tom-tan marked this pull request as ready for review November 10, 2022 20:19
Copy link
Member

@mr-c mr-c left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woohoo!

Can you add a dlang entry to the table at https://github.com/common-workflow-language/schema_salad#codegen-examples ?

@tom-tan
Copy link
Member Author

tom-tan commented Nov 11, 2022

Added!

@codecov
Copy link

codecov bot commented Nov 11, 2022

Codecov Report

Merging #623 (60f5a33) into main (7662c7f) will increase coverage by 0.03%.
The diff coverage is 84.51%.

@@            Coverage Diff             @@
##             main     #623      +/-   ##
==========================================
+ Coverage   83.39%   83.43%   +0.03%     
==========================================
  Files          19       20       +1     
  Lines        3885     4039     +154     
  Branches     1050     1096      +46     
==========================================
+ Hits         3240     3370     +130     
- Misses        414      426      +12     
- Partials      231      243      +12     
Impacted Files Coverage Δ
schema_salad/main.py 78.14% <ø> (ø)
schema_salad/dlang_codegen.py 83.89% <83.89%> (ø)
schema_salad/codegen.py 94.59% <100.00%> (+0.25%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@mr-c mr-c left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking great!

Can you add very minimal tests like we have for c++ in

def test_cwl_cpp_gen(tmp_path: Path) -> None:
?

@tom-tan
Copy link
Member Author

tom-tan commented Nov 12, 2022

I added a unittest for it!

@mr-c mr-c merged commit 1f3fa02 into main Nov 12, 2022
@mr-c mr-c deleted the dlang branch November 12, 2022 16:56
@mr-c
Copy link
Member

mr-c commented Nov 12, 2022

Thank you very much, @tom-tan !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants