error on empty precision

[hkBst]

Fixes #131159 by erroring on missing precision. Alternatively we could document current behavior.

[rustbot]

r? @petrochenkov

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[oli-obk]

@bors try

[oli-obk]

Please add a ui test for this

[bors]

⌛ Trying commit ef04c64 with merge 77e9660...

[bors]

☀️ Try build successful - checks-actions
Build commit: 77e9660 (77e966048a5976133ba6b8f26d38d558f2a84355)

[oli-obk]

@craterbot check

[craterbot]

👌 Experiment pr-136638 created and queued.
🤖 Automatically detected try build 77e9660
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🚧 Experiment pr-136638 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🎉 Experiment pr-136638 is completed!
📊 365 regressed and 5 fixed (578841 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[hkBst]

One report of a bug caught by this because "{:2.?}" was written but "{:.2?}" was meant.

[Noratrieb]

Given the large amount of regressions, it does not appear appropriate to make this an error. Documenting the behavior seems good (and potentially emit a lint that it's useless).

[hkBst]

I agree that a warning would be better, but I'm not sure how to turn this error into one yet. Any pointers would be welcome, otherwise I'm not sure when I'll get to this.

[petrochenkov]

Ah, this is not about lexer, then r? compiler
(I have too many assigned PRs)

[petrochenkov]

This is sort of a library issue too (a built-in proc macro).
cc @m-ou-se in case you have an opinion on this.

[m-ou-se]

Given how many crates were broken in the crater run, I don't think we should change this. (See all the PRs referenced above.)

Perhaps a warning is better. Or we can just document it.

[Amanieu]

We discussed this in the libs-api meeting and decided that the best path forward would be a forward-compatibility warning which will give us the possibility of making this a hard error in the future. We definitely shouldn't make it an error immediately due to the widespread potential breakage.

[traviscross]

Probably this FCW should go ahead and just start by linting in deps.

[m-ou-se]

I'm not convinced that this needs a (forward-compatibility) warning.

A default placeholder can be written as {} or {:} or {:.}. Should we also warn about {:}? I don't think we should. But then I'm not sure why we would warn about {:.}.

It seems that in every single occurence of {:.}, the author got exactly what they wanted: a placeholder with default settings. A warning would make sense if it is an easy mistake that looks very similar to something with a different meaning, but that's not the case here.

[Noratrieb]

I think a clippy lint would be reasonable, it's a needlessly complicated way to write {}. But I agree that we shouldn't be emitting future compat warnings for it, especially not in deps.

[traviscross]

The other option, of course, is to document the current behavior. We do get this right for {:}, because we document:

format := '{' [ argument ] [ ':' format_spec ] [ ws ] * '}'
format_spec := [[fill]align][sign]['#']['0'][width]['.' precision]type
type := '' | '?' | 'x?' | 'X?' | identifier

So since type includes '', we must accept {:}. But, for {:.}, we'd look at:

format := '{' [ argument ] [ ':' format_spec ] [ ws ] * '}'
argument := integer | identifier
format_spec := [[fill]align][sign]['#']['0'][width]['.' precision]type
precision := count | '*'
count := parameter | integer
parameter := argument '$'

Here, this bottoms out at requiring something after the dot.

We could say, instead, if we want, e.g.:

precision := count | '*' | ''

Since we do also accept today, e.g. {:.x}, that seems right. Or alternatively, and maybe more directly, we could say:

format_spec := [[fill]align][sign]['#']['0'][width]['.' [precision]]type