Fix uninline format clippy violations by arminsabouri · Pull Request #667 · payjoin/rust-payjoin

arminsabouri · 2025-04-24T13:57:27Z

This change was automatically generated via cargo clippy --fix. In the lastest nightly release 1.88.0 vars need to be inline in a format string.

https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args

coveralls · 2025-04-24T14:41:01Z

Pull Request Test Coverage Report for Build 14666415050

Details

50 of 145 (34.48%) changed or added relevant lines in 30 files are covered.
3 unchanged lines in 2 files lost coverage.
Overall coverage increased (+0.1%) to 81.936%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
payjoin-cli/src/app/mod.rs	2	3	66.67%
payjoin-directory/src/lib.rs	1	2	50.0%
payjoin/src/hpke.rs	0	1	0.0%
payjoin/src/into_url.rs	0	1	0.0%
payjoin-directory/src/db.rs	1	3	33.33%
payjoin/src/ohttp.rs	1	3	33.33%
payjoin/src/uri/error.rs	1	3	33.33%
payjoin/src/uri/mod.rs	3	5	60.0%
payjoin/src/io.rs	0	3	0.0%
payjoin/src/psbt/mod.rs	0	3	0.0%

Files with Coverage Reduction	New Missed Lines	%
payjoin/src/receive/optional_parameters.rs	1	77.78%
payjoin-cli/src/app/v2.rs	2	76.58%

Totals
Change from base Build 14653048506:	0.1%
Covered Lines:	5325
Relevant Lines:	6499

💛 - Coveralls

arminsabouri · 2025-04-24T16:32:38Z

I have combed through the diff a couple times. Nothing stood out. But please review carefully.

benalleng · 2025-04-24T20:05:28Z

payjoin-cli/tests/e2e.rs

-            let receiver_rpchost = format!("http://{}/wallet/receiver", bitcoind.params.rpc_socket);
-            let sender_rpchost = format!("http://{}/wallet/sender", bitcoind.params.rpc_socket);
+            let receiver_rpchost =
+                format!("http://{host}/wallet/receiver", host = bitcoind.params.rpc_socket);


that's neat, didn't know you could do that

benalleng

TACK couldn't find any egregious misses or vars put in the wrong order in the formatting string.

Personally though, I hate it and wish that we didn't have to put everything inside of these formatters like this.

payjoin/tests/integration.rs

DanGould

I like the change in general but

let's not make semantic changes in the same commit as one where it can be verified by a reviewer using an automated tool and (i.e. keep debug :? semantics to use Debug impl, not Display where applicable)
please don't make opinionated non-semantic style changes in the same commit. changing res = response.status() might feel right but it makes verifying the change from clippy --fix one that can't be automatically verified by comparing a diff after running a command. This is the same idea as keeping semantic changes from changes to whitespace or lints.

DanGould · 2025-04-24T20:20:59Z

payjoin-cli/src/app/config.rs

+                        "Multiple version flags specified. Please use only one of: {flags}",
+                        flags = Self::VERSION_FLAGS
                            .iter()
-                            .map(|(flag, _)| format!("--{}", flag))
+                            .map(|(flag, _)| format!("--{flag}"))


~~I'd rather we just Self::VERSION_FLAGS here and droped the additional var~~

Edit: I realize this change doesn't make sense, flags is a complex expression. But clippy doesn't complain about it so we can just leave it

Removed the flag variable. Clippy is ok with non-inline vars if you are using a expression

payjoin-cli/src/app/v1.rs

payjoin/src/receive/multiparty/error.rs

arminsabouri · 2025-04-25T13:05:28Z

TACK couldn't find any egregious misses or vars put in the wrong order in the formatting string.

Personally though, I hate it and wish that we didn't have to put everything inside of these formatters like this.

If we all hate it we do have the option of skipping this rule. Personally I think its a bit more readable.

arminsabouri · 2025-04-25T13:07:10Z

Split changes into two commits: Automatically generated and manually generated fixes (left out any semantic changes)
Drafting as of now. A couple more things need to be fixed in the second commit. I'll re-request reviews once its out of draft. Thanks

This change was automatically applied using `cargo clippy --fix && cargo fmt`. It addresses the newly introduced `uninlined_format_args` lint, which requires variables to be inlined directly within format strings as of Rust 1.88.0 nightly. [Clippy lint reference](https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args)

The `uninlined_format_args` lint was added in the latest Rust 1.88 nightly. This commit includes the manual changes required to address it. Specifically, the ones that `cargo clippy --fix` could not automatically resolve. [Clippy lint reference](https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args)

benalleng · 2025-04-25T14:32:24Z

I am ok with the change if everyone else thinks its overall more readable

DanGould

Thanks for going through the tedious rigamarole. Setting up review like this is gonna let us move faster on a regular basis as the changes arise. I think that's a long lever so I hope you agree it is worth the short-term tedium.

I can confirm cafd68b is equivalent to running cargo +1.88.0 clippy --fix and cargo +1.88.0 fmt

ACK b42eee4

arminsabouri · 2025-04-25T15:35:14Z

Thanks for going through the tedious rigamarole. Setting up review like this is gonna let us move faster on a regular basis as the changes arise. I think that's a long lever so I hope you agree it is worth the short-term tedium.

I can confirm cafd68b is equivalent to running cargo +1.88.0 clippy --fix and cargo +1.88.0 fmt

ACK b42eee4

Agreed. Automated changes should be reproducable and verifiable.

arminsabouri force-pushed the nightly-lint-errors branch 3 times, most recently from 0b0247a to c9850f1 Compare April 24, 2025 16:26

arminsabouri marked this pull request as ready for review April 24, 2025 16:32

arminsabouri requested a review from DanGould April 24, 2025 19:50

arminsabouri mentioned this pull request Apr 24, 2025

Duplicate proposals catch in ns1r #642

Merged

benalleng reviewed Apr 24, 2025

View reviewed changes

benalleng approved these changes Apr 24, 2025

View reviewed changes

payjoin/tests/integration.rs Show resolved Hide resolved

DanGould requested changes Apr 24, 2025

View reviewed changes

arminsabouri force-pushed the nightly-lint-errors branch from c9850f1 to a0840cf Compare April 25, 2025 13:00

arminsabouri marked this pull request as draft April 25, 2025 13:00

arminsabouri added 2 commits April 25, 2025 09:40

arminsabouri force-pushed the nightly-lint-errors branch from a0840cf to b42eee4 Compare April 25, 2025 14:01

arminsabouri requested a review from DanGould April 25, 2025 14:19

arminsabouri marked this pull request as ready for review April 25, 2025 14:19

DanGould approved these changes Apr 25, 2025

View reviewed changes

DanGould merged commit a7e122e into payjoin:master Apr 25, 2025
7 checks passed

Conversation

arminsabouri commented Apr 24, 2025

Uh oh!

coveralls commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 14666415050

Details

💛 - Coveralls

Uh oh!

arminsabouri commented Apr 24, 2025

Uh oh!

benalleng Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

benalleng left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DanGould left a comment

Choose a reason for hiding this comment

Uh oh!

DanGould Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arminsabouri Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

arminsabouri commented Apr 25, 2025

Uh oh!

arminsabouri commented Apr 25, 2025

Uh oh!

benalleng commented Apr 25, 2025

Uh oh!

DanGould left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arminsabouri commented Apr 25, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coveralls commented Apr 24, 2025 •

edited

Loading

DanGould Apr 24, 2025 •

edited

Loading