Skip to content

Commit

Permalink
Rollup merge of rust-lang#100377 - est31:fluent_grepability, r=davidtwco
Browse files Browse the repository at this point in the history
Replace - with _ in fluent slugs to improve developer workflows

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:
```Rust
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):
```Rust
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):
```fluent
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](rust-lang@cc3c5d2) 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
  • Loading branch information
matthiaskrgr authored Aug 15, 2022
2 parents 969f8af + ca16a8d commit 117085f
Show file tree
Hide file tree
Showing 20 changed files with 443 additions and 384 deletions.
12 changes: 6 additions & 6 deletions compiler/rustc_error_messages/locales/en-US/borrowck.ftl
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
borrowck-move-unsized =
borrowck_move_unsized =
cannot move a value of type `{$ty}`
.label = the size of `{$ty}` cannot be statically determined
borrowck-higher-ranked-lifetime-error =
borrowck_higher_ranked_lifetime_error =
higher-ranked lifetime error
borrowck-could-not-prove =
borrowck_could_not_prove =
could not prove `{$predicate}`
borrowck-could-not-normalize =
borrowck_could_not_normalize =
could not normalize `{$value}`
borrowck-higher-ranked-subtype-error =
borrowck_higher_ranked_subtype_error =
higher-ranked subtype error
generic-does-not-live-long-enough =
generic_does_not_live_long_enough =
`{$kind}` does not live long enough
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
builtin-macros-requires-cfg-pattern =
builtin_macros_requires_cfg_pattern =
macro requires a cfg-pattern as an argument
.label = cfg-pattern required
builtin-macros-expected-one-cfg-pattern = expected 1 cfg-pattern
builtin_macros_expected_one_cfg_pattern = expected 1 cfg-pattern
26 changes: 13 additions & 13 deletions compiler/rustc_error_messages/locales/en-US/const_eval.ftl
Original file line number Diff line number Diff line change
@@ -1,31 +1,31 @@
const-eval-unstable-in-stable =
const_eval_unstable_in_stable =
const-stable function cannot use `#[feature({$gate})]`
.unstable-sugg = if it is not part of the public API, make this function unstably const
.bypass-sugg = otherwise `#[rustc_allow_const_fn_unstable]` can be used to bypass stability checks
.unstable_sugg = if it is not part of the public API, make this function unstably const
.bypass_sugg = otherwise `#[rustc_allow_const_fn_unstable]` can be used to bypass stability checks
const-eval-thread-local-access =
const_eval_thread_local_access =
thread-local statics cannot be accessed at compile-time
const-eval-static-access =
const_eval_static_access =
{$kind}s cannot refer to statics
.help = consider extracting the value of the `static` to a `const`, and referring to that
.teach-note = `static` and `const` variables can refer to other `const` variables. A `const` variable, however, cannot refer to a `static` variable.
.teach-help = To fix this, the value can be extracted to a `const` and then used.
.teach_note = `static` and `const` variables can refer to other `const` variables. A `const` variable, however, cannot refer to a `static` variable.
.teach_help = To fix this, the value can be extracted to a `const` and then used.
const-eval-raw-ptr-to-int =
const_eval_raw_ptr_to_int =
pointers cannot be cast to integers during const eval
.note = at compile-time, pointers do not have an integer value
.note2 = avoiding this restriction via `transmute`, `union`, or raw pointers leads to compile-time undefined behavior
const-eval-raw-ptr-comparison =
const_eval_raw_ptr_comparison =
pointers cannot be reliably compared during const eval
.note = see issue #53020 <https://github.com/rust-lang/rust/issues/53020> for more information
const-eval-panic-non-str = argument to `panic!()` in a const context must have type `&str`
const_eval_panic_non_str = argument to `panic!()` in a const context must have type `&str`
const-eval-mut-deref =
const_eval_mut_deref =
mutation through a reference is not allowed in {$kind}s
const-eval-transient-mut-borrow = mutable references are not allowed in {$kind}s
const_eval_transient_mut_borrow = mutable references are not allowed in {$kind}s
const-eval-transient-mut-borrow-raw = raw mutable references are not allowed in {$kind}s
const_eval_transient_mut_borrow_raw = raw mutable references are not allowed in {$kind}s
4 changes: 2 additions & 2 deletions compiler/rustc_error_messages/locales/en-US/expand.ftl
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
expand-explain-doc-comment-outer =
expand_explain_doc_comment_outer =
outer doc comments expand to `#[doc = "..."]`, which is what this macro attempted to match
expand-explain-doc-comment-inner =
expand_explain_doc_comment_inner =
inner doc comments expand to `#![doc = "..."]`, which is what this macro attempted to match
326 changes: 163 additions & 163 deletions compiler/rustc_error_messages/locales/en-US/lint.ftl

Large diffs are not rendered by default.

24 changes: 12 additions & 12 deletions compiler/rustc_error_messages/locales/en-US/parser.ftl
Original file line number Diff line number Diff line change
@@ -1,34 +1,34 @@
parser-struct-literal-body-without-path =
parser_struct_literal_body_without_path =
struct literal body without path
.suggestion = you might have forgotten to add the struct literal inside the block
parser-maybe-report-ambiguous-plus =
parser_maybe_report_ambiguous_plus =
ambiguous `+` in a type
.suggestion = use parentheses to disambiguate
parser-maybe-recover-from-bad-type-plus =
parser_maybe_recover_from_bad_type_plus =
expected a path on the left-hand side of `+`, not `{$ty}`
parser-add-paren = try adding parentheses
parser_add_paren = try adding parentheses
parser-forgot-paren = perhaps you forgot parentheses?
parser_forgot_paren = perhaps you forgot parentheses?
parser-expect-path = expected a path
parser_expect_path = expected a path
parser-maybe-recover-from-bad-qpath-stage-2 =
parser_maybe_recover_from_bad_qpath_stage_2 =
missing angle brackets in associated item path
.suggestion = try: `{$ty}`
parser-incorrect-semicolon =
parser_incorrect_semicolon =
expected item, found `;`
.suggestion = remove this semicolon
.help = {$name} declarations are not followed by a semicolon
parser-incorrect-use-of-await =
parser_incorrect_use_of_await =
incorrect use of `await`
.parentheses-suggestion = `await` is not a method call, remove the parentheses
.postfix-suggestion = `await` is a postfix operation
.parentheses_suggestion = `await` is not a method call, remove the parentheses
.postfix_suggestion = `await` is a postfix operation
parser-in-in-typo =
parser_in_in_typo =
expected iterable, found keyword `in`
.suggestion = remove the duplicated `in`
Loading

0 comments on commit 117085f

Please sign in to comment.