FIR checker: introduce member function checker

compiler/fir/analysis-tests/testData/resolve/expresssions/importedReceiver.kt

@@ -13,7 +13,7 @@ interface Your<R> {
 
 object My : Your<Double> {
     fun <T> T.bar() {}
-    fun baz()
+    <!NON_ABSTRACT_FUNCTION_WITH_NO_BODY!>fun baz()<!>
     fun Boolean.gau() {}
 }
 

compiler/fir/analysis-tests/testData/resolve/expresssions/privateVisibility.kt

@@ -38,7 +38,7 @@ private class Private {
 
 fun withLocals() {
     class Local {
-        private fun bar()
+        <!NON_ABSTRACT_FUNCTION_WITH_NO_BODY!>private fun bar()<!>
 
         fun baz() {
             bar()

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| {
         }

compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantModalityModifierChecker.kt

@@ -9,14 +9,14 @@ interface Interface {
         get() = 42<!>
     <!REDUNDANT_MODALITY_MODIFIER{LT}!>// Redundant
     <!REDUNDANT_MODALITY_MODIFIER{PSI}!>abstract<!> fun foo()<!>
-    // error
+    <!PRIVATE_FUNCTION_WITH_NO_BODY{LT}!>// error
-    private final fun bar() {}
+    <!PRIVATE_FUNCTION_WITH_NO_BODY{PSI}!>private<!> final fun bar()<!>
 
     <!REDUNDANT_MODALITY_MODIFIER{LT}!><!REDUNDANT_MODALITY_MODIFIER{PSI}!>open<!> fun goo() {}<!>
     <!REDUNDANT_MODALITY_MODIFIER{LT}!><!REDUNDANT_MODALITY_MODIFIER{PSI}!>abstract<!> fun tar()<!>
 
-    // error
+    <!ABSTRACT_FUNCTION_WITH_BODY{LT}!>// error
-    abstract fun too() {}
+    <!ABSTRACT_FUNCTION_WITH_BODY{PSI}!>abstract<!> fun too() {}<!>
 }
 interface B {
     <!REDUNDANT_MODALITY_MODIFIER{LT}!><!REDUNDANT_MODALITY_MODIFIER{PSI}!>abstract<!> var bar: Unit<!>

compiler/fir/analysis-tests/testData/resolve/visibility/exposedFunctionReturnType.kt

@@ -5,7 +5,7 @@ class A {
 }
 
 abstract class B {
-    <!EXPOSED_FUNCTION_RETURN_TYPE{LT}!>fun <!EXPOSED_FUNCTION_RETURN_TYPE{PSI}!>foo<!>(str: String): A.InnerA<!>
+    <!EXPOSED_FUNCTION_RETURN_TYPE{LT}, NON_ABSTRACT_FUNCTION_WITH_NO_BODY!>fun <!EXPOSED_FUNCTION_RETURN_TYPE{PSI}!>foo<!>(str: String): A.InnerA<!>
 }
 
 private enum class Some {

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 }
+
+}

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.*
-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.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<FirSourceElement, PsiElement, KtModifierKeywordToken, KtModifierKeywordToken>()
     val DEPRECATED_MODIFIER_PAIR by error2<FirSourceElement, PsiElement, KtModifierKeywordToken, KtModifierKeywordToken>()
     val INCOMPATIBLE_MODIFIERS by error2<FirSourceElement, PsiElement, KtModifierKeywordToken, KtModifierKeywordToken>()
+    val REDUNDANT_OPEN_IN_INTERFACE by warning0<FirSourceElement, KtModifierListOwner>(SourceElementPositioningStrategies.MODALITY_MODIFIER)
 
     // Applicability
     val NONE_APPLICABLE by error1<FirSourceElement, PsiElement, Collection<AbstractFirBasedSymbol<*>>>()
@@ -149,6 +147,12 @@ object FirErrors {
     val LOCAL_OBJECT_NOT_ALLOWED by error1<FirSourceElement, KtNamedDeclaration, Name>(SourceElementPositioningStrategies.DECLARATION_NAME)
     val LOCAL_INTERFACE_NOT_ALLOWED by error1<FirSourceElement, KtNamedDeclaration, Name>(SourceElementPositioningStrategies.DECLARATION_NAME)
 
+    // Functions
+    val ABSTRACT_FUNCTION_IN_NON_ABSTRACT_CLASS by error0<FirSourceElement, KtModifierListOwner>(SourceElementPositioningStrategies.MODALITY_MODIFIER)
+    val ABSTRACT_FUNCTION_WITH_BODY by error0<FirSourceElement, KtModifierListOwner>(SourceElementPositioningStrategies.MODALITY_MODIFIER)
+    val NON_ABSTRACT_FUNCTION_WITH_NO_BODY by error1<FirSourceElement, PsiElement, FirFunctionSymbol<*>>()
+    val PRIVATE_FUNCTION_WITH_NO_BODY by error0<FirSourceElement, KtModifierListOwner>(SourceElementPositioningStrategies.VISIBILITY_MODIFIER)
+
     // Properties & accessors
     val ABSTRACT_PROPERTY_IN_NON_ABSTRACT_CLASS by error0<FirSourceElement, KtModifierListOwner>(SourceElementPositioningStrategies.MODALITY_MODIFIER)
     val PRIVATE_PROPERTY_IN_INTERFACE by error0<FirSourceElement, KtModifierListOwner>(SourceElementPositioningStrategies.VISIBILITY_MODIFIER)

compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/checkers/CommonDeclarationCheckers.kt

@@ -47,6 +47,7 @@ object CommonDeclarationCheckers : DeclarationCheckers() {
         FirSupertypeInitializedWithoutPrimaryConstructor,
         FirTypeParametersInObjectChecker,
         FirTypeMismatchOnOverrideChecker,
+        FirMemberFunctionChecker,
         FirMemberPropertyChecker,
     )
 

compiler/testData/diagnostics/tests/AbstractInAbstractClass.fir.kt

@@ -23,10 +23,10 @@ abstract class MyAbstractClass() {
     abstract val e3: Int = <!ABSTRACT_PROPERTY_WITH_INITIALIZER!>0<!>; <!ABSTRACT_PROPERTY_WITH_GETTER!>get() = a<!>
 
     //methods
-    fun f()
+    <!NON_ABSTRACT_FUNCTION_WITH_NO_BODY!>fun f()<!>
     fun g() {}
     abstract fun h()
-    abstract fun j() {}
+    <!ABSTRACT_FUNCTION_WITH_BODY!>abstract<!> fun j() {}
 
     //property accessors
     var i: Int                       abstract get  abstract set

compiler/testData/diagnostics/tests/AbstractInClass.fir.kt

@@ -23,10 +23,10 @@ class MyClass() {
     <!ABSTRACT_PROPERTY_IN_NON_ABSTRACT_CLASS!>abstract<!> val e3: Int = 0; get() = a
 
     //methods
-    fun f()
+    <!NON_ABSTRACT_FUNCTION_WITH_NO_BODY!>fun f()<!>
     fun g() {}
-    abstract fun h()
+    <!ABSTRACT_FUNCTION_IN_NON_ABSTRACT_CLASS!>abstract<!> fun h()
-    abstract fun j() {}
+    <!ABSTRACT_FUNCTION_IN_NON_ABSTRACT_CLASS, ABSTRACT_FUNCTION_WITH_BODY!>abstract<!> fun j() {}
 
     //property accessors
     var i: Int                       abstract get  abstract set

compiler/testData/diagnostics/tests/AbstractInTrait.fir.kt

@@ -26,7 +26,7 @@ interface MyTrait {
     fun f()
     fun g() {}
     abstract fun h()
-    abstract fun j() {}
+    <!ABSTRACT_FUNCTION_WITH_BODY!>abstract<!> fun j() {}
 
     //property accessors
     var i: Int                       abstract get  abstract set

compiler/testData/diagnostics/tests/declarationChecks/kt2397.fir.kt

@@ -13,5 +13,5 @@ interface T {
 }
 
 class A {
-    final fun foo()
+    <!NON_ABSTRACT_FUNCTION_WITH_NO_BODY!>final fun foo()<!>
 }

compiler/testData/diagnostics/tests/enum/AbstractInEnum.fir.kt

@@ -25,10 +25,10 @@ enum class MyEnum() {
     abstract val e3: Int = <!ABSTRACT_PROPERTY_WITH_INITIALIZER!>0<!>; <!ABSTRACT_PROPERTY_WITH_GETTER!>get() = a<!>
 
     //methods
-    fun f()
+    <!NON_ABSTRACT_FUNCTION_WITH_NO_BODY!>fun f()<!>
     fun g() {}
     abstract fun h()
-    abstract fun j() {}
+    <!ABSTRACT_FUNCTION_WITH_BODY!>abstract<!> fun j() {}
 
     //property accessors
     var i: Int                       abstract get  abstract set

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_FUNCTION_WITH_NO_BODY!>private<!> fun foo(): Int
     // ok
     private fun bar() = 42
 }

compiler/testData/diagnostics/tests/multiplatform/headerClass/expectClassWithExplicitAbstractMember.fir.kt

@@ -1,23 +0,0 @@
-// !LANGUAGE: +MultiPlatformProjects
-// MODULE: m1-common
-// FILE: common.kt
-
-interface Foo {
-    fun foo()
-}
-
-expect class NonAbstractClass : Foo {
-    abstract fun bar()
-
-    <!ABSTRACT_PROPERTY_IN_NON_ABSTRACT_CLASS!>abstract<!> val baz: Int
-
-    abstract override fun foo()
-}
-
-expect abstract class AbstractClass : Foo {
-    abstract fun bar()
-
-    abstract val baz: Int
-
-    abstract override fun foo()
-}

compiler/testData/diagnostics/tests/multiplatform/headerClass/expectClassWithExplicitAbstractMember.kt

@@ -1,3 +1,4 @@
+// FIR_IDENTICAL
 // !LANGUAGE: +MultiPlatformProjects
 // MODULE: m1-common
 // FILE: common.kt

compiler/testData/diagnostics/tests/multiplatform/headerFunInNonHeaderClass.fir.kt

@@ -3,7 +3,7 @@
 // FILE: common.kt
 
 class Foo {
-    expect fun bar(): String
+    <!NON_ABSTRACT_FUNCTION_WITH_NO_BODY!>expect fun bar(): String<!>
 }
 
 // MODULE: m1-jvm(m1-common)

compiler/testData/diagnostics/tests/multiplatform/modifierApplicability.fir.kt

@@ -9,7 +9,7 @@ class Outer expect constructor() {
 
     expect init {}
 
-    expect fun foo()
+    <!NON_ABSTRACT_FUNCTION_WITH_NO_BODY!>expect fun foo()<!>
     expect val bar: Int
 }
 

compiler/testData/diagnostics/tests/variance/Visibility.fir.kt

@@ -16,5 +16,5 @@ interface Test<in I, out O> {
     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_FUNCTION_WITH_NO_BODY!>private<!> fun private_fun(i: O) : I
 }

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() = {}
+    <!ABSTRACT_FUNCTION_WITH_BODY!>abstract<!> fun foo() = {}
-    fun boo() : Unit
+    <!NON_ABSTRACT_FUNCTION_WITH_NO_BODY!>fun boo() : Unit<!>
     abstract val a = <!ABSTRACT_PROPERTY_WITH_INITIALIZER!>""<!>
     val b
     var d