#32 Simplifying messaging

processor/src/main/java/org/mapstruct/ap/MappingProcessor.java

@@ -129,10 +129,18 @@ public class MappingProcessor extends AbstractProcessor {
         return ANNOTATIONS_CLAIMED_EXCLUSIVELY;
     }
 
+    /**
+     * Applies all registered {@link ModelElementProcessor}s to the given mapper
+     * type.
+     *
+     * @param context The processor context.
+     * @param mapperTypeElement The mapper type element.
+     */
     private void processMapperTypeElement(ProcessorContext context, TypeElement mapperTypeElement) {
-        Object mapper = mapperTypeElement;
+        Object model = null;
+
         for ( ModelElementProcessor<?, ?> processor : getProcessors() ) {
-            mapper = process( context, processor, mapperTypeElement, mapper );
+            model = process( context, processor, mapperTypeElement, model );
         }
     }
 

processor/src/main/java/org/mapstruct/ap/model/Mapper.java

@@ -34,11 +34,9 @@ public class Mapper extends AbstractModelElement {
     private final List<MappingMethod> mappingMethods;
     private final List<MapperReference> referencedMappers;
     private final Options options;
-    private final boolean isErroneous;
 
     public Mapper(String packageName, String interfaceName, String implementationName,
-                  List<MappingMethod> mappingMethods, List<MapperReference> referencedMappers, Options options,
+                  List<MappingMethod> mappingMethods, List<MapperReference> referencedMappers, Options options) {
-                  boolean isErroneous) {
         this.packageName = packageName;
         this.interfaceName = interfaceName;
         this.implementationName = implementationName;
@@ -46,7 +44,6 @@ public class Mapper extends AbstractModelElement {
         this.mappingMethods = mappingMethods;
         this.referencedMappers = referencedMappers;
         this.options = options;
-        this.isErroneous = isErroneous;
     }
 
     @Override
@@ -132,10 +129,6 @@ public class Mapper extends AbstractModelElement {
         return options;
     }
 
-    public boolean isErroneous() {
-        return isErroneous;
-    }
-
     public void addAnnotation(Annotation annotation) {
         annotations.add( annotation );
     }

processor/src/main/java/org/mapstruct/ap/processor/DefaultModelElementProcessorContext.java

@@ -21,8 +21,12 @@ package org.mapstruct.ap.processor;
 import javax.annotation.processing.Filer;
 import javax.annotation.processing.Messager;
 import javax.annotation.processing.ProcessingEnvironment;
+import javax.lang.model.element.AnnotationMirror;
+import javax.lang.model.element.AnnotationValue;
+import javax.lang.model.element.Element;
 import javax.lang.model.util.Elements;
 import javax.lang.model.util.Types;
+import javax.tools.Diagnostic.Kind;
 
 import org.mapstruct.ap.model.Options;
 
@@ -33,11 +37,13 @@ import org.mapstruct.ap.model.Options;
  */
 public class DefaultModelElementProcessorContext implements ModelElementProcessor.ProcessorContext {
 
-    private ProcessingEnvironment processingEnvironment;
+    private final ProcessingEnvironment processingEnvironment;
-    private Options options;
+    private final DelegatingMessager messager;
+    private final Options options;
 
     public DefaultModelElementProcessorContext(ProcessingEnvironment processingEnvironment, Options options) {
         this.processingEnvironment = processingEnvironment;
+        this.messager = new DelegatingMessager( processingEnvironment.getMessager() );
         this.options = options;
     }
 
@@ -58,11 +64,58 @@ public class DefaultModelElementProcessorContext implements ModelElementProcesso
 
     @Override
     public Messager getMessager() {
-        return processingEnvironment.getMessager();
+        return messager;
     }
 
     @Override
     public Options getOptions() {
         return options;
     }
+
+    @Override
+    public boolean isErroneous() {
+        return messager.isErroneous();
+    }
+
+    private static class DelegatingMessager implements Messager {
+
+        private final Messager delegate;
+        private boolean isErroneous = false;
+
+        public DelegatingMessager(Messager delegate) {
+            this.delegate = delegate;
+        }
+
+        public void printMessage(Kind kind, CharSequence msg) {
+            delegate.printMessage( kind, msg );
+            if ( kind == Kind.ERROR ) {
+                isErroneous = true;
+            }
+        }
+
+        public void printMessage(Kind kind, CharSequence msg, Element e) {
+            delegate.printMessage( kind, msg, e );
+            if ( kind == Kind.ERROR ) {
+                isErroneous = true;
+            }
+        }
+
+        public void printMessage(Kind kind, CharSequence msg, Element e, AnnotationMirror a) {
+            delegate.printMessage( kind, msg, e, a );
+            if ( kind == Kind.ERROR ) {
+                isErroneous = true;
+            }
+        }
+
+        public void printMessage(Kind kind, CharSequence msg, Element e, AnnotationMirror a, AnnotationValue v) {
+            delegate.printMessage( kind, msg, e, a, v );
+            if ( kind == Kind.ERROR ) {
+                isErroneous = true;
+            }
+        }
+
+        public boolean isErroneous() {
+            return isErroneous;
+        }
+    }
 }

processor/src/main/java/org/mapstruct/ap/processor/MapperCreationProcessor.java

@@ -28,14 +28,12 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import javax.annotation.processing.Messager;
-import javax.lang.model.element.AnnotationMirror;
-import javax.lang.model.element.AnnotationValue;
-import javax.lang.model.element.Element;
 import javax.lang.model.element.ExecutableElement;
 import javax.lang.model.element.TypeElement;
 import javax.lang.model.type.TypeMirror;
 import javax.lang.model.util.Elements;
 import javax.lang.model.util.Types;
+import javax.tools.Diagnostic.Kind;
 
 import org.mapstruct.ap.MapperPrism;
 import org.mapstruct.ap.conversion.Conversion;
@@ -77,10 +75,8 @@ public class MapperCreationProcessor implements ModelElementProcessor<List<Metho
     private Conversions conversions;
     private Executables executables;
 
-    private boolean isErroneous = false;
-
     @Override
-    public Mapper process(ProcessorContext context, TypeElement mapperTypeElement, List<Method> sourceElement) {
+    public Mapper process(ProcessorContext context, TypeElement mapperTypeElement, List<Method> sourceModel) {
         this.elementUtils = context.getElementUtils();
         this.typeUtils = context.getTypeUtils();
         this.messager = context.getMessager();
@@ -90,7 +86,7 @@ public class MapperCreationProcessor implements ModelElementProcessor<List<Metho
         this.conversions = new Conversions( elementUtils, typeUtils, typeUtil );
         this.executables = new Executables( typeUtil );
 
-        return getMapper( mapperTypeElement, sourceElement );
+        return getMapper( mapperTypeElement, sourceModel );
     }
 
     @Override
@@ -98,7 +94,6 @@ public class MapperCreationProcessor implements ModelElementProcessor<List<Metho
         return 1000;
     }
 
-
     private Mapper getMapper(TypeElement element, List<Method> methods) {
         ReportingPolicy unmappedTargetPolicy = getEffectiveUnmappedTargetPolicy( element );
         List<MappingMethod> mappingMethods = getMappingMethods( methods, unmappedTargetPolicy );
@@ -110,8 +105,7 @@ public class MapperCreationProcessor implements ModelElementProcessor<List<Metho
             element.getSimpleName() + IMPLEMENTATION_SUFFIX,
             mappingMethods,
             mapperReferences,
-            options,
+            options
-            isErroneous
         );
     }
 
@@ -262,8 +256,8 @@ public class MapperCreationProcessor implements ModelElementProcessor<List<Metho
         if ( targetProperties.size() > mappedTargetProperties.size() &&
             unmappedTargetPolicy.requiresReport() ) {
             targetProperties.removeAll( mappedTargetProperties );
-            printMessage(
+            messager.printMessage(
-                unmappedTargetPolicy,
+                unmappedTargetPolicy.getDiagnosticKind(),
                 MessageFormat.format(
                     "Unmapped target {0,choice,1#property|1<properties}: \"{1}\"",
                     targetProperties.size(),
@@ -287,23 +281,29 @@ public class MapperCreationProcessor implements ModelElementProcessor<List<Metho
                                                         Set<String> targetProperties) {
         for ( Mapping mappedProperty : method.getMappings().values() ) {
             if ( !sourceProperties.contains( mappedProperty.getSourceName() ) ) {
-                printMessage(
+                messager.printMessage(
-                    ReportingPolicy.ERROR,
+                    Kind.ERROR,
                     String.format(
                         "Unknown property \"%s\" in parameter type %s.",
                         mappedProperty.getSourceName(),
                         method.getSourceType()
-                    ), method.getExecutable(), mappedProperty.getMirror(), mappedProperty.getSourceAnnotationValue()
+                    ),
+                    method.getExecutable(),
+                    mappedProperty.getMirror(),
+                    mappedProperty.getSourceAnnotationValue()
                 );
             }
             if ( !targetProperties.contains( mappedProperty.getTargetName() ) ) {
-                printMessage(
+                messager.printMessage(
-                    ReportingPolicy.ERROR,
+                    Kind.ERROR,
                     String.format(
                         "Unknown property \"%s\" in return type %s.",
                         mappedProperty.getTargetName(),
                         method.getTargetType()
-                    ), method.getExecutable(), mappedProperty.getMirror(), mappedProperty.getTargetAnnotationValue()
+                    ),
+                    method.getExecutable(),
+                    mappedProperty.getMirror(),
+                    mappedProperty.getTargetAnnotationValue()
                 );
             }
         }
@@ -397,51 +397,33 @@ public class MapperCreationProcessor implements ModelElementProcessor<List<Metho
         return null;
     }
 
+    /**
+     * Reports an error if source and target type of the property are different
+     * and neither a mapping method nor a conversion exists nor the property is
+     * of a collection type with default implementation
+     *
+     * @param method The mapping method owning the property mapping.
+     * @param property The property mapping to check.
+     */
     private void reportErrorIfPropertyCanNotBeMapped(Method method, PropertyMapping property) {
-        if ( property.getSourceType().equals( property.getTargetType() ) ) {
+        if ( property.getSourceType().equals( property.getTargetType() ) ||
+            property.getMappingMethod() != null ||
+            property.getConversion() != null ||
+            ( property.getTargetType().isCollectionType() &&
+                property.getTargetType().getCollectionImplementationType() != null ) ) {
             return;
         }
 
-        //no mapping method nor conversion nor collection with default implementation
+        messager.printMessage(
-        if ( !(
+            Kind.ERROR,
-            property.getMappingMethod() != null ||
+            String.format(
-                property.getConversion() != null ||
+                "Can't map property \"%s %s\" to \"%s %s\".",
-                ( property.getTargetType().isCollectionType() && property.getTargetType()
+                property.getSourceType(),
-                    .getCollectionImplementationType() != null ) ) ) {
+                property.getSourceName(),
-
+                property.getTargetType(),
-            printMessage(
+                property.getTargetName()
-                ReportingPolicy.ERROR,
+            ),
-                String.format(
+            method.getExecutable()
-                    "Can't map property \"%s %s\" to \"%s %s\".",
+        );
-                    property.getSourceType(),
-                    property.getSourceName(),
-                    property.getTargetType(),
-                    property.getTargetName()
-                ),
-                method.getExecutable()
-            );
-        }
-    }
-
-    private void printMessage(ReportingPolicy reportingPolicy, String message, Element element) {
-        messager.printMessage( reportingPolicy.getDiagnosticKind(), message, element );
-        if ( reportingPolicy.failsBuild() ) {
-            isErroneous = true;
-        }
-    }
-
-    private void printMessage(ReportingPolicy reportingPolicy, String message, Element element,
-                              AnnotationMirror annotationMirror, AnnotationValue annotationValue) {
-        messager
-            .printMessage(
-                reportingPolicy.getDiagnosticKind(),
-                message,
-                element,
-                annotationMirror,
-                annotationValue
-            );
-        if ( reportingPolicy.failsBuild() ) {
-            isErroneous = true;
-        }
     }
 }

processor/src/main/java/org/mapstruct/ap/processor/MapperRenderingProcessor.java

@@ -28,17 +28,18 @@ import org.mapstruct.ap.writer.ModelWriter;
 
 /**
  * A {@link ModelElementProcessor} which creates a Java source file representing
- * the given {@link Mapper} object.
+ * the given {@link Mapper} object, unless the given mapper type is erroneous.
  *
  * @author Gunnar Morling
  */
 public class MapperRenderingProcessor implements ModelElementProcessor<Mapper, Void> {
 
     @Override
-    public Void process(ProcessorContext context, TypeElement mapperTypeElement, Mapper sourceElement) {
+    public Void process(ProcessorContext context, TypeElement mapperTypeElement, Mapper mapper) {
-        if ( !sourceElement.isErroneous() ) {
+        if ( !context.isErroneous() ) {
-            writeToSourceFile( context.getFiler(), sourceElement );
+            writeToSourceFile( context.getFiler(), mapper );
         }
+
         return null;
     }
 

processor/src/main/java/org/mapstruct/ap/processor/MethodRetrievalProcessor.java

@@ -23,17 +23,16 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import javax.annotation.processing.Messager;
-import javax.lang.model.element.Element;
 import javax.lang.model.element.ExecutableElement;
 import javax.lang.model.element.TypeElement;
 import javax.lang.model.type.DeclaredType;
 import javax.lang.model.type.TypeMirror;
 import javax.lang.model.util.Types;
+import javax.tools.Diagnostic.Kind;
 
 import org.mapstruct.ap.MapperPrism;
 import org.mapstruct.ap.MappingPrism;
 import org.mapstruct.ap.MappingsPrism;
-import org.mapstruct.ap.model.ReportingPolicy;
 import org.mapstruct.ap.model.Type;
 import org.mapstruct.ap.model.source.Mapping;
 import org.mapstruct.ap.model.source.Method;
@@ -51,17 +50,17 @@ import static javax.lang.model.util.ElementFilter.methodsIn;
  *
  * @author Gunnar Morling
  */
-public class MethodRetrievalProcessor implements ModelElementProcessor<TypeElement, List<Method>> {
+public class MethodRetrievalProcessor implements ModelElementProcessor<Void, List<Method>> {
 
-    private Types typeUtils;
     private Messager messager;
+    private Types typeUtils;
     private TypeUtil typeUtil;
     private Executables executables;
 
     @Override
-    public List<Method> process(ProcessorContext context, TypeElement mapperTypeElement, TypeElement sourceElement) {
+    public List<Method> process(ProcessorContext context, TypeElement mapperTypeElement, Void sourceModel) {
-        this.typeUtils = context.getTypeUtils();
         this.messager = context.getMessager();
+        this.typeUtils = context.getTypeUtils();
         this.typeUtil = new TypeUtil( context.getElementUtils(), typeUtils );
         this.executables = new Executables( typeUtil );
 
@@ -148,8 +147,8 @@ public class MethodRetrievalProcessor implements ModelElementProcessor<TypeEleme
 
     private boolean checkParameterAndReturnType(ExecutableElement method, Type parameterType, Type returnType) {
         if ( parameterType.isIterableType() && !returnType.isIterableType() ) {
-            printMessage(
+            messager.printMessage(
-                ReportingPolicy.ERROR,
+                Kind.ERROR,
                 "Can't generate mapping method from iterable type to non-iterable type.",
                 method
             );
@@ -157,8 +156,8 @@ public class MethodRetrievalProcessor implements ModelElementProcessor<TypeEleme
         }
 
         if ( !parameterType.isIterableType() && returnType.isIterableType() ) {
-            printMessage(
+            messager.printMessage(
-                ReportingPolicy.ERROR,
+                Kind.ERROR,
                 "Can't generate mapping method from non-iterable type to iterable type.",
                 method
             );
@@ -166,20 +165,12 @@ public class MethodRetrievalProcessor implements ModelElementProcessor<TypeEleme
         }
 
         if ( parameterType.isPrimitive() ) {
-            printMessage(
+            messager.printMessage( Kind.ERROR, "Can't generate mapping method with primitive parameter type.", method );
-                ReportingPolicy.ERROR,
-                "Can't generate mapping method with primitive parameter type.",
-                method
-            );
             return false;
         }
 
         if ( returnType.isPrimitive() ) {
-            printMessage(
+            messager.printMessage( Kind.ERROR, "Can't generate mapping method with primitive return type.", method );
-                ReportingPolicy.ERROR,
-                "Can't generate mapping method with primitive return type.",
-                method
-            );
             return false;
         }
 
@@ -210,8 +201,4 @@ public class MethodRetrievalProcessor implements ModelElementProcessor<TypeEleme
 
         return mappings;
     }
-
-    private void printMessage(ReportingPolicy reportingPolicy, String message, Element element) {
-        messager.printMessage( reportingPolicy.getDiagnosticKind(), message, element );
-    }
 }

processor/src/main/java/org/mapstruct/ap/processor/ModelElementProcessor.java

@@ -23,6 +23,7 @@ import javax.annotation.processing.Messager;
 import javax.lang.model.element.TypeElement;
 import javax.lang.model.util.Elements;
 import javax.lang.model.util.Types;
+import javax.tools.Diagnostic.Kind;
 
 import org.mapstruct.ap.model.Options;
 
@@ -57,6 +58,16 @@ public interface ModelElementProcessor<P, R> {
         Messager getMessager();
 
         Options getOptions();
+
+        /**
+         * Whether the currently processed mapper type is erroneous which is the
+         * case if at least one diagnostic with {@link Kind#ERROR} is reported
+         * by any of the participating processors.
+         *
+         * @return {@code true} if the currently processed mapper type is
+         *         erroneous, {@code false} otherwise.
+         */
+        boolean isErroneous();
     }
 
     /**
@@ -66,7 +77,7 @@ public interface ModelElementProcessor<P, R> {
      * @param context Context providing common infrastructure objects.
      * @param mapperTypeElement The original type element from which the given mapper object
      * is derived.
-     * @param sourceElement The current representation of the bean mapper. Never
+     * @param sourceModel The current representation of the bean mapper. Never
      * {@code null} (the very first processor receives the original
      * type element).
      *
@@ -76,7 +87,7 @@ public interface ModelElementProcessor<P, R> {
      *         return {@code null} except for the very last processor which
      *         generates the resulting Java source file.
      */
-    R process(ProcessorContext context, TypeElement mapperTypeElement, P sourceElement);
+    R process(ProcessorContext context, TypeElement mapperTypeElement, P sourceModel);
 
     /**
      * Returns the priority value of this processor which must be between 1

processor/src/test/java/org/mapstruct/ap/testutil/MapperTestBase.java

@@ -150,7 +150,7 @@ public abstract class MapperTestBase {
         while ( actualIterator.hasNext() ) {
             assertThat( expectedIterator.hasNext() ).describedAs(
                 String.format(
-                    "Found less diagnostics than expected. Actual: %s; Expected: %s.",
+                    "Found more diagnostics than expected. Actual: %s; Expected: %s.",
                     actualDiagnostics,
                     expectedDiagnostics
                 )
@@ -174,7 +174,7 @@ public abstract class MapperTestBase {
 
         assertThat( expectedIterator.hasNext() ).describedAs(
             String.format(
-                "Found more diagnostics than expected. Actual: %s; Expected: %s.",
+                "Found less diagnostics than expected. Actual: %s; Expected: %s.",
                 actualDiagnostics,
                 expectedDiagnostics
             )