kaleidawave / ezno

A JavaScript compiler and TypeScript checker written in Rust with a focus on static analysis and runtime performance
https://kaleidawave.github.io/posts/introducing-ezno/
MIT License
2.3k stars 42 forks source link

[WIP] [Refactor] Defining clippy rules #91

Closed JulesGuesnon closed 7 months ago

JulesGuesnon commented 7 months ago

Description

This PR aims to define standards for clippy, and fixes the too_many_arugment error as well.

About clippy

Imo the best strategy is to use #![deny(clippy::all)] which is covering the most important linting rules, and to cherry pick some of clippy::pedantic as using everything can be painful, and raise false positives.

List of all the pedantic lintings

Example of some pendantic linting I think may be nice to have:

kaleidawave commented 7 months ago

Awesome! looking good. Great addition to wrap a bunch of arguments to function calling. I always got a bit mixed up with which one was which, so that's a great fix.

With the Result<> issue, I have opened #92 that outlines where that is ATM. Can add an allow for now linking to the issue.

Also subtyping, probably best to add allow for now. The setup might change in the future.

If you want to do the change to the checkers PropertyKey::String to include privacy then that would be great. If it is too complicated then could add allow for now and add a comment that links to the comment I added on #88.

The tighter requirements seem great, going through the clippy page you linked and I think blocking them all by default will improve the codebase now it is getting more mature. You can update the line here to reflect that #![deny(clippy::all)] is not on by default

https://github.com/kaleidawave/ezno/blob/502126217170121cadb12cad3debe1ff388a37f1/CONTRIBUTING.md?plain=1#L59

JulesGuesnon commented 7 months ago

Hey! About Publicity I'll be glad to do the change, but as it looks likes it involves a lot of noise, I'll do it in another PR I think.

Also about: fn synthesise_function_annotation(...) in checker/src/synthesis/functions.rs there's a lot of TODO above, so it might not be that useful to refactor args into a struct right?

JulesGuesnon commented 7 months ago

Btw, so you're aware of what clippy::pedantic represents, only in checker it's ~483 errors appearing, so it'll take some time to be compliant, but would for sure allow a higher quality code

kaleidawave commented 7 months ago

Nice! Cool that it uncovered a bug in the parser as well.

It is great that you have solved a lot of the current lints. For the 'pedantic' level lints it probably best to prioritise those changes until after 0.1 https://github.com/kaleidawave/ezno/milestone/1

Will check out the changes and merge on Friday when I have the time to go through it 👍

kaleidawave commented 7 months ago

All changes look good. Just need to fix the something in the WASM section. Then will merge !

JulesGuesnon commented 7 months ago

Fixed

kaleidawave commented 7 months ago

Awesome, just merged!