Fix parameters nullability for generated overloads in light classes
When making KtLightNullabilityAnnotation after test org.jetbrains.kotlin.idea.caches.resolve.IdeLightClassTestGenerated.NullabilityAnnotations#testJvmOverloads started failing the wrong assumption was made that for @JvmOverloads-generated overloads their last parameter is always nullable (see removed isNullableInJvmOverloads function) and that lead to a bug, namely KT-28556. The actual problem of this test started failing was incorrect definition of kotlinOrigin in KtLightParameter for case of JvmOverloads: wrong KtParameter was being chosen before This commit fixes the issue by propagating how actually generated parameters in codegen relate to source KtParameters' ^KT-28556 Fixed
This commit is contained in:
@@ -231,7 +231,9 @@ public abstract class ClassBodyCodegen extends MemberCodegen<KtPureClassOrObject
|
|||||||
private void generateDelegationToDefaultImpl(@NotNull FunctionDescriptor interfaceFun, @NotNull FunctionDescriptor inheritedFun) {
|
private void generateDelegationToDefaultImpl(@NotNull FunctionDescriptor interfaceFun, @NotNull FunctionDescriptor inheritedFun) {
|
||||||
|
|
||||||
functionCodegen.generateMethod(
|
functionCodegen.generateMethod(
|
||||||
new JvmDeclarationOrigin(CLASS_MEMBER_DELEGATION_TO_DEFAULT_IMPL, descriptorToDeclaration(interfaceFun), interfaceFun),
|
new JvmDeclarationOrigin(
|
||||||
|
CLASS_MEMBER_DELEGATION_TO_DEFAULT_IMPL, descriptorToDeclaration(interfaceFun), interfaceFun, null
|
||||||
|
),
|
||||||
inheritedFun,
|
inheritedFun,
|
||||||
new FunctionGenerationStrategy.CodegenBased(state) {
|
new FunctionGenerationStrategy.CodegenBased(state) {
|
||||||
@Override
|
@Override
|
||||||
|
|||||||
+12
-2
@@ -21,13 +21,16 @@ import org.jetbrains.kotlin.codegen.binding.CodegenBinding
|
|||||||
import org.jetbrains.kotlin.codegen.state.GenerationState
|
import org.jetbrains.kotlin.codegen.state.GenerationState
|
||||||
import org.jetbrains.kotlin.descriptors.*
|
import org.jetbrains.kotlin.descriptors.*
|
||||||
import org.jetbrains.kotlin.psi.KtClass
|
import org.jetbrains.kotlin.psi.KtClass
|
||||||
|
import org.jetbrains.kotlin.psi.KtParameter
|
||||||
import org.jetbrains.kotlin.psi.KtPureClassOrObject
|
import org.jetbrains.kotlin.psi.KtPureClassOrObject
|
||||||
import org.jetbrains.kotlin.psi.KtPureElement
|
import org.jetbrains.kotlin.psi.KtPureElement
|
||||||
|
import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils
|
||||||
import org.jetbrains.kotlin.resolve.calls.components.hasDefaultValue
|
import org.jetbrains.kotlin.resolve.calls.components.hasDefaultValue
|
||||||
import org.jetbrains.kotlin.resolve.isInlineClass
|
import org.jetbrains.kotlin.resolve.isInlineClass
|
||||||
import org.jetbrains.kotlin.resolve.jvm.AsmTypes
|
import org.jetbrains.kotlin.resolve.jvm.AsmTypes
|
||||||
import org.jetbrains.kotlin.resolve.jvm.annotations.findJvmOverloadsAnnotation
|
import org.jetbrains.kotlin.resolve.jvm.annotations.findJvmOverloadsAnnotation
|
||||||
import org.jetbrains.kotlin.resolve.jvm.diagnostics.OtherOriginFromPure
|
import org.jetbrains.kotlin.resolve.jvm.diagnostics.JvmDeclarationOrigin
|
||||||
|
import org.jetbrains.kotlin.resolve.jvm.diagnostics.JvmDeclarationOriginKind
|
||||||
import org.jetbrains.org.objectweb.asm.Label
|
import org.jetbrains.org.objectweb.asm.Label
|
||||||
import org.jetbrains.org.objectweb.asm.Opcodes
|
import org.jetbrains.org.objectweb.asm.Opcodes
|
||||||
import org.jetbrains.org.objectweb.asm.Type
|
import org.jetbrains.org.objectweb.asm.Type
|
||||||
@@ -126,6 +129,9 @@ class DefaultParameterValueSubstitutor(val state: GenerationState) {
|
|||||||
val isStatic = AsmUtil.isStaticMethod(contextKind, functionDescriptor)
|
val isStatic = AsmUtil.isStaticMethod(contextKind, functionDescriptor)
|
||||||
val baseMethodFlags = AsmUtil.getCommonCallableFlags(functionDescriptor, state) and Opcodes.ACC_VARARGS.inv()
|
val baseMethodFlags = AsmUtil.getCommonCallableFlags(functionDescriptor, state) and Opcodes.ACC_VARARGS.inv()
|
||||||
val remainingParameters = getRemainingParameters(functionDescriptor.original, substituteCount)
|
val remainingParameters = getRemainingParameters(functionDescriptor.original, substituteCount)
|
||||||
|
val remainingParametersDeclarations =
|
||||||
|
remainingParameters.map { DescriptorToSourceUtils.descriptorToDeclaration(it) as? KtParameter }
|
||||||
|
|
||||||
val flags =
|
val flags =
|
||||||
baseMethodFlags or
|
baseMethodFlags or
|
||||||
(if (isStatic) Opcodes.ACC_STATIC else 0) or
|
(if (isStatic) Opcodes.ACC_STATIC else 0) or
|
||||||
@@ -133,7 +139,11 @@ class DefaultParameterValueSubstitutor(val state: GenerationState) {
|
|||||||
(if (remainingParameters.lastOrNull()?.varargElementType != null) Opcodes.ACC_VARARGS else 0)
|
(if (remainingParameters.lastOrNull()?.varargElementType != null) Opcodes.ACC_VARARGS else 0)
|
||||||
val signature = typeMapper.mapSignatureWithCustomParameters(functionDescriptor, contextKind, remainingParameters, false)
|
val signature = typeMapper.mapSignatureWithCustomParameters(functionDescriptor, contextKind, remainingParameters, false)
|
||||||
val mv = classBuilder.newMethod(
|
val mv = classBuilder.newMethod(
|
||||||
OtherOriginFromPure(methodElement, functionDescriptor), flags,
|
JvmDeclarationOrigin(
|
||||||
|
JvmDeclarationOriginKind.JVM_OVERLOADS, methodElement?.psiOrParent, functionDescriptor,
|
||||||
|
remainingParametersDeclarations
|
||||||
|
),
|
||||||
|
flags,
|
||||||
signature.asmMethod.name,
|
signature.asmMethod.name,
|
||||||
signature.asmMethod.descriptor,
|
signature.asmMethod.descriptor,
|
||||||
signature.genericsSignature,
|
signature.genericsSignature,
|
||||||
|
|||||||
+5
-2
@@ -8,6 +8,7 @@ package org.jetbrains.kotlin.resolve.jvm.diagnostics
|
|||||||
import com.intellij.psi.PsiElement
|
import com.intellij.psi.PsiElement
|
||||||
import org.jetbrains.kotlin.descriptors.*
|
import org.jetbrains.kotlin.descriptors.*
|
||||||
import org.jetbrains.kotlin.psi.KtFile
|
import org.jetbrains.kotlin.psi.KtFile
|
||||||
|
import org.jetbrains.kotlin.psi.KtParameter
|
||||||
import org.jetbrains.kotlin.psi.KtPureElement
|
import org.jetbrains.kotlin.psi.KtPureElement
|
||||||
import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils
|
import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils
|
||||||
import org.jetbrains.kotlin.resolve.jvm.diagnostics.JvmDeclarationOriginKind.*
|
import org.jetbrains.kotlin.resolve.jvm.diagnostics.JvmDeclarationOriginKind.*
|
||||||
@@ -31,13 +32,15 @@ enum class JvmDeclarationOriginKind {
|
|||||||
COLLECTION_STUB,
|
COLLECTION_STUB,
|
||||||
AUGMENTED_BUILTIN_API,
|
AUGMENTED_BUILTIN_API,
|
||||||
ERASED_INLINE_CLASS,
|
ERASED_INLINE_CLASS,
|
||||||
UNBOX_METHOD_OF_INLINE_CLASS
|
UNBOX_METHOD_OF_INLINE_CLASS,
|
||||||
|
JVM_OVERLOADS
|
||||||
}
|
}
|
||||||
|
|
||||||
class JvmDeclarationOrigin(
|
class JvmDeclarationOrigin(
|
||||||
val originKind: JvmDeclarationOriginKind,
|
val originKind: JvmDeclarationOriginKind,
|
||||||
val element: PsiElement?,
|
val element: PsiElement?,
|
||||||
val descriptor: DeclarationDescriptor?
|
val descriptor: DeclarationDescriptor?,
|
||||||
|
val parametersForJvmOverload: List<KtParameter?>? = null
|
||||||
) {
|
) {
|
||||||
companion object {
|
companion object {
|
||||||
@JvmField
|
@JvmField
|
||||||
|
|||||||
+10
-4
@@ -19,6 +19,7 @@ package org.jetbrains.kotlin.asJava.builder
|
|||||||
import com.intellij.psi.PsiElement
|
import com.intellij.psi.PsiElement
|
||||||
import org.jetbrains.kotlin.psi.KtAnnotationEntry
|
import org.jetbrains.kotlin.psi.KtAnnotationEntry
|
||||||
import org.jetbrains.kotlin.psi.KtDeclaration
|
import org.jetbrains.kotlin.psi.KtDeclaration
|
||||||
|
import org.jetbrains.kotlin.psi.KtParameter
|
||||||
import org.jetbrains.kotlin.resolve.jvm.diagnostics.JvmDeclarationOrigin
|
import org.jetbrains.kotlin.resolve.jvm.diagnostics.JvmDeclarationOrigin
|
||||||
import org.jetbrains.kotlin.resolve.jvm.diagnostics.JvmDeclarationOriginKind
|
import org.jetbrains.kotlin.resolve.jvm.diagnostics.JvmDeclarationOriginKind
|
||||||
|
|
||||||
@@ -41,7 +42,7 @@ fun JvmDeclarationOrigin.toLightMemberOrigin(): LightElementOrigin {
|
|||||||
val originalElement = element
|
val originalElement = element
|
||||||
return when (originalElement) {
|
return when (originalElement) {
|
||||||
is KtAnnotationEntry -> DefaultLightElementOrigin(originalElement)
|
is KtAnnotationEntry -> DefaultLightElementOrigin(originalElement)
|
||||||
is KtDeclaration -> LightMemberOriginForDeclaration(originalElement, originKind)
|
is KtDeclaration -> LightMemberOriginForDeclaration(originalElement, originKind, parametersForJvmOverload)
|
||||||
else -> LightElementOrigin.None
|
else -> LightElementOrigin.None
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -49,19 +50,24 @@ fun JvmDeclarationOrigin.toLightMemberOrigin(): LightElementOrigin {
|
|||||||
interface LightMemberOrigin : LightElementOrigin {
|
interface LightMemberOrigin : LightElementOrigin {
|
||||||
override val originalElement: KtDeclaration?
|
override val originalElement: KtDeclaration?
|
||||||
override val originKind: JvmDeclarationOriginKind
|
override val originKind: JvmDeclarationOriginKind
|
||||||
|
val parametersForJvmOverloads: List<KtParameter?>? get() = null
|
||||||
|
|
||||||
fun isEquivalentTo(other: LightMemberOrigin?): Boolean
|
fun isEquivalentTo(other: LightMemberOrigin?): Boolean
|
||||||
fun copy(): LightMemberOrigin
|
fun copy(): LightMemberOrigin
|
||||||
}
|
}
|
||||||
|
|
||||||
data class LightMemberOriginForDeclaration(override val originalElement: KtDeclaration, override val originKind: JvmDeclarationOriginKind) : LightMemberOrigin {
|
data class LightMemberOriginForDeclaration(
|
||||||
|
override val originalElement: KtDeclaration,
|
||||||
|
override val originKind: JvmDeclarationOriginKind,
|
||||||
|
override val parametersForJvmOverloads: List<KtParameter?>? = null
|
||||||
|
) : LightMemberOrigin {
|
||||||
override fun isEquivalentTo(other: LightMemberOrigin?): Boolean {
|
override fun isEquivalentTo(other: LightMemberOrigin?): Boolean {
|
||||||
if (other !is LightMemberOriginForDeclaration) return false
|
if (other !is LightMemberOriginForDeclaration) return false
|
||||||
return originalElement.isEquivalentTo(other.originalElement)
|
return originalElement.isEquivalentTo(other.originalElement)
|
||||||
}
|
}
|
||||||
|
|
||||||
override fun copy(): LightMemberOrigin {
|
override fun copy(): LightMemberOrigin {
|
||||||
return LightMemberOriginForDeclaration(originalElement.copy() as KtDeclaration, originKind)
|
return LightMemberOriginForDeclaration(originalElement.copy() as KtDeclaration, originKind, parametersForJvmOverloads)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -71,4 +77,4 @@ data class DefaultLightElementOrigin(override val originalElement: PsiElement?)
|
|||||||
|
|
||||||
fun PsiElement?.toLightClassOrigin(): LightElementOrigin {
|
fun PsiElement?.toLightClassOrigin(): LightElementOrigin {
|
||||||
return if (this != null) DefaultLightElementOrigin(this) else LightElementOrigin.None
|
return if (this != null) DefaultLightElementOrigin(this) else LightElementOrigin.None
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -41,7 +41,7 @@ class KtLightParameter(
|
|||||||
if (jetIndex < 0) return null
|
if (jetIndex < 0) return null
|
||||||
|
|
||||||
if (declaration is KtFunction) {
|
if (declaration is KtFunction) {
|
||||||
val paramList = declaration.valueParameters
|
val paramList = method.lightMemberOrigin?.parametersForJvmOverloads ?: declaration.valueParameters
|
||||||
return if (jetIndex < paramList.size) paramList[jetIndex] else null
|
return if (jetIndex < paramList.size) paramList[jetIndex] else null
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+1
-10
@@ -44,7 +44,6 @@ import org.jetbrains.kotlin.resolve.calls.callUtil.getType
|
|||||||
import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall
|
import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall
|
||||||
import org.jetbrains.kotlin.resolve.calls.model.ResolvedValueArgument
|
import org.jetbrains.kotlin.resolve.calls.model.ResolvedValueArgument
|
||||||
import org.jetbrains.kotlin.resolve.descriptorUtil.declaresOrInheritsDefaultValue
|
import org.jetbrains.kotlin.resolve.descriptorUtil.declaresOrInheritsDefaultValue
|
||||||
import org.jetbrains.kotlin.resolve.jvm.annotations.findJvmOverloadsAnnotation
|
|
||||||
import org.jetbrains.kotlin.resolve.source.getPsi
|
import org.jetbrains.kotlin.resolve.source.getPsi
|
||||||
import org.jetbrains.kotlin.types.KotlinType
|
import org.jetbrains.kotlin.types.KotlinType
|
||||||
import org.jetbrains.kotlin.types.TypeUtils
|
import org.jetbrains.kotlin.types.TypeUtils
|
||||||
@@ -260,7 +259,6 @@ class KtLightNullabilityAnnotation(val member: KtLightElement<*, PsiModifierList
|
|||||||
|
|
||||||
if (annotatedElement is KtParameter) {
|
if (annotatedElement is KtParameter) {
|
||||||
if (annotatedElement.containingClassOrObject?.isAnnotation() == true) return null
|
if (annotatedElement.containingClassOrObject?.isAnnotation() == true) return null
|
||||||
if (isNullableInJvmOverloads(annotatedElement)) return Nullable::class.java.name
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// don't annotate property setters
|
// don't annotate property setters
|
||||||
@@ -289,13 +287,6 @@ class KtLightNullabilityAnnotation(val member: KtLightElement<*, PsiModifierList
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun isNullableInJvmOverloads(annotatedElement: KtParameter): Boolean {
|
|
||||||
if (annotatedElement.ownerFunction?.let { it.analyze()[BindingContext.DECLARATION_TO_DESCRIPTOR, it]?.findJvmOverloadsAnnotation() } == null) return false
|
|
||||||
val lightParameterList = (member as? PsiParameter)?.parent as? PsiParameterList ?: return false
|
|
||||||
val lastParameter = (lightParameterList.parameters.lastOrNull() as? KtLightElement<*, *>)?.kotlinOrigin
|
|
||||||
return lastParameter == annotatedElement
|
|
||||||
}
|
|
||||||
|
|
||||||
internal fun KtTypeReference.getType(): KotlinType? = analyze()[BindingContext.TYPE, this]
|
internal fun KtTypeReference.getType(): KotlinType? = analyze()[BindingContext.TYPE, this]
|
||||||
|
|
||||||
private fun getTargetType(annotatedElement: PsiElement): KotlinType? {
|
private fun getTargetType(annotatedElement: PsiElement): KotlinType? {
|
||||||
@@ -430,4 +421,4 @@ fun <T> withAllowedAnnotationsClsDelegate(body: () -> T): T {
|
|||||||
} finally {
|
} finally {
|
||||||
accessAnnotationsClsDelegateIsAllowed = prev
|
accessAnnotationsClsDelegateIsAllowed = prev
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+1
-10
@@ -43,7 +43,6 @@ import org.jetbrains.kotlin.resolve.calls.callUtil.getType
|
|||||||
import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall
|
import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall
|
||||||
import org.jetbrains.kotlin.resolve.calls.model.ResolvedValueArgument
|
import org.jetbrains.kotlin.resolve.calls.model.ResolvedValueArgument
|
||||||
import org.jetbrains.kotlin.resolve.descriptorUtil.declaresOrInheritsDefaultValue
|
import org.jetbrains.kotlin.resolve.descriptorUtil.declaresOrInheritsDefaultValue
|
||||||
import org.jetbrains.kotlin.resolve.jvm.annotations.findJvmOverloadsAnnotation
|
|
||||||
import org.jetbrains.kotlin.resolve.source.getPsi
|
import org.jetbrains.kotlin.resolve.source.getPsi
|
||||||
import org.jetbrains.kotlin.types.KotlinType
|
import org.jetbrains.kotlin.types.KotlinType
|
||||||
import org.jetbrains.kotlin.types.TypeUtils
|
import org.jetbrains.kotlin.types.TypeUtils
|
||||||
@@ -261,7 +260,6 @@ class KtLightNullabilityAnnotation(val member: KtLightElement<*, PsiModifierList
|
|||||||
|
|
||||||
if (annotatedElement is KtParameter) {
|
if (annotatedElement is KtParameter) {
|
||||||
if (annotatedElement.containingClassOrObject?.isAnnotation() == true) return null
|
if (annotatedElement.containingClassOrObject?.isAnnotation() == true) return null
|
||||||
if (isNullableInJvmOverloads(annotatedElement)) return Nullable::class.java.name
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// don't annotate property setters
|
// don't annotate property setters
|
||||||
@@ -290,13 +288,6 @@ class KtLightNullabilityAnnotation(val member: KtLightElement<*, PsiModifierList
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun isNullableInJvmOverloads(annotatedElement: KtParameter): Boolean {
|
|
||||||
if (annotatedElement.ownerFunction?.let { it.analyze()[BindingContext.DECLARATION_TO_DESCRIPTOR, it]?.findJvmOverloadsAnnotation() } == null) return false
|
|
||||||
val lightParameterList = (member as? PsiParameter)?.parent as? PsiParameterList ?: return false
|
|
||||||
val lastParameter = (lightParameterList.parameters.lastOrNull() as? KtLightElement<*, *>)?.kotlinOrigin
|
|
||||||
return lastParameter == annotatedElement
|
|
||||||
}
|
|
||||||
|
|
||||||
internal fun KtTypeReference.getType(): KotlinType? = analyze()[BindingContext.TYPE, this]
|
internal fun KtTypeReference.getType(): KotlinType? = analyze()[BindingContext.TYPE, this]
|
||||||
|
|
||||||
private fun getTargetType(annotatedElement: PsiElement): KotlinType? {
|
private fun getTargetType(annotatedElement: PsiElement): KotlinType? {
|
||||||
@@ -427,4 +418,4 @@ fun <T> withAllowedAnnotationsClsDelegate(body: () -> T): T {
|
|||||||
} finally {
|
} finally {
|
||||||
accessAnnotationsClsDelegateIsAllowed = prev
|
accessAnnotationsClsDelegateIsAllowed = prev
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+5
@@ -9,5 +9,10 @@ class JvmOverloadsFunctions {
|
|||||||
foo(a.getClass(), a, true, "Some");
|
foo(a.getClass(), a, true, "Some");
|
||||||
foo(a.getClass(), a, true);
|
foo(a.getClass(), a, true);
|
||||||
foo(a.getClass(), a);
|
foo(a.getClass(), a);
|
||||||
|
|
||||||
|
// Before KT-28556 is fixed the second not-nullable parameter wasn't marked as it shoud, so there were no warnings on it
|
||||||
|
foo(<warning descr="Passing 'null' argument to parameter annotated as @NotNull">null</warning>, <warning descr="Passing 'null' argument to parameter annotated as @NotNull">null</warning>, true, <warning descr="Passing 'null' argument to parameter annotated as @NotNull">null</warning>);
|
||||||
|
foo(<warning descr="Passing 'null' argument to parameter annotated as @NotNull">null</warning>, <warning descr="Passing 'null' argument to parameter annotated as @NotNull">null</warning>, true);
|
||||||
|
foo(<warning descr="Passing 'null' argument to parameter annotated as @NotNull">null</warning>, <warning descr="Passing 'null' argument to parameter annotated as @NotNull">null</warning>);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+2
-1
@@ -1 +1,2 @@
|
|||||||
// WITH_RUNTIME
|
// WITH_RUNTIME
|
||||||
|
// TOOL: DataFlowInspection
|
||||||
|
|||||||
Reference in New Issue
Block a user