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

Support raw-identifier fields in format strings #108

Merged
merged 4 commits into from Nov 4, 2020

Conversation

ninevra
Copy link
Contributor

@ninevra ninevra commented Nov 3, 2020

Fixes #96 by stripping the "r#" prefix from format variables.

Background

Previously, given a type like

#[derive(Error, Debug)]
#[error("a raw ident: {r#reserved}")]`
struct RawError {
    r#reserved: String,
}

thiserror would output something like

// ...
Self { r#reserved } = self;
write!("a raw ident: {r#reserved}", r#reserved = r#reserved);
// ...

This would fail during write!() expansion because the character # isn't allowed in named format!() parameters.

Proposed solution

This PR replaces the r# prefix with raw_field_ when generating named parameters, producing output like

// ...
Self { r#reserved } = self;
write!("a raw ident: {raw_field_reserved}", raw_field_reserved = r#reserved);
// ...

Caveat: Possible name collision

The generated named parameters could collide with user-defined named parameters. For example, the following compiles but produces potentially-unexpected output:

#[derive(Error, Debug)]
#[error("a raw ident: {r#reserved}; and a named parameter: {raw_field_reserved}", raw_field_reserved = "some value")]`
struct RawError {
    r#reserved: String,
}

println!("{}", RawError { r#reserved: "some other value" });

This kind of collision is already possible in thiserror. Existing code maps tuple fields to named parameters of the form field_0. These collisions could be avoided by instead mapping field references to anonymous parameters ({}), which is actually what the documentation claims is done already; however, I think that's outside the scope of this PR.

Further work

A similar but disjoint issue: thiserror assumes all user-defined named parameters are valid identifiers, and panics if it encounters an exception. format!() itself does not require this. I think fixing this is outside the scope of this PR.

@@ -55,7 +55,9 @@ impl Display<'_> {
}
'a'..='z' | 'A'..='Z' | '_' => {
let ident = take_ident(&mut read);
Member::Named(Ident::new(&ident, span))
let mut ident = parse_str::<Ident>(&ident).unwrap();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although this line does use unwrap(), it should only panic in the same cases as Ident::new() already does, i.e. invalid identifiers.

Copy link
Owner

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Thanks!

@dtolnay dtolnay merged commit 7014b69 into dtolnay:master Nov 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Escaped field names do not work
2 participants