Use AnnotationSplitter for annotations on setter parameter

Make it possible to specify annotations of the setter parameter when constructing the default setter via DescriptorFactory; pass the split annotations in DescriptorResolver.resolvePropertySetterDescriptor #KT-25500 Fixed
2018-08-07 20:10:30 +02:00
parent 779d9aafe7
commit 823a24e0a3
15 changed files with 93 additions and 82 deletions
@@ -533,21 +533,10 @@ public class FunctionCodegen {
            }

            if (kind == JvmMethodParameterKind.VALUE) {
-                ValueParameterDescriptor parameter = iterator.next();
-                AnnotationCodegen annotationCodegen = AnnotationCodegen.forParameter(i, mv, innerClassConsumer, typeMapper);
-
-                if (functionDescriptor instanceof PropertySetterDescriptor) {
-                    PropertyDescriptor propertyDescriptor = ((PropertySetterDescriptor) functionDescriptor).getCorrespondingProperty();
-                    Annotated targetedAnnotations = new AnnotatedWithOnlyTargetedAnnotations(propertyDescriptor);
-                    annotationCodegen.genAnnotations(targetedAnnotations, parameterSignature.getAsmType(), SETTER_PARAMETER);
-                }
-
-                if (functionDescriptor instanceof ConstructorDescriptor) {
-                    annotationCodegen.genAnnotations(parameter, parameterSignature.getAsmType(), CONSTRUCTOR_PARAMETER);
-                }
-                else {
-                    annotationCodegen.genAnnotations(parameter, parameterSignature.getAsmType());
-                }
+                AnnotationCodegen.forParameter(i, mv, innerClassConsumer, typeMapper).genAnnotations(
+                        iterator.next(),
+                        parameterSignature.getAsmType()
+                );
            }
            else if (kind == JvmMethodParameterKind.RECEIVER) {
                ReceiverParameterDescriptor receiver = JvmCodegenUtil.getDirectMember(functionDescriptor).getExtensionReceiverParameter();
@@ -503,12 +503,18 @@ public class PropertyCodegen {
                                    : DescriptorFactory.createDefaultGetter(descriptor, Annotations.Companion.getEMPTY()));
    }

-    public void generateSetter(@Nullable KtNamedDeclaration p, @NotNull PropertyDescriptor descriptor, @Nullable KtPropertyAccessor setter) {
+    public void generateSetter(
+            @Nullable KtNamedDeclaration p,
+            @NotNull PropertyDescriptor descriptor,
+            @Nullable KtPropertyAccessor setter
+    ) {
        if (!descriptor.isVar()) return;

        generateAccessor(p, setter, descriptor.getSetter() != null
                                    ? descriptor.getSetter()
-                                    : DescriptorFactory.createDefaultSetter(descriptor, Annotations.Companion.getEMPTY()));
+                                    : DescriptorFactory.createDefaultSetter(
+                                            descriptor, Annotations.Companion.getEMPTY(), Annotations.Companion.getEMPTY()
+                                    ));
    }

    private void generateAccessor(
@@ -76,7 +76,10 @@ class AnnotationSplitter(
                add(PROPERTY_SETTER)
            }
            if (hasBackingField) add(FIELD)
-            if (isVar) add(PROPERTY_SETTER)
+            if (isVar) {
+                add(PROPERTY_SETTER)
+                add(SETTER_PARAMETER)
+            }
            if (hasDelegate) add(PROPERTY_DELEGATE_FIELD)
        }
    }
@@ -299,7 +299,8 @@ public class DescriptorResolver {
            @NotNull KtParameter valueParameter,
            int index,
            @NotNull KotlinType type,
-            @NotNull BindingTrace trace
+            @NotNull BindingTrace trace,
+            @NotNull Annotations additionalAnnotations
    ) {
        KotlinType varargElementType = null;
        KotlinType variableType = type;
@@ -308,22 +309,7 @@ public class DescriptorResolver {
            variableType = getVarargParameterType(type);
        }

-        KtModifierList modifierList = valueParameter.getModifierList();
-
-        Annotations allAnnotations =
-                annotationResolver.resolveAnnotationsWithoutArguments(scope, valueParameter.getModifierList(), trace);
-        Annotations valueParameterAnnotations = Annotations.Companion.getEMPTY();
-
-        if (modifierList != null) {
-            if (valueParameter.hasValOrVar()) {
-                AnnotationSplitter annotationSplitter = AnnotationSplitter.create(
-                        storageManager, allAnnotations, SetsKt.setOf(CONSTRUCTOR_PARAMETER));
-                valueParameterAnnotations = annotationSplitter.getAnnotationsForTarget(CONSTRUCTOR_PARAMETER);
-            }
-            else {
-                valueParameterAnnotations = allAnnotations;
-            }
-        }
+        Annotations valueParameterAnnotations = resolveValueParameterAnnotations(scope, valueParameter, trace, additionalAnnotations);

        KtDestructuringDeclaration destructuringDeclaration = valueParameter.getDestructuringDeclaration();

@@ -392,6 +378,27 @@ public class DescriptorResolver {
        return valueParameterDescriptor;
    }

+    @NotNull
+    private Annotations resolveValueParameterAnnotations(
+            @NotNull LexicalScope scope,
+            @NotNull KtParameter parameter,
+            @NotNull BindingTrace trace,
+            @NotNull Annotations additionalAnnotations
+    ) {
+        KtModifierList modifierList = parameter.getModifierList();
+        if (modifierList == null) {
+            return additionalAnnotations;
+        }
+
+        Annotations allAnnotations = annotationResolver.resolveAnnotationsWithoutArguments(scope, modifierList, trace);
+        if (!parameter.hasValOrVar()) {
+            return new CompositeAnnotations(allAnnotations, additionalAnnotations);
+        }
+
+        AnnotationSplitter splitter = AnnotationSplitter.create(storageManager, allAnnotations, SetsKt.setOf(CONSTRUCTOR_PARAMETER));
+        return new CompositeAnnotations(splitter.getAnnotationsForTarget(CONSTRUCTOR_PARAMETER), additionalAnnotations);
+    }
+
    @NotNull
    private KotlinType getVarargParameterType(@NotNull KotlinType elementType) {
        KotlinType primitiveArrayType = builtIns.getPrimitiveArrayKotlinTypeByPrimitiveKotlinType(elementType);
@@ -1068,8 +1075,10 @@ public class DescriptorResolver {
                    }
                }

-                ValueParameterDescriptorImpl valueParameterDescriptor =
-                        resolveValueParameterDescriptor(scopeWithTypeParameters, setterDescriptor, parameter, 0, type, trace);
+                ValueParameterDescriptorImpl valueParameterDescriptor = resolveValueParameterDescriptor(
+                        scopeWithTypeParameters, setterDescriptor, parameter, 0, type, trace,
+                        annotationSplitter.getAnnotationsForTarget(SETTER_PARAMETER)
+                );
                setterDescriptor.initialize(valueParameterDescriptor);
            }
            else {
@@ -1079,15 +1088,17 @@ public class DescriptorResolver {
            trace.record(BindingContext.PROPERTY_ACCESSOR, setter, setterDescriptor);
        }
        else if (property.isVar()) {
-            Annotations setterAnnotations = annotationSplitter.getAnnotationsForTarget(PROPERTY_SETTER);
-            setterDescriptor = DescriptorFactory.createSetter(propertyDescriptor, setterAnnotations, !hasDelegate,
-                                                              /* isExternal = */ false, property.hasModifier(KtTokens.INLINE_KEYWORD),
-                                                              propertyDescriptor.getSource());
+            setterDescriptor = DescriptorFactory.createSetter(
+                    propertyDescriptor,
+                    annotationSplitter.getAnnotationsForTarget(PROPERTY_SETTER),
+                    annotationSplitter.getAnnotationsForTarget(SETTER_PARAMETER),
+                    !hasDelegate, false, property.hasModifier(KtTokens.INLINE_KEYWORD),
+                    propertyDescriptor.getSource()
+            );
        }

        if (!property.isVar()) {
            if (setter != null) {
-                //                trace.getErrorHandler().genericError(setter.asElement().getNode(), "A 'val'-property cannot have a setter");
                trace.report(VAL_WITH_SETTER.on(setter));
            }
        }
@@ -1238,7 +1249,11 @@ public class DescriptorResolver {

        PropertyGetterDescriptorImpl getter = DescriptorFactory.createDefaultGetter(propertyDescriptor, getterAnnotations);
        PropertySetterDescriptor setter =
-                propertyDescriptor.isVar() ? DescriptorFactory.createDefaultSetter(propertyDescriptor, setterAnnotations) : null;
+                propertyDescriptor.isVar()
+                ? DescriptorFactory.createDefaultSetter(
+                        propertyDescriptor, setterAnnotations, annotationSplitter.getAnnotationsForTarget(SETTER_PARAMETER)
+                )
+                : null;

        propertyDescriptor.initialize(getter, setter);
        getter.initialize(propertyDescriptor.getType());
@@ -438,8 +438,7 @@ class FunctionDescriptorResolver(
            }

            val valueParameterDescriptor = descriptorResolver.resolveValueParameterDescriptor(
-                parameterScope, functionDescriptor,
-                valueParameter, i, type, trace
+                parameterScope, functionDescriptor, valueParameter, i, type, trace, Annotations.EMPTY
            )

            // Do not report NAME_SHADOWING for lambda destructured parameters as they may be not fully resolved at this time
@@ -205,7 +205,7 @@ class LocalVariableResolver(

        var setter = propertyDescriptor.setter
        if (setter == null && propertyDescriptor.isVar) {
-            setter = DescriptorFactory.createDefaultSetter(propertyDescriptor, Annotations.EMPTY)
+            setter = DescriptorFactory.createDefaultSetter(propertyDescriptor, Annotations.EMPTY, Annotations.EMPTY)
        }
        propertyDescriptor.initialize(getter, setter)
    }
@@ -109,7 +109,7 @@ class DynamicCallableDescriptors(storageManager: StorageManager, builtIns: Kotli

        val getter = DescriptorFactory.createDefaultGetter(propertyDescriptor, Annotations.EMPTY)
        getter.initialize(propertyDescriptor.type)
-        val setter = DescriptorFactory.createDefaultSetter(propertyDescriptor, Annotations.EMPTY)
+        val setter = DescriptorFactory.createDefaultSetter(propertyDescriptor, Annotations.EMPTY, Annotations.EMPTY)

        propertyDescriptor.initialize(getter, setter)

@@ -5,7 +5,6 @@

 package org.jetbrains.kotlin.psi2ir.generators

-import org.jetbrains.kotlin.descriptors.PropertySetterDescriptor
 import org.jetbrains.kotlin.descriptors.annotations.AnnotationUseSiteTarget
 import org.jetbrains.kotlin.descriptors.annotations.AnnotationWithTarget
 import org.jetbrains.kotlin.ir.IrElement
@@ -13,10 +12,7 @@ import org.jetbrains.kotlin.ir.declarations.*
 import org.jetbrains.kotlin.ir.visitors.IrElementVisitorVoid
 import org.jetbrains.kotlin.ir.visitors.acceptChildrenVoid

-class AnnotationGenerator(
-    context: GeneratorContext
-) : IrElementVisitorVoid {
-
+class AnnotationGenerator(context: GeneratorContext) : IrElementVisitorVoid {
    private val typeTranslator = context.typeTranslator
    private val constantValueGenerator = context.constantValueGenerator

@@ -38,16 +34,7 @@ class AnnotationGenerator(
    override fun visitValueParameter(declaration: IrValueParameter) {
        super.visitValueParameter(declaration)

-        val descriptor = declaration.descriptor
-        val containingDeclaration = descriptor.containingDeclaration
-
-        if (containingDeclaration is PropertySetterDescriptor) {
-            containingDeclaration.correspondingProperty.annotations.getUseSiteTargetedAnnotations()
-                .filter { it.target == AnnotationUseSiteTarget.SETTER_PARAMETER }
-                .generateAnnotationConstructorCalls(declaration)
-        }
-
-        descriptor.type.annotations.getAllAnnotations()
+        declaration.descriptor.type.annotations.getAllAnnotations()
            .filter { it.target == AnnotationUseSiteTarget.RECEIVER }
            .generateAnnotationConstructorCalls(declaration)
    }
@@ -81,5 +68,3 @@ class AnnotationGenerator(
            else -> target == null
        }
 }
-
-
@@ -171,7 +171,7 @@ class Delegate {
    fun test() {
        check(::delegate.getter, annotationExists = true)
        check(::delegate.setter, annotationExists = true)
-        check(::delegate.setter.parameters.first(), annotationExists = false) // https://youtrack.jetbrains.com/issue/KT-25500
+        check(::delegate.setter.parameters.first(), annotationExists = true)
    }

    class CustomDelegate {
@@ -529,14 +529,15 @@ class LazyJavaClassMemberScope(
            initialize(propertyDescriptor.type)
        }

-        val setter = setterMethod?.let { setterMethod ->
+        val setter = if (setterMethod != null) {
+            val parameter = setterMethod.valueParameters.firstOrNull() ?: throw AssertionError("No parameter found for $setterMethod")
            DescriptorFactory.createSetter(
-                propertyDescriptor, setterMethod.annotations, /* isDefault = */false,
+                propertyDescriptor, setterMethod.annotations, parameter.annotations, /* isDefault = */false,
                /* isExternal = */ false, /* isInline = */ false, setterMethod.visibility, setterMethod.source
            ).apply {
                initialSignatureDescriptor = setterMethod
            }
-        }
+        } else null

        return propertyDescriptor.apply { initialize(getter, setter) }
    }
@@ -399,7 +399,11 @@ public class PropertyDescriptorImpl extends VariableDescriptorWithInitializerImp
                // it can not be assigned to because of the projection
                substitutedDescriptor.setSetterProjectedOut(true);
                substitutedValueParameters = Collections.<ValueParameterDescriptor>singletonList(
-                        PropertySetterDescriptorImpl.createSetterParameter(newSetter, getBuiltIns(copyConfiguration.owner).getNothingType())
+                        PropertySetterDescriptorImpl.createSetterParameter(
+                                newSetter,
+                                getBuiltIns(copyConfiguration.owner).getNothingType(),
+                                setter.getValueParameters().get(0).getAnnotations()
+                        )
                );
            }
            if (substitutedValueParameters.size() != 1) {
@@ -58,16 +58,16 @@ public class PropertySetterDescriptorImpl extends PropertyAccessorDescriptorImpl
    }

    public void initializeDefault() {
-        assert parameter == null;
-        parameter = createSetterParameter(this, getCorrespondingProperty().getReturnType());
+        initialize(createSetterParameter(this, getCorrespondingProperty().getType(), Annotations.Companion.getEMPTY()));
    }

    public static ValueParameterDescriptorImpl createSetterParameter(
            @NotNull PropertySetterDescriptor setterDescriptor,
-            @NotNull KotlinType type
+            @NotNull KotlinType type,
+            @NotNull Annotations annotations
    ) {
        return new ValueParameterDescriptorImpl(
-                setterDescriptor, null, 0, Annotations.Companion.getEMPTY(), Name.special("<set-?>"), type,
+                setterDescriptor, null, 0, annotations, Name.special("<set-?>"), type,
                /* declaresDefaultValue = */ false,
                /* isCrossinline = */ false,
                /* isNoinline = */ false,
@@ -46,27 +46,33 @@ public class DescriptorFactory {
    @NotNull
    public static PropertySetterDescriptorImpl createDefaultSetter(
            @NotNull PropertyDescriptor propertyDescriptor,
-            @NotNull Annotations annotations
+            @NotNull Annotations annotations,
+            @NotNull Annotations parameterAnnotations
    ) {
-        return createSetter(propertyDescriptor, annotations, true, false, false, propertyDescriptor.getSource());
+        return createSetter(propertyDescriptor, annotations, parameterAnnotations, true, false, false, propertyDescriptor.getSource());
    }

    @NotNull
    public static PropertySetterDescriptorImpl createSetter(
            @NotNull PropertyDescriptor propertyDescriptor,
            @NotNull Annotations annotations,
+            @NotNull Annotations parameterAnnotations,
            boolean isDefault,
            boolean isExternal,
            boolean isInline,
            @NotNull SourceElement sourceElement
    ) {
-        return createSetter(propertyDescriptor, annotations, isDefault, isExternal, isInline, propertyDescriptor.getVisibility(), sourceElement);
+        return createSetter(
+                propertyDescriptor, annotations, parameterAnnotations, isDefault, isExternal, isInline,
+                propertyDescriptor.getVisibility(), sourceElement
+        );
    }

    @NotNull
    public static PropertySetterDescriptorImpl createSetter(
            @NotNull PropertyDescriptor propertyDescriptor,
            @NotNull Annotations annotations,
+            @NotNull Annotations parameterAnnotations,
            boolean isDefault,
            boolean isExternal,
            boolean isInline,
@@ -77,7 +83,9 @@ public class DescriptorFactory {
                propertyDescriptor, annotations, propertyDescriptor.getModality(), visibility, isDefault, isExternal,
                isInline, CallableMemberDescriptor.Kind.DECLARATION, null, sourceElement
        );
-        setterDescriptor.initializeDefault();
+        ValueParameterDescriptorImpl parameter =
+                PropertySetterDescriptorImpl.createSetterParameter(setterDescriptor, propertyDescriptor.getType(), parameterAnnotations);
+        setterDescriptor.initialize(parameter);
        return setterDescriptor;
    }

@@ -118,7 +118,10 @@ class MemberDeserializer(private val c: DeserializationContext) {
                setter.initialize(valueParameters.single())
                setter
            } else {
-                DescriptorFactory.createDefaultSetter(property, annotations)
+                DescriptorFactory.createDefaultSetter(
+                    property, annotations,
+                    Annotations.EMPTY /* Otherwise presumably the setter is not default */
+                )
            }
        } else {
            null
@@ -8,11 +8,10 @@ package kotlin.reflect.jvm.internal
 import org.jetbrains.kotlin.descriptors.*
 import org.jetbrains.kotlin.descriptors.annotations.Annotations
 import org.jetbrains.kotlin.load.java.JvmAbi
-import org.jetbrains.kotlin.metadata.deserialization.getExtensionOrNull
-import org.jetbrains.kotlin.metadata.jvm.JvmProtoBuf
 import org.jetbrains.kotlin.metadata.jvm.deserialization.JvmProtoBufUtil
 import org.jetbrains.kotlin.resolve.DescriptorFactory
 import org.jetbrains.kotlin.resolve.DescriptorUtils
+import org.jetbrains.kotlin.serialization.deserialization.descriptors.DeserializedPropertyDescriptor
 import org.jetbrains.kotlin.types.TypeUtils
 import java.lang.reflect.Field
 import java.lang.reflect.Modifier
@@ -22,7 +21,6 @@ import kotlin.reflect.KMutableProperty
 import kotlin.reflect.KProperty
 import kotlin.reflect.full.IllegalPropertyDelegateAccessException
 import kotlin.reflect.jvm.internal.JvmPropertySignature.*
-import org.jetbrains.kotlin.serialization.deserialization.descriptors.DeserializedPropertyDescriptor

 internal abstract class KPropertyImpl<out R> private constructor(
    override val container: KDeclarationContainerImpl,
@@ -159,7 +157,7 @@ internal abstract class KPropertyImpl<out R> private constructor(

        override val descriptor: PropertySetterDescriptor by ReflectProperties.lazySoft {
            // TODO: default setter created this way won't have any source information
-            property.descriptor.setter ?: DescriptorFactory.createDefaultSetter(property.descriptor, Annotations.EMPTY)
+            property.descriptor.setter ?: DescriptorFactory.createDefaultSetter(property.descriptor, Annotations.EMPTY, Annotations.EMPTY)
        }

        override val caller: FunctionCaller<*> by ReflectProperties.lazySoft {