oxc-project / oxc-project/backlog
Enforce valid AST via types
Nobody has claimed this yet.
- Dominant language
- No language data
- Stars
- 7
- Forks
- 0
- PR merge metrics
- No merged PRs in 30d
Description
The AST types currently allow various invalid ASTs.
Two examples:
Function declaration or class declaration without an ID
Function expressions can be without an ID (e.g. f = function() {}) but function declarations must have an ID.
To cover both cases, Function's id field is Option<BindingIdentifier<'a>>.
This means when dealing with a function/class declaration, to get the function name, you have to use fn.id.unwrap() (e.g. https://github.com/oxc-project/oxc/pull/3477). This produces unnecessary branches and panic code.
Alternatively you can do if let Some(id) = fn.id, but this makes the code harder to understand, as it reads as if fn.id could also be None.
Class::type field
Class's type field says whether the class is an expression or a declaration. This is also implied by whether it's a Declaration::ClassDeclaration or an Expression::ClassExpression.
3 problems with this:
- Duplication of information = unnecessary memory use (though it's extremely minimal).
- The 2 can get out of sync.
- Some places where you have to assert the invariant with
unreachable!.
Other cases
There are various other examples of similar situations elsewhere in the AST.
The need to maintain these invariants without the compiler's help is at times a footgun, and makes for less readable code.
I think we should encode these invariants in the AST types. For example, have separate types for ClassExpression and ClassDeclaration. This would make it statically impossible to produce invalid ASTs, and reduce the scope for mistakes.
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 locating the AST type definitions and their consumers. No files, tests, or entry points are named in the issue, so first map the existing declaration and expression types and their invariants. Done means the described invalid ASTs are statically unrepresentable and related consumers no longer need invariant assertions or unnecessary unwrapping.
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
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100