Skip to content

refactor: inline format! args in a few places#10730

Merged
sylvestre merged 2 commits intouutils:mainfrom
nyurik:inline-fmt
Feb 7, 2026
Merged

refactor: inline format! args in a few places#10730
sylvestre merged 2 commits intouutils:mainfrom
nyurik:inline-fmt

Conversation

@nyurik
Copy link
Contributor

@nyurik nyurik commented Feb 5, 2026

  • in one spot remove redundant mem alloc

@ChrisDryden
Copy link
Collaborator

I was trying to do some research into why this couldn't be enabled as a clippy rule so that this can be applied going forward and I think I was able to replicate the changes with uninlined_format_args = "warn" in Cargo.toml and adding allow-mixed-uninlined-format-args = false in the .clippy.toml

This change already brings it down from 20 to 3, I'm curious if there was something used to generate this or this was just through reading of the codebase?

(Ok(c), Err(v)) => Err(BadSequence::MultipleCharInEquivalence(format!(
"{}{}",
String::from_utf8_lossy(&[c]).into_owned(),
String::from_utf8_lossy(v).into_owned()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future reference this was the redundant memory allocation

@nyurik
Copy link
Contributor Author

nyurik commented Feb 5, 2026

sadly no - just did a late night regex search :) I tried the lint approach at first, but it was not working for me for some reason - I figured it was due to features or specifics of the platform (i'm on linux)... And yes, i think we should add the clippy setting - i'll do it.

@nyurik
Copy link
Contributor Author

nyurik commented Feb 5, 2026

ah yes, one other thing -- format!-like macros like show_error! may not be caught unless they have a #[clippy::format_args] attribute... but sadly there is a rustc compiler bug that breaks compilation when using an attribute on an exported macro in some cases... So for those, manual search is still the only option

@nyurik nyurik force-pushed the inline-fmt branch 3 times, most recently from 9a6a124 to 87c3dd1 Compare February 5, 2026 21:25
@github-actions
Copy link

github-actions bot commented Feb 5, 2026

GNU testsuite comparison:

Congrats! The gnu test tests/basenc/bounded-memory is now passing!

@nyurik
Copy link
Contributor Author

nyurik commented Feb 6, 2026

P.S. I tried to fix the compiler bug with rust-lang/rust#152190 -- if merged, in 3 months we can have all format-like macros auto-fixed by clippy

@github-actions
Copy link

github-actions bot commented Feb 6, 2026

GNU testsuite comparison:

Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/usage_vs_getopt (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/shuf/shuf-reservoir (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/sort/sort-stale-thread-mem (passes in this run but fails in the 'main' branch)

@sylvestre sylvestre merged commit 0520486 into uutils:main Feb 7, 2026
155 checks passed
@nyurik nyurik deleted the inline-fmt branch February 7, 2026 09:08
@xtqqczze
Copy link
Contributor

xtqqczze commented Feb 7, 2026

ci breakage again, conflicts with 181b247

  error: variables can be used directly in the `format!` string
     --> tests/by-util/test_stdbuf.rs:106:5
      |
  106 | /     assert!(
  107 | |         loaded_from_exe_dir,
  108 | |         "libstdbuf should be loaded from exe directory ({}), not from LIBSTDBUF_DIR. LD_DEBUG output:\n{}",
  109 | |         temp_path.display(),
  110 | |         stderr
  111 | |     );
      | |_____^
      |
      = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.93.0/index.html#uninlined_format_args
      = note: `-D clippy::uninlined-format-args` implied by `-D warnings`
      = help: to override `-D warnings` add `#[allow(clippy::uninlined_format_args)]`

@nyurik
Copy link
Contributor Author

nyurik commented Feb 7, 2026

I wonder if CI can be stabilized somehow - rust project has far larger number of changes and yet the code tends to be more stable. The lowest hanging fruit might be to have early "tidy" warnings - so that each CI run reports fmt/clippy issues much within a few minutes of the commit?

@xtqqczze
Copy link
Contributor

xtqqczze commented Feb 7, 2026

I wonder if CI can be stabilized somehow

The usual way to solve this problem is with a merge queue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants