Add Fields::iter and IntoIterator for &Fields

[sgrif]

These methods were removed in syn 0.12. They seem like an odd removal,
since they're pretty widely used in the wild. This adds these methods
back. Though Fields is the closest structurally to VariantData, I've
opted to put it on DataStruct instead, as it's in the location most
people will look for it. fields.fields() also reads really poorly.

Fixes #316

[dtolnay]

Thanks!

• I would prefer not to introduce a MaybeEmpty type for this. Do you think it would be possible to do this the way proposed in Add a Fields::fields() method which returns a &Punctuated<Field, Token![,]> #316 with a static empty Punctuated?
• I think the methods should be on Fields and not DataStruct. A couple other syntax tree nodes also contain Fields: Variant and ItemStruct.
• To avoid the fields.fields() naming how about AsRef<Punctuated> for Fields and AsMut<Punctuated> for Fields?

[sgrif]

Do you think it would be possible to do this the way proposed in #316 with a static empty Punctuated?

Not on stable. An empty Punctuated would require const fn, and allowing Drop types in statics (or did that get stabilized?). We could use lazy_static! for this, but that felt like overkill vs just adding a small iterator.

I think the methods should be on Fields and not DataStruct. A couple other syntax tree nodes also contain Fields: Variant and ItemStruct.

How would you feel about putting that method on Fields, but then adding a delegator for it in DataStruct (as well as Variant and ItemStruct if you'd like)

[sgrif]

To avoid the fields.fields() naming how about AsRef for Fields and AsMut for Fields?

This can't be done unless we have a static mut EMPTY_FIELDS: Punctuated<Field, Token![,]>, which as I mentioned above we can't do on stable (even if we could, is it worth introducing unsafe code?)

[dtolnay]

Is there a use case for fields_mut? I see that you added it in #26 but I see nothing in diesel's history with git log -S fields_mut and I can't find a single caller in github search.

[sgrif]

I definitely had one at the time I wrote that PR (or I wouldn't have written it), but clearly I didn't go with that implementation. I don't have one off the top of my head. I personally think it's worth having for parity with iter_mut but I'm fine with removing if you'd like

[dtolnay]

I would drop the mut one for now if nobody will use it.

What do you think of adding IntoIterator for &Fields instead of a fields() method? I think for field in &data.fields is clear enough, works equally well for fields in a Variant or ItemStruct, avoids having a fields field and fields() method on the same types that do different things, and avoids fields.fields() stutter.

[sgrif]

I can do that. I do think it's worth continuing to have the explicit iter method on fields in that case though, for the same reason that virtually every type which implements IntoIterator for & does.

[dtolnay]

Seems reasonable. Let's do Fields::iter and IntoIterator for &Fields.

[sgrif]

There you go. :)

[sgrif]

I just figured out how to get rid of MaybeEmpty, too. I'll push that up shortly

[sgrif]

This should be good to go if CI agrees

[mystor]

@sgrif Thanks for getting rid of MaybeEmpty :-). Definitely a cleaner API now.