Impl rewrite_result for ast nodes in items.rs

rust-lang / rustfmt

Format Rust code

https://rust-lang.github.io/rustfmt/

Apache License 2.0

5.98k stars 878 forks source link

Impl rewrite_result for ast nodes in items.rs #6212

Closed ding-young closed 2 months ago

ding-young commented 3 months ago

Tracked by #6206

Description

added RewriteError and blanket implementation for rewrite_result
above 6 ast nodes that implements rewrite, replaced rewrite function body with rewrite_result for 4 nodes; ast::Local, ast::FieldDef, ast::Param, ast::FnRetTy

ding-young commented 3 months ago

@ytmimi Thank you for your review. It seems like I missed some details. I'll take a more careful look at the next PR. I am wondering to what extent I should modify functions like rewrite_*** in this PR. For example, you recommended modifying rewrite_let_else_block and rewrite_explicit_self, while you suggested changing the signature of combine_str_with_missing_comment later. Should I proceed with modifying the simpler ones for now?

ytmimi commented 3 months ago

I'm fairly certain that rewrite_let_else_block and rewrite_explicit_self are only used within these functions so it's likely easier to update their signatures now, while combine_str_with_missing_comment is used throughout the codebase and would require touching a lot of different files.

If you feel like it would be a lot to extend the scope of the current PR then we can always revisit modifying all of these functions in future PRs, but if it's not a heavy lift then we might want to consider doing some of that work in this PR.

ytmimi commented 2 months ago

@ding-young when you have a chance can you squash the commits and rebase this PR to bring it up to date