From 4c1dcc5272f372ac6f07c6b1722d2c28e657c261 Mon Sep 17 00:00:00 2001 From: sjaakd Date: Thu, 11 Jan 2018 23:15:17 +0100 Subject: [PATCH] #1345 ignoring reversed mappings with no target accessor silently --- .../ap/internal/model/source/Mapping.java | 12 ++-- .../model/source/TargetReference.java | 2 +- .../ap/test/bugs/_1345/Issue1345Mapper.java | 62 +++++++++++++++++++ .../ap/test/bugs/_1345/Issue1345Test.java | 40 ++++++++++++ .../NestedSourcePropertiesTest.java | 4 +- 5 files changed, 111 insertions(+), 9 deletions(-) create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_1345/Issue1345Mapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_1345/Issue1345Test.java diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/Mapping.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/Mapping.java index 1536c54f7..b5fa77b6e 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/Mapping.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/Mapping.java @@ -37,7 +37,6 @@ import javax.lang.model.util.Types; import org.mapstruct.ap.internal.model.common.FormattingParameters; import org.mapstruct.ap.internal.model.common.Parameter; import org.mapstruct.ap.internal.model.common.TypeFactory; -import org.mapstruct.ap.internal.prism.CollectionMappingStrategyPrism; import org.mapstruct.ap.internal.prism.MappingPrism; import org.mapstruct.ap.internal.prism.MappingsPrism; import org.mapstruct.ap.internal.util.FormattingMessager; @@ -390,11 +389,6 @@ public class Mapping { return dependsOn; } - private boolean hasPropertyInReverseMethod(String name, SourceMethod method) { - CollectionMappingStrategyPrism cms = method.getMapperConfiguration().getCollectionMappingStrategy(); - return method.getResultType().getPropertyWriteAccessors( cms ).containsKey( name ); - } - public Mapping reverse(SourceMethod method, FormattingMessager messager, TypeFactory typeFactory) { // mapping can only be reversed if the source was not a constant nor an expression nor a nested property @@ -426,6 +420,12 @@ public class Mapping { true, sourceReference != null ? sourceReference.getParameter() : null ); + + // check if the reverse mapping has indeed a write accessor, otherwise the mapping cannot be reversed + if ( !reverse.targetReference.isValid() ) { + return null; + } + return reverse; } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/TargetReference.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/TargetReference.java index b298194f3..f4eb768a9 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/TargetReference.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/TargetReference.java @@ -170,7 +170,7 @@ public class TargetReference { foundEntryMatch = (entries.size() == targetPropertyNames.length); } - if ( !foundEntryMatch && errorMessage != null) { + if ( !foundEntryMatch && errorMessage != null && !isReverse ) { // This is called only for reporting errors errorMessage.report( isReverse ); } diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1345/Issue1345Mapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1345/Issue1345Mapper.java new file mode 100644 index 000000000..5b8733b95 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1345/Issue1345Mapper.java @@ -0,0 +1,62 @@ +/** + * 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._1345; + +import org.mapstruct.InheritInverseConfiguration; +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; +import org.mapstruct.factory.Mappers; + +/** + * + * @author Sjaak Derksen + */ +@Mapper +public interface Issue1345Mapper { + + Issue1345Mapper INSTANCE = Mappers.getMapper( Issue1345Mapper.class ); + + @Mapping(target = "property", source = "readOnlyProperty") + B a2B(A a); + + @InheritInverseConfiguration(name = "a2B") + A b2A(B b); + + class A { + + private String readOnlyProperty; + + public String getReadOnlyProperty() { + return readOnlyProperty; + } + } + + class B { + + private String property; + + public String getProperty() { + return property; + } + + public void setProperty(String property) { + this.property = property; + } + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1345/Issue1345Test.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1345/Issue1345Test.java new file mode 100644 index 000000000..0397fdc89 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1345/Issue1345Test.java @@ -0,0 +1,40 @@ +/** + * 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._1345; + +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.runner.AnnotationProcessorTestRunner; + +/** + * @author Sjaak Derksen + */ +@WithClasses({ + Issue1345Mapper.class +}) +@IssueKey("1345") +@RunWith(AnnotationProcessorTestRunner.class) +public class Issue1345Test { + + @Test + public void shouldCompile() { + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/nestedsourceproperties/NestedSourcePropertiesTest.java b/processor/src/test/java/org/mapstruct/ap/test/nestedsourceproperties/NestedSourcePropertiesTest.java index 364ec0b73..9fd9c1430 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/nestedsourceproperties/NestedSourcePropertiesTest.java +++ b/processor/src/test/java/org/mapstruct/ap/test/nestedsourceproperties/NestedSourcePropertiesTest.java @@ -183,8 +183,8 @@ public class NestedSourcePropertiesTest { diagnostics = { @Diagnostic( type = ArtistToChartEntryErroneous.class, kind = javax.tools.Diagnostic.Kind.ERROR, - line = 46, - messageRegExp = "Unknown property \"position\" in result type java\\.lang\\.Integer\\." ) + line = 47, + messageRegExp = "java.lang.Integer does not have an accessible empty constructor." ) } ) @WithClasses({ ArtistToChartEntryErroneous.class })