Merge pull request gradle#10858 from gradle/lptr/validation/show-fail…

…ed-task-type-in-exception

Display nice type names consistently for validation warnings

subprojects/core/src/integTest/groovy/org/gradle/api/tasks/IncrementalBuildSymlinkHandlingIntegrationTest.groovy

@@ -130,7 +130,7 @@ task work {
 
         expect:
         fails("work")
-        failure.assertHasDescription("A problem was found with the configuration of task ':work'.")
+        failure.assertHasDescription("A problem was found with the configuration of task ':work' (type 'DefaultTask').")
         failure.assertHasCause("File '$link' specified for property '\$1' does not exist.")
     }
 

subprojects/core/src/integTest/groovy/org/gradle/api/tasks/NestedInputIntegrationTest.groovy

@@ -468,7 +468,7 @@ class NestedInputIntegrationTest extends AbstractIntegrationSpec {
 
         expect:
         fails "myTask"
-        failure.assertHasDescription("A problem was found with the configuration of task ':myTask'.")
+        failure.assertHasDescription("A problem was found with the configuration of task ':myTask' (type 'TaskWithAbsentNestedInput').")
         failure.assertHasCause("No value has been specified for property 'nested'.")
     }
 

subprojects/core/src/integTest/groovy/org/gradle/api/tasks/TaskInputFilePropertiesIntegrationTest.groovy

@@ -75,7 +75,7 @@ class TaskInputFilePropertiesIntegrationTest extends AbstractIntegrationSpec {
 
         expect:
         fails "test"
-        failure.assertHasDescription("A problem was found with the configuration of task ':test'.")
+        failure.assertHasDescription("A problem was found with the configuration of task ':test' (type 'DefaultTask').")
         failure.assertHasCause("Value 'task ':dependencyTask'' specified for property 'input' cannot be converted to a ${targetType}.")
 
         where:
@@ -108,7 +108,7 @@ class TaskInputFilePropertiesIntegrationTest extends AbstractIntegrationSpec {
 
         expect:
         fails "customTask"
-        failure.assertHasDescription("A problem was found with the configuration of task ':customTask'.")
+        failure.assertHasDescription("A problem was found with the configuration of task ':customTask' (type 'CustomTask').")
         failure.assertHasCause("Value 'task ':dependencyTask'' specified for property 'input' cannot be converted to a ${targetType}.")
 
         where:

subprojects/core/src/integTest/groovy/org/gradle/api/tasks/TaskParametersIntegrationTest.groovy

@@ -470,7 +470,7 @@ task someTask(type: SomeTask) {
         """
         expect:
         fails "test"
-        failure.assertHasDescription("A problem was found with the configuration of task ':test'.")
+        failure.assertHasDescription("A problem was found with the configuration of task ':test' (type 'DefaultTask').")
         failure.assertHasCause("No value has been specified for property 'input'.")
     }
 
@@ -495,7 +495,7 @@ task someTask(type: SomeTask) {
         """
         expect:
         fails "test"
-        failure.assertHasDescription("A problem was found with the configuration of task ':test'.")
+        failure.assertHasDescription("A problem was found with the configuration of task ':test' (type 'DefaultTask').")
         failure.assertHasCause("No value has been specified for property 'input'.")
 
         where:
@@ -527,7 +527,7 @@ task someTask(type: SomeTask) {
         """
         expect:
         fails "test"
-        failure.assertHasDescription("A problem was found with the configuration of task ':test'.")
+        failure.assertHasDescription("A problem was found with the configuration of task ':test' (type 'DefaultTask').")
         failure.assertHasCause("No value has been specified for property 'output'.")
 
         where:
@@ -560,7 +560,7 @@ task someTask(type: SomeTask) {
 
         expect:
         fails "test"
-        failure.assertHasDescription("A problem was found with the configuration of task ':test'.")
+        failure.assertHasDescription("A problem was found with the configuration of task ':test' (type 'DefaultTask').")
         failure.assertHasCause("$type '${file("missing")}' specified for property 'input' does not exist.")
 
         where:
@@ -582,7 +582,7 @@ task someTask(type: SomeTask) {
 
         expect:
         fails "test"
-        failure.assertHasDescription("A problem was found with the configuration of task ':test'.")
+        failure.assertHasDescription("A problem was found with the configuration of task ':test' (type 'DefaultTask').")
         failure.assertHasCause("${type.capitalize()} '${file(path)}' specified for property 'input' is not a $type.")
 
         where:

subprojects/core/src/test/groovy/org/gradle/api/internal/tasks/properties/DefaultTypeMetadataStoreTest.groovy

@@ -166,7 +166,7 @@ class DefaultTypeMetadataStoreTest extends Specification {
         def typeAnnotationHandler = Stub(TypeAnnotationHandler)
         _ * typeAnnotationHandler.annotationType >> CustomCacheable
         _ * typeAnnotationHandler.validateTypeMetadata(_, _) >> { Class type, TypeValidationContext context ->
-            context.visitTypeProblem(WARNING, Object, "type is broken")
+            context.visitTypeProblem(WARNING, type, "type is broken")
         }
 
         def metadataStore = new DefaultTypeMetadataStore([typeAnnotationHandler], [], [], typeAnnotationMetadataStore, cacheFactory)
@@ -181,7 +181,7 @@ class DefaultTypeMetadataStoreTest extends Specification {
         def typeMetadata = metadataStore.getTypeMetadata(TypeWithCustomAnnotation)
 
         then:
-        collectProblems(typeMetadata) == ["Type '$Object.name': type is broken." as String]
+        collectProblems(typeMetadata) == ["Type 'DefaultTypeMetadataStoreTest.TypeWithCustomAnnotation': type is broken." as String]
     }
 
     @Unroll

subprojects/core/src/test/groovy/org/gradle/api/internal/tasks/properties/PropertyValidationAccessTest.groovy

@@ -30,6 +30,7 @@ import org.gradle.api.tasks.Internal
 import org.gradle.api.tasks.Nested
 import org.gradle.api.tasks.Optional
 import org.gradle.api.tasks.OutputFile
+import org.gradle.model.internal.type.ModelType
 import spock.lang.Specification
 
 import java.nio.file.Path
@@ -257,6 +258,6 @@ class PropertyValidationAccessTest extends Specification {
     }
 
     private static Set<String> validationProblems(Class<?> task, List messages) {
-        messages.collect { "Type '${task.name}': ${it}." }*.toString() as Set
+        messages.collect { "Type '${ModelType.of(task).displayName}': ${it}." }*.toString() as Set
     }
 }

subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/transform/ArtifactTransformCachingIntegrationTest.groovy

@@ -91,7 +91,7 @@ class ArtifactTransformCachingIntegrationTest extends AbstractHttpDependencyReso
         fails "badTask", "--continue"
         then:
         ['lib', 'app', 'util'].each {
-            failure.assertHasDescription("A problem was found with the configuration of task ':${it}:badTask'.")
+            failure.assertHasDescription("A problem was found with the configuration of task ':${it}:badTask' (type 'DefaultTask').")
             failure.assertHasCause("The output ${file("${it}/build/${forbiddenPath}")} must not be in a reserved location.")
         }
     }

subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/transform/ArtifactTransformValuesInjectionIntegrationTest.groovy

@@ -326,8 +326,8 @@ class ArtifactTransformValuesInjectionIntegrationTest extends AbstractDependency
         then:
         failure.assertThatDescription(matchesRegexp('Cannot isolate parameters MakeGreen\\$Parameters_Decorated@.* of artifact transform MakeGreen'))
         failure.assertHasCause('Some problems were found with the configuration of the artifact transform parameter MakeGreen.Parameters.')
-        failure.assertHasCause("Type 'MakeGreen\$Parameters': Cannot use @CacheableTask on type. This annotation can only be used with Task types.")
-        failure.assertHasCause("Type 'MakeGreen\$Parameters': Cannot use @CacheableTransform on type. This annotation can only be used with TransformAction types.")
+        failure.assertHasCause("Type 'MakeGreen.Parameters': Cannot use @CacheableTask on type. This annotation can only be used with Task types.")
+        failure.assertHasCause("Type 'MakeGreen.Parameters': Cannot use @CacheableTransform on type. This annotation can only be used with TransformAction types.")
     }
 
     @Unroll
@@ -820,7 +820,7 @@ abstract class MakeGreen implements TransformAction<TransformParameters.None> {
 
         expect:
         fails('broken')
-        failure.assertHasDescription("A problem was found with the configuration of task ':broken'.")
+        failure.assertHasDescription("A problem was found with the configuration of task ':broken' (type 'MyTask').")
         failure.assertHasCause("Type 'MyTask': Cannot use @CacheableTransform on type. This annotation can only be used with TransformAction types.")
     }
 
@@ -844,7 +844,7 @@ abstract class MakeGreen implements TransformAction<TransformParameters.None> {
         expect:
         // Probably should be eager
         fails('broken')
-        failure.assertHasDescription("Some problems were found with the configuration of task ':broken'.")
+        failure.assertHasDescription("Some problems were found with the configuration of task ':broken' (type 'MyTask').")
         failure.assertHasCause("Type 'Options': Cannot use @CacheableTask on type. This annotation can only be used with Task types.")
         failure.assertHasCause("Type 'Options': Cannot use @CacheableTransform on type. This annotation can only be used with TransformAction types.")
     }

subprojects/execution/src/integTest/groovy/org/gradle/internal/execution/IncrementalExecutionIntegrationTest.groovy

@@ -571,7 +571,7 @@ class IncrementalExecutionIntegrationTest extends Specification {
 
         then:
         def ex = thrown WorkValidationException
-        ex.causes*.message as List == ["Type 'java.lang.Object': Validation error."]
+        ex.causes*.message as List == ["Type '$Object.simpleName': Validation error."]
     }
 
     List<String> inputFilesRemoved(Map<String, List<File>> removedFiles) {

subprojects/execution/src/main/java/org/gradle/internal/execution/steps/ValidateStep.java

@@ -17,6 +17,7 @@
 package org.gradle.internal.execution.steps;
 
 import com.google.common.collect.ImmutableCollection;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMultimap;
 import org.gradle.api.InvalidUserDataException;
 import org.gradle.internal.execution.Context;
@@ -27,8 +28,8 @@
 import org.gradle.internal.reflect.MessageFormattingTypeValidationContext;
 import org.gradle.internal.reflect.TypeValidationContext;
 import org.gradle.internal.reflect.TypeValidationContext.Severity;
+import org.gradle.model.internal.type.ModelType;
 
-import java.util.List;
 import java.util.stream.Collectors;
 
 public class ValidateStep<C extends Context, R extends Result> implements Step<C, R> {
@@ -55,29 +56,45 @@ public R execute(C context) {
         warnings.forEach(warningReporter::reportValidationWarning);
 
         if (!errors.isEmpty()) {
-            String displayName = context.getWork().getDisplayName();
-            String errorMessage = errors.size() == 1
-                ? String.format("A problem was found with the configuration of %s.", displayName)
-                : String.format("Some problems were found with the configuration of %s.", displayName);
-            List<InvalidUserDataException> causes = errors.stream()
-                .limit(5)
-                .sorted()
-                .map(InvalidUserDataException::new)
-                .collect(Collectors.toList());
-            throw new WorkValidationException(errorMessage, causes);
+            throw new WorkValidationException(
+                String.format("%s found with the configuration of %s (%s).",
+                    errors.size() == 1
+                        ? "A problem was"
+                        : "Some problems were",
+                    context.getWork().getDisplayName(),
+                    describeTypesChecked(validationContext.getTypes())
+                ),
+                errors.stream()
+                    .limit(5)
+                    .sorted()
+                    .map(InvalidUserDataException::new)
+                    .collect(Collectors.toList())
+            );
         }
         return delegate.execute(context);
     }
 
+    private static String describeTypesChecked(ImmutableList<Class<?>> types) {
+        return types.size() == 1
+            ? "type '" + getTypeDisplayName(types.get(0)) + "'"
+            : "types '" + types.stream().map(ValidateStep::getTypeDisplayName).collect(Collectors.joining("', '")) + "'";
+    }
+
+    private static String getTypeDisplayName(Class<?> type) {
+        return ModelType.of(type).getDisplayName();
+    }
+
     public interface ValidationWarningReporter {
         void reportValidationWarning(String warning);
     }
 
     private static class DefaultWorkValidationContext implements UnitOfWork.WorkValidationContext {
         private final ImmutableMultimap.Builder<Severity, String> problems = ImmutableMultimap.builder();
+        private final ImmutableList.Builder<Class<?>> types = ImmutableList.builder();
 
         @Override
         public TypeValidationContext createContextFor(Class<?> type, boolean cacheable) {
+            types.add(type);
             return new MessageFormattingTypeValidationContext(null) {
                 @Override
                 protected void recordProblem(Severity severity, String message) {
@@ -92,5 +109,9 @@ protected void recordProblem(Severity severity, String message) {
         public ImmutableMultimap<Severity, String> getProblems() {
             return problems.build();
         }
+
+        public ImmutableList<Class<?>> getTypes() {
+            return types.build();
+        }
     }
 }

subprojects/execution/src/test/groovy/org/gradle/internal/execution/steps/ValidateStepTest.groovy

@@ -52,12 +52,12 @@ class ValidateStepTest extends ContextInsensitiveStepSpec {
 
         then:
         def ex = thrown WorkValidationException
-        ex.message == "A problem was found with the configuration of job ':test'."
+        ex.message == "A problem was found with the configuration of job ':test' (type 'ValidateStepTest.JobType')."
         ex.causes.size() == 1
-        ex.causes[0].message == "Type '$Object.name': Validation error."
+        ex.causes[0].message == "Type '$Object.simpleName': Validation error."
 
         _ * work.validate(_ as  WorkValidationContext) >> {  WorkValidationContext validationContext ->
-            validationContext.createContextFor(Object, true).visitTypeProblem(ERROR, Object, "Validation error")
+            validationContext.createContextFor(JobType, true).visitTypeProblem(ERROR, Object, "Validation error")
         }
         0 * _
     }
@@ -68,15 +68,14 @@ class ValidateStepTest extends ContextInsensitiveStepSpec {
 
         then:
         def ex = thrown WorkValidationException
-        ex.message == "Some problems were found with the configuration of job ':test'."
+        ex.message == "Some problems were found with the configuration of job ':test' (types 'ValidateStepTest.JobType', 'ValidateStepTest.SecondaryJobType')."
         ex.causes.size() == 2
-        ex.causes[0].message == "Type '$Object.name': Validation error #1."
-        ex.causes[1].message == "Type '$Object.name': Validation error #2."
+        ex.causes[0].message == "Type '$Object.simpleName': Validation error #1."
+        ex.causes[1].message == "Type '$Object.simpleName': Validation error #2."
 
         _ * work.validate(_ as  WorkValidationContext) >> {  WorkValidationContext validationContext ->
-            def typeContext = validationContext.createContextFor(Object, true)
-            typeContext.visitTypeProblem(ERROR, Object, "Validation error #1")
-            typeContext.visitTypeProblem(ERROR, Object, "Validation error #2")
+            validationContext.createContextFor(JobType, true).visitTypeProblem(ERROR, Object, "Validation error #1")
+            validationContext.createContextFor(SecondaryJobType, true).visitTypeProblem(ERROR, Object, "Validation error #2")
         }
         0 * _
     }
@@ -87,11 +86,11 @@ class ValidateStepTest extends ContextInsensitiveStepSpec {
 
         then:
         _ * work.validate(_ as  WorkValidationContext) >> {  WorkValidationContext validationContext ->
-            validationContext.createContextFor(Object, true).visitTypeProblem(WARNING, Object, "Validation warning")
+            validationContext.createContextFor(JobType, true).visitTypeProblem(WARNING, Object, "Validation warning")
         }
 
         then:
-        1 * warningReporter.reportValidationWarning("Type '$Object.name': Validation warning.")
+        1 * warningReporter.reportValidationWarning("Type '$Object.simpleName': Validation warning.")
 
         then:
         1 * delegate.execute(context)
@@ -104,19 +103,23 @@ class ValidateStepTest extends ContextInsensitiveStepSpec {
 
         then:
         _ * work.validate(_ as  WorkValidationContext) >> {  WorkValidationContext validationContext ->
-            def typeContext = validationContext.createContextFor(Object, true)
+            def typeContext = validationContext.createContextFor(JobType, true)
             typeContext.visitTypeProblem(ERROR, Object, "Validation error")
             typeContext.visitTypeProblem(WARNING, Object, "Validation warning")
         }
 
         then:
-        1 * warningReporter.reportValidationWarning("Type '$Object.name': Validation warning.")
+        1 * warningReporter.reportValidationWarning("Type '$Object.simpleName': Validation warning.")
 
         then:
         def ex = thrown WorkValidationException
-        ex.message == "A problem was found with the configuration of job ':test'."
+        ex.message == "A problem was found with the configuration of job ':test' (type 'ValidateStepTest.JobType')."
         ex.causes.size() == 1
-        ex.causes[0].message == "Type '$Object.name': Validation error."
+        ex.causes[0].message == "Type '$Object.simpleName': Validation error."
         0 * _
     }
+
+    interface JobType {}
+
+    interface SecondaryJobType {}
 }

subprojects/file-collections/src/integTest/groovy/org/gradle/api/file/FilePropertyIntegrationTest.groovy

@@ -816,7 +816,7 @@ class SomeTask extends DefaultTask {
         fails("consumer")
 
         then:
-        failure.assertHasDescription("A problem was found with the configuration of task ':consumer'.")
+        failure.assertHasDescription("A problem was found with the configuration of task ':consumer' (type 'ConsumerTask').")
         failure.assertHasCause("No value has been specified for property 'bean.inputFile'.")
         failure.assertTasksExecuted(':consumer')
     }

subprojects/integ-test/src/integTest/groovy/org/gradle/integtests/TaskErrorExecutionIntegrationTest.groovy

@@ -116,7 +116,7 @@ class TaskErrorExecutionIntegrationTest extends AbstractIntegrationSpec {
         expect:
         fails "custom"
 
-        failure.assertHasDescription("Some problems were found with the configuration of task ':custom'.")
+        failure.assertHasDescription("Some problems were found with the configuration of task ':custom' (type 'CustomTask').")
         failure.assertHasCause("No value has been specified for property 'srcFile'.")
         failure.assertHasCause("No value has been specified for property 'destFile'.")
     }

subprojects/model-core/src/main/java/org/gradle/internal/reflect/MessageFormattingTypeValidationContext.java

@@ -16,6 +16,8 @@
 
 package org.gradle.internal.reflect;
 
+import org.gradle.model.internal.type.ModelType;
+
 import javax.annotation.Nullable;
 
 abstract public class MessageFormattingTypeValidationContext implements TypeValidationContext {
@@ -30,7 +32,7 @@ public void visitTypeProblem(Severity severity, Class<?> type, String message) {
         @SuppressWarnings("StringBufferReplaceableByString")
         StringBuilder builder = new StringBuilder();
         builder.append("Type '");
-        builder.append(type.getTypeName());
+        builder.append(ModelType.of(type).getDisplayName());
         builder.append("': ");
         builder.append(message);
         builder.append('.');
@@ -42,7 +44,7 @@ public void visitPropertyProblem(Severity severity, @Nullable String parentPrope
         StringBuilder builder = new StringBuilder();
         if (rootType != null) {
             builder.append("Type '");
-            builder.append(rootType.getTypeName());
+            builder.append(ModelType.of(rootType).getDisplayName());
             builder.append("': ");
         }
         if (property != null) {