[ https://issues.apache.org/jira/browse/MJAVADOC-452?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15306998#comment-15306998 ]
Richard Sand commented on MJAVADOC-452: --------------------------------------- Hi Robert, its a pain to split it up, is it ok if I walk through the changes in the patch file? Here is a description of every change by line # (the line # in the patch file that is): 17: added boolean config param "removeUnknownThrows" for removing unknown throws, default is false 30: added boolean config param "maven.javadoc.fix.skip" for skipping the fix goal, default is false 43: inside execute() method, added check for skip=true 57: in properties passed into CLIRR invocation, added "maven.javadoc.fix.skip=true" to prevent CLIRR from invoking fix and looping [I understand you don't like that, but it's simple and straightforward] 68: added capability to detect if any actual changes have been made to the contents of the file before saving - line 68 initializes a new BufferedReader for holding the contents for comparison 75: reads the output file into our BufferedReader 86: closes the new BufferedReader 101: changed the log message from "fixing" to "analyzing", since here we are only looking for fixes, later we will log if any fixes were performed 112: moved "String line" variable declaration into the loop itself 121: added a missing "close" 130: does a line-by-line comparison of the original content to the new content 136: added a log statement saying that we were saving changes to this file because changes were made 148: no code change here, just changed the formatting so each method was on its own line 169: moved "addMissingJavadocTags" above "updateJavadocTags" - this is because we added a new method "writeJavadocTags" which does the actual output, so addMissingJavadocTags doesn't actually output anything, it just adds the missing tags to the list 177: added writeJavadocTags 185: we are inside parseJavadocTags - we are now passing the javaEntityTags object into getJavadocComment 198: here instead of directly putting the throws tag into the javaEntityTags, we call fixThrowsTag 207-312: the method updateJavadocTags has been modified a fair amount. First, it has been renamed to writeJavadocTags - this method writes all of the tags provided, and uniformly ensures that every tag ends in an EOL with no extra whitespaces trailing. The main change is its handling specifically of methods comments. Here, it uses the javaEntityTags to find all of the tags it needs to write, since we have ensured that the javaEntityTags are populated with everything that the comment should have 314: the method writeParamTag is now passed in the 1 specific tag it is supposed to be writing, instead of the array of all parameters 318: removed call to fixQdox because that was called outside of writeParamTag 329: changed log message from "fixed unknown param" to "removed unknown param" to provide a clearer message 338: we renamed the method "writeThrowsTag" to "fixThrowsTag" because this method no longer writes any output, but just fixes the tag within the javaEntityTags. we added a new "writeThrowsTag" later to do the actual output writing 354: instead of writing the tag, put it into javaEntityTags 360-371: no code changes, just formatting 381-386: we don't output the tag, just update the String and put it back into the javaEntityTags 392: add check for "removeUnknownThrows" 394-396: updated log message to explicitly state that the throws is being ignored (removed) 399-405: updated log message to explicitly state that an unknown throws was found but is being included 409-441: new method "writeThrowsTag" which does the actual output 455-456: addMissingJavadocTags no longer needs a StringBuffer since it no longer does any output writing. Instead any changes it makes are added to the javaEntityTags 464-467: instead of writing to the output, we create a new javaEntityTags entry [java parameter] 475-478: instead of writing to the output, we create a new javaEntityTags entry [type parameter] 486-489: instead of writing to the output, we create a new javaEntityTags entry [return tag] 497-500: instead of writing to the output, we create a new javaEntityTags entry [throws tag] 508-511: instead of writing to the output, we create a new javaEntityTags entry [author tag] 515-518: instead of writing to the output, we create a new javaEntityTags entry [version tag] 526-529: instead of writing to the output, we create a new javaEntityTags entry [since tag, new class] 534-538: instead of writing to the output, we create a new javaEntityTags entry [since tag, existing class] 546-549: instead of writing to the output, we create a new javaEntityTags entry [since tag, new method] 557-560: instead of writing to the output, we create a new javaEntityTags entry [since tag, existing] 568: added missing close 578: added missing close 587-592: because we modified "getJavadocComment" to add the javaEntityTags parameter, specifically for dealing with method comments, we add this invocation with null for javaEntityTags to preserve existing invocations 600,605-606: here we add javaEntityTags to getJavadocComment, which allows us to make sure that the same parameter isn't output multiple times 614-624: here we make sure that the tag currently being processed hasn't already been added to the javaEntityTags 630-666: here we add the logic to make sure that two parameters with overlapping names don't conflict, e.g. "@param something" and @param "somethingelse", and if the same tag exists twice, e.g. "@param something", "@param something" > Several fixes for comment corruption in fix goal > ------------------------------------------------ > > Key: MJAVADOC-452 > URL: https://issues.apache.org/jira/browse/MJAVADOC-452 > Project: Maven Javadoc Plugin > Issue Type: Bug > Components: javadoc > Affects Versions: 2.10.3 > Reporter: Richard Sand > Labels: patch > Attachments: maven-javadoc-plugin-MJAVADOC452.patch > > > I've attached a patch to this issue that fixes several problems in > javadoc:fix, including issue MJAVADOC-420,MJAVADOC-434, and MJAVADOC-451, and > adds some improvements to the fix goal. > 1) added a new config parameter "removeUnknownThrows", to tell the plugin to > remove a @throws tag if the exception class is not actually thrown. > Previously it only warned. I set the default value to "false" to preserve > existing behavior > 2) in "processFix", I added a check to see if the content had actually been > changed before writing to disk. This preserves file timestamps and provides a > clear indication if any changes were actually made > 3) a problem in "updateJavadocTags" for method comments specifically, where, > if a throws tag was unknown, it would be written at the bottom of the > comments e.g. after @return, @since etc. I fixed updateJavadocTags so that > method tags are always output in the prescribed order > 4) I fixed issue MJAVADOC-420, where if two method parameters existed with > overlapping names (e.g. @param thing @param thing2), then the second > parameter would be added twice > 5) there was an issue if a comment erroneously contained the same parameter > twice (.e.g @param thing, @param thing), where both tags would be duplicated, > and all instances would have the description of the first. this was fixed in > "updateJavadocTags" and "addMissingJavadocTags" > 6) Includes the same fix as attached to MJAVADOC-451, which adds a "skip" > parameter, which is useful when used in conjunction with "force". Also this > parameter prevents "clirr" from causing javadoc:fix from being re-invoked > when clirr is kicked off, which would cause an infinite loop of > fix-->clirr-->fix-->clirr. This scenario is prevented by internally passing > the "skip" parameter into the invocation of clirr > I believe the patch will also resolve MJAVADOC-434, corrupted files > (basically the end-comment tag was left in the middle of the comment block > causing syntax errors) because I also observed this behavior and I don't now > after the patch, but I didn't actually trace down the root cause of 434 to be > certain > Lastly I updated some of the log messages to be more descriptive and > indicative of the actual activity (e.g. changed "fixing" to "analyzing"). > Now that the tags are output in the prescribed order, two of the JUnit test > "expected" java output files needed to be updated to put the @return tag > below their @param tags > The attached patch file updates AbstractFixJavadocMojo.jar, and two test > resources. All JUnit tests pass successfully. The patch was generated against > the 2.10.3 tag in SVN. -- This message was sent by Atlassian JIRA (v6.3.4#6332)