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

Split Go generation into gogen package. #695

Merged
merged 31 commits into from
Jun 9, 2022
Merged

Conversation

wenovus
Copy link
Collaborator

@wenovus wenovus commented Jun 6, 2022

The change includes the following notable changes:

  • gogen/codegen.go: Instead of using ygen.NewYANGCodeGenerator as the struct for all
    code generation from the single ygen package, gogen.NewGoCodeGenerator is created for Go
    generation. GoOpts is moved in gogen.
  • gogen/genir_test: Since IR generation requires a suitable LangMapper implementation,
    the tests using GoLangMapper are moved to here.
  • gogen/gogen.go, goelements.go: These are moved to this package.
  • gogen/helpers.go: Go generation-specific helpers are moved here.
  • internal/igenutil/genutil.go: Non-Go-specific generation helpers are
    moved here.
  • ygen/genstate_test.go: A fake LangMapper implementation is created for
    unit testing.

I think it makes sense for this to be exported. Not sure why I wanted it
to be extracted from the plugin originally.
The idea is to to call a different method from the interface for each level of hierarchy of the IR to populate the `Flag` fields for that level of the IR. So within `getOrderedDirDetails` for each `ParsedDirectory` and `NodeDetails`, and within the loop of genEnums (in `GenerateIR`) for each `EnumeratedYANGType`. This way the caller of `GenerateIR` is able to produce extra information for any level of the `IR`.

Tests added in genir_test.go
Currently, the individual LangMapper implementation instances (GoLangMapper and ProtoLangMapper) are required to hold these types, which can be confusing to the user since these are not fundamentally part of the LangMapper API -- rather it's their methods (i.e. enum name look-up and leafref resolution) that are part of the LangMapper API interface.

Embedding a special interface (`LangMapperBase`) not only decouples the implementation from the interface, but also provides a clear forward compatibility contract of methods (incl. enum name look-up and leafref resolution) available to implementors should more functionality become available in the future.

The proposed vision is:
* `LangMapper`-defined methods need to be implemented by LangMapper implementors.
* `LangMapperBaseSetup`-defined methods are set-up methods that are called either within `GenerateIR`, or by the unit tests of LangMapper methods.
* `LangMapperBase`-defined methods NOT part of the overall `LangMapper` interface are helper methods available to `LangMapper` implementors in their method implementations.

I will modify the design doc once all is agreed-upon and merged.
This allows unit tests to be created by custom LangMappers on their methods (e.g. `FieldName`, `KeyLeafType`).
The change includes the following notable changes:
* gogen/codegen.go: Instead of using `ygen.NewYANGCodeGenerator` as the struct for all
  code generation from the single `ygen` package, `gogen.NewGoCodeGenerator` is created for Go
  generation. `GoOpts` is moved in `gogen`.
* gogen/genir_test: Since IR generation requires a suitable `LangMapper` implementation,
  the tests using `GoLangMapper` are moved to here.
* gogen/gogen.go, goelements.go: These are moved to this package.
* gogen/helpers.go: Go generation-specific helpers are moved here.
* internal/igenutil/genutil.go: Non-Go-specific generation helpers are
  moved here.
* ygen/genstate_test.go: A fake LangMapper implementation is created for
  unit testing.
@wenovus wenovus requested a review from robshakir June 6, 2022 18:57
@coveralls
Copy link

coveralls commented Jun 6, 2022

Coverage Status

Coverage decreased (-0.1%) to 90.44% when pulling 5566cc9 on split-gogen-package into 9a1316c on master.

@wenovus wenovus changed the base branch from delete-GetDirectoriesAndLeafTypes to add-coverpkg June 7, 2022 02:57
Copy link
Contributor

@robshakir robshakir left a comment

Choose a reason for hiding this comment

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

Thanks for this -- this looks like a big step in encapsulating the Go logic in one place, great work!

.github/workflows/go.yml Outdated Show resolved Hide resolved
generator/generator.go Outdated Show resolved Hide resolved
generator/generator.go Outdated Show resolved Hide resolved
gogen/codegen.go Outdated Show resolved Hide resolved
gogen/codegen.go Show resolved Hide resolved
gogen/codegen.go Outdated Show resolved Hide resolved
gogen/codegen.go Show resolved Hide resolved
gogen/codegen.go Show resolved Hide resolved
gogen/goelements.go Outdated Show resolved Hide resolved
gogen/gogen.go Show resolved Hide resolved
Base automatically changed from add-coverpkg to delete-GetDirectoriesAndLeafTypes June 9, 2022 16:25
Base automatically changed from delete-GetDirectoriesAndLeafTypes to langmapperbase-setup-methods June 9, 2022 16:25
Base automatically changed from langmapperbase-setup-methods to langmapper-base June 9, 2022 16:26
Base automatically changed from langmapper-base to no-mapped-type-enumgen June 9, 2022 16:26
Base automatically changed from no-mapped-type-enumgen to langmapper-ext-typename June 9, 2022 16:26
Base automatically changed from langmapper-ext-typename to langmapper-ext June 9, 2022 16:26
Base automatically changed from langmapper-ext to identitybasename June 9, 2022 16:27
Base automatically changed from identitybasename to master June 9, 2022 16:34
wenovus and others added 2 commits June 9, 2022 09:43
* Split Proto generation into `protogen` package.

The change includes the following notable changes:
* protogen/codegen.go: Instead of using `ygen.NewYANGCodeGenerator` as the struct for all
  code generation from the single `ygen` package, `protogen.NewProtoCodeGenerator` is created for Proto
  generation. `ProtoOpts` is moved to `protogen`.
* protogen/genir_test: Since IR generation requires a suitable `LangMapper` implementation,
  the tests using `ProtoLangMapper` are moved to here.
* protogen/protogen.go, protoelements.go: These are moved to this package.
* integration_tests/generate_errs_test.go: Moved `TestGenerateErrs` out of ygen.

* remove unused code

* remove unused variable

* add coverpkg

* Try again with coverpkg

* Try again with coverpkg

* Fix syntax error

* coverpkg=all

* coverpkg=./...

* coverpkg filter out irrelevant packages

* try to fix coverpkg

* try to fix coverpkg

* use bash instead of coveralls

* Remove 'proto' from various exported names

* GenerateProto3 -> Generate

* Delete unused variables and rephrase package comments for ygen. (#697)

* Delete unused variables and rephrase package comments for ygen.

* Fix `IdentityType` classification bug. (#699)

* Fix `IdentityType` classification bug.

Right now all typedefs are automatically categorized as `DerivedEnumerationType`; this is wrong because identity types could also be in a typedef.

Fixed this bug and added tests.

* Remove debugging statement

* Add coverage for proto generation of typedef identityref

* Run go generate ./... for CI check (#703)

* Run go generate ./... for CI check

* Regenerate exampleoc but don't depend on it for tests

* Delete `StoreRawSchema` from `GeneratorConfig` since it's not being used (#705)

* Delete `StoreRawSchema` from `GeneratorConfig` since it's not being used

I don't see the value of why we need this flag: is it to save memory?
For `exampleoc` this variable took 27MB, a negligible amount.

* Reorganize Code Generation flags and Delete `ygen.GeneratorConfig` (#706)

* ygen now only has `IROptions`.
* `CodeGenerator` in gogen/protogen now carry IROptions and their language-specific post-IR generation options.
* Code generation specific flags that are not used during IR generation are moved to `gogen` and `protogen`, duplicating if used by both (e.g. CallerName).
* `SkipEnumDeduplication` is moved from ParseOptions to TransformationOptions.
@wenovus wenovus merged commit 0793c34 into master Jun 9, 2022
@wenovus wenovus deleted the split-gogen-package branch June 9, 2022 17:09
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

3 participants