From 73ea0e84600ec4f20759f7b3eff35859db71e009 Mon Sep 17 00:00:00 2001 From: Simon Ogorodnik Date: Tue, 13 Dec 2016 19:34:57 +0300 Subject: [PATCH] Fix for KT-15027 J2K: Annotations are set on functions, but not on property accessors #KT-15027 fixed --- .../kotlin/j2k/AnnotationConverter.kt | 41 +++++++++++++------ .../kotlin/j2k/ConstructorConverter.kt | 2 +- j2k/src/org/jetbrains/kotlin/j2k/Converter.kt | 19 ++++++--- .../jetbrains/kotlin/j2k/ast/Annotation.kt | 13 +++++- ...otationFromConstructorParamToProperty.java | 16 ++++++++ ...nnotationFromConstructorParamToProperty.kt | 6 +++ .../annotationsFromPropertyAccessors.java | 18 ++++++++ .../annotationsFromPropertyAccessors.kt | 8 ++++ ...ersToAnnotationsFromPropertyAccessors.java | 11 +++++ ...fiersToAnnotationsFromPropertyAccessors.kt | 11 +++++ .../constructors/constructorAnnotations.kt | 3 +- ...otlinConverterForWebDemoTestGenerated.java | 18 ++++++++ ...otlinConverterSingleFileTestGenerated.java | 18 ++++++++ 13 files changed, 162 insertions(+), 22 deletions(-) create mode 100644 j2k/testData/fileOrElement/annotations/annotationFromConstructorParamToProperty.java create mode 100644 j2k/testData/fileOrElement/annotations/annotationFromConstructorParamToProperty.kt create mode 100644 j2k/testData/fileOrElement/annotations/annotationsFromPropertyAccessors.java create mode 100644 j2k/testData/fileOrElement/annotations/annotationsFromPropertyAccessors.kt create mode 100644 j2k/testData/fileOrElement/annotations/modifiersToAnnotationsFromPropertyAccessors.java create mode 100644 j2k/testData/fileOrElement/annotations/modifiersToAnnotationsFromPropertyAccessors.kt diff --git a/j2k/src/org/jetbrains/kotlin/j2k/AnnotationConverter.kt b/j2k/src/org/jetbrains/kotlin/j2k/AnnotationConverter.kt index c7f9865ef12..b4f041463fe 100644 --- a/j2k/src/org/jetbrains/kotlin/j2k/AnnotationConverter.kt +++ b/j2k/src/org/jetbrains/kotlin/j2k/AnnotationConverter.kt @@ -37,15 +37,15 @@ class AnnotationConverter(private val converter: Converter) { return nameAsString in annotationsToRemove || nameAsString == Target::class.java.name } - fun convertAnnotations(owner: PsiModifierListOwner): Annotations - = convertAnnotationsOnly(owner) + convertModifiersToAnnotations(owner) + fun convertAnnotations(owner: PsiModifierListOwner, target: AnnotationUseTarget? = null): Annotations + = convertAnnotationsOnly(owner, target) + convertModifiersToAnnotations(owner, target) - private fun convertAnnotationsOnly(owner: PsiModifierListOwner): Annotations { + private fun convertAnnotationsOnly(owner: PsiModifierListOwner, target: AnnotationUseTarget?): Annotations { val modifierList = owner.modifierList val annotations = modifierList?.annotations?.filter { it.qualifiedName !in annotationsToRemove } var convertedAnnotations: List = if (annotations != null && annotations.isNotEmpty()) { - val newLines = if (!modifierList!!.isInSingleLine()) { + val newLines = if (!modifierList.isInSingleLine()) { true } else { @@ -57,14 +57,14 @@ class AnnotationConverter(private val converter: Converter) { if (child is PsiWhiteSpace) !child.isInSingleLine() else false } - annotations.mapNotNull { convertAnnotation(it, newLineAfter = newLines) } + annotations.mapNotNull { convertAnnotation(it, newLineAfter = newLines, target = target) } } else { listOf() } if (owner is PsiDocCommentOwner) { - val deprecatedAnnotation = convertDeprecatedJavadocTag(owner) + val deprecatedAnnotation = convertDeprecatedJavadocTag(owner, target) if (deprecatedAnnotation != null) { convertedAnnotations += deprecatedAnnotation } @@ -73,20 +73,29 @@ class AnnotationConverter(private val converter: Converter) { return Annotations(convertedAnnotations).assignNoPrototype() } - private fun convertDeprecatedJavadocTag(element: PsiDocCommentOwner): Annotation? { + private fun convertDeprecatedJavadocTag(element: PsiDocCommentOwner, target: AnnotationUseTarget?): Annotation? { val deprecatedTag = element.docComment?.findTagByName("deprecated") ?: return null val deferredExpression = converter.deferredElement { LiteralExpression("\"" + StringUtil.escapeStringCharacters(deprecatedTag.content()) + "\"").assignNoPrototype() } - return Annotation(Identifier("Deprecated").assignPrototype(deprecatedTag.nameElement), - listOf(null to deferredExpression), newLineAfter = true) + val identifier = Identifier("Deprecated").assignPrototype(deprecatedTag.nameElement) + return Annotation(identifier, + listOf(null to deferredExpression), + true, + effectiveAnnotationUseTarget(identifier.name, target)) .assignPrototype(deprecatedTag) } - private fun convertModifiersToAnnotations(owner: PsiModifierListOwner): Annotations { + private fun convertModifiersToAnnotations(owner: PsiModifierListOwner, target: AnnotationUseTarget?): Annotations { val list = MODIFIER_TO_ANNOTATION .filter { owner.hasModifierProperty(it.first) } - .map { Annotation(Identifier.withNoPrototype(it.second), listOf(), newLineAfter = false).assignNoPrototype() } + .map { + Annotation(Identifier.withNoPrototype(it.second), + listOf(), + newLineAfter = false, + target = target + ).assignNoPrototype() + } return Annotations(list).assignNoPrototype() } @@ -101,9 +110,15 @@ class AnnotationConverter(private val converter: Converter) { return expr.referenceName?.let { JavaAnnotationTargetMapper.mapJavaTargetArgumentByName(it) } ?: emptySet() } - fun convertAnnotation(annotation: PsiAnnotation, newLineAfter: Boolean): Annotation? { + private fun effectiveAnnotationUseTarget(name: String, target: AnnotationUseTarget?): AnnotationUseTarget? = + when (name) { + "Deprecated" -> if (target == AnnotationUseTarget.Param) null else target + else -> target + } + + fun convertAnnotation(annotation: PsiAnnotation, newLineAfter: Boolean, target: AnnotationUseTarget? = null): Annotation? { val (name, arguments) = convertAnnotationValue(annotation) ?: return null - return Annotation(name, arguments, newLineAfter).assignPrototype(annotation) + return Annotation(name, arguments, newLineAfter, effectiveAnnotationUseTarget(name.name, target)).assignPrototype(annotation, CommentsAndSpacesInheritance.NO_SPACES) } private fun convertAnnotationValue(annotation: PsiAnnotation): Pair>>>? { diff --git a/j2k/src/org/jetbrains/kotlin/j2k/ConstructorConverter.kt b/j2k/src/org/jetbrains/kotlin/j2k/ConstructorConverter.kt index c63318d3fbd..7077130c6e5 100644 --- a/j2k/src/org/jetbrains/kotlin/j2k/ConstructorConverter.kt +++ b/j2k/src/org/jetbrains/kotlin/j2k/ConstructorConverter.kt @@ -203,7 +203,7 @@ class ConstructorConverter( FunctionParameter(propertyInfo.identifier, type, if (propertyInfo.isVar) FunctionParameter.VarValModifier.Var else FunctionParameter.VarValModifier.Val, - converter.convertAnnotations(parameter) + converter.convertAnnotations(field), + converter.convertAnnotations(parameter, AnnotationUseTarget.Param) + converter.convertAnnotations(field), propertyInfo.modifiers, default) .assignPrototypes( diff --git a/j2k/src/org/jetbrains/kotlin/j2k/Converter.kt b/j2k/src/org/jetbrains/kotlin/j2k/Converter.kt index 540e956d62d..333f4c0e2ba 100644 --- a/j2k/src/org/jetbrains/kotlin/j2k/Converter.kt +++ b/j2k/src/org/jetbrains/kotlin/j2k/Converter.kt @@ -160,8 +160,8 @@ class Converter private constructor( return File(convertedChildren).assignPrototype(javaFile) } - fun convertAnnotations(owner: PsiModifierListOwner): Annotations - = annotationConverter.convertAnnotations(owner) + fun convertAnnotations(owner: PsiModifierListOwner, target: AnnotationUseTarget? = null): Annotations + = annotationConverter.convertAnnotations(owner, target) fun convertClass(psiClass: PsiClass): Class { if (psiClass.isAnnotationType) { @@ -320,10 +320,19 @@ class Converter private constructor( val getMethod = propertyInfo.getMethod val setMethod = propertyInfo.setMethod - //TODO: annotations from getter/setter? - val annotations = field?.let { convertAnnotations(it) } ?: Annotations.Empty - val modifiers = (propertyInfo.modifiers + (field?.let{ specialModifiersCase(field) } ?: Modifiers.Empty)) + var annotations = Annotations.Empty + + field?.let { annotations += convertAnnotations(it) } + + if (!propertyInfo.needExplicitGetter) + getMethod?.let { annotations += convertAnnotations(it, AnnotationUseTarget.Get) } + + if (!propertyInfo.needExplicitSetter) + setMethod?.let { annotations += convertAnnotations(it, AnnotationUseTarget.Set) } + + + val modifiers = (propertyInfo.modifiers + (field?.let { specialModifiersCase(field) } ?: Modifiers.Empty)) val name = propertyInfo.identifier if (field is PsiEnumConstant) { diff --git a/j2k/src/org/jetbrains/kotlin/j2k/ast/Annotation.kt b/j2k/src/org/jetbrains/kotlin/j2k/ast/Annotation.kt index 17c17af85e5..9f7ddd95ffb 100644 --- a/j2k/src/org/jetbrains/kotlin/j2k/ast/Annotation.kt +++ b/j2k/src/org/jetbrains/kotlin/j2k/ast/Annotation.kt @@ -20,9 +20,20 @@ import org.jetbrains.kotlin.j2k.CodeBuilder import org.jetbrains.kotlin.j2k.append import org.jetbrains.kotlin.j2k.buildList -class Annotation(val name: Identifier, val arguments: List>>, val newLineAfter: Boolean) : Element() { +enum class AnnotationUseTarget(val id: String) { + File("file"), Param("param"), Get("get"), Set("set") +} + +class Annotation(val name: Identifier, + val arguments: List>>, + val newLineAfter: Boolean, + val target: AnnotationUseTarget? = null) : Element() { + override fun generateCode(builder: CodeBuilder) { builder.append("@") + target?.let { + builder.append("${it.id}:") + } if (arguments.isEmpty()) { builder.append(name) } diff --git a/j2k/testData/fileOrElement/annotations/annotationFromConstructorParamToProperty.java b/j2k/testData/fileOrElement/annotations/annotationFromConstructorParamToProperty.java new file mode 100644 index 00000000000..3d74ca54bd1 --- /dev/null +++ b/j2k/testData/fileOrElement/annotations/annotationFromConstructorParamToProperty.java @@ -0,0 +1,16 @@ +@interface A { + +} + +@interface B { + +} + +public class U { + @B + public int i; + + public U(@A int i) { + this.i = i; + } +} \ No newline at end of file diff --git a/j2k/testData/fileOrElement/annotations/annotationFromConstructorParamToProperty.kt b/j2k/testData/fileOrElement/annotations/annotationFromConstructorParamToProperty.kt new file mode 100644 index 00000000000..b51760889f6 --- /dev/null +++ b/j2k/testData/fileOrElement/annotations/annotationFromConstructorParamToProperty.kt @@ -0,0 +1,6 @@ +internal annotation class A + +internal annotation class B + +class U(@param:A @B + var i: Int) \ No newline at end of file diff --git a/j2k/testData/fileOrElement/annotations/annotationsFromPropertyAccessors.java b/j2k/testData/fileOrElement/annotations/annotationsFromPropertyAccessors.java new file mode 100644 index 00000000000..5074eb24e58 --- /dev/null +++ b/j2k/testData/fileOrElement/annotations/annotationsFromPropertyAccessors.java @@ -0,0 +1,18 @@ +@interface An { + String value(); +} + + +public class Test { + private int id; + + @An(value = "get") + public int getId() { + return id; + } + + @An(value = "set") + public void setId(int id) { + this.id = id + } +} diff --git a/j2k/testData/fileOrElement/annotations/annotationsFromPropertyAccessors.kt b/j2k/testData/fileOrElement/annotations/annotationsFromPropertyAccessors.kt new file mode 100644 index 00000000000..161f3d2edf0 --- /dev/null +++ b/j2k/testData/fileOrElement/annotations/annotationsFromPropertyAccessors.kt @@ -0,0 +1,8 @@ +internal annotation class An(val value: String) + + +class Test { + @get:An(value = "get") + @set:An(value = "set") + var id: Int = 0 +} diff --git a/j2k/testData/fileOrElement/annotations/modifiersToAnnotationsFromPropertyAccessors.java b/j2k/testData/fileOrElement/annotations/modifiersToAnnotationsFromPropertyAccessors.java new file mode 100644 index 00000000000..34293acd5c6 --- /dev/null +++ b/j2k/testData/fileOrElement/annotations/modifiersToAnnotationsFromPropertyAccessors.java @@ -0,0 +1,11 @@ +public class WithModifiersOnAccessors { + private synchronized void methSync() {} + protected strictfp void methStrict() {} + + private int sync = 0; + public synchronized int getSync() { return sync; } + public synchronized void setSync(int sync) { this.sync = sync; } + + public double strict = 0.0; + public strictfp double getStrict() { return strict; } +} \ No newline at end of file diff --git a/j2k/testData/fileOrElement/annotations/modifiersToAnnotationsFromPropertyAccessors.kt b/j2k/testData/fileOrElement/annotations/modifiersToAnnotationsFromPropertyAccessors.kt new file mode 100644 index 00000000000..9c86952c1cb --- /dev/null +++ b/j2k/testData/fileOrElement/annotations/modifiersToAnnotationsFromPropertyAccessors.kt @@ -0,0 +1,11 @@ +class WithModifiersOnAccessors { + @Synchronized private fun methSync() { + } + + @Strictfp protected fun methStrict() { + } + + @get:Synchronized @set:Synchronized var sync = 0 + + @get:Strictfp var strict = 0.0 +} \ No newline at end of file diff --git a/j2k/testData/fileOrElement/constructors/constructorAnnotations.kt b/j2k/testData/fileOrElement/constructors/constructorAnnotations.kt index 3e3281a85b3..ca06e1c5cec 100644 --- a/j2k/testData/fileOrElement/constructors/constructorAnnotations.kt +++ b/j2k/testData/fileOrElement/constructors/constructorAnnotations.kt @@ -1,7 +1,6 @@ import javaApi.Anon5 -internal class A -@Anon5(10) +internal class A @Anon5(10) constructor(private val a: Int, private val b: Int) { @Deprecated("") // this constructor will not be replaced by default parameter value in primary because of this annotation diff --git a/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterForWebDemoTestGenerated.java b/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterForWebDemoTestGenerated.java index d8f21463719..f1097232f96 100644 --- a/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterForWebDemoTestGenerated.java +++ b/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterForWebDemoTestGenerated.java @@ -56,6 +56,12 @@ public class JavaToKotlinConverterForWebDemoTestGenerated extends AbstractJavaTo doTest(fileName); } + @TestMetadata("annotationFromConstructorParamToProperty.java") + public void testAnnotationFromConstructorParamToProperty() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/annotations/annotationFromConstructorParamToProperty.java"); + doTest(fileName); + } + @TestMetadata("annotationInterface1.java") public void testAnnotationInterface1() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/annotations/annotationInterface1.java"); @@ -86,6 +92,12 @@ public class JavaToKotlinConverterForWebDemoTestGenerated extends AbstractJavaTo doTest(fileName); } + @TestMetadata("annotationsFromPropertyAccessors.java") + public void testAnnotationsFromPropertyAccessors() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/annotations/annotationsFromPropertyAccessors.java"); + doTest(fileName); + } + @TestMetadata("javaClassArgument.java") public void testJavaClassArgument() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/annotations/javaClassArgument.java"); @@ -116,6 +128,12 @@ public class JavaToKotlinConverterForWebDemoTestGenerated extends AbstractJavaTo doTest(fileName); } + @TestMetadata("modifiersToAnnotationsFromPropertyAccessors.java") + public void testModifiersToAnnotationsFromPropertyAccessors() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/annotations/modifiersToAnnotationsFromPropertyAccessors.java"); + doTest(fileName); + } + @TestMetadata("primaryConstructorAnnotation.java") public void testPrimaryConstructorAnnotation() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/annotations/primaryConstructorAnnotation.java"); diff --git a/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterSingleFileTestGenerated.java b/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterSingleFileTestGenerated.java index ccf60c6caf6..fa1c1d58255 100644 --- a/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterSingleFileTestGenerated.java +++ b/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterSingleFileTestGenerated.java @@ -56,6 +56,12 @@ public class JavaToKotlinConverterSingleFileTestGenerated extends AbstractJavaTo doTest(fileName); } + @TestMetadata("annotationFromConstructorParamToProperty.java") + public void testAnnotationFromConstructorParamToProperty() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/annotations/annotationFromConstructorParamToProperty.java"); + doTest(fileName); + } + @TestMetadata("annotationInterface1.java") public void testAnnotationInterface1() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/annotations/annotationInterface1.java"); @@ -86,6 +92,12 @@ public class JavaToKotlinConverterSingleFileTestGenerated extends AbstractJavaTo doTest(fileName); } + @TestMetadata("annotationsFromPropertyAccessors.java") + public void testAnnotationsFromPropertyAccessors() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/annotations/annotationsFromPropertyAccessors.java"); + doTest(fileName); + } + @TestMetadata("javaClassArgument.java") public void testJavaClassArgument() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/annotations/javaClassArgument.java"); @@ -116,6 +128,12 @@ public class JavaToKotlinConverterSingleFileTestGenerated extends AbstractJavaTo doTest(fileName); } + @TestMetadata("modifiersToAnnotationsFromPropertyAccessors.java") + public void testModifiersToAnnotationsFromPropertyAccessors() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/annotations/modifiersToAnnotationsFromPropertyAccessors.java"); + doTest(fileName); + } + @TestMetadata("primaryConstructorAnnotation.java") public void testPrimaryConstructorAnnotation() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/annotations/primaryConstructorAnnotation.java");