Re: JDK 16 RFR of JDK-8250246: Address reliance on default constructors in security libs
On 7/24/2020 10:32 AM, Sean Mullan wrote:
On 7/24/20 1:18 PM, Joe Darcy wrote:
-
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.
Ok, sounds good.
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?
This might be a question for one of the grammar experts amongst us.
But isn't "N" a vowel sound: "en"?
I suppose looked at that way it is :-)
I'll make the adjustments, copyrights, etc. and push this later today.
Thanks,
-Joe
Re: JDK 16 RFR of JDK-8250246: Address reliance on default constructors in security libs
On 7/24/20 1:18 PM, Joe Darcy wrote:
- 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.
Ok, sounds good.
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?
This might be a question for one of the grammar experts amongst us. But
isn't "N" a vowel sound: "en"?
--Sean
src/jdk.security.auth/share/classes/com/sun/security/auth/module/LdapLoginModule.java
359 * Creates a {@code LdapLoginModule}.
s/a/an/
Okay.
Thanks,
-Joe
Re: JDK 16 RFR of JDK-8250246: Address reliance on default constructors in security libs
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
Re: JDK 16 RFR of JDK-8250246: Address reliance on default constructors in security libs
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.
- 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).
-
src/jdk.security.auth/share/classes/com/sun/security/auth/module/NTLoginModule.java
87 * Creates a {@code NTLoginModule}.
s/a/an/
-
src/jdk.security.auth/share/classes/com/sun/security/auth/module/LdapLoginModule.java
359 * Creates a {@code LdapLoginModule}.
s/a/an/
--Sean
