Feature: Iterate over the capture groups of a match

[RageKnify]

I'm working on boa-dev/boa#686, and really missing the ability to do something like:

for group in mat.groups() {
    // use group
}

I have implemented it with the following struct serving as the iterator:

pub struct Groups<'m> {
    mat: &'m Match,
    i: usize,
}

The Iterator returns Option<Option<Range>> ( Some(None) is possible because a capture group might not exist if it is optional).
I'll open a PR soon.

[ridiculousfish]

You can iterate over captured ranges with for c in match.captures.iter(), does that meet your needs?

[RageKnify]

Not really, most of the places I need it in boa I want to also include "capture group 0", the match itself, and match.captures.iter() doesn't give access to that.
I can work around it, but it becomes an ugly pattern of creating a vector with the required capacity, pushing the match and only then extending it with the iterator of match.captures.

[ridiculousfish]

Ok! I'm happy to merge a new API, I think it can be done without adding a new struct, how about like this?

pub fn groups(&self) -> impl Iterator<Item = Option<Range>> + '_ {
    iter::once(Some(self.total())).chain(self.captures.iter().cloned())
}

[RageKnify]

Yeah, that looks like it would allow us to do what we need in boa.
Want me to modify my PR or do you want to do it yourself?

[ridiculousfish]

Please go ahead and modify your PR because you're in the best position to test it for your needs. Thanks!

[RageKnify]

I've tested it and it seems to be working fine, I'll be updating the PR.

[BurntSushi]

I would advise against putting impl Trait in your APIs unless you need it for anonymous types. One problem, for example, is that you need to list all traits that you need. For example, your proposed return type implies you cannot print a debug representation of the iterator. Secondly, and more importantly, impl Trait is quite restrictive and can't be used in most places. For example, it would be tricky to write a wrapper iterator over impl Trait, which comes in handy from time to time.

[ridiculousfish]

@BurntSushi Thank you, I'm new to Rust and that's valuable API design guidance. Is the convention to define a real struct type for every iterator?

[BurntSushi]

Generally speaking, yes. At least, for public APIs anyway. For internal code, it's less of an issue to use impl Trait more liberally, since you can pretty easily fix it whenever you want.

It is possible that impl Trait will become more versatile in the language, in which case, conventions may change.

[BurntSushi]

(This does wind up creating a preponderance of types. This is an issue IMO because not all types have equal significance, but rustdoc displays all types as if they do. It is IMO one of rustdoc's biggest weaknesses. To counteract that, I usually link to the most significant types in the top level crate docs as quickly as I can.)