Suggest `{var:?}` when finding `{?:var}` in inline format strings #106805

madsravn · 2023-01-13T12:25:01Z

This is my first PR to this project, so hopefully I can get some good pointers with me from the first PR.

Currently my idea was to test out whether or not this is the correct solution to this issue and then hopefully expand upon the idea to not only work for Debug formatting but for all of them. If this is a valid solution, I will create a new issue to give a better error message to a broader range of wrong-order formatting.

rustbot · 2023-01-13T12:25:10Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @TaKO8Ki (or someone else) soon.

Please see the contribution instructions for more information.

Noratrieb · 2023-01-13T12:38:03Z

Thanks! Yes, something like this is the correct approach. If you intend to make more changes with more suggestions then feel free, but you can also land just this change first if you prefer.

mejrs · 2023-01-13T13:04:37Z

We moved the tests from src/test to tests, so you must rebase and move your tests to the correct location.

madsravn · 2023-01-13T13:07:18Z

Thanks! Yes, something like this is the correct approach. If you intend to make more changes with more suggestions then feel free, but you can also land just this change first if you prefer.

If it is OK, then let's just land this one first. Then I can use some time to tinker with the full idea while maybe grabbing one or two other issues to really learn the ropes.

madsravn · 2023-01-13T13:07:51Z

We moved the tests from src/test to tests, so you must rebase and move your tests to the correct location.

That's a roger. Will do later today - just on my way out the door. Thanks :)

rustbot · 2023-01-13T18:38:35Z

Some changes occurred in need_type_info.rs

cc @lcnr

rustc_error_messages was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @estebank, @TaKO8Ki

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

estebank · 2023-01-13T18:53:05Z

There seems to have been an issue with rebasing on top of master. You'll have to rebase interactively on top of master while discarding any commits that you didn't intend to include in the PR. Let us know if you need assistance doing so.

tests/ui/fmt/format-string-wrong-order.stderr

madsravn · 2023-01-13T19:32:28Z

There seems to have been an issue with rebasing on top of master. You'll have to rebase interactively on top of master while discarding any commits that you didn't intend to include in the PR. Let us know if you need assistance doing so.

I would love some assistance with this. For some reason, me and git rebase never became close friends. From the current state, how would you suggest me to unclutter the mess I made and then do it correctly?

mejrs · 2023-01-13T19:57:30Z

See https://rustc-dev-guide.rust-lang.org/git.html#i-made-a-merge-commit-by-accident

mejrs · 2023-01-13T19:58:20Z

Ah, looks like you managed to figure it out :)

madsravn · 2023-01-13T20:00:38Z

Ah, looks like you managed to figure it out :)

Thanks for the link.

I think I sort of stumbled over the way to solve it. I hope that did the trick and I will be more careful in the future :)

compiler/rustc_parse_format/src/lib.rs

madsravn · 2023-01-27T12:01:30Z

@TaKO8Ki Hi, I have been told it would be OK to ping you after a few weeks.

Do you know when you will get time to review this? No need to jump on it now if you don't have the time.

compiler-errors · 2023-02-01T19:29:44Z

r? @compiler-errors

compiler-errors · 2023-02-01T20:04:41Z

@madsravn, can you add the tests requested in #106805 (comment) ?

Also, do you mind rebasing and squashing the commit history into one or two PRs -- all those "Adding" and tweaking commits just clutters up the git history 😄

Afterwards, I think this can be approved, as Esteban had noted above.

compiler-errors · 2023-02-01T20:05:52Z

tests/ui/fmt/format-string-wrong-order.rs

@@ -0,0 +1,5 @@
+fn main() {
+    let bar = 3;
+    format!("{?:}", bar); //~ ERROR invalid format string: expected format parameter to occur after `:`


tiny nitpick, take it or leave it -- it's easier to read, in my opinion, when the error follows the line in question, using ~^ like:

Suggested change

format!("{?:}", bar); //~ ERROR invalid format string: expected format parameter to occur after `:`

format!("{?:}", bar);

//~^ ERROR invalid format string: expected format parameter to occur after `:`

I have squashed the commits - hadn't tried that before. Was surprisingly easy.

And nitpicks are awesome. Those are what changes "good" to "great".

Done

Adding Adding Fixing small issues for PR Adding tests Removing unused binding Changing the wording on note Fixing PR comment

compiler-errors · 2023-02-03T04:08:06Z

@bors r+ rollup

bors · 2023-02-03T04:08:08Z

📌 Commit f922c83 has been approved by compiler-errors

It is now in the queue for this repository.

…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#106575 (Suggest `move` in nested closure when appropriate) - rust-lang#106805 (Suggest `{var:?}` when finding `{?:var}` in inline format strings) - rust-lang#107500 (Add tests to assert current behavior of large future sizes) - rust-lang#107598 (Fix benchmarks in library/core with black_box) - rust-lang#107602 (Parse and recover from type ascription in patterns) - rust-lang#107608 (Use triple rather than arch for fuchsia test-runner) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

Adding alignment to the cases to test for specific error messages. Adding alignment to the list of cases to test for specific error message. Covers `>`, `^` and `<`. Pinging people who chimed in last time ( rust-lang#106805 ): `@estebank` , `@compiler-errors` and `@Nilstrieb`

Adding alignment to the cases to test for specific error messages. Adding alignment to the list of cases to test for specific error message. Covers `>`, `^` and `<`. Pinging people who chimed in last time ( rust-lang#106805 ): ``@estebank`` , ``@compiler-errors`` and ``@Nilstrieb``

Rollup merge of rust-lang#119712 - madsravn:parsing-errors, r=estebank Adding alignment to the cases to test for specific error messages. Adding alignment to the list of cases to test for specific error message. Covers `>`, `^` and `<`. Pinging people who chimed in last time ( rust-lang#106805 ): ``@estebank`` , ``@compiler-errors`` and ``@Nilstrieb``

Adding alignment to the cases to test for specific error messages. Adding alignment to the list of cases to test for specific error message. Covers `>`, `^` and `<`. Pinging people who chimed in last time ( rust-lang/rust#106805 ): ``@estebank`` , ``@compiler-errors`` and ``@Nilstrieb``

rustbot assigned TaKO8Ki Jan 13, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 13, 2023

Noratrieb changed the title ~~Pull request to fix issue #106572 - suggesting {var:?} when finding {:?var} in inline format strings~~ Suggest {var:?} when finding {:?var} in inline format strings Jan 13, 2023

rustbot added A-meta Area: Issues & PRs about the rust-lang/rust repository itself A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic labels Jan 13, 2023

estebank reviewed Jan 13, 2023

View reviewed changes

tests/ui/fmt/format-string-wrong-order.stderr Outdated Show resolved Hide resolved

estebank reviewed Jan 15, 2023

View reviewed changes

compiler/rustc_parse_format/src/lib.rs Show resolved Hide resolved

estebank changed the title ~~Suggest {var:?} when finding {:?var} in inline format strings~~ Suggest {var:?} when finding {?:var} in inline format strings Jan 15, 2023

rustbot assigned compiler-errors and unassigned TaKO8Ki Feb 1, 2023

compiler-errors reviewed Feb 1, 2023

View reviewed changes

PR fixing wrong order of format parameters in strings. Issue #106572

f922c83

Adding Adding Fixing small issues for PR Adding tests Removing unused binding Changing the wording on note Fixing PR comment

bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 3, 2023

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Feb 3, 2023

matthiaskrgr mentioned this pull request Feb 3, 2023

Rollup of 6 pull requests #107625

Merged

bors merged commit 2fdb355 into rust-lang:master Feb 3, 2023

rustbot added this to the 1.69.0 milestone Feb 3, 2023

madsravn mentioned this pull request Jan 7, 2024

Adding alignment to the cases to test for specific error messages. #119712

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Suggest `{var:?}` when finding `{?:var}` in inline format strings #106805

Suggest `{var:?}` when finding `{?:var}` in inline format strings #106805

madsravn commented Jan 13, 2023 •

edited by JohnTitor

Loading

rustbot commented Jan 13, 2023

Noratrieb commented Jan 13, 2023

mejrs commented Jan 13, 2023 •

edited

Loading

madsravn commented Jan 13, 2023

madsravn commented Jan 13, 2023

rustbot commented Jan 13, 2023

estebank commented Jan 13, 2023

madsravn commented Jan 13, 2023

mejrs commented Jan 13, 2023

mejrs commented Jan 13, 2023

madsravn commented Jan 13, 2023

madsravn commented Jan 27, 2023

compiler-errors commented Feb 1, 2023

compiler-errors commented Feb 1, 2023

compiler-errors Feb 1, 2023 •

edited

Loading

madsravn Feb 2, 2023

compiler-errors commented Feb 3, 2023

bors commented Feb 3, 2023

	format!("{?:}", bar); //~ ERROR invalid format string: expected format parameter to occur after `:`
	format!("{?:}", bar);
	//~^ ERROR invalid format string: expected format parameter to occur after `:`

Suggest {var:?} when finding {?:var} in inline format strings #106805

Suggest {var:?} when finding {?:var} in inline format strings #106805

Conversation

madsravn commented Jan 13, 2023 • edited by JohnTitor Loading

rustbot commented Jan 13, 2023

Noratrieb commented Jan 13, 2023

mejrs commented Jan 13, 2023 • edited Loading

madsravn commented Jan 13, 2023

madsravn commented Jan 13, 2023

rustbot commented Jan 13, 2023

estebank commented Jan 13, 2023

madsravn commented Jan 13, 2023

mejrs commented Jan 13, 2023

mejrs commented Jan 13, 2023

madsravn commented Jan 13, 2023

madsravn commented Jan 27, 2023

compiler-errors commented Feb 1, 2023

compiler-errors commented Feb 1, 2023

compiler-errors Feb 1, 2023 • edited Loading

Choose a reason for hiding this comment

madsravn Feb 2, 2023

Choose a reason for hiding this comment

compiler-errors commented Feb 3, 2023

bors commented Feb 3, 2023

Suggest `{var:?}` when finding `{?:var}` in inline format strings #106805

Suggest `{var:?}` when finding `{?:var}` in inline format strings #106805

madsravn commented Jan 13, 2023 •

edited by JohnTitor

Loading

mejrs commented Jan 13, 2023 •

edited

Loading

compiler-errors Feb 1, 2023 •

edited

Loading