[FIR] Introduce FirDeclarationStatus.hasStableParameterNames

This flag is true by default but is set to false for

- Java methods and constructors
- interface delegation methods that delegate to Java

The NAMED_ARGUMENTS_NOT_ALLOWED logic is mostly refactored to use the
new flag though some custom logic remains for determining the correct
message and to work around a corner case with fake overrides.

The flag is (de)serialized from/to metadata. For backward compatibility
with K1, delegated methods to Java types are deserialized as stable.

^KT-40480 Fixed
This commit is contained in:
Kirill Rakhman
2023-02-03 12:08:17 +01:00
committed by Space Team
parent 152e63b198
commit 314784f435
29 changed files with 156 additions and 51 deletions
@@ -21,7 +21,7 @@ import org.jetbrains.kotlin.analysis.api.symbols.pointers.CanNotCreateSymbolPoin
import org.jetbrains.kotlin.analysis.api.symbols.pointers.KtPsiBasedSymbolPointer
import org.jetbrains.kotlin.analysis.api.symbols.pointers.KtSymbolPointer
import org.jetbrains.kotlin.analysis.api.types.KtType
import org.jetbrains.kotlin.fir.resolve.getHasStableParameterNames
import org.jetbrains.kotlin.fir.declarations.utils.hasStableParameterNames
import org.jetbrains.kotlin.fir.symbols.impl.FirAnonymousFunctionSymbol
import org.jetbrains.kotlin.fir.symbols.impl.isExtension
@@ -46,7 +46,7 @@ internal class KtFirAnonymousFunctionSymbol(
override val hasStableParameterNames: Boolean
get() = withValidityAssertion {
firSymbol.fir.getHasStableParameterNames(firSymbol.moduleData.session)
firSymbol.fir.hasStableParameterNames
}
override val isExtension: Boolean get() = withValidityAssertion { firSymbol.isExtension }
@@ -24,8 +24,8 @@ import org.jetbrains.kotlin.analysis.api.symbols.pointers.KtSymbolPointer
import org.jetbrains.kotlin.analysis.api.types.KtType
import org.jetbrains.kotlin.descriptors.Visibility
import org.jetbrains.kotlin.fir.containingClassLookupTag
import org.jetbrains.kotlin.fir.declarations.utils.hasStableParameterNames
import org.jetbrains.kotlin.fir.declarations.utils.visibility
import org.jetbrains.kotlin.fir.resolve.getHasStableParameterNames
import org.jetbrains.kotlin.fir.symbols.impl.FirConstructorSymbol
import org.jetbrains.kotlin.name.ClassId
@@ -41,7 +41,7 @@ internal class KtFirConstructorSymbol(
override val hasStableParameterNames: Boolean
get() = withValidityAssertion {
firSymbol.fir.getHasStableParameterNames(firSymbol.moduleData.session)
firSymbol.fir.hasStableParameterNames
}
override val visibility: Visibility get() = withValidityAssertion { firSymbol.visibility }
@@ -34,7 +34,6 @@ import org.jetbrains.kotlin.descriptors.Visibility
import org.jetbrains.kotlin.fir.containingClassLookupTag
import org.jetbrains.kotlin.fir.contracts.FirEffectDeclaration
import org.jetbrains.kotlin.fir.declarations.utils.*
import org.jetbrains.kotlin.fir.resolve.getHasStableParameterNames
import org.jetbrains.kotlin.fir.symbols.impl.FirNamedFunctionSymbol
import org.jetbrains.kotlin.fir.symbols.impl.isExtension
import org.jetbrains.kotlin.name.CallableId
@@ -65,7 +64,7 @@ internal class KtFirFunctionSymbol(
override val valueParameters: List<KtValueParameterSymbol> by cached { firSymbol.createKtValueParameters(builder) }
override val hasStableParameterNames: Boolean
get() = withValidityAssertion { firSymbol.fir.getHasStableParameterNames(firSymbol.moduleData.session) }
get() = withValidityAssertion { firSymbol.fir.hasStableParameterNames }
override val annotationsList by cached {
KtFirAnnotationListForDeclaration.create(
@@ -22,7 +22,7 @@ import org.jetbrains.kotlin.analysis.api.symbols.KtTypeParameterSymbol
import org.jetbrains.kotlin.analysis.api.symbols.KtValueParameterSymbol
import org.jetbrains.kotlin.analysis.api.symbols.pointers.KtSymbolPointer
import org.jetbrains.kotlin.analysis.api.types.KtType
import org.jetbrains.kotlin.fir.resolve.getHasStableParameterNames
import org.jetbrains.kotlin.fir.declarations.utils.hasStableParameterNames
import org.jetbrains.kotlin.fir.symbols.impl.FirNamedFunctionSymbol
import org.jetbrains.kotlin.fir.symbols.impl.isExtension
import org.jetbrains.kotlin.name.CallableId
@@ -54,7 +54,7 @@ internal class KtFirSamConstructorSymbol(
override val hasStableParameterNames: Boolean
get() = withValidityAssertion {
firSymbol.fir.getHasStableParameterNames(firSymbol.moduleData.session)
firSymbol.fir.hasStableParameterNames
}
override val isExtension: Boolean get() = withValidityAssertion { firSymbol.isExtension }
@@ -22094,6 +22094,18 @@ public class DiagnosisCompilerTestFE10TestdataTestGenerated extends AbstractDiag
runTest("compiler/testData/diagnostics/tests/namedArguments/ambiguousNamedArgumentsWithGenerics3.kt");
}
@Test
@TestMetadata("disallowForDelegationToJavaMethods.kt")
public void testDisallowForDelegationToJavaMethods() throws Exception {
runTest("compiler/testData/diagnostics/tests/namedArguments/disallowForDelegationToJavaMethods.kt");
}
@Test
@TestMetadata("disallowForFunctionTypes.kt")
public void testDisallowForFunctionTypes() throws Exception {
runTest("compiler/testData/diagnostics/tests/namedArguments/disallowForFunctionTypes.kt");
}
@Test
@TestMetadata("disallowForJavaConstructor.kt")
public void testDisallowForJavaConstructor() throws Exception {
@@ -22100,6 +22100,18 @@ public class FirOldFrontendDiagnosticsTestGenerated extends AbstractFirDiagnosti
runTest("compiler/testData/diagnostics/tests/namedArguments/ambiguousNamedArgumentsWithGenerics3.kt");
}
@Test
@TestMetadata("disallowForDelegationToJavaMethods.kt")
public void testDisallowForDelegationToJavaMethods() throws Exception {
runTest("compiler/testData/diagnostics/tests/namedArguments/disallowForDelegationToJavaMethods.kt");
}
@Test
@TestMetadata("disallowForFunctionTypes.kt")
public void testDisallowForFunctionTypes() throws Exception {
runTest("compiler/testData/diagnostics/tests/namedArguments/disallowForFunctionTypes.kt");
}
@Test
@TestMetadata("disallowForJavaConstructor.kt")
public void testDisallowForJavaConstructor() throws Exception {
@@ -22094,6 +22094,18 @@ public class FirOldFrontendDiagnosticsWithLightTreeTestGenerated extends Abstrac
runTest("compiler/testData/diagnostics/tests/namedArguments/ambiguousNamedArgumentsWithGenerics3.kt");
}
@Test
@TestMetadata("disallowForDelegationToJavaMethods.kt")
public void testDisallowForDelegationToJavaMethods() throws Exception {
runTest("compiler/testData/diagnostics/tests/namedArguments/disallowForDelegationToJavaMethods.kt");
}
@Test
@TestMetadata("disallowForFunctionTypes.kt")
public void testDisallowForFunctionTypes() throws Exception {
runTest("compiler/testData/diagnostics/tests/namedArguments/disallowForFunctionTypes.kt");
}
@Test
@TestMetadata("disallowForJavaConstructor.kt")
public void testDisallowForJavaConstructor() throws Exception {
@@ -502,6 +502,7 @@ class FirMemberDeserializer(private val c: FirDeserializationContext) {
isTailRec = Flags.IS_TAILREC.get(flags)
isExternal = Flags.IS_EXTERNAL_FUNCTION.get(flags)
isSuspend = Flags.IS_SUSPEND.get(flags)
hasStableParameterNames = !Flags.IS_FUNCTION_WITH_NON_STABLE_PARAMETER_NAMES.get(flags)
}
this.symbol = symbol
dispatchReceiverType = c.dispatchReceiver
@@ -571,6 +572,7 @@ class FirMemberDeserializer(private val c: FirDeserializationContext) {
visibility.toEffectiveVisibility(classBuilder.symbol)
).apply {
isExpect = Flags.IS_EXPECT_FUNCTION.get(flags)
hasStableParameterNames = !Flags.IS_CONSTRUCTOR_WITH_NON_STABLE_PARAMETER_NAMES.get(flags)
isActual = false
isOverride = false
this.isInner = isInner
@@ -433,8 +433,9 @@ class FirElementSerializer private constructor(
simpleFunction?.isExternal == true,
simpleFunction?.isSuspend == true,
simpleFunction?.isExpect == true,
true // TODO: supply 'hasStableParameterNames' flag for metadata
shouldSetStableParameterNames(simpleFunction),
)
if (flags != builder.flags) {
builder.flags = flags
}
@@ -507,6 +508,15 @@ class FirElementSerializer private constructor(
return builder
}
private fun shouldSetStableParameterNames(simpleFunction: FirSimpleFunction?): Boolean {
return when {
simpleFunction?.hasStableParameterNames == true -> true
// for backward compatibility with K1, remove this line to fix KT-4758
simpleFunction?.origin == FirDeclarationOrigin.Delegated -> true
else -> false
}
}
private fun typeAliasProto(typeAlias: FirTypeAlias): ProtoBuf.TypeAlias.Builder? = whileAnalysing(session, typeAlias) {
if (!extension.shouldSerializeTypeAlias(typeAlias)) return null
@@ -570,7 +580,7 @@ class FirElementSerializer private constructor(
constructor.nonSourceAnnotations(session).isNotEmpty(),
ProtoEnumFlags.visibility(normalizeVisibility(constructor)),
!constructor.isPrimary,
true // TODO: supply 'hasStableParameterNames' flag for metadata
constructor.hasStableParameterNames,
)
if (flags != builder.flags) {
builder.flags = flags
@@ -572,6 +572,7 @@ abstract class FirJavaFacade(
// Approximation: all Java methods with name that allows to use it in operator form are considered operators
// We need here more detailed checks (see modifierChecks.kt)
isOperator = name in ALL_JAVA_OPERATION_NAMES || OperatorNameConventions.COMPONENT_REGEX.matches(name.asString())
hasStableParameterNames = false
}
if (!javaMethod.isStatic) {
@@ -628,6 +629,7 @@ abstract class FirJavaFacade(
visibility.toEffectiveVisibility(ownerClassBuilder.symbol)
).apply {
isInner = isThisInner
hasStableParameterNames = false
}
this.visibility = visibility
isPrimary = javaConstructor == null
@@ -21,10 +21,7 @@ import org.jetbrains.kotlin.fir.declarations.impl.FirDeclarationStatusImpl
import org.jetbrains.kotlin.fir.declarations.impl.FirResolvedDeclarationStatusImpl
import org.jetbrains.kotlin.fir.declarations.synthetic.FirSyntheticProperty
import org.jetbrains.kotlin.fir.declarations.synthetic.buildSyntheticProperty
import org.jetbrains.kotlin.fir.declarations.utils.isInner
import org.jetbrains.kotlin.fir.declarations.utils.isStatic
import org.jetbrains.kotlin.fir.declarations.utils.modality
import org.jetbrains.kotlin.fir.declarations.utils.visibility
import org.jetbrains.kotlin.fir.declarations.utils.*
import org.jetbrains.kotlin.fir.expressions.FirAnnotation
import org.jetbrains.kotlin.fir.expressions.FirConstExpression
import org.jetbrains.kotlin.fir.expressions.unexpandedClassId
@@ -262,6 +259,8 @@ class FirSignatureEnhancement(
FirDeclarationStatusImpl(firMethod.visibility, Modality.FINAL)
}.apply {
isInner = firMethod.isInner
// Java annotation class constructors have stable names, copy flag.
hasStableParameterNames = firMethod.hasStableParameterNames
}
this.symbol = symbol
dispatchReceiverType = firMethod.dispatchReceiverType
@@ -74,13 +74,16 @@ abstract class FirSyntheticFunctionInterfaceProviderBase(
}
@FirSymbolProviderInternals
override fun getTopLevelCallableSymbolsTo(destination: MutableList<FirCallableSymbol<*>>, packageFqName: FqName, name: Name) {}
override fun getTopLevelCallableSymbolsTo(destination: MutableList<FirCallableSymbol<*>>, packageFqName: FqName, name: Name) {
}
@FirSymbolProviderInternals
override fun getTopLevelFunctionSymbolsTo(destination: MutableList<FirNamedFunctionSymbol>, packageFqName: FqName, name: Name) {}
override fun getTopLevelFunctionSymbolsTo(destination: MutableList<FirNamedFunctionSymbol>, packageFqName: FqName, name: Name) {
}
@FirSymbolProviderInternals
override fun getTopLevelPropertySymbolsTo(destination: MutableList<FirPropertySymbol>, packageFqName: FqName, name: Name) {}
override fun getTopLevelPropertySymbolsTo(destination: MutableList<FirPropertySymbol>, packageFqName: FqName, name: Name) {
}
override fun getPackage(fqName: FqName): FqName? {
return fqName.takeIf { session.functionTypeService.hasKindWithSpecificPackage(it) }
@@ -162,6 +165,7 @@ abstract class FirSyntheticFunctionInterfaceProviderBase(
).apply {
isOperator = true
isSuspend = kind.isSuspendOrKSuspendFunction
hasStableParameterNames = false
}
val typeArguments = typeParameters.map {
ConeTypeParameterTypeImpl(it.symbol.toLookupTag(), false).toFirResolvedTypeRef()
@@ -14,10 +14,7 @@ import org.jetbrains.kotlin.descriptors.Modality
import org.jetbrains.kotlin.fakeElement
import org.jetbrains.kotlin.fir.*
import org.jetbrains.kotlin.fir.declarations.*
import org.jetbrains.kotlin.fir.declarations.utils.canNarrowDownGetterType
import org.jetbrains.kotlin.fir.declarations.utils.expandedConeType
import org.jetbrains.kotlin.fir.declarations.utils.isFinal
import org.jetbrains.kotlin.fir.declarations.utils.modality
import org.jetbrains.kotlin.fir.declarations.utils.*
import org.jetbrains.kotlin.fir.diagnostics.*
import org.jetbrains.kotlin.fir.expressions.*
import org.jetbrains.kotlin.fir.expressions.builder.*
@@ -560,20 +557,14 @@ fun FirFunction.getAsForbiddenNamedArgumentsTarget(
// for intersection/substitution overrides
originScope: FirTypeScope? = null
): ForbiddenNamedArgumentsTarget? {
if (this is FirConstructor && this.isPrimary) {
this.getContainingClass(session)?.let { containingClass ->
if (containingClass.classKind == ClassKind.ANNOTATION_CLASS) {
// Java annotation classes allow (actually require) named parameters.
return null
}
}
}
if (hasStableParameterNames) return null
return when (origin) {
FirDeclarationOrigin.Source, FirDeclarationOrigin.Precompiled, FirDeclarationOrigin.Library -> null
FirDeclarationOrigin.Delegated -> delegatedWrapperData?.wrapped?.getAsForbiddenNamedArgumentsTarget(session)
FirDeclarationOrigin.ImportedFromObjectOrStatic -> importedFromObjectOrStaticData?.original?.getAsForbiddenNamedArgumentsTarget(session)
is FirDeclarationOrigin.Java, FirDeclarationOrigin.Enhancement -> ForbiddenNamedArgumentsTarget.NON_KOTLIN_FUNCTION
FirDeclarationOrigin.SamConstructor -> null
FirDeclarationOrigin.ImportedFromObjectOrStatic ->
importedFromObjectOrStaticData?.original?.getAsForbiddenNamedArgumentsTarget(session)
FirDeclarationOrigin.IntersectionOverride, FirDeclarationOrigin.SubstitutionOverride -> {
var result: ForbiddenNamedArgumentsTarget? =
originalIfFakeOverride()?.getAsForbiddenNamedArgumentsTarget(session) ?: return null
@@ -587,25 +578,13 @@ fun FirFunction.getAsForbiddenNamedArgumentsTarget(
}
result
}
// referenced function of a Kotlin function type
FirDeclarationOrigin.BuiltIns -> runIf(dispatchReceiverClassLookupTagOrNull()?.isSomeFunctionType(session) == true) {
ForbiddenNamedArgumentsTarget.INVOKE_ON_FUNCTION_TYPE
}
FirDeclarationOrigin.Synthetic,
FirDeclarationOrigin.DynamicScope,
FirDeclarationOrigin.RenamedForOverride,
FirDeclarationOrigin.WrappedIntegerOperator,
FirDeclarationOrigin.ScriptCustomization,
FirDeclarationOrigin.BuiltIns -> ForbiddenNamedArgumentsTarget.INVOKE_ON_FUNCTION_TYPE
is FirDeclarationOrigin.Plugin -> null // TODO: figure out what to do with plugin generated functions
else -> ForbiddenNamedArgumentsTarget.NON_KOTLIN_FUNCTION
}
}
// TODO: handle functions with non-stable parameter names, see also
// org.jetbrains.kotlin.fir.serialization.FirElementSerializer.functionProto
// org.jetbrains.kotlin.fir.serialization.FirElementSerializer.constructorProto
fun FirFunction.getHasStableParameterNames(session: FirSession): Boolean = getAsForbiddenNamedArgumentsTarget(session) == null
@OptIn(ExperimentalContracts::class)
fun FirExpression?.isIntegerLiteralOrOperatorCall(): Boolean {
contract {
@@ -38,6 +38,7 @@ interface FirDeclarationStatus : FirElement {
val isFromSealedClass: Boolean
val isFromEnumClass: Boolean
val isFun: Boolean
val hasStableParameterNames: Boolean
override fun <R, D> accept(visitor: FirVisitor<R, D>, data: D): R = visitor.visitDeclarationStatus(this, data)
@@ -39,6 +39,7 @@ interface FirResolvedDeclarationStatus : FirDeclarationStatus {
override val isFromSealedClass: Boolean
override val isFromEnumClass: Boolean
override val isFun: Boolean
override val hasStableParameterNames: Boolean
val effectiveVisibility: EffectiveVisibility
override fun <R, D> accept(visitor: FirVisitor<R, D>, data: D): R = visitor.visitResolvedDeclarationStatus(this, data)
@@ -110,6 +110,7 @@ fun FirDeclarationStatus.copy(
isFromSealedClass: Boolean = this.isFromSealedClass,
isFromEnumClass: Boolean = this.isFromEnumClass,
isFun: Boolean = this.isFun,
hasStableParameterNames: Boolean = this.hasStableParameterNames,
): FirDeclarationStatus {
val newVisibility = visibility ?: this.visibility
val newModality = modality ?: this.modality
@@ -137,6 +138,7 @@ fun FirDeclarationStatus.copy(
this.isFromSealedClass = isFromSealedClass
this.isFromEnumClass = isFromEnumClass
this.isFun = isFun
this.hasStableParameterNames = hasStableParameterNames
}
}
@@ -20,7 +20,7 @@ open class FirDeclarationStatusImpl(
override val modality: Modality?
) : FirPureAbstractElement(), FirDeclarationStatus {
override val source: KtSourceElement? get() = null
protected var flags: Int = 0
protected var flags: Int = HAS_STABLE_PARAMETER_NAMES.mask
operator fun get(modifier: Modifier): Boolean = (flags and modifier.mask) != 0
@@ -140,6 +140,12 @@ open class FirDeclarationStatusImpl(
this[FUN] = value
}
override var hasStableParameterNames: Boolean
get() = this[HAS_STABLE_PARAMETER_NAMES]
set(value) {
this[HAS_STABLE_PARAMETER_NAMES] = value
}
enum class Modifier(val mask: Int) {
EXPECT(0x1),
ACTUAL(0x2),
@@ -158,7 +164,8 @@ open class FirDeclarationStatusImpl(
STATIC(0x4000),
FROM_SEALED(0x8000),
FROM_ENUM(0x10000),
FUN(0x20000)
FUN(0x20000),
HAS_STABLE_PARAMETER_NAMES(0x40000),
}
override fun <R, D> acceptChildren(visitor: FirVisitor<R, D>, data: D) {}
@@ -46,6 +46,7 @@ inline val FirMemberDeclaration.isLateInit: Boolean get() = status.isLateInit
inline val FirMemberDeclaration.isFromSealedClass: Boolean get() = status.isFromSealedClass
inline val FirMemberDeclaration.isFromEnumClass: Boolean get() = status.isFromEnumClass
inline val FirMemberDeclaration.isFun: Boolean get() = status.isFun
inline val FirMemberDeclaration.hasStableParameterNames: Boolean get() = status.hasStableParameterNames
inline val FirClassLikeDeclaration.isLocal: Boolean get() = symbol.classId.isLocal
@@ -382,7 +382,7 @@ object NodeConfigurator : AbstractFieldConfigurator<FirTreeBuilder>(FirTreeBuild
generateBooleanFields(
"expect", "actual", "override", "operator", "infix", "inline", "tailRec",
"external", "const", "lateInit", "inner", "companion", "data", "suspend", "static",
"fromSealedClass", "fromEnumClass", "fun"
"fromSealedClass", "fromEnumClass", "fun", "hasStableParameterNames",
)
}
@@ -22,7 +22,7 @@ abstract class AbstractFieldConfigurator<T : AbstractFirTreeBuilder>(private val
fun generateBooleanFields(vararg names: String) {
names.forEach {
+booleanField("is${it.replaceFirstChar(Char::uppercaseChar)}")
+booleanField(if (it.startsWith("is") || it.startsWith("has")) it else "is${it.replaceFirstChar(Char::uppercaseChar)}")
}
}
@@ -0,0 +1,3 @@
public interface JavaInterface {
public void foo(int javaName);
}
@@ -0,0 +1,4 @@
public class JavaSuperClass implements JavaInterface {
@Override
public void foo(int javaName) {}
}
@@ -0,0 +1 @@
class KtClass : JavaInterface by JavaSuperClass()
@@ -0,0 +1,6 @@
fun test() {
val ktInstance = KtClass()
// Shouldn't work but is kept for backward compatibility with K1
// When KT-4758 is fixed, this should break.
ktInstance.foo(javaName = 1)
}
@@ -0,0 +1,24 @@
// FIR_IDENTICAL
// SKIP_TXT
// FILE: JavaInterface.java
public interface JavaInterface {
public void foo(int javaName);
}
// FILE: JavaClass.java
public class JavaSuperClass implements JavaInterface {
@Override
public void foo(int javaName) {}
}
// FILE: 1.kt
class KtClass: JavaInterface by JavaSuperClass()
fun test() {
val ktInstance = KtClass()
ktInstance.foo(<!NAMED_ARGUMENTS_NOT_ALLOWED!>javaName<!> = 1)
}
@@ -0,0 +1,6 @@
// FIR_IDENTICAL
// SKIP_TXT
fun test(f: (String) -> Unit) {
f(<!NAMED_ARGUMENTS_NOT_ALLOWED!>p1<!> = "")
}
@@ -22100,6 +22100,18 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest {
runTest("compiler/testData/diagnostics/tests/namedArguments/ambiguousNamedArgumentsWithGenerics3.kt");
}
@Test
@TestMetadata("disallowForDelegationToJavaMethods.kt")
public void testDisallowForDelegationToJavaMethods() throws Exception {
runTest("compiler/testData/diagnostics/tests/namedArguments/disallowForDelegationToJavaMethods.kt");
}
@Test
@TestMetadata("disallowForFunctionTypes.kt")
public void testDisallowForFunctionTypes() throws Exception {
runTest("compiler/testData/diagnostics/tests/namedArguments/disallowForFunctionTypes.kt");
}
@Test
@TestMetadata("disallowForJavaConstructor.kt")
public void testDisallowForJavaConstructor() throws Exception {
@@ -371,6 +371,11 @@ class CompileKotlinAgainstCustomBinariesTest : AbstractKotlinCompilerIntegration
compileKotlin("source.kt", tmpdir, listOf(compileLibrary("library")))
}
fun testHasStableParameterNames() {
val library = compileLibrary("library", additionalOptions = listOf("-language-version", "2.0"))
compileKotlin("source.kt", tmpdir, listOf(library), additionalOptions = listOf("-language-version", "2.0"))
}
fun testRequireKotlinInNestedClasses() {
compileKotlin("source.kt", tmpdir, listOf(compileLibrary("library")))
}