Re: RFR: 8161255, jdk build "all" (docs) fails on all platforms, error from DefaultLoggerFinder.java
Hi Bhavesh, This looks fine to get the build going again; please push this right away. As we discussed off-list, there will probably need to be some additional javadoc mechanisms so that checking for this kind of implementation detail doesn't run afoul of doclint unnecessarily. Thanks, -Joe On 7/13/2016 2:02 PM, Bhavesh Patel wrote: Hi, This is the fix for the issue in which the JDK API documentation build fails due to comment in DefaultLoggerFinder.java that points to a type in an unexported (internal) API. Doclint reports this is an error during the reference check and the documentation build fails. The makefile for javadoc needs to be updated to disable the reference check in the jdk.internal.logger package. JBS: https://bugs.openjdk.java.net/browse/JDK-8161255 Webrev: http://cr.openjdk.java.net/~bpatel/8161255/webrev/ Please review this change. Thanks, Bhavesh.
Re: RFR: JDK-8217214: Recent new javadoc test needs to be updated
+1
-Joe
On 1/15/2019 3:02 PM, Jonathan Gibbons wrote:
Please review this trivial change to reconcile a bad merge.
One changeset added a new test which depends on the JavadocTester
library; a different changeset moved the library. The test needs to
be updated for the new location of the library in its new package.
There is no other change in functionality.
Change is inline, below.
JBS: https://bugs.openjdk.java.net/browse/JDK-8217214
-- Jon
$ hg diff
diff -r 142b179dd60e
test/langtools/jdk/javadoc/doclet/testLinkOption/TestLinkOptionWithAutomaticModule.java
---
a/test/langtools/jdk/javadoc/doclet/testLinkOption/TestLinkOptionWithAutomaticModule.java
Tue Jan 15 19:24:07 2019 -0300
+++
b/test/langtools/jdk/javadoc/doclet/testLinkOption/TestLinkOptionWithAutomaticModule.java
Tue Jan 15 14:56:25 2019 -0800
@@ -25,12 +25,12 @@
* @test
* @bug 8212233
* @summary The code being documented uses modules but the packages
defined in $URL are in the unnamed module.
- * @library /tools/lib ../lib
+ * @library /tools/lib ../../lib
* @modules
* jdk.javadoc/jdk.javadoc.internal.tool
* jdk.compiler/com.sun.tools.javac.api
* jdk.compiler/com.sun.tools.javac.main
- * @build JavadocTester toolbox.JarTask toolbox.JavacTask
toolbox.ModuleBuilder toolbox.ToolBox
+ * @build javadoc.tester.* toolbox.JarTask toolbox.JavacTask
toolbox.ModuleBuilder toolbox.ToolBox
* @run main TestLinkOptionWithAutomaticModule
*/
@@ -43,6 +43,8 @@
import toolbox.ModuleBuilder;
import toolbox.ToolBox;
+import javadoc.tester.JavadocTester;
+
public class TestLinkOptionWithAutomaticModule extends JavadocTester {
public static void main(String... args) throws Exception {
Re: RFR [15] 8242230: Whitespace typos, relaxed javadoc, formatting
Hi Pavel,
Looks fine in general, assuming the change to Class.java renders
correctly in output.
Thanks,
-Joe
On 4/7/2020 8:28 AM, Pavel Rappo wrote:
Hi Ivan,
On 7 Apr 2020, at 09:11, Ivan Gerasimov wrote:
Hi Pavel!
A couple of comments.
1)
java/util/logging/Formatter.java
This has one extra open curly brace:
"{{@literal }"
The leftmost curly brace is not a part of the "literal" inline tag, but rather
a part of the message format string. Have a look at the method that the comment
belongs to. Note what that method is looking for in a message string.
2)
grep finds some more typos of the same kind that you've spotted.
a) rgrep '^[ ]*\*'|grep ' ,'|less
This find number of potential typos. For example, the javadoc for VarHandle
[1] has 53 occurrances of space-before-comma. A few more are found in
j.l.Thread, j.io.DataOutput, j.l.String, etc.
b) rgrep '^[ ]*\*'|grep '\w- '|less
This find the word 'network' broken with a hyphen at [2] and also in
share/classes/sun/net/util/IPAddressUtil.java
(I only searched under src/java.base)
Thanks. I've included some of those: true positive, non-controversial,
and significant cases where I believe I sufficiently understood context.
For instance, I was not sure if I reliably grasped the applicability of the
"statically represented using varargs" phrase used widely across the VarHandle
type. It looks like that phrase was blankedly added to @return and @return tags.
So, I left it out for now.
The updated patch can be found here:
http://cr.openjdk.java.net/~prappo/8242230/webrev.01/
***
Some of the cases this patch addresses suggest that we might need to do
something about how the Standard Doclet treats newline characters in source
files. More often than not, newline characters are purely to control the width
of the source lines. When carried over to the output, they may produce
undesirable effects. Punctuation marks and contents of the Standard Doclet tags
may be affected.
That problem [1] is neither new nor trivial to address. Still, we should add
something to the Standard Doclet Specification [2].
I'm not sure what we can do about it now other than work around the current
behavior where it is significant.
-Pavel
P.S. CC'ing to javadoc-dev@openjdk.java.net
---
[1] https://en.wikipedia.org/wiki/Line_wrap_and_word_wrap
[2]
https://docs.oracle.com/en/java/javase/14/docs/specs/javadoc/doc-comment-spec.html
With kind regards,
Ivan
[1]
https://docs.oracle.com/en/java/javase/14/docs/api/java.base/java/lang/invoke/VarHandle.html
[2]
https://docs.oracle.com/en/java/javase/14/docs/api/java.base/java/net/Inet4Address.html
On 4/6/20 11:28 AM, Pavel Rappo wrote:
Hello,
Please review the change for https://bugs.openjdk.java.net/browse/JDK-8242230:
http://cr.openjdk.java.net/~prappo/8242230/webrev.00/
This is a documentation cleanup. There are no code changes involved,
and the changes in documentation are mostly trivial.
The following packages are affected:
java.lang,
java.text,
java.util,
java.util.logging,
javax.lang.model.util,
jdk.internal.reflect
-Pavel
--
With kind regards,
Ivan Gerasimov
Re: RFR [15] 8242230: Whitespace typos, relaxed javadoc, formatting
Hi Pavel,
On 4/7/2020 1:07 PM, Pavel Rappo wrote:
Thanks, Joe. I presume you mean me changing this
* @{@code interface}
to this
* {@code @interface}
Correct.
Right? If so, then it renders the same way, and the `@interface` does not
confuse the Standard Doclet. After JDK-8241780 "Allow \n@ inside inline tags"
has been done and dusted we'll never have to think about that ever again.
I didn't double-check the history, but I may have added
@{@code interface}
way back when and if note in this particular instance I recall that kind
of javadoc contortion was necessary at the time to get (almost) the
desired effect.
Thanks,
-Joe
On 7 Apr 2020, at 20:23, Joe Darcy wrote:
Hi Pavel,
Looks fine in general, assuming the change to Class.java renders correctly in
output.
Thanks,
-Joe
On 4/7/2020 8:28 AM, Pavel Rappo wrote:
Hi Ivan,
On 7 Apr 2020, at 09:11, Ivan Gerasimov wrote:
Hi Pavel!
A couple of comments.
1)
java/util/logging/Formatter.java
This has one extra open curly brace:
"{{@literal }"
The leftmost curly brace is not a part of the "literal" inline tag, but rather
a part of the message format string. Have a look at the method that the comment
belongs to. Note what that method is looking for in a message string.
2)
grep finds some more typos of the same kind that you've spotted.
a) rgrep '^[ ]*\*'|grep ' ,'|less
This find number of potential typos. For example, the javadoc for VarHandle
[1] has 53 occurrances of space-before-comma. A few more are found in
j.l.Thread, j.io.DataOutput, j.l.String, etc.
b) rgrep '^[ ]*\*'|grep '\w- '|less
This find the word 'network' broken with a hyphen at [2] and also in
share/classes/sun/net/util/IPAddressUtil.java
(I only searched under src/java.base)
Thanks. I've included some of those: true positive, non-controversial,
and significant cases where I believe I sufficiently understood context.
For instance, I was not sure if I reliably grasped the applicability of the
"statically represented using varargs" phrase used widely across the VarHandle
type. It looks like that phrase was blankedly added to @return and @return tags.
So, I left it out for now.
The updated patch can be found here:
http://cr.openjdk.java.net/~prappo/8242230/webrev.01/
***
Some of the cases this patch addresses suggest that we might need to do
something about how the Standard Doclet treats newline characters in source
files. More often than not, newline characters are purely to control the width
of the source lines. When carried over to the output, they may produce
undesirable effects. Punctuation marks and contents of the Standard Doclet tags
may be affected.
That problem [1] is neither new nor trivial to address. Still, we should add
something to the Standard Doclet Specification [2].
I'm not sure what we can do about it now other than work around the current
behavior where it is significant.
-Pavel
P.S. CC'ing to javadoc-dev@openjdk.java.net
---
[1] https://en.wikipedia.org/wiki/Line_wrap_and_word_wrap
[2]
https://docs.oracle.com/en/java/javase/14/docs/specs/javadoc/doc-comment-spec.html
With kind regards,
Ivan
[1]
https://docs.oracle.com/en/java/javase/14/docs/api/java.base/java/lang/invoke/VarHandle.html
[2]
https://docs.oracle.com/en/java/javase/14/docs/api/java.base/java/net/Inet4Address.html
On 4/6/20 11:28 AM, Pavel Rappo wrote:
Hello,
Please review the change for https://bugs.openjdk.java.net/browse/JDK-8242230:
http://cr.openjdk.java.net/~prappo/8242230/webrev.00/
This is a documentation cleanup. There are no code changes involved,
and the changes in documentation are mostly trivial.
The following packages are affected:
java.lang,
java.text,
java.util,
java.util.logging,
javax.lang.model.util,
jdk.internal.reflect
-Pavel
--
With kind regards,
Ivan Gerasimov
Re: RFR: [15, doc] JDK-8247382 : doclint errors (missing comments) in jdk.compiler and jdk.javadoc
Looks fine Jon; cheers, -Joe On 6/10/2020 9:07 PM, Jonathan Gibbons wrote: Please review a trivial doc-only change to fix 3 missing doc comments detected by doclint. The new comments do not add any semantically new or interesting information, so no CSR. For the javac case, a preferable solution would have been to remove the undocumented `throws Exception`, except that would be an incompatible signature change for anyone unlikely enough to be calling the method directly. For now, the `throws` is just documented, even though in practice the exception will never be thrown. -- Jon JBS: https://bugs.openjdk.java.net/browse/JDK-8247382 Webrev: http://cr.openjdk.java.net/~jjg/8247382/webrev.00/
Re: RFR: 8246774: Record Classes (final) implementation
On Mon, 21 Sep 2020 21:36:39 GMT, Vicente Romero wrote: >> Co-authored-by: Vicente Romero >> Co-authored-by: Harold Seigel >> Co-authored-by: Jonathan Gibbons >> Co-authored-by: Brian Goetz >> Co-authored-by: Maurizio Cimadamore >> Co-authored-by: Joe Darcy >> Co-authored-by: Chris Hegarty >> Co-authored-by: Jan Lahoda > > Please review the fix for [1]. The intention of this patch is to make records > final removing the need to > use --enable-preview in order to be able to include a record declaration in a > source or for the VM to execute code > compiled from record classes, Thanks > > [1] https://bugs.openjdk.java.net/browse/JDK-8246774 Hi Vicente, Please file a separate subtask for the javax.lang.model changes. This helps with the JSR 269 MR paperwork. Thanks, -Joe - PR: https://git.openjdk.java.net/jdk/pull/290
Re: RFR: JDK-8075778: Add javadoc tag to avoid duplication of return information in simple situations. [v3]
FYI, I had a good experience taking a trial run of this patch to update
the java.compiler APIs to use the new feature. I didn't find any issues;
a specdiff comparing with and without use of the new tag didn't have any
unexpected diffs. (There were cases where small wording differences
existed and were regularized in the patch.)
After this goes back, looking forward to pushing a fix for JDK-8256917:
Use combo @returns tag in java.compiler javadoc.
Cheers,
-Joe
On 11/20/2020 4:34 PM, Jonathan Gibbons wrote:
This change extends the functionality of the `@return` tag so that it can also
be used as an inline tag in the first sentence of a description.
The goal is to be able to simplify the following common pattern:
/**
* Returns the result. Optional additional text.
* @return the result
*/
int method() {
by
/**
* {@return the result} Optional additional text.
*/
int method() {
Note:
* The inline tag may only be used at the beginning of the description. A
warning will be given if it is used elsewhere.
* The expansion of the inline tag is `Returns " _content_ `.` where _content_
is the content of the tag.
* If there is no block `@return` tag, the standard doclet will look for an
inline tag at the beginning of the description
* The inline tag can be inherited into overriding methods as if it was provided
as a block tag.
Jonathan Gibbons has updated the pull request incrementally with one additional
commit since the last revision:
Update DocCommentParser to permit nested inline tags in specified cases:
@return
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/1355/files
- new: https://git.openjdk.java.net/jdk/pull/1355/files/89846ff1..87edfb0c
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1355&range=02
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1355&range=01-02
Stats: 88 lines in 3 files changed: 82 ins; 3 del; 3 mod
Patch: https://git.openjdk.java.net/jdk/pull/1355.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/1355/head:pull/1355
PR: https://git.openjdk.java.net/jdk/pull/1355
Re: RFR: 8257617: TestLinkPlatform fails with new Java source version
On Thu, 3 Dec 2020 06:52:49 GMT, Hannes Wallnöfer wrote: > This fixes the problem that some tests in TestLinkPlatform.java rely on a > static list of properties, causing them to fail when a new Java source > version is added. The solution is to create the properties file on the fly. > > I also replaced the custom documentation domain with "example.com" as > suggested in the original PR. Thanks for the fix. The test passes with this change when run in my in-development JDK 17 build. - Marked as reviewed by darcy (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1588
Re: RFR: JDK-8075778: Add javadoc tag to avoid duplication of return information in simple situations.
On Wed, 2 Dec 2020 15:39:56 GMT, Roger Riggs wrote: > > > There is lots of other duplication/repetition in most javadoc. I'd rather see > some kind of text macro that would allow a single definition of a string that > can be repeated. The source would be a bit less readable, but it would be > lower maintenance when the same phrase or sentence is repeated to make the > javadoc more locally complete and easier to read in isolation. Now many times > do you have to say "throws NullPointerException when the reference is null" > or similar assertion. IMO this is a case to avoid the perfect being the enemy of the good. There are many structural cases of repeated or nearly repeated return information in the first sentence and @return tag. Therefore, I think it is reasonable for don't-repeat-yourself purposes to have dedicated support for this usage pattern. Separately, I agree it would be helpful to have a more general facility to allow structured placement of repeated text blocks. - PR: https://git.openjdk.java.net/jdk/pull/1355
Re: RFR: JDK-8075778: Add javadoc tag to avoid duplication of return information in simple situations.
On Fri, 27 Nov 2020 16:33:27 GMT, Roger Riggs wrote:
>
>
> ```
> /**
> * {@return the result} Optional additional text.
> */
> ```
>
> The java source looks a bit odd/unusual because the "first sentence" does not
> appear to end with a period.
> Though it seems like a convenience to include the '.' in the expansion, the
> source might be clearer if it did not, as in:
>
> ```
> /**
> * {@return the result}. Optional additional text.
> */
> ```
>
> Similarly, prepending the "Returns", forces the verb, which is conventional
> but always the most appropriate word.
> Changing the expansion to be only the contents of @ return would allow more
> flexible use.
> It would put more responsibility on the developer to form the first sentence.
> (With "Returns"... and the "." outside the tag.
I've done a trial use of this feature in the java.compiler API. While initially
I found the lack of a period after "{@return ...}" odd, I quickly adapted to
this usage.
-
PR: https://git.openjdk.java.net/jdk/pull/1355
Re: RFR: JDK-8262269: javadoc test TestGeneratedClasses.java fails on Windows
On Wed, 24 Feb 2021 02:09:10 GMT, Jonathan Gibbons wrote: > Fix test to work on Windows. Marked as reviewed by darcy (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2702
Re: RFR: JDK-8262421: doclint warnings in jdk.compiler module
On Thu, 25 Feb 2021 21:37:01 GMT, Jonathan Gibbons wrote: > Please review this doc fix to provide a couple of missing `@param` tags Marked as reviewed by darcy (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2731
Re: RFR: 8267832: SimpleVisitors and Scanners in jdk.compiler should use @implSpec [v2]
On Fri, 28 May 2021 19:07:17 GMT, Jan Lahoda wrote: >> As noted in: >> https://bugs.openjdk.java.net/browse/JDK-8265981?focusedCommentId=14423316&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14423316 >> >> Methods in various utility visitor classes in jdk.compiler should use >> @implSpec to specify the implementation behavior. This patch tries to add >> the @implSpec tag to methods which already contain a text specifying the >> implementation, and adds new javadoc to the handful of methods that are >> missing it so far. >> >> The CSR is started for review here: >> https://bugs.openjdk.java.net/browse/JDK-8267838 > > Jan Lahoda has updated the pull request incrementally with one additional > commit since the last revision: > > Reordering @implSpec and example as suggested. Marked as reviewed by darcy (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4223
RFR: 8268299: jvms tag produces incorrect URL
The @jls and @jvms taglet share most of their functionality. A JLS URL looks like https://docs.oracle.com/javase/specs/jls/se16/html/**jls**-8.html#jls-8.1 and a JVMS URL looks like https://docs.oracle.com/javase/specs/jvms/se16/html/**jvms**-4.html#jvms-4.3.2 The current taglet incorrectly uses "jls" in from the chapter for both JLS and JVMS URLs. The patch corrects this to use "jvms" for JVMS URLs. - Commit messages: - 8268299: jvms tag produces incorrect URL Changes: https://git.openjdk.java.net/jdk/pull/4381/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4381&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8268299 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/4381.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4381/head:pull/4381 PR: https://git.openjdk.java.net/jdk/pull/4381
Integrated: 8268299: jvms tag produces incorrect URL
On Sun, 6 Jun 2021 22:03:46 GMT, Joe Darcy wrote: > The @jls and @jvms taglet share most of their functionality. A JLS URL > looks like > > https://docs.oracle.com/javase/specs/jls/se16/html/**jls**-8.html#jls-8.1 > > and a JVMS URL looks like > > > https://docs.oracle.com/javase/specs/jvms/se16/html/**jvms**-4.html#jvms-4.3.2 > > The current taglet incorrectly uses "jls" in from the chapter for both JLS > and JVMS URLs. The patch corrects this to use "jvms" for JVMS URLs. This pull request has now been integrated. Changeset: e663ba96 Author:Joe Darcy URL: https://git.openjdk.java.net/jdk/commit/e663ba961f25c83758815bbfce97a58d9560c7a2 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod 8268299: jvms tag produces incorrect URL Reviewed-by: iris, erikj, jjg - PR: https://git.openjdk.java.net/jdk/pull/4381
RFR: 8264866: Remove unneeded WorkArounds.isAutomaticModule
Simple cleanup as a follow-on to JDK-8264865. Clean langtools test run. - Commit messages: - 8264866: Remove unneeded WorkArounds.isAutomaticModule Changes: https://git.openjdk.java.net/jdk/pull/4417/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4417&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8264866 Stats: 13 lines in 2 files changed: 0 ins; 12 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4417.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4417/head:pull/4417 PR: https://git.openjdk.java.net/jdk/pull/4417
Integrated: 8264866: Remove unneeded WorkArounds.isAutomaticModule
On Tue, 8 Jun 2021 19:37:28 GMT, Joe Darcy wrote: > Simple cleanup as a follow-on to JDK-8264865. Clean langtools test run. This pull request has now been integrated. Changeset: 7a378165 Author: Joe Darcy URL: https://git.openjdk.java.net/jdk/commit/7a37816548b913494b9671df9469b159cc62ae73 Stats: 13 lines in 2 files changed: 0 ins; 12 del; 1 mod 8264866: Remove unneeded WorkArounds.isAutomaticModule Reviewed-by: jjg - PR: https://git.openjdk.java.net/jdk/pull/4417
RFR: 8271711: Remove WorkArounds.isSynthetic
Switch out logic in WorkArounds for a different expression implemented using javax.lang.model.Elements logic. Langtools regression test suite passes with the changes. - Commit messages: - 8271711: Remove WorkArounds.isSynthetic Changes: https://git.openjdk.java.net/jdk/pull/4967/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4967&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8271711 Stats: 5 lines in 2 files changed: 0 ins; 1 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/4967.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4967/head:pull/4967 PR: https://git.openjdk.java.net/jdk/pull/4967
Integrated: 8271711: Remove WorkArounds.isSynthetic
On Tue, 3 Aug 2021 04:40:33 GMT, Joe Darcy wrote: > Switch out logic in WorkArounds for a different expression implemented using > javax.lang.model.Elements logic. > > Langtools regression test suite passes with the changes. This pull request has now been integrated. Changeset: 6594d3a3 Author: Joe Darcy URL: https://git.openjdk.java.net/jdk/commit/6594d3a3ef175a71ea34c7698ab96537c761f022 Stats: 5 lines in 2 files changed: 0 ins; 1 del; 4 mod 8271711: Remove WorkArounds.isSynthetic Reviewed-by: jjg - PR: https://git.openjdk.java.net/jdk/pull/4967
Re: [jdk17] RFR: JDK-8270872: Final nroff manpage update for JDK 17
On Thu, 5 Aug 2021 19:20:50 GMT, Jonathan Gibbons wrote: > Please review a semi-automatic update of the nroff man pages from the > upstream files. Marked as reviewed by darcy (Reviewer). - PR: https://git.openjdk.java.net/jdk17/pull/303
Re: RFR: JDK-8271159: [REDO] JDK-8249634 doclint should report implicit constructor as missing javadoc comments
On Wed, 11 Aug 2021 17:38:49 GMT, Jonathan Gibbons wrote: > Please review a do-over of JDK-8249634, to report a missing doc comment when > an implicit/default constructor is used. > > The `src` code is the same as before. The previous version had missing test > files (now added), and had a test fail because an interaction with another > changeset that was pushed while the previous version was in review. The > solution for that is the use of `-Xdoclint:all,-missing` in > `TestDocTreeDiags.java`. Marked as reviewed by darcy (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5088
RFR: 8202056: Expand serial warning to check for bad overloads of serial-related methods and ineffectual fields
This is an initial PR for expanded lint warnings done under two bugs:
8202056: Expand serial warning to check for bad overloads of serial-related
methods and ineffectual fields
8160675: Issue lint warning for non-serializable non-transient instance fields
in serializable type
to get feedback on the general approach and test strategy before further
polishing the implementation.
The implementation initially started as an annotation processor I wrote several
years ago. The refined version being incorporated into Attr has been
refactored, had its checks expanded, and been partially ported to idiomatic
javac coding style rather than using the javax.lang.model API from annotation
processing.
Subsequent versions of this PR are expected to move the implementation closer
to idiomatic javac, in particular to use javac flags rather than
javax.lang.model.Modifier's. Additional resources keys will be defined for the
serialization-related fields and methods not having the expected modifiers,
types, etc. The resource keys for the existing checks related to
serialVersionUID and reused.
Please also review the corresponding CSRs:
https://bugs.openjdk.java.net/browse/JDK-8274335
https://bugs.openjdk.java.net/browse/JDK-8274336
Informative serialization-related warning messages must take into account
whether a class, interface, annotation, record, and enum is being analyzed.
Enum classes and record classes have special handling in serialization. This
implementation under review has been augmented with checks for interface types
recommended by Chris Hegarty in an attachment on 8202056.
The JDK build has the Xlint:serial check enabled. The build did not pass with
the augmented checks. For most modules, this PR contains the library changes
necessary for the build to pass. I will start separate PRs in those library
areas to get the needed SuppressWarning("serial") or other changes in place.
For one module, I temporarily disabled the Xlint:serial check.
In terms of performance, I have not done benchmarks of the JDK build with and
without these changes, but informally the build seems to take about as long as
before.
-
Commit messages:
- Appease jcheck
- Implement checks chegar recommended for interfaces.
- Update comment.
- Add tests for instance field checks.
- Clean build with instance field checks in place.
- Merge branch 'master' into JDK-8202056
- Put Externalizable checks last.
- Add checks for constructor access in Serializable classes.
- Add no-arg ctor check for Externalizable classes.
- Improve Externalization warnings.
- ... and 19 more: https://git.openjdk.java.net/jdk/compare/5756385c...d498ff5f
Changes: https://git.openjdk.java.net/jdk/pull/5709/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5709&range=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8202056
Stats: 1519 lines in 79 files changed: 1511 ins; 1 del; 7 mod
Patch: https://git.openjdk.java.net/jdk/pull/5709.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/5709/head:pull/5709
PR: https://git.openjdk.java.net/jdk/pull/5709
Re: RFR: 8202056: Expand serial warning to check for bad overloads of serial-related methods and ineffectual fields [v2]
> This is an initial PR for expanded lint warnings done under two bugs:
>
> 8202056: Expand serial warning to check for bad overloads of serial-related
> methods and ineffectual fields
> 8160675: Issue lint warning for non-serializable non-transient instance
> fields in serializable type
>
> to get feedback on the general approach and test strategy before further
> polishing the implementation.
>
> The implementation initially started as an annotation processor I wrote
> several years ago. The refined version being incorporated into Attr has been
> refactored, had its checks expanded, and been partially ported to idiomatic
> javac coding style rather than using the javax.lang.model API from annotation
> processing.
>
> Subsequent versions of this PR are expected to move the implementation closer
> to idiomatic javac, in particular to use javac flags rather than
> javax.lang.model.Modifier's. Additional resources keys will be defined for
> the serialization-related fields and methods not having the expected
> modifiers, types, etc. The resource keys for the existing checks related to
> serialVersionUID and reused.
>
> Please also review the corresponding CSRs:
>
> https://bugs.openjdk.java.net/browse/JDK-8274335
> https://bugs.openjdk.java.net/browse/JDK-8274336
>
> Informative serialization-related warning messages must take into account
> whether a class, interface, annotation, record, and enum is being analyzed.
> Enum classes and record classes have special handling in serialization. This
> implementation under review has been augmented with checks for interface
> types recommended by Chris Hegarty in an attachment on 8202056.
>
> The JDK build has the Xlint:serial check enabled. The build did not pass with
> the augmented checks. For most modules, this PR contains the library changes
> necessary for the build to pass. I will start separate PRs in those library
> areas to get the needed SuppressWarning("serial") or other changes in place.
> For one module, I temporarily disabled the Xlint:serial check.
>
> In terms of performance, I have not done benchmarks of the JDK build with and
> without these changes, but informally the build seems to take about as long
> as before.
Joe Darcy has updated the pull request with a new target base due to a merge or
a rebase. The incremental webrev excludes the unrelated changes brought in by
the merge/rebase. The pull request contains 30 additional commits since the
last revision:
- Merge branch 'master' into JDK-8202056
- Appease jcheck
- Implement checks chegar recommended for interfaces.
- Update comment.
- Add tests for instance field checks.
- Clean build with instance field checks in place.
- Merge branch 'master' into JDK-8202056
- Put Externalizable checks last.
- Add checks for constructor access in Serializable classes.
- Add no-arg ctor check for Externalizable classes.
- ... and 20 more: https://git.openjdk.java.net/jdk/compare/fa1a96de...053de6bb
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/5709/files
- new: https://git.openjdk.java.net/jdk/pull/5709/files/d498ff5f..053de6bb
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5709&range=01
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5709&range=00-01
Stats: 469 lines in 32 files changed: 252 ins; 70 del; 147 mod
Patch: https://git.openjdk.java.net/jdk/pull/5709.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/5709/head:pull/5709
PR: https://git.openjdk.java.net/jdk/pull/5709
RFR: 8274405: Suppress warnings on non-serializable non-transient instance fields in javac and javadoc
Augmentations to javac's Xlint:serial checking are out for review (#5709) and various javac and javadoc implementation libraries would need some changes to pass under the expanded checks. The changes are to suppress warnings where non-transient fields in serializable types are not declared with a type statically known to be serializable. That isn't necessarily a correctness issues, but it does merit further scrutiny. I'll run a script to update the copyright year before pushing. Sending this change out for review separately in an effort to de-bulk the review needed for the new checks themselves. - Commit messages: - 8274405: Suppress warnings on non-serializable non-transient instance fields in javac and javadoc Changes: https://git.openjdk.java.net/jdk/pull/5728/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5728&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274405 Stats: 12 lines in 12 files changed: 12 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/5728.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5728/head:pull/5728 PR: https://git.openjdk.java.net/jdk/pull/5728
Re: RFR: 8202056: Expand serial warning to check for bad overloads of serial-related methods and ineffectual fields [v3]
> This is an initial PR for expanded lint warnings done under two bugs:
>
> 8202056: Expand serial warning to check for bad overloads of serial-related
> methods and ineffectual fields
> 8160675: Issue lint warning for non-serializable non-transient instance
> fields in serializable type
>
> to get feedback on the general approach and test strategy before further
> polishing the implementation.
>
> The implementation initially started as an annotation processor I wrote
> several years ago. The refined version being incorporated into Attr has been
> refactored, had its checks expanded, and been partially ported to idiomatic
> javac coding style rather than using the javax.lang.model API from annotation
> processing.
>
> Subsequent versions of this PR are expected to move the implementation closer
> to idiomatic javac, in particular to use javac flags rather than
> javax.lang.model.Modifier's. Additional resources keys will be defined for
> the serialization-related fields and methods not having the expected
> modifiers, types, etc. The resource keys for the existing checks related to
> serialVersionUID and reused.
>
> Please also review the corresponding CSRs:
>
> https://bugs.openjdk.java.net/browse/JDK-8274335
> https://bugs.openjdk.java.net/browse/JDK-8274336
>
> Informative serialization-related warning messages must take into account
> whether a class, interface, annotation, record, and enum is being analyzed.
> Enum classes and record classes have special handling in serialization. This
> implementation under review has been augmented with checks for interface
> types recommended by Chris Hegarty in an attachment on 8202056.
>
> The JDK build has the Xlint:serial check enabled. The build did not pass with
> the augmented checks. For most modules, this PR contains the library changes
> necessary for the build to pass. I will start separate PRs in those library
> areas to get the needed SuppressWarning("serial") or other changes in place.
> For one module, I temporarily disabled the Xlint:serial check.
>
> In terms of performance, I have not done benchmarks of the JDK build with and
> without these changes, but informally the build seems to take about as long
> as before.
Joe Darcy has updated the pull request with a new target base due to a merge or
a rebase. The incremental webrev excludes the unrelated changes brought in by
the merge/rebase. The pull request contains 33 additional commits since the
last revision:
- Merge branch 'master' into JDK-8202056
- Merge branch 'master' into JDK-8202056
- Restore serial checks on java.xml module.
- Merge branch 'master' into JDK-8202056
- Appease jcheck
- Implement checks chegar recommended for interfaces.
- Update comment.
- Add tests for instance field checks.
- Clean build with instance field checks in place.
- Merge branch 'master' into JDK-8202056
- ... and 23 more: https://git.openjdk.java.net/jdk/compare/7925d0cc...bacff4e9
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/5709/files
- new: https://git.openjdk.java.net/jdk/pull/5709/files/053de6bb..bacff4e9
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5709&range=02
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5709&range=01-02
Stats: 2951 lines in 77 files changed: 2399 ins; 397 del; 155 mod
Patch: https://git.openjdk.java.net/jdk/pull/5709.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/5709/head:pull/5709
PR: https://git.openjdk.java.net/jdk/pull/5709
Re: RFR: 8202056: Expand serial warning to check for bad overloads of serial-related methods and ineffectual fields [v4]
> This is an initial PR for expanded lint warnings done under two bugs:
>
> 8202056: Expand serial warning to check for bad overloads of serial-related
> methods and ineffectual fields
> 8160675: Issue lint warning for non-serializable non-transient instance
> fields in serializable type
>
> to get feedback on the general approach and test strategy before further
> polishing the implementation.
>
> The implementation initially started as an annotation processor I wrote
> several years ago. The refined version being incorporated into Attr has been
> refactored, had its checks expanded, and been partially ported to idiomatic
> javac coding style rather than using the javax.lang.model API from annotation
> processing.
>
> Subsequent versions of this PR are expected to move the implementation closer
> to idiomatic javac, in particular to use javac flags rather than
> javax.lang.model.Modifier's. Additional resources keys will be defined for
> the serialization-related fields and methods not having the expected
> modifiers, types, etc. The resource keys for the existing checks related to
> serialVersionUID and reused.
>
> Please also review the corresponding CSRs:
>
> https://bugs.openjdk.java.net/browse/JDK-8274335
> https://bugs.openjdk.java.net/browse/JDK-8274336
>
> Informative serialization-related warning messages must take into account
> whether a class, interface, annotation, record, and enum is being analyzed.
> Enum classes and record classes have special handling in serialization. This
> implementation under review has been augmented with checks for interface
> types recommended by Chris Hegarty in an attachment on 8202056.
>
> The JDK build has the Xlint:serial check enabled. The build did not pass with
> the augmented checks. For most modules, this PR contains the library changes
> necessary for the build to pass. I will start separate PRs in those library
> areas to get the needed SuppressWarning("serial") or other changes in place.
> For one module, I temporarily disabled the Xlint:serial check.
>
> In terms of performance, I have not done benchmarks of the JDK build with and
> without these changes, but informally the build seems to take about as long
> as before.
Joe Darcy has updated the pull request with a new target base due to a merge or
a rebase. The pull request now contains 35 commits:
- Fix whitespace.
- Merge branch 'master' into JDK-8202056
- Merge branch 'master' into JDK-8202056
- Merge branch 'master' into JDK-8202056
- Restore serial checks on java.xml module.
- Merge branch 'master' into JDK-8202056
- Appease jcheck
- Implement checks chegar recommended for interfaces.
- Update comment.
- Add tests for instance field checks.
- ... and 25 more: https://git.openjdk.java.net/jdk/compare/d8a278f3...37ad8778
-
Changes: https://git.openjdk.java.net/jdk/pull/5709/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5709&range=03
Stats: 1495 lines in 64 files changed: 1488 ins; 0 del; 7 mod
Patch: https://git.openjdk.java.net/jdk/pull/5709.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/5709/head:pull/5709
PR: https://git.openjdk.java.net/jdk/pull/5709
Re: RFR: 8274405: Suppress warnings on non-serializable non-transient instance fields in javac and javadoc
On Wed, 29 Sep 2021 17:35:03 GMT, Pavel Rappo wrote: > > > Is there any semantic difference between "not statically Serilizable" and > "not a Serilizable type"? Also, there's a typo: Seri-a-lizable. Same semantics. The first phase of this cleanup used "not statically Serilizable"; however, I thought "not a Serilizable type" is a more precise statement. I can make a pass over the PR and make the wording consistent in the new comments. - PR: https://git.openjdk.java.net/jdk/pull/5728
Re: RFR: 8274405: Suppress warnings on non-serializable non-transient instance fields in javac and javadoc [v2]
On Wed, 29 Sep 2021 18:11:24 GMT, Pavel Rappo wrote: > > > > > Is there any semantic difference between "not statically Serilizable" and > > > "not a Serilizable type"? Also, there's a typo: Seri-a-lizable. > > > > > > Same semantics. The first phase of this cleanup used "not statically > > Serilizable"; however, I thought "not a Serilizable type" is a more precise > > statement. I can make a pass over the PR and make the wording consistent in > > the new comments. > > You could update this PR if only to fix the typo; the phrasing is up to you. Wording updated. - PR: https://git.openjdk.java.net/jdk/pull/5728
Re: RFR: 8274405: Suppress warnings on non-serializable non-transient instance fields in javac and javadoc [v2]
> Augmentations to javac's Xlint:serial checking are out for review (#5709) and > various javac and javadoc implementation libraries would need some changes to > pass under the expanded checks. > > The changes are to suppress warnings where non-transient fields in > serializable types are not declared with a type statically known to be > serializable. That isn't necessarily a correctness issues, but it does merit > further scrutiny. > > I'll run a script to update the copyright year before pushing. > > Sending this change out for review separately in an effort to de-bulk the > review needed for the new checks themselves. Joe Darcy has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains two additional commits since the last revision: - Merge branch 'master' into JDK-8274405 - 8274405: Suppress warnings on non-serializable non-transient instance fields in javac and javadoc - Changes: - all: https://git.openjdk.java.net/jdk/pull/5728/files - new: https://git.openjdk.java.net/jdk/pull/5728/files/c7cfa110..7ad3a07c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5728&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5728&range=00-01 Stats: 4793 lines in 167 files changed: 3399 ins; 976 del; 418 mod Patch: https://git.openjdk.java.net/jdk/pull/5728.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5728/head:pull/5728 PR: https://git.openjdk.java.net/jdk/pull/5728
Re: RFR: 8274405: Suppress warnings on non-serializable non-transient instance fields in javac and javadoc [v3]
> Augmentations to javac's Xlint:serial checking are out for review (#5709) and > various javac and javadoc implementation libraries would need some changes to > pass under the expanded checks. > > The changes are to suppress warnings where non-transient fields in > serializable types are not declared with a type statically known to be > serializable. That isn't necessarily a correctness issues, but it does merit > further scrutiny. > > I'll run a script to update the copyright year before pushing. > > Sending this change out for review separately in an effort to de-bulk the > review needed for the new checks themselves. Joe Darcy has updated the pull request incrementally with one additional commit since the last revision: Respond to review feedback. - Changes: - all: https://git.openjdk.java.net/jdk/pull/5728/files - new: https://git.openjdk.java.net/jdk/pull/5728/files/7ad3a07c..ceb0450e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5728&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5728&range=01-02 Stats: 8 lines in 8 files changed: 0 ins; 0 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/5728.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5728/head:pull/5728 PR: https://git.openjdk.java.net/jdk/pull/5728
Re: RFR: 8274405: Suppress warnings on non-serializable non-transient instance fields in javac and javadoc [v4]
> Augmentations to javac's Xlint:serial checking are out for review (#5709) and > various javac and javadoc implementation libraries would need some changes to > pass under the expanded checks. > > The changes are to suppress warnings where non-transient fields in > serializable types are not declared with a type statically known to be > serializable. That isn't necessarily a correctness issues, but it does merit > further scrutiny. > > I'll run a script to update the copyright year before pushing. > > Sending this change out for review separately in an effort to de-bulk the > review needed for the new checks themselves. Joe Darcy has updated the pull request incrementally with one additional commit since the last revision: Update copyright years. - Changes: - all: https://git.openjdk.java.net/jdk/pull/5728/files - new: https://git.openjdk.java.net/jdk/pull/5728/files/ceb0450e..b2a9e636 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5728&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5728&range=02-03 Stats: 10 lines in 10 files changed: 0 ins; 0 del; 10 mod Patch: https://git.openjdk.java.net/jdk/pull/5728.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5728/head:pull/5728 PR: https://git.openjdk.java.net/jdk/pull/5728
Integrated: 8274405: Suppress warnings on non-serializable non-transient instance fields in javac and javadoc
On Tue, 28 Sep 2021 00:07:16 GMT, Joe Darcy wrote: > Augmentations to javac's Xlint:serial checking are out for review (#5709) and > various javac and javadoc implementation libraries would need some changes to > pass under the expanded checks. > > The changes are to suppress warnings where non-transient fields in > serializable types are not declared with a type statically known to be > serializable. That isn't necessarily a correctness issues, but it does merit > further scrutiny. > > I'll run a script to update the copyright year before pushing. > > Sending this change out for review separately in an effort to de-bulk the > review needed for the new checks themselves. This pull request has now been integrated. Changeset: 97385d4f Author:Joe Darcy URL: https://git.openjdk.java.net/jdk/commit/97385d4f166fbd63a7c91d2ee28b5ed75cb02518 Stats: 22 lines in 12 files changed: 12 ins; 0 del; 10 mod 8274405: Suppress warnings on non-serializable non-transient instance fields in javac and javadoc Reviewed-by: prappo, jjg - PR: https://git.openjdk.java.net/jdk/pull/5728
Re: RFR: 8274405: Suppress warnings on non-serializable non-transient instance fields in javac and javadoc [v3]
On Wed, 29 Sep 2021 22:52:08 GMT, Jonathan Gibbons wrote:
>
>
> There are 3, maybe just 2, groups of files in this review.
>
> `sjavac` is an internal utility that ought not to be in the `src/` tree, so
> the changes there don't matter.
>
> The various `Result` classes and the javadoc exceptions are all "internal"
> exceptions used for internal control flow, and not intended to be seen by API
> clients. As @pavelrappo notes, we have plans to make the `Result` classes go
> away by rewriting the relevant scanners to make them unnecessary. That leaves
> the javadoc exceptions. The commented annotations have a slight code-smell
> about them, in the sense of brushing the warning under the carpet, so to
> speak. It would be better if there was a better way to remove the cause of
> the warning, rather than just hiding the warning itself. But, that's not
> easy, and the original sin was making `Throwable` (and hence all subtypes)
> implement `Serializable` in the first place. But, that's the serialization we
> have and we have to deal with it.
>
> As a final note, I never did like the `Runnable` in the last exception, and
> seeing it again may be a good reason to try and get rid of it, like those
> `Result` classes mentioned earlier. I also note that `DocPath` _could_ be
> made `Serializable` but `DocFile` cannot reasonably be made serializable
> (incompatible API change to `Location`) and even thinking about it seems like
> a case of the tail wagging the elephant!
>
> So, with some disappointment, I accept that the changes you propose are the
> least bad of the possible solutions, at least for now, and so I approve the
> review.
If I were to see
@SuppressWarning("serial") // Type of field not Serializable
Foo foo;
in new code I would question the serial-design of the class; that is one of my
goals with augmenting the Xlint:serial checks, notice possibly questionable
serial coding patterns sooner.
For types where maintaining cross-release serial compatibility is not strictly
needed, one approach would be to mark the fields as transient and give the
class a different serialVersionUID. If serial compatibility is needed the
conceptually transient field, i.e. one that cannot meaningful be serialized,
could be nulled-out in a writeObject method and ignored in a readObject method,
at the cost of maintaining those additional methods.
Fortunately, the the javax.lang.model API, the issues with exception classes
having non-Serializable fields was recognized during the design and we marked
all such fields and transient and documented the corresponding getters to
possibly return null.
-
PR: https://git.openjdk.java.net/jdk/pull/5728
Re: RFR: 8202056: Expand serial warning to check for bad overloads of serial-related methods and ineffectual fields [v5]
> This is an initial PR for expanded lint warnings done under two bugs:
>
> 8202056: Expand serial warning to check for bad overloads of serial-related
> methods and ineffectual fields
> 8160675: Issue lint warning for non-serializable non-transient instance
> fields in serializable type
>
> to get feedback on the general approach and test strategy before further
> polishing the implementation.
>
> The implementation initially started as an annotation processor I wrote
> several years ago. The refined version being incorporated into Attr has been
> refactored, had its checks expanded, and been partially ported to idiomatic
> javac coding style rather than using the javax.lang.model API from annotation
> processing.
>
> Subsequent versions of this PR are expected to move the implementation closer
> to idiomatic javac, in particular to use javac flags rather than
> javax.lang.model.Modifier's. Additional resources keys will be defined for
> the serialization-related fields and methods not having the expected
> modifiers, types, etc. The resource keys for the existing checks related to
> serialVersionUID and reused.
>
> Please also review the corresponding CSRs:
>
> https://bugs.openjdk.java.net/browse/JDK-8274335
> https://bugs.openjdk.java.net/browse/JDK-8274336
>
> Informative serialization-related warning messages must take into account
> whether a class, interface, annotation, record, and enum is being analyzed.
> Enum classes and record classes have special handling in serialization. This
> implementation under review has been augmented with checks for interface
> types recommended by Chris Hegarty in an attachment on 8202056.
>
> The JDK build has the Xlint:serial check enabled. The build did not pass with
> the augmented checks. For most modules, this PR contains the library changes
> necessary for the build to pass. I will start separate PRs in those library
> areas to get the needed SuppressWarning("serial") or other changes in place.
> For one module, I temporarily disabled the Xlint:serial check.
>
> In terms of performance, I have not done benchmarks of the JDK build with and
> without these changes, but informally the build seems to take about as long
> as before.
Joe Darcy has updated the pull request with a new target base due to a merge or
a rebase. The pull request now contains 36 commits:
- Merge branch 'master' into JDK-8202056
- Fix whitespace.
- Merge branch 'master' into JDK-8202056
- Merge branch 'master' into JDK-8202056
- Merge branch 'master' into JDK-8202056
- Restore serial checks on java.xml module.
- Merge branch 'master' into JDK-8202056
- Appease jcheck
- Implement checks chegar recommended for interfaces.
- Update comment.
- ... and 26 more: https://git.openjdk.java.net/jdk/compare/355356c4...33504e85
-
Changes: https://git.openjdk.java.net/jdk/pull/5709/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5709&range=04
Stats: 1483 lines in 52 files changed: 1476 ins; 0 del; 7 mod
Patch: https://git.openjdk.java.net/jdk/pull/5709.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/5709/head:pull/5709
PR: https://git.openjdk.java.net/jdk/pull/5709
Re: RFR: 8276635: Use blessed modifier order in compiler code
On Thu, 4 Nov 2021 11:48:04 GMT, Magnus Ihse Bursie wrote: > I ran bin/blessed-modifier-order.sh on source owned by compiler. This scripts > verifies that modifiers are in the "blessed" order, and fixes it otherwise. I > have manually checked the changes made by the script to make sure they are > sound. Looks fine. - Marked as reviewed by darcy (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6253
Re: RFR: JDK-8280488: doclint reference checks withstand warning suppression
On Wed, 26 Jan 2022 02:20:24 GMT, Jonathan Gibbons wrote: > Please review a small modification to the way that bad references are > reported by DocLint. > > A new "mode" is introduced, `strictReferenceChecks`. > > If the mode is _not_ set, references that explicitly include a module name > when that module name is not resolved in the module graph will be reported > with a (suppressible) warning instead of an error. All other issues with > references will be reported as errors. This is the mode used by javac. > > If the mode is set, all issues with references will be reported as errors. > This is the mode used by javadoc. > > This will need to be documented in the tool guide (man page). Question: for the JDK use case in particular, to turn all the doclint warning on during compilation, the cross-module references need to be SuppressWarning'ed? - PR: https://git.openjdk.java.net/jdk/pull/7222
RFR: JDK-8280534: Enable compile-time doclint reference checking
The changes in this PR on top of the out-for-review changes in
https://git.openjdk.java.net/jdk/pull/7222 allow compile-time doclint checking
to be enabled in all JDK modules.
Typically, a @SuppressWarnings("doclint:refernce") annotation is added to
declaration with javadoc blocks that have already had distinguished
cross-module links added (JDK-8280492).
One exception is in src/java.base/share/classes/java/net/package-info.java
where the cross-module link was (for now) removed. Currently the
SuppressWarnings annotation type is not declared to allow its annotations to be
applied to package declarations. I'll look into amending that, but in the mean
time, I think it is beneficial for the JDK build, and the base module in
particular, to have compile-time doclint protections turned on.
-
Commit messages:
- Cover java.base.
- JDK-8280534: Enable compile-time doclint reference checking
Changes: https://git.openjdk.java.net/jdk/pull/7237/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7237&range=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8280534
Stats: 28 lines in 21 files changed: 20 ins; 0 del; 8 mod
Patch: https://git.openjdk.java.net/jdk/pull/7237.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/7237/head:pull/7237
PR: https://git.openjdk.java.net/jdk/pull/7237
Re: RFR: JDK-8280534: Enable compile-time doclint reference checking
On Fri, 28 Jan 2022 00:04:34 GMT, Naoto Sato wrote: > Looks fine. Nit: some files need copyright year updates. Acknowledged; I'll run a copyright update script before pushing (I tend to run that close to pushing to avoid spurious, if minor, merge conflicts). Thanks. - PR: https://git.openjdk.java.net/jdk/pull/7237
Re: RFR: JDK-8280488: doclint reference checks withstand warning suppression [v2]
On Wed, 26 Jan 2022 16:46:23 GMT, Jonathan Gibbons wrote: >> Please review a small modification to the way that bad references are >> reported by DocLint. >> >> A new "mode" is introduced, `strictReferenceChecks`. >> >> If the mode is _not_ set, references that explicitly include a module name >> when that module name is not resolved in the module graph will be reported >> with a (suppressible) warning instead of an error. All other issues with >> references will be reported as errors. This is the mode used by javac. >> >> If the mode is set, all issues with references will be reported as errors. >> This is the mode used by javadoc. >> >> This will need to be documented in the tool guide (man page). > > Jonathan Gibbons has updated the pull request incrementally with one > additional commit since the last revision: > > fix typos FYI, the build completes with the changes in this PR, expanding the target set of of SuppressWarnings (JDK-8280744), and https://github.com/openjdk/jdk/pull/7237. - PR: https://git.openjdk.java.net/jdk/pull/7222
Re: RFR: JDK-8280534: Enable compile-time doclint reference checking [v2]
> The changes in this PR on top of the out-for-review changes in
> https://git.openjdk.java.net/jdk/pull/7222 allow compile-time doclint
> checking to be enabled in all JDK modules.
>
> Typically, a @SuppressWarnings("doclint:refernce") annotation is added to
> declaration with javadoc blocks that have already had distinguished
> cross-module links added (JDK-8280492).
>
> One exception is in src/java.base/share/classes/java/net/package-info.java
> where the cross-module link was (for now) removed. Currently the
> SuppressWarnings annotation type is not declared to allow its annotations to
> be applied to package declarations. I'll look into amending that, but in the
> mean time, I think it is beneficial for the JDK build, and the base module in
> particular, to have compile-time doclint protections turned on.
Joe Darcy has updated the pull request with a new target base due to a merge or
a rebase. The incremental webrev excludes the unrelated changes brought in by
the merge/rebase. The pull request contains four additional commits since the
last revision:
- Use capabilities of JDK-8280744.
- Merge branch 'master' into JDK-8280534
- Cover java.base.
- JDK-8280534: Enable compile-time doclint reference checking
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/7237/files
- new: https://git.openjdk.java.net/jdk/pull/7237/files/70e9fb4a..d03401c6
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7237&range=01
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7237&range=00-01
Stats: 2743 lines in 129 files changed: 1556 ins; 659 del; 528 mod
Patch: https://git.openjdk.java.net/jdk/pull/7237.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/7237/head:pull/7237
PR: https://git.openjdk.java.net/jdk/pull/7237
Re: RFR: JDK-8280488: doclint reference checks withstand warning suppression [v2]
On Wed, 26 Jan 2022 16:46:23 GMT, Jonathan Gibbons wrote: >> Please review a small modification to the way that bad references are >> reported by DocLint. >> >> A new "mode" is introduced, `strictReferenceChecks`. >> >> If the mode is _not_ set, references that explicitly include a module name >> when that module name is not resolved in the module graph will be reported >> with a (suppressible) warning instead of an error. All other issues with >> references will be reported as errors. This is the mode used by javac. >> >> If the mode is set, all issues with references will be reported as errors. >> This is the mode used by javadoc. >> >> This will need to be documented in the tool guide (man page). > > Jonathan Gibbons has updated the pull request incrementally with one > additional commit since the last revision: > > fix typos Marked as reviewed by darcy (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7222
Re: RFR: JDK-8280534: Enable compile-time doclint reference checking [v3]
> The changes in this PR on top of the out-for-review changes in
> https://git.openjdk.java.net/jdk/pull/7222 allow compile-time doclint
> checking to be enabled in all JDK modules.
>
> Typically, a @SuppressWarnings("doclint:refernce") annotation is added to
> declaration with javadoc blocks that have already had distinguished
> cross-module links added (JDK-8280492).
>
> One exception is in src/java.base/share/classes/java/net/package-info.java
> where the cross-module link was (for now) removed. Currently the
> SuppressWarnings annotation type is not declared to allow its annotations to
> be applied to package declarations. I'll look into amending that, but in the
> mean time, I think it is beneficial for the JDK build, and the base module in
> particular, to have compile-time doclint protections turned on.
Joe Darcy has updated the pull request with a new target base due to a merge or
a rebase. The incremental webrev excludes the unrelated changes brought in by
the merge/rebase. The pull request contains five additional commits since the
last revision:
- Merge branch 'master' into JDK-8280534
- Use capabilities of JDK-8280744.
- Merge branch 'master' into JDK-8280534
- Cover java.base.
- JDK-8280534: Enable compile-time doclint reference checking
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/7237/files
- new: https://git.openjdk.java.net/jdk/pull/7237/files/d03401c6..a0b37495
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7237&range=02
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7237&range=01-02
Stats: 1473 lines in 53 files changed: 585 ins; 557 del; 331 mod
Patch: https://git.openjdk.java.net/jdk/pull/7237.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/7237/head:pull/7237
PR: https://git.openjdk.java.net/jdk/pull/7237
Integrated: JDK-8280534: Enable compile-time doclint reference checking
On Wed, 26 Jan 2022 20:05:07 GMT, Joe Darcy wrote:
> The changes in this PR on top of the out-for-review changes in
> https://git.openjdk.java.net/jdk/pull/7222 allow compile-time doclint
> checking to be enabled in all JDK modules.
>
> Typically, a @SuppressWarnings("doclint:refernce") annotation is added to
> declaration with javadoc blocks that have already had distinguished
> cross-module links added (JDK-8280492).
>
> One exception is in src/java.base/share/classes/java/net/package-info.java
> where the cross-module link was (for now) removed. Currently the
> SuppressWarnings annotation type is not declared to allow its annotations to
> be applied to package declarations. I'll look into amending that, but in the
> mean time, I think it is beneficial for the JDK build, and the base module in
> particular, to have compile-time doclint protections turned on.
This pull request has now been integrated.
Changeset: 4dbebb62
Author:Joe Darcy
URL:
https://git.openjdk.java.net/jdk/commit/4dbebb62aa264adda8d96a06f608ef9d13155a26
Stats: 28 lines in 21 files changed: 21 ins; 0 del; 7 mod
8280534: Enable compile-time doclint reference checking
Reviewed-by: serb, naoto, mchung, lancea, iris
-
PR: https://git.openjdk.java.net/jdk/pull/7237
Re: RFR: JDK-8281007: Test jdk/javadoc/doclet/checkStylesheetClasses/CheckStylesheetClasses.java fails after JDK-8280738
On Tue, 1 Feb 2022 00:22:31 GMT, Jonathan Gibbons wrote: > Remove review a trivial fix to remove a workaround for the absence of some > CSS classes in HtmlStyle.java. Those snippet-related classes have now been > added, and the workaround needs to be removed. Marked as reviewed by darcy (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7293
Re: RFR: 8280713: Related to comment inheritance jdk.javadoc cleanup and refactoring [v3]
On Fri, 25 Feb 2022 17:46:05 GMT, Pavel Rappo wrote: >> Explorative refactoring performed while looking into multiple `@inheritDoc` >> issues. The easiest way to review it is to, probably, go commit by commit; >> they are quite focused and commented. Not only the branch as a whole, but >> all the constituent commits should pass tests and leave JDK API >> Documentation unchanged. > > Pavel Rappo has updated the pull request incrementally with two additional > commits since the last revision: > > - Fix outdated code > >Uses a set instead of list for quick method search. In a future commit > we'll try to figure out why `found` are not unique. > - Fix outdated code > >Fix outdated inline comments and names. Both the assertion and the > `contains` method will be removed in a future commit. > I've looked over the Utils.java class. From my initial reading, I think it would be reasonable to add ElementKind.isExecutable[Element] ElementKind.isVariable[Element] ElementKind.isDeclaredType() // isClass || isInterface currently I'm less convinced of the benefit of adding various "isFoo()" methods. I've filed JDK-8282411: Add useful predicates to ElementKind to track this work. - PR: https://git.openjdk.java.net/jdk/pull/7233
Re: RFR: 8280713: Related to comment inheritance jdk.javadoc cleanup and refactoring [v3]
On Sat, 26 Feb 2022 01:12:39 GMT, Jonathan Gibbons wrote: > > ElementKind.isExecutable[Element] > > ElementKind.isVariable[Element] > > ElementKind.isDeclaredType() // isClass || isInterface currently > > Yes, predicates for the "unions" would be useful, especially now that the > language is adding more specialized variants of plain old class and plain old > interface. Changes out for review under https://github.com/openjdk/jdk/pull/7637 - PR: https://git.openjdk.java.net/jdk/pull/7233
Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines
On Fri, 28 Jan 2022 14:39:31 GMT, Matteo Baccan wrote: > Hi > > I have reviewed the code for removing double semicolons at the end of lines > > all the best > matteo Marked as reviewed by darcy (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7268
Re: RFR: 8282756: Make ElementKind checks more specific
On Tue, 8 Mar 2022 17:58:28 GMT, Jonathan Gibbons wrote:
>> Out of all executable elements, inherit documentation only for methods.
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java
> line 311:
>
>> 309: // Note that e.getKind().isInterface() is not the same as
>> e.getKind() == INTERFACE
>> 310: public boolean isInterface(Element e) {
>> 311: return e.getKind() == INTERFACE;
>
> This seems asymmetrically different from `isClass`. We should come up with
> more consistent terminology/usage.
isClassExact? isInterfaceExact?
-
PR: https://git.openjdk.java.net/jdk/pull/7747
Re: RFR: JDK-8285496: DocLint does not check for missing `@param` tags for type parameters on classes and interfaces
On Thu, 28 Apr 2022 22:32:59 GMT, Jonathan Gibbons wrote: > Please review a trivial update for doclint, to check for `@param` tags for > type parameters of classes and interfaces. > > The bug was discovered recently, while making an update for record > components, but this part of the fix was effectively blocked by the presence > of missing tags in API documentation. > > As well as a minor update to an existing test, the change has been tested by > running `make docs`, `make docs-javase` and `make docs-reference`, all of > which succeed without any warnings about missing tags. Marked as reviewed by darcy (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8458
Re: RFR: 8286654: Add an optional description accessor on ToolProvider interface
On Wed, 18 May 2022 14:56:47 GMT, Christian Stein wrote: > This PR adds an optional description accessor on `ToolProvider` interface. > > This PR also adds short description for each of the listed tool: > - `jar` > - `javac` > - `javadoc` > - `javap` and `jdeps` > - `jlink` and `jmod` > - `jpackage` Marked as reviewed by darcy (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8772
