#1283 Handle no suitable empty constructor during BeanMappingMethod creation

This commit is contained in:
Filip Hrisafov 2017-09-07 17:44:53 +02:00 committed by GitHub
parent 47697a2391
commit 779c16cc2a
19 changed files with 338 additions and 51 deletions

View File

@ -190,6 +190,14 @@ public class BeanMappingMethod extends NormalTypeMappingMethod {
Message.BEANMAPPING_NOT_ASSIGNABLE, resultType, method.getResultType()
);
}
else if ( !resultType.hasEmptyAccessibleContructor() ) {
ctx.getMessager().printMessage(
method.getExecutable(),
beanMappingPrism.mirror,
Message.GENERAL_NO_SUITABLE_CONSTRUCTOR,
resultType
);
}
}
else if ( !method.isUpdateMethod() && method.getReturnType().isAbstract() ) {
ctx.getMessager().printMessage(
@ -198,6 +206,13 @@ public class BeanMappingMethod extends NormalTypeMappingMethod {
method.getReturnType()
);
}
else if ( !method.isUpdateMethod() && !method.getReturnType().hasEmptyAccessibleContructor() ) {
ctx.getMessager().printMessage(
method.getExecutable(),
Message.GENERAL_NO_SUITABLE_CONSTRUCTOR,
method.getReturnType()
);
}
}
sortPropertyMappingsByDependencies();

View File

@ -863,8 +863,7 @@ public class Type extends ModelElement implements Comparable<Type> {
hasEmptyAccessibleContructor = false;
List<ExecutableElement> constructors = ElementFilter.constructorsIn( typeElement.getEnclosedElements() );
for ( ExecutableElement constructor : constructors ) {
if ( (constructor.getModifiers().contains( Modifier.PUBLIC )
|| constructor.getModifiers().contains( Modifier.PROTECTED ) )
if ( !constructor.getModifiers().contains( Modifier.PRIVATE )
&& constructor.getParameters().isEmpty() ) {
hasEmptyAccessibleContructor = true;
break;

View File

@ -386,17 +386,6 @@ public class SourceMethod implements Method {
return isEnumMapping;
}
public boolean isBeanMapping() {
if ( isBeanMapping == null ) {
isBeanMapping = !isIterableMapping()
&& !isMapMapping()
&& !isEnumMapping()
&& !isValueMapping()
&& !isStreamMapping();
}
return isBeanMapping;
}
/**
* The default enum mapping (no mappings specified) will from now on be handled as a value mapping. If there
* are any @Mapping / @Mappings defined on the method, then the deprecated enum behavior should be executed.

View File

@ -23,16 +23,17 @@ import java.util.Arrays;
import java.util.List;
import java.util.Set;
import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.type.DeclaredType;
import org.mapstruct.ap.internal.model.common.Parameter;
import org.mapstruct.ap.internal.model.common.Type;
import org.mapstruct.ap.internal.model.common.TypeFactory;
import org.mapstruct.ap.internal.prism.CollectionMappingStrategyPrism;
import org.mapstruct.ap.internal.prism.InheritInverseConfigurationPrism;
import org.mapstruct.ap.internal.util.Executables;
import org.mapstruct.ap.internal.util.FormattingMessager;
import org.mapstruct.ap.internal.util.Message;
import org.mapstruct.ap.internal.util.Strings;
import org.mapstruct.ap.internal.util.accessor.Accessor;
@ -161,7 +162,7 @@ public class TargetReference {
if ( !foundEntryMatch && errorMessage != null) {
// This is called only for reporting errors
errorMessage.report();
errorMessage.report( isReverse );
}
// foundEntryMatch = isValid, errors are handled here, and the BeanMapping uses that to ignore
@ -364,14 +365,21 @@ public class TargetReference {
this.messager = messager;
}
abstract void report();
abstract void report(boolean isReverse);
protected void printErrorMessage(Message message, Object... args) {
protected void printErrorMessage(Message message, boolean isReverse, Object... args) {
Object[] errorArgs = new Object[args.length + 2];
errorArgs[0] = mapping.getTargetName();
errorArgs[1] = method.getResultType();
System.arraycopy( args, 0, errorArgs, 2, args.length );
messager.printMessage( method.getExecutable(), mapping.getMirror(), mapping.getSourceAnnotationValue(),
AnnotationMirror annotationMirror = mapping.getMirror();
if ( isReverse ) {
InheritInverseConfigurationPrism reversePrism = InheritInverseConfigurationPrism.getInstanceOn(
method.getExecutable() );
annotationMirror = reversePrism == null ? annotationMirror : reversePrism.mirror;
}
messager.printMessage( method.getExecutable(), annotationMirror, mapping.getSourceAnnotationValue(),
message, errorArgs
);
}
@ -384,8 +392,8 @@ public class TargetReference {
}
@Override
public void report() {
printErrorMessage( Message.BEANMAPPING_PROPERTY_HAS_NO_WRITE_ACCESSOR_IN_RESULTTYPE );
public void report(boolean isReverse) {
printErrorMessage( Message.BEANMAPPING_PROPERTY_HAS_NO_WRITE_ACCESSOR_IN_RESULTTYPE, isReverse );
}
}
@ -404,7 +412,7 @@ public class TargetReference {
}
@Override
public void report() {
public void report(boolean isReverse) {
Set<String> readAccessors = nextType.getPropertyReadAccessors().keySet();
String mostSimilarProperty = Strings.getMostSimilarWord(
@ -415,7 +423,11 @@ public class TargetReference {
List<String> elements = new ArrayList<String>( Arrays.asList( entryNames ).subList( 0, index ) );
elements.add( mostSimilarProperty );
printErrorMessage( Message.BEANMAPPING_UNKNOWN_PROPERTY_IN_RESULTTYPE, Strings.join( elements, "." ) );
printErrorMessage(
Message.BEANMAPPING_UNKNOWN_PROPERTY_IN_RESULTTYPE,
isReverse,
Strings.join( elements, "." )
);
}
}

View File

@ -517,15 +517,6 @@ public class MapperCreationProcessor implements ModelElementProcessor<List<Sourc
if ( reversePrism != null ) {
// is there a suitable constructor
if ( method.isBeanMapping()
&& !method.isUpdateMethod()
&& !method.getReturnType().isCollectionOrMapType()
&& !method.getReturnType().hasEmptyAccessibleContructor() ) {
reportErrorWhenNoSuitableConstrutor( method, reversePrism );
return null;
}
// method is configured as being reverse method, collect candidates
List<SourceMethod> candidates = new ArrayList<SourceMethod>();
for ( SourceMethod oneMethod : rawMethods ) {
@ -689,17 +680,6 @@ public class MapperCreationProcessor implements ModelElementProcessor<List<Sourc
}
}
private void reportErrorWhenNoSuitableConstrutor( SourceMethod method,
InheritInverseConfigurationPrism reversePrism ) {
messager.printMessage( method.getExecutable(),
reversePrism.mirror,
Message.INHERITINVERSECONFIGURATION_NO_SUITABLE_CONSTRUCTOR,
reversePrism.name()
);
}
private void reportErrorWhenSeveralNamesMatch(List<SourceMethod> candidates, SourceMethod method,
InheritInverseConfigurationPrism reversePrism) {

View File

@ -89,6 +89,7 @@ public enum Message {
GENERAL_VALID_DATE( "Given date format \"%s\" is valid.", Diagnostic.Kind.NOTE ),
GENERAL_INVALID_DATE( "Given date format \"%s\" is invalid. Message: \"%s\"." ),
GENERAL_NOT_ALL_FORGED_CREATED( "Internal Error in creation of Forged Methods, it was expected all Forged Methods to finished with creation, but %s did not" ),
GENERAL_NO_SUITABLE_CONSTRUCTOR( "%s does not have an accessible empty constructor." ),
RETRIEVAL_NO_INPUT_ARGS( "Can't generate mapping method with no input arguments." ),
RETRIEVAL_DUPLICATE_MAPPING_TARGETS( "Can't generate mapping method with more than one @MappingTarget parameter." ),
@ -111,7 +112,6 @@ public enum Message {
INHERITINVERSECONFIGURATION_DUPLICATES( "Several matching inverse methods exist: %s(). Specify a name explicitly." ),
INHERITINVERSECONFIGURATION_INVALID_NAME( "None of the candidates %s() matches given name: \"%s\"." ),
INHERITINVERSECONFIGURATION_DUPLICATE_MATCHES( "Given name \"%s\" matches several candidate methods: %s." ),
INHERITINVERSECONFIGURATION_NO_SUITABLE_CONSTRUCTOR( "There is no suitable result type constructor for reversing this method." ),
INHERITINVERSECONFIGURATION_NO_NAME_MATCH( "Given name \"%s\" does not match the only candidate. Did you mean: \"%s\"." ),
INHERITCONFIGURATION_DUPLICATES( "Several matching methods exist: %s(). Specify a name explicitly." ),
INHERITCONFIGURATION_INVALIDNAME( "None of the candidates %s() matches given name: \"%s\"." ),

View File

@ -76,7 +76,11 @@ public class Issue1242Test {
+ " .*TargetB .*TargetFactories\\.createTargetB\\(.*SourceB source,"
+ " @TargetType .*Class<.*TargetB> clazz\\),"
+ " .*TargetB .*TargetFactories\\.createTargetB\\(@TargetType java.lang.Class<.*TargetB> clazz\\),"
+ " .*TargetB .*TargetFactories\\.createTargetB\\(\\).")
+ " .*TargetB .*TargetFactories\\.createTargetB\\(\\)."),
@Diagnostic(type = ErroneousIssue1242MapperMultipleSources.class,
kind = javax.tools.Diagnostic.Kind.ERROR,
line = 33,
messageRegExp = ".*TargetB does not have an accessible empty constructor\\.")
})
public void ambiguousMethodErrorForTwoFactoryMethodsWithSourceParam() {
}

View File

@ -0,0 +1,36 @@
/**
* Copyright 2012-2017 Gunnar Morling (http://www.gunnarmorling.de/)
* and/or other contributors as indicated by the @authors tag. See the
* copyright.txt file in the distribution for a full listing of all
* contributors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.mapstruct.ap.test.bugs._1283;
import org.mapstruct.InheritInverseConfiguration;
import org.mapstruct.Mapper;
import org.mapstruct.Mapping;
/**
* @author Filip Hrisafov
*/
@Mapper
public interface ErroneousInverseTargetHasNoSuitableConstructorMapper {
@Mapping(target = "target", source = "source")
Target fromSource(Source source);
@InheritInverseConfiguration
Source fromTarget(Target target);
}

View File

@ -0,0 +1,32 @@
/**
* Copyright 2012-2017 Gunnar Morling (http://www.gunnarmorling.de/)
* and/or other contributors as indicated by the @authors tag. See the
* copyright.txt file in the distribution for a full listing of all
* contributors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.mapstruct.ap.test.bugs._1283;
import org.mapstruct.Mapper;
import org.mapstruct.Mapping;
/**
* @author Filip Hrisafov
*/
@Mapper
public interface ErroneousTargetHasNoSuitableConstructorMapper {
@Mapping(target = "source", source = "target")
Source fromTarget(Target target);
}

View File

@ -0,0 +1,70 @@
/**
* Copyright 2012-2017 Gunnar Morling (http://www.gunnarmorling.de/)
* and/or other contributors as indicated by the @authors tag. See the
* copyright.txt file in the distribution for a full listing of all
* contributors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.mapstruct.ap.test.bugs._1283;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mapstruct.ap.testutil.IssueKey;
import org.mapstruct.ap.testutil.WithClasses;
import org.mapstruct.ap.testutil.compilation.annotation.CompilationResult;
import org.mapstruct.ap.testutil.compilation.annotation.Diagnostic;
import org.mapstruct.ap.testutil.compilation.annotation.ExpectedCompilationOutcome;
import org.mapstruct.ap.testutil.runner.AnnotationProcessorTestRunner;
/**
* @author Filip Hrisafov
*/
@IssueKey("1283")
@RunWith(AnnotationProcessorTestRunner.class)
@WithClasses({
Source.class,
Target.class
})
public class Issue1283Test {
@Test
@WithClasses(ErroneousInverseTargetHasNoSuitableConstructorMapper.class)
@ExpectedCompilationOutcome(
value = CompilationResult.FAILED,
diagnostics = {
@Diagnostic(type = ErroneousInverseTargetHasNoSuitableConstructorMapper.class,
kind = javax.tools.Diagnostic.Kind.ERROR,
line = 35L,
messageRegExp = ".*\\._1283\\.Source does not have an accessible empty constructor"
)
}
)
public void inheritInverseConfigurationReturnTypeHasNoSuitableConstructor() {
}
@Test
@WithClasses(ErroneousTargetHasNoSuitableConstructorMapper.class)
@ExpectedCompilationOutcome(
value = CompilationResult.FAILED,
diagnostics = {
@Diagnostic(type = ErroneousTargetHasNoSuitableConstructorMapper.class,
kind = javax.tools.Diagnostic.Kind.ERROR,
line = 31L,
messageRegExp = ".*\\._1283\\.Source does not have an accessible empty constructor"
)
}
)
public void returnTypeHasNoSuitableConstructor() {
}
}

View File

@ -0,0 +1,39 @@
/**
* Copyright 2012-2017 Gunnar Morling (http://www.gunnarmorling.de/)
* and/or other contributors as indicated by the @authors tag. See the
* copyright.txt file in the distribution for a full listing of all
* contributors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.mapstruct.ap.test.bugs._1283;
/**
* @author Filip Hrisafov
*/
public class Source {
private String source;
public Source(String source) {
this.source = source;
}
public String getSource() {
return source;
}
public void setSource(String source) {
this.source = source;
}
}

View File

@ -0,0 +1,35 @@
/**
* Copyright 2012-2017 Gunnar Morling (http://www.gunnarmorling.de/)
* and/or other contributors as indicated by the @authors tag. See the
* copyright.txt file in the distribution for a full listing of all
* contributors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.mapstruct.ap.test.bugs._1283;
/**
* @author Filip Hrisafov
*/
public class Target {
private String target;
public String getTarget() {
return target;
}
public void setTarget(String target) {
this.target = target;
}
}

View File

@ -49,7 +49,13 @@ public class AmbiguousAnnotatedFactoryTest {
+ "createBar\\(org.mapstruct.ap.test.erroneous.ambiguousannotatedfactorymethod.Foo foo\\), "
+ "org.mapstruct.ap.test.erroneous.ambiguousannotatedfactorymethod.Bar "
+ ".*AmbiguousBarFactory.createBar\\(org.mapstruct.ap.test.erroneous."
+ "ambiguousannotatedfactorymethod.Foo foo\\)." )
+ "ambiguousannotatedfactorymethod.Foo foo\\)."),
@Diagnostic(type = SourceTargetMapperAndBarFactory.class,
kind = javax.tools.Diagnostic.Kind.ERROR,
line = 35,
messageRegExp = ".*\\.ambiguousannotatedfactorymethod.Bar does not have an accessible empty " +
"constructor\\.")
}
)
public void shouldUseTwoFactoryMethods() {

View File

@ -50,7 +50,12 @@ public class FactoryTest {
messageRegExp = "Ambiguous factory methods found for creating "
+ "org.mapstruct.ap.test.erroneous.ambiguousfactorymethod.Bar: "
+ "org.mapstruct.ap.test.erroneous.ambiguousfactorymethod.Bar createBar\\(\\), "
+ "org.mapstruct.ap.test.erroneous.ambiguousfactorymethod.Bar .*BarFactory.createBar\\(\\)." )
+ "org.mapstruct.ap.test.erroneous.ambiguousfactorymethod.Bar .*BarFactory.createBar\\(\\)."),
@Diagnostic(type = SourceTargetMapperAndBarFactory.class,
kind = javax.tools.Diagnostic.Kind.ERROR,
line = 35,
messageRegExp = ".*\\.ambiguousfactorymethod\\.Bar does not have an accessible empty constructor\\.")
}
)
public void shouldUseTwoFactoryMethods() {

View File

@ -184,7 +184,7 @@ public class NestedSourcePropertiesTest {
@Diagnostic( type = ArtistToChartEntryErroneous.class,
kind = javax.tools.Diagnostic.Kind.ERROR,
line = 46,
messageRegExp = "There is no suitable result type constructor for reversing this method\\." )
messageRegExp = "Unknown property \"position\" in result type java\\.lang\\.Integer\\." )
}
)
@WithClasses({ ArtistToChartEntryErroneous.class })

View File

@ -177,7 +177,13 @@ public class ConversionTest {
messageRegExp = "Can't map property \".*NoProperties "
+ "foo\\.wrapped\" to"
+ " \"org.mapstruct.ap.test.selection.generics.TypeA " +
"foo\\.wrapped\"")
"foo\\.wrapped\""),
@Diagnostic(type = ErroneousSourceTargetMapper6.class,
kind = javax.tools.Diagnostic.Kind.ERROR,
line = 29,
messageRegExp = ".*\\.generics\\.WildCardSuperWrapper<.*\\.generics\\.TypeA> does not have an " +
"accessible empty constructor\\.")
})
public void shouldFailOnNonMatchingWildCards() {
}

View File

@ -0,0 +1,34 @@
/**
* Copyright 2012-2017 Gunnar Morling (http://www.gunnarmorling.de/)
* and/or other contributors as indicated by the @authors tag. See the
* copyright.txt file in the distribution for a full listing of all
* contributors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.mapstruct.ap.test.selection.resulttype;
import org.mapstruct.BeanMapping;
import org.mapstruct.Mapper;
import org.mapstruct.Mapping;
/**
* @author Filip Hrisafov
*/
@Mapper
public interface ErroneousResultTypeNoEmptyConstructorMapper {
@BeanMapping(resultType = Banana.class)
@Mapping(target = "type", ignore = true)
Fruit map(FruitDto source);
}

View File

@ -60,12 +60,31 @@ public class InheritanceSelectionTest {
line = 36,
messageRegExp = "Ambiguous factory methods found for creating .*Fruit: "
+ ".*Apple .*ConflictingFruitFactory\\.createApple\\(\\), "
+ ".*Banana .*ConflictingFruitFactory\\.createBanana\\(\\)\\.")
+ ".*Banana .*ConflictingFruitFactory\\.createBanana\\(\\)\\."),
@Diagnostic(type = ErroneousFruitMapper.class,
kind = Kind.ERROR,
line = 36,
messageRegExp = ".*Fruit does not have an accessible empty constructor\\.")
}
)
public void testForkedInheritanceHierarchyShouldResultInAmbigousMappingMethod() {
}
@IssueKey("1283")
@Test
@WithClasses( { ErroneousResultTypeNoEmptyConstructorMapper.class, Banana.class } )
@ExpectedCompilationOutcome(
value = CompilationResult.FAILED,
diagnostics = {
@Diagnostic(type = ErroneousResultTypeNoEmptyConstructorMapper.class,
kind = Kind.ERROR,
line = 31,
messageRegExp = ".*\\.resulttype\\.Banana does not have an accessible empty constructor\\.")
}
)
public void testResultTypeHasNoSuitableEmptyConstructor() {
}
@Test
@WithClasses( { ConflictingFruitFactory.class, ResultTypeSelectingFruitMapper.class, Banana.class } )
public void testResultTypeBasedFactoryMethodSelection() {

View File

@ -184,8 +184,14 @@ public class UpdateMethodsTest {
@Diagnostic(type = ErroneousOrganizationMapper2.class,
kind = javax.tools.Diagnostic.Kind.ERROR,
line = 37,
messageRegExp = "No read accessor found for property \"type\" in target type.")
} )
messageRegExp = "No read accessor found for property \"type\" in target type."),
@Diagnostic(type = ErroneousOrganizationMapper2.class,
kind = javax.tools.Diagnostic.Kind.ERROR,
line = 48,
messageRegExp = ".*\\.updatemethods\\.DepartmentEntity does not have an accessible empty " +
"constructor\\.")
})
public void testShouldFailOnConstantMappingNoPropertyGetter() { }
@Test