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.

Sign up for GitHub

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

Add support for captured identifiers #182

Merged
merged 17 commits into from
Feb 9, 2022
Merged

Add support for captured identifiers #182

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
984eb97
Replace `peg` crate with manually implemented parser
ilslv Jan 19, 2022
c0ac03b
Fix for 1.36
ilslv Jan 19, 2022
9cdb6d8
Return Cargo.lock and fmt
ilslv Jan 19, 2022
e0bd159
Revert "Return Cargo.lock and fmt"
ilslv Jan 19, 2022
9bdbe11
Fix
ilslv Jan 19, 2022
1ae08e5
Remove redundant check
ilslv Jan 19, 2022
bc86fca
Remove redundant check
ilslv Jan 19, 2022
74149c4
Implement interpolated strings
ilslv Jan 20, 2022
80194d5
Add width and precision tests
ilslv Jan 20, 2022
91ff444
Document
ilslv Jan 20, 2022
000eefc
Merge branch 'replace-peg-with-manual-parser' into interpolated-strings
ilslv Jan 20, 2022
73c870a
Fix duplicated fields
ilslv Jan 20, 2022
633fa22
Merge branch 'master' into interpolated-strings
ilslv Feb 8, 2022
c0f520a
Merge branch 'master' into interpolated-strings
ilslv Feb 8, 2022
9ce2b69
Corrections
ilslv Feb 8, 2022
3bb76fd
Remove early return
ilslv Feb 8, 2022
1d262f2
Corrections
tyranron Feb 9, 2022
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
7 changes: 7 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ syn = "1.0.81"
convert_case = { version = "0.4", optional = true}
unicode-xid = { version = "0.2.2", optional = true }

[dev-dependencies]
rustversion = "1.0.6"

[build-dependencies]
rustc_version = { version = "0.4", optional = true }

Expand Down
8 changes: 6 additions & 2 deletions doc/display.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ The variables available in the arguments is `self` and each member of the varian
with members of tuple structs being named with a leading underscore and their index,
i.e. `_0`, `_1`, `_2`, etc.

Although [captured identifiers in format strings are supported only since `1.58`](https://blog.rust-lang.org/2022/01/13/Rust-1.58.0.html#captured-identifiers-in-format-strings) we support this feature on earlier version of Rust. This means that `#[display(fmt = "Prefix: {field}")]` is completely valid on MSRV.

> __NOTE:__ Underscored named parameters like `#[display(fmt = "Prefix: {_0}")]` [are supported since `1.41`](https://github.com/rust-lang/rust/pull/66847)

## Other formatting traits

The syntax does not change, but the name of the attribute is the snake case version of the trait.
Expand Down Expand Up @@ -110,11 +114,11 @@ use std::path::PathBuf;
struct MyInt(i32);

#[derive(DebugCustom)]
#[debug(fmt = "MyIntDbg(as hex: {:x}, as dec: {})", _0, _0)]
#[debug(fmt = "MyIntDbg(as hex: {_0:x}, as dec: {_0})")]
struct MyIntDbg(i32);

#[derive(Display)]
#[display(fmt = "({}, {})", x, y)]
#[display(fmt = "({x}, {y})")]
struct Point2D {
x: i32,
y: i32,
Expand Down
160 changes: 109 additions & 51 deletions src/display.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ impl<'a, 'b> State<'a, 'b> {
}
};

// TODO: Check for a single `Display` group?
match &list.nested[0] {
syn::NestedMeta::Meta(syn::Meta::NameValue(syn::MetaNameValue {
path,
Expand All @@ -359,43 +360,26 @@ impl<'a, 'b> State<'a, 'b> {
== "fmt" =>
{
let expected_affix_usage = "outer `enum` `fmt` is an affix spec that expects no args and at most 1 placeholder for inner variant display";
let placeholders = Placeholder::parse_fmt_string(&fmt.value());
if outer_enum {
if list.nested.iter().skip(1).count() != 0 {
return Err(Error::new(
list.nested[1].span(),
expected_affix_usage,
));
}
// TODO: Check for a single `Display` group?
let fmt_string = match &list.nested[0] {
syn::NestedMeta::Meta(syn::Meta::NameValue(
syn::MetaNameValue {
path,
lit: syn::Lit::Str(s),
..
},
)) if path
.segments
if placeholders.len() > 1
|| placeholders
.first()
.expect("path shouldn't be empty")
.ident
== "fmt" =>
{
s.value()
}
// This one has been checked already in get_meta_fmt() method.
_ => unreachable!(),
};

let num_placeholders =
Placeholder::parse_fmt_string(&fmt_string).len();
if num_placeholders > 1 {
.map(|p| p.arg != Argument::Integer(0))
.unwrap_or_default()
{
return Err(Error::new(
list.nested[1].span(),
expected_affix_usage,
));
}
if num_placeholders == 1 {
if placeholders.len() == 1 {
return Ok((quote_spanned!(fmt.span()=> #fmt), true));
}
}
Expand All @@ -421,8 +405,28 @@ impl<'a, 'b> State<'a, 'b> {
Ok(quote_spanned!(list.span()=> #args #arg,))
})?;

let interpolated_args = placeholders
.into_iter()
.flat_map(|p| {
let map_argument = |arg| match arg {
Argument::Ident(i) => Some(i),
Argument::Integer(_) => None,
};
map_argument(p.arg)
.into_iter()
.chain(p.width.and_then(map_argument))
.chain(p.precision.and_then(map_argument))
})
.collect::<HashSet<_>>()
.into_iter()
.map(|ident| {
let ident = syn::Ident::new(&ident, fmt.span());
quote! { #ident = #ident, }
})
.collect::<TokenStream>();

Ok((
quote_spanned!(meta.span()=> write!(_derive_more_display_formatter, #fmt, #args)),
quote_spanned!(meta.span()=> write!(_derive_more_display_formatter, #fmt, #args #interpolated_args)),
false,
))
}
Expand Down Expand Up @@ -665,10 +669,7 @@ impl<'a, 'b> State<'a, 'b> {
_ => unreachable!(),
})
.collect();
if fmt_args.is_empty() {
return HashMap::default();
}
let fmt_string = match &list.nested[0] {
let (fmt_string, fmt_span) = match &list.nested[0] {
syn::NestedMeta::Meta(syn::Meta::NameValue(syn::MetaNameValue {
path,
lit: syn::Lit::Str(s),
Expand All @@ -680,7 +681,7 @@ impl<'a, 'b> State<'a, 'b> {
.ident
== "fmt" =>
{
s.value()
(s.value(), s.span())
}
// This one has been checked already in get_meta_fmt() method.
_ => unreachable!(),
Expand All @@ -689,7 +690,12 @@ impl<'a, 'b> State<'a, 'b> {
Placeholder::parse_fmt_string(&fmt_string).into_iter().fold(
HashMap::default(),
|mut bounds, pl| {
if let Some(arg) = fmt_args.get(&pl.position) {
let arg = match pl.arg {
Argument::Integer(i) => fmt_args.get(&i).cloned(),
Argument::Ident(i) => Some(syn::Ident::new(&i, fmt_span).into()),
};

if let Some(arg) = &arg {
if fields_type_params.contains_key(arg) {
bounds
.entry(fields_type_params[arg].clone())
Expand Down Expand Up @@ -733,11 +739,45 @@ impl<'a, 'b> State<'a, 'b> {
}
}

/// [`Placeholder`] argument.
#[derive(Debug, PartialEq)]
enum Argument {
/// [Positional parameter][1].
///
/// [1]: https://doc.rust-lang.org/stable/std/fmt/index.html#positional-parameters
Integer(usize),

/// [Named parameter][1].
///
/// [1]: https://doc.rust-lang.org/stable/std/fmt/index.html#named-parameters
Ident(String),
}

impl<'a> From<parsing::Argument<'a>> for Argument {
fn from(arg: parsing::Argument<'a>) -> Self {
match arg {
parsing::Argument::Integer(i) => Argument::Integer(i),
parsing::Argument::Identifier(i) => Argument::Ident(i.to_owned()),
}
}
}

/// Representation of formatting placeholder.
#[derive(Debug, PartialEq)]
struct Placeholder {
/// Position of formatting argument to be used for this placeholder.
position: usize,
arg: Argument,

/// [Width argument][1], if present.
///
/// [1]: https://doc.rust-lang.org/stable/std/fmt/index.html#width
width: Option<Argument>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we better align here with the terminology used in the linked docs, not the one proposed by the formal grammar (we may keep it just for the parsing).

So, Argument - > Parameter, Ident -> Named, Integer -> Positional.


/// [Precision argument][1], if present.
///
/// [1]: https://doc.rust-lang.org/stable/std/fmt/index.html#precision
precision: Option<Argument>,

/// Name of [`std::fmt`] trait to be used for rendering this placeholder.
trait_name: &'static str,
}
Expand All @@ -754,19 +794,25 @@ impl Placeholder {
format.arg,
format.spec.map(|s| s.ty).unwrap_or(parsing::Type::Display),
);
let position = maybe_arg
.and_then(|arg| match arg {
parsing::Argument::Integer(i) => Some(i),
parsing::Argument::Identifier(_) => None,
})
.unwrap_or_else(|| {
// Assign "the next argument".
// https://doc.rust-lang.org/stable/std/fmt/index.html#positional-parameters
n += 1;
n - 1
});
let position = maybe_arg.map(Into::into).unwrap_or_else(|| {
// Assign "the next argument".
// https://doc.rust-lang.org/stable/std/fmt/index.html#positional-parameters
n += 1;
Argument::Integer(n - 1)
});

Placeholder {
position,
arg: position,
width: format.spec.and_then(|s| match s.width {
Some(parsing::Count::Parameter(arg)) => Some(arg.into()),
_ => None,
}),
precision: format.spec.and_then(|s| match s.precision {
Some(parsing::Precision::Count(parsing::Count::Parameter(
arg,
))) => Some(arg.into()),
_ => None,
}),
trait_name: ty.trait_name(),
}
})
Expand All @@ -780,32 +826,44 @@ mod placeholder_parse_fmt_string_spec {

#[test]
fn indicates_position_and_trait_name_for_each_fmt_placeholder() {
let fmt_string = "{},{:?},{{}},{{{1:0$}}}-{2:.1$x}{0:#?}{:width$}";
let fmt_string = "{},{:?},{{}},{{{1:0$}}}-{2:.1$x}{par:#?}{:width$}";
assert_eq!(
Placeholder::parse_fmt_string(&fmt_string),
vec![
Placeholder {
position: 0,
arg: Argument::Integer(0),
width: None,
precision: None,
trait_name: "Display",
},
Placeholder {
position: 1,
arg: Argument::Integer(1),
width: None,
precision: None,
trait_name: "Debug",
},
Placeholder {
position: 1,
arg: Argument::Integer(1),
width: Some(Argument::Integer(0)),
precision: None,
trait_name: "Display",
},
Placeholder {
position: 2,
arg: Argument::Integer(2),
width: None,
precision: Some(Argument::Integer(1)),
trait_name: "LowerHex",
},
Placeholder {
position: 0,
arg: Argument::Ident("par".to_owned()),
width: None,
precision: None,
trait_name: "Debug",
},
Placeholder {
position: 2,
arg: Argument::Integer(2),
width: Some(Argument::Ident("width".to_owned())),
precision: None,
trait_name: "Display",
},
],
Expand Down
Loading