Update generator to use validtypes - #48
Draft
ian-noaa wants to merge 35 commits into
Draft
Conversation
Validtypes now handle data validation & error handling so refactor the fill functions we generate for the linetypes packages to take advantage of that. Due to this we can remove the SetValueForField & GetLeadFromInitValid functions. GetLeadFromInitValid was unused - there are no "LEAD" terms.
- Pull Document & Header struct creation out into standalone functions.
- Move the majority of strings to raw strings to preserve formatting.
- Start migrating the generator to have 3 steps. Get data, assemble it
into the format for code generation. Then generate the code.
- We still need to separate concerns.
This is solely whitespace & comment updates.
With the exception of switch/case ordering in GetDocForID and AddDataElement; this change doesn't make changes to the generated code. This refactor does a number of things: - moves code generation from string manipulation to Go's text/templates. - consolidates each generated function into a distinct template - separates the main function into 3 distinct steps gathering data, assembling data to use for code generation, and then code generation. - To support the separation of main, it splits existing fill functions into distinct steps, focused on the template that it is providing data for. - sorts the case ordering in the switch statements in GetDocForID and AddDataElement. This change improves maintainability of the generator function, mainly by separating concerns & centralizing code generation.
Changes include: - adding spacing before comments denoting different sections - ordering the switch/case statements in GetDocForID and AddDataElement
Update the generator to address some linter errors. - don't enforce tagliatelle settings on `json:"data"` tags. - propagate errors generated by any UnmarshalText calls. - add a helper function to reduce boilerplate for using errors.Join()
Now that we have structs to represent a full document, we can switch addDataElement to be a method on each struct. This change: - Updates code generation ordering to reflect that addDataElement is a method of the parent document struct - Adds a METdocument interface to util so we can easily refer to any document struct - Preserves type information in our map of ID keys & document structs This introduces a change to the signature of ParseLine() - we now require our map of docs to be of type map[string]METdocument.
This better describes the purpose of the function - constructors in Go typically start with "New...".
These tests don't require external resources, like the contents of MET-parser-testdata
We need to be able to easily extract the ID from a METdocument interface. We want to keep the list of methods in METdocument minimal. However, we may also want to allow LINE_TYPE extraction so we can more easily determine which type we have, especially if we wanted to avoid reflection.
This adds the GetID() method to our linetypes so we can easily extract the document ID from the METdocument interface.
Main changes are that we now use the METdocument interface in place of map[string]interface's, and that we use the new METdocument.GetID() method to retrieve the document ID.
There was a problem hiding this comment.
Pull Request Overview
This PR updates the generator to use validtypes for field types instead of primitive Go types, introduces an interface for MET documents to improve type safety, and refactors the codebase to use structured types throughout.
- Updates generator to produce validtypes-based field types (ValidInt, ValidFloat, ValidString) instead of primitive types
- Introduces METdocument interface to standardize document operations
- Refactors parser and test code to use strongly-typed documents instead of map[string]interface{}
Reviewed Changes
Copilot reviewed 6 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/util/util.go | Adds METdocument interface and DataSetName field to VxMetadata, removes dataSetName parameter from GetId function |
| pkg/parser/parser_test.go | Updates test functions to use METdocument interface and removes test skipping logic |
| pkg/parser/parser.go | Refactors ParseLine to use METdocument interface and removes deprecated map conversion functions |
| generator/generator.go | Complete rewrite using templates and validtypes, generates code with structured approach instead of string concatenation |
| examples/sample_parser/sample_parser.go | Updates sample parser to use METdocument interface |
| docs/development-notes.md | Updates documentation to reference NewDocForId instead of deprecated GetDocForId |
util.GetID() was previously setting the ID field on the passed-in VxMetadata struct and returning a pointer to the updated struct. This made usage unclear. Returning the ID string makes the purpose of BuildID clear. This could have been a method on the VxMetadata struct. However, it requires that other fields in VxMetadata be correctly set so requiring them as part of the function signature here seemed clearer.
_err and err were both in use and were occasionally shadowing each. Using one variable name for errors reduces the opportunities for confusion.
This is slightly concerning as ideally, having the wrong type would've failed.
METdocument and VxMetadata are needed by consumers of the library, so it's safest to separate them from the rest of the helpers in util.
There are now some test failures I need to figure out.
I'm slowly learning how these functions work in order to understand why overrideDefinedMetDataTypes isn't overriding a token type as expected
Move the token type overriding out into main to make what's happening more apparent.
Additionally, separate token detection from code generation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.