From 4bba93f63387dbf0ba8dd2d7200efad39fc267f4 Mon Sep 17 00:00:00 2001 From: Nikolay Lunyak Date: Fri, 7 Jul 2023 17:00:51 +0300 Subject: [PATCH] [FIR] Implement PROTECTED_CONSTRUCTOR_NOT_IN_SUPER_CALL ^KT-59382 Fixed --- .../diagnostics/KtFirDataClassConverters.kt | 7 +++ .../api/fir/diagnostics/KtFirDiagnostics.kt | 5 ++ .../fir/diagnostics/KtFirDiagnosticsImpl.kt | 6 +++ .../diagnostics/FirDiagnosticsList.kt | 3 ++ .../fir/analysis/diagnostics/FirErrors.kt | 1 + .../checkers/CommonExpressionCheckers.kt | 1 + ...otectedConstructorNotInSuperCallChecker.kt | 48 +++++++++++++++++++ .../diagnostics/FirErrorsDefaultMessages.kt | 2 + .../tests/generics/protectedSuperCall.fir.kt | 21 -------- .../tests/generics/protectedSuperCall.kt | 1 + .../diagnostics/tests/inline/kt15410.fir.kt | 2 +- .../nonSuperCallConstructor.fir.kt | 26 ---------- .../nonSuperCallConstructor.kt | 1 + ...CallConstructorJavaDifferentPackage.fir.kt | 32 ------------- ...uperCallConstructorJavaDifferentPackage.kt | 1 + ...typeAliasConstructorWrongVisibility.fir.kt | 4 +- 16 files changed, 79 insertions(+), 82 deletions(-) create mode 100644 compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirProtectedConstructorNotInSuperCallChecker.kt delete mode 100644 compiler/testData/diagnostics/tests/generics/protectedSuperCall.fir.kt delete mode 100644 compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructor.fir.kt delete mode 100644 compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructorJavaDifferentPackage.fir.kt diff --git a/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/diagnostics/KtFirDataClassConverters.kt b/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/diagnostics/KtFirDataClassConverters.kt index 89394ee9398..14f573fb739 100644 --- a/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/diagnostics/KtFirDataClassConverters.kt +++ b/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/diagnostics/KtFirDataClassConverters.kt @@ -724,6 +724,13 @@ internal val KT_DIAGNOSTIC_CONVERTER = KtDiagnosticConverterBuilder.buildConvert token, ) } + add(FirErrors.PROTECTED_CONSTRUCTOR_NOT_IN_SUPER_CALL) { firDiagnostic -> + ProtectedConstructorNotInSuperCallImpl( + firSymbolBuilder.buildSymbol(firDiagnostic.a), + firDiagnostic as KtPsiDiagnostic, + token, + ) + } add(FirErrors.SUPERTYPE_NOT_INITIALIZED) { firDiagnostic -> SupertypeNotInitializedImpl( firDiagnostic as KtPsiDiagnostic, diff --git a/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/diagnostics/KtFirDiagnostics.kt b/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/diagnostics/KtFirDiagnostics.kt index ab932b4c7ab..3cefef60688 100644 --- a/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/diagnostics/KtFirDiagnostics.kt +++ b/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/diagnostics/KtFirDiagnostics.kt @@ -542,6 +542,11 @@ sealed interface KtFirDiagnostic : KtDiagnosticWithPsi { override val diagnosticClass get() = PrimaryConstructorDelegationCallExpected::class } + interface ProtectedConstructorNotInSuperCall : KtFirDiagnostic { + override val diagnosticClass get() = ProtectedConstructorNotInSuperCall::class + val symbol: KtSymbol + } + interface SupertypeNotInitialized : KtFirDiagnostic { override val diagnosticClass get() = SupertypeNotInitialized::class } diff --git a/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/diagnostics/KtFirDiagnosticsImpl.kt b/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/diagnostics/KtFirDiagnosticsImpl.kt index 89393a99353..66c39e2616e 100644 --- a/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/diagnostics/KtFirDiagnosticsImpl.kt +++ b/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/diagnostics/KtFirDiagnosticsImpl.kt @@ -640,6 +640,12 @@ internal class PrimaryConstructorDelegationCallExpectedImpl( token: KtLifetimeToken, ) : KtAbstractFirDiagnostic(firDiagnostic, token), KtFirDiagnostic.PrimaryConstructorDelegationCallExpected +internal class ProtectedConstructorNotInSuperCallImpl( + override val symbol: KtSymbol, + firDiagnostic: KtPsiDiagnostic, + token: KtLifetimeToken, +) : KtAbstractFirDiagnostic(firDiagnostic, token), KtFirDiagnostic.ProtectedConstructorNotInSuperCall + internal class SupertypeNotInitializedImpl( firDiagnostic: KtPsiDiagnostic, token: KtLifetimeToken, diff --git a/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirDiagnosticsList.kt b/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirDiagnosticsList.kt index 867b18808ce..226abaa333e 100644 --- a/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirDiagnosticsList.kt +++ b/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirDiagnosticsList.kt @@ -259,6 +259,9 @@ object DIAGNOSTICS_LIST : DiagnosticList("FirErrors") { val NON_PRIVATE_OR_PROTECTED_CONSTRUCTOR_IN_SEALED by error() val CYCLIC_CONSTRUCTOR_DELEGATION_CALL by error() val PRIMARY_CONSTRUCTOR_DELEGATION_CALL_EXPECTED by error(PositioningStrategy.SECONDARY_CONSTRUCTOR_DELEGATION_CALL) + val PROTECTED_CONSTRUCTOR_NOT_IN_SUPER_CALL by error { + parameter>("symbol") + } // TODO: change it to KtSuperTypeEntry when possible (after re-targeter implementation) val SUPERTYPE_NOT_INITIALIZED by error() diff --git a/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt b/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt index bc019977784..ef602157f21 100644 --- a/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt +++ b/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt @@ -224,6 +224,7 @@ object FirErrors { val NON_PRIVATE_OR_PROTECTED_CONSTRUCTOR_IN_SEALED by error0() val CYCLIC_CONSTRUCTOR_DELEGATION_CALL by error0() val PRIMARY_CONSTRUCTOR_DELEGATION_CALL_EXPECTED by error0(SourceElementPositioningStrategies.SECONDARY_CONSTRUCTOR_DELEGATION_CALL) + val PROTECTED_CONSTRUCTOR_NOT_IN_SUPER_CALL by error1>() val SUPERTYPE_NOT_INITIALIZED by error0() val SUPERTYPE_INITIALIZED_WITHOUT_PRIMARY_CONSTRUCTOR by error0() val DELEGATION_SUPER_CALL_IN_ENUM_CONSTRUCTOR by error0() diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/CommonExpressionCheckers.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/CommonExpressionCheckers.kt index d30b87c7023..6892ca78ed9 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/CommonExpressionCheckers.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/CommonExpressionCheckers.kt @@ -70,6 +70,7 @@ object CommonExpressionCheckers : ExpressionCheckers() { FirNamedVarargChecker, FirUnderscoredTypeArgumentSyntaxChecker, FirContractNotFirstStatementChecker, + FirProtectedConstructorNotInSuperCallChecker, ) override val propertyAccessExpressionCheckers: Set diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirProtectedConstructorNotInSuperCallChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirProtectedConstructorNotInSuperCallChecker.kt new file mode 100644 index 00000000000..127c446c1ed --- /dev/null +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirProtectedConstructorNotInSuperCallChecker.kt @@ -0,0 +1,48 @@ +/* + * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.fir.analysis.checkers.expression + +import org.jetbrains.kotlin.descriptors.Visibilities +import org.jetbrains.kotlin.diagnostics.DiagnosticReporter +import org.jetbrains.kotlin.diagnostics.reportOn +import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext +import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors +import org.jetbrains.kotlin.fir.declarations.getConstructedClass +import org.jetbrains.kotlin.fir.declarations.isJavaOrEnhancement +import org.jetbrains.kotlin.fir.declarations.utils.visibility +import org.jetbrains.kotlin.fir.expressions.FirFunctionCall +import org.jetbrains.kotlin.fir.originalIfFakeOverride +import org.jetbrains.kotlin.fir.packageFqName +import org.jetbrains.kotlin.fir.references.isError +import org.jetbrains.kotlin.fir.references.resolved +import org.jetbrains.kotlin.fir.references.toResolvedConstructorSymbol +import org.jetbrains.kotlin.fir.symbols.impl.FirConstructorSymbol +import org.jetbrains.kotlin.util.PrivateForInline + +object FirProtectedConstructorNotInSuperCallChecker : FirFunctionCallChecker() { + override fun check(expression: FirFunctionCall, context: CheckerContext, reporter: DiagnosticReporter) { + val reference = expression.calleeReference.resolved ?: return + val symbol = reference.toResolvedConstructorSymbol() ?: return + val constructedClass = symbol.getConstructedClass(context.session) + + if ( + !shouldAllowSuchCallNonetheless(symbol, context) && + symbol.visibility.normalize() == Visibilities.Protected && + // Prevent reporting for already invisible references + !reference.isError() && + context.containingDeclarations.none { it.symbol == constructedClass } + ) { + reporter.reportOn(expression.calleeReference.source, FirErrors.PROTECTED_CONSTRUCTOR_NOT_IN_SUPER_CALL, symbol, context) + } + } + + @OptIn(PrivateForInline::class) + private fun shouldAllowSuchCallNonetheless(symbol: FirConstructorSymbol, context: CheckerContext): Boolean { + val containingFile = context.containingFile ?: return false + val original = symbol.originalIfFakeOverride() ?: symbol + return original.origin.isJavaOrEnhancement && original.callableId.packageName == containingFile.packageFqName + } +} \ No newline at end of file diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrorsDefaultMessages.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrorsDefaultMessages.kt index 92f452ceb99..5fd6f1ab514 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrorsDefaultMessages.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrorsDefaultMessages.kt @@ -476,6 +476,7 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.PROPERTY_WITH_NO_ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.PROTECTED_CALL_FROM_PUBLIC_INLINE import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.PROTECTED_CALL_FROM_PUBLIC_INLINE_ERROR import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.PROTECTED_CONSTRUCTOR_CALL_FROM_PUBLIC_INLINE +import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.PROTECTED_CONSTRUCTOR_NOT_IN_SUPER_CALL import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.QUALIFIED_SUPERTYPE_EXTENDED_BY_OTHER_SUPERTYPE import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.RECURSION_IN_IMPLICIT_TYPES import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.RECURSION_IN_INLINE @@ -841,6 +842,7 @@ object FirErrorsDefaultMessages : BaseDiagnosticRendererFactory() { map.put(NON_PRIVATE_OR_PROTECTED_CONSTRUCTOR_IN_SEALED, "Constructor must be private or protected in sealed class") map.put(CYCLIC_CONSTRUCTOR_DELEGATION_CALL, "There's a cycle in the delegation calls chain") map.put(PRIMARY_CONSTRUCTOR_DELEGATION_CALL_EXPECTED, "Primary constructor call expected") + map.put(PROTECTED_CONSTRUCTOR_NOT_IN_SUPER_CALL, "Protected constructor ''{0}'' from other classes can only be used in super-call", SYMBOL) map.put(SUPERTYPE_INITIALIZED_WITHOUT_PRIMARY_CONSTRUCTOR, "Supertype initialization is impossible without primary constructor") map.put(DELEGATION_SUPER_CALL_IN_ENUM_CONSTRUCTOR, "Call to super is not allowed in enum constructor") map.put(PRIMARY_CONSTRUCTOR_REQUIRED_FOR_DATA_CLASS, "Primary constructor required for data class") diff --git a/compiler/testData/diagnostics/tests/generics/protectedSuperCall.fir.kt b/compiler/testData/diagnostics/tests/generics/protectedSuperCall.fir.kt deleted file mode 100644 index 0f89e995456..00000000000 --- a/compiler/testData/diagnostics/tests/generics/protectedSuperCall.fir.kt +++ /dev/null @@ -1,21 +0,0 @@ -// !DIAGNOSTICS: -UNUSED_PARAMETER -// FILE: GA.kt - -package test.x - -open class GA protected constructor() - -// FILE: Main.kt -package test - -import test.x.GA - -class C : GA() { - companion object { - fun bar() = GA() // Should be error - } -} - -fun main(args: Array) { - C.bar() -} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/generics/protectedSuperCall.kt b/compiler/testData/diagnostics/tests/generics/protectedSuperCall.kt index 0b128cd097d..f7c7d6ba269 100644 --- a/compiler/testData/diagnostics/tests/generics/protectedSuperCall.kt +++ b/compiler/testData/diagnostics/tests/generics/protectedSuperCall.kt @@ -1,3 +1,4 @@ +// FIR_IDENTICAL // !DIAGNOSTICS: -UNUSED_PARAMETER // FILE: GA.kt diff --git a/compiler/testData/diagnostics/tests/inline/kt15410.fir.kt b/compiler/testData/diagnostics/tests/inline/kt15410.fir.kt index e455a47e3be..cb4a847079a 100644 --- a/compiler/testData/diagnostics/tests/inline/kt15410.fir.kt +++ b/compiler/testData/diagnostics/tests/inline/kt15410.fir.kt @@ -5,5 +5,5 @@ open class Foo protected constructor() inline fun foo(f: () -> Unit) = object: Foo() {} class A : Foo() { - inline fun foo(f: () -> Unit) = Foo() + inline fun foo(f: () -> Unit) = Foo() } diff --git a/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructor.fir.kt b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructor.fir.kt deleted file mode 100644 index 1a1fc5669b5..00000000000 --- a/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructor.fir.kt +++ /dev/null @@ -1,26 +0,0 @@ -// !DIAGNOSTICS: -UNUSED_PARAMETER - -open class A protected constructor(x: Int) { - protected constructor() : this(1) - public constructor(x: Double) : this(3) -} - -class B4 : A(1) { - init { - A() - A(1) - A(5.0) - } - - fun foo() { - A() - A(1) - A(5.0) - - object : A() {} - object : A(1) {} - object : A(5.0) {} - - class Local : A() - } -} diff --git a/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructor.kt b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructor.kt index 65fb3936ae6..39431cd6d66 100644 --- a/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructor.kt +++ b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructor.kt @@ -1,3 +1,4 @@ +// FIR_IDENTICAL // !DIAGNOSTICS: -UNUSED_PARAMETER open class A protected constructor(x: Int) { diff --git a/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructorJavaDifferentPackage.fir.kt b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructorJavaDifferentPackage.fir.kt deleted file mode 100644 index 679c584e96c..00000000000 --- a/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructorJavaDifferentPackage.fir.kt +++ /dev/null @@ -1,32 +0,0 @@ -// !DIAGNOSTICS: -UNUSED_PARAMETER - -// FILE: abc/A.java -package abc; -public class A { - protected A() {} - protected A(int x) {} - public A(double x) {} -} - -// FILE: main.kt -import abc.* - -class B4 : A(1) { - init { - A() - A(1) - A(5.0) - } - - fun foo() { - A() - A(1) - A(5.0) - - object : A() {} - object : A(1) {} - object : A(5.0) {} - - class Local : A() - } -} diff --git a/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructorJavaDifferentPackage.kt b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructorJavaDifferentPackage.kt index 8aaef71defa..defa126fe67 100644 --- a/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructorJavaDifferentPackage.kt +++ b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructorJavaDifferentPackage.kt @@ -1,3 +1,4 @@ +// FIR_IDENTICAL // !DIAGNOSTICS: -UNUSED_PARAMETER // FILE: abc/A.java diff --git a/compiler/testData/diagnostics/tests/typealias/typeAliasConstructorWrongVisibility.fir.kt b/compiler/testData/diagnostics/tests/typealias/typeAliasConstructorWrongVisibility.fir.kt index 70ec9199f0d..50d47aa6731 100644 --- a/compiler/testData/diagnostics/tests/typealias/typeAliasConstructorWrongVisibility.fir.kt +++ b/compiler/testData/diagnostics/tests/typealias/typeAliasConstructorWrongVisibility.fir.kt @@ -21,8 +21,8 @@ val test3a = MyClass(1.0) class MyDerived : MyClass(1.0) { val test4 = MyAlias(1) val test4a = MyClass(1) - val test5 = MyAlias("") - val test5a = MyClass("") + val test5 = MyAlias("") + val test5a = MyClass("") val test6 = MyAlias(1.0) val test6a = MyClass(1.0) }