[BUG] javadoc warnings not causing failures

[andrross]

We set the -Werror flag for all projects: https://github.com/opensearch-project/OpenSearch/blob/a24e9626fe3dbe64e929dd48ef73c4a5f7ddef8e/build.gradle#L275

However, it has been observed that these warnings cause compilation failures in Java 14, but not Java 17 (or 19). I believe the intent is to fail the build if warnings are present, but that is not happening.

Related:

5984

3936

[reta]

@andrross I think there is some confusion with that:

> Task :server:compileJava
/local/home/kaituo/code/github/OpenSearch/server/src/main/java/org/opensearch/index/translog/TranslogManager.java:101: warning: no description for @throws
     * @throws IOException
       ^
/local/home/kaituo/code/github/OpenSearch/server/src/main/java/org/opensearch/index/translog/TranslogManager.java:107: warning: no description for @param
     * @param operation

Those are javadoc warnings and if we want to fail the build we would need something like that [1]:

javadoc.options.addStringOption('Xwerror', '-quiet')

[1] https://bugs.openjdk.org/browse/JDK-8200363

[reta]

PS: Feel free to assign it to me, should be a trivial fix, thank you!

[andrross]

Given that we currently don't have any Javadoc warnings (other than the few on 2.x that are being fixed with #5994), I think we should go ahead and enable the option to fail the build so that we don't add more warnings. Thanks @reta!

[dbwiddis]

FYI, there was a big change in JDK 18+ wherein a (non-existent) default (no-arg) constructor requires a javadoc.

See also #4541 for a more fine-tuned approach than the antiquated lucene/solr-copied one we're using.

[reta]

Oh, totally forgot about https://github.com/opensearch-project/OpenSearch/issues/4541, thank you @dbwiddis !

[tlfeng]

Although the issue seems to be fixed, I can still see compile errors about lacking description in javadoc only when using JDK 14

> Task :server:compileTestJava FAILED
/home/user/github/OpenSearch/server/src/test/java/org/opensearch/common/lucene/LuceneTests.java:333: warning: no description for @throws
     * @throws IOException
       ^
error: warnings found and -Werror specified
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
1 error
1 warning

https://github.com/opensearch-project/OpenSearch/blob/1f4cdd2f75ca5e98bd81f3226566d263ec00455c/server/src/test/java/org/opensearch/common/lucene/LuceneTests.java#L333

> Task :server:compileInternalClusterTestJava
/home/user/github/OpenSearch/server/src/internalClusterTest/java/org/opensearch/search/SearchWeightedRoutingIT.java:844: warning: no description for @throws
     * @throws Exception
       ^
error: warnings found and -Werror specified
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
1 error
1 warning

https://github.com/opensearch-project/OpenSearch/blob/1f4cdd2f75ca5e98bd81f3226566d263ec00455c/server/src/internalClusterTest/java/org/opensearch/search/SearchWeightedRoutingIT.java#L844

[andrross]

@tlfeng Java 14 has been end of life for over two years. Is there any need to support compiling with JDK 14?

[tlfeng]

Haha, there is totally no need to build the project with JDK14. I forgot to change the JDK version at sometime, and version 14 used to build ES 7.10. I thought the objective for this issue is to "fail the build if warnings are present" when using other JDK versions, but the above 2 javadoc warnings didn't fail the build in JDK 11 or 17.

[reta]

Haha, there is totally no need to build the project with JDK14. I forgot to change the JDK version at sometime, and version 14 used to build ES 7.10.

😄 there were a number of bug (and fixes) for javadoc for different JDKs, so this is probably fine that some old ones do not work as expected

[dblock]

The issue was that we wanted to fail compilation with these warnings. Looks like @reta did it in https://github.com/opensearch-project/OpenSearch/pull/5996. I put a @throws into server/src/main/java/org/opensearch/index/translog/TranslogManager.java without a description and got a proper error.

$ ./gradlew :server:compileJava
Picked up JAVA_TOOL_OPTIONS: -Dlog4j2.formatMsgNoLookups=true
Starting a Gradle Daemon, 2 busy Daemons could not be reused, use --status for details

> Configure project :qa:os
Cannot add task 'destructiveDistroTest.docker' as a task with that name already exists.
=======================================
OpenSearch Build Hamster says Hello!
  Gradle Version        : 7.6
  OS Info               : Mac OS X 12.6.1 (aarch64)
  JDK Version           : 17 (Eclipse Temurin JDK)
  JAVA_HOME             : /Library/Java/JavaVirtualMachines/temurin-17.jdk/Contents/Home
  Random Testing Seed   : D417A58AC7FD101C
  In FIPS 140 mode      : false
=======================================

> Task :server:compileJava
Picked up JAVA_TOOL_OPTIONS: -Dlog4j2.formatMsgNoLookups=true
/Users/dblock/source/opensearch-project/OpenSearch/dblock-OpenSearch/server/src/main/java/org/opensearch/index/translog/TranslogManager.java:106: error: syntax error in reference
     * @throws
       ^
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
1 error

> Task :server:compileJava FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':server:compileJava'.
> Compilation failed; see the compiler error output for details.

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.

* Get more help at https://help.gradle.org

BUILD FAILED in 31s
21 actionable tasks: 1 executed, 20 up-to-date

[andrross]

@dblock I think things are working as expected. What @tlfeng has uncovered is that JDK 14 appears to enforce some javadoc rules against tests (which we do not want).

[tlfeng]

Ah, I see. @andrross Thank you for the clarification! If the build don't need to fail due to javadocs warning in tests, then there is no issue. My comment above can be ignored, because the 2 warnings I mentioned above are all in test classes. Anyway, the 2 empty throws annotation in the javadocs are removed in a new commit https://github.com/opensearch-project/OpenSearch/commit/4f5fd9c4d9ab3bf92a010e659a07b8a372f10ecd 😄. Thanks @andrross pointing out.