Skip to content

Remove mixed enums; add ast-grep#116

Merged
just-be-dev merged 5 commits intomainfrom
no-mixed-enums
Feb 7, 2025
Merged

Remove mixed enums; add ast-grep#116
just-be-dev merged 5 commits intomainfrom
no-mixed-enums

Conversation

@just-be-dev
Copy link
Copy Markdown
Owner

@just-be-dev just-be-dev commented Feb 7, 2025

@manzt pointed out some broken generation in the python client in #78. After investigating this for a bit, it looks like it's due to mix enum styles in the rust code.

Here's a basic example

pub enum WindowSize {
    States(WindowSizeStates),
    Size { width: f64, height: f64 },
}

My typescript generation code outputs the following for this case

export type WindowSize = WindowSizeStates | {
  height: number;
  width: number;
};

Typescript has the advantage of allowing for anonymous objects in unions. What should the second object be in python? Currently my code is spitting out a class called undefined because I don't have a name for it.

As with #108, often the solution is to simplify the rust data structures to simplify the generation. The final structure I ended up with for the above example is

pub enum WindowSize {
    States(WindowSizeStates),
    Size(Size),
}

All of the mixed enums had to do with size, so unifying the enums actually reduced some duplication anyway.


This PR also introduces ast-grep to the codebase. I've been using ast-grep significantly more at val.town and it's quickly becoming one of my go-to tools for code quality. I added a no-mixed-enums rule which ensure this pattern doesn't get reintroduced in the future.

@just-be-dev just-be-dev changed the title Remove mixed enums Remove mixed enums; add ast-grep Feb 7, 2025
@just-be-dev just-be-dev merged commit 019ef30 into main Feb 7, 2025
7 checks passed
@just-be-dev just-be-dev deleted the no-mixed-enums branch February 7, 2025 02:53
@just-be-dev just-be-dev mentioned this pull request Feb 7, 2025
1 task
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.

1 participant