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

Lint against escape sequences in Fluent files #109700

Merged
merged 1 commit into from
Mar 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion compiler/rustc_codegen_llvm/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ codegen_llvm_error_calling_dlltool =
Error calling dlltool: {$error}

codegen_llvm_dlltool_fail_import_library =
Dlltool could not create import library: {$stdout}\n{$stderr}
Dlltool could not create import library: {$stdout}
{$stderr}

codegen_llvm_target_feature_disable_or_enable =
the target features {$features} must all be either enabled or disabled together
Expand Down
16 changes: 12 additions & 4 deletions compiler/rustc_const_eval/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,25 @@ const_eval_unstable_const_fn = `{$def_path}` is not yet stable as a const fn
const_eval_unallowed_mutable_refs =
mutable references are not allowed in the final value of {$kind}s
.teach_note =
References in statics and constants may only refer to immutable values.\n\n
References in statics and constants may only refer to immutable values.


Statics are shared everywhere, and if they refer to mutable data one might violate memory
safety since holding multiple mutable references to shared data is not allowed.\n\n
safety since holding multiple mutable references to shared data is not allowed.


If you really want global mutable state, try using static mut or a global UnsafeCell.

const_eval_unallowed_mutable_refs_raw =
raw mutable references are not allowed in the final value of {$kind}s
.teach_note =
References in statics and constants may only refer to immutable values.\n\n
References in statics and constants may only refer to immutable values.


Statics are shared everywhere, and if they refer to mutable data one might violate memory
safety since holding multiple mutable references to shared data is not allowed.\n\n
safety since holding multiple mutable references to shared data is not allowed.


If you really want global mutable state, try using static mut or a global UnsafeCell.

const_eval_non_const_fmt_macro_call =
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_incremental/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ incremental_field_associated_value_expected = associated value expected for `{$n
incremental_no_field = no field `{$name}`

incremental_assertion_auto =
`except` specified DepNodes that can not be affected for \"{$name}\": \"{$e}\"
`except` specified DepNodes that can not be affected for "{$name}": "{$e}"

incremental_undefined_clean_dirty_assertions_item =
clean/dirty auto-assertions not yet defined for Node::Item.node={$kind}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ lint_ty_qualified = usage of qualified `ty::{$ty}`
lint_lintpass_by_hand = implementing `LintPass` by hand
.help = try using `declare_lint_pass!` or `impl_lint_pass!` instead

lint_non_existant_doc_keyword = found non-existing keyword `{$keyword}` used in `#[doc(keyword = \"...\")]`
lint_non_existant_doc_keyword = found non-existing keyword `{$keyword}` used in `#[doc(keyword = "...")]`
.help = only existing keywords are allowed in core/std

lint_diag_out_of_impl =
Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_macros/src/diagnostics/fluent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,18 @@ pub(crate) fn fluent_messages(input: proc_macro::TokenStream) -> proc_macro::Tok
.emit();
return failed(&crate_name);
}
let mut bad = false;
for esc in ["\\n", "\\\"", "\\'"] {
for _ in resource_contents.matches(esc) {
bad = true;
Diagnostic::spanned(resource_span, Level::Error, format!("invalid escape `{esc}` in Fluent resource"))
.note("Fluent does not interpret these escape sequences (<https://projectfluent.org/fluent/guide/special.html>)")
.emit();
}
}
if bad {
return failed(&crate_name);
}

let resource = match FluentResource::try_new(resource_contents) {
Ok(resource) => resource,
Expand Down
1 change: 1 addition & 0 deletions tests/ui-fulldeps/fluent-messages/invalid-escape.ftl
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
no_crate_bad_escape = don't use \n, \', or \"
9 changes: 9 additions & 0 deletions tests/ui-fulldeps/fluent-messages/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,12 @@ mod missing_message_ref {
fluent_messages! { "./missing-message-ref.ftl" }
//~^ ERROR referenced message `message` does not exist
}

mod bad_escape {
use super::fluent_messages;

fluent_messages! { "./invalid-escape.ftl" }
//~^ ERROR invalid escape `\n`
//~| ERROR invalid escape `\"`
//~| ERROR invalid escape `\'`
}
26 changes: 25 additions & 1 deletion tests/ui-fulldeps/fluent-messages/test.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -83,5 +83,29 @@ LL | fluent_messages! { "./missing-message-ref.ftl" }
|
= help: you may have meant to use a variable reference (`{$message}`)

error: aborting due to 10 previous errors
error: invalid escape `\n` in Fluent resource
--> $DIR/test.rs:99:24
|
LL | fluent_messages! { "./invalid-escape.ftl" }
| ^^^^^^^^^^^^^^^^^^^^^^
|
= note: os-specific message
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit interesting, it's caused by // normalize-stderr-test "note.*" -> "note: os-specific message" in test.rs. It was added because the "could not open Fluent resource" error contains an OS specific error as the note. I think it would be nice if this replacement didn't affect this error.

To give two suggestions how to address this, either one could extend the note of the "could not open" error to something that a better pattern can be made for, or one could move that error into a different file.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made a PR: #109798


error: invalid escape `\"` in Fluent resource
--> $DIR/test.rs:99:24
|
LL | fluent_messages! { "./invalid-escape.ftl" }
| ^^^^^^^^^^^^^^^^^^^^^^
|
= note: os-specific message

error: invalid escape `\'` in Fluent resource
--> $DIR/test.rs:99:24
|
LL | fluent_messages! { "./invalid-escape.ftl" }
| ^^^^^^^^^^^^^^^^^^^^^^
|
= note: os-specific message

error: aborting due to 13 previous errors

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: found non-existing keyword `tadam` used in `#[doc(keyword = \"...\")]`
error: found non-existing keyword `tadam` used in `#[doc(keyword = "...")]`
--> $DIR/existing_doc_keyword.rs:10:1
|
LL | #[doc(keyword = "tadam")]
Expand Down