Skip to content

Commit

Permalink
Add GeneralCodingRule to discourage use of field injection
Browse files Browse the repository at this point in the history
Resolves TNG#288
  • Loading branch information
rweisleder committed Dec 31, 2019
1 parent 2e6f178 commit 8bfe5a8
Show file tree
Hide file tree
Showing 4 changed files with 110 additions and 1 deletion.
3 changes: 3 additions & 0 deletions archunit/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ dependencies {
exclude module: 'assertj-core'
exclude module: 'guava'
}
testCompile dependency.springBeans
testCompile dependency.jakartaInject
testCompile dependency.jakartaAnnotations
}

ext.repackagesAsm = true // Will cause the ASM License to be packaged -> license.gradle
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.tngtech.archunit.core.domain.AccessTarget.FieldAccessTarget;
import com.tngtech.archunit.core.domain.JavaAccess.Functions.Get;
import com.tngtech.archunit.core.domain.JavaClass;
import com.tngtech.archunit.core.domain.JavaField;
import com.tngtech.archunit.core.domain.JavaFieldAccess;
import com.tngtech.archunit.core.importer.ClassFileImporter;
import com.tngtech.archunit.lang.ArchCondition;
Expand All @@ -37,12 +38,14 @@
import static com.tngtech.archunit.core.domain.properties.HasParameterTypes.Predicates.rawParameterTypes;
import static com.tngtech.archunit.core.domain.properties.HasType.Functions.GET_RAW_TYPE;
import static com.tngtech.archunit.lang.conditions.ArchConditions.accessField;
import static com.tngtech.archunit.lang.conditions.ArchConditions.beAnnotatedWith;
import static com.tngtech.archunit.lang.conditions.ArchConditions.callCodeUnitWhere;
import static com.tngtech.archunit.lang.conditions.ArchConditions.callMethodWhere;
import static com.tngtech.archunit.lang.conditions.ArchConditions.dependOnClassesThat;
import static com.tngtech.archunit.lang.conditions.ArchConditions.setFieldWhere;
import static com.tngtech.archunit.lang.conditions.ArchPredicates.is;
import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noClasses;
import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noFields;

/**
* When checking these rules, it is always important to remember that all necessary classes need to be
Expand Down Expand Up @@ -151,4 +154,28 @@ private static ArchCondition<JavaClass> throwGenericExceptions() {
@PublicAPI(usage = ACCESS)
public static final ArchRule NO_CLASSES_SHOULD_USE_JODATIME =
noClasses().should(USE_JODATIME).because("modern Java projects use the [java.time] API instead");

/**
* For information about checking this condition, refer to {@link GeneralCodingRules}.
*/
@PublicAPI(usage = ACCESS)
public static final ArchCondition<JavaField> USE_FIELD_INJECTION = useFieldInjection().as("use field injection");

private static ArchCondition<JavaField> useFieldInjection() {
ArchCondition<JavaField> annotatedWithAutowired = beAnnotatedWith("org.springframework.beans.factory.annotation.Autowired");
ArchCondition<JavaField> annotatedWithValue = beAnnotatedWith("org.springframework.beans.factory.annotation.Value");
ArchCondition<JavaField> annotatedWithInject = beAnnotatedWith("javax.inject.Inject");
ArchCondition<JavaField> annotatedWithResource = beAnnotatedWith("javax.annotation.Resource");
return annotatedWithAutowired.or(annotatedWithValue).or(annotatedWithInject).or(annotatedWithResource);
}

/**
* Field injection is seen as an anti-pattern.
* It is a good practice to use constructor injection for mandatory dependencies and setter injection for optional dependencies.
* <br>
* For information about checking this rule, refer to {@link GeneralCodingRules}.
*/
@PublicAPI(usage = ACCESS)
public static final ArchRule NO_CLASSES_SHOULD_USE_FIELD_INJECTION =
noFields().should(USE_FIELD_INJECTION).because("field injection is considered harmful");
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package com.tngtech.archunit.library;

import javax.annotation.Resource;
import javax.inject.Inject;

import com.tngtech.archunit.core.domain.JavaClasses;
import org.junit.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;

import static com.tngtech.archunit.core.domain.TestUtils.importClasses;
import static com.tngtech.archunit.library.GeneralCodingRules.NO_CLASSES_SHOULD_USE_FIELD_INJECTION;
import static com.tngtech.archunit.testutil.Assertions.assertThat;

public class GeneralCodingRulesTest {

@Test
public void classesShouldNotUseFieldInjection_ClassWithoutFieldInjection() {
JavaClasses classes = importClasses(ClassWithoutFieldInjection.class);
assertThat(NO_CLASSES_SHOULD_USE_FIELD_INJECTION).checking(classes).hasNoViolation();
}

@Test
public void classesShouldNotUseFieldInjection_ClassWithSpringAutowiredFieldInjection() {
JavaClasses classes = importClasses(ClassWithSpringAutowiredFieldInjection.class);
assertThat(NO_CLASSES_SHOULD_USE_FIELD_INJECTION).checking(classes).hasViolations(1);
}

@Test
public void classesShouldNotUseFieldInjection_ClassWithSpringValueFieldInjection() {
JavaClasses classes = importClasses(ClassWithSpringValueFieldInjection.class);
assertThat(NO_CLASSES_SHOULD_USE_FIELD_INJECTION).checking(classes).hasViolations(1);
}

@Test
public void classesShouldNotUseFieldInjection_ClassWithJakartaInjectFieldInjection() {
JavaClasses classes = importClasses(ClassWithJakartaInjectFieldInjection.class);
assertThat(NO_CLASSES_SHOULD_USE_FIELD_INJECTION).checking(classes).hasViolations(1);
}

@Test
public void classesShouldNotUseFieldInjection_ClassWithJakartaResourceFieldInjection() {
JavaClasses classes = importClasses(ClassWithJakartaResourceFieldInjection.class);
assertThat(NO_CLASSES_SHOULD_USE_FIELD_INJECTION).checking(classes).hasViolations(1);
}

private static class ClassWithoutFieldInjection {
@SuppressWarnings("unused")
private String value;
}

private static class ClassWithSpringAutowiredFieldInjection {
@Autowired
@SuppressWarnings("unused")
private String value;
}

private static class ClassWithSpringValueFieldInjection {
@Value("${name}")
@SuppressWarnings("unused")
private String value;
}

private static class ClassWithJakartaInjectFieldInjection {
@Inject
@SuppressWarnings("unused")
private String value;
}

private static class ClassWithJakartaResourceFieldInjection {
@Resource
@SuppressWarnings("unused")
private String value;
}
}
6 changes: 5 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,11 @@ ext {
junit_dataprovider : 'com.tngtech.java:junit-dataprovider:1.11.0',
mockito : 'org.mockito:mockito-core:2.27.0',
assertj : 'org.assertj:assertj-core:2.9.1',
assertj_guava : 'org.assertj:assertj-guava:2.0.1'
assertj_guava : 'org.assertj:assertj-guava:2.0.1',

springBeans : 'org.springframework:spring-beans:5.2.2.RELEASE',
jakartaInject : 'jakarta.inject:jakarta.inject-api:1.0',
jakartaAnnotations : 'jakarta.annotation:jakarta.annotation-api:1.3.5',
]

postfixedJar = { File jarFile, String postfix ->
Expand Down

0 comments on commit 8bfe5a8

Please sign in to comment.