guava icon indicating copy to clipboard operation
guava copied to clipboard

Support building `ImmutableRangeSet`s using overlapping ranges

Open Stephan202 opened this issue 7 years ago • 4 comments

In the ImmutableRangeSet.Builder source code we have this comment:

https://github.com/google/guava/blob/ce73003d8420a091fd36ca2c7c23deb6437e4fe1/guava/src/com/google/common/collect/ImmutableRangeSet.java#L725

That sounds like a neat improvement onto itself. But the ImmutableRangeSet collector introduced in #2750 is also defined in terms of ImmutableRangeSet.Builder:

https://github.com/google/guava/blob/ce73003d8420a091fd36ca2c7c23deb6437e4fe1/guava/src/com/google/common/collect/CollectCollectors.java#L106-L113

So as it stands one can only use toImmutableRangeSet() to collect a stream of non-overlapping ranges; otherwise an IllegalArgumentException is thrown. This is somewhat unintuitive from an API point of view. Maybe the introduction of the collector increases the utility of the builder sufficiently to justify spending time on adding support for overlapping ranges?

Stephan202 avatar Jan 16 '18 22:01 Stephan202

If I understand right, we have at least two separate questions:

  1. Should the collector be based on add or union?
  2. Should the builder expose a union method?

kevinb9n avatar Jan 18 '18 21:01 kevinb9n

@kevinb9n exactly! I guess I should have summarized this more clearly myself.

Stephan202 avatar Jan 18 '18 21:01 Stephan202

Error Prone was also impacted by this limitation: google/error-prone#4040. Would still be nice to have ImmutableRangeSet.Builder support overlapping ranges. :pray:

Stephan202 avatar Aug 07 '23 11:08 Stephan202

Now google-java-format: https://github.com/google/google-java-format/commit/c4b467d50bd6bb52935075b438f602ff902db167

cpovirk avatar Apr 06 '24 15:04 cpovirk