-
Notifications
You must be signed in to change notification settings - Fork 48
Add format_concat_args lint to optimize string formatting #1638
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
… with `concat!()`
… with `concat!()`
|
Hello @smoelius, sorry to bother you. Could you please help me with the CI failures I'm encountering? I tried to fix it myself, but it looks like I am repeatedly getting the clippy_utils = { workspace = true }But I'm still seeing the same errors. Any idea what might be going wrong? Thank You! |
smoelius
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ginzahatemi Sorry for being slow to respond.
Here are a few things I noticed.
I think the compilation errors you are seeing are from a mismatch between the rust-toolchain file and the clippy_utils version.
Specifically, for the clippy_utils version you are using, I think the toolchain should be nightly-2025-04-03.
| [target.'cfg(all())'] | ||
| rustflags = ["-C", "linker=dylint-link"] | ||
|
|
||
| # For Rust versions 1.74.0 and onward, the following alternative can be used | ||
| # (see https://github.com/rust-lang/cargo/pull/12535): | ||
| # linker = "dylint-link" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| [target.'cfg(all())'] | |
| rustflags = ["-C", "linker=dylint-link"] | |
| # For Rust versions 1.74.0 and onward, the following alternative can be used | |
| # (see https://github.com/rust-lang/cargo/pull/12535): | |
| # linker = "dylint-link" | |
| [build] | |
| target-dir = "../../target/examples" | |
| [target.'cfg(all())'] | |
| linker = "dylint-link" |
To match the other examples' configs.
| @@ -0,0 +1 @@ | |||
| /target | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file can be removed. A .gitignore in a parent directory accomplishes this.
| version = "0.1.0" | ||
| authors = ["Your Name <[email protected]>"] | ||
| description = "A Dylint lint to suggest using `concat!(...)` instead of `format!(...)` for all-constant Display arguments." | ||
| edition = "2021" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| edition = "2021" | |
| edition = "2024" |
| @@ -0,0 +1,3 @@ | |||
| [toolchain] | |||
| channel = "nightly-2025-02-20" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| channel = "nightly-2025-02-20" | |
| channel = "nightly-2025-04-03" |
There's a test to ensure that all examples use the same toolchain, and this is the toolchain they currently use.
Also, I think this is the toolchain that goes with the version of clippy_utils you are currently using.
| @@ -0,0 +1,26 @@ | |||
| [package] | |||
| name = "format_concat_args" | |||
| version = "0.1.0" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| version = "0.1.0" | |
| version = "4.1.0" |
To match the versions used elsewhere in this repository.
|
@ginzahatemi Thank you very much for your work on this! I will pick up this PR, if you don't mind. |
This PR addresses #1601 by implementing a new lint that detects cases where
format!()could bereplaced with
concat!()for improved performance.Problem
When all arguments to a
format!()macro are compile-time constants and usethe standard Display formatting trait, the formatting can be done at compile time
with
concat!()instead of being deferred to runtime.Solution
This lint analyzes format string placeholders and their arguments to detect:
{}format)When both conditions are met, it suggests replacing with an equivalent
concat!()expression.
Examples
Fixes #1601