Skip to content

Commit 4326817

Browse files
philwebbphilwebb
committed
Polish ArchitectureCheck
Extract rules to a new class and use more helper methods.
1 parent 51889b8 commit 4326817

File tree

2 files changed

+305
-223
lines changed

2 files changed

+305
-223
lines changed

‎buildSrc/src/main/java/org/springframework/boot/build/architecture/ArchitectureCheck.java

+33-223
Original file line numberDiff line numberDiff line change
@@ -18,41 +18,22 @@
1818

1919
import java.io.File;
2020
import java.io.IOException;
21-
import java.net.URLDecoder;
22-
import java.net.URLEncoder;
2321
import java.nio.file.Files;
22+
import java.nio.file.Path;
2423
import java.nio.file.StandardOpenOption;
2524
import java.util.Collections;
2625
import java.util.List;
27-
import java.util.Map;
28-
import java.util.Objects;
2926
import java.util.function.Supplier;
30-
import java.util.stream.Collectors;
27+
import java.util.stream.Stream;
3128

32-
import com.tngtech.archunit.base.DescribedPredicate;
33-
import com.tngtech.archunit.core.domain.JavaAnnotation;
34-
import com.tngtech.archunit.core.domain.JavaCall;
35-
import com.tngtech.archunit.core.domain.JavaClass;
36-
import com.tngtech.archunit.core.domain.JavaClass.Predicates;
3729
import com.tngtech.archunit.core.domain.JavaClasses;
38-
import com.tngtech.archunit.core.domain.JavaMethod;
39-
import com.tngtech.archunit.core.domain.JavaParameter;
40-
import com.tngtech.archunit.core.domain.JavaType;
41-
import com.tngtech.archunit.core.domain.properties.CanBeAnnotated;
42-
import com.tngtech.archunit.core.domain.properties.HasName;
43-
import com.tngtech.archunit.core.domain.properties.HasOwner.Predicates.With;
44-
import com.tngtech.archunit.core.domain.properties.HasParameterTypes;
4530
import com.tngtech.archunit.core.importer.ClassFileImporter;
46-
import com.tngtech.archunit.lang.ArchCondition;
4731
import com.tngtech.archunit.lang.ArchRule;
48-
import com.tngtech.archunit.lang.ConditionEvents;
4932
import com.tngtech.archunit.lang.EvaluationResult;
50-
import com.tngtech.archunit.lang.SimpleConditionEvent;
51-
import com.tngtech.archunit.lang.syntax.ArchRuleDefinition;
52-
import com.tngtech.archunit.library.dependencies.SlicesRuleDefinition;
5333
import org.gradle.api.DefaultTask;
5434
import org.gradle.api.GradleException;
5535
import org.gradle.api.Task;
36+
import org.gradle.api.Transformer;
5637
import org.gradle.api.file.DirectoryProperty;
5738
import org.gradle.api.file.FileCollection;
5839
import org.gradle.api.file.FileTree;
@@ -69,232 +50,63 @@
6950
import org.gradle.api.tasks.SkipWhenEmpty;
7051
import org.gradle.api.tasks.TaskAction;
7152

72-
import org.springframework.util.ResourceUtils;
73-
7453
/**
7554
* {@link Task} that checks for architecture problems.
7655
*
7756
* @author Andy Wilkinson
7857
* @author Yanming Zhou
7958
* @author Scott Frederick
8059
* @author Ivan Malutin
60+
* @author Phillip Webb
8161
*/
8262
public abstract class ArchitectureCheck extends DefaultTask {
8363

8464
private FileCollection classes;
8565

8666
public ArchitectureCheck() {
8767
getOutputDirectory().convention(getProject().getLayout().getBuildDirectory().dir(getName()));
88-
getProhibitObjectsRequireNonNull().convention(true);
89-
getRules().addAll(allPackagesShouldBeFreeOfTangles(),
90-
allBeanPostProcessorBeanMethodsShouldBeStaticAndHaveParametersThatWillNotCausePrematureInitialization(),
91-
allBeanFactoryPostProcessorBeanMethodsShouldBeStaticAndHaveNoParameters(),
92-
noClassesShouldCallStepVerifierStepVerifyComplete(),
93-
noClassesShouldConfigureDefaultStepVerifierTimeout(), noClassesShouldCallCollectorsToList(),
94-
noClassesShouldCallURLEncoderWithStringEncoding(), noClassesShouldCallURLDecoderWithStringEncoding(),
95-
noClassesShouldLoadResourcesUsingResourceUtils(), noClassesShouldCallStringToUpperCaseWithoutLocale(),
96-
noClassesShouldCallStringToLowerCaseWithoutLocale(),
97-
conditionalOnMissingBeanShouldNotSpecifyOnlyATypeThatIsTheSameAsMethodReturnType());
98-
getRules().addAll(getProhibitObjectsRequireNonNull()
99-
.map((prohibit) -> prohibit ? noClassesShouldCallObjectsRequireNonNull() : Collections.emptyList()));
100-
getRuleDescriptions().set(getRules().map((rules) -> rules.stream().map(ArchRule::getDescription).toList()));
68+
getRules().addAll(getProhibitObjectsRequireNonNull().convention(true)
69+
.map(whenTrue(ArchitectureRules::noClassesShouldCallObjectsRequireNonNull)));
70+
getRules().addAll(ArchitectureRules.standard());
71+
getRuleDescriptions().set(getRules().map(this::asDescriptions));
72+
}
73+
74+
private Transformer<List<ArchRule>, Boolean> whenTrue(Supplier<List<ArchRule>> rules) {
75+
return (in) -> (!in) ? Collections.emptyList() : rules.get();
76+
}
77+
78+
private List<String> asDescriptions(List<ArchRule> rules) {
79+
return rules.stream().map(ArchRule::getDescription).toList();
10180
}
10281

10382
@TaskAction
10483
void checkArchitecture() throws IOException {
105-
JavaClasses javaClasses = new ClassFileImporter()
106-
.importPaths(this.classes.getFiles().stream().map(File::toPath).toList());
107-
List<EvaluationResult> violations = getRules().get()
108-
.stream()
109-
.map((rule) -> rule.evaluate(javaClasses))
110-
.filter(EvaluationResult::hasViolation)
111-
.toList();
84+
JavaClasses javaClasses = new ClassFileImporter().importPaths(classFilesPaths());
85+
List<EvaluationResult> violations = evaluate(javaClasses).filter(EvaluationResult::hasViolation).toList();
11286
File outputFile = getOutputDirectory().file("failure-report.txt").get().getAsFile();
113-
outputFile.getParentFile().mkdirs();
87+
writeViolationReport(violations, outputFile);
11488
if (!violations.isEmpty()) {
115-
StringBuilder report = new StringBuilder();
116-
for (EvaluationResult violation : violations) {
117-
report.append(violation.getFailureReport());
118-
report.append(String.format("%n"));
119-
}
120-
Files.writeString(outputFile.toPath(), report.toString(), StandardOpenOption.CREATE,
121-
StandardOpenOption.TRUNCATE_EXISTING);
12289
throw new GradleException("Architecture check failed. See '" + outputFile + "' for details.");
12390
}
124-
else {
125-
outputFile.createNewFile();
126-
}
127-
}
128-
129-
private ArchRule allPackagesShouldBeFreeOfTangles() {
130-
return SlicesRuleDefinition.slices().matching("(**)").should().beFreeOfCycles();
131-
}
132-
133-
private ArchRule allBeanPostProcessorBeanMethodsShouldBeStaticAndHaveParametersThatWillNotCausePrematureInitialization() {
134-
return ArchRuleDefinition.methods()
135-
.that()
136-
.areAnnotatedWith("org.springframework.context.annotation.Bean")
137-
.and()
138-
.haveRawReturnType(Predicates.assignableTo("org.springframework.beans.factory.config.BeanPostProcessor"))
139-
.should(onlyHaveParametersThatWillNotCauseEagerInitialization())
140-
.andShould()
141-
.beStatic()
142-
.allowEmptyShould(true);
143-
}
144-
145-
private ArchCondition<JavaMethod> onlyHaveParametersThatWillNotCauseEagerInitialization() {
146-
DescribedPredicate<CanBeAnnotated> notAnnotatedWithLazy = DescribedPredicate
147-
.not(CanBeAnnotated.Predicates.annotatedWith("org.springframework.context.annotation.Lazy"));
148-
DescribedPredicate<JavaClass> notOfASafeType = DescribedPredicate
149-
.not(Predicates.assignableTo("org.springframework.beans.factory.ObjectProvider")
150-
.or(Predicates.assignableTo("org.springframework.context.ApplicationContext"))
151-
.or(Predicates.assignableTo("org.springframework.core.env.Environment")));
152-
return new ArchCondition<>("not have parameters that will cause eager initialization") {
153-
154-
@Override
155-
public void check(JavaMethod item, ConditionEvents events) {
156-
item.getParameters()
157-
.stream()
158-
.filter(notAnnotatedWithLazy)
159-
.filter((parameter) -> notOfASafeType.test(parameter.getRawType()))
160-
.forEach((parameter) -> events.add(SimpleConditionEvent.violated(parameter,
161-
parameter.getDescription() + " will cause eager initialization as it is "
162-
+ notAnnotatedWithLazy.getDescription() + " and is "
163-
+ notOfASafeType.getDescription())));
164-
}
165-
166-
};
167-
}
168-
169-
private ArchRule allBeanFactoryPostProcessorBeanMethodsShouldBeStaticAndHaveNoParameters() {
170-
return ArchRuleDefinition.methods()
171-
.that()
172-
.areAnnotatedWith("org.springframework.context.annotation.Bean")
173-
.and()
174-
.haveRawReturnType(
175-
Predicates.assignableTo("org.springframework.beans.factory.config.BeanFactoryPostProcessor"))
176-
.should(haveNoParameters())
177-
.andShould()
178-
.beStatic()
179-
.allowEmptyShould(true);
180-
}
181-
182-
private ArchCondition<JavaMethod> haveNoParameters() {
183-
return new ArchCondition<>("have no parameters") {
184-
185-
@Override
186-
public void check(JavaMethod item, ConditionEvents events) {
187-
List<JavaParameter> parameters = item.getParameters();
188-
if (!parameters.isEmpty()) {
189-
events
190-
.add(SimpleConditionEvent.violated(item, item.getDescription() + " should have no parameters"));
191-
}
192-
}
193-
194-
};
19591
}
19692

197-
private ArchRule noClassesShouldCallStringToLowerCaseWithoutLocale() {
198-
return ArchRuleDefinition.noClasses()
199-
.should()
200-
.callMethod(String.class, "toLowerCase")
201-
.because("String.toLowerCase(Locale.ROOT) should be used instead");
93+
private List<Path> classFilesPaths() {
94+
return this.classes.getFiles().stream().map(File::toPath).toList();
20295
}
20396

204-
private ArchRule noClassesShouldCallStringToUpperCaseWithoutLocale() {
205-
return ArchRuleDefinition.noClasses()
206-
.should()
207-
.callMethod(String.class, "toUpperCase")
208-
.because("String.toUpperCase(Locale.ROOT) should be used instead");
97+
private Stream<EvaluationResult> evaluate(JavaClasses javaClasses) {
98+
return getRules().get().stream().map((rule) -> rule.evaluate(javaClasses));
20999
}
210100

211-
private ArchRule noClassesShouldCallStepVerifierStepVerifyComplete() {
212-
return ArchRuleDefinition.noClasses()
213-
.should()
214-
.callMethod("reactor.test.StepVerifier$Step", "verifyComplete")
215-
.because("it can block indefinitely and expectComplete().verify(Duration) should be used instead");
216-
}
217-
218-
private ArchRule noClassesShouldConfigureDefaultStepVerifierTimeout() {
219-
return ArchRuleDefinition.noClasses()
220-
.should()
221-
.callMethod("reactor.test.StepVerifier", "setDefaultTimeout", "java.time.Duration")
222-
.because("expectComplete().verify(Duration) should be used instead");
223-
}
224-
225-
private ArchRule noClassesShouldCallCollectorsToList() {
226-
return ArchRuleDefinition.noClasses()
227-
.should()
228-
.callMethod(Collectors.class, "toList")
229-
.because("java.util.stream.Stream.toList() should be used instead");
230-
}
231-
232-
private ArchRule noClassesShouldCallURLEncoderWithStringEncoding() {
233-
return ArchRuleDefinition.noClasses()
234-
.should()
235-
.callMethod(URLEncoder.class, "encode", String.class, String.class)
236-
.because("java.net.URLEncoder.encode(String s, Charset charset) should be used instead");
237-
}
238-
239-
private ArchRule noClassesShouldCallURLDecoderWithStringEncoding() {
240-
return ArchRuleDefinition.noClasses()
241-
.should()
242-
.callMethod(URLDecoder.class, "decode", String.class, String.class)
243-
.because("java.net.URLDecoder.decode(String s, Charset charset) should be used instead");
244-
}
245-
246-
private ArchRule noClassesShouldLoadResourcesUsingResourceUtils() {
247-
return ArchRuleDefinition.noClasses()
248-
.should()
249-
.callMethodWhere(JavaCall.Predicates.target(With.owner(Predicates.type(ResourceUtils.class)))
250-
.and(JavaCall.Predicates.target(HasName.Predicates.name("getURL")))
251-
.and(JavaCall.Predicates.target(HasParameterTypes.Predicates.rawParameterTypes(String.class)))
252-
.or(JavaCall.Predicates.target(With.owner(Predicates.type(ResourceUtils.class)))
253-
.and(JavaCall.Predicates.target(HasName.Predicates.name("getFile")))
254-
.and(JavaCall.Predicates.target(HasParameterTypes.Predicates.rawParameterTypes(String.class)))))
255-
.because("org.springframework.boot.io.ApplicationResourceLoader should be used instead");
256-
}
257-
258-
private List<ArchRule> noClassesShouldCallObjectsRequireNonNull() {
259-
return List.of(
260-
ArchRuleDefinition.noClasses()
261-
.should()
262-
.callMethod(Objects.class, "requireNonNull", Object.class, String.class)
263-
.because("org.springframework.utils.Assert.notNull(Object, String) should be used instead"),
264-
ArchRuleDefinition.noClasses()
265-
.should()
266-
.callMethod(Objects.class, "requireNonNull", Object.class, Supplier.class)
267-
.because("org.springframework.utils.Assert.notNull(Object, Supplier) should be used instead"));
268-
}
269-
270-
private ArchRule conditionalOnMissingBeanShouldNotSpecifyOnlyATypeThatIsTheSameAsMethodReturnType() {
271-
return ArchRuleDefinition.methods()
272-
.that()
273-
.areAnnotatedWith("org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean")
274-
.should(notSpecifyOnlyATypeThatIsTheSameAsTheMethodReturnType())
275-
.allowEmptyShould(true);
276-
}
277-
278-
private ArchCondition<? super JavaMethod> notSpecifyOnlyATypeThatIsTheSameAsTheMethodReturnType() {
279-
return new ArchCondition<>("not specify only a type that is the same as the method's return type") {
280-
281-
@Override
282-
public void check(JavaMethod item, ConditionEvents events) {
283-
JavaAnnotation<JavaMethod> conditional = item
284-
.getAnnotationOfType("org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean");
285-
Map<String, Object> properties = conditional.getProperties();
286-
if (!properties.containsKey("type") && !properties.containsKey("name")) {
287-
conditional.get("value").ifPresent((value) -> {
288-
JavaType[] types = (JavaType[]) value;
289-
if (types.length == 1 && item.getReturnType().equals(types[0])) {
290-
events.add(SimpleConditionEvent.violated(item, conditional.getDescription()
291-
+ " should not specify only a value that is the same as the method's return type"));
292-
}
293-
});
294-
}
295-
}
296-
297-
};
101+
private void writeViolationReport(List<EvaluationResult> violations, File outputFile) throws IOException {
102+
outputFile.getParentFile().mkdirs();
103+
StringBuilder report = new StringBuilder();
104+
for (EvaluationResult violation : violations) {
105+
report.append(violation.getFailureReport());
106+
report.append(String.format("%n"));
107+
}
108+
Files.writeString(outputFile.toPath(), report.toString(), StandardOpenOption.CREATE,
109+
StandardOpenOption.TRUNCATE_EXISTING);
298110
}
299111

300112
public void setClasses(FileCollection classes) {
@@ -328,9 +140,7 @@ final FileTree getInputClasses() {
328140
@Internal
329141
public abstract Property<Boolean> getProhibitObjectsRequireNonNull();
330142

331-
@Input
332-
// The rules themselves can't be an input as they aren't serializable so we use
333-
// their descriptions instead
143+
@Input // Use descriptions as input since rules aren't serializable
334144
abstract ListProperty<String> getRuleDescriptions();
335145

336146
}

0 commit comments

Comments
 (0)