From 6aa39ff4283b8fff5373bd292d69e1cb1da5cf0f Mon Sep 17 00:00:00 2001 From: Filip Hrisafov Date: Fri, 17 Jul 2020 22:10:26 +0200 Subject: [PATCH] #2142: Strip leading underscore when sanitizing identifier name --- .../ap/internal/model/ForgedMethod.java | 3 +- .../mapstruct/ap/internal/util/Strings.java | 19 +++++++- .../ap/internal/util/StringsTest.java | 10 ++++ .../ap/test/bugs/_2142/Issue2142Mapper.java | 46 +++++++++++++++++++ .../ap/test/bugs/_2142/Issue2142Test.java | 34 ++++++++++++++ .../test/bugs/_2142/Issue2142MapperImpl.java | 31 +++++++++++++ 6 files changed, 140 insertions(+), 3 deletions(-) create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_2142/Issue2142Mapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_2142/Issue2142Test.java create mode 100644 processor/src/test/resources/fixtures/org/mapstruct/ap/test/bugs/_2142/Issue2142MapperImpl.java diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/ForgedMethod.java b/processor/src/main/java/org/mapstruct/ap/internal/model/ForgedMethod.java index e7a64e913..ce1d8c024 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/ForgedMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/ForgedMethod.java @@ -125,8 +125,7 @@ public class ForgedMethod implements Method { boolean forgedNameBased) { // establish name - String sourceParamName = Strings.decapitalize( sourceType.getName() ); - String sourceParamSafeName = Strings.getSafeVariableName( sourceParamName ); + String sourceParamSafeName = Strings.getSafeVariableName( sourceType.getName() ); // establish parameters this.parameters = new ArrayList<>( 1 + additionalParameters.size() ); diff --git a/processor/src/main/java/org/mapstruct/ap/internal/util/Strings.java b/processor/src/main/java/org/mapstruct/ap/internal/util/Strings.java index 71acee3cf..07049c17a 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/util/Strings.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/util/Strings.java @@ -73,6 +73,8 @@ public class Strings { "while" ); + private static final char UNDERSCORE = '_'; + private Strings() { } @@ -166,7 +168,22 @@ public class Strings { * @return the identifier without any characters that are not allowed as part of a Java identifier. */ public static String sanitizeIdentifierName(String identifier) { - return identifier.replace( "[]", "Array" ); + if ( identifier != null && identifier.length() > 0 ) { + + int firstNonUnderScoreIndex = 0; + while ( firstNonUnderScoreIndex < identifier.length() && + identifier.charAt( firstNonUnderScoreIndex ) == UNDERSCORE ) { + firstNonUnderScoreIndex++; + } + + if ( firstNonUnderScoreIndex < identifier.length()) { + // If it is not consisted of only underscores + return identifier.substring( firstNonUnderScoreIndex ).replace( "[]", "Array" ); + } + + return identifier.replace( "[]", "Array" ); + } + return identifier; } /** diff --git a/processor/src/test/java/org/mapstruct/ap/internal/util/StringsTest.java b/processor/src/test/java/org/mapstruct/ap/internal/util/StringsTest.java index ced598330..e0eb961d1 100644 --- a/processor/src/test/java/org/mapstruct/ap/internal/util/StringsTest.java +++ b/processor/src/test/java/org/mapstruct/ap/internal/util/StringsTest.java @@ -83,6 +83,8 @@ public class StringsTest { assertThat( Strings.getSafeVariableName( "Case" ) ).isEqualTo( "case1" ); assertThat( Strings.getSafeVariableName( "Synchronized" ) ).isEqualTo( "synchronized1" ); assertThat( Strings.getSafeVariableName( "prop", "prop", "prop_" ) ).isEqualTo( "prop1" ); + assertThat( Strings.getSafeVariableName( "_Test" ) ).isEqualTo( "test" ); + assertThat( Strings.getSafeVariableName( "__Test" ) ).isEqualTo( "test" ); } @Test @@ -98,12 +100,20 @@ public class StringsTest { assertThat( Strings.getSafeVariableName( "prop", Arrays.asList( "prop", "prop1" ) ) ).isEqualTo( "prop2" ); assertThat( Strings.getSafeVariableName( "prop.font", Arrays.asList( "propFont", "propFont_" ) ) ) .isEqualTo( "propFont1" ); + assertThat( Strings.getSafeVariableName( "_Test", new ArrayList<>() ) ).isEqualTo( "test" ); + assertThat( Strings.getSafeVariableName( "__Test", Arrays.asList( "test" ) ) ).isEqualTo( "test1" ); + assertThat( Strings.getSafeVariableName( "___", new ArrayList<>() ) ).isEqualTo( "___" ); } @Test public void testSanitizeIdentifierName() { assertThat( Strings.sanitizeIdentifierName( "test" ) ).isEqualTo( "test" ); assertThat( Strings.sanitizeIdentifierName( "int[]" ) ).isEqualTo( "intArray" ); + assertThat( Strings.sanitizeIdentifierName( "_Test" ) ).isEqualTo( "Test" ); + assertThat( Strings.sanitizeIdentifierName( "_int[]" ) ).isEqualTo( "intArray" ); + assertThat( Strings.sanitizeIdentifierName( "__int[]" ) ).isEqualTo( "intArray" ); + assertThat( Strings.sanitizeIdentifierName( "test_" ) ).isEqualTo( "test_" ); + assertThat( Strings.sanitizeIdentifierName( "___" ) ).isEqualTo( "___" ); } @Test diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2142/Issue2142Mapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2142/Issue2142Mapper.java new file mode 100644 index 000000000..198976b37 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2142/Issue2142Mapper.java @@ -0,0 +1,46 @@ +/* + * Copyright MapStruct Authors. + * + * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0 + */ +package org.mapstruct.ap.test.bugs._2142; + +import org.mapstruct.Mapper; +import org.mapstruct.factory.Mappers; + +/** + * @author Filip Hrisafov + */ +@Mapper +public interface Issue2142Mapper { + + Issue2142Mapper INSTANCE = Mappers.getMapper( Issue2142Mapper.class ); + + _Target map(Source source); + + // CHECKSTYLE:OFF + class _Target { + // CHECKSTYLE:ON + private final String value; + + public _Target(String value) { + this.value = value; + } + + public String getValue() { + return value; + } + } + + class Source { + private final String value; + + public Source(String value) { + this.value = value; + } + + public String getValue() { + return value; + } + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2142/Issue2142Test.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2142/Issue2142Test.java new file mode 100644 index 000000000..8cfff232e --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2142/Issue2142Test.java @@ -0,0 +1,34 @@ +/* + * Copyright MapStruct Authors. + * + * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0 + */ +package org.mapstruct.ap.test.bugs._2142; + +import org.junit.Rule; +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; +import org.mapstruct.ap.testutil.runner.GeneratedSource; + +import static org.assertj.core.api.Assertions.assertThat; + +@IssueKey("2142") +@WithClasses(Issue2142Mapper.class) +@RunWith(AnnotationProcessorTestRunner.class) +public class Issue2142Test { + + @Rule + public final GeneratedSource generatedSource = new GeneratedSource() + .addComparisonToFixtureFor( Issue2142Mapper.class ); + + @Test + public void underscorePrefixShouldBeStrippedFromGeneratedLocalVariables() { + Issue2142Mapper._Target target = Issue2142Mapper.INSTANCE.map( new Issue2142Mapper.Source( "value1" ) ); + + assertThat( target ).isNotNull(); + assertThat( target.getValue() ).isEqualTo( "value1" ); + } +} diff --git a/processor/src/test/resources/fixtures/org/mapstruct/ap/test/bugs/_2142/Issue2142MapperImpl.java b/processor/src/test/resources/fixtures/org/mapstruct/ap/test/bugs/_2142/Issue2142MapperImpl.java new file mode 100644 index 000000000..8a6d7d1b7 --- /dev/null +++ b/processor/src/test/resources/fixtures/org/mapstruct/ap/test/bugs/_2142/Issue2142MapperImpl.java @@ -0,0 +1,31 @@ +/* + * Copyright MapStruct Authors. + * + * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0 + */ +package org.mapstruct.ap.test.bugs._2142; + +import javax.annotation.Generated; + +@Generated( + value = "org.mapstruct.ap.MappingProcessor", + date = "2019-02-10T09:58:11+0100", + comments = "version: , compiler: javac, environment: Java 1.8.0_181 (Oracle Corporation)" +) +public class Issue2142MapperImpl implements Issue2142Mapper { + + @Override + public _Target map(Source source) { + if ( source == null ) { + return null; + } + + String value = null; + + value = source.getValue(); + + _Target target = new _Target( value ); + + return target; + } +}