On Wed, 3 Jan 2024 12:09:42 GMT, Pavel Rappo <pra...@openjdk.org> wrote:

> Please review this PR to update `blessed-modifier.order.sh` for the 
> `default`, `sealed`, `non-sealed`, and `strictfp` modifiers.
> 
> In a [discussion][] preceding this PR, it was agreed that the script should 
> better refer to relevant JLS sections rather than to 
> [`java.lang.reflect.Modifier#toString`][], which does not model Java source.
> 
> - the `sealed` and `non-sealed` class or interface modifiers were introduced 
> in JEP 409, but have never been accounted for
> 
> - the `default` method modifier was introduced in Java 8, but has not been 
> explicitly accounted for. It's unclear whether it was an omission or a 
> conscious decision. The current edition of JLS [suggests] that in compilable, 
> modern and customary formatted code, `default` appears alone and, thus, is 
> always canonically ordered.
> 
>   However, a somewhat similar argument applies to the `public` modifier on an 
> interface method. Its use is discouraged, yet the script would enforce its 
> order if `public` appears not alone.
> 
>   So for completeness, this PR proposes to explicitly account for `default`.
> 
> * While not proposing to do anyting about it, this PR recognises (for the 
> record) that `strictfp` class, interface, or method modifier became 
> [obsolete][] in JDK 17.
> 
> I've run the updated script on JDK. The script found 8 missorted `sealed` and 
> 20 missorted `default`. The script also found 3 false positive `default`:
> 
> - 
> https://github.com/openjdk/jdk/blob/249d553659ab75a2271e98c77e7d62f662ffa684/src/java.desktop/share/classes/java/awt/dnd/DragSource.java#L526-L527
> - 
> https://github.com/openjdk/jdk/blob/98da03af50e2372817a7b5e381eea5ee6f2cb919/src/java.management/share/classes/javax/management/MBeanServerFactory.java#L91
> - 
> https://github.com/openjdk/jdk/blob/6765f902505fbdd02f25b599f942437cd805cad1/src/java.desktop/share/classes/javax/print/PrintServiceLookup.java#L193
> 
> None of those findings are addressed in this PR.
> 
> [discussion]: 
> https://mail.openjdk.org/pipermail/core-libs-dev/2024-January/117347.html
> [`java.lang.reflect.Modifier#toString`]: 
> https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/reflect/Modifier.html#toString(int)
> [suggests]: 
> https://docs.oracle.com/javase/specs/jls/se21/html/jls-9.html#jls-9.4
> [obsolete]: https://openjdk.org/jeps/306

bin/blessed-modifier-order.sh line 34:

> 32:     echo
> 33:     echo "See:"
> 34:     echo 
> "https://docs.oracle.com/javase/8/docs/api/java/lang/reflect/Modifier.html#toString-int-";

Looking at the latest docs for Modifier#toString()
it also needs a corresponding update
https://download.java.net/java/early_access/jdk23/docs/api/java.base/java/lang/reflect/Modifier.html#toString(int)

and if that is done, then we can keep referring to its javadoc.

References to javadoc and jls both have problems of becoming stale over time. 
There is no way to reference the "latest" spec via a URL.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17242#discussion_r1440697074

Reply via email to