From c8563d918ddd4bf03bd1ff52605be015ee7716fb Mon Sep 17 00:00:00 2001 From: Roland Weisleder Date: Mon, 20 Nov 2023 16:31:26 +0100 Subject: [PATCH] add possibility to check for overriding methods to domain and lang Resolves: #1198 Signed-off-by: Roland Weisleder --- .../archunit/core/domain/JavaMethod.java | 64 +++++++++++++++++++ .../lang/syntax/MethodsShouldInternal.java | 14 ++++ .../lang/syntax/MethodsThatInternal.java | 19 ++++++ .../lang/syntax/elements/MethodsShould.java | 19 ++++++ .../lang/syntax/elements/MethodsThat.java | 19 ++++++ .../archunit/core/domain/JavaMethodTest.java | 59 +++++++++++++++++ .../syntax/elements/GivenMethodsTest.java | 10 ++- .../syntax/elements/MethodsShouldTest.java | 10 ++- 8 files changed, 212 insertions(+), 2 deletions(-) create mode 100644 archunit/src/test/java/com/tngtech/archunit/core/domain/JavaMethodTest.java diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaMethod.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaMethod.java index 926f39e7a7..0ac51e6166 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaMethod.java +++ b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaMethod.java @@ -21,18 +21,29 @@ import java.util.Set; import java.util.function.Function; import java.util.function.Supplier; +import java.util.stream.Stream; import com.tngtech.archunit.PublicAPI; import com.tngtech.archunit.base.ArchUnitException.InconsistentClassPathException; +import com.tngtech.archunit.base.DescribedPredicate; import com.tngtech.archunit.base.MayResolveTypesViaReflection; import com.tngtech.archunit.base.ResolvesTypesViaReflection; import com.tngtech.archunit.base.Suppliers; +import com.tngtech.archunit.core.domain.properties.CanBeAnnotated; +import com.tngtech.archunit.core.domain.properties.HasModifiers; +import com.tngtech.archunit.core.domain.properties.HasName; +import com.tngtech.archunit.core.domain.properties.HasOwner; +import com.tngtech.archunit.core.domain.properties.HasParameterTypes; +import com.tngtech.archunit.core.domain.properties.HasReturnType; +import com.tngtech.archunit.core.domain.properties.HasThrowsClause; import com.tngtech.archunit.core.importer.DomainBuilders; import static com.google.common.collect.Sets.union; import static com.tngtech.archunit.PublicAPI.Usage.ACCESS; +import static com.tngtech.archunit.base.DescribedPredicate.describe; import static com.tngtech.archunit.core.domain.Formatters.formatMethod; import static com.tngtech.archunit.core.domain.properties.HasName.Utils.namesOf; +import static java.util.stream.Collectors.toList; @PublicAPI(usage = ACCESS) public final class JavaMethod extends JavaCodeUnit { @@ -125,6 +136,30 @@ public String getDescription() { return "Method <" + getFullName() + ">"; } + /** + * Returns true, if this method overrides or implements a method from a supertype. + * The annotation {@link Override} may or may not be present on this method. + * + * @see Override + */ + @PublicAPI(usage = ACCESS) + public boolean isOverriding() { + List supertypes = Stream.concat( + getOwner().getAllRawSuperclasses().stream(), + getOwner().getAllRawInterfaces().stream() + ).collect(toList()); + + String name = getName(); + String[] parameterTypes = getRawParameterTypes().stream().map(JavaClass::getFullName).toArray(String[]::new); + for (JavaClass supertype : supertypes) { + if (supertype.tryGetMethod(name, parameterTypes).isPresent()) { + return true; + } + } + + return false; + } + @ResolvesTypesViaReflection @MayResolveTypesViaReflection(reason = "Just part of a bigger resolution process") private class ReflectMethodSupplier implements Supplier { @@ -140,4 +175,33 @@ public Method get() { } } + /** + * Predefined {@link DescribedPredicate predicates} targeting {@link JavaMethod}. + * Note that due to inheritance further predicates for {@link JavaMethod} can be found in the following locations: + *
    + *
  • {@link JavaCodeUnit.Predicates}
    • + *
  • {@link JavaMember.Predicates}
    • + *
  • {@link HasName.Predicates}
    • + *
  • {@link HasName.AndFullName.Predicates}
    • + *
  • {@link HasModifiers.Predicates}
    • + *
  • {@link CanBeAnnotated.Predicates}
    • + *
  • {@link HasOwner.Predicates}
    • + *
  • {@link HasParameterTypes.Predicates}
    • + *
  • {@link HasReturnType.Predicates}
    • + *
  • {@link HasThrowsClause.Predicates}
    • + *
+ */ + @PublicAPI(usage = ACCESS) + public static final class Predicates { + private Predicates() { + } + + /** + * @see JavaMethod#isOverriding() + */ + @PublicAPI(usage = ACCESS) + public static DescribedPredicate overriding() { + return describe("overriding", JavaMethod::isOverriding); + } + } } diff --git a/archunit/src/main/java/com/tngtech/archunit/lang/syntax/MethodsShouldInternal.java b/archunit/src/main/java/com/tngtech/archunit/lang/syntax/MethodsShouldInternal.java index d17a3e6aac..9ca1f83682 100644 --- a/archunit/src/main/java/com/tngtech/archunit/lang/syntax/MethodsShouldInternal.java +++ b/archunit/src/main/java/com/tngtech/archunit/lang/syntax/MethodsShouldInternal.java @@ -24,6 +24,10 @@ import com.tngtech.archunit.lang.syntax.elements.MethodsShould; import com.tngtech.archunit.lang.syntax.elements.MethodsShouldConjunction; +import static com.tngtech.archunit.core.domain.JavaMethod.Predicates.overriding; +import static com.tngtech.archunit.lang.conditions.ArchConditions.be; +import static com.tngtech.archunit.lang.conditions.ArchConditions.not; + class MethodsShouldInternal extends AbstractCodeUnitsShouldInternal implements MethodsShould, MethodsShouldConjunction { @@ -58,4 +62,14 @@ private MethodsShouldInternal( MethodsShouldInternal copyWithNewCondition(ConditionAggregator newCondition) { return new MethodsShouldInternal(classesTransformer, priority, newCondition, prepareCondition); } + + @Override + public MethodsShouldInternal beOverriding() { + return addCondition(be(overriding())); + } + + @Override + public MethodsShouldInternal notBeOverriding() { + return addCondition(not(be(overriding()))); + } } diff --git a/archunit/src/main/java/com/tngtech/archunit/lang/syntax/MethodsThatInternal.java b/archunit/src/main/java/com/tngtech/archunit/lang/syntax/MethodsThatInternal.java index 761c86c87c..b447aa5295 100644 --- a/archunit/src/main/java/com/tngtech/archunit/lang/syntax/MethodsThatInternal.java +++ b/archunit/src/main/java/com/tngtech/archunit/lang/syntax/MethodsThatInternal.java @@ -15,9 +15,14 @@ */ package com.tngtech.archunit.lang.syntax; +import com.tngtech.archunit.base.DescribedPredicate; import com.tngtech.archunit.core.domain.JavaMethod; import com.tngtech.archunit.lang.syntax.elements.MethodsThat; +import static com.tngtech.archunit.base.DescribedPredicate.not; +import static com.tngtech.archunit.core.domain.JavaMethod.Predicates.overriding; +import static com.tngtech.archunit.lang.conditions.ArchPredicates.are; + class MethodsThatInternal extends CodeUnitsThatInternal implements MethodsThat { @@ -25,4 +30,18 @@ class MethodsThatInternal MethodsThatInternal(GivenMethodsInternal givenMethods, PredicateAggregator currentPredicate) { super(givenMethods, currentPredicate); } + + @Override + public GivenMethodsInternal areOverriding() { + return withPredicate(are(overriding())); + } + + @Override + public GivenMethodsInternal areNotOverriding() { + return withPredicate(are(not(overriding()))); + } + + private GivenMethodsInternal withPredicate(DescribedPredicate predicate) { + return givenMembers.with(currentPredicate.add(predicate)); + } } diff --git a/archunit/src/main/java/com/tngtech/archunit/lang/syntax/elements/MethodsShould.java b/archunit/src/main/java/com/tngtech/archunit/lang/syntax/elements/MethodsShould.java index ceae776f37..5cede2a0c1 100644 --- a/archunit/src/main/java/com/tngtech/archunit/lang/syntax/elements/MethodsShould.java +++ b/archunit/src/main/java/com/tngtech/archunit/lang/syntax/elements/MethodsShould.java @@ -16,6 +16,7 @@ package com.tngtech.archunit.lang.syntax.elements; import com.tngtech.archunit.PublicAPI; +import com.tngtech.archunit.core.domain.JavaMethod; import static com.tngtech.archunit.PublicAPI.Usage.ACCESS; @@ -53,4 +54,22 @@ public interface MethodsShould ext */ @PublicAPI(usage = ACCESS) CONJUNCTION notBeFinal(); + + /** + * Asserts that methods override other methods. + * + * @return A syntax element that can either be used as working rule, or to continue specifying a more complex rule + * @see JavaMethod#isOverriding() + */ + @PublicAPI(usage = ACCESS) + CONJUNCTION beOverriding(); + + /** + * Asserts that methods do not override other methods. + * + * @return A syntax element that can either be used as working rule, or to continue specifying a more complex rule + * @see JavaMethod#isOverriding() + */ + @PublicAPI(usage = ACCESS) + CONJUNCTION notBeOverriding(); } diff --git a/archunit/src/main/java/com/tngtech/archunit/lang/syntax/elements/MethodsThat.java b/archunit/src/main/java/com/tngtech/archunit/lang/syntax/elements/MethodsThat.java index 2f28181f61..27d085f2df 100644 --- a/archunit/src/main/java/com/tngtech/archunit/lang/syntax/elements/MethodsThat.java +++ b/archunit/src/main/java/com/tngtech/archunit/lang/syntax/elements/MethodsThat.java @@ -16,6 +16,7 @@ package com.tngtech.archunit.lang.syntax.elements; import com.tngtech.archunit.PublicAPI; +import com.tngtech.archunit.core.domain.JavaMethod; import static com.tngtech.archunit.PublicAPI.Usage.ACCESS; @@ -53,4 +54,22 @@ public interface MethodsThat extends CodeUnitsThat { */ @PublicAPI(usage = ACCESS) CONJUNCTION areNotFinal(); + + /** + * Matches methods that override other methods. + * + * @return A syntax conjunction element, which can be completed to form a full rule + * @see JavaMethod#isOverriding() + */ + @PublicAPI(usage = ACCESS) + CONJUNCTION areOverriding(); + + /** + * Matches methods that do not override other methods. + * + * @return A syntax conjunction element, which can be completed to form a full rule + * @see JavaMethod#isOverriding() + */ + @PublicAPI(usage = ACCESS) + CONJUNCTION areNotOverriding(); } diff --git a/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaMethodTest.java b/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaMethodTest.java new file mode 100644 index 0000000000..8eea409cae --- /dev/null +++ b/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaMethodTest.java @@ -0,0 +1,59 @@ +package com.tngtech.archunit.core.domain; + +import org.junit.Test; + +import static com.tngtech.archunit.core.domain.TestUtils.importMethod; +import static org.assertj.core.api.Assertions.assertThat; + +public class JavaMethodTest { + + @Test + public void isOverriding_returns_true_for_implemented_interface_method() { + assertThat(importMethod(SomeClass.class, "doSomething").isOverriding()).isTrue(); + } + + @Test + public void isOverriding_returns_false_for_interface_method() { + assertThat(importMethod(SomeInterface.class, "doSomething").isOverriding()).isFalse(); + } + + @Test + public void isOverriding_returns_true_for_overriding_method_of_java_lang_Object() { + assertThat(importMethod(SomeClass.class, "toString").isOverriding()).isTrue(); + } + + @Test + public void isOverriding_returns_false_for_non_overriding_method() { + assertThat(importMethod(SomeClass.class, "doSomethingElse").isOverriding()).isFalse(); + } + + @Test + public void isOverriding_returns_false_for_non_overriding_overloaded_method() { + assertThat(importMethod(SomeClass.class, "doSomething", Object.class).isOverriding()).isFalse(); + } + + @SuppressWarnings("unused") + private interface SomeInterface { + + void doSomething(); + } + + @SuppressWarnings("unused") + private static class SomeClass implements SomeInterface { + + @Override + public void doSomething() { + } + + public void doSomething(Object o) { + } + + public void doSomethingElse() { + } + + @Override + public String toString() { + return super.toString(); + } + } +} diff --git a/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenMethodsTest.java b/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenMethodsTest.java index b14b43206a..9d8a9cf9bb 100644 --- a/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenMethodsTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenMethodsTest.java @@ -27,6 +27,8 @@ public static Object[][] restricted_property_rule_starts() { $(described(methods().that().areNotFinal()), ImmutableSet.of(METHOD_C, METHOD_D)), $(described(methods().that().areStatic()), ImmutableSet.of(METHOD_B, METHOD_D)), $(described(methods().that().areNotStatic()), ImmutableSet.of(METHOD_A, METHOD_C)), + $(described(methods().that().areOverriding()), ImmutableSet.of(METHOD_A)), + $(described(methods().that().areNotOverriding()), ImmutableSet.of(METHOD_B, METHOD_C, METHOD_D)), $(described(methods().that().areFinal().and().areStatic()), ImmutableSet.of(METHOD_B)), $(described(methods().that().areFinal().or().areStatic()), ImmutableSet.of(METHOD_A, METHOD_B, METHOD_D)) ); @@ -47,7 +49,7 @@ public void property_predicates(DescribedRuleStart ruleStart, Collection private static final String METHOD_D = "methodD()"; @SuppressWarnings({"unused"}) - private static class ClassWithVariousMembers { + private static class ClassWithVariousMembers extends SuperclassWithVariousMembers { public final void methodA(int[] array) { } protected static final void methodB(boolean flag) { @@ -58,4 +60,10 @@ static int methodD() { return 0; } } + + @SuppressWarnings("unused") + private static class SuperclassWithVariousMembers { + public void methodA(int[] array) { + } + } } diff --git a/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/MethodsShouldTest.java b/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/MethodsShouldTest.java index 8193627128..411d3ebb68 100644 --- a/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/MethodsShouldTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/MethodsShouldTest.java @@ -29,6 +29,8 @@ public static Object[][] restricted_property_rule_ends() { $(methods().should().notBeFinal(), ImmutableSet.of(METHOD_A, METHOD_B)), $(methods().should().beStatic(), ImmutableSet.of(METHOD_A, METHOD_C)), $(methods().should().notBeStatic(), ImmutableSet.of(METHOD_B, METHOD_D)), + $(methods().should().beOverriding(), ImmutableSet.of(METHOD_B, METHOD_C, METHOD_D)), + $(methods().should().notBeOverriding(), ImmutableSet.of(METHOD_A)), $(methods().should().notBeFinal().andShould().notBeStatic(), ImmutableSet.of(METHOD_A, METHOD_B, METHOD_D)), $(methods().should().notBeFinal().orShould().notBeStatic(), ImmutableSet.of(METHOD_B)) ); @@ -49,7 +51,7 @@ public void property_predicates(ArchRule ruleStart, Collection expectedM private static final String METHOD_D = "methodD()"; @SuppressWarnings({"unused"}) - private static class ClassWithVariousMembers { + private static class ClassWithVariousMembers extends SuperclassWithVariousMembers { public final void methodA(int[] array) { } protected static final void methodB(boolean flag) { @@ -60,4 +62,10 @@ static int methodD() { return 0; } } + + @SuppressWarnings("unused") + private static class SuperclassWithVariousMembers { + public void methodA(int[] array) { + } + } }