boardgame icon indicating copy to clipboard operation
boardgame copied to clipboard

Clean up implementation of codegen pkg

Open jkomoros opened this issue 5 years ago • 3 comments

If you run boardgame-util codegen in a package that doesn't have any reader structs decorated with //boardgame-util:codegen, then it outputs auto_reader.go, with imports but no code, which will then cause the package to not compile since it has unused imports.

This is rarely run into in practice because you have to invoke boardgame-util codegen via //go:generate boardgame-util codegen, and if you included that then you almost certainly have at least one struct that needs a reader.

Noticed while working on #741

jkomoros avatar Dec 03 '19 14:12 jkomoros

Actually, codegen needs more cleanup anyway, making this issue mor general.

Importing from #741: codegen.ProcessReaders is a ball of spaghetti. Create an internal struct that is codeGenStruct for each struct to output code for that encapsulates all of the information necessary to append generated code to output, which will include type names as well as the struct name, whether it has a FinishStateSetUp, which of read, readset, and readsetconfigurer to output, and which behaviors need to be connected, etc

jkomoros avatar Dec 03 '19 14:12 jkomoros

  • [x] Fix bug where if there are no readers to generate an auto_reader.go with no substantive code is emitted
  • [x] finish moving boardgame.PropertyType methods into boardgame package that belong e.g. zeroValue
  • [ ] Consider moving the various info methods on PropertyTYpe to PropertyTYpe.Info().WHATEVER to avoid polluting the main namespace and make it clear that all of htem are most useful in codegen contexts.
  • [ ] Switch to passing only anon structs to template
  • [x] Factor shared template properties down into embeddeable substructs
  • [x] can get rid of ismutable, etc helpers now that we just use types.
  • [ ] boardgame.PropertyType grows. ImmutableSpecificGoTypes which returns e.g. ["ImmutableRangeVal", "ImmutableTreeVal"]. Also will need PropertyType.PackageImport() to return e.g. "enum" or ""
  • [ ] Consider using https://godoc.org/golang.org/x/tools/go/packages to load files and discover their location... and to go/types to process type info without using the marcgrol package (although that doesn't have doclines does it? ... that's OK; just have a top-line thing to process a single package to extract the struct names that have the magic comment, and then hand off to a normal parsing thing with the map of structs to export and do all of that with the go.Types package)
  • [ ] at init go through and make a map of all possible type strings we'll come across (including all SpecificTypes) by enumerating all PropertyTypes, and noting unique MutableGoType and ImmutableGoType.
  • [ ] Update to use a newer version of marcgrol
  • [ ] Get rid of allStructs in structFields; instead every time you look for another struct, pass a getStruct(packageName, structName strring) model.Struct and have that be memoized and keep track of every struct in every package its ever returned.
  • [ ] use the boardgame library methods on PropertyType in templates directly and get rid of special code in codegen for it (structTypes is the main remaining example)
  • [ ] typeInfo should be renamed to be fieldInfo and represent those three properties for a given key
  • [ ] make a propertyType which wraps boardgame.PropertyType and adds methods for zeroValue, etc, etc
  • [ ] Move all calculation for output generation (e.g headerForStruct) into private methods on readerGenerator
  • [ ] hoist readerGenerator.types information directly into readerGenerator struct
  • [ ] typeInfo shouldn't be three maps, it should be a map of type name to a struct with those three values
  • [ ] newReaderGenerator ideally would only take a model.Struct
  • [ ] Clean up all of the enum generator code in a similar way
  • [ ] Remove dependency on marcgrol/parser

jkomoros avatar Dec 05 '19 13:12 jkomoros

348c901fc1e5dd59c91dfb7ed1e53f62e7e4ae4d and 781ab3670d2fafa116811f045d139e20b6dbb938 are also part of this but were erroneously labeled as being part of #764

806e189f2321a7e523145b8ae00d8fd9853ab61f should have said issue 746

jkomoros avatar Dec 10 '19 13:12 jkomoros