Project

General

Profile

Bug #11342

Clean javadoc warnings

Added by Razvan-Nicolae Chichirau 4 months ago. Updated 3 months ago.

Status:
Test
Priority:
Normal
Assignee:
Razvan-Nicolae Chichirau
Target version:
-
Start date:
Due date:
% Done:

100%

billable:
No
vendor_id:
GCD
case_num:
version_reported:
version_resolved:
reviewer:
Ovidiu Maxiniuc, Radu Apetrii
production:
No
env_name:
topics:

History

#1 Updated by Razvan-Nicolae Chichirau 4 months ago

  • Assignee set to Razvan-Nicolae Chichirau

As of trunk rev. 16484, there are 330 warning messages generated by the javadoc task. These are listed here:

The scope of this issue is to clean these javadoc warnings such that the gradle task does not output any errors.

#2 Updated by Razvan-Nicolae Chichirau 4 months ago

  • Status changed from New to WIP
  • % Done changed from 0 to 100
  • reviewer Ovidiu Maxiniuc added

Ovidiu: Please review 11342a/rev. 16494.

This revision should fix all warnings generated by javadoc and javac. For the compiler-related warnings, we had to convert the null to an Object[] to avoid ambiguity. Functionally, it is the same, and you can test through this little program:

public static void main(String[] args)
{
   abc(1, null);
   abc(1, (Object[]) null);
}

public static void abc(int a, Object... args)
{
  System.out.print(a + " ");
  if (args == null)
  {
     System.out.println("Args is null");
  }
  else
  {
     System.out.println(Arrays.toString(args));
  }
}

#3 Updated by Razvan-Nicolae Chichirau 4 months ago

  • Status changed from WIP to Review

#4 Updated by Ovidiu Maxiniuc 4 months ago

Review 11342a / r16494

This is a nice update. Not only that a code which compile without warnings will make it easier for us to quickly spot possible future regressions, but it will also show to our customers that we take good care of the product.
I also like that, beside the missing parameters, the description was fixed and improved for some of existing ones.

  • ArtifactManager.java
    • at 2nd line, the name of the file is incorrect (extra "s");
  • BorderType.java, FormControl.java, ArtifactWorker.java, SpreadsheetPushServlet.java, SpreadsheetWidget.java:
    • Please also remove the space at the start of the header lines (for both history and copyright sections).
  • SyncConfigChangesAspect.java: line 88: {@link ConfigSyncManager#markScopeEnd(ConfigManager)}
  • ConversionDriver.java: line 1237: I think the intended form was {@code <file-specs>} instead of &lt;file-specs&gt;. See below, other similar occurrences;
  • ProgressTransformDriver.java: line 253: similar as above;
  • TransformDriver.java: line 788: {@code failedSources} should be used. Drop off the #, failedSources is the first parameter of the method.
  • DBUtils.java: line 202: move the first line of the field description on next line, leaving /** simple;
  • Query.java:
    • line 113: the public class lacks its javadoc. Please add the following text:
      A class that acts as a factory for a wrapped SQL statement, based on FQL predicate, a list of parameters
and other SQL specific options. It allows the predicate to be rewritten in order to support compatibility
with 4GL idioms.
      Additional information is welcomed.
    • line 172: typo: developonet :-);
    • line 178: an empty line is required before @param list.
  • RestHandler.java: line 857: an empty line is required before @param list;
  • FuzzyMethodCvt.java: line 206/207: I think the intention was to have those expressions written in {@code tag, which would allow the < and > to be written naturally. Ex: {@code object<? extends _CLS_>};
  • FuzzyMethodRt: same for lines 427/428;
  • LexerOptions.java: since there a no JPRM, this column can be dropped from the history header section;
  • progress.g; lines 11719/11722/3: the < / > are still present for buffer. Let's use {@code, as above (FuzzyMethodCvt);
  • Settable.java: let's upgrade this ancient history header section to latest format:
    • drop column -T- - it is useless
    • expand the Description to fit the available space up to 110 columns limit;
  • Widget.java: line 512: extra )} was added;
  • IVBDataObject.java and IVBDataObjectFiles.java:
    • lines 73-77. <li> idea is good, but let's use {@code tag for items, instead of old HTML escape style. It looks much better in final doc and it's also easier to read in Java source code;
  • ConfigItem.java: lines 227-237: why the mixed approach? This is strange. Let's wrap the full XML directory section in a code block.
  • FileSystemOps.java: lines 1238 and 1277: see above ({@code tag).
  • SourceNameMapper.java same for line 418;
  • CustomXMLEscapes.java same for lines 73, 80 and 97;
  • comhandle.java: line 1326: I think the intention was to refer the parameter but code was skipped, so {@code value}.
Other things to check:
  • presence of 0x09 (hard TAB characters). As noted in coding style, these should be replaced with 3 spaces. Depending on the specific location, the number may be different to get the proper indentation, depending on the editor settings of the author. At least for .java sources and .rules and .xml scripts in TRPL.

#5 Updated by Razvan-Nicolae Chichirau 4 months ago

Ovidiu, thanks for the detailed review!
Rebased 11342a to trunk rev. 16505. Please check rev. 16507.

Other things to check:

presence of 0x09 (hard TAB characters). As noted in coding style, these should be replaced with 3 spaces. Depending on the specific location, the number may be different to get the proper indentation, depending on the editor settings of the author. At least for .java sources and .rules and .xml scripts in TRPL.

Checked my IDE and when pressing the hard TAB character, 3 spaces (soft tabs) are inserted and not the actual 0x09 character.

#6 Updated by Ovidiu Maxiniuc 4 months ago

  • Status changed from Review to Internal Test

Looks good. I built clean javadoc and I can confirm there are no more warnings.

Please ignore my note on TAB characters. I did not said they were introduced in this branch. Sorry if the note implied it. There are some of these characters in the trunk, scattered across the project. Let's leave them for now since they are not so visible. We will remove them when they are encountered with logic code changes.

I think this branch can be directly merged. There is no logic code directly affected (except maybe for the cast from 2nd note). The compiled .class code should not be change at all (that can be actually tested with a binary compare tool, if the debug info is stripped).

#7 Updated by Razvan-Nicolae Chichirau 3 months ago

Using the example from #11342-2, I used the javap tool to disassemble and analyze the bytecode.
  • Calling abc(1, null) yields:
    2: invokestatic  #7                  // Method abc:(I[Ljava/lang/Object;)V
  • Calling abc(1, (Object[]) null) yields:
    2: checkcast     #7                  // class "[Ljava/lang/Object;" 
5: invokestatic  #9                  // Method abc:(I[Ljava/lang/Object;)V

The only difference is a checkcast instruction which verifies if null can be cast to Object[]. This is always true, since null can be converted to any object or single/multi-array of objects. So, the consequence is that there is no functional difference.

As such, there is nothing to test. 11342a should be merged.

#8 Updated by Razvan-Nicolae Chichirau 3 months ago

Rebased 11342a to trunk rev. 16510. Unfortunately, there are 7 new warnings:

> Task :javadoc
/media/rnc/Lexar/gcd/trunk/src/com/goldencode/p2j/util/RemoteStream.java:251: warning: Tag @link: reference not found: #cachedIsIn
    * Reset the {@link #cachedIsIn} state of this stream.
                ^
/media/rnc/Lexar/gcd/trunk/src/com/goldencode/p2j/util/RemoteStream.java:251: warning: Tag @link: reference not found: #cachedIsIn
    * Reset the {@link #cachedIsIn} state of this stream.
                ^
/media/rnc/Lexar/gcd/trunk/src/com/goldencode/p2j/util/osresource/ServerFileSystemDaemon.java:70: warning: Tag @link: reference not found: FileSystemDaemon$WorkArea
 * Server-side support for file-system.  This allows a context-local {@link FileSystemDaemon$WorkArea}.
                                                                     ^
/media/rnc/Lexar/gcd/trunk/src/com/goldencode/p2j/util/ServerStreamDaemon.java:71: warning: Tag @link: reference not found: StreamDaemon$WorkArea
 * Server-side support for streams.  This allows a context-local {@link StreamDaemon$WorkArea}.
                                                                 ^
/media/rnc/Lexar/gcd/trunk/src/com/goldencode/p2j/util/StreamDaemon.java:1586: warning: @param argument "s" is not a parameter name.
    * @param    s
      ^
/media/rnc/Lexar/gcd/trunk/src/com/goldencode/p2j/util/osresource/StreamHelper.java:101: warning: @param argument "s" is not a parameter name.
    * @param    s
      ^
/media/rnc/Lexar/gcd/trunk/src/com/goldencode/p2j/util/RemoteStream.java:251: warning: Tag @link: reference not found: #cachedIsIn
    * Reset the {@link #cachedIsIn} state of this stream.
                ^
7 warnings

I'll fix these and we should merge ASAP.

#9 Updated by Razvan-Nicolae Chichirau 3 months ago

  • Status changed from Internal Test to Review
  • reviewer Radu Apetrii added

Radu: Please review 11342a/rev. 16513, as I think Ovidiu is not available right now. There are some doc changes, nothing special.

If the review is OK, let's merge it straight away as another trunk merges may bring additional warnings.

#10 Updated by Radu Apetrii 3 months ago

  • Status changed from Review to Merge Pending

I don't have any objections related to 11342a rev.16513.

Since this doesn't require testing, I can say that 11342a can be merged right now.

#11 Updated by Razvan-Nicolae Chichirau 3 months ago

  • Status changed from Merge Pending to Test

Branch 11342a was merged into trunk as rev. 16512 and archived.

Also available in: Atom PDF