make @AnalyzeClasses import package of test class by default #828

In particular coming from a Spring framework background it is a somewhat surprising behavior that a plain `@AnalyzeClasses` without any options will import the whole classpath. The more intuitive behavior would be to import the package of the annotated class. Since importing the whole classpath is likely not what most users want, and the performance drain is large, we will change the behavior to import the package of the annotated class by default if no further locations are specified.
However, this might break some existing use cases where scanning the whole classpath in combination with `ImportOptions` was used consciously. I.e. something like

```
@AnalyzeClasses(importOptions = MySpecialFilter.class)
```

To make sure ArchUnit can still satisfy this use case we introduce a new property `@AnalyzeClasses(wholeClasspath = true)`, which is `false` by default, but can be set to `true` to restore the old behavior.

archunit-junit/build.gradle

@@ -50,6 +50,10 @@ dependencies {
         exclude module: 'guava'
     }
     testImplementation project(path: ':archunit', configuration: 'tests')
+
+    // This is a hack for running tests with IntelliJ instead of delegating to Gradle,
+    // because for some reason this dependency cannot be resolved otherwise :-(
+    testRuntimeOnly dependency.asm
 }
 
 archUnitTest {

archunit-junit/junit4/build.gradle

@@ -27,7 +27,7 @@ dependencies {
     testImplementation project(path: ':archunit-junit', configuration: 'tests')
 
     // This is a hack for running tests with IntelliJ instead of delegating to Gradle,
-    // because for some reason this dependency cannot be resolved otherwise only for archunit-junit4 :-(
+    // because for some reason this dependency cannot be resolved otherwise :-(
     testRuntimeOnly dependency.asm
 }
 

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

@@ -62,6 +62,12 @@
      */
     Class<? extends LocationProvider>[] locations() default {};
 
+    /**
+     * @return Whether to import all classes on the classpath.
+     *         Warning: Without any further filtering this can require a lot of resources.
+     */
+    boolean wholeClasspath() default false;
+
     /**
      * 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

archunit-junit/junit4/src/main/java/com/tngtech/archunit/junit/ArchUnitRunner.java

@@ -217,5 +217,10 @@ public Class<? extends ImportOption>[] getImportOptions() {
         public CacheMode getCacheMode() {
             return analyzeClasses.cacheMode();
         }
+
+        @Override
+        public boolean scanWholeClasspath() {
+            return analyzeClasses.wholeClasspath();
+        }
     }
 }

archunit-junit/junit4/src/test/java/com/tngtech/archunit/junit/ArchUnitRunnerTest.java

@@ -66,6 +66,7 @@ public void runner_creates_correct_analysis_request() {
         assertThat(analysisRequest.getPackageNames()).isEqualTo(analyzeClasses.packages());
         assertThat(analysisRequest.getPackageRoots()).isEqualTo(analyzeClasses.packagesOf());
         assertThat(analysisRequest.getLocationProviders()).isEqualTo(analyzeClasses.locations());
+        assertThat(analysisRequest.scanWholeClasspath()).as("scan whole classpath").isTrue();
         assertThat(analysisRequest.getImportOptions()).isEqualTo(analyzeClasses.importOptions());
     }
 
@@ -149,11 +150,12 @@ public boolean includes(Location location) {
             packages = {"com.foo", "com.bar"},
             packagesOf = {ArchUnitRunner.class, ArchUnitRunnerTest.class},
             locations = {DummyLocation.class, OtherDummyLocation.class},
+            wholeClasspath = true,
             importOptions = {DummyImportOption.class, OtherDummyImportOption.class}
     )
     public static class MaxAnnotatedTest {
         @ArchTest
         public static void someTest(JavaClasses classes) {
         }
     }
-}
+}

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

@@ -66,6 +66,12 @@
      */
     Class<? extends LocationProvider>[] locations() default {};
 
+    /**
+     * @return Whether to look for classes on the whole classpath.
+     *         Warning: Without any further {@link #importOptions() filtering} this can require a lot of resources.
+     */
+    boolean wholeClasspath() default false;
+
     /**
      * 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

archunit-junit/junit5/engine/src/main/java/com/tngtech/archunit/junit/ArchUnitTestDescriptor.java

@@ -289,5 +289,10 @@ public Class<? extends ImportOption>[] getImportOptions() {
         public CacheMode getCacheMode() {
             return analyzeClasses.cacheMode();
         }
+
+        @Override
+        public boolean scanWholeClasspath() {
+            return analyzeClasses.wholeClasspath();
+        }
     }
 }

archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/ArchUnitTestEngineTest.java

@@ -879,6 +879,7 @@ void passes_AnalyzeClasses_to_cache() {
             assertThat(request.getPackageNames()).isEqualTo(expected.packages());
             assertThat(request.getPackageRoots()).isEqualTo(expected.packagesOf());
             assertThat(request.getLocationProviders()).isEqualTo(expected.locations());
+            assertThat(request.scanWholeClasspath()).as("scan whole classpath").isTrue();
             assertThat(request.getImportOptions()).isEqualTo(expected.importOptions());
         }
 

archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/testexamples/FullAnalyzeClassesSpec.java

@@ -23,6 +23,7 @@
         packages = {"first.pkg", "second.pkg"},
         packagesOf = {Object.class, File.class},
         locations = {FirstLocationProvider.class, SecondLocationProvider.class},
+        wholeClasspath = true,
         importOptions = {DoNotIncludeTests.class, DoNotIncludeJars.class})
 public class FullAnalyzeClassesSpec {
     @ArchTest

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

@@ -15,4 +15,6 @@ interface ClassAnalysisRequest {
     Class<? extends ImportOption>[] getImportOptions();
 
     CacheMode getCacheMode();
+
+    boolean scanWholeClasspath();
 }

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

@@ -16,6 +16,7 @@
 package com.tngtech.archunit.junit;
 
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Objects;
@@ -154,40 +155,43 @@ private abstract static class RequestedLocations {
 
         abstract LocationsKey asKey();
 
-        private static class All extends RequestedLocations {
-            private Class<? extends ImportOption>[] importOptions;
+        private static class AllInTestPackage extends RequestedLocations {
+            private final Class<? extends ImportOption>[] importOptions;
+            private final Set<Location> locationsOfTestPackage;
 
-            private All(Class<? extends ImportOption>[] importOptions) {
+            private AllInTestPackage(Class<? extends ImportOption>[] importOptions, Class<?> testClass) {
                 this.importOptions = importOptions;
+                locationsOfTestPackage = Locations.ofPackage(testClass.getPackage().getName());
             }
 
             @Override
             LocationsKey asKey() {
-                return new LocationsKey(importOptions, Locations.inClassPath());
+                return new LocationsKey(importOptions, locationsOfTestPackage);
             }
         }
 
         private static class Specific extends RequestedLocations {
-            private final ClassAnalysisRequest classAnalysisRequest;
+            private final Class<? extends ImportOption>[] importOptions;
             private final Set<Location> declaredLocations;
 
             private Specific(ClassAnalysisRequest classAnalysisRequest, Class<?> testClass) {
-                this.classAnalysisRequest = classAnalysisRequest;
+                importOptions = classAnalysisRequest.getImportOptions();
                 declaredLocations = ImmutableSet.<Location>builder()
-                        .addAll(getLocationsOfPackages())
-                        .addAll(getLocationsOfProviders(testClass))
+                        .addAll(getLocationsOfPackages(classAnalysisRequest))
+                        .addAll(getLocationsOfProviders(classAnalysisRequest, testClass))
+                        .addAll(classAnalysisRequest.scanWholeClasspath() ? Locations.inClassPath() : Collections.<Location>emptySet())
                         .build();
             }
 
-            private Set<Location> getLocationsOfPackages() {
+            private Set<Location> getLocationsOfPackages(ClassAnalysisRequest classAnalysisRequest) {
                 Set<String> packages = ImmutableSet.<String>builder()
                         .add(classAnalysisRequest.getPackageNames())
                         .addAll(toPackageStrings(classAnalysisRequest.getPackageRoots()))
                         .build();
                 return locationsOf(packages);
             }
 
-            private Set<Location> getLocationsOfProviders(Class<?> testClass) {
+            private Set<Location> getLocationsOfProviders(ClassAnalysisRequest classAnalysisRequest, Class<?> testClass) {
                 Set<Location> result = new HashSet<>();
                 for (Class<? extends LocationProvider> providerClass : classAnalysisRequest.getLocationProviders()) {
                     result.addAll(tryCreate(providerClass).get(testClass));
@@ -224,20 +228,21 @@ private Set<Location> locationsOf(Set<String> packages) {
 
             @Override
             LocationsKey asKey() {
-                return new LocationsKey(classAnalysisRequest.getImportOptions(), declaredLocations);
+                return new LocationsKey(importOptions, declaredLocations);
             }
         }
 
         public static RequestedLocations by(ClassAnalysisRequest classAnalysisRequest, Class<?> testClass) {
             return noSpecificLocationRequested(classAnalysisRequest) ?
-                    new All(classAnalysisRequest.getImportOptions()) :
+                    new AllInTestPackage(classAnalysisRequest.getImportOptions(), testClass) :
                     new Specific(classAnalysisRequest, testClass);
         }
 
         private static boolean noSpecificLocationRequested(ClassAnalysisRequest classAnalysisRequest) {
             return classAnalysisRequest.getPackageNames().length == 0
                     && classAnalysisRequest.getPackageRoots().length == 0
-                    && classAnalysisRequest.getLocationProviders().length == 0;
+                    && classAnalysisRequest.getLocationProviders().length == 0
+                    && !classAnalysisRequest.scanWholeClasspath();
         }
     }
 }

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

@@ -6,6 +6,7 @@
 
 import com.tngtech.archunit.core.domain.JavaClass;
 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;
@@ -15,6 +16,7 @@
 import com.tngtech.java.junit.dataprovider.DataProvider;
 import com.tngtech.java.junit.dataprovider.DataProviderRunner;
 import com.tngtech.java.junit.dataprovider.UseDataProvider;
+import org.assertj.core.api.Condition;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -32,6 +34,7 @@
 import static com.tngtech.java.junit.dataprovider.DataProviders.testForEach;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
@@ -125,7 +128,6 @@ public void get_all_classes_by_LocationProvider() {
 
     @Test
     public void rejects_LocationProviders_without_default_constructor() {
-
         thrown.expect(ArchTestExecutionException.class);
         thrown.expectMessage("public default constructor");
         thrown.expectMessage(LocationProvider.class.getSimpleName());
@@ -134,10 +136,38 @@ public void rejects_LocationProviders_without_default_constructor() {
                 analyzeLocation(WrongLocationProviderWithConstructorParam.class));
     }
 
+    @Test
+    public void if_no_import_locations_are_specified_and_whole_classpath_is_set_false_then_the_default_is_the_package_of_the_test_class() {
+        TestAnalysisRequest defaultOptions = new TestAnalysisRequest().withWholeClasspath(false);
+
+        JavaClasses classes = cache.getClassesToAnalyzeFor(TestClass.class, defaultOptions);
+
+        assertThatTypes(classes).contain(getClass(), TestAnalysisRequest.class);
+        assertThatTypes(classes).doNotContain(ClassFileImporter.class);
+    }
+
+    @Test
+    public void if_whole_classpath_is_set_true_then_the_whole_classpath_is_imported() {
+        TestAnalysisRequest defaultOptions = new TestAnalysisRequest().withWholeClasspath(true);
+        Class<?>[] expectedImportResult = new Class[]{getClass()};
+        doReturn(new ClassFileImporter().importClasses(expectedImportResult))
+                .when(cacheClassFileImporter).importClasses(any(ImportOptions.class), ArgumentMatchers.<Location>anyCollection());
+
+        JavaClasses classes = cache.getClassesToAnalyzeFor(TestClass.class, defaultOptions);
+
+        assertThatTypes(classes).matchExactly(expectedImportResult);
+        verify(cacheClassFileImporter).importClasses(any(ImportOptions.class), locationCaptor.capture());
+        assertThat(locationCaptor.getValue())
+                .has(locationContaining("archunit"))
+                .has(locationContaining("asm"))
+                .has(locationContaining("google"))
+                .has(locationContaining("mockito"));
+    }
+
     @Test
     public void filters_urls() {
         JavaClasses classes = cache.getClassesToAnalyzeFor(TestClass.class,
-                new TestAnalysisRequest().withImportOptions(TestFilterForJUnitJars.class));
+                new TestAnalysisRequest().withImportOptions(TestFilterForJUnitJars.class).withWholeClasspath(true));
 
         assertThat(classes).isNotEmpty();
         for (JavaClass clazz : classes) {
@@ -218,6 +248,20 @@ private void verifyNumberOfImports(int number) {
         verifyNoMoreInteractions(cacheClassFileImporter);
     }
 
+    private static Condition<Iterable<? extends Location>> locationContaining(final String part) {
+        return new Condition<Iterable<? extends Location>>() {
+            @Override
+            public boolean matches(Iterable<? extends Location> locations) {
+                for (Location location : locations) {
+                    if (location.contains(part)) {
+                        return true;
+                    }
+                }
+                return false;
+            }
+        };
+    }
+
     public static class TestClass {
     }
 

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

@@ -7,6 +7,7 @@ class TestAnalysisRequest implements ClassAnalysisRequest {
     private String[] packages = new String[0];
     private Class<?>[] packageRoots = new Class<?>[0];
     private Class<? extends LocationProvider>[] locationProviders = new Class[0];
+    private boolean wholeClasspath = false;
     private Class<? extends ImportOption>[] importOptions = new Class[0];
     private CacheMode cacheMode = CacheMode.FOREVER;
 
@@ -25,6 +26,11 @@ public Class<? extends LocationProvider>[] getLocationProviders() {
         return locationProviders;
     }
 
+    @Override
+    public boolean scanWholeClasspath() {
+        return wholeClasspath;
+    }
+
     @Override
     public Class<? extends ImportOption>[] getImportOptions() {
         return importOptions;
@@ -51,6 +57,11 @@ final TestAnalysisRequest withLocationProviders(Class<? extends LocationProvider
         return this;
     }
 
+    final TestAnalysisRequest withWholeClasspath(boolean wholeClasspath) {
+        this.wholeClasspath = wholeClasspath;
+        return this;
+    }
+
     @SafeVarargs
     final TestAnalysisRequest withImportOptions(Class<? extends ImportOption>... importOptions) {
         this.importOptions = importOptions;