From 71a524e07d1045747264bcfd1b8f0a6e0a5cce7f Mon Sep 17 00:00:00 2001 From: Peter Gafert Date: Mon, 29 Jul 2019 01:07:59 +0200 Subject: [PATCH] Review: - Added some Javadoc (since I'm not sure if 'full name' can be immediately understood by every user) - Removed the generic return type from the predicates in HasName and HasName.AndFullName since the caller should be responsible to map to the subtype (or it would have to be consistently changed in many other places) - Tried to make some tests more decisive by matching at least some element of the passed collection - Unrelated: Added HasName.AndFullName.Functions.GET_FULL_NAME to be consistent with HasName Signed-off-by: Peter Gafert --- .../archunit/core/domain/AccessTarget.java | 4 ++ .../archunit/core/domain/JavaClass.java | 9 +++ .../archunit/core/domain/JavaCodeUnit.java | 3 + .../archunit/core/domain/JavaField.java | 3 + .../core/domain/properties/HasName.java | 60 ++++++++++++------- .../properties/HasSourceCodeLocation.java | 3 + .../lang/conditions/ArchConditions.java | 20 ++++--- .../lang/syntax/elements/MembersShould.java | 12 ++-- .../core/domain/properties/HasNameTest.java | 9 +++ .../syntax/elements/GivenMembersTest.java | 9 +-- .../syntax/elements/MembersShouldTest.java | 6 +- 11 files changed, 99 insertions(+), 39 deletions(-) diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/AccessTarget.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/AccessTarget.java index 7e2a3edb25..a3f1871f97 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/domain/AccessTarget.java +++ b/archunit/src/main/java/com/tngtech/archunit/core/domain/AccessTarget.java @@ -91,6 +91,10 @@ public JavaClass getOwner() { return owner; } + /** + * @return The full name of this {@link AccessTarget}, i.e. a string containing {@code ${declaringClass}.${name}} for a field and + * {@code ${declaringClass}.${name}(${parameterTypes})} for a code unit + */ @Override @PublicAPI(usage = ACCESS) public String getFullName() { diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClass.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClass.java index 25c4609b97..7cf2cdad97 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClass.java +++ b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClass.java @@ -109,6 +109,9 @@ public Set get() { javaPackage = JavaPackage.simple(this); } + /** + * @return The {@link Source} of this {@link JavaClass}, i.e. where this class has been imported from + */ @PublicAPI(usage = ACCESS) public Optional getSource() { return source; @@ -126,12 +129,18 @@ public String getDescription() { return "Class <" + getName() + ">"; } + /** + * @return The fully qualified name of this {@link JavaClass}, compare {@link Class#getName()} of the Reflection API + */ @Override @PublicAPI(usage = ACCESS) public String getName() { return javaType.getName(); } + /** + * @return The fully qualified name of this {@link JavaClass}, i.e. the result is the same as invoking {@link #getName()} + */ @Override @PublicAPI(usage = ACCESS) public String getFullName() { diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaCodeUnit.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaCodeUnit.java index f6441f4358..db00df07ad 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaCodeUnit.java +++ b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaCodeUnit.java @@ -62,6 +62,9 @@ public abstract class JavaCodeUnit extends JavaMember implements HasParameterTyp fullName = formatMethod(getOwner().getName(), getName(), getRawParameterTypes()); } + /** + * @return The full name of this {@link JavaCodeUnit}, i.e. a string containing {@code ${declaringClass}.${name}(${parameterTypes})} + */ @Override @PublicAPI(usage = ACCESS) public String getFullName() { diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaField.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaField.java index d4ec5b65a2..c64cd3306a 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaField.java +++ b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaField.java @@ -42,6 +42,9 @@ public class JavaField extends JavaMember implements HasType { fieldSupplier = Suppliers.memoize(new ReflectFieldSupplier()); } + /** + * @return The full name of this {@link JavaField}, i.e. a string containing {@code ${declaringClass}.${name}} + */ @Override @PublicAPI(usage = ACCESS) public String getFullName() { diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/properties/HasName.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/properties/HasName.java index fba3042730..fb152f9d4a 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/domain/properties/HasName.java +++ b/archunit/src/main/java/com/tngtech/archunit/core/domain/properties/HasName.java @@ -28,6 +28,9 @@ public interface HasName { String getName(); interface AndFullName extends HasName { + /** + * @return The full name of the given object. Varies by context, for details consult Javadoc of the concrete subclass. + */ @PublicAPI(usage = ACCESS) String getFullName(); @@ -44,8 +47,8 @@ public static DescribedPredicate fullName(String fullName) * Matches full names against a regular expression. */ @PublicAPI(usage = ACCESS) - public static DescribedPredicate fullNameMatching(String regex) { - return new FullNameMatchingPredicate<>(regex); + public static DescribedPredicate fullNameMatching(String regex) { + return new FullNameMatchingPredicate(regex); } private static class FullNameEqualsPredicate extends DescribedPredicate { @@ -62,7 +65,7 @@ public boolean apply(HasName.AndFullName input) { } } - private static class FullNameMatchingPredicate extends DescribedPredicate { + private static class FullNameMatchingPredicate extends DescribedPredicate { private final Pattern pattern; FullNameMatchingPredicate(String regex) { @@ -76,50 +79,63 @@ public boolean apply(HasName.AndFullName input) { } } } + + final class Functions { + private Functions() { + } + + @PublicAPI(usage = ACCESS) + public static final ChainableFunction GET_FULL_NAME = new ChainableFunction() { + @Override + public String apply(HasName.AndFullName input) { + return input.getFullName(); + } + }; + } } final class Predicates { private Predicates() { } - /** - * Matches names against a regular expression. - */ @PublicAPI(usage = ACCESS) - public static DescribedPredicate nameMatching(final String regex) { - return new NameMatchingPredicate<>(regex); + public static DescribedPredicate name(final String name) { + return new NameEqualsPredicate(name); } + /** + * Matches names against a regular expression. + */ @PublicAPI(usage = ACCESS) - public static DescribedPredicate name(final String name) { - return new NameEqualsPredicate(name); + public static DescribedPredicate nameMatching(final String regex) { + return new NameMatchingPredicate(regex); } - private static class NameMatchingPredicate extends DescribedPredicate { - private final Pattern pattern; + private static class NameEqualsPredicate extends DescribedPredicate { + private final String name; - NameMatchingPredicate(String regex) { - super(String.format("name matching '%s'", regex)); - this.pattern = Pattern.compile(regex); + NameEqualsPredicate(String name) { + super(String.format("name '%s'", name)); + this.name = name; } @Override public boolean apply(HasName input) { - return pattern.matcher(input.getName()).matches(); + return input.getName().equals(name); } } - private static class NameEqualsPredicate extends DescribedPredicate { - private final String name; + private static class NameMatchingPredicate extends DescribedPredicate { + private final Pattern pattern; - NameEqualsPredicate(String name) { - super(String.format("name '%s'", name)); - this.name = name; + NameMatchingPredicate(String regex) { + super(String.format("name matching '%s'", regex)); + this.pattern = Pattern.compile(regex); } @Override public boolean apply(HasName input) { - return input.getName().equals(name); + return pattern.matcher(input.getName()).matches(); } } } diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/properties/HasSourceCodeLocation.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/properties/HasSourceCodeLocation.java index 8b242994e1..5547d93ce2 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/domain/properties/HasSourceCodeLocation.java +++ b/archunit/src/main/java/com/tngtech/archunit/core/domain/properties/HasSourceCodeLocation.java @@ -21,6 +21,9 @@ import static com.tngtech.archunit.PublicAPI.Usage.ACCESS; public interface HasSourceCodeLocation { + /** + * @return The {@link SourceCodeLocation} of this object, i.e. how to locate the respective object within the set of source files. + */ @PublicAPI(usage = ACCESS) SourceCodeLocation getSourceCodeLocation(); } diff --git a/archunit/src/main/java/com/tngtech/archunit/lang/conditions/ArchConditions.java b/archunit/src/main/java/com/tngtech/archunit/lang/conditions/ArchConditions.java index a5cb97159a..d699fbdb0e 100644 --- a/archunit/src/main/java/com/tngtech/archunit/lang/conditions/ArchConditions.java +++ b/archunit/src/main/java/com/tngtech/archunit/lang/conditions/ArchConditions.java @@ -99,7 +99,9 @@ import static com.tngtech.archunit.core.domain.properties.CanBeAnnotated.Predicates.metaAnnotatedWith; import static com.tngtech.archunit.core.domain.properties.HasModifiers.Predicates.modifier; import static com.tngtech.archunit.core.domain.properties.HasName.AndFullName.Predicates.fullName; +import static com.tngtech.archunit.core.domain.properties.HasName.AndFullName.Predicates.fullNameMatching; import static com.tngtech.archunit.core.domain.properties.HasName.Predicates.name; +import static com.tngtech.archunit.core.domain.properties.HasName.Predicates.nameMatching; import static com.tngtech.archunit.core.domain.properties.HasOwner.Predicates.With.owner; import static com.tngtech.archunit.core.domain.properties.HasParameterTypes.Predicates.rawParameterTypes; import static com.tngtech.archunit.core.domain.properties.HasReturnType.Predicates.rawReturnType; @@ -452,12 +454,14 @@ public static ArchCondition haveFullName(String fullName) { + public static + ArchCondition haveFullName(String fullName) { return new HaveConditionByPredicate<>(fullName(fullName)); } @PublicAPI(usage = ACCESS) - public static ArchCondition notHaveFullName(String fullName) { + public static + ArchCondition notHaveFullName(String fullName) { return not(new HaveConditionByPredicate(fullName(fullName))); } @@ -526,7 +530,7 @@ public static ArchCondition haveSimpleNameNotEndingWith(String suffix @PublicAPI(usage = ACCESS) public static ArchCondition haveNameMatching(final String regex) { - final DescribedPredicate haveNameMatching = have(HasName.Predicates.nameMatching(regex)); + final DescribedPredicate haveNameMatching = have(nameMatching(regex)).forSubType(); return new MatchingCondition<>(haveNameMatching, regex); } @@ -536,13 +540,15 @@ public static ArchCondition haveFullNameMatching(String regex) { - final DescribedPredicate haveFullNameMatching = have(HasName.AndFullName.Predicates.fullNameMatching(regex)); - return new MatchingCondition(haveFullNameMatching, regex); + public static + ArchCondition haveFullNameMatching(String regex) { + final DescribedPredicate haveFullNameMatching = have(fullNameMatching(regex)).forSubType(); + return new MatchingCondition<>(haveFullNameMatching, regex); } @PublicAPI(usage = ACCESS) - public static ArchCondition haveFullNameNotMatching(String regex) { + public static ArchCondition + haveFullNameNotMatching(String regex) { return not(ArchConditions.haveFullNameMatching(regex)).as("have full name not matching '%s'", regex); } diff --git a/archunit/src/main/java/com/tngtech/archunit/lang/syntax/elements/MembersShould.java b/archunit/src/main/java/com/tngtech/archunit/lang/syntax/elements/MembersShould.java index 5aca2f0fcf..74ce1e7ffd 100644 --- a/archunit/src/main/java/com/tngtech/archunit/lang/syntax/elements/MembersShould.java +++ b/archunit/src/main/java/com/tngtech/archunit/lang/syntax/elements/MembersShould.java @@ -21,7 +21,9 @@ import com.tngtech.archunit.base.DescribedPredicate; import com.tngtech.archunit.core.domain.JavaAnnotation; import com.tngtech.archunit.core.domain.JavaClass; +import com.tngtech.archunit.core.domain.JavaCodeUnit; import com.tngtech.archunit.core.domain.JavaConstructor; +import com.tngtech.archunit.core.domain.JavaField; import com.tngtech.archunit.core.domain.JavaModifier; import com.tngtech.archunit.lang.syntax.ArchRuleDefinition; @@ -68,7 +70,7 @@ public interface MembersShould> CONJUNCTION haveNameNotMatching(String regex); /** - * Asserts that members have a certain full name. + * Asserts that members have a certain full name (compare {@link JavaField#getFullName()} and {@link JavaCodeUnit#getFullName()}). * * @param fullName The member's full name * @return A syntax element that can either be used as working rule, or to continue specifying a more complex rule @@ -77,7 +79,7 @@ public interface MembersShould> CONJUNCTION haveFullName(String fullName); /** - * Asserts that members do not have a certain full name. + * Asserts that members do not have a certain full name (compare {@link JavaField#getFullName()} and {@link JavaCodeUnit#getFullName()}). * * @param fullName The member's full name * @return A syntax element that can either be used as working rule, or to continue specifying a more complex rule @@ -86,7 +88,8 @@ public interface MembersShould> CONJUNCTION notHaveFullName(String fullName); /** - * Asserts that members have a full name matching a given regular expression. + * Asserts that members have a full name matching a given regular expression (compare {@link JavaField#getFullName()} + * and {@link JavaCodeUnit#getFullName()}). * * @param regex A regular expression * @return A syntax element that can either be used as working rule, or to continue specifying a more complex rule @@ -95,7 +98,8 @@ public interface MembersShould> CONJUNCTION haveFullNameMatching(String regex); /** - * Asserts that members have a full name not matching a given regular expression. + * Asserts that members have a full name not matching a given regular expression (compare {@link JavaField#getFullName()} + * and {@link JavaCodeUnit#getFullName()}). * * @param regex A regular expression * @return A syntax element that can either be used as working rule, or to continue specifying a more complex rule diff --git a/archunit/src/test/java/com/tngtech/archunit/core/domain/properties/HasNameTest.java b/archunit/src/test/java/com/tngtech/archunit/core/domain/properties/HasNameTest.java index 4d13d23671..ed9bc1d773 100644 --- a/archunit/src/test/java/com/tngtech/archunit/core/domain/properties/HasNameTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/core/domain/properties/HasNameTest.java @@ -3,8 +3,10 @@ import org.assertj.core.api.AbstractBooleanAssert; import org.junit.Test; +import static com.tngtech.archunit.core.domain.properties.HasName.AndFullName.Functions.GET_FULL_NAME; import static com.tngtech.archunit.core.domain.properties.HasName.AndFullName.Predicates.fullName; import static com.tngtech.archunit.core.domain.properties.HasName.AndFullName.Predicates.fullNameMatching; +import static com.tngtech.archunit.core.domain.properties.HasName.Functions.GET_NAME; import static com.tngtech.archunit.core.domain.properties.HasName.Predicates.name; import static com.tngtech.archunit.core.domain.properties.HasName.Predicates.nameMatching; import static com.tngtech.archunit.testutil.Assertions.assertThat; @@ -53,6 +55,13 @@ public void fullNameMatching_predicate() { .hasDescription("full name matching '.*method\\(.*\\)'"); } + @Test + public void functions() { + HasName.AndFullName test = newHasNameAndFullName("simple", "full"); + assertThat(GET_NAME.apply(test)).isEqualTo("simple"); + assertThat(GET_FULL_NAME.apply(test)).isEqualTo("full"); + } + private AbstractBooleanAssert assertNameMatches(String input, String regex) { return assertThat(nameMatching(regex).apply(newHasName(input))) .as(input + " =~ " + regex); diff --git a/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenMembersTest.java b/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenMembersTest.java index 42db09e141..e83e127d72 100644 --- a/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenMembersTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenMembersTest.java @@ -5,7 +5,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.regex.Pattern; import com.google.common.collect.FluentIterable; import com.google.common.collect.ImmutableList; @@ -195,7 +194,7 @@ public static Object[][] restricted_property_rule_starts() { $(described(constructors().that().haveFullName(classNameDot + CONSTRUCTOR_ONE_ARG)), ImmutableSet.of(CONSTRUCTOR_ONE_ARG)), $(described(fields().that().haveFullName(classNameDot + FIELD_A)), ImmutableSet.of(FIELD_A)), $(described(members().that().doNotHaveFullName(classNameDot + FIELD_A)), union(allFieldsExcept(FIELD_A), ALL_CODE_UNIT_DESCRIPTIONS)), - $(described(codeUnits().that().doNotHaveFullName(classNameDot + FIELD_A)), ALL_CODE_UNIT_DESCRIPTIONS), + $(described(codeUnits().that().doNotHaveFullName(classNameDot + METHOD_A)), allCodeUnitsExcept(METHOD_A)), $(described(methods().that().doNotHaveFullName(classNameDot + METHOD_A)), allMethodsExcept(METHOD_A)), $(described(constructors().that().doNotHaveFullName(classNameDot + CONSTRUCTOR_ONE_ARG)), allConstructorsExcept(CONSTRUCTOR_ONE_ARG)), $(described(fields().that().doNotHaveFullName(classNameDot + FIELD_A)), allFieldsExcept(FIELD_A)), @@ -203,14 +202,16 @@ public static Object[][] restricted_property_rule_starts() { $(described(members().that().haveFullNameMatching(quote(classNameDot) + ".*A.*")), ImmutableSet.of(FIELD_A, METHOD_A)), $(described(codeUnits().that().haveFullNameMatching(quote(classNameDot) + ".*A.*")), ImmutableSet.of(METHOD_A)), $(described(methods().that().haveFullNameMatching(quote(classNameDot) + ".*A.*")), ImmutableSet.of(METHOD_A)), - $(described(constructors().that().haveFullNameMatching(".*init.*")), ALL_CONSTRUCTOR_DESCRIPTIONS), + $(described(constructors().that().haveFullNameMatching(quote(classNameDot) + ".*init.*String\\)")), + ImmutableSet.of(CONSTRUCTOR_ONE_ARG)), $(described(fields().that().haveFullNameMatching(quote(classNameDot) + ".*A.*")), ImmutableSet.of(FIELD_A)), $(described(members().that().haveFullNameNotMatching(quote(classNameDot) + "f.*A.*")), union( allFieldsExcept(FIELD_A), ALL_CODE_UNIT_DESCRIPTIONS)), $(described(codeUnits().that().haveFullNameNotMatching(quote(classNameDot) + "f.*A.*")), ALL_CODE_UNIT_DESCRIPTIONS), $(described(methods().that().haveFullNameNotMatching(quote(classNameDot) + ".*A.*")), allMethodsExcept(METHOD_A)), - $(described(constructors().that().haveFullNameNotMatching(".*init.*")), emptySet()), + $(described(constructors().that().haveFullNameNotMatching(quote(classNameDot) + ".*init.*String\\)")), + allConstructorsExcept(CONSTRUCTOR_ONE_ARG)), $(described(fields().that().haveFullNameNotMatching(quote(classNameDot) + ".*A.*")), allFieldsExcept(FIELD_A)), $(described(members().that().arePublic()), ImmutableSet.of( diff --git a/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/MembersShouldTest.java b/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/MembersShouldTest.java index 235f782806..3725b15664 100644 --- a/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/MembersShouldTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/MembersShouldTest.java @@ -46,6 +46,7 @@ import static com.tngtech.archunit.lang.syntax.elements.GivenMembersTest.ALL_OTHER_MEMBER_DESCRIPTIONS; import static com.tngtech.archunit.lang.syntax.elements.GivenMembersTest.ALL_OTHER_METHOD_DESCRIPTIONS; import static com.tngtech.archunit.lang.syntax.elements.GivenMembersTest.CONSTRUCTOR_ANNOTATED_WITH_A; +import static com.tngtech.archunit.lang.syntax.elements.GivenMembersTest.CONSTRUCTOR_ONE_ARG; import static com.tngtech.archunit.lang.syntax.elements.GivenMembersTest.CONSTRUCTOR_PACKAGE_PRIVATE; import static com.tngtech.archunit.lang.syntax.elements.GivenMembersTest.CONSTRUCTOR_PRIVATE; import static com.tngtech.archunit.lang.syntax.elements.GivenMembersTest.CONSTRUCTOR_PROTECTED; @@ -149,14 +150,15 @@ public static Object[][] restricted_property_rule_ends() { $(codeUnits().should().haveFullNameMatching(quote(classNameDot) + ".*A" + quote("()")), allCodeUnitsExcept(METHOD_A)), $(methods().should().haveFullNameMatching(quote(classNameDot) + ".*A" + quote("()")), allMethodsExcept(METHOD_A)), $(codeUnits().should().haveFullNameMatching(quote(classNameDot) + "..*init.*"), ALL_METHOD_DESCRIPTIONS), - $(constructors().should().haveFullNameMatching(quote(classNameDot) + ".*init.*"), emptySet()), + $(constructors().should().haveFullNameMatching(quote(classNameDot) + ".*init.*String\\)"), + allConstructorsExcept(CONSTRUCTOR_ONE_ARG)), $(members().should().haveFullNameNotMatching(quote(classNameDot) + ".*A\\(?\\)?"), ImmutableSet.of(FIELD_A, METHOD_A)), $(codeUnits().should().haveFullNameNotMatching(quote(classNameDot) + ".*A"), emptySet()), $(fields().should().haveFullNameNotMatching(quote(classNameDot) + ".*A"), ImmutableSet.of(FIELD_A)), $(codeUnits().should().haveFullNameNotMatching(quote(classNameDot) + ".*A" + quote("()")), ImmutableSet.of(METHOD_A)), $(methods().should().haveFullNameNotMatching(quote(classNameDot) + ".*A" + quote("()")), ImmutableSet.of(METHOD_A)), $(codeUnits().should().haveFullNameNotMatching(quote(classNameDot) + ".*init.*"), ALL_CONSTRUCTOR_DESCRIPTIONS), - $(constructors().should().haveFullNameNotMatching(quote(classNameDot) + ".*init.*"), ALL_CONSTRUCTOR_DESCRIPTIONS), + $(constructors().should().haveFullNameNotMatching(quote(classNameDot) + ".*init.*String\\)"), ImmutableSet.of(CONSTRUCTOR_ONE_ARG)), $(members().should().bePublic(), allMembersExcept( FIELD_PUBLIC, METHOD_PUBLIC, CONSTRUCTOR_PUBLIC)),