RFR: 8328821: Map.of().entrySet() mutators should throw UnsupportedOperationException [v3]

[Liam Miller-Cushon]

On Mon, 8 Jul 2024 20:39:38 GMT, Chen Liang <liach at openjdk.org> wrote:

>> Liam Miller-Cushon has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains six additional commits since the last revision:
>> 
>>  - Merge remote-tracking branch 'origin/master' into JDK-8328821-make-clear-consistent
>>  - Check m.entrySet().hashCode() in MOAT
>>  - Merge remote-tracking branch 'origin/master' into JDK-8328821-make-clear-consistent
>>  - Use AbstractImmutableSet
>>  - Throw UOE for all Map.of().entrySet() mutator methods
>>  - 8328821: Make the ImmutableCollections clear() call consistent
>>    
>>    Without overriding clear(), a call to it in an empty map would
>>    just return, as iterator.hasNext() would be false. However if
>>    calling Map.of().clear() throws an exception. To make the
>>    behavior of Map.of().entrySet().clear() consistent, we need to
>>    have an implementation of clear() for the entry set that throws
>>    as well.
>
> test/jdk/java/util/Map/MapFactories.java line 505:
> 
>> 503: 
>> 504:     @Test(expectedExceptions=UnsupportedOperationException.class)
>> 505:     public void immutableEntrySetAddAllDisallowed() {
> 
> Looking back at MOAT, do you think we should add these into MOAT?
> https://github.com/openjdk/jdk/blob/598af2e51464be089b64da4024e62865c2c6ec72/test/jdk/java/util/Collection/MOAT.java#L594-L619
> 
> We just need to add calls to `testMapMutatorsAlwaysThrow` and `testEmptyMapMutatorsAlwaysThrow` to check
> `test(Empty)CollMutatorsAlwaysThrow(map.entrySet());`, `test(Empty)CollMutatorsAlwaysThrow(map.keySet());`, and `test(Empty)CollMutatorsAlwaysThrow(map.values());`

`testCollMutatorsAlwaysThrow` expects a `Collection<Integer>` (not e.g. a `Collection<Entry<Integer, Integer>>`). MOAT could be refactored to handle that case. Do you think that's worth it, or have thoughts about what the cleanest way to do that would be?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18522#discussion_r1670899976