Skip to content
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

feat: Implicit format args support #16027

Merged
merged 5 commits into from Dec 5, 2023
Merged

Conversation

Veykril
Copy link
Member

@Veykril Veykril commented Dec 5, 2023

Fixes #11260
Fixes #11296

image
Too lazy to make a gif of this right now (would probably be good to show renaming)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 5, 2023
@Veykril Veykril changed the title Improve macro descension API Implicit format args support Dec 5, 2023
@Veykril
Copy link
Member Author

Veykril commented Dec 5, 2023

Needs a bunch of special casing due to the format_args! expansion changes after all but thats fine for the time being. We'll have to revisit whether we want to undo that change (which would make things work more uniformly on the AST), or if we finally move more things over to the HIR.

@Veykril Veykril marked this pull request as ready for review December 5, 2023 15:33
@Veykril
Copy link
Member Author

Veykril commented Dec 5, 2023

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 5, 2023

📌 Commit 9b7ec5e has been approved by Veykril

It is now in the queue for this repository.

@Veykril Veykril changed the title Implicit format args support feat: Implicit format args support Dec 5, 2023
@bors
Copy link
Collaborator

bors commented Dec 5, 2023

⌛ Testing commit 9b7ec5e with merge 05df6c5...

@bors
Copy link
Collaborator

bors commented Dec 5, 2023

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 05df6c5 to master...

@bors bors merged commit 05df6c5 into rust-lang:master Dec 5, 2023
10 checks passed
@Veykril Veykril deleted the implicit-format-args branch December 5, 2023 21:30
@lnicola
Copy link
Member

lnicola commented Dec 11, 2023

rename-format-args.mp4

@nyurik
Copy link
Contributor

nyurik commented Dec 12, 2023

so now we can re-enable clippy lint for this? :) Assuming it will only work by default in the "all params can be included" context. If its a mixed case, that would stay as pedantic.

@Veykril
Copy link
Member Author

Veykril commented Dec 12, 2023

Yes you should be able to re-enable it now.

avelanarius added a commit to avelanarius/scylla-rust-driver that referenced this pull request Mar 4, 2024
This reverts commit 4964b44.

The main reason that uninlined_format_args lint was disabled in Rust 
Driver's CI/Makefile was that inlined variables in format strings had 
spotty support in rust-analyzer: rust-analyzer did not support renaming
variables inside format strings.

Fortunately, rust-analyzer recently gained support for renaming 
variables in format strings: rust-lang/rust-analyzer#16027, therefore
4964b44 can now be reverted. 

Moreover, Rust 1.67.1 downgraded uninlined_format_args to pedantic, 
meaning that it's disabled by default and we don't have to disable it
manually. If it's ever promoted back to a enforced rule by default,
we'll be ready to deal with it again (with a proper rust-analyzer 
support) - not addressing it now to avoid merge conflicts with other
bigger planned PRs.

Closes scylladb#643
avelanarius added a commit to avelanarius/scylla-rust-driver that referenced this pull request Mar 4, 2024
This reverts commit 4964b44.

The main reason that uninlined_format_args lint was disabled in Rust 
Driver's CI/Makefile was that inlined variables in format strings had 
spotty support in rust-analyzer: rust-analyzer did not support renaming
variables inside format strings.

Fortunately, rust-analyzer recently gained support for renaming 
variables in format strings: rust-lang/rust-analyzer#16027, therefore
4964b44 can now be reverted. 

Moreover, Rust 1.67.1 downgraded uninlined_format_args to pedantic, 
meaning that it's disabled by default and we don't have to disable it
manually. If it's ever promoted back to an enforced rule by default,
we'll be ready to deal with it again (with a proper rust-analyzer 
support) - not addressing it now to avoid merge conflicts with other
bigger planned PRs.

Closes scylladb#643
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Renaming a captured variable in a formatted string doesn't work Support format_args_capture
5 participants