Some minor code improvements (#965)

Contributes some random code improvements for minor _"issues"_ detected
by the IDE:
* make some fields final
* make some inner classes static
* simplify code: remove redundant interface definition/cast/;, !anyMatch
-> noneMatch
* remove useless throws declarations
* update Mockito and use mockito-junit-jupiter's MockitoExtension
* replace deprecated `com.google.common.io.Files` API
* replace `assertThat(x).containsOnlyElementsOf` with
`.hasSameElementsAs`
* replace deprecated JUnit 4 `ExpectedException`

archunit-junit/junit4/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerRunsRuleFieldsTest.java

@@ -13,7 +13,6 @@
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
-import org.junit.rules.ExpectedException;
 import org.junit.runner.Description;
 import org.junit.runner.notification.Failure;
 import org.junit.runner.notification.RunNotifier;
@@ -38,15 +37,14 @@
 import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes;
 import static java.lang.annotation.RetentionPolicy.RUNTIME;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 public class ArchUnitRunnerRunsRuleFieldsTest {
-    @Rule
-    public final ExpectedException thrown = ExpectedException.none();
     @Rule
     public final MockitoRule mockitoRule = MockitoJUnit.rule();
     @Rule
@@ -69,7 +67,7 @@ public class ArchUnitRunnerRunsRuleFieldsTest {
     @InjectMocks
     private ArchUnitRunnerInternal runner = ArchUnitRunnerTestUtils.newRunnerFor(SomeArchTest.class);
 
-    private JavaClasses cachedClasses = importClassesWithContext(Object.class);
+    private final JavaClasses cachedClasses = importClassesWithContext(Object.class);
 
     @Before
     public void setUp() {
@@ -123,11 +121,11 @@ public void should_allow_instance_field_in_abstract_base_class() {
     public void should_fail_on_wrong_field_type() {
         ArchUnitRunnerInternal runner = newRunnerFor(WrongArchTestWrongFieldType.class, cache);
 
-        thrown.expectMessage("Rule field " +
-                WrongArchTestWrongFieldType.class.getSimpleName() + "." + NO_RULE_AT_ALL_FIELD_NAME +
-                " to check must be of type " + ArchRule.class.getSimpleName());
-
-        runner.runChild(ArchUnitRunnerTestUtils.getRule(NO_RULE_AT_ALL_FIELD_NAME, runner), runNotifier);
+        assertThatThrownBy(
+                () -> runner.runChild(ArchUnitRunnerTestUtils.getRule(NO_RULE_AT_ALL_FIELD_NAME, runner), runNotifier)
+        )
+                .hasMessage("Rule field %s.%s to check must be of type %s",
+                        WrongArchTestWrongFieldType.class.getSimpleName(), NO_RULE_AT_ALL_FIELD_NAME, ArchRule.class.getSimpleName());
     }
 
     @Test

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

@@ -17,7 +17,6 @@
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
-import org.junit.rules.ExpectedException;
 import org.junit.runner.notification.RunNotifier;
 import org.junit.runners.model.InitializationError;
 import org.mockito.ArgumentCaptor;
@@ -30,6 +29,7 @@
 import static com.tngtech.archunit.core.domain.TestUtils.importClasses;
 import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.verify;
@@ -38,8 +38,6 @@
 public class ArchUnitRunnerTest {
     @Rule
     public final MockitoRule mockitoRule = MockitoJUnit.rule();
-    @Rule
-    public final ExpectedException thrown = ExpectedException.none();
 
     @Mock
     private ClassCache cache;
@@ -88,13 +86,14 @@ public void runner_clears_cache_after_exception_during_test_run() {
     }
 
     @Test
-    public void rejects_missing_analyze_annotation() throws InitializationError {
-        thrown.expect(ArchTestInitializationException.class);
-        thrown.expectMessage(Object.class.getSimpleName());
-        thrown.expectMessage("must be annotated");
-        thrown.expectMessage(AnalyzeClasses.class.getSimpleName());
-
-        new ArchUnitRunnerInternal(Object.class);
+    public void rejects_missing_analyze_annotation() {
+        assertThatThrownBy(
+                () -> new ArchUnitRunnerInternal(Object.class)
+        )
+                .isInstanceOf(ArchTestInitializationException.class)
+                .hasMessageContaining(Object.class.getSimpleName())
+                .hasMessageContaining("must be annotated")
+                .hasMessageContaining(AnalyzeClasses.class.getSimpleName());
     }
 
     private ArchUnitRunnerInternal newRunner(Class<?> testClass) {

archunit-junit/junit5/engine/build.gradle

@@ -17,6 +17,7 @@ dependencies {
     testImplementation project(path: ':archunit', configuration: 'tests')
     testImplementation dependency.assertj
     testImplementation dependency.mockito
+    testImplementation dependency.mockito_junit5
     testImplementation dependency.junit5JupiterApi
     testImplementation dependency.log4j_core
     testImplementation dependency.log4j_slf4j

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

@@ -71,6 +71,7 @@
 public final class ArchUnitTestEngine extends HierarchicalTestEngine<ArchUnitEngineExecutionContext> {
     static final String UNIQUE_ID = "archunit";
 
+    @SuppressWarnings("FieldMayBeFinal")
     private SharedCache cache = new SharedCache(); // NOTE: We want to change this in tests -> no static/final reference
 
     @Override

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

@@ -159,7 +159,7 @@ private static Deque<UniqueId.Segment> getRemainingSegments(UniqueId rootId, Uni
         return remainingSegments;
     }
 
-    abstract class PossiblyResolvedClass {
+    abstract static class PossiblyResolvedClass {
         void ifRequestedButUnresolved(BiConsumer<Class<?>, ElementResolver> doIfResolved) {
         }
 
@@ -168,7 +168,7 @@ PossiblyResolvedClass ifRequestedAndResolved(BiConsumer<CreatesChildren, Element
         }
     }
 
-    private class RequestedAndSuccessfullyResolvedClass extends PossiblyResolvedClass {
+    private static class RequestedAndSuccessfullyResolvedClass extends PossiblyResolvedClass {
         private final CreatesChildren classDescriptor;
         private final ElementResolver childResolver;
 
@@ -203,22 +203,22 @@ void ifRequestedButUnresolved(BiConsumer<Class<?>, ElementResolver> doWithChildR
         }
     }
 
-    private class ClassNotRequested extends PossiblyResolvedClass {
+    private static class ClassNotRequested extends PossiblyResolvedClass {
     }
 
-    abstract class PossiblyResolvedMember {
+    abstract static class PossiblyResolvedMember {
         abstract void ifUnresolved(Consumer<ElementResolver> childResolver);
     }
 
-    private class SuccessfullyResolvedMember extends PossiblyResolvedMember {
+    private static class SuccessfullyResolvedMember extends PossiblyResolvedMember {
         @Override
         void ifUnresolved(Consumer<ElementResolver> childResolver) {
         }
     }
 
     private class UnresolvedMember extends PossiblyResolvedMember {
         private final Member member;
-        private String segmentType;
+        private final String segmentType;
 
         UnresolvedMember(Member member, String segmentType) {
             this.member = member;

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

@@ -54,7 +54,6 @@
 import com.tngtech.archunit.junit.internal.testexamples.wrong.WrongRuleMethodNotStatic;
 import com.tngtech.archunit.junit.internal.testexamples.wrong.WrongRuleMethodWrongParameters;
 import com.tngtech.archunit.junit.internal.testutil.LogCaptor;
-import com.tngtech.archunit.junit.internal.testutil.MockitoExtension;
 import com.tngtech.archunit.junit.internal.testutil.TestLogExtension;
 import com.tngtech.archunit.testutil.TestLogRecorder.RecordedLogEvent;
 import org.junit.jupiter.api.AfterEach;
@@ -75,6 +74,9 @@
 import org.mockito.Captor;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.mockito.junit.jupiter.MockitoSettings;
+import org.mockito.quality.Strictness;
 
 import static com.google.common.collect.Iterables.getLast;
 import static com.google.common.collect.Iterables.getOnlyElement;
@@ -116,6 +118,7 @@
 import static org.mockito.Mockito.when;
 
 @ExtendWith(MockitoExtension.class)
+@MockitoSettings(strictness = Strictness.WARN)
 class ArchUnitTestEngineTest {
     @Mock
     private ClassCache classCache;

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

@@ -67,6 +67,7 @@ public LazyJavaClasses load(LocationsKey key) {
                 }
             });
 
+    @SuppressWarnings("FieldMayBeFinal") // We want to change this in tests
     private CacheClassFileImporter cacheClassFileImporter = new CacheClassFileImporter();
 
     JavaClasses getClassesToAnalyzeFor(Class<?> testClass, ClassAnalysisRequest classAnalysisRequest) {

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

@@ -19,7 +19,6 @@
 import org.assertj.core.api.Condition;
 import org.junit.Rule;
 import org.junit.Test;
-import org.junit.rules.ExpectedException;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
 import org.mockito.Captor;
@@ -32,6 +31,7 @@
 import static com.tngtech.archunit.testutil.Assertions.assertThatTypes;
 import static com.tngtech.java.junit.dataprovider.DataProviders.testForEach;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.anyCollection;
 import static org.mockito.ArgumentMatchers.anySet;
 import static org.mockito.Mockito.doReturn;
@@ -45,9 +45,6 @@ public class ClassCacheTest {
     @Rule
     public final MockitoRule mockitoRule = MockitoJUnit.rule();
 
-    @Rule
-    public final ExpectedException thrown = ExpectedException.none();
-
     @Rule
     public final ArchConfigurationRule archConfigurationRule = new ArchConfigurationRule()
             .resolveAdditionalDependenciesFromClassPath(false);
@@ -128,12 +125,13 @@ 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());
-
-        cache.getClassesToAnalyzeFor(WrongLocationProviderWithConstructorParam.class,
-                analyzeLocation(WrongLocationProviderWithConstructorParam.class));
+        assertThatThrownBy(
+                () -> cache.getClassesToAnalyzeFor(WrongLocationProviderWithConstructorParam.class,
+                        analyzeLocation(WrongLocationProviderWithConstructorParam.class))
+        )
+                .isInstanceOf(ArchTestExecutionException.class)
+                .hasMessageContaining("public default constructor")
+                .hasMessageContaining(LocationProvider.class.getSimpleName());
     }
 
     @Test

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

@@ -670,7 +670,7 @@ String getFinishedName(String name) {
                 };
             }
 
-            static JavaTypeFinisher IDENTITY = new JavaTypeFinisher() {
+            static final JavaTypeFinisher IDENTITY = new JavaTypeFinisher() {
                 @Override
                 JavaType finish(JavaType input, ImportedClasses classes) {
                     return input;

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

@@ -29,7 +29,7 @@ class RawAccessRecord {
     final CodeUnit caller;
     final TargetInfo target;
     final int lineNumber;
-    public boolean declaredInLambda;
+    final boolean declaredInLambda;
 
     RawAccessRecord(CodeUnit caller, TargetInfo target, int lineNumber, boolean declaredInLambda) {
         this.caller = checkNotNull(caller);

archunit/src/main/java/com/tngtech/archunit/lang/syntax/GivenObjectsInternal.java

@@ -22,9 +22,8 @@
 import com.tngtech.archunit.lang.ArchRule;
 import com.tngtech.archunit.lang.ClassesTransformer;
 import com.tngtech.archunit.lang.Priority;
-import com.tngtech.archunit.lang.syntax.elements.GivenObjects;
 
-class GivenObjectsInternal<T> extends AbstractGivenObjects<T, GivenObjectsInternal<T>> implements GivenObjects<T> {
+class GivenObjectsInternal<T> extends AbstractGivenObjects<T, GivenObjectsInternal<T>> {
 
     GivenObjectsInternal(Priority priority, ClassesTransformer<T> classesTransformer) {
         this(priority, classesTransformer, Function.identity());

archunit/src/main/java/com/tngtech/archunit/library/dependencies/JohnsonCycleFinder.java

@@ -115,7 +115,7 @@ private boolean findCycles(Result result, int originNodeIndex, JohnsonComponent
 
     static class Result implements Iterable<int[]> {
         private final CycleConfiguration configuration = new CycleConfiguration();
-        private List<int[]> cycles = new ArrayList<>();
+        private final List<int[]> cycles = new ArrayList<>();
         private boolean maxNumberOfCyclesReached = false;
 
         private Result() {

archunit/src/main/java/com/tngtech/archunit/library/freeze/ViolationStoreFactory.java

@@ -19,6 +19,7 @@
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.nio.file.Files;
 import java.util.List;
 import java.util.Properties;
 import java.util.UUID;
@@ -27,7 +28,6 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
 import com.google.common.base.Splitter;
-import com.google.common.io.Files;
 import com.tngtech.archunit.ArchConfiguration;
 import com.tngtech.archunit.base.MayResolveTypesViaReflection;
 import com.tngtech.archunit.lang.ArchRule;
@@ -133,7 +133,7 @@ public void save(ArchRule rule, List<String> violations) {
         private void write(List<String> violations, File ruleDetails) {
             String updatedViolations = Joiner.on("\n").join(escape(violations));
             try {
-                Files.write(updatedViolations, ruleDetails, UTF_8);
+                Files.write(ruleDetails.toPath(), updatedViolations.getBytes(UTF_8));
             } catch (IOException e) {
                 throw new StoreUpdateFailedException(e);
             }

archunit/src/test/java/com/tngtech/archunit/ArchConfigurationTest.java

@@ -12,7 +12,6 @@
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
-import org.junit.rules.ExpectedException;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
@@ -21,15 +20,13 @@
 import static com.tngtech.archunit.testutil.TestUtils.properties;
 import static com.tngtech.archunit.testutil.TestUtils.singleProperty;
 import static com.tngtech.archunit.testutil.TestUtils.toUri;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.assertj.core.api.Assertions.entry;
 
 public class ArchConfigurationTest {
     private static final String PROPERTIES_FILE_NAME = "archconfigtest.properties";
     private final File testPropsFile = new File(new File(toUri(getClass().getResource("/"))), PROPERTIES_FILE_NAME);
 
-    @Rule
-    public final ExpectedException thrown = ExpectedException.none();
-
     @Rule
     public final SystemPropertiesRule systemPropertiesRule = new SystemPropertiesRule();
 
@@ -210,10 +207,12 @@ public void allows_to_specify_custom_properties() {
         assertThat(configuration.containsProperty("not.there"))
                 .as("configuration contains property name 'not.there'").isFalse();
 
-        thrown.expect(NullPointerException.class);
-        thrown.expectMessage("'not.there'");
-        thrown.expectMessage("not configured");
-        configuration.getProperty("not.there");
+        assertThatThrownBy(
+                () -> configuration.getProperty("not.there")
+        )
+                .isInstanceOf(NullPointerException.class)
+                .hasMessageContaining("'not.there'")
+                .hasMessageContaining("not configured");
     }
 
     @Test