diff --git a/compiler/fir/analysis-tests/testData/resolve/expresssions/importedReceiver.kt b/compiler/fir/analysis-tests/testData/resolve/expresssions/importedReceiver.kt index 74f1a947dba..50e569f38b2 100644 --- a/compiler/fir/analysis-tests/testData/resolve/expresssions/importedReceiver.kt +++ b/compiler/fir/analysis-tests/testData/resolve/expresssions/importedReceiver.kt @@ -13,7 +13,7 @@ interface Your { object My : Your { fun T.bar() {} - fun baz() + fun baz() fun Boolean.gau() {} } diff --git a/compiler/fir/analysis-tests/testData/resolve/expresssions/privateVisibility.kt b/compiler/fir/analysis-tests/testData/resolve/expresssions/privateVisibility.kt index 54a9235fef8..f1426b42391 100644 --- a/compiler/fir/analysis-tests/testData/resolve/expresssions/privateVisibility.kt +++ b/compiler/fir/analysis-tests/testData/resolve/expresssions/privateVisibility.kt @@ -38,7 +38,7 @@ private class Private { fun withLocals() { class Local { - private fun bar() + private fun bar() fun baz() { bar() diff --git a/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantModalityModifierChecker.fir.txt b/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantModalityModifierChecker.fir.txt index 194e6a3c588..81abe698f01 100644 --- a/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantModalityModifierChecker.fir.txt +++ b/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantModalityModifierChecker.fir.txt @@ -16,8 +16,7 @@ FILE: RedundantModalityModifierChecker.kt public abstract fun foo(): R|kotlin/Unit| - private final fun bar(): R|kotlin/Unit| { - } + private final fun bar(): R|kotlin/Unit| public open fun goo(): R|kotlin/Unit| { } diff --git a/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantModalityModifierChecker.kt b/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantModalityModifierChecker.kt index 6a0140e094a..0489203b341 100644 --- a/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantModalityModifierChecker.kt +++ b/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantModalityModifierChecker.kt @@ -9,14 +9,14 @@ interface Interface { get() = 42 // Redundant abstract fun foo() - // error - private final fun bar() {} + // error + private final fun bar() open fun goo() {} abstract fun tar() - // error - abstract fun too() {} + // error + abstract fun too() {} } interface B { abstract var bar: Unit diff --git a/compiler/fir/analysis-tests/testData/resolve/visibility/exposedFunctionReturnType.kt b/compiler/fir/analysis-tests/testData/resolve/visibility/exposedFunctionReturnType.kt index 49c79e79ccd..ce2119b2250 100644 --- a/compiler/fir/analysis-tests/testData/resolve/visibility/exposedFunctionReturnType.kt +++ b/compiler/fir/analysis-tests/testData/resolve/visibility/exposedFunctionReturnType.kt @@ -5,7 +5,7 @@ class A { } abstract class B { - fun foo(str: String): A.InnerA + fun foo(str: String): A.InnerA } private enum class Some { 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 new file mode 100644 index 00000000000..5c848407600 --- /dev/null +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberFunctionChecker.kt @@ -0,0 +1,69 @@ +/* + * 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.descriptors.Visibilities +import org.jetbrains.kotlin.fir.FirFakeSourceElementKind +import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext +import org.jetbrains.kotlin.fir.analysis.checkers.extended.report +import org.jetbrains.kotlin.fir.analysis.diagnostics.DiagnosticReporter +import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors +import org.jetbrains.kotlin.fir.declarations.* +import org.jetbrains.kotlin.lexer.KtTokens + +// See old FE's [DeclarationsChecker] +object FirMemberFunctionChecker : FirRegularClassChecker() { + override fun check(declaration: FirRegularClass, context: CheckerContext, reporter: DiagnosticReporter) { + for (member in declaration.declarations) { + if (member is FirSimpleFunction) { + checkFunction(declaration, member, context, reporter) + } + } + } + + private fun checkFunction( + containingDeclaration: FirRegularClass, + function: FirSimpleFunction, + context: CheckerContext, + reporter: DiagnosticReporter + ) { + val source = function.source ?: return + if (source.kind is FirFakeSourceElementKind) return + // 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 + if (isAbstract) { + if (!containingDeclaration.canHaveAbstractDeclaration) { + reporter.report(source, FirErrors.ABSTRACT_FUNCTION_IN_NON_ABSTRACT_CLASS) + } + if (function.hasBody) { + reporter.report(source, FirErrors.ABSTRACT_FUNCTION_WITH_BODY) + } + } + val isInsideExpectClass = isInsideExpectClass(containingDeclaration, context) + val isOpen = function.isOpen || 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) { + 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 + 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/diagnostics/FirErrors.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt index 45ac8058c1b..975966d07ab 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 @@ -14,10 +14,7 @@ import org.jetbrains.kotlin.fir.declarations.FirClass import org.jetbrains.kotlin.fir.declarations.FirMemberDeclaration import org.jetbrains.kotlin.fir.expressions.FirExpression import org.jetbrains.kotlin.fir.symbols.AbstractFirBasedSymbol -import org.jetbrains.kotlin.fir.symbols.impl.FirPropertyAccessorSymbol -import org.jetbrains.kotlin.fir.symbols.impl.FirPropertySymbol -import org.jetbrains.kotlin.fir.symbols.impl.FirRegularClassSymbol -import org.jetbrains.kotlin.fir.symbols.impl.FirTypeParameterSymbol +import org.jetbrains.kotlin.fir.symbols.impl.* import org.jetbrains.kotlin.fir.types.ConeKotlinType import org.jetbrains.kotlin.lexer.KtModifierKeywordToken import org.jetbrains.kotlin.name.Name @@ -111,6 +108,7 @@ object FirErrors { val REDUNDANT_MODIFIER by error2() val DEPRECATED_MODIFIER_PAIR by error2() val INCOMPATIBLE_MODIFIERS by error2() + val REDUNDANT_OPEN_IN_INTERFACE by warning0(SourceElementPositioningStrategies.MODALITY_MODIFIER) // Applicability val NONE_APPLICABLE by error1>>() @@ -149,6 +147,12 @@ object FirErrors { val LOCAL_OBJECT_NOT_ALLOWED by error1(SourceElementPositioningStrategies.DECLARATION_NAME) val LOCAL_INTERFACE_NOT_ALLOWED by error1(SourceElementPositioningStrategies.DECLARATION_NAME) + // Functions + val ABSTRACT_FUNCTION_IN_NON_ABSTRACT_CLASS by error0(SourceElementPositioningStrategies.MODALITY_MODIFIER) + val ABSTRACT_FUNCTION_WITH_BODY by error0(SourceElementPositioningStrategies.MODALITY_MODIFIER) + val NON_ABSTRACT_FUNCTION_WITH_NO_BODY by error1>() + val PRIVATE_FUNCTION_WITH_NO_BODY by error0(SourceElementPositioningStrategies.VISIBILITY_MODIFIER) + // Properties & accessors val ABSTRACT_PROPERTY_IN_NON_ABSTRACT_CLASS by error0(SourceElementPositioningStrategies.MODALITY_MODIFIER) val PRIVATE_PROPERTY_IN_INTERFACE by error0(SourceElementPositioningStrategies.VISIBILITY_MODIFIER) diff --git a/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/checkers/CommonDeclarationCheckers.kt b/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/checkers/CommonDeclarationCheckers.kt index d71196ed94b..fd289afbdb5 100644 --- a/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/checkers/CommonDeclarationCheckers.kt +++ b/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/checkers/CommonDeclarationCheckers.kt @@ -47,6 +47,7 @@ object CommonDeclarationCheckers : DeclarationCheckers() { FirSupertypeInitializedWithoutPrimaryConstructor, FirTypeParametersInObjectChecker, FirTypeMismatchOnOverrideChecker, + FirMemberFunctionChecker, FirMemberPropertyChecker, ) diff --git a/compiler/testData/diagnostics/tests/AbstractInAbstractClass.fir.kt b/compiler/testData/diagnostics/tests/AbstractInAbstractClass.fir.kt index b44c165a968..a5c0060cc34 100644 --- a/compiler/testData/diagnostics/tests/AbstractInAbstractClass.fir.kt +++ b/compiler/testData/diagnostics/tests/AbstractInAbstractClass.fir.kt @@ -23,10 +23,10 @@ abstract class MyAbstractClass() { abstract val e3: Int = 0; get() = a //methods - fun f() + fun f() fun g() {} abstract fun h() - abstract fun j() {} + abstract fun j() {} //property accessors var i: Int abstract get abstract set diff --git a/compiler/testData/diagnostics/tests/AbstractInClass.fir.kt b/compiler/testData/diagnostics/tests/AbstractInClass.fir.kt index 4bdd5e8f34f..4de56d72753 100644 --- a/compiler/testData/diagnostics/tests/AbstractInClass.fir.kt +++ b/compiler/testData/diagnostics/tests/AbstractInClass.fir.kt @@ -23,10 +23,10 @@ class MyClass() { abstract val e3: Int = 0; get() = a //methods - fun f() + fun f() fun g() {} - abstract fun h() - abstract fun j() {} + abstract fun h() + abstract fun j() {} //property accessors var i: Int abstract get abstract set diff --git a/compiler/testData/diagnostics/tests/AbstractInTrait.fir.kt b/compiler/testData/diagnostics/tests/AbstractInTrait.fir.kt index 04439b920e5..d047654732f 100644 --- a/compiler/testData/diagnostics/tests/AbstractInTrait.fir.kt +++ b/compiler/testData/diagnostics/tests/AbstractInTrait.fir.kt @@ -26,7 +26,7 @@ interface MyTrait { fun f() fun g() {} abstract fun h() - abstract fun j() {} + abstract fun j() {} //property accessors var i: Int abstract get abstract set diff --git a/compiler/testData/diagnostics/tests/declarationChecks/kt2397.fir.kt b/compiler/testData/diagnostics/tests/declarationChecks/kt2397.fir.kt index 08fac11d23c..2de9ebda214 100644 --- a/compiler/testData/diagnostics/tests/declarationChecks/kt2397.fir.kt +++ b/compiler/testData/diagnostics/tests/declarationChecks/kt2397.fir.kt @@ -13,5 +13,5 @@ interface T { } class A { - final fun foo() + final fun foo() } \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/enum/AbstractInEnum.fir.kt b/compiler/testData/diagnostics/tests/enum/AbstractInEnum.fir.kt index 343cf6d9488..f26b0922a84 100644 --- a/compiler/testData/diagnostics/tests/enum/AbstractInEnum.fir.kt +++ b/compiler/testData/diagnostics/tests/enum/AbstractInEnum.fir.kt @@ -25,10 +25,10 @@ enum class MyEnum() { abstract val e3: Int = 0; get() = a //methods - fun f() + fun f() fun g() {} abstract fun h() - abstract fun j() {} + abstract fun j() {} //property accessors var i: Int abstract get abstract set diff --git a/compiler/testData/diagnostics/tests/modifiers/privateInInterface.fir.kt b/compiler/testData/diagnostics/tests/modifiers/privateInInterface.fir.kt index 940c479dc64..ab643640a12 100644 --- a/compiler/testData/diagnostics/tests/modifiers/privateInInterface.fir.kt +++ b/compiler/testData/diagnostics/tests/modifiers/privateInInterface.fir.kt @@ -6,7 +6,7 @@ interface My { final val y: Int final val yy: Int get() = 1 - private fun foo(): Int + private fun foo(): Int // ok private fun bar() = 42 } diff --git a/compiler/testData/diagnostics/tests/multiplatform/headerClass/expectClassWithExplicitAbstractMember.fir.kt b/compiler/testData/diagnostics/tests/multiplatform/headerClass/expectClassWithExplicitAbstractMember.fir.kt deleted file mode 100644 index 98add0b1735..00000000000 --- a/compiler/testData/diagnostics/tests/multiplatform/headerClass/expectClassWithExplicitAbstractMember.fir.kt +++ /dev/null @@ -1,23 +0,0 @@ -// !LANGUAGE: +MultiPlatformProjects -// MODULE: m1-common -// FILE: common.kt - -interface Foo { - fun foo() -} - -expect class NonAbstractClass : Foo { - abstract fun bar() - - abstract val baz: Int - - abstract override fun foo() -} - -expect abstract class AbstractClass : Foo { - abstract fun bar() - - abstract val baz: Int - - abstract override fun foo() -} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/multiplatform/headerClass/expectClassWithExplicitAbstractMember.kt b/compiler/testData/diagnostics/tests/multiplatform/headerClass/expectClassWithExplicitAbstractMember.kt index 5297d35e3bc..7938a4d950e 100644 --- a/compiler/testData/diagnostics/tests/multiplatform/headerClass/expectClassWithExplicitAbstractMember.kt +++ b/compiler/testData/diagnostics/tests/multiplatform/headerClass/expectClassWithExplicitAbstractMember.kt @@ -1,3 +1,4 @@ +// FIR_IDENTICAL // !LANGUAGE: +MultiPlatformProjects // MODULE: m1-common // FILE: common.kt diff --git a/compiler/testData/diagnostics/tests/multiplatform/headerFunInNonHeaderClass.fir.kt b/compiler/testData/diagnostics/tests/multiplatform/headerFunInNonHeaderClass.fir.kt index 160a0998301..f4158f8e9aa 100644 --- a/compiler/testData/diagnostics/tests/multiplatform/headerFunInNonHeaderClass.fir.kt +++ b/compiler/testData/diagnostics/tests/multiplatform/headerFunInNonHeaderClass.fir.kt @@ -3,7 +3,7 @@ // FILE: common.kt class Foo { - expect fun bar(): String + expect fun bar(): String } // MODULE: m1-jvm(m1-common) diff --git a/compiler/testData/diagnostics/tests/multiplatform/modifierApplicability.fir.kt b/compiler/testData/diagnostics/tests/multiplatform/modifierApplicability.fir.kt index a4fc5ba6c13..cbfc524646a 100644 --- a/compiler/testData/diagnostics/tests/multiplatform/modifierApplicability.fir.kt +++ b/compiler/testData/diagnostics/tests/multiplatform/modifierApplicability.fir.kt @@ -9,7 +9,7 @@ class Outer expect constructor() { expect init {} - expect fun foo() + expect fun foo() expect val bar: Int } diff --git a/compiler/testData/diagnostics/tests/variance/Visibility.fir.kt b/compiler/testData/diagnostics/tests/variance/Visibility.fir.kt index 09204ae6c0b..af00cc013bf 100644 --- a/compiler/testData/diagnostics/tests/variance/Visibility.fir.kt +++ b/compiler/testData/diagnostics/tests/variance/Visibility.fir.kt @@ -16,5 +16,5 @@ interface Test { fun internal_fun(i: O) : I public fun public_fun(i: O) : I protected fun protected_fun(i: O) : I - private fun private_fun(i: O) : I + private fun private_fun(i: O) : I } \ No newline at end of file diff --git a/compiler/tests-spec/testData/diagnostics/linked/declarations/classifier-declaration/class-declaration/abstract-classes/p-1/neg/1.1.fir.kt b/compiler/tests-spec/testData/diagnostics/linked/declarations/classifier-declaration/class-declaration/abstract-classes/p-1/neg/1.1.fir.kt index 3f6551a9ac9..2f37daf279b 100644 --- a/compiler/tests-spec/testData/diagnostics/linked/declarations/classifier-declaration/class-declaration/abstract-classes/p-1/neg/1.1.fir.kt +++ b/compiler/tests-spec/testData/diagnostics/linked/declarations/classifier-declaration/class-declaration/abstract-classes/p-1/neg/1.1.fir.kt @@ -5,8 +5,8 @@ // TESTCASE NUMBER: 1 abstract class Base() { - abstract fun foo() = {} - fun boo() : Unit + abstract fun foo() = {} + fun boo() : Unit abstract val a = "" val b var d