From d80b809a40944832d1140b847149c9e929aa4543 Mon Sep 17 00:00:00 2001 From: Tianyu Geng Date: Wed, 8 Dec 2021 12:58:40 -0800 Subject: [PATCH] FIR checker: remove inappropriate usage of FirFileChecker This to conform with the contract expected by LL API that no file checker should report diagnostics on its contained declarations --- .../kotlin/fir/checkers/generator/Main.kt | 1 + .../ComposedDeclarationCheckers.kt | 4 +++ .../declaration/DeclarationCheckers.kt | 20 ++++++------ .../FirDeclarationCheckerAliases.kt | 2 ++ .../checkers/CommonDeclarationCheckers.kt | 10 ++++-- ...rKClassWithIncorrectTypeArgumentChecker.kt | 30 ++++++----------- .../FirTopLevelFunctionsChecker.kt | 30 +++++++---------- .../FirTopLevelPropertiesChecker.kt | 32 +++++++++---------- 8 files changed, 62 insertions(+), 67 deletions(-) diff --git a/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/Main.kt b/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/Main.kt index 32f50a8ee34..da9af3eae3a 100644 --- a/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/Main.kt +++ b/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/Main.kt @@ -61,6 +61,7 @@ fun main(args: Array) { val declarationPackage = "$basePackage.checkers.declaration" generateCheckersComponents(generationPath, declarationPackage, "FirDeclarationChecker") { alias("BasicDeclarationChecker") + alias("CallableDeclarationChecker") alias("FunctionChecker") alias("SimpleFunctionChecker") alias("PropertyChecker") diff --git a/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/checkers/declaration/ComposedDeclarationCheckers.kt b/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/checkers/declaration/ComposedDeclarationCheckers.kt index f2ed266023d..99f3975de0a 100644 --- a/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/checkers/declaration/ComposedDeclarationCheckers.kt +++ b/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/checkers/declaration/ComposedDeclarationCheckers.kt @@ -17,6 +17,8 @@ import org.jetbrains.kotlin.fir.analysis.checkers.cfa.FirControlFlowChecker class ComposedDeclarationCheckers : DeclarationCheckers() { override val basicDeclarationCheckers: Set get() = _basicDeclarationCheckers + override val callableDeclarationCheckers: Set + get() = _callableDeclarationCheckers override val functionCheckers: Set get() = _functionCheckers override val simpleFunctionCheckers: Set @@ -57,6 +59,7 @@ class ComposedDeclarationCheckers : DeclarationCheckers() { get() = _variableAssignmentCfaBasedCheckers private val _basicDeclarationCheckers: MutableSet = mutableSetOf() + private val _callableDeclarationCheckers: MutableSet = mutableSetOf() private val _functionCheckers: MutableSet = mutableSetOf() private val _simpleFunctionCheckers: MutableSet = mutableSetOf() private val _propertyCheckers: MutableSet = mutableSetOf() @@ -80,6 +83,7 @@ class ComposedDeclarationCheckers : DeclarationCheckers() { @CheckersComponentInternal fun register(checkers: DeclarationCheckers) { _basicDeclarationCheckers += checkers.basicDeclarationCheckers + _callableDeclarationCheckers += checkers.callableDeclarationCheckers _functionCheckers += checkers.functionCheckers _simpleFunctionCheckers += checkers.simpleFunctionCheckers _propertyCheckers += checkers.propertyCheckers diff --git a/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/checkers/declaration/DeclarationCheckers.kt b/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/checkers/declaration/DeclarationCheckers.kt index a6dfe30fb11..6e185ddd642 100644 --- a/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/checkers/declaration/DeclarationCheckers.kt +++ b/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/checkers/declaration/DeclarationCheckers.kt @@ -20,6 +20,7 @@ abstract class DeclarationCheckers { } open val basicDeclarationCheckers: Set = emptySet() + open val callableDeclarationCheckers: Set = emptySet() open val functionCheckers: Set = emptySet() open val simpleFunctionCheckers: Set = emptySet() open val propertyCheckers: Set = emptySet() @@ -42,21 +43,22 @@ abstract class DeclarationCheckers { open val variableAssignmentCfaBasedCheckers: Set = emptySet() @CheckersComponentInternal internal val allBasicDeclarationCheckers: Set by lazy { basicDeclarationCheckers } - @CheckersComponentInternal internal val allFunctionCheckers: Set by lazy { functionCheckers + basicDeclarationCheckers } - @CheckersComponentInternal internal val allSimpleFunctionCheckers: Set by lazy { simpleFunctionCheckers + functionCheckers + basicDeclarationCheckers } - @CheckersComponentInternal internal val allPropertyCheckers: Set by lazy { propertyCheckers + basicDeclarationCheckers } + @CheckersComponentInternal internal val allCallableDeclarationCheckers: Set by lazy { callableDeclarationCheckers + basicDeclarationCheckers } + @CheckersComponentInternal internal val allFunctionCheckers: Set by lazy { functionCheckers + callableDeclarationCheckers + basicDeclarationCheckers } + @CheckersComponentInternal internal val allSimpleFunctionCheckers: Set by lazy { simpleFunctionCheckers + functionCheckers + callableDeclarationCheckers + basicDeclarationCheckers } + @CheckersComponentInternal internal val allPropertyCheckers: Set by lazy { propertyCheckers + callableDeclarationCheckers + basicDeclarationCheckers } @CheckersComponentInternal internal val allClassLikeCheckers: Set by lazy { classLikeCheckers + basicDeclarationCheckers } @CheckersComponentInternal internal val allClassCheckers: Set by lazy { classCheckers + classLikeCheckers + basicDeclarationCheckers } @CheckersComponentInternal internal val allRegularClassCheckers: Set by lazy { regularClassCheckers + classCheckers + classLikeCheckers + basicDeclarationCheckers } - @CheckersComponentInternal internal val allConstructorCheckers: Set by lazy { constructorCheckers + functionCheckers + basicDeclarationCheckers } + @CheckersComponentInternal internal val allConstructorCheckers: Set by lazy { constructorCheckers + functionCheckers + callableDeclarationCheckers + basicDeclarationCheckers } @CheckersComponentInternal internal val allFileCheckers: Set by lazy { fileCheckers + basicDeclarationCheckers } @CheckersComponentInternal internal val allTypeParameterCheckers: Set by lazy { typeParameterCheckers + basicDeclarationCheckers } @CheckersComponentInternal internal val allTypeAliasCheckers: Set by lazy { typeAliasCheckers + classLikeCheckers + basicDeclarationCheckers } - @CheckersComponentInternal internal val allAnonymousFunctionCheckers: Set by lazy { anonymousFunctionCheckers + functionCheckers + basicDeclarationCheckers } - @CheckersComponentInternal internal val allPropertyAccessorCheckers: Set by lazy { propertyAccessorCheckers + functionCheckers + basicDeclarationCheckers } - @CheckersComponentInternal internal val allBackingFieldCheckers: Set by lazy { backingFieldCheckers + basicDeclarationCheckers } - @CheckersComponentInternal internal val allValueParameterCheckers: Set by lazy { valueParameterCheckers + basicDeclarationCheckers } - @CheckersComponentInternal internal val allEnumEntryCheckers: Set by lazy { enumEntryCheckers + basicDeclarationCheckers } + @CheckersComponentInternal internal val allAnonymousFunctionCheckers: Set by lazy { anonymousFunctionCheckers + functionCheckers + callableDeclarationCheckers + basicDeclarationCheckers } + @CheckersComponentInternal internal val allPropertyAccessorCheckers: Set by lazy { propertyAccessorCheckers + functionCheckers + callableDeclarationCheckers + basicDeclarationCheckers } + @CheckersComponentInternal internal val allBackingFieldCheckers: Set by lazy { backingFieldCheckers + callableDeclarationCheckers + basicDeclarationCheckers } + @CheckersComponentInternal internal val allValueParameterCheckers: Set by lazy { valueParameterCheckers + callableDeclarationCheckers + basicDeclarationCheckers } + @CheckersComponentInternal internal val allEnumEntryCheckers: Set by lazy { enumEntryCheckers + callableDeclarationCheckers + basicDeclarationCheckers } @CheckersComponentInternal internal val allAnonymousObjectCheckers: Set by lazy { anonymousObjectCheckers + classCheckers + classLikeCheckers + basicDeclarationCheckers } @CheckersComponentInternal internal val allAnonymousInitializerCheckers: Set by lazy { anonymousInitializerCheckers + basicDeclarationCheckers } } diff --git a/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirDeclarationCheckerAliases.kt b/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirDeclarationCheckerAliases.kt index 652da5cece3..fa67d0a9d7b 100644 --- a/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirDeclarationCheckerAliases.kt +++ b/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirDeclarationCheckerAliases.kt @@ -14,6 +14,7 @@ import org.jetbrains.kotlin.fir.declarations.FirAnonymousFunction import org.jetbrains.kotlin.fir.declarations.FirAnonymousInitializer import org.jetbrains.kotlin.fir.declarations.FirAnonymousObject import org.jetbrains.kotlin.fir.declarations.FirBackingField +import org.jetbrains.kotlin.fir.declarations.FirCallableDeclaration import org.jetbrains.kotlin.fir.declarations.FirClass import org.jetbrains.kotlin.fir.declarations.FirClassLikeDeclaration import org.jetbrains.kotlin.fir.declarations.FirConstructor @@ -30,6 +31,7 @@ import org.jetbrains.kotlin.fir.declarations.FirTypeParameter import org.jetbrains.kotlin.fir.declarations.FirValueParameter typealias FirBasicDeclarationChecker = FirDeclarationChecker +typealias FirCallableDeclarationChecker = FirDeclarationChecker typealias FirFunctionChecker = FirDeclarationChecker typealias FirSimpleFunctionChecker = FirDeclarationChecker typealias FirPropertyChecker = FirDeclarationChecker diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/CommonDeclarationCheckers.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/CommonDeclarationCheckers.kt index fc2f5aa8e9a..c66c6efc5dd 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/CommonDeclarationCheckers.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/CommonDeclarationCheckers.kt @@ -35,6 +35,11 @@ object CommonDeclarationCheckers : DeclarationCheckers() { FirOptInMarkedDeclarationChecker, ) + override val callableDeclarationCheckers: Set + get() = setOf( + FirKClassWithIncorrectTypeArgumentChecker, + ) + override val functionCheckers: Set get() = setOf( FirContractChecker, @@ -49,6 +54,7 @@ object CommonDeclarationCheckers : DeclarationCheckers() { FirFunctionTypeParametersSyntaxChecker, FirOperatorModifierChecker, FirTailrecFunctionChecker, + FirTopLevelFunctionsChecker, ) override val propertyCheckers: Set @@ -65,6 +71,7 @@ object CommonDeclarationCheckers : DeclarationCheckers() { FirPropertyFromParameterChecker, FirLocalVariableTypeParametersSyntaxChecker, FirDelegateUsesExtensionPropertyTypeParameterChecker, + FirTopLevelPropertiesChecker, ) override val backingFieldCheckers: Set @@ -116,9 +123,6 @@ object CommonDeclarationCheckers : DeclarationCheckers() { override val fileCheckers: Set get() = setOf( - FirKClassWithIncorrectTypeArgumentChecker, - FirTopLevelFunctionsChecker, - FirTopLevelPropertiesChecker, FirImportsChecker, FirUnresolvedInMiddleOfImportChecker, ) diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirKClassWithIncorrectTypeArgumentChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirKClassWithIncorrectTypeArgumentChecker.kt index 2ff53f83119..cb8da7e7ac8 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirKClassWithIncorrectTypeArgumentChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirKClassWithIncorrectTypeArgumentChecker.kt @@ -6,36 +6,26 @@ package org.jetbrains.kotlin.fir.analysis.checkers.declaration import org.jetbrains.kotlin.KtFakeSourceElementKind -import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext import org.jetbrains.kotlin.diagnostics.DiagnosticReporter -import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors 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.FirCallableDeclaration -import org.jetbrains.kotlin.fir.declarations.FirFile import org.jetbrains.kotlin.fir.resolve.diagnostics.ConeTypeParameterInQualifiedAccess -import org.jetbrains.kotlin.fir.types.isKClassType import org.jetbrains.kotlin.fir.scopes.impl.toConeType import org.jetbrains.kotlin.fir.symbols.impl.FirTypeParameterSymbol import org.jetbrains.kotlin.fir.types.* // See FE1.0 [KClassWithIncorrectTypeArgumentChecker] -object FirKClassWithIncorrectTypeArgumentChecker : FirFileChecker() { - override fun check(declaration: FirFile, context: CheckerContext, reporter: DiagnosticReporter) { - for (topLevelDeclaration in declaration.declarations) { - if (topLevelDeclaration is FirCallableDeclaration) { - checkTopLevelDeclaration(topLevelDeclaration, context, reporter) - } - } - } +object FirKClassWithIncorrectTypeArgumentChecker : FirCallableDeclarationChecker() { - // When a type parameter is used as a type argument for KClass, it shouldn't be nullable. - // bad: fun test1() = T::class - // okay: fun test2() = T::class - private fun checkTopLevelDeclaration( - declaration: FirCallableDeclaration, - context: CheckerContext, - reporter: DiagnosticReporter - ) { + override fun check(declaration: FirCallableDeclaration, context: CheckerContext, reporter: DiagnosticReporter) { + // Only report on top level callable declarations + if (context.containingDeclarations.size > 1) return + + // When a type parameter is used as a type argument for KClass, it shouldn't be nullable. + // bad: fun test1() = T::class + // okay: fun test2() = T::class val source = declaration.source ?: return if (source.kind is KtFakeSourceElementKind) return diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelFunctionsChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelFunctionsChecker.kt index a1eb37fce95..2fa4b59fa52 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelFunctionsChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelFunctionsChecker.kt @@ -6,12 +6,11 @@ package org.jetbrains.kotlin.fir.analysis.checkers.declaration import org.jetbrains.kotlin.KtFakeSourceElementKind +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.checkers.hasModifier -import org.jetbrains.kotlin.diagnostics.DiagnosticReporter import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors -import org.jetbrains.kotlin.diagnostics.reportOn -import org.jetbrains.kotlin.fir.declarations.FirFile import org.jetbrains.kotlin.fir.declarations.FirSimpleFunction import org.jetbrains.kotlin.fir.declarations.utils.hasBody import org.jetbrains.kotlin.fir.declarations.utils.isExpect @@ -19,26 +18,21 @@ import org.jetbrains.kotlin.fir.declarations.utils.isExternal import org.jetbrains.kotlin.lexer.KtTokens // See old FE's [DeclarationsChecker] -object FirTopLevelFunctionsChecker : FirFileChecker() { - override fun check(declaration: FirFile, context: CheckerContext, reporter: DiagnosticReporter) { - for (topLevelDeclaration in declaration.declarations) { - if (topLevelDeclaration is FirSimpleFunction) { - checkFunction(topLevelDeclaration, reporter, context) - } - } - } +object FirTopLevelFunctionsChecker : FirSimpleFunctionChecker() { + override fun check(declaration: FirSimpleFunction, context: CheckerContext, reporter: DiagnosticReporter) { + // Only report on top level callable declarations + if (context.containingDeclarations.size > 1) return - private fun checkFunction(function: FirSimpleFunction, reporter: DiagnosticReporter, context: CheckerContext) { - val source = function.source ?: return + val source = declaration.source ?: return if (source.kind is KtFakeSourceElementKind) 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. - if (function.hasModifier(KtTokens.ABSTRACT_KEYWORD)) return - if (function.isExternal) return - if (!function.hasBody && !function.isExpect) { - reporter.reportOn(source, FirErrors.NON_MEMBER_FUNCTION_NO_BODY, function.symbol, context) + if (declaration.hasModifier(KtTokens.ABSTRACT_KEYWORD)) return + if (declaration.isExternal) return + if (!declaration.hasBody && !declaration.isExpect) { + reporter.reportOn(source, FirErrors.NON_MEMBER_FUNCTION_NO_BODY, declaration.symbol, context) } - checkExpectDeclarationVisibilityAndBody(function, source, reporter, context) + checkExpectDeclarationVisibilityAndBody(declaration, source, reporter, context) } } diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelPropertiesChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelPropertiesChecker.kt index f7ea1fd8f6b..99cecaa0cb7 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelPropertiesChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelPropertiesChecker.kt @@ -8,15 +8,18 @@ package org.jetbrains.kotlin.fir.analysis.checkers.declaration import org.jetbrains.kotlin.KtFakeSourceElementKind import org.jetbrains.kotlin.KtSourceElement 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.FirModifierList import org.jetbrains.kotlin.fir.analysis.checkers.contains import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext import org.jetbrains.kotlin.fir.analysis.checkers.getModifierList -import org.jetbrains.kotlin.diagnostics.DiagnosticReporter import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors -import org.jetbrains.kotlin.diagnostics.reportOn import org.jetbrains.kotlin.fir.analysis.diagnostics.withSuppressedDiagnostics -import org.jetbrains.kotlin.fir.declarations.* +import org.jetbrains.kotlin.fir.declarations.FirClass +import org.jetbrains.kotlin.fir.declarations.FirMemberDeclaration +import org.jetbrains.kotlin.fir.declarations.FirProperty +import org.jetbrains.kotlin.fir.declarations.FirSimpleFunction import org.jetbrains.kotlin.fir.declarations.impl.FirDefaultPropertyAccessor import org.jetbrains.kotlin.fir.declarations.utils.* import org.jetbrains.kotlin.fir.resolve.diagnostics.ConeLocalVariableNoTypeOrInitializer @@ -24,32 +27,27 @@ import org.jetbrains.kotlin.fir.types.FirErrorTypeRef import org.jetbrains.kotlin.lexer.KtTokens // See old FE's [DeclarationsChecker] -object FirTopLevelPropertiesChecker : FirFileChecker() { - override fun check(declaration: FirFile, context: CheckerContext, reporter: DiagnosticReporter) { - for (topLevelDeclaration in declaration.declarations) { - if (topLevelDeclaration is FirProperty) { - checkProperty(topLevelDeclaration, reporter, context) - } - } - } +object FirTopLevelPropertiesChecker : FirPropertyChecker() { + override fun check(declaration: FirProperty, context: CheckerContext, reporter: DiagnosticReporter) { + // Only report on top level callable declarations + if (context.containingDeclarations.size > 1) return - private fun checkProperty(property: FirProperty, reporter: DiagnosticReporter, context: CheckerContext) { - val source = property.source ?: return + val source = declaration.source ?: return if (source.kind is KtFakeSourceElementKind) 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 = source.getModifierList() - withSuppressedDiagnostics(property, context) { + withSuppressedDiagnostics(declaration, context) { checkPropertyInitializer( containingClass = null, - property, + declaration, modifierList, - isInitialized = property.initializer != null, + isInitialized = declaration.initializer != null, reporter, context ) - checkExpectDeclarationVisibilityAndBody(property, source, reporter, context) + checkExpectDeclarationVisibilityAndBody(declaration, source, reporter, context) } } }