rust-lang / rustfmt

Format Rust code
https://rust-lang.github.io/rustfmt/
Apache License 2.0
6.02k stars 887 forks source link

rustfmt leaves behind a too-long line inside a closure #6344

Open RalfJung opened 1 month ago

RalfJung commented 1 month ago

Here's some example code:

impl<'tcx> LateLintPass<'tcx> for UnqualifiedLocalImports {
    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) {
        let hir::ItemKind::Use(path, _kind) = item.kind else { return };
        let is_local_import = |res: &Res| {
            matches!(res, hir::def::Res::Def(def_kind, def_id) if def_id.is_local() && !matches!(def_kind, DefKind::Macro(_)))
        };
    }
}

It's not the best code, that line is too long. So let's format it. Now we have:

impl<'tcx> LateLintPass<'tcx> for UnqualifiedLocalImports {
    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) {
        let hir::ItemKind::Use(path, _kind) = item.kind else {
            return;
        };
        let is_local_import = |res: &Res| matches!(res, hir::def::Res::Def(def_kind, def_id) if def_id.is_local() && !matches!(def_kind, DefKind::Macro(_)));
    }
}

Now the line is even longer! It's way past the 100 character limit.

I guess rustfmt is reluctant to introduce linebreaks inside a macro, but then it should just avoid touching this code -- but instead it makes things worse by putting the entire closure on a single line.

ytmimi commented 1 month ago

This is a case where the macro fails to parse, and I assume that since it's formatted on one line and the closure only contains a single item rustfmt really wants to update the closure to not use braces.

If you formatted the matches! call by hand like this rustfmt should leave it alone:

impl<'tcx> LateLintPass<'tcx> for UnqualifiedLocalImports {
    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) {
        let hir::ItemKind::Use(path, _kind) = item.kind else { return };
        let is_local_import = |res: &Res| {
            matches!(
                res,
                hir::def::Res::Def(def_kind, def_id)
                    if def_id.is_local() && !matches!(def_kind, DefKind::Macro(_))
            )
        };
    }
}

I'll add that If https://github.com/rust-lang/rustfmt/pull/5554 lands that's the formatting that the input would produce.