Replace - with _ in fluent slugs to improve developer workflows

[est31]

This is a proposal to smoothen the compiler contribution experience in the face of the move to fluent.

Context

The fluent project has introduced a layer of abstraction to compiler errors. Previously, people would write down error messages directly in the same file the code was located to emit them. Now, there is a slug that connects the code in the compiler to the error message in the ftl file.

You can look at 7ef610c to see an example of the changes:

Old:

let msg = format!(
    "bounds on `{}` are most likely incorrect, consider instead \
        using `{}` to detect whether a type can be trivially dropped",
    predicate,
    cx.tcx.def_path_str(needs_drop)
);
lint.build(&msg).emit();

New (Rust side):

lint.build(fluent::lint::drop_trait_constraints)
    .set_arg("predicate", predicate)
    .set_arg("needs_drop", cx.tcx.def_path_str(needs_drop))
    .emit();

New (Fluent side):

lint-drop-trait-constraints =
    bounds on `{$predicate}` are most likely incorrect, consider instead using `{$needs_drop}` to detect whether a type can be trivially dropped

You will note that in the ftl file, the slug is slightly different from the slug in the Rust file: The ftl slug uses - (e.g. lint-drop-trait-constraints) while the rust slug uses :: and _ (e.g. lint::drop_trait_constraints). This choice was probably done due to:

• Rust not accepting - in identifiers (as it is an operator)
• fluent not supporting the : character in slug names (parse error upon attempts)
• all official fluent documentation using - instead of _

The problem

The two different types of slugs, one with -, and one with _, cause difficulties for contributors. Imagine you don't have perfect knowledge of where stuff is in the compiler (i would say this is most people), and you encounter an error for which you think there is something you could improve that is not just a rewording.

So you want to find out where in the compiler's code that error is being emitted. The best way is via grepping.

1. you grep for the message in the compiler's source code. You discover the ftl file and find out the slug for that error.
2. That slug however contains - instead of _, so you have to manually translate the -'s into _s, and furthermore either remove the leading module name, or replace the first - with a ::.
3. you do a second grep to get to the emitting location in the compiler's code.

This translation difficulty in step 2 appears also in the other direction when you want to figure out what some code in the compiler is doing and use error messages to help your understanding. Comments and variable names are way less exposed to users so are more likely going to lie than error messages.

I think that at least the -→_ translation which makes up most of step 2 can be removed at low cost.

The solution

If you look closely, the practice of fluent to use - is only a stylistic choice and it is not enforced by fluent implementations, neither the playground nor the one the rust compiler uses, that slugs may not contain _. Thus, we can in fact migrate the ftl side to _. So now we'll have slugs like lint_drop_trait_constraints on the ftl side. You only have to do one replacement now to get to the Rust slug: remove the first _ and place a :: in its stead. I would argue that this change is in fact useful as it allows you to control whether you want to look at the rust side of things or the ftl side of things via changing the query string only: with an increased number of translations checked into the repository, grepping for raw slugs will return the slug in many ftl files, so an explicit step to look for the source code is always useful. In the other direction (rust to fluent), you don't need a translation at all any more, as you can just take the final piece of the slug (e.g. drop_trait_constraints) and grep for that. The PR also adds enforcement to forbid usage of _ in slug names. Internal slug names (those leading with a -) are exempt from that enforcement.

As another workflow that benefits from this change, people who add new errors don't have to do that - conversion either.

Before/After	Fluent slug	Rust slug (no change)
Before	lint-drop-trait-constraints	lint::drop_trait_constraints
After	lint_drop_trait_constraints	lint::drop_trait_constraints

Note that I've suggested this previously in the translation thread on zulip. I think it's important to think about non-translator contribution impact of fluent. I have certainly plans for more improvements, but this is a good first step.

@rustbot label A-diagnostics

[rust-highfive]

r? @cjgillot

(rust-highfive has picked a reviewer for you, use r? to override)

[compiler-errors]

cc @davidtwco

[compiler-errors]

I think this change is fine, we really should be using underscores for grep-ability.

[davidtwco]

r? @davidtwco

[davidtwco]

I'm okay with this change - I don't anticipate any issues with an all-_ slug scheme and it increases uniformity.

Looks like you need to fix src/test/run-make/translation which relies on overriding one of the built-in slugs.

[davidtwco]

Could you submit a PR to the dev guide documenting this and updating the examples there too? Please ping me on it :)

(this can be after this lands, just so that it happens)

[deltragon]

Is it possible to mix - and _ in a slug? If so, would it make sense to make it lint::drop_trait_constraints in Rust and lint-drop_trait_constraints in fluent, to differentiate which part can be grepped for?
Because with this change, grepping for lint_drop_trait_constraints would still not result in anything, but if the slug in fluent is somewhat split by one part using - and another using _ it might nudge people to grep for drop_trait_constraints instead.

[est31]

Thanks @davidtwco and @compiler-errors for the positive feedback!

@deltragon from a technical point of view, it's possible, yes. And at least both in Kate and in VS Codium - constitutes a double click boundary while _ unifies, so you can obtain the part you can grep for more quickly. On the other hand, it looks a bit weird compared to pure - or _ slugs. And the naming scheme might be harder to understand, idk. Personally I'd be fine with both pure _ slugs and the mixed scheme. Note that for helper slugs, one needs a starting - so some mixing is present already. I'm interested in what others think.

The only allowed chars for fluent message identifiers are ones for which u8::is_ascii_alphanumeric returns true as well as - and _ (and at the start only ascii alphabetic chars are allowed), so, barring modifications of fluent itself, any solution would have to be built out of those.

[davidtwco]

@deltragon from a technical point of view, it's possible, yes. And at least both in Kate and in VS Codium - constitutes a double click boundary while _ unifies, so you can obtain the part you can grep for more quickly. On the other hand, it looks a bit weird compared to pure - or _ slugs. And the naming scheme might be harder to understand, idk. Personally I'd be fine with both pure _ slugs and the mixed scheme. Note that for helper slugs, one needs a starting - so some mixing is present already. I'm interested in what others think.

I don't have a strong opinion on this - either would be fine with me. I guess I'd prefer that if we do have mixed - and _ that the - seperator is used only once, between the crate name and the slug, but not within the slug or within the crate name.

[davidtwco]

@bors r+

[bors]

📌 Commit ddc194171118535d473ff7195b4e4e1763d86abd has been approved by davidtwco

It is now in the queue for this repository.

[est31]

@rustbot ready

[est31]

Can someone add r=davidtwco? I've just rebased it. Thanks.

[davidtwco]

@bors r+

[bors]

📌 Commit ca16a8d has been approved by davidtwco

It is now in the queue for this repository.