Topics

Several questions on Javadoc


Bob Jacobsen
 

Something changed recently in the Javadoc processing. I noticed this because Jenkins started showing 35 Javadoc warnings.

Looking into this, I have a couple questions:

1) Some of our Javadoc build tasks build only public methods and members; others show protected and package access levels. Public is appropriate for a general API. Protected (or even package) is appropriate as a developer reference. I think we should rationalize the tasks so they all create the same one. I prefer the larger result of including package access, but it is quite a bit larger. Anybody have preferences here?

One of the reasons to rationalize the targets is that Jenkins is now showing warnings from protected members that I hadn’t been seeing when building Javadoc locally.

Which brings up another item:

2) The warnings are all about missing @param or @return tags. The real Javadocs do have all those entered, having them is the standard form, and we agreed a long time ago to keep up with the Java formatting standards. (Originally, Javadoc did not require these, and created them in minimalist form if missing; that stopped a long time ago, and we haven’t caught up) So it’s reasonable to warn on those so we can fix them. I’m going to PR a few shortly. But we need to fix them all, and most of the warnings are now coming from the generated jjtree Java files. Two examples:

[WARNING] /Users/jake/Documents/Trains/JMRI/projects/JMRI/target/generated-sources/jjtree/jmri/jmrix/srcp/parser/Node.java:22: warning: no @param for n
[WARNING] public void jjtSetParent(Node n);
[WARNING] ^
[WARNING] /Users/jake/Documents/Trains/JMRI/projects/JMRI/target/generated-sources/jjtree/jmri/jmrix/srcp/parser/Node.java:27: warning: no @param for n
[WARNING] public void jjtAddChild(Node n, int i);
[WARNING] ^

I tried a couple of things to suppress those warnings from the automatically-built classes without success. Does anybody know how to do that?

3) I’d like to confirm that our goal is to get these warnings down to zero, and the have CI reject PRs that are showing Javadoc warnings. (It won’t be insisting on Javadoc, just on correct Javadoc; there’s no warning if there’s nothing on a method, only a warning when partial/incomplete) Anybody concerned about that?

Bob
--
Bob Jacobsen
@BobJacobsen


Randall Wood <rhwood@...>
 

I thought we had something to strip the second * in comments in the jjtree generated code.

On May 19, 2020, at 11:30, Bob Jacobsen <@BobJacobsen> wrote:

Something changed recently in the Javadoc processing. I noticed this because Jenkins started showing 35 Javadoc warnings.

Looking into this, I have a couple questions:

1) Some of our Javadoc build tasks build only public methods and members; others show protected and package access levels. Public is appropriate for a general API. Protected (or even package) is appropriate as a developer reference. I think we should rationalize the tasks so they all create the same one. I prefer the larger result of including package access, but it is quite a bit larger. Anybody have preferences here?

One of the reasons to rationalize the targets is that Jenkins is now showing warnings from protected members that I hadn’t been seeing when building Javadoc locally.

Which brings up another item:

2) The warnings are all about missing @param or @return tags. The real Javadocs do have all those entered, having them is the standard form, and we agreed a long time ago to keep up with the Java formatting standards. (Originally, Javadoc did not require these, and created them in minimalist form if missing; that stopped a long time ago, and we haven’t caught up) So it’s reasonable to warn on those so we can fix them. I’m going to PR a few shortly. But we need to fix them all, and most of the warnings are now coming from the generated jjtree Java files. Two examples:

[WARNING] /Users/jake/Documents/Trains/JMRI/projects/JMRI/target/generated-sources/jjtree/jmri/jmrix/srcp/parser/Node.java:22: warning: no @param for n
[WARNING] public void jjtSetParent(Node n);
[WARNING] ^
[WARNING] /Users/jake/Documents/Trains/JMRI/projects/JMRI/target/generated-sources/jjtree/jmri/jmrix/srcp/parser/Node.java:27: warning: no @param for n
[WARNING] public void jjtAddChild(Node n, int i);
[WARNING] ^

I tried a couple of things to suppress those warnings from the automatically-built classes without success. Does anybody know how to do that?

3) I’d like to confirm that our goal is to get these warnings down to zero, and the have CI reject PRs that are showing Javadoc warnings. (It won’t be insisting on Javadoc, just on correct Javadoc; there’s no warning if there’s nothing on a method, only a warning when partial/incomplete) Anybody concerned about that?

Bob
--
Bob Jacobsen
@BobJacobsen






Bob Jacobsen
 

Thanks! That was the hint I needed. We had that in the JavaCC target, but not JJTree. Working on that now.

Now for another question:

Turns out that setting Xmaxwarns to 36 i.e.

<arg value="-Xdoclint:all"/><!-- remove -missing to get warnings about missing javadoc tags -->
<arg value="-Xmaxwarns"/>
<arg value="36"/><!-- change from default 100 warnings; keep synced with pom.xml, other javadoc targets —>

_limits_ the warnings, but doesn’t cause the task to fail if you go over that. I.e. there are _lots_ of warning-causing-conditions present in the code, but only 36 are displayed. That’s not great, as it gives a discouraging experience: Fix one, rerun Javadocs, get new ones showing up.

Can we convert this to “show them all, and fail if too many, and set ’too many’ to how many there are now’”? Does anybody know how to set such a limit?

Bob

On May 19, 2020, at 8:52 AM, Randall Wood via groups.io <rhwood=mac.com@groups.io> wrote:

I thought we had something to strip the second * in comments in the jjtree generated code.
On May 19, 2020, at 11:30, Bob Jacobsen <@BobJacobsen> wrote:

Something changed recently in the Javadoc processing. I noticed this because Jenkins started showing 35 Javadoc warnings.

Looking into this, I have a couple questions:

1) Some of our Javadoc build tasks build only public methods and members; others show protected and package access levels. Public is appropriate for a general API. Protected (or even package) is appropriate as a developer reference. I think we should rationalize the tasks so they all create the same one. I prefer the larger result of including package access, but it is quite a bit larger. Anybody have preferences here?

One of the reasons to rationalize the targets is that Jenkins is now showing warnings from protected members that I hadn’t been seeing when building Javadoc locally.

Which brings up another item:

2) The warnings are all about missing @param or @return tags. The real Javadocs do have all those entered, having them is the standard form, and we agreed a long time ago to keep up with the Java formatting standards. (Originally, Javadoc did not require these, and created them in minimalist form if missing; that stopped a long time ago, and we haven’t caught up) So it’s reasonable to warn on those so we can fix them. I’m going to PR a few shortly. But we need to fix them all, and most of the warnings are now coming from the generated jjtree Java files. Two examples:

[WARNING] /Users/jake/Documents/Trains/JMRI/projects/JMRI/target/generated-sources/jjtree/jmri/jmrix/srcp/parser/Node.java:22: warning: no @param for n
[WARNING] public void jjtSetParent(Node n);
[WARNING] ^
[WARNING] /Users/jake/Documents/Trains/JMRI/projects/JMRI/target/generated-sources/jjtree/jmri/jmrix/srcp/parser/Node.java:27: warning: no @param for n
[WARNING] public void jjtAddChild(Node n, int i);
[WARNING] ^

I tried a couple of things to suppress those warnings from the automatically-built classes without success. Does anybody know how to do that?

3) I’d like to confirm that our goal is to get these warnings down to zero, and the have CI reject PRs that are showing Javadoc warnings. (It won’t be insisting on Javadoc, just on correct Javadoc; there’s no warning if there’s nothing on a method, only a warning when partial/incomplete) Anybody concerned about that?

Bob
--
Bob Jacobsen
@BobJacobsen






--
Bob Jacobsen
@BobJacobsen


Randall Wood <rhwood@...>
 

It seems to me that, for both maven and ant, the fail on warning is absolute — i.e. the javadoc process can be set to fail if there are 1 or more warnings, or to not fail on any warnings, but cannot be set to fail only if there are (for example) 10 or more warnings, but pass with 9 or less warnings.

Randall

On 19-May-2020, at 15:25, Bob Jacobsen <rgj1927@...> wrote:

Thanks! That was the hint I needed.  We had that in the JavaCC target, but not JJTree.  Working on that now.

Now for another question:

Turns out that setting Xmaxwarns to 36 i.e.

           <arg value="-Xdoclint:all"/><!-- remove -missing to get warnings about missing javadoc tags -->
           <arg value="-Xmaxwarns"/>
           <arg value="36"/><!-- change from default 100 warnings; keep synced with pom.xml, other javadoc targets —>

_limits_ the warnings, but doesn’t cause the task to fail if you go over that.  I.e. there are _lots_ of warning-causing-conditions present in the code, but only 36 are displayed.  That’s not great, as it gives a discouraging experience:  Fix one, rerun Javadocs, get new ones showing up.

Can we convert this to “show them all, and fail if too many, and set ’too many’ to how many there are now’”?  Does anybody know how to set such a limit?

Bob

On May 19, 2020, at 8:52 AM, Randall Wood via groups.io <rhwood@...> wrote:

I thought we had something to strip the second * in comments in the jjtree generated code.
On May 19, 2020, at 11:30, Bob Jacobsen <rgj1927@...> wrote:

Something changed recently in the Javadoc processing. I noticed this because Jenkins started showing 35 Javadoc warnings.

Looking into this, I have a couple questions:

1) Some of our Javadoc build tasks build only public methods and members; others show protected and package access levels.  Public is appropriate for a general API.  Protected (or even package) is appropriate as a developer reference.  I think we should rationalize the tasks so they all create the same one.  I prefer the larger result of including package access, but it is quite a bit larger. Anybody have preferences here?

One of the reasons to rationalize the targets is that Jenkins is now showing warnings from protected members that I hadn’t been seeing when building Javadoc locally.

Which brings up another item:

2) The warnings are all about missing @param or @return tags. The real Javadocs do have all those entered, having them is the standard form, and we agreed a long time ago to keep up with the Java formatting standards.  (Originally, Javadoc did not require these, and created them in minimalist form if missing; that stopped a long time ago, and we haven’t caught up)  So it’s reasonable to warn on those so we can fix them. I’m going to PR a few shortly. But we need to fix them all, and most of the warnings are now coming from the generated jjtree Java files. Two examples:

[WARNING] /Users/jake/Documents/Trains/JMRI/projects/JMRI/target/generated-sources/jjtree/jmri/jmrix/srcp/parser/Node.java:22: warning: no @param for n
[WARNING] public void jjtSetParent(Node n);
[WARNING] ^
[WARNING] /Users/jake/Documents/Trains/JMRI/projects/JMRI/target/generated-sources/jjtree/jmri/jmrix/srcp/parser/Node.java:27: warning: no @param for n
[WARNING] public void jjtAddChild(Node n, int i);
[WARNING] ^

I tried a couple of things to suppress those warnings from the automatically-built classes without success.  Does anybody know how to do that?

3) I’d like to confirm that our goal is to get these warnings down to zero, and the have CI reject PRs that are showing Javadoc warnings.  (It won’t be insisting on Javadoc, just on correct Javadoc; there’s no warning if there’s nothing on a method, only a warning when partial/incomplete) Anybody concerned about that?

Bob
--
Bob Jacobsen
rgj1927@...










--
Bob Jacobsen
rgj1927@...






Bob Jacobsen
 

Thanks again for the clue on processing /** to /* to avoid tripping Javadoc warnings.

I’ve got that working for the Ant targets by copying the JavaCC handler to JJTree.

But I haven’t found anything similar in pom.xml Any hints on how it’s done there?

Thanks again.

Bob

On May 19, 2020, at 12:55 PM, Randall Wood via groups.io <rhwood=mac.com@groups.io> wrote:

It seems to me that, for both maven and ant, the fail on warning is absolute — i.e. the javadoc process can be set to fail if there are 1 or more warnings, or to not fail on any warnings, but cannot be set to fail only if there are (for example) 10 or more warnings, but pass with 9 or less warnings.

Randall

On 19-May-2020, at 15:25, Bob Jacobsen <@BobJacobsen> wrote:

Thanks! That was the hint I needed. We had that in the JavaCC target, but not JJTree. Working on that now.

Now for another question:

Turns out that setting Xmaxwarns to 36 i.e.
<arg value="-Xdoclint:all"/><!-- remove -missing to get warnings about missing javadoc tags -->
<arg value="-Xmaxwarns"/>
<arg value="36"/><!-- change from default 100 warnings; keep synced with pom.xml, other javadoc targets —>

_limits_ the warnings, but doesn’t cause the task to fail if you go over that. I.e. there are _lots_ of warning-causing-conditions present in the code, but only 36 are displayed. That’s not great, as it gives a discouraging experience: Fix one, rerun Javadocs, get new ones showing up.

Can we convert this to “show them all, and fail if too many, and set ’too many’ to how many there are now’”? Does anybody know how to set such a limit?

Bob

On May 19, 2020, at 8:52 AM, Randall Wood via groups.io <rhwood=mac.com@groups.io> wrote:

I thought we had something to strip the second * in comments in the jjtree generated code.
On May 19, 2020, at 11:30, Bob Jacobsen <@BobJacobsen> wrote:

Something changed recently in the Javadoc processing. I noticed this because Jenkins started showing 35 Javadoc warnings.

Looking into this, I have a couple questions:

1) Some of our Javadoc build tasks build only public methods and members; others show protected and package access levels. Public is appropriate for a general API. Protected (or even package) is appropriate as a developer reference. I think we should rationalize the tasks so they all create the same one. I prefer the larger result of including package access, but it is quite a bit larger. Anybody have preferences here?

One of the reasons to rationalize the targets is that Jenkins is now showing warnings from protected members that I hadn’t been seeing when building Javadoc locally.

Which brings up another item:

2) The warnings are all about missing @param or @return tags. The real Javadocs do have all those entered, having them is the standard form, and we agreed a long time ago to keep up with the Java formatting standards. (Originally, Javadoc did not require these, and created them in minimalist form if missing; that stopped a long time ago, and we haven’t caught up) So it’s reasonable to warn on those so we can fix them. I’m going to PR a few shortly. But we need to fix them all, and most of the warnings are now coming from the generated jjtree Java files. Two examples:

[WARNING] /Users/jake/Documents/Trains/JMRI/projects/JMRI/target/generated-sources/jjtree/jmri/jmrix/srcp/parser/Node.java:22: warning: no @param for n
[WARNING] public void jjtSetParent(Node n);
[WARNING] ^
[WARNING] /Users/jake/Documents/Trains/JMRI/projects/JMRI/target/generated-sources/jjtree/jmri/jmrix/srcp/parser/Node.java:27: warning: no @param for n
[WARNING] public void jjtAddChild(Node n, int i);
[WARNING] ^

I tried a couple of things to suppress those warnings from the automatically-built classes without success. Does anybody know how to do that?

3) I’d like to confirm that our goal is to get these warnings down to zero, and the have CI reject PRs that are showing Javadoc warnings. (It won’t be insisting on Javadoc, just on correct Javadoc; there’s no warning if there’s nothing on a method, only a warning when partial/incomplete) Anybody concerned about that?

Bob
--
Bob Jacobsen
@BobJacobsen






--
Bob Jacobsen
@BobJacobsen



--
Bob Jacobsen
@BobJacobsen


Randall Wood <rhwood@...>
 

Actually, now that I look at, we don’t need to do that processing at all anymore.

We originally did that processing because the output of javac, javacc, and jjtree all went into the same directory structure the way we wrote build.xml, but now each of those compilers writes to its own directory structure, so we should be able to drop that special processing and modify line 677 in pom.xml* and remove lines 1821-1826 in build.xml** (and any similar blocks) to remove the javacc and jjtree results from the javadoc inputs.

On 21-May-2020, at 00:45, Bob Jacobsen <rgj1927@...> wrote:

Thanks again for the clue on processing /** to /* to avoid tripping Javadoc warnings.

I’ve got that working for the Ant targets by copying the JavaCC handler to JJTree.

But I haven’t found anything similar in pom.xml  Any hints on how it’s done there?

Thanks again.

Bob

On May 19, 2020, at 12:55 PM, Randall Wood via groups.io <rhwood@...> wrote:

It seems to me that, for both maven and ant, the fail on warning is absolute — i.e. the javadoc process can be set to fail if there are 1 or more warnings, or to not fail on any warnings, but cannot be set to fail only if there are (for example) 10 or more warnings, but pass with 9 or less warnings.

Randall

On 19-May-2020, at 15:25, Bob Jacobsen <rgj1927@...> wrote:

Thanks! That was the hint I needed.  We had that in the JavaCC target, but not JJTree.  Working on that now.

Now for another question:

Turns out that setting Xmaxwarns to 36 i.e.
          <arg value="-Xdoclint:all"/><!-- remove -missing to get warnings about missing javadoc tags -->
          <arg value="-Xmaxwarns"/>
          <arg value="36"/><!-- change from default 100 warnings; keep synced with pom.xml, other javadoc targets —>

_limits_ the warnings, but doesn’t cause the task to fail if you go over that.  I.e. there are _lots_ of warning-causing-conditions present in the code, but only 36 are displayed.  That’s not great, as it gives a discouraging experience:  Fix one, rerun Javadocs, get new ones showing up.

Can we convert this to “show them all, and fail if too many, and set ’too many’ to how many there are now’”?  Does anybody know how to set such a limit?

Bob

On May 19, 2020, at 8:52 AM, Randall Wood via groups.io <rhwood@...> wrote:

I thought we had something to strip the second * in comments in the jjtree generated code.
On May 19, 2020, at 11:30, Bob Jacobsen <rgj1927@...> wrote:

Something changed recently in the Javadoc processing. I noticed this because Jenkins started showing 35 Javadoc warnings.

Looking into this, I have a couple questions:

1) Some of our Javadoc build tasks build only public methods and members; others show protected and package access levels.  Public is appropriate for a general API.  Protected (or even package) is appropriate as a developer reference.  I think we should rationalize the tasks so they all create the same one.  I prefer the larger result of including package access, but it is quite a bit larger. Anybody have preferences here?

One of the reasons to rationalize the targets is that Jenkins is now showing warnings from protected members that I hadn’t been seeing when building Javadoc locally.

Which brings up another item:

2) The warnings are all about missing @param or @return tags. The real Javadocs do have all those entered, having them is the standard form, and we agreed a long time ago to keep up with the Java formatting standards.  (Originally, Javadoc did not require these, and created them in minimalist form if missing; that stopped a long time ago, and we haven’t caught up)  So it’s reasonable to warn on those so we can fix them. I’m going to PR a few shortly. But we need to fix them all, and most of the warnings are now coming from the generated jjtree Java files. Two examples:

[WARNING] /Users/jake/Documents/Trains/JMRI/projects/JMRI/target/generated-sources/jjtree/jmri/jmrix/srcp/parser/Node.java:22: warning: no @param for n
[WARNING] public void jjtSetParent(Node n);
[WARNING] ^
[WARNING] /Users/jake/Documents/Trains/JMRI/projects/JMRI/target/generated-sources/jjtree/jmri/jmrix/srcp/parser/Node.java:27: warning: no @param for n
[WARNING] public void jjtAddChild(Node n, int i);
[WARNING] ^

I tried a couple of things to suppress those warnings from the automatically-built classes without success.  Does anybody know how to do that?

3) I’d like to confirm that our goal is to get these warnings down to zero, and the have CI reject PRs that are showing Javadoc warnings.  (It won’t be insisting on Javadoc, just on correct Javadoc; there’s no warning if there’s nothing on a method, only a warning when partial/incomplete) Anybody concerned about that?

Bob
--
Bob Jacobsen
rgj1927@...










--
Bob Jacobsen
rgj1927@...







--
Bob Jacobsen
rgj1927@...






Bob Jacobsen
 

Thanks for looking into this!

Unfortunately, that generates a different set of errors for references can’t be resolved:

[WARNING] Javadoc Warnings
[WARNING] /Users/jake/Documents/Trains/JMRI/projects/JMRI/java/src/jmri/jmris/simpleserver/SimplePowerServer.java:9: error: cannot find symbol
[WARNING] import jmri.jmris.simpleserver.parser.JmriServerParser;
[WARNING] ^
[WARNING] symbol: class JmriServerParser
[WARNING] location: package jmri.jmris.simpleserver.parser
[WARNING] /Users/jake/Documents/Trains/JMRI/projects/JMRI/java/src/jmri/jmris/simpleserver/SimplePowerServer.java:10: error: cannot find symbol
[WARNING] import jmri.jmris.simpleserver.parser.ParseException;
[WARNING] ^
[WARNING] symbol: class ParseException
[WARNING] location: package jmri.jmris.simpleserver.parser
[WARNING] /Users/jake/Documents/Trains/JMRI/projects/JMRI/java/src/jmri/jmris/simpleserver/SimplePowerServer.java:11: error: cannot find symbol
[WARNING] import jmri.jmris.simpleserver.parser.SimpleNode;
[WARNING] ^
[WARNING] symbol: class SimpleNode

The problem seems to be that Javadoc effectively includes a compile step. That fails with these missing from the covered source directories. But _maybe_ it would be OK if they existed in a jar file on the class path, or even as .class files that had been previous generated. We do generate Javadoc with code that references Swing classes, etc, that we don’t have source and/or Javadocs for.

Unfortunately, I don’t understand enough about Maven and our pom.xml to be able to find where to control that.

Bob




On May 21, 2020, at 3:41 AM, Randall Wood via groups.io <rhwood=mac.com@groups.io> wrote:

Actually, now that I look at, we don’t need to do that processing at all anymore.

We originally did that processing because the output of javac, javacc, and jjtree all went into the same directory structure the way we wrote build.xml, but now each of those compilers writes to its own directory structure, so we should be able to drop that special processing and modify line 677 in pom.xml* and remove lines 1821-1826 in build.xml** (and any similar blocks) to remove the javacc and jjtree results from the javadoc inputs.

* https://github.com/JMRI/JMRI/blob/0c44bb57f911ef95555aab13f2c4c3b13bb3aa2b/pom.xml#L677
** https://github.com/JMRI/JMRI/blob/0c44bb57f911ef95555aab13f2c4c3b13bb3aa2b/build.xml#L1821-L1826

On 21-May-2020, at 00:45, Bob Jacobsen <@BobJacobsen> wrote:

Thanks again for the clue on processing /** to /* to avoid tripping Javadoc warnings.

I’ve got that working for the Ant targets by copying the JavaCC handler to JJTree.

But I haven’t found anything similar in pom.xml Any hints on how it’s done there?

Thanks again.

Bob

On May 19, 2020, at 12:55 PM, Randall Wood via groups.io <rhwood=mac.com@groups.io> wrote:

It seems to me that, for both maven and ant, the fail on warning is absolute — i.e. the javadoc process can be set to fail if there are 1 or more warnings, or to not fail on any warnings, but cannot be set to fail only if there are (for example) 10 or more warnings, but pass with 9 or less warnings.

Randall

On 19-May-2020, at 15:25, Bob Jacobsen <@BobJacobsen> wrote:

Thanks! That was the hint I needed. We had that in the JavaCC target, but not JJTree. Working on that now.

Now for another question:

Turns out that setting Xmaxwarns to 36 i.e.
<arg value="-Xdoclint:all"/><!-- remove -missing to get warnings about missing javadoc tags -->
<arg value="-Xmaxwarns"/>
<arg value="36"/><!-- change from default 100 warnings; keep synced with pom.xml, other javadoc targets —>

_limits_ the warnings, but doesn’t cause the task to fail if you go over that. I.e. there are _lots_ of warning-causing-conditions present in the code, but only 36 are displayed. That’s not great, as it gives a discouraging experience: Fix one, rerun Javadocs, get new ones showing up.

Can we convert this to “show them all, and fail if too many, and set ’too many’ to how many there are now’”? Does anybody know how to set such a limit?

Bob

On May 19, 2020, at 8:52 AM, Randall Wood via groups.io <rhwood=mac.com@groups.io> wrote:

I thought we had something to strip the second * in comments in the jjtree generated code.
On May 19, 2020, at 11:30, Bob Jacobsen <@BobJacobsen> wrote:

Something changed recently in the Javadoc processing. I noticed this because Jenkins started showing 35 Javadoc warnings.

Looking into this, I have a couple questions:

1) Some of our Javadoc build tasks build only public methods and members; others show protected and package access levels. Public is appropriate for a general API. Protected (or even package) is appropriate as a developer reference. I think we should rationalize the tasks so they all create the same one. I prefer the larger result of including package access, but it is quite a bit larger. Anybody have preferences here?

One of the reasons to rationalize the targets is that Jenkins is now showing warnings from protected members that I hadn’t been seeing when building Javadoc locally.

Which brings up another item:

2) The warnings are all about missing @param or @return tags. The real Javadocs do have all those entered, having them is the standard form, and we agreed a long time ago to keep up with the Java formatting standards. (Originally, Javadoc did not require these, and created them in minimalist form if missing; that stopped a long time ago, and we haven’t caught up) So it’s reasonable to warn on those so we can fix them. I’m going to PR a few shortly. But we need to fix them all, and most of the warnings are now coming from the generated jjtree Java files. Two examples:

[WARNING] /Users/jake/Documents/Trains/JMRI/projects/JMRI/target/generated-sources/jjtree/jmri/jmrix/srcp/parser/Node.java:22: warning: no @param for n
[WARNING] public void jjtSetParent(Node n);
[WARNING] ^
[WARNING] /Users/jake/Documents/Trains/JMRI/projects/JMRI/target/generated-sources/jjtree/jmri/jmrix/srcp/parser/Node.java:27: warning: no @param for n
[WARNING] public void jjtAddChild(Node n, int i);
[WARNING] ^

I tried a couple of things to suppress those warnings from the automatically-built classes without success. Does anybody know how to do that?

3) I’d like to confirm that our goal is to get these warnings down to zero, and the have CI reject PRs that are showing Javadoc warnings. (It won’t be insisting on Javadoc, just on correct Javadoc; there’s no warning if there’s nothing on a method, only a warning when partial/incomplete) Anybody concerned about that?

Bob
--
Bob Jacobsen
@BobJacobsen






--
Bob Jacobsen
@BobJacobsen



--
Bob Jacobsen
@BobJacobsen



--
Bob Jacobsen
@BobJacobsen


Randall Wood <rhwood@...>
 

There is a PR on your branch to fix this by excluding the package names in pom.xml. There is also a "excludepackagenames” parameter for ant’s javadoc task that has the same effect (both of these implement the javadoc tool -exclude option) although I did not implement it.

On 21-May-2020, at 13:33, Bob Jacobsen <rgj1927@...> wrote:

Thanks for looking into this!

Unfortunately, that generates a different set of errors for references can’t be resolved:

[WARNING] Javadoc Warnings
[WARNING] /Users/jake/Documents/Trains/JMRI/projects/JMRI/java/src/jmri/jmris/simpleserver/SimplePowerServer.java:9: error: cannot find symbol
[WARNING] import jmri.jmris.simpleserver.parser.JmriServerParser;
[WARNING] ^
[WARNING] symbol:   class JmriServerParser
[WARNING] location: package jmri.jmris.simpleserver.parser
[WARNING] /Users/jake/Documents/Trains/JMRI/projects/JMRI/java/src/jmri/jmris/simpleserver/SimplePowerServer.java:10: error: cannot find symbol
[WARNING] import jmri.jmris.simpleserver.parser.ParseException;
[WARNING] ^
[WARNING] symbol:   class ParseException
[WARNING] location: package jmri.jmris.simpleserver.parser
[WARNING] /Users/jake/Documents/Trains/JMRI/projects/JMRI/java/src/jmri/jmris/simpleserver/SimplePowerServer.java:11: error: cannot find symbol
[WARNING] import jmri.jmris.simpleserver.parser.SimpleNode;
[WARNING] ^
[WARNING] symbol:   class SimpleNode

The problem seems to be that Javadoc effectively includes a compile step.  That fails with these missing from the covered source directories.  But _maybe_ it would be OK if they existed in a jar file on the class path, or even as .class files that had been previous generated. We do generate Javadoc with code that references Swing classes, etc, that we don’t have source and/or Javadocs for.

Unfortunately, I don’t understand enough about Maven and our pom.xml to be able to find where to control that. 

Bob




On May 21, 2020, at 3:41 AM, Randall Wood via groups.io <rhwood@...> wrote:

Actually, now that I look at, we don’t need to do that processing at all anymore.

We originally did that processing because the output of javac, javacc, and jjtree all went into the same directory structure the way we wrote build.xml, but now each of those compilers writes to its own directory structure, so we should be able to drop that special processing and modify line 677 in pom.xml* and remove lines 1821-1826 in build.xml** (and any similar blocks) to remove the javacc and jjtree results from the javadoc inputs.

* https://github.com/JMRI/JMRI/blob/0c44bb57f911ef95555aab13f2c4c3b13bb3aa2b/pom.xml#L677
** https://github.com/JMRI/JMRI/blob/0c44bb57f911ef95555aab13f2c4c3b13bb3aa2b/build.xml#L1821-L1826

On 21-May-2020, at 00:45, Bob Jacobsen <rgj1927@...> wrote:

Thanks again for the clue on processing /** to /* to avoid tripping Javadoc warnings.

I’ve got that working for the Ant targets by copying the JavaCC handler to JJTree.

But I haven’t found anything similar in pom.xml  Any hints on how it’s done there?

Thanks again.

Bob

On May 19, 2020, at 12:55 PM, Randall Wood via groups.io <rhwood@...> wrote:

It seems to me that, for both maven and ant, the fail on warning is absolute — i.e. the javadoc process can be set to fail if there are 1 or more warnings, or to not fail on any warnings, but cannot be set to fail only if there are (for example) 10 or more warnings, but pass with 9 or less warnings.

Randall

On 19-May-2020, at 15:25, Bob Jacobsen <rgj1927@...> wrote:

Thanks! That was the hint I needed.  We had that in the JavaCC target, but not JJTree.  Working on that now.

Now for another question:

Turns out that setting Xmaxwarns to 36 i.e.
         <arg value="-Xdoclint:all"/><!-- remove -missing to get warnings about missing javadoc tags -->
         <arg value="-Xmaxwarns"/>
         <arg value="36"/><!-- change from default 100 warnings; keep synced with pom.xml, other javadoc targets —>

_limits_ the warnings, but doesn’t cause the task to fail if you go over that.  I.e. there are _lots_ of warning-causing-conditions present in the code, but only 36 are displayed.  That’s not great, as it gives a discouraging experience:  Fix one, rerun Javadocs, get new ones showing up.

Can we convert this to “show them all, and fail if too many, and set ’too many’ to how many there are now’”?  Does anybody know how to set such a limit?

Bob

On May 19, 2020, at 8:52 AM, Randall Wood via groups.io <rhwood@...> wrote:

I thought we had something to strip the second * in comments in the jjtree generated code.
On May 19, 2020, at 11:30, Bob Jacobsen <rgj1927@...> wrote:

Something changed recently in the Javadoc processing. I noticed this because Jenkins started showing 35 Javadoc warnings.

Looking into this, I have a couple questions:

1) Some of our Javadoc build tasks build only public methods and members; others show protected and package access levels.  Public is appropriate for a general API.  Protected (or even package) is appropriate as a developer reference.  I think we should rationalize the tasks so they all create the same one.  I prefer the larger result of including package access, but it is quite a bit larger. Anybody have preferences here?

One of the reasons to rationalize the targets is that Jenkins is now showing warnings from protected members that I hadn’t been seeing when building Javadoc locally.

Which brings up another item:

2) The warnings are all about missing @param or @return tags. The real Javadocs do have all those entered, having them is the standard form, and we agreed a long time ago to keep up with the Java formatting standards.  (Originally, Javadoc did not require these, and created them in minimalist form if missing; that stopped a long time ago, and we haven’t caught up)  So it’s reasonable to warn on those so we can fix them. I’m going to PR a few shortly. But we need to fix them all, and most of the warnings are now coming from the generated jjtree Java files. Two examples:

[WARNING] /Users/jake/Documents/Trains/JMRI/projects/JMRI/target/generated-sources/jjtree/jmri/jmrix/srcp/parser/Node.java:22: warning: no @param for n
[WARNING] public void jjtSetParent(Node n);
[WARNING] ^
[WARNING] /Users/jake/Documents/Trains/JMRI/projects/JMRI/target/generated-sources/jjtree/jmri/jmrix/srcp/parser/Node.java:27: warning: no @param for n
[WARNING] public void jjtAddChild(Node n, int i);
[WARNING] ^

I tried a couple of things to suppress those warnings from the automatically-built classes without success.  Does anybody know how to do that?

3) I’d like to confirm that our goal is to get these warnings down to zero, and the have CI reject PRs that are showing Javadoc warnings.  (It won’t be insisting on Javadoc, just on correct Javadoc; there’s no warning if there’s nothing on a method, only a warning when partial/incomplete) Anybody concerned about that?

Bob
--
Bob Jacobsen
rgj1927@...










--
Bob Jacobsen
rgj1927@...







--
Bob Jacobsen
rgj1927@...







--
Bob Jacobsen
rgj1927@...