You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by rf...@apache.org on 2016/07/09 09:34:18 UTC
svn commit: r1751993 - in /maven/plugins/trunk/maven-javadoc-plugin: pom.xml
src/main/java/org/apache/maven/plugin/javadoc/AbstractFixJavadocMojo.java
src/test/java/org/apache/maven/plugin/javadoc/FixJavadocMojoTest.java
Author: rfscholte
Date: Sat Jul 9 09:34:18 2016
New Revision: 1751993
URL: http://svn.apache.org/viewvc?rev=1751993&view=rev
Log:
[MJAVADOC-452] Several fixes for comment corruption in fix goal
Only write file if there's actually an updateEntityComment
Patch contributed by Richard Sand; reviewed and applied with small adjustments by Robert Scholte
Modified:
maven/plugins/trunk/maven-javadoc-plugin/pom.xml
maven/plugins/trunk/maven-javadoc-plugin/src/main/java/org/apache/maven/plugin/javadoc/AbstractFixJavadocMojo.java
maven/plugins/trunk/maven-javadoc-plugin/src/test/java/org/apache/maven/plugin/javadoc/FixJavadocMojoTest.java
Modified: maven/plugins/trunk/maven-javadoc-plugin/pom.xml
URL: http://svn.apache.org/viewvc/maven/plugins/trunk/maven-javadoc-plugin/pom.xml?rev=1751993&r1=1751992&r2=1751993&view=diff
==============================================================================
--- maven/plugins/trunk/maven-javadoc-plugin/pom.xml (original)
+++ maven/plugins/trunk/maven-javadoc-plugin/pom.xml Sat Jul 9 09:34:18 2016
@@ -95,6 +95,9 @@ under the License.
<contributor>
<name>Omair Majid</name>
</contributor>
+ <contributor>
+ <name>Richard Sand</name>
+ </contributor>
</contributors>
<dependencies>
Modified: maven/plugins/trunk/maven-javadoc-plugin/src/main/java/org/apache/maven/plugin/javadoc/AbstractFixJavadocMojo.java
URL: http://svn.apache.org/viewvc/maven/plugins/trunk/maven-javadoc-plugin/src/main/java/org/apache/maven/plugin/javadoc/AbstractFixJavadocMojo.java?rev=1751993&r1=1751992&r2=1751993&view=diff
==============================================================================
--- maven/plugins/trunk/maven-javadoc-plugin/src/main/java/org/apache/maven/plugin/javadoc/AbstractFixJavadocMojo.java (original)
+++ maven/plugins/trunk/maven-javadoc-plugin/src/main/java/org/apache/maven/plugin/javadoc/AbstractFixJavadocMojo.java Sat Jul 9 09:34:18 2016
@@ -976,11 +976,12 @@ public abstract class AbstractFixJavadoc
if ( getLog().isDebugEnabled() )
{
- getLog().debug( "Fixing " + javaClass.getFullyQualifiedName() );
+ getLog().debug( "Analyzing " + javaClass.getFullyQualifiedName() );
}
final StringWriter stringWriter = new StringWriter();
BufferedReader reader = null;
+ boolean changeDetected = false;
try
{
reader = new BufferedReader( new StringReader( originalContent ) );
@@ -997,7 +998,7 @@ public abstract class AbstractFixJavadoc
{
if ( lineNumber == javaClass.getAnnotations()[0].getLineNumber() )
{
- fixClassComment( stringWriter, originalContent, javaClass, indent );
+ changeDetected |= fixClassComment( stringWriter, originalContent, javaClass, indent );
takeCareSingleComment( stringWriter, originalContent, javaClass );
}
@@ -1006,7 +1007,7 @@ public abstract class AbstractFixJavadoc
{
if ( lineNumber == javaClass.getLineNumber() )
{
- fixClassComment( stringWriter, originalContent, javaClass, indent );
+ changeDetected |= fixClassComment( stringWriter, originalContent, javaClass, indent );
takeCareSingleComment( stringWriter, originalContent, javaClass );
}
@@ -1015,13 +1016,11 @@ public abstract class AbstractFixJavadoc
// fixing fields
if ( javaClass.getFields() != null )
{
- for ( int i = 0; i < javaClass.getFields().length; i++ )
+ for ( JavaField field : javaClass.getFields() )
{
- JavaField field = javaClass.getFields()[i];
-
if ( lineNumber == field.getLineNumber() )
{
- fixFieldComment( stringWriter, javaClass, field, indent );
+ changeDetected |= fixFieldComment( stringWriter, javaClass, field, indent );
}
}
}
@@ -1029,13 +1028,11 @@ public abstract class AbstractFixJavadoc
// fixing methods
if ( javaClass.getMethods() != null )
{
- for ( int i = 0; i < javaClass.getMethods().length; i++ )
+ for ( JavaMethod method : javaClass.getMethods() )
{
- JavaMethod method = javaClass.getMethods()[i];
-
if ( lineNumber == method.getLineNumber() )
{
- fixMethodComment( stringWriter, originalContent, method, indent );
+ changeDetected |= fixMethodComment( stringWriter, originalContent, method, indent );
takeCareSingleComment( stringWriter, originalContent, method );
}
@@ -1054,20 +1051,30 @@ public abstract class AbstractFixJavadoc
IOUtil.close( reader );
}
- if ( getLog().isDebugEnabled() )
+ if ( changeDetected )
{
- getLog().debug( "Saving " + javaClass.getFullyQualifiedName() );
- }
+ if ( getLog().isInfoEnabled() )
+ {
+ getLog().info( "Saving changes to " + javaClass.getFullyQualifiedName() );
+ }
- if ( outputDirectory != null && !outputDirectory.getAbsolutePath().equals(
- getProjectSourceDirectory().getAbsolutePath() ) )
+ if ( outputDirectory != null && !outputDirectory.getAbsolutePath().equals(
+ getProjectSourceDirectory().getAbsolutePath() ) )
+ {
+ String path = StringUtils.replace( javaFile.getAbsolutePath().replaceAll( "\\\\", "/" ),
+ project.getBuild().getSourceDirectory().replaceAll( "\\\\", "/" ), "" );
+ javaFile = new File( outputDirectory, path );
+ javaFile.getParentFile().mkdirs();
+ }
+ writeFile( javaFile, encoding, stringWriter.toString() );
+ }
+ else
{
- String path = StringUtils.replace( javaFile.getAbsolutePath().replaceAll( "\\\\", "/" ),
- project.getBuild().getSourceDirectory().replaceAll( "\\\\", "/" ), "" );
- javaFile = new File( outputDirectory, path );
- javaFile.getParentFile().mkdirs();
+ if ( getLog().isDebugEnabled() )
+ {
+ getLog().debug( "No changes made to " + javaClass.getFullyQualifiedName() );
+ }
}
- writeFile( javaFile, encoding, stringWriter.toString() );
}
/**
@@ -1097,6 +1104,8 @@ public abstract class AbstractFixJavadoc
* @param stringWriter not null
* @param originalContent not null
* @param entity not null
+ * @param changeDetected
+ * @return the updated changeDetected flag
* @throws IOException if any
* @see #extractOriginalJavadoc(String, AbstractJavaEntity)
*/
@@ -1132,32 +1141,33 @@ public abstract class AbstractFixJavadoc
* @param originalContent
* @param javaClass
* @param indent
+ * @return {@code true} if the comment is updated, otherwise {@code false}
* @throws MojoExecutionException
* @throws IOException
*/
- private void fixClassComment( final StringWriter stringWriter, final String originalContent,
+ private boolean fixClassComment( final StringWriter stringWriter, final String originalContent,
final JavaClass javaClass, final String indent )
throws MojoExecutionException, IOException
{
if ( !fixClassComment )
{
- return;
+ return false;
}
if ( !isInLevel( javaClass.getModifiers() ) )
{
- return;
+ return false;
}
// add
if ( javaClass.getComment() == null )
{
addDefaultClassComment( stringWriter, javaClass, indent );
- return;
+ return true;
}
// update
- updateEntityComment( stringWriter, originalContent, javaClass, indent );
+ return updateEntityComment( stringWriter, originalContent, javaClass, indent );
}
/**
@@ -1267,30 +1277,32 @@ public abstract class AbstractFixJavadoc
* @param javaClass not null
* @param field not null
* @param indent not null
+ * @return {@code true} if comment was updated, otherwise {@code false}
* @throws IOException if any
*/
- private void fixFieldComment( final StringWriter stringWriter, final JavaClass javaClass, final JavaField field,
+ private boolean fixFieldComment( final StringWriter stringWriter, final JavaClass javaClass, final JavaField field,
final String indent )
throws IOException
{
if ( !fixFieldComment )
{
- return;
+ return false;
}
if ( !javaClass.isInterface() && ( !isInLevel( field.getModifiers() ) || !field.isStatic() ) )
{
- return;
+ return false;
}
// add
if ( field.getComment() == null )
{
addDefaultFieldComment( stringWriter, field, indent );
- return;
+ return true;
}
// no update
+ return false;
}
/**
@@ -1385,32 +1397,33 @@ public abstract class AbstractFixJavadoc
* @param originalContent not null
* @param javaMethod not null
* @param indent not null
+ * @return {@code true} if comment was updated, otherwise {@code false}
* @throws MojoExecutionException if any
* @throws IOException if any
*/
- private void fixMethodComment( final StringWriter stringWriter, final String originalContent,
+ private boolean fixMethodComment( final StringWriter stringWriter, final String originalContent,
final JavaMethod javaMethod, final String indent )
throws MojoExecutionException, IOException
{
if ( !fixMethodComment )
{
- return;
+ return false;
}
if ( !javaMethod.getParentClass().isInterface() && !isInLevel( javaMethod.getModifiers() ) )
{
- return;
+ return false;
}
// add
if ( javaMethod.getComment() == null )
{
addDefaultMethodComment( stringWriter, javaMethod, indent );
- return;
+ return true;
}
// update
- updateEntityComment( stringWriter, originalContent, javaMethod, indent );
+ return updateEntityComment( stringWriter, originalContent, javaMethod, indent );
}
/**
@@ -1525,13 +1538,18 @@ public abstract class AbstractFixJavadoc
* @param originalContent not null
* @param entity not null
* @param indent not null
+ * @param changeDetected
+ * @return the updated changeDetected flag
* @throws MojoExecutionException if any
* @throws IOException if any
*/
- private void updateEntityComment( final StringWriter stringWriter, final String originalContent,
- final AbstractInheritableJavaEntity entity, final String indent )
+ private boolean updateEntityComment( final StringWriter stringWriter, final String originalContent,
+ final AbstractInheritableJavaEntity entity, final String indent )
throws MojoExecutionException, IOException
{
+ boolean changeDetected = false;
+
+ String old = null;
String s = stringWriter.toString();
int i = s.lastIndexOf( START_JAVADOC );
if ( i != -1 )
@@ -1541,12 +1559,26 @@ public abstract class AbstractFixJavadoc
{
tmp = tmp.substring( 0, tmp.lastIndexOf( EOL ) );
}
+
+ old = stringWriter.getBuffer().substring( i );
+
stringWriter.getBuffer().delete( 0, stringWriter.getBuffer().length() );
stringWriter.write( tmp );
stringWriter.write( EOL );
}
+ else
+ {
+ changeDetected = true;
+ }
updateJavadocComment( stringWriter, originalContent, entity, indent );
+
+ if ( changeDetected )
+ {
+ return true; // return now if we already know there's a change
+ }
+
+ return !stringWriter.getBuffer().substring( i ).equals( old );
}
/**
Modified: maven/plugins/trunk/maven-javadoc-plugin/src/test/java/org/apache/maven/plugin/javadoc/FixJavadocMojoTest.java
URL: http://svn.apache.org/viewvc/maven/plugins/trunk/maven-javadoc-plugin/src/test/java/org/apache/maven/plugin/javadoc/FixJavadocMojoTest.java?rev=1751993&r1=1751992&r2=1751993&view=diff
==============================================================================
--- maven/plugins/trunk/maven-javadoc-plugin/src/test/java/org/apache/maven/plugin/javadoc/FixJavadocMojoTest.java (original)
+++ maven/plugins/trunk/maven-javadoc-plugin/src/test/java/org/apache/maven/plugin/javadoc/FixJavadocMojoTest.java Sat Jul 9 09:34:18 2016
@@ -28,7 +28,6 @@ import java.util.List;
import junitx.util.PrivateAccessor;
-import org.apache.commons.lang.SystemUtils;
import org.apache.maven.plugin.logging.Log;
import org.apache.maven.plugin.testing.AbstractMojoTestCase;
import org.apache.maven.shared.invoker.MavenInvocationException;
@@ -649,10 +648,10 @@ public class FixJavadocMojoTest
private static void assertEquals( File expected, File actual )
throws IOException
{
- assertTrue( expected.exists() );
+ assertTrue( " Expected file DNE: " + expected, expected.exists() );
String expectedContent = StringUtils.unifyLineSeparators( readFile( expected ) );
- assertTrue( actual.exists() );
+ assertTrue( " Actual file DNE: " + actual, actual.exists() );
String actualContent = StringUtils.unifyLineSeparators( readFile( actual ) );
assertEquals( "Expected file: " + expected.getAbsolutePath() + ", actual file: "