From bbe27e324f2f6dbd1e8be42cb48558dce6868266 Mon Sep 17 00:00:00 2001 From: John Burns Date: Tue, 17 Mar 2026 16:47:00 -0500 Subject: [PATCH] add new gradle rule for single file properties re-organize task related rules --- archrules-common/gradle.lockfile | 2 +- .../archrules/common/CanBeAnnotated.java | 29 +++ .../common/JavaBeanGetterPredicate.java | 2 +- .../archrules/common/CanBeAnnotatedTest.java | 29 ++- .../gradle.lockfile | 4 +- .../gradleplugins/GradleInternalApiRule.java | 4 +- .../GradlePluginBestPractices.java | 11 +- .../GradleTaskProviderApiRule.java | 218 ------------------ .../gradleplugins/GradleTaskRules.java | 161 +++++++++++++ .../archrules/gradleplugins/Predicates.java | 21 +- .../gradleplugins/TaskAbstractGetterRule.java | 45 ++++ ...tRule.java => TaskHasInputOutputRule.java} | 23 +- .../TaskInputOutputFieldRule.java | 24 ++ .../TaskRegularFilePropertyRule.java | 26 +++ ...RuleTest.java => GradleTaskRulesTest.java} | 112 +-------- .../gradleplugins/PredicatesTest.java | 26 ++- .../TaskAbstractGetterRuleTest.java | 63 +++++ ...t.java => TaskHasInputOutputRuleTest.java} | 20 +- .../TaskInputOutputFieldRuleTest.java | 61 +++++ .../TaskRegularFilePropertyRuleTest.java | 43 ++++ 20 files changed, 538 insertions(+), 386 deletions(-) delete mode 100644 archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/GradleTaskProviderApiRule.java create mode 100644 archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/GradleTaskRules.java create mode 100644 archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/TaskAbstractGetterRule.java rename archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/{GradleTaskInputOutputRule.java => TaskHasInputOutputRule.java} (66%) create mode 100644 archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/TaskInputOutputFieldRule.java create mode 100644 archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/TaskRegularFilePropertyRule.java rename archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/{GradleTaskProviderApiRuleTest.java => GradleTaskRulesTest.java} (53%) create mode 100644 archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/TaskAbstractGetterRuleTest.java rename archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/{GradleTaskInputOutputRuleTest.java => TaskHasInputOutputRuleTest.java} (81%) create mode 100644 archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/TaskInputOutputFieldRuleTest.java create mode 100644 archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/TaskRegularFilePropertyRuleTest.java diff --git a/archrules-common/gradle.lockfile b/archrules-common/gradle.lockfile index 5439cf6..af972b2 100644 --- a/archrules-common/gradle.lockfile +++ b/archrules-common/gradle.lockfile @@ -3,7 +3,7 @@ # This file is expected to be part of source control. ch.qos.logback:logback-classic:1.5.20=testCompileClasspath,testRuntimeClasspath ch.qos.logback:logback-core:1.5.20=testCompileClasspath,testRuntimeClasspath -com.netflix.nebula:nebula-archrules-core:0.15.1=testCompileClasspath,testRuntimeClasspath +com.netflix.nebula:nebula-archrules-core:0.15.2=testCompileClasspath,testRuntimeClasspath com.tngtech.archunit:archunit:1.4.1=compileClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath net.bytebuddy:byte-buddy:1.17.7=testCompileClasspath,testRuntimeClasspath org.apiguardian:apiguardian-api:1.1.2=testCompileClasspath diff --git a/archrules-common/src/main/java/com/netflix/nebula/archrules/common/CanBeAnnotated.java b/archrules-common/src/main/java/com/netflix/nebula/archrules/common/CanBeAnnotated.java index 18cb2f4..ac1dec6 100644 --- a/archrules-common/src/main/java/com/netflix/nebula/archrules/common/CanBeAnnotated.java +++ b/archrules-common/src/main/java/com/netflix/nebula/archrules/common/CanBeAnnotated.java @@ -3,7 +3,12 @@ import com.tngtech.archunit.base.DescribedPredicate; import org.jspecify.annotations.NullMarked; +import java.util.Arrays; +import java.util.stream.StreamSupport; + +import static com.tngtech.archunit.core.domain.JavaClass.Predicates.assignableTo; import static com.tngtech.archunit.core.domain.properties.CanBeAnnotated.Predicates.annotatedWith; +import static com.tngtech.archunit.core.domain.properties.HasType.Predicates.rawType; @NullMarked public class CanBeAnnotated { @@ -26,5 +31,29 @@ public static DescribedPredicate annotatedWithAny( + String... annotationTypes) { + return Arrays.stream(annotationTypes) + .map(com.tngtech.archunit.core.domain.properties.CanBeAnnotated.Predicates::annotatedWith) + .reduce((a, b) -> a.or(b)) + .orElseGet(() -> annotatedWith(rawType(assignableTo(Object.class)))) + .as("annotated with any [%s]", String.join(", ", annotationTypes)); + } + + /** + * Creates a predicate matching elements annotated with any of the given annotations. + */ + public static DescribedPredicate annotatedWithAny( + Iterable annotationTypes) { + return StreamSupport.stream(annotationTypes.spliterator(), false) + .map(com.tngtech.archunit.core.domain.properties.CanBeAnnotated.Predicates::annotatedWith) + .reduce((a, b) -> a.or(b)) + .orElseGet(() -> annotatedWith(rawType(assignableTo(Object.class)))) + .as("annotated with any [%s]", String.join(", ", annotationTypes)); + } } } diff --git a/archrules-common/src/main/java/com/netflix/nebula/archrules/common/JavaBeanGetterPredicate.java b/archrules-common/src/main/java/com/netflix/nebula/archrules/common/JavaBeanGetterPredicate.java index 88860df..030cc35 100644 --- a/archrules-common/src/main/java/com/netflix/nebula/archrules/common/JavaBeanGetterPredicate.java +++ b/archrules-common/src/main/java/com/netflix/nebula/archrules/common/JavaBeanGetterPredicate.java @@ -12,7 +12,7 @@ class JavaBeanGetterPredicate extends DescribedPredicate { JavaBeanGetterPredicate() { - super("getter"); + super("a getter"); } @Override diff --git a/archrules-common/src/test/java/com/netflix/nebula/archrules/common/CanBeAnnotatedTest.java b/archrules-common/src/test/java/com/netflix/nebula/archrules/common/CanBeAnnotatedTest.java index e48b57c..995eed7 100644 --- a/archrules-common/src/test/java/com/netflix/nebula/archrules/common/CanBeAnnotatedTest.java +++ b/archrules-common/src/test/java/com/netflix/nebula/archrules/common/CanBeAnnotatedTest.java @@ -2,6 +2,7 @@ import org.junit.jupiter.api.Test; +import static com.netflix.nebula.archrules.common.CanBeAnnotated.Predicates.annotatedWithAny; import static com.netflix.nebula.archrules.common.Util.scanClass; import static org.assertj.core.api.Assertions.assertThat; @@ -16,21 +17,43 @@ public void test_javaDeprecatedClass() { @Test public void test_javaDeprecatedForRemovalClass() { assertThat(CanBeAnnotated.Predicates.deprecatedForRemoval().test(scanClass(Usage.class))).isFalse(); - assertThat(CanBeAnnotated.Predicates.deprecatedForRemoval().getDescription()).isEqualTo("deprecated for removal"); + assertThat(CanBeAnnotated.Predicates.deprecatedForRemoval().getDescription()) + .isEqualTo("deprecated for removal"); + } + + @Test + public void test_annotatedWithAny() { + final var rule = annotatedWithAny(SomeAnnotation.class.getName(), AnotherAnnotation.class.getName()); + assertThat(rule.test(scanClass(Annotated1.class))).isTrue(); + assertThat(rule.test(scanClass(Annotated2.class))).isTrue(); + assertThat(rule.test(scanClass(Usage.class))).isFalse(); + assertThat(rule.getDescription()).startsWith("annotated with any ["); } @Deprecated static class JavaDeprecatedClass { - } @Deprecated(forRemoval = true) static class JavaDeprecatedForRemovalClass { - } static class Usage { JavaDeprecatedClass aField; JavaDeprecatedForRemovalClass deprecatedForRemoval; } + + @SomeAnnotation + static class Annotated1 { + } + + @AnotherAnnotation + static class Annotated2 { + } + + @interface SomeAnnotation { + } + + @interface AnotherAnnotation { + } } diff --git a/archrules-gradle-plugin-development/gradle.lockfile b/archrules-gradle-plugin-development/gradle.lockfile index aafd7cb..824538c 100644 --- a/archrules-gradle-plugin-development/gradle.lockfile +++ b/archrules-gradle-plugin-development/gradle.lockfile @@ -12,9 +12,9 @@ com.netflix.nebula:archrules-joda:0.9.0=archRules,archRulesArchRulesRuntime,arch com.netflix.nebula:archrules-nullability:0.9.0=archRules,archRulesArchRulesRuntime,archRulesTestArchRulesRuntime,mainArchRulesRuntime,testArchRulesRuntime com.netflix.nebula:archrules-security:0.9.0=archRules,archRulesArchRulesRuntime,archRulesTestArchRulesRuntime,mainArchRulesRuntime,testArchRulesRuntime com.netflix.nebula:archrules-testing-frameworks:0.9.0=archRules,archRulesArchRulesRuntime,archRulesTestArchRulesRuntime,mainArchRulesRuntime,testArchRulesRuntime -com.netflix.nebula:nebula-archrules-core:0.15.1=archRulesArchRulesRuntime,archRulesCompileClasspath,archRulesJsonReportingResolved,archRulesRuntimeClasspath,archRulesTestArchRulesRuntime,archRulesTestCompileClasspath,archRulesTestRuntimeClasspath +com.netflix.nebula:nebula-archrules-core:0.15.2=archRulesArchRulesRuntime,archRulesCompileClasspath,archRulesJsonReportingResolved,archRulesRuntimeClasspath,archRulesTestArchRulesRuntime,archRulesTestCompileClasspath,archRulesTestRuntimeClasspath com.netflix.nebula:nebula-archrules-core:0.8.0=archRules,mainArchRulesRuntime,testArchRulesRuntime -com.netflix.nebula:nebula-archrules-gradle-plugin:0.15.1=archRulesJsonReportingResolved +com.netflix.nebula:nebula-archrules-gradle-plugin:0.15.2=archRulesJsonReportingResolved com.tngtech.archunit:archunit:1.4.1=archRules,archRulesArchRulesRuntime,archRulesCompileClasspath,archRulesJsonReportingResolved,archRulesRuntimeClasspath,archRulesTestArchRulesRuntime,archRulesTestCompileClasspath,archRulesTestRuntimeClasspath,mainArchRulesRuntime,testArchRulesRuntime net.bytebuddy:byte-buddy:1.17.7=archRulesTestArchRulesRuntime,archRulesTestCompileClasspath,archRulesTestRuntimeClasspath org.assertj:assertj-core:3.27.6=archRulesTestArchRulesRuntime,archRulesTestCompileClasspath,archRulesTestRuntimeClasspath diff --git a/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/GradleInternalApiRule.java b/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/GradleInternalApiRule.java index b667062..b46e8c7 100644 --- a/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/GradleInternalApiRule.java +++ b/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/GradleInternalApiRule.java @@ -24,7 +24,7 @@ public class GradleInternalApiRule { * and may change or be removed between versions without notice. Use only public * Gradle APIs to ensure compatibility across Gradle versions. */ - public static final ArchRule PLUGIN_INTERNAL = ArchRuleDefinition.priority(Priority.MEDIUM) + public static final ArchRule PLUGIN_INTERNAL = ArchRuleDefinition.priority(Priority.LOW) .noClasses() .that().implement("org.gradle.api.Plugin") .should().dependOnClassesThat(internalGradleClass) @@ -43,7 +43,7 @@ public class GradleInternalApiRule { * and may change or be removed between versions without notice. Use only public * Gradle APIs to ensure compatibility across Gradle versions. */ - public static final ArchRule TASK_INTERNAL = ArchRuleDefinition.priority(Priority.MEDIUM) + public static final ArchRule TASK_INTERNAL = ArchRuleDefinition.priority(Priority.LOW) .noClasses() .that().areAssignableTo("org.gradle.api.Task") .and().areNotInterfaces() diff --git a/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/GradlePluginBestPractices.java b/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/GradlePluginBestPractices.java index 0442b56..75582bc 100644 --- a/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/GradlePluginBestPractices.java +++ b/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/GradlePluginBestPractices.java @@ -24,9 +24,6 @@ import static com.netflix.nebula.archrules.gradleplugins.GradleTaskCacheabilityRule.METHODS_PATH_SENSITIVITY; import static com.netflix.nebula.archrules.gradleplugins.GradleTaskContainerApiRule.USE_CONFIGURE_EACH_INSTEAD_OF_ALL; import static com.netflix.nebula.archrules.gradleplugins.GradleTaskContainerApiRule.USE_NAMED_INSTEAD_OF_GET_BY_NAME; -import static com.netflix.nebula.archrules.gradleplugins.GradleTaskInputOutputRule.INPUTS_OUTPUTS; -import static com.netflix.nebula.archrules.gradleplugins.GradleTaskProviderApiRule.ABSTRACT_GETTERS; -import static com.netflix.nebula.archrules.gradleplugins.GradleTaskProviderApiRule.PROVIDER_PROPERTIES; @NullMarked @SuppressWarnings("unused") @@ -34,8 +31,11 @@ public class GradlePluginBestPractices implements ArchRulesService { @Override public Map getRules() { Map rules = new HashMap<>(); - rules.put("provider properties", PROVIDER_PROPERTIES); - rules.put("abstract getters", ABSTRACT_GETTERS); + rules.put("Task input/output file should be regular", TaskRegularFilePropertyRule.RULE); + rules.put("abstract getters", TaskAbstractGetterRule.RULE); + rules.put("Task declares inputs and/or outputs", TaskHasInputOutputRule.RULE); + rules.put("Task input/output should not be fields", TaskInputOutputFieldRule.RULE); + rules.put("Task input/output should use Provider API", GradleTaskRules.PROVIDER_PROPERTIES); rules.put("task project access", taskActionShouldNotAccessProject); rules.put("task dependencies", taskActionShouldNotCallGetTaskDependencies); rules.put("lazy task registration", LAZY_TASK_CREATION); @@ -50,7 +50,6 @@ public Map getRules() { rules.put("Plugin should inject ProviderFactory", USE_INJECTED_PROVIDER_FACTORY); rules.put("Extension fields use Provider API", EXTENSION_FIELDS_USE_PROVIDER_API); rules.put("Extension abstract getters", EXTENSION_ABSTRACT_GETTERS); - rules.put("Task declares inputs and/or outputs", INPUTS_OUTPUTS); rules.put("Cacheable Task input field path sensitivity", FIELDS_PATH_SENSITIVITY); rules.put("Cacheable Task input method path sensitivity", METHODS_PATH_SENSITIVITY); rules.put("Apply plugins by ID", APPLY_BY_ID); diff --git a/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/GradleTaskProviderApiRule.java b/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/GradleTaskProviderApiRule.java deleted file mode 100644 index 7067967..0000000 --- a/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/GradleTaskProviderApiRule.java +++ /dev/null @@ -1,218 +0,0 @@ -package com.netflix.nebula.archrules.gradleplugins; - -import com.tngtech.archunit.base.DescribedPredicate; -import com.tngtech.archunit.core.domain.JavaClass; -import com.tngtech.archunit.core.domain.JavaField; -import com.tngtech.archunit.core.domain.JavaMethod; -import com.tngtech.archunit.core.domain.JavaModifier; -import com.tngtech.archunit.core.domain.properties.CanBeAnnotated; -import com.tngtech.archunit.lang.ArchCondition; -import com.tngtech.archunit.lang.ArchRule; -import com.tngtech.archunit.lang.ConditionEvents; -import com.tngtech.archunit.lang.Priority; -import com.tngtech.archunit.lang.SimpleConditionEvent; -import com.tngtech.archunit.lang.conditions.ArchPredicates; -import com.tngtech.archunit.lang.syntax.ArchRuleDefinition; -import org.jspecify.annotations.NullMarked; - -import java.util.HashMap; -import java.util.Map; - -import static com.netflix.nebula.archrules.common.JavaMethod.Predicates.aGetter; -import static com.netflix.nebula.archrules.gradleplugins.Predicates.hasRichPropertyReturnType; -import static com.netflix.nebula.archrules.gradleplugins.Predicates.isProviderApiType; -import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.ANNOTATION_INPUT_DIRECTORY; -import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.ANNOTATION_INPUT_FILE; -import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.ANNOTATION_OUTPUT_DIRECTORY; -import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.ANNOTATION_OUTPUT_FILE; -import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.JAVA_IO_FILE; -import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.JAVA_UTIL_LIST; -import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.JAVA_UTIL_MAP; -import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.JAVA_UTIL_SET; -import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.RECOMMENDATION_DIRECTORY_PROPERTY; -import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.RECOMMENDATION_LIST_PROPERTY; -import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.RECOMMENDATION_MAP_PROPERTY; -import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.RECOMMENDATION_REGULAR_FILE_PROPERTY; -import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.RECOMMENDATION_SET_PROPERTY; -import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.TASK_TYPES_REQUIRING_PROVIDER; -import static com.tngtech.archunit.base.DescribedPredicate.not; -import static com.tngtech.archunit.core.domain.JavaMember.Predicates.declaredIn; -import static com.tngtech.archunit.core.domain.JavaModifier.PRIVATE; -import static com.tngtech.archunit.core.domain.properties.CanBeAnnotated.Predicates.annotatedWith; -import static com.tngtech.archunit.core.domain.properties.HasModifiers.Predicates.modifier; -import static com.tngtech.archunit.lang.conditions.ArchPredicates.are; - -/** - * Rules for Gradle task classes to ensure they use the Provider API for lazy configuration. - *

- * The Provider API ({@code Property}, {@code Provider}, {@code ConfigurableFileCollection}) - * enables lazy configuration and configuration avoidance, which are critical for build performance. - */ -@NullMarked -class GradleTaskProviderApiRule { - - private static class LazyHolder { - private static final Map TYPE_TO_PROVIDER; - - static { - Map map = new HashMap<>(); - map.put(JAVA_UTIL_LIST, RECOMMENDATION_LIST_PROPERTY); - map.put(JAVA_UTIL_SET, RECOMMENDATION_SET_PROPERTY); - map.put(JAVA_UTIL_MAP, RECOMMENDATION_MAP_PROPERTY); - TYPE_TO_PROVIDER = map; - } - } - - private static java.util.Set getMutableTypesThatShouldUseProvider() { - return TASK_TYPES_REQUIRING_PROVIDER; - } - - private static Map getTypeToProviderMap() { - return LazyHolder.TYPE_TO_PROVIDER; - } - - /** - * Detects task input/output properties that should use Provider API types. - *

- * Task properties annotated with {@code @Input}, {@code @InputFile}, {@code @InputDirectory}, - * {@code @OutputFile}, {@code @OutputDirectory}, etc. should use Provider API types - * ({@code Property}, {@code RegularFileProperty}, {@code DirectoryProperty}, etc.) - * instead of plain types for lazy configuration. - */ - static final ArchRule PROVIDER_PROPERTIES = ArchRuleDefinition.priority(Priority.MEDIUM) - .classes() - .that().areAssignableTo("org.gradle.api.Task") - .and().areNotInterfaces() - .should(useProviderApiForInputOutputProperties()) - .allowEmptyShould(true) - .because( - "Task input/output properties should use Provider API types (Property, RegularFileProperty, " + - "DirectoryProperty, ConfigurableFileCollection) instead of plain types. " + - "This enables lazy configuration and configuration avoidance, which significantly improves build performance. " + - "See https://docs.gradle.org/current/userguide/lazy_configuration.html" - ); - - private static ArchCondition useProviderApiForInputOutputProperties() { - return new ArchCondition("use Provider API for input/output properties") { - @Override - public void check(JavaClass taskClass, ConditionEvents events) { - for (JavaField field : taskClass.getAllFields()) { - checkFieldForProviderApiUsage(taskClass, field, events); - } - - for (JavaMethod method : taskClass.getAllMethods()) { - checkMethodForProviderApiUsage(taskClass, method, events); - } - } - - private void checkFieldForProviderApiUsage(JavaClass taskClass, JavaField field, ConditionEvents events) { - if (!hasInputOutputAnnotation(field)) { - return; - } - - if (shouldUseProviderApi(field.getRawType()) && !isProviderApiType(field.getRawType())) { - String recommendation = getSpecificRecommendation(field.getRawType(), field); - String message = String.format( - "Task %s has field '%s' of type %s with input/output annotation. " + - "Use %s for lazy configuration.", - taskClass.getSimpleName(), - field.getName(), - field.getRawType().getSimpleName(), - recommendation - ); - events.add(SimpleConditionEvent.violated(field, message)); - } - } - - private void checkMethodForProviderApiUsage(JavaClass taskClass, JavaMethod method, ConditionEvents events) { - if (!hasInputOutputAnnotation(method)) { - return; - } - - if (!aGetter().test(method)) { - return; - } - - JavaClass returnType = method.getRawReturnType(); - if (shouldUseProviderApi(returnType) && !isProviderApiType(returnType)) { - String recommendation = getSpecificRecommendation(returnType, method); - String message = String.format( - "Task %s has getter '%s()' returning type %s with input/output annotation. " + - "Use %s for lazy configuration.", - taskClass.getSimpleName(), - method.getName(), - returnType.getSimpleName(), - recommendation - ); - events.add(SimpleConditionEvent.violated(method, message)); - } - } - - private boolean hasInputOutputAnnotation(CanBeAnnotated element) { - return Predicates.hasInputOutputAnnotation.test(element); - } - - private boolean shouldUseProviderApi(JavaClass type) { - String typeName = type.getName(); - return getMutableTypesThatShouldUseProvider().contains(typeName); - } - - private String getSpecificRecommendation(JavaClass type, JavaField field) { - return getRecommendationForType( - type, - field.isAnnotatedWith(ANNOTATION_INPUT_FILE) || field.isAnnotatedWith(ANNOTATION_OUTPUT_FILE), - field.isAnnotatedWith(ANNOTATION_INPUT_DIRECTORY) || field.isAnnotatedWith(ANNOTATION_OUTPUT_DIRECTORY) - ); - } - - private String getSpecificRecommendation(JavaClass type, JavaMethod method) { - return getRecommendationForType( - type, - method.isAnnotatedWith(ANNOTATION_INPUT_FILE) || method.isAnnotatedWith(ANNOTATION_OUTPUT_FILE), - method.isAnnotatedWith(ANNOTATION_INPUT_DIRECTORY) || method.isAnnotatedWith(ANNOTATION_OUTPUT_DIRECTORY) - ); - } - - private String getRecommendationForType(JavaClass type, boolean isFileAnnotation, boolean isDirectoryAnnotation) { - String typeName = type.getName(); - - if (JAVA_IO_FILE.equals(typeName)) { - if (isFileAnnotation) { - return RECOMMENDATION_REGULAR_FILE_PROPERTY; - } - if (isDirectoryAnnotation) { - return RECOMMENDATION_DIRECTORY_PROPERTY; - } - return RECOMMENDATION_REGULAR_FILE_PROPERTY + " or " + RECOMMENDATION_DIRECTORY_PROPERTY; - } - - String mappedRecommendation = getTypeToProviderMap().get(typeName); - if (mappedRecommendation != null) { - return mappedRecommendation; - } - - return "Property<" + type.getSimpleName() + ">"; - } - }; - } - - static final DescribedPredicate richTaskPropertyGetters = ArchPredicates.are(aGetter()) - .and(are(hasRichPropertyReturnType)) - .and(not(modifier(PRIVATE))) - .and(not(annotatedWith("javax.inject.Inject"))) - .and(not(annotatedWith("org.gradle.api.tasks.options.OptionValues"))) - .and(not(declaredIn("org.gradle.api.Task"))) - .and(not(declaredIn("org.gradle.api.DefaultTask"))) - .and(not(declaredIn("org.gradle.api.internal.AbstractTask"))) - .as("task property getters"); - - /** - * Inspired by - * {@link Gradle} - */ - static final ArchRule ABSTRACT_GETTERS = ArchRuleDefinition.priority(Priority.MEDIUM) - .methods().that(are(richTaskPropertyGetters)) - .should().haveModifier(JavaModifier.ABSTRACT) - .allowEmptyShould(true) - .because("task implementations should define properties as abstract getters"); -} diff --git a/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/GradleTaskRules.java b/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/GradleTaskRules.java new file mode 100644 index 0000000..9aa41ba --- /dev/null +++ b/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/GradleTaskRules.java @@ -0,0 +1,161 @@ +package com.netflix.nebula.archrules.gradleplugins; + +import com.tngtech.archunit.base.DescribedPredicate; +import com.tngtech.archunit.core.domain.JavaClass; +import com.tngtech.archunit.core.domain.JavaMethod; +import com.tngtech.archunit.core.domain.JavaModifier; +import com.tngtech.archunit.lang.ArchCondition; +import com.tngtech.archunit.lang.ArchRule; +import com.tngtech.archunit.lang.ConditionEvents; +import com.tngtech.archunit.lang.Priority; +import com.tngtech.archunit.lang.SimpleConditionEvent; +import com.tngtech.archunit.lang.conditions.ArchPredicates; +import com.tngtech.archunit.lang.syntax.ArchRuleDefinition; +import org.jspecify.annotations.NullMarked; + +import java.util.HashMap; +import java.util.Map; + +import static com.netflix.nebula.archrules.common.CanBeAnnotated.Predicates.annotatedWithAny; +import static com.netflix.nebula.archrules.common.JavaMethod.Predicates.aGetter; +import static com.netflix.nebula.archrules.gradleplugins.Predicates.aGradleTaskClass; +import static com.netflix.nebula.archrules.gradleplugins.Predicates.annotatedWithInputOutputAnnotations; +import static com.netflix.nebula.archrules.gradleplugins.Predicates.containAnyFieldsInClassHierarchyThat; +import static com.netflix.nebula.archrules.gradleplugins.Predicates.containAnyMethodsInClassHierarchyThat; +import static com.netflix.nebula.archrules.gradleplugins.Predicates.hasRichPropertyReturnType; +import static com.netflix.nebula.archrules.gradleplugins.Predicates.haveTaskAction; +import static com.netflix.nebula.archrules.gradleplugins.Predicates.isProviderApiType; +import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.ANNOTATION_INPUT_DIRECTORY; +import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.ANNOTATION_INPUT_FILE; +import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.ANNOTATION_OUTPUT_DIRECTORY; +import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.ANNOTATION_OUTPUT_FILE; +import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.JAVA_IO_FILE; +import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.JAVA_UTIL_LIST; +import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.JAVA_UTIL_MAP; +import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.JAVA_UTIL_SET; +import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.RECOMMENDATION_DIRECTORY_PROPERTY; +import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.RECOMMENDATION_LIST_PROPERTY; +import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.RECOMMENDATION_MAP_PROPERTY; +import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.RECOMMENDATION_REGULAR_FILE_PROPERTY; +import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.RECOMMENDATION_SET_PROPERTY; +import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.TASK_TYPES_REQUIRING_PROVIDER; +import static com.tngtech.archunit.base.DescribedPredicate.not; +import static com.tngtech.archunit.core.domain.JavaMember.Predicates.declaredIn; +import static com.tngtech.archunit.core.domain.JavaModifier.PRIVATE; +import static com.tngtech.archunit.core.domain.properties.CanBeAnnotated.Predicates.annotatedWith; +import static com.tngtech.archunit.core.domain.properties.HasModifiers.Predicates.modifier; +import static com.tngtech.archunit.lang.ArchCondition.from; +import static com.tngtech.archunit.lang.conditions.ArchPredicates.are; +import static com.tngtech.archunit.lang.conditions.ArchPredicates.is; + +/** + * Rules for Gradle task classes to ensure they use the Provider API for lazy configuration. + *

+ * The Provider API ({@code Property}, {@code Provider}, {@code ConfigurableFileCollection}) + * enables lazy configuration and configuration avoidance, which are critical for build performance. + */ +@NullMarked +class GradleTaskRules { + + private static class LazyHolder { + private static final Map TYPE_TO_PROVIDER; + + static { + Map map = new HashMap<>(); + map.put(JAVA_UTIL_LIST, RECOMMENDATION_LIST_PROPERTY); + map.put(JAVA_UTIL_SET, RECOMMENDATION_SET_PROPERTY); + map.put(JAVA_UTIL_MAP, RECOMMENDATION_MAP_PROPERTY); + TYPE_TO_PROVIDER = map; + } + } + + private static java.util.Set getMutableTypesThatShouldUseProvider() { + return TASK_TYPES_REQUIRING_PROVIDER; + } + + private static Map getTypeToProviderMap() { + return LazyHolder.TYPE_TO_PROVIDER; + } + + /** + * Detects task input/output properties that should use Provider API types. + *

+ * Task abstract getter methods annotated with {@code @Input}, {@code @InputFile}, {@code @InputDirectory}, + * {@code @OutputFile}, {@code @OutputDirectory}, etc. should use Provider API types + * ({@code Property}, {@code RegularFileProperty}, {@code DirectoryProperty}, etc.) + * instead of plain types for lazy configuration. + */ + static final ArchRule PROVIDER_PROPERTIES = ArchRuleDefinition.priority(Priority.MEDIUM) + .methods() + .that().areDeclaredInClassesThat(are(aGradleTaskClass())) + .and(annotatedWithInputOutputAnnotations) + .and(is(aGetter())) + .should(useProviderApiForInputOutputPropertiesMethods()) + .allowEmptyShould(true) + .because( + "Task input/output properties should use Provider API types (Property, RegularFileProperty, " + + "DirectoryProperty, ConfigurableFileCollection) instead of plain types. " + + "This enables lazy configuration and configuration avoidance, which significantly improves build performance. " + + "See https://docs.gradle.org/current/userguide/lazy_configuration.html" + ); + + private static ArchCondition useProviderApiForInputOutputPropertiesMethods() { + return new ArchCondition("use Provider API for input/output properties") { + @Override + public void check(JavaMethod method, ConditionEvents events) { + checkMethodForProviderApiUsage(method.getOwner(), method, events); + } + }; + } + + private static void checkMethodForProviderApiUsage(JavaClass taskClass, JavaMethod method, ConditionEvents events) { + JavaClass returnType = method.getRawReturnType(); + if (shouldUseProviderApi(returnType) && !isProviderApiType(returnType)) { + String recommendation = getSpecificRecommendation(returnType, method); + String message = String.format( + "Task %s has getter '%s()' returning type %s with input/output annotation. " + + "Use %s for lazy configuration.", + taskClass.getSimpleName(), + method.getName(), + returnType.getSimpleName(), + recommendation + ); + events.add(SimpleConditionEvent.violated(method, message)); + } + } + + private static boolean shouldUseProviderApi(JavaClass type) { + String typeName = type.getName(); + return getMutableTypesThatShouldUseProvider().contains(typeName); + } + + private static String getSpecificRecommendation(JavaClass type, JavaMethod method) { + return getRecommendationForType( + type, + method.isAnnotatedWith(ANNOTATION_INPUT_FILE) || method.isAnnotatedWith(ANNOTATION_OUTPUT_FILE), + method.isAnnotatedWith(ANNOTATION_INPUT_DIRECTORY) || method.isAnnotatedWith(ANNOTATION_OUTPUT_DIRECTORY) + ); + } + + private static String getRecommendationForType(JavaClass type, boolean isFileAnnotation, boolean isDirectoryAnnotation) { + String typeName = type.getName(); + + if (JAVA_IO_FILE.equals(typeName)) { + if (isFileAnnotation) { + return RECOMMENDATION_REGULAR_FILE_PROPERTY; + } + if (isDirectoryAnnotation) { + return RECOMMENDATION_DIRECTORY_PROPERTY; + } + return RECOMMENDATION_REGULAR_FILE_PROPERTY + " or " + RECOMMENDATION_DIRECTORY_PROPERTY; + } + + String mappedRecommendation = getTypeToProviderMap().get(typeName); + if (mappedRecommendation != null) { + return mappedRecommendation; + } + + return "Property<" + type.getSimpleName() + ">"; + } + +} diff --git a/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/Predicates.java b/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/Predicates.java index c2a7b94..4860280 100644 --- a/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/Predicates.java +++ b/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/Predicates.java @@ -15,6 +15,7 @@ import java.util.Set; +import static com.netflix.nebula.archrules.common.CanBeAnnotated.Predicates.annotatedWithAny; import static com.tngtech.archunit.base.DescribedPredicate.not; import static com.tngtech.archunit.core.domain.JavaAccess.Predicates.target; import static com.tngtech.archunit.core.domain.JavaAccess.Predicates.targetOwner; @@ -73,15 +74,6 @@ class Predicates { static final DescribedPredicate haveTaskAction = ArchPredicates.have(containAnyMethodsThat(are(annotatedWith("org.gradle.api.tasks.TaskAction")))); - /** Creates a predicate matching elements annotated with any of the given annotations. */ - static DescribedPredicate annotatedWithAny(Set annotationClasses) { - return annotationClasses.stream() - .map(CanBeAnnotated.Predicates::annotatedWith) - .reduce((a, b) -> a.or(b)) - .orElseGet(() -> annotatedWith(rawType(assignableTo(Object.class)))) - .as("annotated with any [%s]", String.join(", ", annotationClasses)); - } - /** Matches classes with at least one method in their hierarchy satisfying the predicate. */ static DescribedPredicate containAnyMethodsInClassHierarchyThat(DescribedPredicate predicate) { return new ContainAnyMembersInClassHierarchyThatPredicate<>("methods", GET_ALL_METHODS, predicate); @@ -116,9 +108,9 @@ public Set apply(JavaClass input) { .as("annotated with Input file annotations"); /** Matches elements annotated with any input or output annotation. */ - static final DescribedPredicate hasInputOutputAnnotation = + static final DescribedPredicate annotatedWithInputOutputAnnotations = annotatedWithAny(INPUT_OUTPUT_ANNOTATIONS) - .as("has input or output annotation"); + .as("annotated with Input and/or Output annotations"); /** Creates a predicate matching fields with a specific raw type. */ static DescribedPredicate fieldWithType(String typeName) { @@ -140,6 +132,13 @@ public boolean test(JavaField field) { }; } + /** Predicate matching Provider API types (Property, Provider, FileCollection, etc.). */ + static DescribedPredicate aGradleTaskClass(){ + return assignableTo("org.gradle.api.Task") + .and(not(INTERFACES)) + .as("a gradle task"); + } + /** Returns true if the type is a Gradle Provider API type (Property, Provider, FileCollection, etc.). */ static boolean isProviderApiType(JavaClass type) { return type.isAssignableTo("org.gradle.api.provider.Property") || diff --git a/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/TaskAbstractGetterRule.java b/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/TaskAbstractGetterRule.java new file mode 100644 index 0000000..84df913 --- /dev/null +++ b/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/TaskAbstractGetterRule.java @@ -0,0 +1,45 @@ +package com.netflix.nebula.archrules.gradleplugins; + +import com.tngtech.archunit.base.DescribedPredicate; +import com.tngtech.archunit.core.domain.JavaMethod; +import com.tngtech.archunit.core.domain.JavaModifier; +import com.tngtech.archunit.lang.ArchRule; +import com.tngtech.archunit.lang.Priority; +import com.tngtech.archunit.lang.conditions.ArchPredicates; +import com.tngtech.archunit.lang.syntax.ArchRuleDefinition; +import org.jspecify.annotations.NullMarked; + +import static com.netflix.nebula.archrules.common.JavaMethod.Predicates.aGetter; +import static com.netflix.nebula.archrules.gradleplugins.Predicates.aGradleTaskClass; +import static com.netflix.nebula.archrules.gradleplugins.Predicates.hasRichPropertyReturnType; +import static com.tngtech.archunit.base.DescribedPredicate.not; +import static com.tngtech.archunit.core.domain.JavaMember.Predicates.declaredIn; +import static com.tngtech.archunit.core.domain.JavaModifier.PRIVATE; +import static com.tngtech.archunit.core.domain.properties.CanBeAnnotated.Predicates.annotatedWith; +import static com.tngtech.archunit.core.domain.properties.HasModifiers.Predicates.modifier; +import static com.tngtech.archunit.lang.conditions.ArchPredicates.are; + +@NullMarked +public class TaskAbstractGetterRule { + + private static final DescribedPredicate richTaskPropertyGetters = ArchPredicates.are(aGetter()) + .and(are(hasRichPropertyReturnType)) + .and(not(modifier(PRIVATE))) + .and(not(annotatedWith("javax.inject.Inject"))) + .and(not(annotatedWith("org.gradle.api.tasks.options.OptionValues"))) + .and(are(declaredIn(aGradleTaskClass()))) + .and(not(declaredIn("org.gradle.api.Task"))) + .and(not(declaredIn("org.gradle.api.DefaultTask"))) + .and(not(declaredIn("org.gradle.api.internal.AbstractTask"))) + .as("task property getters"); + + /** + * Inspired by + * {@link Gradle} + */ + static final ArchRule RULE = ArchRuleDefinition.priority(Priority.MEDIUM) + .methods().that(are(richTaskPropertyGetters)) + .should().haveModifier(JavaModifier.ABSTRACT) + .allowEmptyShould(true) + .because("task implementations should define properties as abstract getters"); +} diff --git a/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/GradleTaskInputOutputRule.java b/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/TaskHasInputOutputRule.java similarity index 66% rename from archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/GradleTaskInputOutputRule.java rename to archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/TaskHasInputOutputRule.java index 2b1aa4f..8bee285 100644 --- a/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/GradleTaskInputOutputRule.java +++ b/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/TaskHasInputOutputRule.java @@ -1,42 +1,29 @@ package com.netflix.nebula.archrules.gradleplugins; -import com.tngtech.archunit.base.DescribedPredicate; -import com.tngtech.archunit.core.domain.properties.CanBeAnnotated; import com.tngtech.archunit.lang.ArchRule; import com.tngtech.archunit.lang.Priority; import com.tngtech.archunit.lang.syntax.ArchRuleDefinition; import org.jspecify.annotations.NullMarked; -import static com.netflix.nebula.archrules.gradleplugins.Predicates.annotatedWithAny; +import static com.netflix.nebula.archrules.gradleplugins.Predicates.aGradleTaskClass; +import static com.netflix.nebula.archrules.gradleplugins.Predicates.annotatedWithInputOutputAnnotations; import static com.netflix.nebula.archrules.gradleplugins.Predicates.containAnyFieldsInClassHierarchyThat; import static com.netflix.nebula.archrules.gradleplugins.Predicates.containAnyMethodsInClassHierarchyThat; import static com.netflix.nebula.archrules.gradleplugins.Predicates.haveTaskAction; -import static com.netflix.nebula.archrules.gradleplugins.TypeConstants.INPUT_OUTPUT_ANNOTATIONS; import static com.tngtech.archunit.lang.ArchCondition.from; import static com.tngtech.archunit.lang.conditions.ArchPredicates.are; -/** - * Rules to ensure Gradle tasks properly declare their inputs and outputs. - *

- * Tasks must declare inputs and outputs for incremental builds and caching to work correctly. - */ @NullMarked -public class GradleTaskInputOutputRule { - - private static final DescribedPredicate annotatedWithInputOutputAnnotations = - annotatedWithAny(INPUT_OUTPUT_ANNOTATIONS) - .as("annotated with Input and/or Output annotations"); - +public class TaskHasInputOutputRule { /** * Ensures that task classes declare at least one input or output. *

* Tasks without declared inputs/outputs cannot participate in incremental builds * or build caching, which significantly impacts build performance. */ - public static final ArchRule INPUTS_OUTPUTS = ArchRuleDefinition.priority(Priority.HIGH) + static final ArchRule RULE = ArchRuleDefinition.priority(Priority.HIGH) .classes() - .that().areAssignableTo("org.gradle.api.DefaultTask") - .and().areNotInterfaces() + .that(are(aGradleTaskClass())) .and(haveTaskAction) .and().doNotHaveSimpleName("DefaultTask") .should(from(containAnyMethodsInClassHierarchyThat(are(annotatedWithInputOutputAnnotations)))) diff --git a/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/TaskInputOutputFieldRule.java b/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/TaskInputOutputFieldRule.java new file mode 100644 index 0000000..03341d2 --- /dev/null +++ b/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/TaskInputOutputFieldRule.java @@ -0,0 +1,24 @@ +package com.netflix.nebula.archrules.gradleplugins; + +import com.tngtech.archunit.lang.ArchRule; +import com.tngtech.archunit.lang.Priority; +import com.tngtech.archunit.lang.syntax.ArchRuleDefinition; +import org.jspecify.annotations.NullMarked; + +import static com.netflix.nebula.archrules.gradleplugins.Predicates.aGradleTaskClass; +import static com.netflix.nebula.archrules.gradleplugins.Predicates.annotatedWithInputOutputAnnotations; +import static com.tngtech.archunit.lang.conditions.ArchConditions.be; +import static com.tngtech.archunit.lang.conditions.ArchPredicates.are; + +@NullMarked +public class TaskInputOutputFieldRule { + /** + * Detects task input/output properties that are fields instead of abstract getter methods + */ + static final ArchRule RULE = ArchRuleDefinition.priority(Priority.MEDIUM) + .noFields() + .that().areDeclaredInClassesThat(are(aGradleTaskClass())) + .should(be(annotatedWithInputOutputAnnotations)) + .allowEmptyShould(true) + .because("Task input/output properties should be declared as abstract getter methods"); +} diff --git a/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/TaskRegularFilePropertyRule.java b/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/TaskRegularFilePropertyRule.java new file mode 100644 index 0000000..2be093f --- /dev/null +++ b/archrules-gradle-plugin-development/src/archRules/java/com/netflix/nebula/archrules/gradleplugins/TaskRegularFilePropertyRule.java @@ -0,0 +1,26 @@ +package com.netflix.nebula.archrules.gradleplugins; + +import com.tngtech.archunit.lang.ArchRule; +import com.tngtech.archunit.lang.Priority; +import com.tngtech.archunit.lang.syntax.ArchRuleDefinition; +import org.jspecify.annotations.NullMarked; + +import static com.netflix.nebula.archrules.common.CanBeAnnotated.Predicates.annotatedWithAny; +import static com.netflix.nebula.archrules.common.JavaMethod.Predicates.aGetter; +import static com.netflix.nebula.archrules.gradleplugins.Predicates.aGradleTaskClass; +import static com.tngtech.archunit.lang.conditions.ArchPredicates.are; + +@NullMarked +public class TaskRegularFilePropertyRule { + + static final ArchRule RULE = ArchRuleDefinition.priority(Priority.MEDIUM) + .methods() + .that().areDeclaredInClassesThat(are(aGradleTaskClass())) + .and(are(aGetter())) + .and(are(annotatedWithAny("org.gradle.api.tasks.InputFile", "org.gradle.api.tasks.OutputFile"))) + .should().haveRawReturnType("org.gradle.api.file.RegularFileProperty") + .allowEmptyShould(true) + .because("Single file inputs and outputs should use RegularFileProperty " + + "for better API cohesion and to prevent misuse"); + +} diff --git a/archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/GradleTaskProviderApiRuleTest.java b/archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/GradleTaskRulesTest.java similarity index 53% rename from archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/GradleTaskProviderApiRuleTest.java rename to archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/GradleTaskRulesTest.java index 8cb0664..4bcc076 100644 --- a/archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/GradleTaskProviderApiRuleTest.java +++ b/archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/GradleTaskRulesTest.java @@ -1,14 +1,11 @@ package com.netflix.nebula.archrules.gradleplugins; import com.netflix.nebula.archrules.core.Runner; -import com.tngtech.archunit.core.importer.ClassFileImporter; import com.tngtech.archunit.lang.EvaluationResult; import org.gradle.api.DefaultTask; import org.gradle.api.file.DirectoryProperty; import org.gradle.api.file.RegularFileProperty; -import org.gradle.api.internal.provider.DefaultProvider; import org.gradle.api.provider.Property; -import org.gradle.api.provider.Provider; import org.gradle.api.tasks.Input; import org.gradle.api.tasks.InputFile; import org.gradle.api.tasks.OutputDirectory; @@ -20,38 +17,15 @@ import java.io.File; -import static com.netflix.nebula.archrules.gradleplugins.GradleTaskProviderApiRule.richTaskPropertyGetters; import static org.assertj.core.api.Assertions.assertThat; -public class GradleTaskProviderApiRuleTest { - private static final Logger LOG = LoggerFactory.getLogger(GradleTaskProviderApiRuleTest.class); - - @Test - public void taskWithPlainStringInput_should_fail() { - final EvaluationResult result = Runner.check( - GradleTaskProviderApiRule.PROVIDER_PROPERTIES, - TaskWithPlainStringInput.class - ); - LOG.info(result.getFailureReport().toString()); - assertThat(result.hasViolation()).isTrue(); - assertThat(result.getFailureReport().toString()).contains("Use Property"); - } - - @Test - public void taskWithPlainFileInputField_should_fail() { - final EvaluationResult result = Runner.check( - GradleTaskProviderApiRule.PROVIDER_PROPERTIES, - TaskWithPlainFileInputField.class - ); - LOG.info(result.getFailureReport().toString()); - assertThat(result.hasViolation()).isTrue(); - assertThat(result.getFailureReport().toString()).contains("Use RegularFileProperty"); - } +public class GradleTaskRulesTest { + private static final Logger LOG = LoggerFactory.getLogger(GradleTaskRulesTest.class); @Test public void taskWithPlainFileOutputGetter_should_fail() { final EvaluationResult result = Runner.check( - GradleTaskProviderApiRule.PROVIDER_PROPERTIES, + GradleTaskRules.PROVIDER_PROPERTIES, TaskWithPlainFileOutputGetter.class ); LOG.info(result.getFailureReport().toString()); @@ -62,7 +36,7 @@ public void taskWithPlainFileOutputGetter_should_fail() { @Test public void taskWithPropertyApiInput_should_pass() { final EvaluationResult result = Runner.check( - GradleTaskProviderApiRule.PROVIDER_PROPERTIES, + GradleTaskRules.PROVIDER_PROPERTIES, TaskWithPropertyApiInput.class ); LOG.info(result.getFailureReport().toString()); @@ -72,7 +46,7 @@ public void taskWithPropertyApiInput_should_pass() { @Test public void taskWithRegularFileProperty_should_pass() { final EvaluationResult result = Runner.check( - GradleTaskProviderApiRule.PROVIDER_PROPERTIES, + GradleTaskRules.PROVIDER_PROPERTIES, TaskWithRegularFileProperty.class ); LOG.info(result.getFailureReport().toString()); @@ -82,7 +56,7 @@ public void taskWithRegularFileProperty_should_pass() { @Test public void taskWithDirectoryProperty_should_pass() { final EvaluationResult result = Runner.check( - GradleTaskProviderApiRule.PROVIDER_PROPERTIES, + GradleTaskRules.PROVIDER_PROPERTIES, TaskWithDirectoryProperty.class ); LOG.info(result.getFailureReport().toString()); @@ -92,85 +66,13 @@ public void taskWithDirectoryProperty_should_pass() { @Test public void taskWithoutAnnotations_should_pass() { final EvaluationResult result = Runner.check( - GradleTaskProviderApiRule.PROVIDER_PROPERTIES, + GradleTaskRules.PROVIDER_PROPERTIES, TaskWithoutAnnotations.class ); LOG.info(result.getFailureReport().toString()); assertThat(result.hasViolation()).isFalse(); } - @Test - public void test_richTaskPropertyGetters() { - boolean result = richTaskPropertyGetters - .test(new ClassFileImporter().importClass(TaskWithAbstractGetter.class).getMethod("getMessage")); - assertThat(result).isTrue(); - } - - @Test - public void test_abstractGetters_fail() { - final EvaluationResult result = Runner.check( - GradleTaskProviderApiRule.ABSTRACT_GETTERS, - TaskWithConcreteGetter.class - ); - assertThat(result.hasViolation()).isTrue(); - } - - @Test - public void test_abstractGetters_pass() { - final EvaluationResult result = Runner.check( - GradleTaskProviderApiRule.ABSTRACT_GETTERS, - TaskWithAbstractGetter.class - ); - assertThat(result.hasViolation()) - .as(result.getFailureReport().toString()) - .isFalse(); - } - - public static abstract class TaskWithAbstractGetter extends DefaultTask { - - @Input - public abstract Provider getMessage(); - - @TaskAction - public void execute() { - System.out.println(" "); - } - } - - public static abstract class TaskWithConcreteGetter extends DefaultTask { - public String message; - - @Input - public Provider getMessage() { - return new DefaultProvider<>(() -> message); - } - - @TaskAction - public void execute() { - System.out.println(message); - } - } - - public static abstract class TaskWithPlainStringInput extends DefaultTask { - @Input - public String message; - - @TaskAction - public void execute() { - System.out.println(message); - } - } - - public static abstract class TaskWithPlainFileInputField extends DefaultTask { - @InputFile - public File inputFile; - - @TaskAction - public void execute() { - System.out.println("Processing: " + inputFile); - } - } - @SuppressWarnings("unused") public static abstract class TaskWithPlainFileOutputGetter extends DefaultTask { private File outputFile; diff --git a/archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/PredicatesTest.java b/archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/PredicatesTest.java index 2c71861..269e184 100644 --- a/archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/PredicatesTest.java +++ b/archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/PredicatesTest.java @@ -6,6 +6,7 @@ import com.tngtech.archunit.core.importer.ClassFileImporter; import org.gradle.FakeDeprecatedGradleClass; import org.gradle.FakeDeprecatedGradleMethod; +import org.gradle.api.DefaultTask; import org.gradle.api.Project; import org.gradle.api.file.ConfigurableFileCollection; import org.gradle.api.file.DirectoryProperty; @@ -28,8 +29,15 @@ import static org.assertj.core.api.Assertions.assertThat; class PredicatesTest { + + private static final ClassFileImporter importer = new ClassFileImporter(); + private static JavaClass scan(Class clazz) { - return new ClassFileImporter().importClass(clazz); + return importer.importClass(clazz); + } + + private static JavaClasses scan(Class... classes) { + return importer.importClasses(classes); } @Test @@ -88,7 +96,7 @@ public void test_isProviderApiType_shouldReturnFalse(Class type) { @Test public void test_callsMethodOn_shouldMatchCorrectMethodCall() { - JavaClasses classes = new ClassFileImporter().importClasses(ClassCallingGetObjects.class, Project.class); + JavaClasses classes = scan(ClassCallingGetObjects.class, Project.class); Set> accesses = classes.get(ClassCallingGetObjects.class).getAccessesFromSelf(); long matchCount = accesses.stream() @@ -100,7 +108,7 @@ public void test_callsMethodOn_shouldMatchCorrectMethodCall() { @Test public void test_callsMethodOn_shouldNotMatchDifferentMethod() { - JavaClasses classes = new ClassFileImporter().importClasses(ClassCallingGetObjects.class, Project.class); + JavaClasses classes = scan(ClassCallingGetObjects.class, Project.class); Set> accesses = classes.get(ClassCallingGetObjects.class).getAccessesFromSelf(); long matchCount = accesses.stream() @@ -112,7 +120,7 @@ public void test_callsMethodOn_shouldNotMatchDifferentMethod() { @Test public void test_callsMethodOnAny_shouldMatchCallOnFirstOwner() { - JavaClasses classes = new ClassFileImporter().importClasses(ClassCallingGetByName.class, Project.class, TaskContainer.class); + JavaClasses classes = scan(ClassCallingGetByName.class, Project.class, TaskContainer.class); Set> accesses = classes.get(ClassCallingGetByName.class).getAccessesFromSelf(); long matchCount = accesses.stream() @@ -124,7 +132,7 @@ public void test_callsMethodOnAny_shouldMatchCallOnFirstOwner() { @Test public void test_callsMethodOnAny_shouldNotMatchDifferentOwner() { - JavaClasses classes = new ClassFileImporter().importClasses(ClassCallingGetByName.class, Project.class, TaskContainer.class); + JavaClasses classes = scan(ClassCallingGetByName.class, Project.class, TaskContainer.class); Set> accesses = classes.get(ClassCallingGetByName.class).getAccessesFromSelf(); long matchCount = accesses.stream() @@ -134,6 +142,14 @@ public void test_callsMethodOnAny_shouldNotMatchDifferentOwner() { assertThat(matchCount).isEqualTo(0); } + @Test + public void test_aGradleTaskClass() { + assertThat(Predicates.aGradleTaskClass().test(scan(CustomTask.class))).isTrue(); + } + + static class CustomTask extends DefaultTask { + } + @SuppressWarnings("unused") static class ClassCallingGetObjects { public void method(Project project) { diff --git a/archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/TaskAbstractGetterRuleTest.java b/archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/TaskAbstractGetterRuleTest.java new file mode 100644 index 0000000..972d8ef --- /dev/null +++ b/archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/TaskAbstractGetterRuleTest.java @@ -0,0 +1,63 @@ +package com.netflix.nebula.archrules.gradleplugins; + +import com.netflix.nebula.archrules.core.Runner; +import com.tngtech.archunit.lang.EvaluationResult; +import org.gradle.api.DefaultTask; +import org.gradle.api.internal.provider.DefaultProvider; +import org.gradle.api.provider.Provider; +import org.gradle.api.tasks.Input; +import org.gradle.api.tasks.TaskAction; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +class TaskAbstractGetterRuleTest { + + @Test + public void test_abstractGetters_fail() { + final EvaluationResult result = Runner.check( + TaskAbstractGetterRule.RULE, + TaskWithConcreteGetter.class + ); + assertThat(result.hasViolation()).isTrue(); + assertThat(result.getFailureReport().toString()) + .contains("methods that are task property getters should have modifier ABSTRACT") + .contains("because task implementations should define properties as abstract getters"); + } + + @Test + public void test_abstractGetters_pass() { + final EvaluationResult result = Runner.check( + TaskAbstractGetterRule.RULE, + TaskWithAbstractGetter.class + ); + assertThat(result.hasViolation()) + .as(result.getFailureReport().toString()) + .isFalse(); + } + + public static abstract class TaskWithAbstractGetter extends DefaultTask { + + @Input + public abstract Provider getMessage(); + + @TaskAction + public void execute() { + System.out.println(" "); + } + } + + public static abstract class TaskWithConcreteGetter extends DefaultTask { + public String message; + + @Input + public Provider getMessage() { + return new DefaultProvider<>(() -> message); + } + + @TaskAction + public void execute() { + System.out.println(message); + } + } +} diff --git a/archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/GradleTaskInputOutputRuleTest.java b/archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/TaskHasInputOutputRuleTest.java similarity index 81% rename from archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/GradleTaskInputOutputRuleTest.java rename to archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/TaskHasInputOutputRuleTest.java index b77c375..c43b40a 100644 --- a/archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/GradleTaskInputOutputRuleTest.java +++ b/archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/TaskHasInputOutputRuleTest.java @@ -8,20 +8,16 @@ import org.gradle.api.tasks.OutputFile; import org.gradle.api.tasks.TaskAction; import org.junit.jupiter.api.Test; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.io.File; import static org.assertj.core.api.Assertions.assertThat; -public class GradleTaskInputOutputRuleTest { - private static final Logger LOG = LoggerFactory.getLogger(GradleTaskInputOutputRuleTest.class); - +class TaskHasInputOutputRuleTest { @Test public void taskWithoutInputOutput_should_fail() { final EvaluationResult result = Runner.check( - GradleTaskInputOutputRule.INPUTS_OUTPUTS, + TaskHasInputOutputRule.RULE, TaskWithoutInputOutput.class ); assertThat(result.hasViolation()).isTrue(); @@ -33,40 +29,36 @@ public void taskWithoutInputOutput_should_fail() { @Test public void taskWithInputAnnotation_should_pass() { final EvaluationResult result = Runner.check( - GradleTaskInputOutputRule.INPUTS_OUTPUTS, + TaskHasInputOutputRule.RULE, TaskWithInputAnnotation.class ); - LOG.info(result.getFailureReport().toString()); assertThat(result.hasViolation()).isFalse(); } @Test public void taskWithInputFileAnnotation_should_pass() { final EvaluationResult result = Runner.check( - GradleTaskInputOutputRule.INPUTS_OUTPUTS, + TaskHasInputOutputRule.RULE, TaskWithInputFileAnnotation.class ); - LOG.info(result.getFailureReport().toString()); assertThat(result.hasViolation()).isFalse(); } @Test public void taskWithOutputAnnotation_should_pass() { final EvaluationResult result = Runner.check( - GradleTaskInputOutputRule.INPUTS_OUTPUTS, + TaskHasInputOutputRule.RULE, TaskWithOutputAnnotation.class ); - LOG.info(result.getFailureReport().toString()); assertThat(result.hasViolation()).isFalse(); } @Test public void taskWithoutTaskAction_should_pass() { final EvaluationResult result = Runner.check( - GradleTaskInputOutputRule.INPUTS_OUTPUTS, + TaskHasInputOutputRule.RULE, TaskWithoutTaskAction.class ); - LOG.info(result.getFailureReport().toString()); assertThat(result.hasViolation()).isFalse(); } diff --git a/archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/TaskInputOutputFieldRuleTest.java b/archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/TaskInputOutputFieldRuleTest.java new file mode 100644 index 0000000..71b2883 --- /dev/null +++ b/archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/TaskInputOutputFieldRuleTest.java @@ -0,0 +1,61 @@ +package com.netflix.nebula.archrules.gradleplugins; + +import com.netflix.nebula.archrules.core.Runner; +import com.tngtech.archunit.lang.EvaluationResult; +import org.gradle.api.DefaultTask; +import org.gradle.api.tasks.Input; +import org.gradle.api.tasks.InputFile; +import org.gradle.api.tasks.TaskAction; +import org.junit.jupiter.api.Test; + +import java.io.File; + +import static org.assertj.core.api.Assertions.assertThat; + +class TaskInputOutputFieldRuleTest { + @Test + public void test_input() { + final EvaluationResult result = Runner.check( + TaskInputOutputFieldRule.RULE, + TaskWithInputField.class + ); + assertThat(result.hasViolation()).isTrue(); + assertThat(result.getFailureReport().toString()) + .contains("no fields that are declared in classes that are a gradle task") + .contains("should be annotated with Input and/or Output annotations, ") + .contains("because Task input/output properties should be declared as abstract getter methods"); + } + + @Test + public void test_input_file() { + final EvaluationResult result = Runner.check( + TaskInputOutputFieldRule.RULE, + TaskWithFileInputField.class + ); + assertThat(result.hasViolation()).isTrue(); + assertThat(result.getFailureReport().toString()) + .contains("no fields that are declared in classes that are a gradle task") + .contains("should be annotated with Input and/or Output annotations, ") + .contains("because Task input/output properties should be declared as abstract getter methods"); + } + + static abstract class TaskWithInputField extends DefaultTask { + @Input + public String message; + + @TaskAction + public void execute() { + System.out.println(message); + } + } + + public static abstract class TaskWithFileInputField extends DefaultTask { + @InputFile + public File inputFile; + + @TaskAction + public void execute() { + System.out.println("Processing: " + inputFile); + } + } +} diff --git a/archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/TaskRegularFilePropertyRuleTest.java b/archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/TaskRegularFilePropertyRuleTest.java new file mode 100644 index 0000000..a149e0e --- /dev/null +++ b/archrules-gradle-plugin-development/src/archRulesTest/java/com/netflix/nebula/archrules/gradleplugins/TaskRegularFilePropertyRuleTest.java @@ -0,0 +1,43 @@ +package com.netflix.nebula.archrules.gradleplugins; + +import com.netflix.nebula.archrules.core.Runner; +import com.tngtech.archunit.lang.EvaluationResult; +import org.gradle.api.DefaultTask; +import org.gradle.api.provider.Property; +import org.gradle.api.tasks.OutputFile; +import org.gradle.api.tasks.TaskAction; +import org.junit.jupiter.api.Test; + +import java.io.File; + +import static org.assertj.core.api.Assertions.assertThat; + +class TaskRegularFilePropertyRuleTest { + + @Test + public void test_nonregular_file() { + final EvaluationResult result = Runner.check( + TaskRegularFilePropertyRule.RULE, + TaskWithFileProperty.class + ); + assertThat(result.hasViolation()).isTrue(); + assertThat(result.getFailureReport().toString()) + .contains("methods that are declared in classes that are a gradle task") + .contains("and are a getter") + .contains("and are annotated with any [org.gradle.api.tasks.InputFile, org.gradle.api.tasks.OutputFile]") + .contains("should have raw return type org.gradle.api.file.RegularFileProperty") + .contains("because Single file inputs and outputs should use RegularFileProperty") + .contains("for better API cohesion and to prevent misuse"); + } + + static abstract class TaskWithFileProperty extends DefaultTask { + @OutputFile + public abstract Property getOutputFile(); + + @TaskAction + public void execute() { + System.out.println("Writing to: " + getOutputFile().get()); + } + } + +}