oxidecomputer / oxidecomputer/typify
Major rewrite notes
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 898
- Forks
- 114
- Avg merge
- 4h 18m
- Merged PRs (30d)
- 14
Description
In the course of building and enhancing typify I've found many shortcomings of the current design. This issue is to catalogue those observations and ideas.
Moving away from numeric IDs
Managing a numeric ID space, allocating IDs, looking up by IDs... it's not very Rust-y. I think we could do something far more idiomatic by referencing types with an Rc<RefCell<TypeEntry>> -- we could call it a TypeRef. Anywhere we currently have a TypeId embedded within a TypeEntry (its details and variants, etc.) would become a TypeRef. This would allow for two things: direct use of the referenced type (i.e. we would not need to pass around the TypeSpace everywhere to do lookups), and (so-called) "interior-mutability" by which we could modify types as we walk the graph. The latter would be helpful e.g. when resolving type-containment cycles.
Regarding type-containment cycles. They're bad. Why? Because the generated code won't build. We need to break them by replacing some of the types with Box<T> of the type. With the RefCell we could borrow_mut() and break the cycles as we find them. Without it, we would need to effectively discover all splices required, return out a list, and then process the list (i.e. once no longer holding the shared references required when talking the type graph).
Type states
We have some (many?) implicit assumptions floating around. Some that are the most subtle are the implied constraints when processing a collection of types that may be the target of a $ref construct. In particular, while we can resolve references (lookup by name in a string -> type mapping) we can't inspect the referenced TypeEntry. In current terms this means we can't self.id_to_entry.get(type_id).unwrap() because we may not yet have converted the referenced type. In the RefCell world above, it means that we may not borrow() or borrow_mut() on the referenced type because we might either get the converted type or a placeholder reference (the latter of which would be RefCell::replaced).
Ideally we would enforce these sorts of constraints using the type system. We need to be thoughtful and careful about this design so that we don't end up copying huge amounts of state.
Note that we can avoid some of this if the type graph is acyclic by topo-sorting, but ideally we'd like to support type graphs that contain reference cycles.
Type bundles / transactions
Related to type states, we might think about the conversion process in bundles or transactions: whether for a single schema or a dictionary of schemas (i.e. references), we could build up a bundle of types, navigate them through various states, and then apply them to the TypeSpace. Note that these bundles could (and would) contain references (TypeId or TypeRef) to extant types, and that would be fine.
This could help clean up error semantics: if a schema conversion failed, the TypeSpace could be kept in a known, consistent, unperturbed state rather than potentially containing some of a converted dictionary (for example).
Separate mutable and immutable structures
This may be less of a concern if we move to the interior mutable of TypeRef, but currently we can get snagged on shared references to TypeSpace::definitions while passing around an exclusive reference during conversions. We could avoid this by separating out mutable and immutable state as we pass it around various functions.
This can happen, for example, if we "resolve" a schema i.e. pick it apart and chase references. This process results in the TypeSpace being borrowed (via the borrow on the Schema) which means we can't get the exclusive reference to e.g. add new types.
Conversion on Schemas only
This is already mostly true, but we should enforce that the conversion step only deals with raw schemas and dictionaries, but (as noted above) prohibit this pass from looking at processed types (which may not yet be fully processed).
We should have as many passes as we want, moving between different IRs; each pass should be forced to look only at structures that are stable during the pass. We should not look at structures being actively mutated during the pass.
Optional dependencies
Right now we assume that users are fine using chrono and uuid. We might add some other dependencies, for example one for constrained numeric types. It would be cool if users could input the list of dependencies they're willing to have. Indeed, if schemars or something similar included an extension to identify a type, we could even automatically sub in types from an allow-list of crates.
Naming and defaults
Naming and defaults are both inconsistently and verbosely handled. We get a bit confused because most conversion functions return both metadata and a type. We use the metadata e.g. to add docs to a struct property. We should evaluate metadata that's directly applicable to the generated type and then pass out metadata explicitly and exclusively for use e.g. to construct struct properties. Note that these may have some overlap, for example (maybe) with defaults.
Variant names
In particular for untagged enums, it can be challenging to infer a good variant name when initially decoding the schema. It might be reasonable to add an explicit pass through the graph where we revisit variant naming. This pass would likely happen before containment cycle elimination.
Closer-to-zero copy
We don't need to get crazy about zero copy, but we can do more to copy less. In particular, it would be cool to have types retain their JSON schema information without needing to copy it around everywhere. This is relevant for both debugging and generation e.g. of a JsonSchema impl or doc comments.
IR / Canonical forms
In particular to handle different schema types, we should have an intermediate representation that is slightly less redundant than base JSON schema. For example, use enum rather than const, eliminate if / then / else in favor of oneOf: [ allOf: [ if, then ], allOf: [ not: [if], else ] ], etc. Beyond that we probably want to take another pass to put things into a canonical form (or perhaps that would be another IR).
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by tracing TypeSpace, TypeEntry, TypeId, Schema, and the conversion passes described in the issue, since no specific files or tests are named. This is a broad redesign covering references, type states, transactions, dependencies, naming, intermediate representations, and cycle handling; done would require an agreed design plus coordinated implementation across those areas.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- compilers
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 15/100