diff --git a/compiler/fir/analysis-tests/testData/resolve/openInInterface.kt b/compiler/fir/analysis-tests/testData/resolve/openInInterface.kt index 916f320e75f..fdd553a0018 100644 --- a/compiler/fir/analysis-tests/testData/resolve/openInInterface.kt +++ b/compiler/fir/analysis-tests/testData/resolve/openInInterface.kt @@ -1,12 +1,12 @@ interface Some { - open fun foo() + open fun foo() open fun bar() {} - open val x: Int + open val x: Int open val y = 1 open val z get() = 1 - open var xx: Int + open var xx: Int open var yy = 1 open var zz: Int set(value) { diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirDeclarationCheckerUtils.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirDeclarationCheckerUtils.kt new file mode 100644 index 00000000000..13a4196d6e3 --- /dev/null +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirDeclarationCheckerUtils.kt @@ -0,0 +1,14 @@ +/* + * Copyright 2010-2021 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.declaration + +import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext +import org.jetbrains.kotlin.fir.declarations.FirRegularClass +import org.jetbrains.kotlin.fir.declarations.isExpect + +// Note that the class that contains the currently visiting declaration will *not* be in the context's containing declarations *yet*. +internal fun isInsideExpectClass(containingDeclaration: FirRegularClass, context: CheckerContext): Boolean = + containingDeclaration.isExpect || context.containingDeclarations.asReversed().any { it is FirRegularClass && it.isExpect } diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberFunctionChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberFunctionChecker.kt index 5c848407600..7596902d254 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberFunctionChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberFunctionChecker.kt @@ -35,7 +35,8 @@ object FirMemberFunctionChecker : FirRegularClassChecker() { // If multiple (potentially conflicting) modality modifiers are specified, not all modifiers are recorded at `status`. // So, our source of truth should be the full modifier list retrieved from the source. val modifierList = with(FirModifierList) { source.getModifierList() } - val isAbstract = function.isAbstract || modifierList?.modifiers?.any { it.token == KtTokens.ABSTRACT_KEYWORD } == true + val hasAbstractModifier = modifierList?.modifiers?.any { it.token == KtTokens.ABSTRACT_KEYWORD } == true + val isAbstract = function.isAbstract || hasAbstractModifier if (isAbstract) { if (!containingDeclaration.canHaveAbstractDeclaration) { reporter.report(source, FirErrors.ABSTRACT_FUNCTION_IN_NON_ABSTRACT_CLASS) @@ -45,25 +46,20 @@ object FirMemberFunctionChecker : FirRegularClassChecker() { } } val isInsideExpectClass = isInsideExpectClass(containingDeclaration, context) - val isOpen = function.isOpen || modifierList?.modifiers?.any { it.token == KtTokens.OPEN_KEYWORD } == true + val hasOpenModifier = modifierList?.modifiers?.any { it.token == KtTokens.OPEN_KEYWORD } == true val isExternal = function.isExternal || modifierList?.modifiers?.any { it.token == KtTokens.EXTERNAL_KEYWORD } == true if (!function.hasBody) { if (containingDeclaration.isInterface) { if (Visibilities.isPrivate(function.visibility)) { reporter.report(source, FirErrors.PRIVATE_FUNCTION_WITH_NO_BODY) } - if (!isInsideExpectClass && !isAbstract && isOpen) { + if (!isInsideExpectClass && !hasAbstractModifier && hasOpenModifier) { reporter.report(source, FirErrors.REDUNDANT_OPEN_IN_INTERFACE) } - } else if (!isInsideExpectClass && !isAbstract && !isExternal) { - // TODO: we need to check if modifiers of the function already get some errors, e.g., INCOMPATIBLE_MODIFIERS + } else if (!isInsideExpectClass && !hasAbstractModifier && !isExternal) { reporter.report(FirErrors.NON_ABSTRACT_FUNCTION_WITH_NO_BODY.on(source, function.symbol)) } } } - private fun isInsideExpectClass(containingDeclaration: FirRegularClass, context: CheckerContext): Boolean = - // Note that the class that contains the currently visiting function is *not* in the context's containing declarations *yet*. - containingDeclaration.isExpect || context.containingDeclarations.asReversed().any { it is FirRegularClass && it.isExpect } - } diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberPropertyChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberPropertyChecker.kt index 9932190300c..b9918582b4e 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberPropertyChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberPropertyChecker.kt @@ -23,16 +23,22 @@ object FirMemberPropertyChecker : FirRegularClassChecker() { override fun check(declaration: FirRegularClass, context: CheckerContext, reporter: DiagnosticReporter) { for (member in declaration.declarations) { if (member is FirProperty) { - checkProperty(declaration, member, reporter) + checkProperty(declaration, member, context, reporter) } } } - private fun checkProperty(containingDeclaration: FirRegularClass, property: FirProperty, reporter: DiagnosticReporter) { + private fun checkProperty( + containingDeclaration: FirRegularClass, + property: FirProperty, + context: CheckerContext, + reporter: DiagnosticReporter + ) { // If multiple (potentially conflicting) modality modifiers are specified, not all modifiers are recorded at `status`. // So, our source of truth should be the full modifier list retrieved from the source. val modifierList = with(FirModifierList) { property.source.getModifierList() } - val isAbstract = property.isAbstract || modifierList?.modifiers?.any { it.token == KtTokens.ABSTRACT_KEYWORD } == true + val hasAbstractModifier = modifierList?.modifiers?.any { it.token == KtTokens.ABSTRACT_KEYWORD } == true + val isAbstract = property.isAbstract || hasAbstractModifier if (containingDeclaration.isInterface && Visibilities.isPrivate(property.visibility) && !isAbstract && @@ -75,7 +81,18 @@ object FirMemberPropertyChecker : FirRegularClassChecker() { checkPropertyInitializer(containingDeclaration, property, isAbstract, reporter) - val isOpen = property.isOpen || modifierList?.modifiers?.any { it.token == KtTokens.OPEN_KEYWORD } == true + val hasOpenModifier = modifierList?.modifiers?.any { it.token == KtTokens.OPEN_KEYWORD } == true + if (hasOpenModifier && + containingDeclaration.isInterface && + !hasAbstractModifier && + property.isAbstract && + !isInsideExpectClass(containingDeclaration, context) + ) { + property.source?.let { + reporter.report(it, FirErrors.REDUNDANT_OPEN_IN_INTERFACE) + } + } + val isOpen = property.isOpen || hasOpenModifier if (isOpen) { checkAccessor(property.setter, property.delegate) { src, symbol -> if (symbol.fir.visibility == Visibilities.Private && property.visibility != Visibilities.Private) { diff --git a/compiler/testData/diagnostics/tests/delegation/Delegation_MultipleDelegates.fir.kt b/compiler/testData/diagnostics/tests/delegation/Delegation_MultipleDelegates.fir.kt index 22c4949bf8d..76eeed7b956 100644 --- a/compiler/testData/diagnostics/tests/delegation/Delegation_MultipleDelegates.fir.kt +++ b/compiler/testData/diagnostics/tests/delegation/Delegation_MultipleDelegates.fir.kt @@ -1,10 +1,10 @@ // !DIAGNOSTICS: -CONFLICTING_JVM_DECLARATIONS interface One { - public open fun foo() : Int + public open fun foo() : Int private fun boo() = 10 } interface Two { - public open fun foo() : Int + public open fun foo() : Int } interface OneImpl : One { diff --git a/compiler/testData/diagnostics/tests/modifiers/openInInterface.fir.kt b/compiler/testData/diagnostics/tests/modifiers/openInInterface.fir.kt deleted file mode 100644 index b3124a6f394..00000000000 --- a/compiler/testData/diagnostics/tests/modifiers/openInInterface.fir.kt +++ /dev/null @@ -1,10 +0,0 @@ -interface My { - open fun foo() - open fun bar() {} - open abstract fun baz(): Int - - open val x: Int - open val y: String - get() = "" - open abstract val z: Double -} diff --git a/compiler/testData/diagnostics/tests/modifiers/openInInterface.kt b/compiler/testData/diagnostics/tests/modifiers/openInInterface.kt index b2e1578e77c..50e6ecafc92 100644 --- a/compiler/testData/diagnostics/tests/modifiers/openInInterface.kt +++ b/compiler/testData/diagnostics/tests/modifiers/openInInterface.kt @@ -1,3 +1,4 @@ +// FIR_IDENTICAL interface My { open fun foo() open fun bar() {} diff --git a/compiler/testData/diagnostics/tests/override/kt880.fir.kt b/compiler/testData/diagnostics/tests/override/kt880.fir.kt deleted file mode 100644 index 814e808b9fe..00000000000 --- a/compiler/testData/diagnostics/tests/override/kt880.fir.kt +++ /dev/null @@ -1,16 +0,0 @@ -// KT-880 Overload resolution ambiguity - -public interface I { - open fun test() : Unit -} - -abstract public class A() { - open public fun test() : Unit { - } -} - -public open class T() : A(), I { - open fun main() : Unit { - test() // Test no "Overload resolution ambiguity" is here - } -} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/override/kt880.kt b/compiler/testData/diagnostics/tests/override/kt880.kt index 5b6b9042761..63d96fffe55 100644 --- a/compiler/testData/diagnostics/tests/override/kt880.kt +++ b/compiler/testData/diagnostics/tests/override/kt880.kt @@ -1,3 +1,4 @@ +// FIR_IDENTICAL // KT-880 Overload resolution ambiguity public interface I {