From 0f213e58db65676d9e245b57fd8ac1361bb552da Mon Sep 17 00:00:00 2001 From: Nick Date: Fri, 31 Jul 2020 18:01:52 +0300 Subject: [PATCH] [FIR] Add diagnostic for primary constructor not called --- .../diagnostics/annotationClassMember.kt | 2 +- .../cyclicConstructorDelegationCall.kt | 6 ++-- ...rCyclicConstructorDelegationCallChecker.kt | 33 +++++++++++++++++-- .../fir/analysis/diagnostics/FirErrors.kt | 1 + .../mulitpleVarargParameters.fir.kt | 6 ++-- .../enum/constructorInHeaderEnum.fir.kt | 2 +- .../headerClassWithFunctionBody.fir.kt | 2 +- .../tests/multiplatform/namedArguments.fir.kt | 2 +- .../tests/secondaryConstructors/enums.fir.kt | 2 +- .../expectedPrimaryConstructorCall.fir.kt | 8 ----- .../expectedPrimaryConstructorCall.kt | 1 + .../implicitSuperCallErrorsIfPrimary.fir.kt | 2 +- 12 files changed, 44 insertions(+), 23 deletions(-) delete mode 100644 compiler/testData/diagnostics/tests/secondaryConstructors/expectedPrimaryConstructorCall.fir.kt diff --git a/compiler/fir/analysis-tests/testData/resolve/diagnostics/annotationClassMember.kt b/compiler/fir/analysis-tests/testData/resolve/diagnostics/annotationClassMember.kt index 7007a05b541..2e12e471c7a 100644 --- a/compiler/fir/analysis-tests/testData/resolve/diagnostics/annotationClassMember.kt +++ b/compiler/fir/analysis-tests/testData/resolve/diagnostics/annotationClassMember.kt @@ -1,5 +1,5 @@ annotation class A() { - constructor(s: Nothing?) {} + constructor(s: Nothing?) {} init {} fun foo() {} val bar: Nothing? diff --git a/compiler/fir/analysis-tests/testData/resolve/diagnostics/cyclicConstructorDelegationCall.kt b/compiler/fir/analysis-tests/testData/resolve/diagnostics/cyclicConstructorDelegationCall.kt index c5d6f2e7a4f..9405986d625 100644 --- a/compiler/fir/analysis-tests/testData/resolve/diagnostics/cyclicConstructorDelegationCall.kt +++ b/compiler/fir/analysis-tests/testData/resolve/diagnostics/cyclicConstructorDelegationCall.kt @@ -42,16 +42,16 @@ class J { } class F(s: String) { - constructor(i: Boolean) {} + constructor(i: Boolean) {} constructor(i: Int) : this(3) {} } class G(x: Int) { - constructor() {} + constructor() {} } class H(x: Int) { - constructor() + constructor() } class K(x: Int) { diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirCyclicConstructorDelegationCallChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirCyclicConstructorDelegationCallChecker.kt index a8364061ff2..80d38d7619f 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirCyclicConstructorDelegationCallChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirCyclicConstructorDelegationCallChecker.kt @@ -13,6 +13,8 @@ import org.jetbrains.kotlin.fir.declarations.FirConstructor import org.jetbrains.kotlin.fir.declarations.FirMemberDeclaration import org.jetbrains.kotlin.fir.declarations.FirRegularClass import org.jetbrains.kotlin.fir.references.FirResolvedNamedReference +import org.jetbrains.kotlin.fir.types.FirResolvedTypeRef +import org.jetbrains.kotlin.fir.types.FirTypeRef import org.jetbrains.kotlin.utils.addToStdlib.safeAs object FirCyclicConstructorDelegationCallChecker : FirMemberDeclarationChecker() { @@ -22,11 +24,27 @@ object FirCyclicConstructorDelegationCallChecker : FirMemberDeclarationChecker() } val cyclicConstructors = mutableSetOf() + var hasPrimaryConstructor = false for (it in declaration.declarations) { - if (it is FirConstructor && !it.isPrimary) { - it.findCycle(cyclicConstructors)?.let { visited -> - cyclicConstructors += visited + if (it is FirConstructor) { + if (!it.isPrimary) { + it.findCycle(cyclicConstructors)?.let { visited -> + cyclicConstructors += visited + } + } else { + hasPrimaryConstructor = true + } + } + } + + if (hasPrimaryConstructor) { + for (it in declaration.declarations) { + if ( + it is FirConstructor && !it.isPrimary && it !in cyclicConstructors && + it.delegatedConstructor?.constructedTypeRef?.getType() != it.returnTypeRef.getType() + ) { + reporter.reportPrimaryConstructorDelegationCallExpected(it.delegatedConstructor?.source) } } } @@ -60,7 +78,16 @@ object FirCyclicConstructorDelegationCallChecker : FirMemberDeclarationChecker() ?.resolvedSymbol ?.fir.safeAs() + private fun FirTypeRef.getType() = when (this) { + is FirResolvedTypeRef -> type + else -> null + } + private fun DiagnosticReporter.reportCyclicConstructorDelegationCall(source: FirSourceElement?) { source?.let { report(FirErrors.CYCLIC_CONSTRUCTOR_DELEGATION_CALL.on(it)) } } + + private fun DiagnosticReporter.reportPrimaryConstructorDelegationCallExpected(source: FirSourceElement?) { + source?.let { report(FirErrors.PRIMARY_CONSTRUCTOR_DELEGATION_CALL_EXPECTED.on(it)) } + } } \ No newline at end of file diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt index 84821a5baed..2b253cce496 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt @@ -65,6 +65,7 @@ object FirErrors { val NOT_AN_ANNOTATION_CLASS by error1() val CYCLIC_CONSTRUCTOR_DELEGATION_CALL by warning0() + val PRIMARY_CONSTRUCTOR_DELEGATION_CALL_EXPECTED by warning0() // Exposed visibility group val EXPOSED_TYPEALIAS_EXPANDED_TYPE by error3() diff --git a/compiler/testData/diagnostics/tests/declarationChecks/mulitpleVarargParameters.fir.kt b/compiler/testData/diagnostics/tests/declarationChecks/mulitpleVarargParameters.fir.kt index a51b6884050..f8e2c2e73d7 100644 --- a/compiler/testData/diagnostics/tests/declarationChecks/mulitpleVarargParameters.fir.kt +++ b/compiler/testData/diagnostics/tests/declarationChecks/mulitpleVarargParameters.fir.kt @@ -2,7 +2,7 @@ fun test(vararg x1: Int, vararg x2: Int) { fun test2(vararg x1: Int, vararg x2: Int) { class LocalClass(vararg x1: Int, vararg x2: Int) { - constructor(vararg x1: Int, vararg x2: Int, xx: Int) {} + constructor(vararg x1: Int, vararg x2: Int, xx: Int) {} } fun test3(vararg x1: Int, vararg x2: Int) {} } @@ -20,7 +20,7 @@ abstract class C(vararg x1: Int, vararg x2: Int, b: Boolean) { abstract fun test2(vararg x1: Int, vararg x2: Int) class CC(vararg x1: Int, vararg x2: Int, b: Boolean) { - constructor(vararg x1: Int, vararg x2: Int) {} + constructor(vararg x1: Int, vararg x2: Int) {} fun test(vararg x1: Int, vararg x2: Int) {} } } @@ -29,7 +29,7 @@ object O { fun test(vararg x1: Int, vararg x2: Int) {} class CC(vararg x1: Int, vararg x2: Int, b: Boolean) { - constructor(vararg x1: Int, vararg x2: Int) {} + constructor(vararg x1: Int, vararg x2: Int) {} fun test(vararg x1: Int, vararg x2: Int) {} } } diff --git a/compiler/testData/diagnostics/tests/multiplatform/enum/constructorInHeaderEnum.fir.kt b/compiler/testData/diagnostics/tests/multiplatform/enum/constructorInHeaderEnum.fir.kt index 71224ec2125..5546bcea3c4 100644 --- a/compiler/testData/diagnostics/tests/multiplatform/enum/constructorInHeaderEnum.fir.kt +++ b/compiler/testData/diagnostics/tests/multiplatform/enum/constructorInHeaderEnum.fir.kt @@ -7,7 +7,7 @@ expect enum class En(x: Int) { E2(42), ; - constructor(s: String) + constructor(s: String) } expect enum class En2 { diff --git a/compiler/testData/diagnostics/tests/multiplatform/headerClass/headerClassWithFunctionBody.fir.kt b/compiler/testData/diagnostics/tests/multiplatform/headerClass/headerClassWithFunctionBody.fir.kt index 338c52d9816..359647a50ee 100644 --- a/compiler/testData/diagnostics/tests/multiplatform/headerClass/headerClassWithFunctionBody.fir.kt +++ b/compiler/testData/diagnostics/tests/multiplatform/headerClass/headerClassWithFunctionBody.fir.kt @@ -9,7 +9,7 @@ expect class Foo( "no" } - constructor(s: String) { + constructor(s: String) { "no" } diff --git a/compiler/testData/diagnostics/tests/multiplatform/namedArguments.fir.kt b/compiler/testData/diagnostics/tests/multiplatform/namedArguments.fir.kt index 49db2f1bf5c..bd800858adb 100644 --- a/compiler/testData/diagnostics/tests/multiplatform/namedArguments.fir.kt +++ b/compiler/testData/diagnostics/tests/multiplatform/namedArguments.fir.kt @@ -4,7 +4,7 @@ // FILE: common.kt expect class Foo(zzz: Int) { - constructor(aaa: Boolean) + constructor(aaa: Boolean) fun f1(xxx: String): String } diff --git a/compiler/testData/diagnostics/tests/secondaryConstructors/enums.fir.kt b/compiler/testData/diagnostics/tests/secondaryConstructors/enums.fir.kt index 1dc6540dd7a..4035ec25be1 100644 --- a/compiler/testData/diagnostics/tests/secondaryConstructors/enums.fir.kt +++ b/compiler/testData/diagnostics/tests/secondaryConstructors/enums.fir.kt @@ -14,7 +14,7 @@ enum class B(x: Int) { constructor(x: Int, y: Int): this(x+y) constructor(x: Double): this(x.toInt(), 1) - constructor(x: String): super(x, 1) + constructor(x: String): super(x, 1) } enum class C { diff --git a/compiler/testData/diagnostics/tests/secondaryConstructors/expectedPrimaryConstructorCall.fir.kt b/compiler/testData/diagnostics/tests/secondaryConstructors/expectedPrimaryConstructorCall.fir.kt deleted file mode 100644 index 71ffad646e1..00000000000 --- a/compiler/testData/diagnostics/tests/secondaryConstructors/expectedPrimaryConstructorCall.fir.kt +++ /dev/null @@ -1,8 +0,0 @@ -// !DIAGNOSTICS: -UNUSED_PARAMETER -class A(x: Int) { - constructor() -} -open class B(x: Int) -class C(x: Int) : B(x) { - constructor(): super(1) -} diff --git a/compiler/testData/diagnostics/tests/secondaryConstructors/expectedPrimaryConstructorCall.kt b/compiler/testData/diagnostics/tests/secondaryConstructors/expectedPrimaryConstructorCall.kt index 653855d878c..e92cc5e9a15 100644 --- a/compiler/testData/diagnostics/tests/secondaryConstructors/expectedPrimaryConstructorCall.kt +++ b/compiler/testData/diagnostics/tests/secondaryConstructors/expectedPrimaryConstructorCall.kt @@ -1,3 +1,4 @@ +// FIR_IDENTICAL // !DIAGNOSTICS: -UNUSED_PARAMETER class A(x: Int) { constructor() diff --git a/compiler/testData/diagnostics/tests/secondaryConstructors/implicitSuperCallErrorsIfPrimary.fir.kt b/compiler/testData/diagnostics/tests/secondaryConstructors/implicitSuperCallErrorsIfPrimary.fir.kt index 0802ad93ff6..a91d28edfc5 100644 --- a/compiler/testData/diagnostics/tests/secondaryConstructors/implicitSuperCallErrorsIfPrimary.fir.kt +++ b/compiler/testData/diagnostics/tests/secondaryConstructors/implicitSuperCallErrorsIfPrimary.fir.kt @@ -2,6 +2,6 @@ open class A(p1: String) class B() : A("") { - constructor(s: String) { + constructor(s: String) { } }