Re: RFR: 8332103: Add missing `@since` tags to `java.desktop`
On Wed, 29 May 2024 11:38:02 GMT, Alexey Ivanov wrote: > Why is it? There's history beyond 10 and 9 As I've explained before, a program relying on the historical data built into `javac` can only give accurate reports for newer releases. As that data only goes back so far. I was left to investigate and couldn't find source code of JDK 1-2 internally after some digging. To fix old code, we would need to find a new way to map elements. I don't think this would need to be added to the checker tool as it doesn't need to run regularly. It can be a one time script, fix it once and be done with this. It's out of scope for my current work. The good thing is that once these tags (in this PR) are fixed, this will no longer be an issue and the errors reported will be clear and simple. As we would only be dealing with missing/wrong information in current releases. I agree I should've looked more, because the `@since` values deserve to be correct. - PR Comment: https://git.openjdk.org/jdk/pull/19192#issuecomment-2138307870
Re: RFR: 8332103: Add missing `@since` tags to `java.desktop` [v2]
On Tue, 14 May 2024 23:36:13 GMT, Nizar Benalla wrote:
>> src/java.desktop/share/classes/javax/swing/plaf/basic/BasicSliderUI.java
>> line 154:
>>
>>> 152: * Constructs a {@code BasicSliderUI}.
>>> 153: *
>>> 154: * @since 16
>>
>> Hmm, the *explicit* default constructor was added in JDK 16, but it was
>> implicit before then.
>> So I am not 100% sure what the right answer is - the same as the class, or
>> when it was explicitly added.
>
> When mapping methods and when they first appeared (by using the historical
> record built into javac) I use an id in the form of
> `method:
> .()` so for
> covariant overrides in general, when the return type changes I consider it to
> be a new method.
>
> Looking at the contents of the dictionnary:
> This explicit constructor existed for a long time but then this new was added
> a new one was added in JDK 16
> | Key | Value |
> | - | - |
> | `method: void
> javax.swing.plaf.basic.BasicSliderUI.(javax.swing.JSlider):` | 9 |
> | `method: void javax.swing.plaf.basic.BasicSliderUI.():` | 16 |
>
> Note: JDK 9 is used as the "base" as that's how far I can reliably use the
> `--release` info, so if something was added in JDK 2,5.7,9. It has a value of
> "9" in the dictionnary. I mainly check for errors in newer code.
> Hmm, the _explicit_ default constructor was added in JDK 16, but it was
> implicit before then. So I am not 100% sure what the right answer is - the
> same as the class, or when it was explicitly added.
I believe there was no default constructor in `BasicSliderUI()` because there
was a constructor with a parameter `BasicSliderUI(JSlider b)`.
Thus, this case seem to be correct `BasicSliderUI()` is available since 16.
At the same time, `BasicSliderUI(JSlider b)` has existed since at least 7, the
constructor is present in the history of the file. The history in GitHub goes
up to 1st December 2007 which corresponds to Java 7 timeline. I'm pretty sure
this constructor existed in previous releases, and you have to dig further to
find when it was added.
Very much likely, the constructor `BasicSliderUI(JSlider b)` was added when the
`BasicSliderUI` class was added. The class does not have `@since` tag, so it's
inherited from the package, isn't it? The same rule applies to the constructor,
doesn't it?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19192#discussion_r1618748176
Re: RFR: 8332103: Add missing `@since` tags to `java.desktop`
On Tue, 14 May 2024 23:45:23 GMT, Nizar Benalla wrote: > but for older code you can only guess "Element: X existed before JDK 10". So > I was left to check on my own, and made a mistake. Why is it? There's history beyond 10 and 9, yet accessing it requires more effort. In addition to that, old JDK releases are available in an archive so that you can verify if a method existed in a certain release. If all the methods need `@since` marker, it has to be accurate and represent the real picture rather than an estimate. - PR Comment: https://git.openjdk.org/jdk/pull/19192#issuecomment-2137200506
Re: RFR: 8332103: Add missing `@since` tags to `java.desktop` [v2]
> If you're currently reviewing this PR, thank you! > Most fixes here are according to the reports by the since checker tool in > #18934 and are pretty simple. > > To make reviewing easier > - `BasicSliderUI` has the constructor `public BasicSliderUI(JSlider b)` for a > long time so the default constructor (without parameters) didn't exist until > JDK 16 > > For the `package-info` files, it is pretty hard to find source code of JDK > 1-5 so I used the `grep` command to find the oldest instance of an `@since` > in those packages. > > I found instances of `@since 1.1` in the other packages but > `javax/swing/plaf/synth/package-info.java` might be worth checking as most > classes there had no `@since`. Nizar Benalla has updated the pull request incrementally with one additional commit since the last revision: Swing was added in JDK 1.2 - Changes: - all: https://git.openjdk.org/jdk/pull/19192/files - new: https://git.openjdk.org/jdk/pull/19192/files/dec1c1b5..7e8244db Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19192=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19192=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19192.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19192/head:pull/19192 PR: https://git.openjdk.org/jdk/pull/19192
Re: RFR: 8332103: Add missing `@since` tags to `java.desktop`
On Sat, 11 May 2024 17:52:28 GMT, Nizar Benalla wrote: > If you're currently reviewing this PR, thank you! > Most fixes here are according to the reports by the since checker tool in > #18934 and are pretty simple. > > To make reviewing easier > - `BasicSliderUI` has the constructor `public BasicSliderUI(JSlider b)` for a > long time so the default constructor (without parameters) didn't exist until > JDK 16 > > For the `package-info` files, it is pretty hard to find source code of JDK > 1-5 so I used the `grep` command to find the oldest instance of an `@since` > in those packages. > > I found instances of `@since 1.1` in the other packages but > `javax/swing/plaf/synth/package-info.java` might be worth checking as most > classes there had no `@since`. As I'm using the historical data built into `javac` to determine the correct `@since` tag to be used, I can only check code added after JDK 9. So a lot of errors in "older" code will go unnoticed, but I can make sure new tags are correct. When an `@since` is missing from an element added in newer JDK releases I get an accurate error message i.e > method: void > javax.swing.JSlider.AccessibleJSlider.stateChanged(javax.swing.event.ChangeEvent): > `@since` version is 9 instead of 16 but for older code you can only guess "Element: X existed before JDK 10" - PR Comment: https://git.openjdk.org/jdk/pull/19192#issuecomment-2111335875
Re: RFR: 8332103: Add missing `@since` tags to `java.desktop`
On Tue, 14 May 2024 21:53:24 GMT, Phil Race wrote:
>> If you're currently reviewing this PR, thank you!
>> Most fixes here are according to the reports by the since checker tool in
>> #18934 and are pretty simple.
>>
>> To make reviewing easier
>> - `BasicSliderUI` has the constructor `public BasicSliderUI(JSlider b)` for
>> a long time so the default constructor (without parameters) didn't exist
>> until JDK 16
>>
>> For the `package-info` files, it is pretty hard to find source code of JDK
>> 1-5 so I used the `grep` command to find the oldest instance of an `@since`
>> in those packages.
>>
>> I found instances of `@since 1.1` in the other packages but
>> `javax/swing/plaf/synth/package-info.java` might be worth checking as most
>> classes there had no `@since`.
>
> src/java.desktop/share/classes/javax/swing/package-info.java line 153:
>
>> 151: * @serial exclude
>> 152: *
>> 153: * @since 1.1
>
> This isn't right. Where did you get this from ?
> Swing only became part of the JDK in JDK 1.2
> It was an unbundled library before then.
> If you find any @since 1.1 tags in the Swing API they are a mistake.
My bad then, this is a mistake.
> src/java.desktop/share/classes/javax/swing/plaf/basic/BasicSliderUI.java line
> 154:
>
>> 152: * Constructs a {@code BasicSliderUI}.
>> 153: *
>> 154: * @since 16
>
> Hmm, the *explicit* default constructor was added in JDK 16, but it was
> implicit before then.
> So I am not 100% sure what the right answer is - the same as the class, or
> when it was explicitly added.
When mapping methods and when they first appeared (by using the historical
record built into javac) I use an id in the form of
`method:
.()` so for
covariant overrides in general, when the return type changes I consider it to
be a new method.
Looking at the contents of the dictionnary:
This explicit constructor existed for a long time but then this new was added a
new one was added in JDK 16
| Key | Value |
| - | - |
| `method: void
javax.swing.plaf.basic.BasicSliderUI.(javax.swing.JSlider):` | 9 |
| `method: void javax.swing.plaf.basic.BasicSliderUI.():` | 16 |
Note: JDK 9 is used as the "base" as that's how far I can reliably use the
`--release` info, so if something was added in JDK 2,5.7,9. It has a value of
"9" in the dictionnary. I mainly check for errors in newer code.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19192#discussion_r1600782386
PR Review Comment: https://git.openjdk.org/jdk/pull/19192#discussion_r1600780531
Re: RFR: 8332103: Add missing `@since` tags to `java.desktop`
On Sat, 11 May 2024 17:52:28 GMT, Nizar Benalla wrote:
> If you're currently reviewing this PR, thank you!
> Most fixes here are according to the reports by the since checker tool in
> #18934 and are pretty simple.
>
> To make reviewing easier
> - `BasicSliderUI` has the constructor `public BasicSliderUI(JSlider b)` for a
> long time so the default constructor (without parameters) didn't exist until
> JDK 16
>
> For the `package-info` files, it is pretty hard to find source code of JDK
> 1-5 so I used the `grep` command to find the oldest instance of an `@since`
> in those packages.
>
> I found instances of `@since 1.1` in the other packages but
> `javax/swing/plaf/synth/package-info.java` might be worth checking as most
> classes there had no `@since`.
I'm not sure I understand the methodology here.
I think there must be hundreds of similar missing tags and this seems to be
just a few random ones that aren't always correct.
src/java.desktop/share/classes/javax/swing/plaf/basic/BasicSliderUI.java line
154:
> 152: * Constructs a {@code BasicSliderUI}.
> 153: *
> 154: * @since 16
Hmm, the *explicit* default constructor was added in JDK 16, but it was
implicit before then.
So I am not 100% sure what the right answer is - the same as the class, or when
it was explicitly added.
-
PR Comment: https://git.openjdk.org/jdk/pull/19192#issuecomment-2111206725
PR Review Comment: https://git.openjdk.org/jdk/pull/19192#discussion_r1600699721
Re: RFR: 8332103: Add missing `@since` tags to `java.desktop`
On Sat, 11 May 2024 17:52:28 GMT, Nizar Benalla wrote: > If you're currently reviewing this PR, thank you! > Most fixes here are according to the reports by the since checker tool in > #18934 and are pretty simple. > > To make reviewing easier > - `BasicSliderUI` has the constructor `public BasicSliderUI(JSlider b)` for a > long time so the default constructor (without parameters) didn't exist until > JDK 16 > > For the `package-info` files, it is pretty hard to find source code of JDK > 1-5 so I used the `grep` command to find the oldest instance of an `@since` > in those packages. > > I found instances of `@since 1.1` in the other packages but > `javax/swing/plaf/synth/package-info.java` might be worth checking as most > classes there had no `@since`. src/java.desktop/share/classes/javax/swing/package-info.java line 153: > 151: * @serial exclude > 152: * > 153: * @since 1.1 This isn't right. Where did you get this from ? Swing only became part of the JDK in JDK 1.2 It was an unbundled library before then. If you find any @since 1.1 tags in the Swing API they are a mistake. - PR Review Comment: https://git.openjdk.org/jdk/pull/19192#discussion_r1600695814
Re: RFR: 8332103: Add missing `@since` tags to `java.desktop`
On Sat, 11 May 2024 17:52:28 GMT, Nizar Benalla wrote: > If you're currently reviewing this PR, thank you! > Most fixes here are according to the reports by the since checker tool in > #18934 and are pretty simple. > > To make reviewing easier > - `BasicSliderUI` has the constructor `public BasicSliderUI(JSlider b)` for a > long time so the default constructor (without parameters) didn't exist until > JDK 16 > > For the `package-info` files, it is pretty hard to find source code of JDK > 1-5 so I used the `grep` command to find the oldest instance of an `@since` > in those packages. > > I found instances of `@since 1.1` in the other packages but > `javax/swing/plaf/synth/package-info.java` might be worth checking as most > classes there had no `@since`. I did verify the updates against the release versions and looks good to me. - Marked as reviewed by tr (Committer). PR Review: https://git.openjdk.org/jdk/pull/19192#pullrequestreview-2054975318
