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

C# source generator for docopt usage #77

Merged
merged 143 commits into from
Sep 13, 2021
Merged

Conversation

atifaziz
Copy link
Collaborator

@atifaziz atifaziz commented Apr 4, 2021

This PR implements a C# source generator for docopt. It generates strong-typed C# given a *.docopt.txt file.

There is an example of NavalFate using the source generator.

@atifaziz
Copy link
Collaborator Author

atifaziz commented Apr 4, 2021

There is no concept or specification for the final shape of this feature. In its first draft, the source generator simply generates C# source code similar to the T4 template, but it would be a shame to leave it there. It can and should go much further; it can be a lot smarter given that the source generator has access to the syntactic and semantic model of a project at generation-time.

The PR should help to explore and develop the feature.

Meanwhile, take it for a spin:

dotnet run -p src\Examples\SourceGenerator\NavalFate -- ship new foo bar baz

@voieducode
Copy link
Member

I love this - lots to unpack for me, I will take it for a spin!

@atifaziz
Copy link
Collaborator Author

atifaziz commented Apr 5, 2021

I love this - lots to unpack for me, I will take it for a spin!

Cool and needless to say, feature ideas welcome!

I am not sure how well this is doable/mappable, but I'd like to toy with the idea that the source generate can generate code to bind the following usage:

naval_fate ship new <name>...
naval_fate ship <name> move <x> <y> [--speed=<kn>]
naval_fate ship shoot <x> <y>
naval_fate mine (set|remove) <x> <y> [--moored|--drifting]

to the following method signatures:

int ShipNew(string[] name);
int ShipMove(string name, int x, int y, int speed);
int ShipShoot(int x, int y, int);
int Mine(Choice<Set, Remove> action, int x, int y, Choice<Set, Remove>? disposition);

// generated types:

enum Set { Set }
enum Remove { Remove }
enum Moored { Moored }
enum Drifting { Drifting }

Conflicts resolved:

- eg/SourceGenerator/NavalFate/NavalFate.csproj
- eg/SourceGenerator/NavalFate/Program.cs
- eg/SourceGenerator/NavalFate/Program.docopt.txt
For example:

    // Required:
    //   Either:
    //     Required:
    //       Command(ship, False) -> CommandNode ship Bool
    //       Command(new, False) -> CommandNode new Bool
    //       OneOrMore:
    //         Argument(<name>, []) -> ArgumentNode <name> List
    //     Required:
    //       Command(ship, False) -> CommandNode ship Bool
    //       Argument(<name>, []) -> ArgumentNode <name> List
    //       Command(move, False) -> CommandNode move Bool
    //       Argument(<x>, ) -> ArgumentNode <x> String
    //       Argument(<y>, ) -> ArgumentNode <y> String
    //       Optional:
    //         Option(,--speed,1,10) -> OptionNode speed String
    //     Required:
    //       Command(ship, False) -> CommandNode ship Bool
    //       Command(shoot, False) -> CommandNode shoot Bool
    //       Argument(<x>, ) -> ArgumentNode <x> String
    //       Argument(<y>, ) -> ArgumentNode <y> String
    //     Required:
    //       Command(mine, False) -> CommandNode mine Bool
    //       Required:
    //         Either:
    //           Command(set, False) -> CommandNode set Bool
    //           Command(remove, False) -> CommandNode remove Bool
    //       Argument(<x>, ) -> ArgumentNode <x> String
    //       Argument(<y>, ) -> ArgumentNode <y> String
    //       Optional:
    //         Either:
    //           Option(,--moored,0,False) -> OptionNode moored Bool
    //           Option(,--drifting,0,False) -> OptionNode drifting Bool
    //     Required:
    //       Required:
    //         Option(-h,--help,0,False) -> OptionNode help Bool
    //     Required:
    //       Option(,--version,0,False) -> OptionNode version Bool
Example of what's generated:

    static readonly Pattern Pattern =
        new Required(ImmutableArray.Create<Pattern>(
            new Either(ImmutableArray.Create<Pattern>(
                new Required(ImmutableArray.Create<Pattern>(
                    new Command("ship"),
                    new Command("new"),
                    new OneOrMore(
                        new Argument("<name>", null)))),
                new Required(ImmutableArray.Create<Pattern>(
                    new Command("ship"),
                    new Argument("<name>", null),
                    new Command("move"),
                    new Argument("<x>", null),
                    new Argument("<y>", null),
                    new Optional(ImmutableArray.Create<Pattern>(
                        new Option("", "--speed", 1, null))))),
                new Required(ImmutableArray.Create<Pattern>(
                    new Command("ship"),
                    new Command("shoot"),
                    new Argument("<x>", null),
                    new Argument("<y>", null))),
                new Required(ImmutableArray.Create<Pattern>(
                    new Command("mine"),
                    new Required(ImmutableArray.Create<Pattern>(
                        new Either(ImmutableArray.Create<Pattern>(
                            new Command("set"),
                            new Command("remove"))))),
                    new Argument("<x>", null),
                    new Argument("<y>", null),
                    new Optional(ImmutableArray.Create<Pattern>(
                        new Either(ImmutableArray.Create<Pattern>(
                            new Option("", "--moored", 0, null),
                            new Option("", "--drifting", 0, null))))))),
                new Required(ImmutableArray.Create<Pattern>(
                    new Required(ImmutableArray.Create<Pattern>(
                        new Option("-h", "--help", 0, null))))),
                new Required(ImmutableArray.Create<Pattern>(
                    new Option("", "--version", 0, null)))))));
@atifaziz
Copy link
Collaborator Author

atifaziz commented Apr 5, 2021

With 41bf0c0 and 54825a7, the source generator now creates a static version of the pattern tree created from the usage text:

// Required:
//   Either:
//     Required:
//       Command(ship, False) -> CommandNode ship Bool
//       Command(new, False) -> CommandNode new Bool
//       OneOrMore:
//         Argument(<name>, []) -> ArgumentNode <name> List
//     Required:
//       Command(ship, False) -> CommandNode ship Bool
//       Argument(<name>, []) -> ArgumentNode <name> List
//       Command(move, False) -> CommandNode move Bool
//       Argument(<x>, ) -> ArgumentNode <x> String
//       Argument(<y>, ) -> ArgumentNode <y> String
//       Optional:
//         Option(,--speed,1,10) -> OptionNode speed String
//     Required:
//       Command(ship, False) -> CommandNode ship Bool
//       Command(shoot, False) -> CommandNode shoot Bool
//       Argument(<x>, ) -> ArgumentNode <x> String
//       Argument(<y>, ) -> ArgumentNode <y> String
//     Required:
//       Command(mine, False) -> CommandNode mine Bool
//       Required:
//         Either:
//           Command(set, False) -> CommandNode set Bool
//           Command(remove, False) -> CommandNode remove Bool
//       Argument(<x>, ) -> ArgumentNode <x> String
//       Argument(<y>, ) -> ArgumentNode <y> String
//       Optional:
//         Either:
//           Option(,--moored,0,False) -> OptionNode moored Bool
//           Option(,--drifting,0,False) -> OptionNode drifting Bool
//     Required:
//       Required:
//         Option(-h,--help,0,False) -> OptionNode help Bool
//     Required:
//       Option(,--version,0,False) -> OptionNode version Bool

static readonly Pattern Pattern =
    new Required(ImmutableArray.Create<Pattern>(
        new Either(ImmutableArray.Create<Pattern>(
            new Required(ImmutableArray.Create<Pattern>(
                new Command("ship"),
                new Command("new"),
                new OneOrMore(
                    new Argument("<name>", null)))),
            new Required(ImmutableArray.Create<Pattern>(
                new Command("ship"),
                new Argument("<name>", null),
                new Command("move"),
                new Argument("<x>", null),
                new Argument("<y>", null),
                new Optional(ImmutableArray.Create<Pattern>(
                    new Option("", "--speed", 1, null))))),
            new Required(ImmutableArray.Create<Pattern>(
                new Command("ship"),
                new Command("shoot"),
                new Argument("<x>", null),
                new Argument("<y>", null))),
            new Required(ImmutableArray.Create<Pattern>(
                new Command("mine"),
                new Required(ImmutableArray.Create<Pattern>(
                    new Either(ImmutableArray.Create<Pattern>(
                        new Command("set"),
                        new Command("remove"))))),
                new Argument("<x>", null),
                new Argument("<y>", null),
                new Optional(ImmutableArray.Create<Pattern>(
                    new Either(ImmutableArray.Create<Pattern>(
                        new Option("", "--moored", 0, null),
                        new Option("", "--drifting", 0, null))))))),
            new Required(ImmutableArray.Create<Pattern>(
                new Required(ImmutableArray.Create<Pattern>(
                    new Option("-h", "--help", 0, null))))),
            new Required(ImmutableArray.Create<Pattern>(
                new Option("", "--version", 0, null)))))));

This means that there is zero run-time cost of parsing the document usage, which is now all passed to build-time. With the above tree, one technically only needs ParseArgv:

internal static IList<LeafPattern> ParseArgv(Tokens tokens, ICollection<Option> options,
bool optionsFirst = false)

and the PatternMatcher added with PR #82 for the final bit. The runtime cost of these two functions should be very minimal. Ideally, even the PatternMatcher can be statically computed/generated code from the tree, one downside of which would be maintenance of duplicate code (one used for DocoptNet as library code and the other for source generation). The duplication could be removed by some intermediate language that's interpreted at run-time but can also be used to generate code; though that might be too fancy for patterns that are not going to change much given that the docopt specification has been stable for a very long time now.

@atifaziz
Copy link
Collaborator Author

atifaziz commented May 1, 2021

0100f0f adds the first generated version of Docopt.Apply with the entire pattern matching tree in-lined as code. During development, I will be sharing the generated code for those interested in following the progress or providing feedback.

At this stage, I am not even sure the generated code works so the next step is to add tests, fix bugs and then refactor (with tests avoiding regressions).

@atifaziz
Copy link
Collaborator Author

atifaziz commented Sep 2, 2021

Notes for reviewers

There are a large number (75+) of files changed/added in this PR, but keep in mind that 50 of those belong to examples added (from the reference Python implementation) that helped to shape & drive the generated code and testing:

PS> git diff --name-only "$(git merge-base --fork-point master)..HEAD" eg/ | grep -n .

The examples include the generated code and technically don't need to be committed/tracked. They were added nonetheless to help with online review as well as regressions during development. They should/will be removed before the final merge of this PR. There are 14 such files:

PS> git diff --name-only "$(git merge-base --fork-point master)..HEAD" eg/*/obj/* | grep -n .

When you exclude the examples, then changes to review only amount to a third (~25 files):

PS> git diff --name-only "$(git merge-base --fork-point master)..HEAD" | grep -vF eg/SourceGenerator/ | grep -n .

There are even fewer when you consider core project changes (i.e. excluding tests):

PS> git diff --name-only "$(git merge-base --fork-point master)..HEAD" | grep -F src/ | grep -n .
1:src/DocoptNet.CodeGeneration/AnalyzerReleases.Shipped.md
2:src/DocoptNet.CodeGeneration/AnalyzerReleases.Unshipped.md
3:src/DocoptNet.CodeGeneration/CSharpSourceBuilder.cs
4:src/DocoptNet.CodeGeneration/DocoptNet.CodeGeneration.csproj
5:src/DocoptNet.CodeGeneration/DocoptNet.CodeGeneration.targets
6:src/DocoptNet.CodeGeneration/Extensions.cs
7:src/DocoptNet.CodeGeneration/SourceGenerator.cs
8:src/DocoptNet.CodeGeneration/StringBuilderSourceText.cs
9:src/DocoptNet/BranchPatterns.cs
10:src/DocoptNet/Docopt.cs
11:src/DocoptNet/GenerateCodeHelper.cs
12:src/DocoptNet/GeneratedSourceModule.cs
13:src/DocoptNet/Pattern.cs
14:src/DocoptNet/PatternMatcher.cs
15:src/DocoptNet/ValueObject.cs

In the very core project (i.e. excluding the source generator project), the bulk of the changes went into PatternMatcher.cs:

PS> git diff --numstat "$(git merge-base --fork-point master)..HEAD" | grep -F src/DocoptNet/
6       1       src/DocoptNet/BranchPatterns.cs
1       1       src/DocoptNet/Docopt.cs
27      5       src/DocoptNet/GenerateCodeHelper.cs
28      0       src/DocoptNet/GeneratedSourceModule.cs
1       1       src/DocoptNet/Pattern.cs
279     106     src/DocoptNet/PatternMatcher.cs
2       0       src/DocoptNet/ValueObject.cs

PatternMatcher.cs was refactored extensively (by consolidating duplication) so that the generated code could efficiently share as much of the pattern matching logic as possible with the runtime/library version.

Apart from the technical changes, reviewers should focus on the overall design of the generated code in terms of:

  1. Simplicity
  2. Experience
  3. Above all, hindering any customisation or extensibility in the future

@voieducode
Copy link
Member

Super helpful notes, thank you @atifaziz

@atifaziz
Copy link
Collaborator Author

atifaziz commented Sep 5, 2021

In my own review, I noticed that the exit argument of Apply was never used in the generated code and the implementation always acted as if exit: true was the supplied argument. With, fa0c013 (+ 783d813), I've removed the exit parameter entirely. Eventually, I think all the parameters will go away except args, but those tweaks can come in post merge of this PR.

Copy link
Member

@voieducode voieducode left a comment

Choose a reason for hiding this comment

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

After our offline conversation about the PR, I have no additional questions or comments - thanks!

foreach (var (name, value) in arguments)
Console.WriteLine($"{name} = {value}");

Console.WriteLine($@"{{
Copy link
Member

Choose a reason for hiding this comment

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

This is as clean and simple as the Python version. Someone on my team was commenting that there was a lot of magic happening. The good news is that magic is not as costly as one would expect.

@@ -0,0 +1,290 @@
#nullable enable annotations
Copy link
Member

Choose a reason for hiding this comment

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

Source generator is one step towards addressing the ask from issue #8. What do you think @prabirshrestha?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think #76 kind of addressed that already.


namespace ArgumentsExample
{
partial class ProgramArguments : IEnumerable<KeyValuePair<string, object?>>
Copy link
Member

Choose a reason for hiding this comment

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

This generated code is so legible! 😮 - And there are so many ways we could extend this to support new scenarios, really exciting stuff.

@@ -0,0 +1,21 @@
usage: git [--version] [--exec-path=<path>] [--html-path]
Copy link
Member

Choose a reason for hiding this comment

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

I have posted issue #114 to capture a weird scenario where exec-path is both a boolean and a string.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Did you see my reply there?

@atifaziz atifaziz merged commit a8bcd5e into docopt:master Sep 13, 2021
@atifaziz atifaziz deleted the source-generator branch September 13, 2021 15:56
@voieducode
Copy link
Member

Congratulations for the merge!!! 🎉🎉🎉

@atifaziz
Copy link
Collaborator Author

Congratulations for the merge!!! 🎉🎉🎉

Thanks! Slowly, but surely, made it to the finishing line! 🏁 Appreciate all your trust, patience and support during the months it took to work through this PR! 😄

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.

2 participants