On Fri, 20 Jun 2025 20:56:17 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:
>> The intent here is not to change behaviour regarding nulls.
>> Nulls have been permitted, and should stay permitted.
>> Other object types (that don't cast to Role, in this file) should fail.
>>
>> The Role related files are quite unusual to use, so expect they are mostly
>> used only by the JDK.
>> MBean code might more commonly manipulate an AttributeList, and we can still
>> permit nulls in case such code relies on nulls being accepted.
>
> My question was because there are checks for `null` in this class:
>
> public void add(Role role)
> throws IllegalArgumentException {
>
> if (role == null) {
> throw new IllegalArgumentException("Invalid parameter");
> }
> checkTypeSafe(role);
> super.add(role);
> }
>
> It is kind of confusing and not clear where `null` is allowed and where it is
> not.
> Should it be more consistent?
Sorry, to be clearer:
javax/management/AttributeList does accept nulls being added, and should
continue to do so because it might be a disruptive change.
RoleList and RoleUnresolvedList do not accept null Role/RoleUnresolved objects:
they document that the add() method throws if given a null.
We don't have a need to change either behaviour about accepting nulls.
It should be clear when reading the api docs. It might be unclear as I am
grouping together somewhat unrelated classes here. There is no reason for a
user of AttributeList to expect it behaves the same way as a RoleList.
But we do want to stop accepting alien Objects!
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25856#discussion_r2161794760
