let BanDuplicateClasses ignore when the bytecode matches exactly (#50)

Fixed #54 - let ban duplicate classes ignore when the bytecode matches exactly
* fix checkstyle warnings and errors
* update documentation with new BanDuplicateClasses option

pom.xml

@@ -117,6 +117,11 @@
       <artifactId>plexus-container-default</artifactId>
       <version>1.0-alpha-9</version>
     </dependency>
+    <dependency>
+      <groupId>commons-codec</groupId>
+      <artifactId>commons-codec</artifactId>
+      <version>1.11</version>
+    </dependency>
     <dependency>
       <groupId>junit</groupId>
       <artifactId>junit</artifactId>

src/it/banduplicate-classes-fail-when-not-identical/invoker.properties

@@ -0,0 +1,2 @@
+invoker.goals = enforcer:enforce
+invoker.buildResult = failure

src/it/banduplicate-classes-fail-when-not-identical/pom.xml

@@ -0,0 +1,64 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+
+  <groupId>org.apache.maven</groupId>
+  <artifactId>banduplicate-classes-ignore-when-identical</artifactId>
+  <version>1.0-SNAPSHOT</version>
+
+  <properties>
+    <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+  </properties>
+
+  <build>
+    <plugins>
+      <plugin>
+        <artifactId>maven-enforcer-plugin</artifactId>
+        <version>@enforcerPluginVersion@</version>
+        <dependencies>
+          <dependency>
+            <groupId>@project.groupId@</groupId>
+            <artifactId>@project.artifactId@</artifactId>
+            <version>@project.version@</version>
+          </dependency>
+        </dependencies>
+        <configuration>
+          <rules>
+            <BanDuplicateClasses>
+              <ignoreWhenIdentical>true</ignoreWhenIdentical>
+            </BanDuplicateClasses>
+          </rules>
+        </configuration>
+        <executions>
+          <execution>
+          <phase>compile</phase>
+          </execution>
+        </executions>
+      </plugin>
+    </plugins>
+  </build>
+
+  <dependencies>
+    <dependency>
+      <groupId>org.apache.logging.log4j</groupId>
+      <artifactId>log4j-api</artifactId>
+      <version>2.9.0</version>
+      <scope>runtime</scope>
+    </dependency>
+
+    <dependency>
+      <groupId>commons-logging</groupId>
+      <artifactId>commons-logging</artifactId>
+      <version>1.2</version>
+    </dependency>
+
+    <!-- these classes duplicate those in commons-logging, and the bytecode won't match
+         (that's the trick SLF4J uses: it rewrites some of the commons-logging classes
+         to make them work with SLF4J instead) -->
+    <dependency>
+      <groupId>org.slf4j</groupId>
+      <artifactId>jcl-over-slf4j</artifactId>
+      <version>1.7.25</version>
+    </dependency>
+
+  </dependencies>
+</project>

src/it/banduplicate-classes-fail-when-not-identical/verify.groovy

@@ -0,0 +1,6 @@
+File file = new File( basedir, "build.log" )
+assert file.exists()
+
+String text = file.getText( "utf-8" )
+
+return true

src/it/banduplicate-classes-ignore-when-identical/invoker.properties

@@ -0,0 +1,2 @@
+invoker.goals = enforcer:enforce
+invoker.buildResult = success

src/it/banduplicate-classes-ignore-when-identical/pom.xml

@@ -0,0 +1,63 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+
+  <groupId>org.apache.maven</groupId>
+  <artifactId>banduplicate-classes-ignore-when-identical</artifactId>
+  <version>1.0-SNAPSHOT</version>
+
+  <properties>
+    <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+  </properties>
+
+  <build>
+    <plugins>
+      <plugin>
+        <artifactId>maven-enforcer-plugin</artifactId>
+        <version>@enforcerPluginVersion@</version>
+        <dependencies>
+          <dependency>
+            <groupId>@project.groupId@</groupId>
+            <artifactId>@project.artifactId@</artifactId>
+            <version>@project.version@</version>
+          </dependency>
+        </dependencies>
+        <configuration>
+          <rules>
+            <BanDuplicateClasses>
+              <ignoreWhenIdentical>true</ignoreWhenIdentical>
+              <findAllDuplicates>true</findAllDuplicates>
+            </BanDuplicateClasses>
+          </rules>
+        </configuration>
+        <executions>
+          <execution>
+          <phase>compile</phase>
+          </execution>
+        </executions>
+      </plugin>
+    </plugins>
+  </build>
+
+  <dependencies>
+    <dependency>
+      <groupId>org.apache.logging.log4j</groupId>
+      <artifactId>log4j-api</artifactId>
+      <version>2.9.0</version>
+      <scope>runtime</scope>
+    </dependency>
+
+    <dependency>
+      <groupId>org.mockito</groupId>
+      <artifactId>mockito-core</artifactId>
+      <version>1.10.19</version>
+    </dependency>
+
+    <!-- these classes duplicate those in mockito-core -->
+    <dependency>
+      <groupId>org.mockito</groupId>
+      <artifactId>mockito-all</artifactId>
+      <version>1.10.19</version>
+    </dependency>
+
+  </dependencies>
+</project>

src/it/banduplicate-classes-ignore-when-identical/verify.groovy

@@ -0,0 +1,6 @@
+File file = new File( basedir, "build.log" )
+assert file.exists()
+
+String text = file.getText( "utf-8" )
+
+return true

src/main/java/org/apache/maven/plugins/enforcer/BanDuplicateClasses.java

@@ -25,7 +25,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.LinkedHashSet;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -38,6 +38,8 @@
 import org.codehaus.mojo.enforcer.Dependency;
 import org.codehaus.plexus.util.FileUtils;
 
+import static org.apache.maven.plugins.enforcer.JarUtils.isJarFile;
+
 /**
  * Bans duplicate classes on the classpath.
  */
@@ -51,7 +53,7 @@ public class BanDuplicateClasses
      * also contained several times.
      */
     private static final String[] DEFAULT_CLASSES_IGNORES = { "module-info", "META-INF/versions/*/module-info" };
-    
+
     /**
      * The failure message
      */
@@ -75,6 +77,12 @@ public class BanDuplicateClasses
      */
     private List<String> scopes;
 
+    /**
+     * If {@code true} do not fail the build when duplicate classes exactly match each other. In other words, ignore
+     * duplication if the bytecode in the class files match. Default is {@code false}.
+     */
+    private boolean ignoreWhenIdentical;
+
     @Override
     protected void handleArtifacts( Set<Artifact> artifacts ) throws EnforcerRuleException
     {
@@ -114,9 +122,9 @@ protected void handleArtifacts( Set<Artifact> artifacts ) throws EnforcerRuleExc
                 ignorableDependencies.add( ignorableDependency );
             }
         }
-        
-        Map<String, Artifact> classNames = new HashMap<String, Artifact>();
-        Map<String, Set<Artifact>> duplicates = new HashMap<String, Set<Artifact>>();
+
+        Map<String, ClassesWithSameName> classesSeen = new HashMap<String, ClassesWithSameName>();
+        Set<String> duplicateClassNames = new HashSet<String>();
         for ( Artifact o : artifacts )
         {
             if( scopes != null && !scopes.contains( o.getScope() ) )
@@ -140,7 +148,7 @@ else if ( file.isDirectory() )
                     for ( String name : FileUtils.getFileNames( file, null, null, false ) )
                     {
                         getLog().debug( "  " + name );
-                        checkAndAddName( o, name, classNames, duplicates, ignorableDependencies );
+                        checkAndAddName( o, name, classesSeen, duplicateClassNames, ignorableDependencies );
                     }
                 }
                 catch ( IOException e )
@@ -149,7 +157,7 @@ else if ( file.isDirectory() )
                         "Unable to process dependency " + o.toString() + " due to " + e.getLocalizedMessage(), e );
                 }
             }
-            else if ( file.isFile() && "jar".equals( o.getType() ) )
+            else if ( isJarFile( o ) )
             {
                 try
                 {
@@ -159,7 +167,8 @@ else if ( file.isFile() && "jar".equals( o.getType() ) )
                     {
                         for ( JarEntry entry : Collections.<JarEntry>list( jar.entries() ) )
                         {
-                            checkAndAddName( o, entry.getName(), classNames, duplicates, ignorableDependencies );
+                            String fileName = entry.getName();
+                            checkAndAddName( o, fileName, classesSeen, duplicateClassNames, ignorableDependencies );
                         }
                     }
                     finally
@@ -181,18 +190,21 @@ else if ( file.isFile() && "jar".equals( o.getType() ) )
                 }
             }
         }
-        if ( !duplicates.isEmpty() )
+        if ( !duplicateClassNames.isEmpty() )
         {
             Map<Set<Artifact>, List<String>> inverted = new HashMap<Set<Artifact>, List<String>>();
-            for ( Map.Entry<String, Set<Artifact>> entry : duplicates.entrySet() )
+            for ( String className : duplicateClassNames )
             {
-                List<String> s = inverted.get( entry.getValue() );
+                ClassesWithSameName classesWithSameName = classesSeen.get( className );
+                Set<Artifact> artifactsOfDuplicateClass = classesWithSameName.getAllArtifactsThisClassWasFoundIn();
+
+                List<String> s = inverted.get( artifactsOfDuplicateClass );
                 if ( s == null )
                 {
                     s = new ArrayList<String>();
                 }
-                s.add( entry.getKey() );
-                inverted.put( entry.getValue(), s );
+                s.add( classesWithSameName.toOutputString( ignoreWhenIdentical ) );
+                inverted.put( artifactsOfDuplicateClass, s );
             }
             StringBuilder buf = new StringBuilder( message == null ? "Duplicate classes found:" : message );
             buf.append( '\n' );
@@ -205,10 +217,10 @@ else if ( file.isFile() && "jar".equals( o.getType() ) )
                     buf.append( a );
                 }
                 buf.append( "\n  Duplicate classes:" );
-                for ( String className : entry.getValue() )
+                for ( String classNameWithDuplicationInfo : entry.getValue() )
                 {
                     buf.append( "\n    " );
-                    buf.append( className );
+                    buf.append( classNameWithDuplicationInfo );
                 }
                 buf.append( '\n' );
             }
@@ -217,73 +229,66 @@ else if ( file.isFile() && "jar".equals( o.getType() ) )
 
     }
 
-    private void checkAndAddName( Artifact artifact, String name, Map<String, Artifact> classNames,
-                                  Map<String, Set<Artifact>> duplicates, Collection<IgnorableDependency> ignores )
+    private void checkAndAddName( Artifact artifact, String pathToClassFile, Map<String,
+                                  ClassesWithSameName> classesSeen, Set<String> duplicateClasses,
+                                  Collection<IgnorableDependency> ignores )
         throws EnforcerRuleException
     {
-        if ( !name.endsWith( ".class" ) )
+        if ( !pathToClassFile.endsWith( ".class" ) )
         {
             return;
         }
-        
+
         for ( IgnorableDependency c : ignores )
         {
-            if ( c.matchesArtifact( artifact ) && c.matches( name ) )
+            if ( c.matchesArtifact( artifact ) && c.matches( pathToClassFile ) )
             {
-                if( classNames.containsKey( name ) )
+                if ( classesSeen.containsKey( pathToClassFile ) )
                 {
-                    getLog().debug( "Ignoring excluded class " + name );
+                    getLog().debug( "Ignoring excluded class " + pathToClassFile );
                 }
                 return;
             }
         }
 
-        if ( classNames.containsKey( name ) )
+        ClassesWithSameName classesWithSameName = classesSeen.get( pathToClassFile );
+        boolean isFirstTimeSeeingThisClass = ( classesWithSameName == null );
+        ClassFile classFile = new ClassFile( pathToClassFile, artifact );
+
+        if ( isFirstTimeSeeingThisClass )
         {
-            Artifact dup = classNames.put( name, artifact );
-            if ( !( findAllDuplicates && duplicates.containsKey( name ) ) )
-            {
-                for ( IgnorableDependency c : ignores )
-                {
-                    if ( c.matchesArtifact( artifact ) && c.matches(name) )
-                    {
-                        getLog().debug( "Ignoring duplicate class " + name );
-                        return;
-                    }
-                }
-            }
-
-            if ( findAllDuplicates )
-            {
-                Set<Artifact> dups = duplicates.get( name );
-                if ( dups == null )
-                {
-                    dups = new LinkedHashSet<Artifact>();
-                    dups.add( dup );
-                }
-                dups.add( artifact );
-                duplicates.put( name, dups );
-            }
-            else
-            {
-                StringBuilder buf = new StringBuilder( message == null ? "Duplicate class found:" : message );
-                buf.append( '\n' );
-                buf.append( "\n  Found in:" );
-                buf.append( "\n    " );
-                buf.append( dup );
-                buf.append( "\n    " );
-                buf.append( artifact );
-                buf.append( "\n  Duplicate classes:" );
-                buf.append( "\n    " );
-                buf.append( name );
-                buf.append( '\n' );
-                buf.append( "There may be others but <findAllDuplicates> was set to false, so failing fast" );
-                throw new EnforcerRuleException( buf.toString() );
-            }
+            classesSeen.put( pathToClassFile, new ClassesWithSameName( getLog(), classFile ) );
+            return;
         }
-        else 
+
+        classesWithSameName.add( classFile );
+
+        if ( !classesWithSameName.hasDuplicates( ignoreWhenIdentical ) )
         {
-            classNames.put( name, artifact );
+            return;
+        }
+
+        if ( findAllDuplicates )
+        {
+            duplicateClasses.add( pathToClassFile );
+        }
+        else
+        {
+            Artifact previousArtifact = classesWithSameName.previous().getArtifactThisClassWasFoundIn();
+
+            StringBuilder buf = new StringBuilder( message == null ? "Duplicate class found:" : message );
+            buf.append( '\n' );
+            buf.append( "\n  Found in:" );
+            buf.append( "\n    " );
+            buf.append( previousArtifact );
+            buf.append( "\n    " );
+            buf.append( artifact );
+            buf.append( "\n  Duplicate classes:" );
+            buf.append( "\n    " );
+            buf.append( pathToClassFile );
+            buf.append( '\n' );
+            buf.append( "There may be others but <findAllDuplicates> was set to false, so failing fast" );
+            throw new EnforcerRuleException( buf.toString() );
         }
     }
 }