Skip to content

Commit

Permalink
Fix positional tuple fields in Display impl (#20)
Browse files Browse the repository at this point in the history
* Fix positional tuple fields in Display impl

The field names (numeric indices) were not being stripped for tuple
variants. This caused errors when attempting to output Display impls
which did not reference every sequential index.

This change makes it possible to reference arbitrary tuple fields
as intended.

* Remove an unnecessary unwrap

`fmt::Write` for `String` is infallible. We can safely ignore the
`Result`.
  • Loading branch information
parasyte authored Jun 30, 2024
1 parent 6e34eb7 commit 2a9a45a
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 11 deletions.
1 change: 1 addition & 0 deletions compile_tests/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ fn compile_tests() {
if rustversion::cfg!(all(stable, since(1.68.0))) {
t.compile_fail("compile_tests/no_display_no_impl.rs");
}
t.pass("compile_tests/skip_positional.rs");
}
2 changes: 2 additions & 0 deletions compile_tests/empty.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![allow(dead_code)]

#[derive(Debug, onlyerror::Error)]
enum Error {}

Expand Down
9 changes: 9 additions & 0 deletions compile_tests/skip_positional.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#![allow(dead_code)]

#[derive(Debug, onlyerror::Error)]
enum Error {
/// Skip positional tuple fields {1}
Skip(u8, u8),
}

fn main() {}
19 changes: 9 additions & 10 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,22 +142,21 @@ pub fn derive_error(input: TokenStream) -> TokenStream {
v.display_fields
.iter()
.fold(String::new(), |mut fields, field| {
write!(fields, "{field},").unwrap();
let _ = write!(fields, "{field},");
fields
});

Ok(match &v.ty {
VariantType::Unit => format!("Self::{name} => write!(f, {display:?}),"),
VariantType::Tuple => {
let fields = (0..v.fields.len())
.map(|i| {
if v.display_fields.contains(&Rc::from(format!("field_{i}"))) {
format!("field_{i},")
} else {
String::from("_,")
}
})
.collect::<String>();
let fields = (0..v.fields.len()).fold(String::new(), |mut fields, i| {
if v.display_fields.contains(&Rc::from(format!("field_{i}"))) {
let _ = write!(fields, "field_{i},");
} else {
let _ = fields.write_str("_,");
}
fields
});
format!("Self::{name}({fields}) => write!(f, {display:?}, {display_fields}),")
}
VariantType::Struct => {
Expand Down
12 changes: 11 additions & 1 deletion src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ impl Variant {
.to_string();

// Collect field references.
let display_fields = display
let display_fields: Vec<Rc<str>> = display
.split('{')
.skip(1)
.filter_map(|s| s.split('}').next())
Expand All @@ -170,6 +170,16 @@ impl Variant {

// Remove field references from format string.
for field in &display_fields {
// Special case for tuples
if ty == VariantType::Tuple {
if let Some(num) = field.strip_prefix("field_") {
display = display
.replace(&format!("{{{num}:"), "{:")
.replace(&format!("{{{num}}}"), "{}");
continue;
}
}

display = display
.replace(&format!("{{{field}:"), "{:")
.replace(&format!("{{{field}}}"), "{}");
Expand Down

0 comments on commit 2a9a45a

Please sign in to comment.