Merge pull request #2 from TNG/allow-multiple-import-options

Allow multiple import options and be more restrictive within ImportOption.DontIncludeTests

archunit-integration-test/src/test/java/com/tngtech/archunit/ArchUnitArchitectureTest.java

@@ -39,7 +39,7 @@
 @RunWith(ArchUnitRunner.class)
 @AnalyzeClasses(
         packagesOf = ArchUnitArchitectureTest.class,
-        importOption = ArchUnitArchitectureTest.ArchUnitProductionCode.class)
+        importOptions = ArchUnitArchitectureTest.ArchUnitProductionCode.class)
 public class ArchUnitArchitectureTest {
     static final String THIRDPARTY_PACKAGE_IDENTIFIER = "..thirdparty..";
 

archunit-junit/src/main/java/com/tngtech/archunit/junit/AnalyzeClasses.java

@@ -18,7 +18,9 @@
 import java.lang.annotation.Retention;
 import java.lang.annotation.Target;
 
+import com.tngtech.archunit.core.importer.ClassFileImporter;
 import com.tngtech.archunit.core.importer.ImportOption;
+import com.tngtech.archunit.core.importer.ImportOptions;
 
 import static java.lang.annotation.ElementType.TYPE;
 import static java.lang.annotation.RetentionPolicy.RUNTIME;
@@ -31,9 +33,22 @@
 @Target(TYPE)
 @Retention(RUNTIME)
 public @interface AnalyzeClasses {
+    /**
+     * @return Packages to look for in all URLs known to the actual {@link java.net.URLClassLoader}
+     */
     String[] packages() default {};
 
+    /**
+     * @return Classes that specify packages to look for in all URLs known to the actual {@link java.net.URLClassLoader}
+     */
     Class[] packagesOf() default {};
 
-    Class<? extends ImportOption> importOption() default ImportOption.Everything.class;
+    /**
+     * Allows to filter the class import. The supplied types will be instantiated and used to create the
+     * {@link ImportOptions} passed to the {@link ClassFileImporter}. Considering caching, compare the notes on
+     * {@link ImportOption}.
+     *
+     * @return The types of {@link ImportOption} to use for the import
+     */
+    Class<? extends ImportOption>[] importOptions() default {};
 }

archunit-junit/src/main/java/com/tngtech/archunit/junit/ClassCache.java

@@ -26,6 +26,7 @@
 import com.tngtech.archunit.core.domain.JavaClasses;
 import com.tngtech.archunit.core.importer.ClassFileImporter;
 import com.tngtech.archunit.core.importer.ImportOption;
+import com.tngtech.archunit.core.importer.ImportOptions;
 import com.tngtech.archunit.core.importer.Location;
 import com.tngtech.archunit.core.importer.Locations;
 
@@ -55,7 +56,7 @@ private LocationsKey locationsToImport(Class<?> testClass) {
                 .addAll(toPackageStrings(analyzeClasses.packagesOf()))
                 .build();
         Set<Location> locations = packages.isEmpty() ? Locations.inClassPath() : locationsOf(packages);
-        return new LocationsKey(analyzeClasses.importOption(), locations);
+        return new LocationsKey(analyzeClasses.importOptions(), locations);
     }
 
     private Set<String> toPackageStrings(Class[] classes) {
@@ -119,31 +120,34 @@ public JavaClasses get() {
 
         private synchronized void initialize() {
             if (javaClasses == null) {
-                ImportOption importOption = newInstanceOf(locationsKey.importOptionClass);
-                javaClasses = cacheClassFileImporter.importClasses(importOption, locationsKey.locations);
+                ImportOptions importOptions = new ImportOptions();
+                for (Class<? extends ImportOption> optionClass : locationsKey.importOptionClasses) {
+                    importOptions = importOptions.with(newInstanceOf(optionClass));
+                }
+                javaClasses = cacheClassFileImporter.importClasses(importOptions, locationsKey.locations);
             }
         }
     }
 
     // Used for testing -> that's also the reason it's declared top level
     static class CacheClassFileImporter {
-        JavaClasses importClasses(ImportOption importOption, Collection<Location> locations) {
-            return new ClassFileImporter().withImportOption(importOption).importLocations(locations);
+        JavaClasses importClasses(ImportOptions importOptions, Collection<Location> locations) {
+            return new ClassFileImporter(importOptions).importLocations(locations);
         }
     }
 
     private static class LocationsKey {
-        private final Class<? extends ImportOption> importOptionClass;
+        private final Set<Class<? extends ImportOption>> importOptionClasses;
         private final Set<Location> locations;
 
-        private LocationsKey(Class<? extends ImportOption> importOptionClass, Set<Location> locations) {
-            this.importOptionClass = importOptionClass;
+        private LocationsKey(Class<? extends ImportOption>[] importOptionClasses, Set<Location> locations) {
+            this.importOptionClasses = ImmutableSet.copyOf(importOptionClasses);
             this.locations = locations;
         }
 
         @Override
         public int hashCode() {
-            return Objects.hash(importOptionClass, locations);
+            return Objects.hash(importOptionClasses, locations);
         }
 
         @Override
@@ -155,7 +159,7 @@ public boolean equals(Object obj) {
                 return false;
             }
             final LocationsKey other = (LocationsKey) obj;
-            return Objects.equals(this.importOptionClass, other.importOptionClass)
+            return Objects.equals(this.importOptionClasses, other.importOptionClasses)
                     && Objects.equals(this.locations, other.locations);
         }
     }

archunit-junit/src/test/java/com/tngtech/archunit/junit/ClassCacheConcurrencyTest.java

@@ -8,7 +8,7 @@
 import java.util.concurrent.Future;
 
 import com.tngtech.archunit.Slow;
-import com.tngtech.archunit.core.importer.ImportOption;
+import com.tngtech.archunit.core.importer.ImportOptions;
 import com.tngtech.archunit.junit.ClassCache.CacheClassFileImporter;
 import org.junit.Rule;
 import org.junit.Test;
@@ -53,7 +53,7 @@ public void concurrent_access() throws Exception {
         for (Future<?> future : futures) {
             future.get(1, MINUTES);
         }
-        verify(classFileImporter, atMost(TEST_CLASSES.size())).importClasses(any(ImportOption.class), anyCollection());
+        verify(classFileImporter, atMost(TEST_CLASSES.size())).importClasses(any(ImportOptions.class), anyCollection());
         verifyNoMoreInteractions(classFileImporter);
     }
 

archunit-junit/src/test/java/com/tngtech/archunit/junit/ClassCacheTest.java

@@ -3,6 +3,7 @@
 import com.tngtech.archunit.core.domain.JavaClass;
 import com.tngtech.archunit.core.domain.JavaClasses;
 import com.tngtech.archunit.core.importer.ImportOption;
+import com.tngtech.archunit.core.importer.ImportOptions;
 import com.tngtech.archunit.core.importer.Location;
 import com.tngtech.archunit.junit.ClassCache.CacheClassFileImporter;
 import com.tngtech.archunit.testutil.ArchConfigurationRule;
@@ -127,7 +128,7 @@ public void distinguishes_import_option_when_caching() {
     }
 
     private void verifyNumberOfImports(int number) {
-        verify(cacheClassFileImporter, times(number)).importClasses(any(ImportOption.class), anyCollection());
+        verify(cacheClassFileImporter, times(number)).importClasses(any(ImportOptions.class), anyCollection());
         verifyNoMoreInteractions(cacheClassFileImporter);
     }
 
@@ -147,11 +148,11 @@ public static class TestClassWithFilterJustByPackageOfClass {
     public static class TestClassWithNonExistingPackage {
     }
 
-    @AnalyzeClasses(importOption = TestFilterForJUnitJars.class)
+    @AnalyzeClasses(importOptions = TestFilterForJUnitJars.class)
     public static class TestClassFilteringJustJUnitJars {
     }
 
-    @AnalyzeClasses(importOption = AnotherTestFilterForJUnitJars.class)
+    @AnalyzeClasses(importOptions = AnotherTestFilterForJUnitJars.class)
     public static class TestClassFilteringJustJUnitJarsWithDifferentFilter {
     }
 

archunit-maven-test/src/test/java/com/tngtech/archunit/maventest/ArchUnitSmokeTest.java

@@ -3,6 +3,7 @@
 import com.tngtech.archunit.core.domain.JavaClass;
 import com.tngtech.archunit.core.domain.JavaClasses;
 import com.tngtech.archunit.core.importer.ImportOption;
+import com.tngtech.archunit.core.importer.ImportOption.DontIncludeTests;
 import com.tngtech.archunit.core.importer.Location;
 import com.tngtech.archunit.junit.AnalyzeClasses;
 import com.tngtech.archunit.junit.ArchTest;
@@ -12,7 +13,7 @@
 import static org.junit.Assert.assertEquals;
 
 @RunWith(ArchUnitRunner.class)
-@AnalyzeClasses(packages = "com.tngtech.archunit.maventest", importOption = ArchUnitSmokeTest.NoTests.class)
+@AnalyzeClasses(packages = "com.tngtech.archunit.maventest", importOptions = DontIncludeTests.class)
 public class ArchUnitSmokeTest {
     @ArchTest
     public static void runs_without_exception(JavaClasses classes) {
@@ -27,10 +28,4 @@ public static void runs_without_exception(JavaClasses classes) {
         assertEquals("Number of fields in ClassTwo", classes.get(ClassTwo.class).getFields().size(), 0);
         assertEquals("Number of methods in ClassTwo", classes.get(ClassTwo.class).getMethods().size(), 1);
     }
-
-    public static class NoTests implements ImportOption {
-        public boolean includes(Location location) {
-            return !location.asURI().toString().contains("test-classes");
-        }
-    }
 }

archunit/src/main/java/com/tngtech/archunit/core/importer/ClassFileImporter.java

@@ -37,6 +37,12 @@
 import static java.util.Collections.singleton;
 import static java.util.Collections.singletonList;
 
+/**
+ * The central API to import {@link JavaClasses}. Supports various types of {@link Location}, e.g. {@link Path},
+ * {@link JarFile} or {@link URL}. The {@link Location}s that are scanned, can be filtered by passing any number of
+ * {@link ImportOption} to {@link #withImportOption(ImportOption)}, which will then be <b>AND</b>ed (compare
+ * {@link ImportOptions}.
+ */
 public final class ClassFileImporter {
     private final ImportOptions importOptions;
 

archunit/src/main/java/com/tngtech/archunit/core/importer/ImportOption.java

@@ -15,7 +15,6 @@
  */
 package com.tngtech.archunit.core.importer;
 
-import java.io.File;
 import java.util.Set;
 
 import com.google.common.collect.ImmutableSet;
@@ -60,26 +59,17 @@ public boolean includes(Location location) {
         }
     }
 
-    final class Everything implements ImportOption {
-        @Override
-        public boolean includes(Location location) {
-            return true;
-        }
-    }
-
     /**
-     * NOTE: This excludes all class files residing in some directory ../test/.. or
-     * ../test-classes/.. (Maven/Gradle standard), so don't use this, if you have a package
-     * test that you want to import.
+     * NOTE: This excludes all class files residing in some directory
+     * ../target/test-classes/.. or ../build/classes/test/.. (Maven/Gradle standard).
+     * Thus it is just a best guess, how tests can be identified,
+     * in other environments, it might be necessary, to implement the correct {@link ImportOption} yourself.
      */
     final class DontIncludeTests implements ImportOption {
-        private static final Set<String> EXCLUDED_INFIXES = ImmutableSet.of(
-                anyFolder("test"),
-                anyFolder("test-classes"));
+        private static final String MAVEN_INFIX = "/target/test-classes/";
+        private static final String GRADLE_INFIX = "/build/classes/test/";
 
-        private static String anyFolder(String infix) {
-            return String.format("%s%s%s", File.separator, infix, File.separator);
-        }
+        private static final Set<String> EXCLUDED_INFIXES = ImmutableSet.of(MAVEN_INFIX, GRADLE_INFIX);
 
         @Override
         public boolean includes(Location location) {

archunit/src/main/java/com/tngtech/archunit/core/importer/ImportOptions.java

@@ -24,6 +24,11 @@
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.tngtech.archunit.PublicAPI.Usage.ACCESS;
 
+/**
+ * A collection of {@link ImportOption} to filter class locations. All supplied {@link ImportOption}s will be joined
+ * with <b>AND</b>, i.e. only {@link Location}s that are accepted by <b>all</b> {@link ImportOption}s
+ * will be imported.
+ */
 public final class ImportOptions {
     private final Set<ImportOption> options;
 
@@ -36,6 +41,10 @@ private ImportOptions(Set<ImportOption> options) {
         this.options = checkNotNull(options);
     }
 
+    /**
+     * @param option An {@link ImportOption} to evaluate on {@link Location}s of class files
+     * @return self to add further {@link ImportOption}s in a fluent way
+     */
     @PublicAPI(usage = ACCESS)
     public ImportOptions with(ImportOption option) {
         return new ImportOptions(ImmutableSet.<ImportOption>builder().addAll(options).add(option).build());

archunit/src/main/java/com/tngtech/archunit/core/importer/Location.java

@@ -33,6 +33,11 @@
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.tngtech.archunit.PublicAPI.Usage.ACCESS;
 
+/**
+ * Handles various forms of location, from where classes can be imported, in a consistent way. Any location
+ * will be treated like an {@link URI}, thus there won't be any platform dependent file separator problems,
+ * or similar.
+ */
 public abstract class Location {
     private static final String FILE_SCHEME = "file";
     private static final String JAR_SCHEME = "jar";
@@ -50,6 +55,10 @@ public URI asURI() {
 
     abstract ClassFileSource asClassFileSource(ImportOptions importOptions);
 
+    /**
+     * @param part A part to check the respective location {@link URI} for
+     * @return true, if the respective {@link URI} contains the given part
+     */
     @PublicAPI(usage = ACCESS)
     public boolean contains(String part) {
         return uri.toString().contains(part);

archunit/src/main/java/com/tngtech/archunit/lang/SimpleConditionEvent.java

@@ -56,7 +56,7 @@ public void describeTo(CollectsLines messages) {
 
     @Override
     public T getCorrespondingObject() {
-        return null;
+        return correspondingObject;
     }
 
     @Override

archunit/src/test/java/com/tngtech/archunit/core/importer/DontIncludeTestsTest.java

@@ -37,16 +37,18 @@ public void excludes_test_class() {
     @DataProvider
     public static Object[][] folders() {
         return $$(
-                $("test", false),
-                $("test-classes", false),
-                $("classes", true),
-                $("main", true)
+                $(new String[]{"build", "classes", "test"}, false),
+                $(new String[]{"target", "classes", "test"}, true),
+                $(new String[]{"target", "test-classes"}, false),
+                $(new String[]{"build", "test-classes"}, true),
+                $(new String[]{"build", "classes", "main"}, true),
+                $(new String[]{"target", "classes"}, true)
         );
     }
 
     @Test
     @UseDataProvider("folders")
-    public void detects_both_Gradle_and_Maven_style(String folderName, boolean expectedInclude) throws IOException {
+    public void detects_both_Gradle_and_Maven_style(String[] folderName, boolean expectedInclude) throws IOException {
         File folder = temporaryFolder.newFolder(folderName);
         File targetFile = new File(folder, getClass().getSimpleName() + ".class");
         Files.copy(locationOf(getClass()).asURI().toURL().openStream(), targetFile.toPath());