Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support to detect accesses of fields by specific methods #1105

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1274,6 +1274,21 @@ public static ArchCondition<JavaCodeUnit> onlyBeCalledByConstructorsThat(Describ
origin.is(matching(JavaConstructor.class, predicate)), GET_CALLS_OF_SELF);
}

@PublicAPI(usage = ACCESS)
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)
.forEach(access -> {
boolean satisfied = predicate.test((JavaMethod) access.getOrigin());
events.add(new SimpleConditionEvent(field, satisfied, access.getDescription()));
});
}
};
}

/**
* Derives an {@link ArchCondition} from a {@link DescribedPredicate}. Similar to {@link ArchCondition#from(DescribedPredicate)},
* but more conveniently creates a message to be used within a 'have'-sentence.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.tngtech.archunit.base.DescribedPredicate;
import com.tngtech.archunit.core.domain.JavaClass;
import com.tngtech.archunit.core.domain.JavaField;
import com.tngtech.archunit.core.domain.JavaMethod;
import com.tngtech.archunit.lang.ArchCondition;
import com.tngtech.archunit.lang.ClassesTransformer;
import com.tngtech.archunit.lang.Priority;
Expand Down Expand Up @@ -93,4 +94,14 @@ public FieldsShouldInternal haveRawType(DescribedPredicate<? super JavaClass> pr
public FieldsShouldInternal notHaveRawType(DescribedPredicate<? super JavaClass> predicate) {
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(not(ArchConditions.beAccessedByMethodsThat(predicate)));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.tngtech.archunit.PublicAPI;
import com.tngtech.archunit.base.DescribedPredicate;
import com.tngtech.archunit.core.domain.JavaClass;
import com.tngtech.archunit.core.domain.JavaMethod;
import com.tngtech.archunit.core.domain.properties.HasName;
import com.tngtech.archunit.lang.syntax.ArchRuleDefinition;

Expand Down Expand Up @@ -156,6 +157,35 @@ 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);

/**
* Asserts that fields are static.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import java.util.Set;

import com.google.common.collect.ImmutableList;
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;
Expand All @@ -17,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)
Expand Down Expand Up @@ -71,6 +78,37 @@ 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";
Expand Down