rustfmt icon indicating copy to clipboard operation
rustfmt copied to clipboard

`format_strings` can cause code to no longer be reformatted

Open VorpalBlade opened this issue 5 months ago • 5 comments

The following code fragment:

fn sanity_check(
    path: &Utf8Path,
    style: Style,
    chezmoi: &impl Chezmoi,
) -> Result<(), anyhow::Error> {
    if !path.is_file() {
        return Err(anyhow!("{} is not a regular file", path));
    }
    if Style::InPath == style && chezmoi.version()? < CHEZMOI_AUTO_SOURCE_VERSION {
        return Err(anyhow!(
            "To use \"--style path\" you need chezmoi {CHEZMOI_AUTO_SOURCE_VERSION} or newer"
        ));
    }
    match crate::doctor::hook_paths(chezmoi)?.as_slice() {
        [] => Ok(()),
        _ => {
            Err(anyhow!("Legacy hook script found, see chezmoi_modify_manager --doctor and please read https://github.com/VorpalBlade/chezmoi_modify_manager/blob/main/doc/migration_3.md"))
        }
    }
}

Normally gets reformatted as follows (using edition 2024):

diff --git a/src/add.rs b/src/add.rs
index c8d2689..d2a9f58 100644
--- a/src/add.rs
+++ b/src/add.rs
@@ -340,9 +340,9 @@ fn sanity_check(
     }
     match crate::doctor::hook_paths(chezmoi)?.as_slice() {
         [] => Ok(()),
-        _ => {
-            Err(anyhow!("Legacy hook script found, see chezmoi_modify_manager --doctor and please read https://github.com/VorpalBlade/chezmoi_modify_manager/blob/main/doc/migration_3.md"))
-        }
+        _ => Err(anyhow!(
+            "Legacy hook script found, see chezmoi_modify_manager --doctor and please read https://github.com/VorpalBlade/chezmoi_modify_manager/blob/main/doc/migration_3.md"
+        )),
     }
 }

However, if I enable the nightly format_strings = true, this reformatting doesn't take place. But if it is already reformatted, the code isn't touched (so it is "bi-stable" as it were).

I don't see why format_strings should affect this at all. Tried with rustc 1.89.0-nightly (d13a431a6 2025-06-09) /rustfmt 1.8.0-nightly (d13a431a6c 2025-06-09) (strange that they have separate version numbers, didn't know that).

This can be seen at commit 1308218f328882a1b452e0a3e8d84754e4d6863b in https://github.com/VorpalBlade/chezmoi_modify_manager (two files are affected by the exact same issue when using "format_strings = true" and nightly rustfmt).

VorpalBlade avatar Jun 10 '25 18:06 VorpalBlade

Thanks for the report. When you're enabling format_strings=true are you applying that config to the diffed code output or the original input? It wasn't totally clear to me based on the description. Can you post complete before and after code snippets along with all enabled configuration options used to produce the formatting or lack thereof.

ytmimi avatar Jun 10 '25 20:06 ytmimi

Also, is this only an issue for style_edition=2024 or do you see the same results with style_edition={2015| 2018 | 2021}?

ytmimi avatar Jun 10 '25 20:06 ytmimi

Thanks for the report. When you're enabling format_strings=true are you applying that config to the diffed code output or the original input? It wasn't totally clear to me based on the description. Can you post complete before and after code snippets along with all enabled configuration options used to produce the formatting or lack thereof.

I don't think I fully understand your question. I upgraded the crate from 2021 to 2024, and reformatted with nightly rustfmt. Then CI failed (it was running stable rustfmt, which was wrong but made me notice this).

I'll post the full code snippets after work this evening.

Also, is this only an issue for style_edition=2024 or do you see the same results with style_edition={2015| 2018 | 2021}?

I haven't tried this, but before upgrading to edition 2024 nightly and stable rustfmt agreed. But maybe it is bi-stable in another way there? Some further experiments would be needed.

VorpalBlade avatar Jun 11 '25 06:06 VorpalBlade

but before upgrading to edition 2024 nightly and stable rustfmt agreed.

Assuming you weren't setting any nightly only configuration options then it makes sense that nightly and stable would agree before style_edition=2024.

However, style_edition=2024 has some different behavior from style_edition=2021 (2021, 2018, and 2015 are equivalent) and applies bug fixes that we couldn't release to stable prior to stabilizing style_edition=2024 and letting users opt into those changes.

ytmimi avatar Jun 11 '25 20:06 ytmimi

Sorry, didn't get around to it today. I aim to do it after work tomorrow instead.

VorpalBlade avatar Jun 11 '25 20:06 VorpalBlade

Sorry for the delay. @ytmimi I'm still not entirely sure what you want differently than the initial report.

  1. Before (same as original post):
fn sanity_check(
    path: &Utf8Path,
    style: Style,
    chezmoi: &impl Chezmoi,
) -> Result<(), anyhow::Error> {
    if !path.is_file() {
        return Err(anyhow!("{} is not a regular file", path));
    }
    if Style::InPath == style && chezmoi.version()? < CHEZMOI_AUTO_SOURCE_VERSION {
        return Err(anyhow!(
            "To use \"--style path\" you need chezmoi {CHEZMOI_AUTO_SOURCE_VERSION} or newer"
        ));
    }
    match crate::doctor::hook_paths(chezmoi)?.as_slice() {
        [] => Ok(()),
        _ => {
            Err(anyhow!("Legacy hook script found, see chezmoi_modify_manager --doctor and please read https://github.com/VorpalBlade/chezmoi_modify_manager/blob/main/doc/migration_3.md"))
        }
    }
}
  1. After (with edition 2024, all stable):
fn sanity_check(
    path: &Utf8Path,
    style: Style,
    chezmoi: &impl Chezmoi,
) -> Result<(), anyhow::Error> {
    if !path.is_file() {
        return Err(anyhow!("{} is not a regular file", path));
    }
    if Style::InPath == style && chezmoi.version()? < CHEZMOI_AUTO_SOURCE_VERSION {
        return Err(anyhow!(
            "To use \"--style path\" you need chezmoi {CHEZMOI_AUTO_SOURCE_VERSION} or newer"
        ));
    }
    match crate::doctor::hook_paths(chezmoi)?.as_slice() {
        [] => Ok(()),
        _ => Err(anyhow!(
            "Legacy hook script found, see chezmoi_modify_manager --doctor and please read https://github.com/VorpalBlade/chezmoi_modify_manager/blob/main/doc/migration_3.md"
        )),
    }
}
  1. After (with edition 2024, format_strings = true), starting with 1. As can be seen, same as 1:
fn sanity_check(
    path: &Utf8Path,
    style: Style,
    chezmoi: &impl Chezmoi,
) -> Result<(), anyhow::Error> {
    if !path.is_file() {
        return Err(anyhow!("{} is not a regular file", path));
    }
    if Style::InPath == style && chezmoi.version()? < CHEZMOI_AUTO_SOURCE_VERSION {
        return Err(anyhow!(
            "To use \"--style path\" you need chezmoi {CHEZMOI_AUTO_SOURCE_VERSION} or newer"
        ));
    }
    match crate::doctor::hook_paths(chezmoi)?.as_slice() {
        [] => Ok(()),
        _ => {
            Err(anyhow!("Legacy hook script found, see chezmoi_modify_manager --doctor and please read https://github.com/VorpalBlade/chezmoi_modify_manager/blob/main/doc/migration_3.md"))
        }
    }
}
  1. After (with edition 2024, format_strings = true), starting with 2. As can be seen, same as 2:
fn sanity_check(
    path: &Utf8Path,
    style: Style,
    chezmoi: &impl Chezmoi,
) -> Result<(), anyhow::Error> {
    if !path.is_file() {
        return Err(anyhow!("{} is not a regular file", path));
    }
    if Style::InPath == style && chezmoi.version()? < CHEZMOI_AUTO_SOURCE_VERSION {
        return Err(anyhow!(
            "To use \"--style path\" you need chezmoi {CHEZMOI_AUTO_SOURCE_VERSION} or newer"
        ));
    }
    match crate::doctor::hook_paths(chezmoi)?.as_slice() {
        [] => Ok(()),
        _ => Err(anyhow!(
            "Legacy hook script found, see chezmoi_modify_manager --doctor and please read https://github.com/VorpalBlade/chezmoi_modify_manager/blob/main/doc/migration_3.md"
        )),
    }
}

This means that when format_strings is in use, the code does not format to one canonical format, but it is at least bistable.

VorpalBlade avatar Jun 22 '25 06:06 VorpalBlade