oxc-project / oxc-project/backlog

Enforce valid AST via types

Open
#28 2 comments 0 reactions 0 assignees View on GitHub

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:

  1. Duplication of information = unnecessary memory use (though it's extremely minimal).
  2. The 2 can get out of sync.
  3. 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

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.