JDK 16 RFR of JDK-8250246: Address reliance on default constructors in security libs

[Joe Darcy]

Hi Sean,

On 7/24/2020 4:52 AM, Sean Mullan wrote:
> Hi Joe,
>
> On 7/24/20 1:17 AM, Joe Darcy wrote:
>> Hello,
>>
>> Please review a set of changes to add explicit constructors to 
>> replace default (implicit) constructors in various classes in 
>> security libs across several modules:
>>
>>      JDK-8250246: Address reliance on default constructors in 
>> security libs
>>      http://cr.openjdk.java.net/~darcy/8250246.0/
>>      CSR: https://bugs.openjdk.java.net/browse/JDK-8250248
>>
>> (This is a companion to similar changes being made across other 
>> libraries in preparation for creating a new javac lint warning.)
>>
>> One of the classes seems to only accidentally have a constructor, so 
>> I added that one as terminally deprecated.
>
> You have not updated the copyright dates; not sure if that is 
> necessary for this type of change.

Right; I run a script that handles the copyright updates before a push 
-- less clutter in a review like this where the per-file changes is 
small and reduces the possibility of spurious merge conflicts for the 
copyright line during the review period.


>
> - src/jdk.security.jgss/share/classes/com/sun/security/jgss/GSSUtil.java
>
>   37     /**
>   38      * Do not call.
>   39      */
>   40     @Deprecated(since="16", forRemoval=true)
>   41     public GSSUtil() {}
>
> Is your concern that there may be some code out there that might be 
> erroneously using the default constructor so you want to give some 
> warning first before making this a private ctor after it is removed? 
> (I think I agree, but I want to make sure I understand your thinking).

That is my concern here, yes. We've had a number of other cases in the 
JDK where the default constructor appeared in an static-only class as an 
attractive nuisance. Before static imports, subclassing such a class was 
a way to get its names in the namespace of the subclass, but that is an 
anti-pattern that should not be encouraged.


>
> - 
> src/jdk.security.auth/share/classes/com/sun/security/auth/module/NTLoginModule.java
>
> 87      * Creates a {@code NTLoginModule}.
>
> s/a/an/

Hmm. If the this is pronounced as "N T Login Module", "N" would be a 
consonant sound so should be prefixed by the indefinite article "a", no?


>
> - 
> src/jdk.security.auth/share/classes/com/sun/security/auth/module/LdapLoginModule.java
>
> 359      * Creates a {@code LdapLoginModule}.
>
> s/a/an/

Okay.

Thanks,

-Joe