Skip to content

Commit

Permalink
support method and constructor parameter annotations #701
Browse files Browse the repository at this point in the history
This will extend the domain model of ArchUnit by providing 

* `List<Parameter> JavaCodeUnit.getParameters()`
* `List<List<JavaAnnotation<Parameter>> JavaCodeUnit.getParameterAnnotations()`

where `Parameter` also offers `getParameterType()`, `getRawParameterType()` and `getAnnotations()`.

Furthermore parameter annotations are now part of `JavaClass.directDependencies{From/To}Self`.

On the contrary to the Java Reflection API the `JavaCodeUnit.getParameters()` will exactly mirror the (possibly generic) signature and not the raw types. This means that for generic signatures synthetic raw type parameter types (like `name` and `ordinal` for an enum constructor) will not be visible in the `List<Parameter>`. I think for ArchUnit this makes sense, as there is limited interest in synthetic parts of the code and users are only interested in the parameters that have really been introduced by source code. Unfortunately there are many cases where the bytecode does not contain the signature attribute (whenever the signature is non-generic, i.e. does not contain any parameterized types, type variables, etc.). In these cases the parameters will still mirror the raw types (including possible synthetic parameters), because there is no other source of information where the parameters could be derived from. It is also not i.g. possible to derive which parameters are synthetic and which are not, so ArchUnit does not attempt this at all at the moment.

Resolves: #404
Resolves: #373
Resolves: #113
  • Loading branch information
codecholeric authored Oct 27, 2021
2 parents ac1441e + 98787a0 commit ec51005
Show file tree
Hide file tree
Showing 34 changed files with 1,309 additions and 220 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package com.tngtech.archunit.example.layers.controller;

import com.tngtech.archunit.example.layers.controller.marshaller.StringUnmarshaller;

@SuppressWarnings("unused")
public class OtherController {
void receive(@UnmarshalTransport(StringUnmarshaller.class) Object param) {
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package com.tngtech.archunit.example.layers.controller;

import java.lang.annotation.Retention;

import com.tngtech.archunit.example.layers.controller.marshaller.Unmarshaller;

import static java.lang.annotation.RetentionPolicy.RUNTIME;

@Retention(RUNTIME)
public @interface UnmarshalTransport {
Class<? extends Unmarshaller<?>>[] value();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package com.tngtech.archunit.example.layers.controller.marshaller;

public class ByteUnmarshaller implements Unmarshaller<Byte> {
@Override
public <T> T unmarschal(Byte from) {
return null;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package com.tngtech.archunit.example.layers.controller.marshaller;

public class StringUnmarshaller implements Unmarshaller<String> {
@Override
public <T> T unmarschal(String from) {
return null;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package com.tngtech.archunit.example.layers.controller.marshaller;

public interface Unmarshaller<F> {
@SuppressWarnings("unused")
<T> T unmarschal(F from);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.tngtech.archunit.example.layers.service;

import com.tngtech.archunit.example.layers.controller.UnmarshalTransport;
import com.tngtech.archunit.example.layers.controller.marshaller.ByteUnmarshaller;
import com.tngtech.archunit.example.layers.controller.marshaller.StringUnmarshaller;

public class OtherServiceViolatingLayerRules {
@SuppressWarnings("unused")
public void dependentOnParameterAnnotation(@UnmarshalTransport({StringUnmarshaller.class, ByteUnmarshaller.class}) Object param) {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public class PublicAPIRules {
.and(doNot(inheritPublicAPI()))
.and(are(relevantArchUnitMembers()))

.should(notBePublic())
.should().notBePublic()

.because("users of ArchUnit should only access intended members, to preserve maintainability");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,11 @@
import com.tngtech.archunit.example.layers.controller.SomeController;
import com.tngtech.archunit.example.layers.controller.SomeGuiController;
import com.tngtech.archunit.example.layers.controller.SomeUtility;
import com.tngtech.archunit.example.layers.controller.UnmarshalTransport;
import com.tngtech.archunit.example.layers.controller.WronglyAnnotated;
import com.tngtech.archunit.example.layers.controller.marshaller.ByteUnmarshaller;
import com.tngtech.archunit.example.layers.controller.marshaller.StringUnmarshaller;
import com.tngtech.archunit.example.layers.controller.marshaller.Unmarshaller;
import com.tngtech.archunit.example.layers.controller.one.SomeEnum;
import com.tngtech.archunit.example.layers.controller.one.UseCaseOneThreeController;
import com.tngtech.archunit.example.layers.controller.one.UseCaseOneTwoController;
Expand All @@ -100,6 +104,7 @@
import com.tngtech.archunit.example.layers.security.Secured;
import com.tngtech.archunit.example.layers.service.Async;
import com.tngtech.archunit.example.layers.service.ComplexServiceAnnotation;
import com.tngtech.archunit.example.layers.service.OtherServiceViolatingLayerRules;
import com.tngtech.archunit.example.layers.service.ProxiedConnection;
import com.tngtech.archunit.example.layers.service.ServiceHelper;
import com.tngtech.archunit.example.layers.service.ServiceInterface;
Expand Down Expand Up @@ -174,6 +179,7 @@
import static com.tngtech.archunit.testutils.ExpectedAccess.callFromMethod;
import static com.tngtech.archunit.testutils.ExpectedAccess.callFromStaticInitializer;
import static com.tngtech.archunit.testutils.ExpectedDependency.annotatedClass;
import static com.tngtech.archunit.testutils.ExpectedDependency.annotatedParameter;
import static com.tngtech.archunit.testutils.ExpectedDependency.constructor;
import static com.tngtech.archunit.testutils.ExpectedDependency.field;
import static com.tngtech.archunit.testutils.ExpectedDependency.genericFieldType;
Expand Down Expand Up @@ -233,7 +239,7 @@ Stream<DynamicTest> CodingRulesTest() {
.inLine(9));

expectFailures.ofRule("fields that have raw type java.util.logging.Logger should be private " +
"and should be static and should be final, because we agreed on this convention")
"and should be static and should be final, because we agreed on this convention")
.by(ExpectedField.of(ClassViolatingCodingRules.class, "log").doesNotHaveModifier(JavaModifier.PRIVATE))
.by(ExpectedField.of(ClassViolatingCodingRules.class, "log").doesNotHaveModifier(JavaModifier.FINAL));

Expand All @@ -245,8 +251,8 @@ Stream<DynamicTest> CodingRulesTest() {
.by(method(ClassViolatingCodingRules.class, "jodaTimeIsBad")
.withReturnType(org.joda.time.DateTime.class));

expectFailures.ofRule("no classes should use field injection, because field injection is considered harmful; "
+ "use constructor injection or setter injection instead; see https://stackoverflow.com/q/39890849 for detailed explanations")
expectFailures.ofRule("no classes should use field injection, because field injection is considered harmful; " +
"use constructor injection or setter injection instead; see https://stackoverflow.com/q/39890849 for detailed explanations")
.by(ExpectedField.of(ClassViolatingInjectionRules.class, "badBecauseAutowiredField").beingAnnotatedWith(Autowired.class))
.by(ExpectedField.of(ClassViolatingInjectionRules.class, "badBecauseValueField").beingAnnotatedWith(Value.class))
.by(ExpectedField.of(ClassViolatingInjectionRules.class, "badBecauseJavaxInjectField").beingAnnotatedWith(javax.inject.Inject.class))
Expand Down Expand Up @@ -815,6 +821,15 @@ Stream<DynamicTest> LayerDependencyRulesTest() {
.by(annotatedClass(ServiceViolatingLayerRules.class).withAnnotationParameterType(ComplexControllerAnnotation.class))
.by(annotatedClass(ServiceViolatingLayerRules.class).withAnnotationParameterType(SimpleControllerAnnotation.class))
.by(annotatedClass(ServiceViolatingLayerRules.class).withAnnotationParameterType(SomeEnum.class))
.by(annotatedParameter(Object.class)
.ofMethod(OtherServiceViolatingLayerRules.class, "dependentOnParameterAnnotation", Object.class)
.annotatedWith(UnmarshalTransport.class))
.by(annotatedParameter(Object.class)
.ofMethod(OtherServiceViolatingLayerRules.class, "dependentOnParameterAnnotation", Object.class)
.withAnnotationParameterType(StringUnmarshaller.class))
.by(annotatedParameter(Object.class)
.ofMethod(OtherServiceViolatingLayerRules.class, "dependentOnParameterAnnotation", Object.class)
.withAnnotationParameterType(ByteUnmarshaller.class))
.by(method(ComplexServiceAnnotation.class, "controllerAnnotation").withReturnType(ComplexControllerAnnotation.class))
.by(method(ComplexServiceAnnotation.class, "controllerEnum").withReturnType(SomeEnum.class))

Expand Down Expand Up @@ -898,6 +913,15 @@ Stream<DynamicTest> LayerDependencyRulesTest() {
.by(annotatedClass(ServiceViolatingLayerRules.class).withAnnotationParameterType(ComplexControllerAnnotation.class))
.by(annotatedClass(ServiceViolatingLayerRules.class).withAnnotationParameterType(SimpleControllerAnnotation.class))
.by(annotatedClass(ServiceViolatingLayerRules.class).withAnnotationParameterType(SomeEnum.class))
.by(annotatedParameter(Object.class)
.ofMethod(OtherServiceViolatingLayerRules.class, "dependentOnParameterAnnotation", Object.class)
.annotatedWith(UnmarshalTransport.class))
.by(annotatedParameter(Object.class)
.ofMethod(OtherServiceViolatingLayerRules.class, "dependentOnParameterAnnotation", Object.class)
.withAnnotationParameterType(StringUnmarshaller.class))
.by(annotatedParameter(Object.class)
.ofMethod(OtherServiceViolatingLayerRules.class, "dependentOnParameterAnnotation", Object.class)
.withAnnotationParameterType(ByteUnmarshaller.class))
.by(method(ComplexServiceAnnotation.class, "controllerAnnotation").withReturnType(ComplexControllerAnnotation.class))
.by(method(ComplexServiceAnnotation.class, "controllerEnum").withReturnType(SomeEnum.class))

Expand Down Expand Up @@ -977,6 +1001,15 @@ Stream<DynamicTest> LayeredArchitectureTest() {
.by(annotatedClass(ServiceViolatingLayerRules.class).withAnnotationParameterType(ComplexControllerAnnotation.class))
.by(annotatedClass(ServiceViolatingLayerRules.class).withAnnotationParameterType(SimpleControllerAnnotation.class))
.by(annotatedClass(ServiceViolatingLayerRules.class).withAnnotationParameterType(SomeEnum.class))
.by(annotatedParameter(Object.class)
.ofMethod(OtherServiceViolatingLayerRules.class, "dependentOnParameterAnnotation", Object.class)
.annotatedWith(UnmarshalTransport.class))
.by(annotatedParameter(Object.class)
.ofMethod(OtherServiceViolatingLayerRules.class, "dependentOnParameterAnnotation", Object.class)
.withAnnotationParameterType(StringUnmarshaller.class))
.by(annotatedParameter(Object.class)
.ofMethod(OtherServiceViolatingLayerRules.class, "dependentOnParameterAnnotation", Object.class)
.withAnnotationParameterType(ByteUnmarshaller.class))
.by(method(ComplexServiceAnnotation.class, "controllerAnnotation").withReturnType(ComplexControllerAnnotation.class))
.by(method(ComplexServiceAnnotation.class, "controllerEnum").withReturnType(SomeEnum.class))
.by(method(OtherJpa.class, "testConnection")
Expand Down Expand Up @@ -1117,6 +1150,10 @@ Stream<DynamicTest> NamingConventionTest() {
.by(simpleNameOf(InheritedControllerImpl.class).notEndingWith("Controller"))
.by(simpleNameOf(ComplexControllerAnnotation.class).notEndingWith("Controller"))
.by(simpleNameOf(SimpleControllerAnnotation.class).notEndingWith("Controller"))
.by(simpleNameOf(UnmarshalTransport.class).notEndingWith("Controller"))
.by(simpleNameOf(Unmarshaller.class).notEndingWith("Controller"))
.by(simpleNameOf(StringUnmarshaller.class).notEndingWith("Controller"))
.by(simpleNameOf(ByteUnmarshaller.class).notEndingWith("Controller"))
.by(simpleNameOf(SomeUtility.class).notEndingWith("Controller"))
.by(simpleNameOf(WronglyAnnotated.class).notEndingWith("Controller"))
.by(simpleNameOf(SomeEnum.class).notEndingWith("Controller"))
Expand Down Expand Up @@ -1398,14 +1435,15 @@ Stream<DynamicTest> SlicesIsolationTest() {
// controllers_should_only_use_their_own_slice
addExpectedCommonFailureFor_controllers_should_only_use_their_own_slice
.accept("controllers_should_only_use_their_own_slice", expectedTestFailures);
expectedTestFailures.by(sliceDependency()
.described("Controller one depends on Controller two")
.by(callFromMethod(UseCaseOneTwoController.class, doSomethingOne)
.toConstructor(UseCaseTwoController.class)
.inLine(10))
.by(callFromMethod(UseCaseOneTwoController.class, doSomethingOne)
.toMethod(UseCaseTwoController.class, doSomethingTwo)
.inLine(10)))
expectedTestFailures
.by(sliceDependency()
.described("Controller one depends on Controller two")
.by(callFromMethod(UseCaseOneTwoController.class, doSomethingOne)
.toConstructor(UseCaseTwoController.class)
.inLine(10))
.by(callFromMethod(UseCaseOneTwoController.class, doSomethingOne)
.toMethod(UseCaseTwoController.class, doSomethingTwo)
.inLine(10)))
.by(sliceDependency()
.described("Controller three depends on Controller one")
.by(callFromMethod(UseCaseThreeController.class, doSomethingThree)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.collect.Iterables.getOnlyElement;
import static com.tngtech.archunit.core.domain.Formatters.formatMethod;
import static com.tngtech.archunit.core.domain.Formatters.formatNamesOf;
import static com.tngtech.archunit.core.domain.JavaConstructor.CONSTRUCTOR_NAME;
import static java.util.regex.Pattern.quote;

Expand Down Expand Up @@ -82,6 +84,10 @@ public static AnnotationDependencyCreator annotatedClass(Class<?> clazz) {
return new AnnotationDependencyCreator(clazz);
}

public static AnnotationParameterDependencyCreator annotatedParameter(Class<?> parameterType) {
return new AnnotationParameterDependencyCreator(parameterType);
}

public static AccessCreator accessFrom(Class<?> clazz) {
return new AccessCreator(clazz);
}
Expand Down Expand Up @@ -317,21 +323,40 @@ public ExpectedDependency inLine(int lineNumber) {
}
}

public static class AnnotationParameterDependencyCreator {
private final Class<?> parameterType;

private AnnotationParameterDependencyCreator(Class<?> parameterType) {
this.parameterType = parameterType;
}

public AnnotationDependencyCreator ofMethod(Class<?> originClass, String methodName, Class<?>... paramTypes) {
return new AnnotationDependencyCreator(originClass, "Parameter <" + parameterType.getName() + "> of method <" +
formatMethod(originClass.getName(), methodName, formatNamesOf(paramTypes)) + ">");
}
}

public static class AnnotationDependencyCreator {
private final Class<?> owner;
private final Class<?> originClass;
private final String originDescription;

AnnotationDependencyCreator(Class<?> owner) {
this.owner = owner;
AnnotationDependencyCreator(Class<?> originClass) {
this(originClass, originClass.getName());
}

AnnotationDependencyCreator(Class<?> originClass, String originDescription) {
this.originClass = originClass;
this.originDescription = originDescription;
}

public ExpectedDependency annotatedWith(Class<?> annotationType) {
String dependencyPattern = getDependencyPattern(owner.getName(), "is annotated with", annotationType.getName(), 0);
return new ExpectedDependency(owner, annotationType, dependencyPattern);
String dependencyPattern = getDependencyPattern(originDescription, "is annotated with", annotationType.getName(), 0);
return new ExpectedDependency(originClass, annotationType, dependencyPattern);
}

public ExpectedDependency withAnnotationParameterType(Class<?> type) {
String dependencyPattern = getDependencyPattern(owner.getName(), "has annotation member of type", type.getName(), 0);
return new ExpectedDependency(owner, type, dependencyPattern);
String dependencyPattern = getDependencyPattern(originDescription, "has annotation member of type", type.getName(), 0);
return new ExpectedDependency(originClass, type, dependencyPattern);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ public static DescribedPredicate<Iterable<?>> empty() {
return new EmptyPredicate();
}

public static <T> DescribedPredicate<Iterable<T>> anyElementThat(final DescribedPredicate<? super T> predicate) {
public static <T> DescribedPredicate<Iterable<? extends T>> anyElementThat(final DescribedPredicate<? super T> predicate) {
return new AnyElementPredicate<>(predicate);
}

Expand Down Expand Up @@ -320,7 +320,7 @@ public boolean apply(Iterable<?> input) {
}
}

private static class AnyElementPredicate<T> extends DescribedPredicate<Iterable<T>> {
private static class AnyElementPredicate<T> extends DescribedPredicate<Iterable<? extends T>> {
private final DescribedPredicate<T> predicate;

AnyElementPredicate(DescribedPredicate<? super T> predicate) {
Expand All @@ -329,7 +329,7 @@ private static class AnyElementPredicate<T> extends DescribedPredicate<Iterable<
}

@Override
public boolean apply(Iterable<T> iterable) {
public boolean apply(Iterable<? extends T> iterable) {
for (T javaClass : iterable) {
if (predicate.apply(javaClass)) {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,10 @@ private static Origin findSuitableOrigin(Object dependencyCause, Object originCa
JavaClass clazz = (JavaClass) originCandidate;
return new Origin(clazz, clazz.getDescription());
}
if (originCandidate instanceof JavaCodeUnit.Parameter) {
JavaCodeUnit.Parameter parameter = (JavaCodeUnit.Parameter) originCandidate;
return new Origin(parameter.getOwner().getOwner(), parameter.getDescription());
}
throw new IllegalStateException("Could not find suitable dependency origin for " + dependencyCause);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,4 @@ public interface ImportContext {
Set<JavaConstructorCall> createConstructorCallsFor(JavaCodeUnit codeUnit);

JavaClass resolveClass(String fullyQualifiedClassName);

Optional<JavaClass> getMethodReturnType(String declaringClassName, String methodName);
}
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ private JavaAnnotation(JavaClass type, OWNER owner, CanBeAnnotated annotatedElem

private static CanBeAnnotated getAnnotatedElement(Object owner) {
Object candiate = owner;
while (!(candiate instanceof JavaClass) && !(candiate instanceof JavaMember) && (candiate instanceof HasOwner<?>)) {
while (!(candiate instanceof CanBeAnnotated) && (candiate instanceof HasOwner<?>)) {
candiate = ((HasOwner<?>) candiate).getOwner();
}
if (!(candiate instanceof CanBeAnnotated)) {
Expand Down
Loading

0 comments on commit ec51005

Please sign in to comment.