Review: I think to be consistent we need to offer both the positive a…

…nd the negative form of `beAccessedByMethodsThat`, just like the other methods.

Signed-off-by: Peter Gafert <[email protected]>

archunit/src/main/java/com/tngtech/archunit/lang/conditions/ArchConditions.java

@@ -1275,13 +1275,16 @@ public static ArchCondition<JavaCodeUnit> onlyBeCalledByConstructorsThat(Describ
     }
 
     @PublicAPI(usage = ACCESS)
-    public static ArchCondition<JavaField> notBeAccessedByMethodsThat(DescribedPredicate<? super JavaMethod> predicate) {
-        return new ArchCondition<JavaField>("not be accessed by methods that " + predicate.getDescription()) {
+    public static ArchCondition<JavaField> beAccessedByMethodsThat(DescribedPredicate<? super JavaMethod> predicate) {
+        return new ArchCondition<JavaField>("be accessed by methods that " + predicate.getDescription()) {
             @Override
             public void check(JavaField field, ConditionEvents events) {
                 field.getAccessesToSelf().stream()
-                        .filter(access -> access.getOrigin() instanceof JavaMethod && predicate.test((JavaMethod) access.getOrigin()))
-                        .forEach(violation -> events.add(SimpleConditionEvent.violated(field, violation.getDescription())));
+                        .filter(access -> access.getOrigin() instanceof JavaMethod)
+                        .forEach(access -> {
+                            boolean satisfied = predicate.test((JavaMethod) access.getOrigin());
+                            events.add(new SimpleConditionEvent(field, satisfied, access.getDescription()));
+                        });
             }
         };
     }

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

@@ -95,8 +95,13 @@ public FieldsShouldInternal notHaveRawType(DescribedPredicate<? super JavaClass>
         return addCondition(not(ArchConditions.haveRawType(predicate)));
     }
 
+    @Override
+    public FieldsShouldInternal beAccessedByMethodsThat(DescribedPredicate<? super JavaMethod> predicate) {
+        return addCondition(ArchConditions.beAccessedByMethodsThat(predicate));
+    }
+
     @Override
     public FieldsShouldInternal notBeAccessedByMethodsThat(DescribedPredicate<? super JavaMethod> predicate) {
-        return addCondition(ArchConditions.notBeAccessedByMethodsThat(predicate));
+        return addCondition(not(ArchConditions.beAccessedByMethodsThat(predicate)));
     }
 }

archunit/src/main/java/com/tngtech/archunit/lang/syntax/elements/FieldsShould.java

@@ -157,11 +157,31 @@ public interface FieldsShould<CONJUNCTION extends FieldsShouldConjunction> exten
     @PublicAPI(usage = ACCESS)
     CONJUNCTION notHaveRawType(DescribedPredicate<? super JavaClass> predicate);
 
+    /**
+     * Asserts that fields are accessed by at least one method matching the given predicate.
+     * This mostly makes sense in the negated form
+     * <pre><code>
+     * {@link ArchRuleDefinition#noFields() noFields()}.{@link GivenFields#should() should()}.{@link #beAccessedByMethodsThat(DescribedPredicate)}
+     * </code></pre>
+     * In this form it is equivalent to
+     * <pre><code>
+     * {@link ArchRuleDefinition#fields()} fields()}.{@link GivenFields#should() should()}.{@link #notBeAccessedByMethodsThat(DescribedPredicate)}
+     * </code></pre>
+     * but might be useful for chaining multiple conditions via <code>{@link FieldsShouldConjunction#andShould()}</code>.
+     *
+     * @param predicate A predicate determining the methods this field should not be accessed by
+     * @return A syntax element that can either be used as working rule, or to continue specifying a more complex rule
+     * @see #notBeAccessedByMethodsThat(DescribedPredicate)
+     */
+    @PublicAPI(usage = ACCESS)
+    CONJUNCTION beAccessedByMethodsThat(DescribedPredicate<? super JavaMethod> predicate);
+
     /**
      * Asserts that fields are not accessed by methods matching the given predicate.
      *
      * @param predicate A predicate determining the methods this field should not be accessed by
      * @return A syntax element that can either be used as working rule, or to continue specifying a more complex rule
+     * @see #beAccessedByMethodsThat(DescribedPredicate)
      */
     @PublicAPI(usage = ACCESS)
     CONJUNCTION notBeAccessedByMethodsThat(DescribedPredicate<? super JavaMethod> predicate);

archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/FieldsShouldTest.java

@@ -6,7 +6,7 @@
 import java.util.Set;
 
 import com.google.common.collect.ImmutableList;
-import com.tngtech.archunit.base.DescribedPredicate;
+import com.tngtech.archunit.core.importer.ClassFileImporter;
 import com.tngtech.archunit.lang.ArchRule;
 import com.tngtech.archunit.lang.EvaluationResult;
 import com.tngtech.java.junit.dataprovider.DataProvider;
@@ -18,12 +18,18 @@
 import static com.google.common.collect.Iterables.getOnlyElement;
 import static com.tngtech.archunit.core.domain.JavaClass.Predicates.equivalentTo;
 import static com.tngtech.archunit.core.domain.TestUtils.importClasses;
+import static com.tngtech.archunit.core.domain.properties.HasName.Predicates.name;
+import static com.tngtech.archunit.lang.conditions.ArchPredicates.have;
 import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.fields;
+import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noFields;
 import static com.tngtech.archunit.lang.syntax.elements.GivenMembersTest.areNoFieldsWithType;
 import static com.tngtech.archunit.lang.syntax.elements.GivenMembersTest.assertViolation;
 import static com.tngtech.archunit.lang.syntax.elements.MembersShouldTest.parseMembers;
+import static com.tngtech.archunit.testutil.Assertions.assertThatRule;
 import static com.tngtech.java.junit.dataprovider.DataProviders.$;
 import static com.tngtech.java.junit.dataprovider.DataProviders.$$;
+import static com.tngtech.java.junit.dataprovider.DataProviders.testForEach;
+import static java.util.regex.Pattern.quote;
 import static org.assertj.core.api.Assertions.assertThat;
 
 @RunWith(DataProviderRunner.class)
@@ -37,18 +43,13 @@ public void complex_field_syntax() {
                 .should().beAnnotatedWith(A.class)
                 .andShould().notBePublic()
                 .orShould().haveRawType(Map.class)
-                .andShould().notBeAccessedByMethodsThat(DescribedPredicate.alwaysTrue())
                 .evaluate(importClasses(ClassWithVariousMembers.class));
 
         assertViolation(result);
-        String fieldAFailures = getOnlyElement(result.filterDescriptionsMatching(s -> s.contains(FIELD_A)).getFailureReport().getDetails());
-        assertThat(fieldAFailures)
+        String failure = getOnlyElement(result.getFailureReport().getDetails());
+        assertThat(failure)
                 .containsPattern(String.format("%s.* does not have raw type %s", FIELD_A, Map.class.getName()))
                 .containsPattern(String.format("%s.* is not annotated with @%s", FIELD_A, A.class.getSimpleName()));
-
-        String fieldDFailures =
-                getOnlyElement(result.filterDescriptionsMatching(description -> description.contains(FIELD_D)).getFailureReport().getDetails());
-        assertThat(fieldDFailures).containsPattern(String.format("%s.* gets field .*%s", METHOD_A, FIELD_D));
     }
 
     @DataProvider
@@ -77,24 +78,49 @@ public void property_predicates(ArchRule rule, Collection<String> expectedViolat
         assertThat(actualFields).hasSameElementsAs(expectedViolatingFields);
     }
 
+    @DataProvider
+    public static Object[][] data_be_accessed_by_methods() {
+        return testForEach(
+                fields().should().notBeAccessedByMethodsThat(have(name("toFind"))),
+                noFields().should().beAccessedByMethodsThat(have(name("toFind")))
+        );
+    }
+
+    @Test
+    @UseDataProvider
+    public void test_be_accessed_by_methods(ArchRule ruleCheckingAccessToMethod) {
+        class ClassWithAccessedField {
+            String field;
+        }
+        @SuppressWarnings("unused")
+        class AccessFromMethod {
+            void toFind(ClassWithAccessedField target) {
+                target.field = "changed";
+            }
+
+            void toIgnore(ClassWithAccessedField target) {
+                target.field = "changed";
+            }
+        }
+
+        assertThatRule(ruleCheckingAccessToMethod)
+                .checking(new ClassFileImporter().importClasses(ClassWithAccessedField.class, AccessFromMethod.class))
+                .hasOnlyOneViolationMatching(".*" + quote(AccessFromMethod.class.getName()) + ".*toFind.*")
+                .hasNoViolationMatching(".*toIgnore.*");
+    }
+
     private static final String FIELD_A = "fieldA";
     private static final String FIELD_B = "fieldB";
     private static final String FIELD_C = "fieldC";
     private static final String FIELD_D = "fieldD";
 
-    private static final String METHOD_A = "methodA()";
-
     @SuppressWarnings({"unused"})
     private static class ClassWithVariousMembers {
         private final String fieldA = "A";
         @A
         protected static final Object fieldB = 'B';
         public List<?> fieldC;
         static Map<?, ?> fieldD;
-
-        public void methodA() {
-            Map<?, ?> tmp = fieldD;
-        }
     }
 
     private @interface A {