RFR: JDK-8291509 Minor cleanup could be done in sun.security [v4]

[Weijun Wang]

On Thu, 1 Sep 2022 02:26:48 GMT, Mark Powers <mpowers at openjdk.org> wrote:

>> https://bugs.openjdk.org/browse/JDK-8291509
>
> Mark Powers has updated the pull request incrementally with one additional commit since the last revision:
> 
>   comments from Sean and Max

This includes all in `sun.security.provider.certpath`.

BTW, sometimes some fields are final and some are not, is it better to put the final ones together?

src/java.base/share/classes/sun/security/provider/certpath/BasicChecker.java line 105:

> 103:             prevPubKey = trustedPubKey;
> 104:             if (PKIX.isDSAPublicKeyWithoutParams(prevPubKey)) {
> 105:                 // If TrustAnchor is a DSA public key, and it has no params, it

Not a native English speaker, but I found the sentence more fluent without the comma.

src/java.base/share/classes/sun/security/provider/certpath/BuildStep.java line 192:

> 190:         String resultString;
> 191:         switch (res) {
> 192:         case POSSIBLE:

`case` should be indented here. See http://cr.openjdk.java.net/~alundblad/styleguide/index-v6.html#toc-indentation.

That said, I think this is a good chance to use the switch expressions syntax, i.e `return switch (res)...`.

src/java.base/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java line 339:

> 337:                 idpExt.get
> 338:                     (IssuingDistributionPointExtension.INDIRECT_CRL).equals
> 339:                         (Boolean.FALSE)) {

There are only 2 `Boolean` values, so it's safe to use `==`. Also, I'd like

idpExt.get(IssuingDistributionPointExtension.INDIRECT_CRL)
        == Boolean.FALSE

src/java.base/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java line 572:

> 570:             if (interimReasonsMask[i] &&
> 571:                     !(i < reasonsMask.length && reasonsMask[i]))
> 572:             {

More brace to previous line.

src/java.base/share/classes/sun/security/provider/certpath/ForwardBuilder.java line 544:

> 542:                     if (distanceTto1 == distanceTto2) {
> 543:                         return -1;
> 544:                     } else if (distanceTto1 > 0 && distanceTto2 <= 0) {

I feel there is no need for `distanceTto1 > 0 &&` in the above line.

src/java.base/share/classes/sun/security/provider/certpath/OCSPResponse.java line 703:

> 701:      *
> 702:      * @return the {@code ResponderId} from this response or {@code null}
> 703:      *      if no responder ID is in the body of the response e.g. a

I'd rather add a closed parenthesis at the end, or at least add a comma before `e.g.`

src/java.base/share/classes/sun/security/provider/certpath/PolicyNodeImpl.java line 95:

> 93:         mChildren = new HashSet<>();
> 94: 
> 95:         mValidPolicy = Objects.requireNonNullElse(validPolicy, "");

I prefer `?:`.

src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java line 479:

> 477: 
> 478:         switch (type) {
> 479:         case "LDAP":

`case` lines should be indented.

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

PR: https://git.openjdk.org/jdk/pull/9972