[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: RFR: 8319938: com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java fails
From:       Alexey Ivanov <aivanov () openjdk ! org>
Date:       2023-11-23 20:59:04
Message-ID: 50RQfFptQOxqTU70iCJQY0kJ_WVdzZ6o82P9Y3FaxAs=.d11f8a70-77b3-4e4d-a136-17921c9f29d2 () github ! com
[Download RAW message or body]

On Wed, 22 Nov 2023 13:50:54 GMT, Alexey Ivanov <aivanov@openjdk.org> wrote:

> > > And whilst import java.io sorts after java.awt, it is long standing convention \
> > > that the "core" packages (easily distinguished these days as those in the \
> > > java.base module) are listed before the desktop / AWT / Swing ones. This is \
> > > true of product source as well as tests.
> > 
> > @prrace This is the first time I hear this. In all the classes I've seen in AWT \
> > and Swing, the imports are sorted alphabetically with `java.io` after `java.awt`. \
> >  I just looked at \
> > [`JComponent`](https://github.com/openjdk/jdk/blob/5e818318eac8cda7d42b599dc7d7d44e5c299a9f/src/java.desktop/share/classes/javax/swing/JComponent.java#L54-L61), \
> > [`Component`](https://github.com/openjdk/jdk/blob/5e818318eac8cda7d42b599dc7d7d44e5c299a9f/src/java.desktop/share/classes/java/awt/Component.java#L61-L65), \
> > [Font](https://github.com/openjdk/jdk/blob/6ce0ebb858d3112f136e12d3ad595f805f6871a0/src/java.desktop/share/classes/java/awt/Font.java#L32-L38), \
> > and 
> > https://github.com/openjdk/jdk/blob/6ce0ebb858d3112f136e12d3ad595f805f6871a0/src/java.desktop/share/classes/sun/font/FontManager.java#L28-L30
> >  
> > https://github.com/openjdk/jdk/blob/6ce0ebb858d3112f136e12d3ad595f805f6871a0/src/java.desktop/share/classes/sun/font/Font2D.java#L28-L36
> >  
> > [Sun/Oracle Code Conventions for Java Programming \
> > Language](https://www.oracle.com/java/technologies/javase/codeconventions-fileorganization.html#277) \
> > say nothing about the order of imports. (The HTML version has absolutely broken \
> > formatting now.) [Google Java Style \
> > Guide](https://google.github.io/styleguide/javaguide.html#s3.3-import-statements) \
> > states, <q cite="https://google.github.io/styleguide/javaguide.html#s3.3-import-statements">…the \
> > imported names appear in ASCII sort order.</q> 
> > Andreas Lundblad's draft for [Java Style \
> > Guidelines](https://cr.openjdk.org/~alundblad/styleguide/index-v6.html#toc-import-statements), \
> > which is more or less followed by the OpenJDK project, defines the following \
> > order: 
> > > Import statements should be sorted…
> > > 
> > > *  …primarily by non-static / static with non-static imports first.
> > > * …secondarily by package origin according to the following order
> > > * `java` packages
> > > * `javax` packages
> > > * external packages (e.g. `org.xml`)
> > > * internal packages (e.g. `com.sun`)
> > > * …tertiary by package and class identifier in lexicographical order
> > 
> > [Andreas](https://github.com/aioobe) opened [PR \
> > 14](https://github.com/openjdk/guide/pull/14) to incorporate it into OpenJDK \
> > Developers' Guide, but there are objecti...
> 
> > I don't know why it was necessary to move all around all the above lines.
> 
> @prrace I nearly always ask to move the jtreg tags to the class declaration, \
> especially for new tests. When you open the test file in IDE, the copyright block \
> above imports is collapsed, so is the jtreg tags if they're placed above the \
> imports. 
> If the jtreg tags are placed in a comment that precedes the class declaration, \
> after the imports, they're not collapsed — you can see them right away without \
> scrolling or clicking. I consider the jtreg tags quite relevant to see them easily. \
>  Most older tests place the jtreg tags above the imports; it could've been the \
> requirement or limitation of jtreg at that time. It's not a limitation any more. 
> As for moving the jtreg tags in existing tests when the test is modified, I also do \
> it occasionally for the reasons outlined above: the jtreg tags describe the test \
> and belong to the class declaration, akin a javadoc comment. (Yet I am against \
> using the javadoc syntax, starting with two asterisks, for the jtreg tasks because \
> it is not a javadoc and because it removes the IDE warnings for the unknown javadoc \
> tasks.)

> > And whilst import java.io sorts after java.awt, it is long standing convention \
> > that the "core" packages (easily distinguished these days as those in the \
> > java.base module) are listed before the desktop / AWT / Swing ones. This is true \
> > of product source as well as tests.
> 
> @prrace This is the first time I hear this. In all the classes I've seen in AWT and \
> Swing, the imports are sorted alphabetically with `java.io` after `java.awt`.

Today I've stumbled upon a few files where `java.io` and `java.util` come before \
`java.awt`, such as [`DialogOwnerTest.java`](https://github.com/openjdk/jdk/blob/9131a \
8f5f241b04c28a875fddb7a060cc9a3c252/test/jdk/java/awt/print/Dialog/DialogOwnerTest.java#L30-L32), \
[`CustomFont.java`](https://github.com/openjdk/jdk/blob/4a4fbbaaa97e22a8ec6dcc5d72a1ed \
43bc0b691e/test/jdk/java/awt/print/PrinterJob/CustomFont/CustomFont.java#L31-L32), \
[`DeviceScale.java`](https://github.com/openjdk/jdk/blob/7cb9e5821e0e5bcc483bc7c587ea921e9b515e77/test/jdk/java/awt/print/PrinterJob/DeviceScale.java#L31-L33),


https://github.com/openjdk/jdk/blob/3789983e89c9de252ef546a1b98a732a7d066650/test/jdk/java/awt/print/PrinterJob/DrawImage.java#L32-L36


It's a handful of files out of more than a hundred modified files in #16785; it \
doesn't look as a convention. Do wildcard imports even count?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16674#discussion_r1403718344


[prev in list] [next in list] [prev in thread] [next in thread]