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 2018/12/09 16:03:05 UTC

[maven-javadoc-plugin] branch master updated: [MJAVADOC-548] Directoryname mixed up with excludePackageNames

This is an automated email from the ASF dual-hosted git repository.

rfscholte pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven-javadoc-plugin.git


The following commit(s) were added to refs/heads/master by this push:
     new 936cf7d  [MJAVADOC-548] Directoryname mixed up with excludePackageNames
936cf7d is described below

commit 936cf7d5c91f5f3fa0fc54c1326601432374b112
Author: rfscholte <rf...@apache.org>
AuthorDate: Sun Dec 9 17:02:57 2018 +0100

    [MJAVADOC-548] Directoryname mixed up with excludePackageNames
---
 .gitignore                                         |   1 +
 .../com/foo/bar/internal/{NotApi.java => Api.java} |   4 +-
 src/it/projects/MJAVADOC-384/verify.bsh            |  16 +-
 src/it/projects/MJAVADOC-497/pom.xml               |   2 +-
 .../maven/plugins/javadoc/AbstractJavadocMojo.java |  49 +++---
 .../apache/maven/plugins/javadoc/JavadocUtil.java  | 171 ++++++++++-----------
 .../maven/plugins/javadoc/JavadocUtilTest.java     |  15 ++
 7 files changed, 136 insertions(+), 122 deletions(-)

diff --git a/.gitignore b/.gitignore
index 384de6f..bdbdf67 100644
--- a/.gitignore
+++ b/.gitignore
@@ -15,3 +15,4 @@ out/
 .java-version
 /javadoc-options-javadoc-resources.xml
 .checkstyle
+/src/it/mrm/3rdparty/_doclet-1.0.jar
diff --git a/src/it/projects/MJAVADOC-384/src/main/java/com/foo/bar/internal/NotApi.java b/src/it/projects/MJAVADOC-384/src/main/java/com/foo/bar/internal/Api.java
similarity index 95%
rename from src/it/projects/MJAVADOC-384/src/main/java/com/foo/bar/internal/NotApi.java
rename to src/it/projects/MJAVADOC-384/src/main/java/com/foo/bar/internal/Api.java
index 53480de..7121d40 100644
--- a/src/it/projects/MJAVADOC-384/src/main/java/com/foo/bar/internal/NotApi.java
+++ b/src/it/projects/MJAVADOC-384/src/main/java/com/foo/bar/internal/Api.java
@@ -19,7 +19,7 @@ package com.foo.bar.internal;
  * under the License.
  */
 
-/** Not API */
-public class NotApi {
+/** API */
+public class Api {
 
 }
diff --git a/src/it/projects/MJAVADOC-384/verify.bsh b/src/it/projects/MJAVADOC-384/verify.bsh
index dce4671..1c80f44 100644
--- a/src/it/projects/MJAVADOC-384/verify.bsh
+++ b/src/it/projects/MJAVADOC-384/verify.bsh
@@ -36,11 +36,19 @@ for ( File javadocFile : nonExistingJavadocFiles )
     }
 }
 
-File javadocFile = new File ( basedir, "target/site/apidocs/com/foo/bar/Api.html" );
-if ( !javadocFile.exists() ) 
+File[] existingJavadocFiles = new File[] {
+    new File( basedir, "target/site/apidocs/com/foo/bar/Api.html" ),
+    new File( basedir, "target/site/apidocs/com/foo/bar/internal/Api.html" ),
+};
+
+for ( File javadocFile : existingJavadocFiles )
 {
-    System.err.println( javadocFile.getAbsolutePath() + " is missing." );
-    return false;
+    if ( !javadocFile.exists() ) 
+    {
+        System.err.println( javadocFile.getAbsolutePath() + " is missing." );
+        return false;
+    }
 }
 
+
 return true;
diff --git a/src/it/projects/MJAVADOC-497/pom.xml b/src/it/projects/MJAVADOC-497/pom.xml
index 9b89c69..c664cd7 100644
--- a/src/it/projects/MJAVADOC-497/pom.xml
+++ b/src/it/projects/MJAVADOC-497/pom.xml
@@ -46,7 +46,7 @@
           <version>@pom.version@</version>
           <configuration>
             <subpackages>com.example.foo</subpackages>
-            <excludePackageNames>*.impl.*</excludePackageNames>
+            <excludePackageNames>**.impl</excludePackageNames>
           </configuration>
         </plugin>
       </plugins>
diff --git a/src/main/java/org/apache/maven/plugins/javadoc/AbstractJavadocMojo.java b/src/main/java/org/apache/maven/plugins/javadoc/AbstractJavadocMojo.java
index addcd36..4ff10ec 100644
--- a/src/main/java/org/apache/maven/plugins/javadoc/AbstractJavadocMojo.java
+++ b/src/main/java/org/apache/maven/plugins/javadoc/AbstractJavadocMojo.java
@@ -111,6 +111,7 @@ import java.net.URLClassLoader;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.nio.file.StandardCopyOption;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -765,7 +766,14 @@ public abstract class AbstractJavadocMojo
      * Unconditionally excludes the specified packages and their subpackages from the list formed by
      * <code>-subpackages</code>. Multiple packages can be separated by commas (<code>,</code>), colons (<code>:</code>)
      * or semicolons (<code>;</code>).
-     * <br/>
+     * <p>
+     * Wildcards work as followed:
+     * <ul>
+     *   <li>a wildcard at the beginning should match 1 or more folders</li>
+     *   <li>any other wildcard must match exactly one folder</li>
+     * </ul>
+     * </p>
+     * <p>
      * Example:
      * <pre>
      * &lt;excludePackageNames&gt;*.internal:org.acme.exclude1.*:org.acme.exclude2&lt;/excludePackageNames&gt;
@@ -775,6 +783,7 @@ public abstract class AbstractJavadocMojo
      * <br/>
      * Since <a href="http://docs.oracle.com/javase/7/docs/technotes/guides/javadoc/whatsnew-1.4.html#summary">Java
      * 1.4</a>.
+     * </p>
      */
     @Parameter( property = "excludePackageNames" )
     private String excludePackageNames;
@@ -2209,13 +2218,13 @@ public abstract class AbstractJavadocMojo
         List<String> files = new ArrayList<>();
         if ( StringUtils.isEmpty( subpackages ) )
         {
-            String[] excludedPackages = getExcludedPackages();
-
+            Collection<String> excludedPackages = getExcludedPackages();
+            
             for ( String sourcePath : sourcePaths )
             {
                 File sourceDirectory = new File( sourcePath );
-                JavadocUtil.addFilesFromSource( files, sourceDirectory, sourceFileIncludes, sourceFileExcludes,
-                                                excludedPackages );
+                files.addAll( JavadocUtil.getFilesFromSource( sourceDirectory, sourceFileIncludes, sourceFileExcludes,
+                                                              excludedPackages ) );
             }
         }
 
@@ -2463,17 +2472,22 @@ public abstract class AbstractJavadocMojo
      * @return a String that contains the exclude argument that will be used by javadoc
      * @throws MavenReportException
      */
-    private String getExcludedPackages( Collection<String> sourcePaths )
+    private String getExcludedPackages( Collection<String> sourceFolders )
         throws MavenReportException
     {
         List<String> excludedNames = null;
+        
+        Collection<Path> sourcePaths = new ArrayList<>( sourceFolders.size() );
+        for ( String folder : sourceFolders )
+        {
+            sourcePaths.add( Paths.get( folder ) );
+        }
 
         if ( StringUtils.isNotEmpty( sourcepath ) && StringUtils.isNotEmpty( subpackages ) )
         {
-            String[] excludedPackages = getExcludedPackages();
-            String[] subpackagesList = subpackages.split( "[:]" );
-
-            excludedNames = JavadocUtil.getExcludedNames( sourcePaths, excludedPackages );
+            Collection<String> excludedPackages = getExcludedPackages();
+            
+            excludedNames = JavadocUtil.getExcludedPackages( sourcePaths, excludedPackages );
         }
 
         String excludeArg = "";
@@ -2513,7 +2527,7 @@ public abstract class AbstractJavadocMojo
      * @return an array of String objects that contain the package names
      * @throws MavenReportException
      */
-    private String[] getExcludedPackages()
+    private Collection<String> getExcludedPackages()
         throws MavenReportException
     {
         Set<String> excluded = new LinkedHashSet<>();
@@ -2550,18 +2564,7 @@ public abstract class AbstractJavadocMojo
             excluded.addAll( trimValues( packageNames ) );
         }
 
-        String[] result = new String[excluded.size()];
-        if ( isNotEmpty( excluded ) )
-        {
-            int idx = 0;
-            for ( String exclude : excluded )
-            {
-                result[idx] = exclude.replace( '.', File.separatorChar );
-                idx++;
-            }
-        }
-
-        return result;
+        return excluded;
     }
 
     private static List<String> trimValues( List<String> items )
diff --git a/src/main/java/org/apache/maven/plugins/javadoc/JavadocUtil.java b/src/main/java/org/apache/maven/plugins/javadoc/JavadocUtil.java
index 22cab64..2e24e06 100644
--- a/src/main/java/org/apache/maven/plugins/javadoc/JavadocUtil.java
+++ b/src/main/java/org/apache/maven/plugins/javadoc/JavadocUtil.java
@@ -78,6 +78,11 @@ import java.net.URL;
 import java.net.URLClassLoader;
 import java.nio.charset.Charset;
 import java.nio.charset.IllegalCharsetNameException;
+import java.nio.file.FileVisitResult;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.attribute.BasicFileAttributes;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -193,15 +198,15 @@ public class JavadocUtil
      *
      * @param sourcePaths the path to the source files
      * @param excludedPackages the package names to be excluded in the javadoc
-     * @return a List of the source files to be excluded in the generated javadoc
+     * @return a List of the packages to be excluded in the generated javadoc
      */
-    protected static List<String> getExcludedNames( Collection<String> sourcePaths, String[] excludedPackages )
+    protected static List<String> getExcludedPackages( Collection<Path> sourcePaths,
+                                                       Collection<String> excludedPackages )
     {
         List<String> excludedNames = new ArrayList<>();
-        for ( String path : sourcePaths )
+        for ( Path sourcePath : sourcePaths )
         {
-            List<String> excludes = getExcludedPackages( path, excludedPackages );
-            excludedNames.addAll( excludes );
+            excludedNames.addAll( getExcludedPackages( sourcePath, excludedPackages ) );
         }
 
         return excludedNames;
@@ -333,70 +338,40 @@ public class JavadocUtil
      * Method that gets the files or classes that would be included in the javadocs using the subpackages parameter.
      *
      * @param sourceDirectory the directory where the source files are located
-     * @param fileList the list of all files found in the sourceDirectory
+     * @param fileList the list of all relative files found in the sourceDirectory
      * @param excludePackages package names to be excluded in the javadoc
      * @return a StringBuilder that contains the appended file names of the files to be included in the javadoc
      */
-    protected static List<String> getIncludedFiles( File sourceDirectory, String[] fileList, String[] excludePackages )
+    protected static List<String> getIncludedFiles( File sourceDirectory, String[] fileList,
+                                                    Collection<String> excludePackages )
     {
         List<String> files = new ArrayList<>();
+        
+        List<Pattern> excludePackagePatterns = new ArrayList<>( excludePackages.size() );
+        for ( String excludePackage :  excludePackages )
+        {
+            excludePackagePatterns.add( Pattern.compile( excludePackage.replace( '.', File.separatorChar )
+                                                                       .replace( "\\", "\\\\" )
+                                                                       .replace( "*", ".+" )
+                                                                       .concat( "[\\\\/][^\\\\/]+\\.java" )
+                                                                                ) );
+        }
 
-        for ( String aFileList : fileList )
+        for ( String file : fileList )
         {
-            boolean include = true;
-            for ( int k = 0; k < excludePackages.length && include; k++ )
+            boolean excluded = false;
+            for ( Pattern excludePackagePattern :  excludePackagePatterns )
             {
-                // handle wildcards (*) in the excludePackageNames
-                String[] excludeName = excludePackages[k].split( "[*]" );
-
-                if ( excludeName.length == 0 )
-                {
-                    continue;
-                }
-
-                if ( excludeName.length > 1 )
+                if ( excludePackagePattern.matcher( file ).matches() )
                 {
-                    int u = 0;
-                    while ( include && u < excludeName.length )
-                    {
-                        if ( !"".equals( excludeName[u].trim() ) && aFileList.contains( excludeName[u] ) )
-                        {
-                            include = false;
-                        }
-                        u++;
-                    }
-                }
-                else
-                {
-                    if ( aFileList.startsWith( sourceDirectory.toString() + File.separatorChar + excludeName[0] ) )
-                    {
-                        if ( excludeName[0].endsWith( String.valueOf( File.separatorChar ) ) )
-                        {
-                            int i = aFileList.lastIndexOf( File.separatorChar );
-                            String packageName = aFileList.substring( 0, i + 1 );
-                            File currentPackage = new File( packageName );
-                            File excludedPackage = new File( sourceDirectory, excludeName[0] );
-                            if ( currentPackage.equals( excludedPackage )
-                                && aFileList.substring( i ).contains( ".java" ) )
-                            {
-                                include = true;
-                            }
-                            else
-                            {
-                                include = false;
-                            }
-                        }
-                        else
-                        {
-                            include = false;
-                        }
-                    }
+                    excluded = true;
+                    break;
                 }
             }
-
-            if ( include )
+            
+            if ( !excluded )
             {
-                files.add( quotedPathArgument( aFileList ) );
+                files.add( file );
             }
         }
 
@@ -411,57 +386,69 @@ public class JavadocUtil
      * @param excludePackagenames package names to be excluded in the javadoc
      * @return a List of the packagenames to be excluded
      */
-    protected static List<String> getExcludedPackages( String sourceDirectory, String[] excludePackagenames )
+    protected static Collection<String> getExcludedPackages( final Path sourceDirectory,
+                                                             Collection<String> excludePackagenames )
     {
-        List<String> files = new ArrayList<>();
-        for ( String excludePackagename : excludePackagenames )
+        final String regexFileSeparator = File.separator.replace( "\\", "\\\\" );
+        
+        final Collection<String> fileList = new ArrayList<>();
+        
+        try
         {
-            String[] fileList = FileUtils.getFilesFromExtension( sourceDirectory, new String[] { "java" } );
-            for ( String aFileList : fileList )
+            Files.walkFileTree( sourceDirectory, new SimpleFileVisitor<Path>()
             {
-                String[] excludeName = excludePackagename.split( "[*]" );
-                int u = 0;
-                while ( u < excludeName.length )
+                @Override
+                public FileVisitResult visitFile( Path file, BasicFileAttributes attrs )
+                    throws IOException
                 {
-                    if ( !"".equals( excludeName[u].trim() ) && aFileList.contains( excludeName[u] )
-                        && !sourceDirectory.contains( excludeName[u] ) )
+                    if ( file.getFileName().toString().endsWith( ".java" ) )
                     {
-                        files.add( aFileList );
+                        fileList.add( sourceDirectory.relativize( file.getParent() ).toString() );
                     }
-                    u++;
+                    return FileVisitResult.CONTINUE;
                 }
-            }
+            } );
         }
-
-        List<String> excluded = new ArrayList<>();
-        for ( String file : files )
+        catch ( IOException e )
         {
-            int idx = file.lastIndexOf( File.separatorChar );
-            String tmpStr = file.substring( 0, idx );
-            tmpStr = tmpStr.replace( '\\', '/' );
-            String[] srcSplit = tmpStr.split( Pattern.quote( sourceDirectory.replace( '\\', '/' ) + '/' ) );
-            String excludedPackage = srcSplit[1].replace( '/', '.' );
+            // noop
+        }
 
-            if ( !excluded.contains( excludedPackage ) )
+        List<String> files = new ArrayList<>();
+        for ( String excludePackagename : excludePackagenames )
+        {
+            // Usage of wildcard was bad specified and bad implemented, i.e. using String.contains() 
+            //   without respecting surrounding context
+            // Following implementation should match requirements as defined in the examples:  
+            // - A wildcard at the beginning should match 1 or more folders
+            // - Any other wildcard must match exactly one folder
+            Pattern p = Pattern.compile( excludePackagename.replace( ".", regexFileSeparator )
+                                                           .replaceFirst( "^\\*", ".+" )
+                                                           .replace( "*", "[^" + regexFileSeparator + "]+" ) );
+            
+            for ( String aFileList : fileList )
             {
-                excluded.add( excludedPackage );
+                if ( p.matcher( aFileList ).matches() )
+                {
+                    files.add( aFileList.replace( File.separatorChar, '.' ) );
+                }
             }
         }
 
-        return excluded;
+        return files;
     }
 
     /**
      * Convenience method that gets the files to be included in the javadoc.
      *
      * @param sourceDirectory the directory where the source files are located
-     * @param files the variable that contains the appended filenames of the files to be included in the javadoc
      * @param excludePackages the packages to be excluded in the javadocs
      * @param sourceFileIncludes files to include.
      * @param sourceFileExcludes files to exclude.
      */
-    protected static void addFilesFromSource( List<String> files, File sourceDirectory, List<String> sourceFileIncludes,
-                                              List<String> sourceFileExcludes, String[] excludePackages )
+    protected static List<String> getFilesFromSource( File sourceDirectory, List<String> sourceFileIncludes,
+                                                      List<String> sourceFileExcludes,
+                                                      Collection<String> excludePackages )
     {
         DirectoryScanner ds = new DirectoryScanner();
         if ( sourceFileIncludes == null )
@@ -477,17 +464,17 @@ public class JavadocUtil
         ds.scan();
 
         String[] fileList = ds.getIncludedFiles();
-        String[] pathList = new String[fileList.length];
-        for ( int x = 0; x < fileList.length; x++ )
-        {
-            pathList[x] = new File( sourceDirectory, fileList[x] ).getAbsolutePath();
-        }
 
-        if ( pathList.length != 0 )
+        List<String> files = new ArrayList<>();
+        if ( fileList.length != 0 )
         {
-            List<String> tmpFiles = getIncludedFiles( sourceDirectory, pathList, excludePackages );
-            files.addAll( tmpFiles );
+            for ( String includedFile : getIncludedFiles( sourceDirectory, fileList, excludePackages ) )
+            {
+                files.add( quotedPathArgument( new File( sourceDirectory, includedFile ).getAbsolutePath() ) );
+            }
         }
+
+        return files;
     }
 
     /**
diff --git a/src/test/java/org/apache/maven/plugins/javadoc/JavadocUtilTest.java b/src/test/java/org/apache/maven/plugins/javadoc/JavadocUtilTest.java
index 825c2a0..139a09f 100644
--- a/src/test/java/org/apache/maven/plugins/javadoc/JavadocUtilTest.java
+++ b/src/test/java/org/apache/maven/plugins/javadoc/JavadocUtilTest.java
@@ -1,5 +1,7 @@
 package org.apache.maven.plugins.javadoc;
 
+import static org.junit.Assert.assertArrayEquals;
+
 /*
  * Licensed to the Apache Software Foundation (ASF) under one
  * or more contributor license agreements.  See the NOTICE file
@@ -27,6 +29,7 @@ import java.net.SocketTimeoutException;
 import java.net.URI;
 import java.net.URL;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -775,6 +778,18 @@ public class JavadocUtilTest
         assertEquals( path1 + ps + path2 + ps + path1 + ps + path2, JavadocUtil.unifyPathSeparator( path1 + ";"
             + path2 + ":" + path1 + ":" + path2 ) );
     }
+    
+    
+    public void testGetIncludedFiles() throws Exception
+    {
+        File sourceDirectory = new File("target/it").getAbsoluteFile();
+        String[] fileList = new String[] { "Main.java" };
+        Collection<String> excludePackages = Collections.singleton( "*.it" );
+        
+        List<String> includedFiles = JavadocUtil.getIncludedFiles( sourceDirectory, fileList, excludePackages );
+        
+        assertArrayEquals( fileList, includedFiles.toArray( new String[0] ) );
+    }
 
     private void stopSilently( Server server )
     {