next.js
ea1618d3 - codemod(turbopack): Rewrite `self: Vc<Self>` as `&self` in trivial cases (#70412)

Commit
1 year ago
codemod(turbopack): Rewrite `self: Vc<Self>` as `&self` in trivial cases (#70412) Noticed this pattern while touching `turbo-tasks-fs`. Functions that take `self: Vc<Self>` and immediately await it without ever using the `Vc` version of the argument should just take `&self` as an argument. There are some remaining cleanup opportunities here, which I have some still-WIP codemods for: - [x] If we have a trivial `let this = self` or `let this = &self.0`, we should rewrite references to `this` to use `self`. *Done in #70431* - [ ] If we can be sure that a function never returns anything other than `Ok(...)`, we should remove `Result` from the return type. Those changes need to be performed as a separate codemod pass due to limitations with `ast-grep`. ## How? ast-grep: https://ast-grep.github.io/ Using the ast-grep config: https://gist.github.com/bgw/b7bc0a921cf3e3447acaf8feda60b518 Ran it with: ``` sg scan -U -r ../codemod_rewrite_vc_self.yml . ``` ## Should this be a lint? **Maybe.** I've managed to get the false-positive rate down to zero on our existing codebase (at the cost of missing some opportunities for cleanup). ast-grep supports running as a lint rule, and this can be autofixed. However, if we fix the `let this = ...` and `Result<...>` cases as well with additional lint rules, **this might be a bit annoying as it'll trigger cascading lint rules**. You *might* need to run the linter multiple times for it to eventually settle. It does not appear to be possible to do all these changes in a single lint rule, as we require modifying overlapping ranges of code (which ast-grep sadly doesn't seem to support with the `rewriters` rules). We should also consider `dynlint` before heavily investing into `ast-grep` as a linter: https://github.com/trailofbits/dylint
Author
bgw bgw
Parents
Loading