From 1a3f47baddd2513879972eb6e3e9d3af730c65a1 Mon Sep 17 00:00:00 2001 From: Nikolay Lunyak Date: Mon, 2 Aug 2021 23:13:03 +0300 Subject: [PATCH] [FIR] Make FirJvmStaticChecker more consistent with the legacy version --- .../declaration/FirJvmStaticChecker.kt | 238 +++++++++--------- .../annotations/jvmStatic/functions.fir.kt | 53 ---- .../annotations/jvmStatic/functions.kt | 1 + .../jvmStatic/functions_LL13.fir.kt | 53 ---- .../annotations/jvmStatic/functions_LL13.kt | 1 + .../jvmStatic/interfaceCompanion_LL12.fir.kt | 22 +- .../interfaceCompanion_LL13_16.fir.kt | 6 +- .../interfaceCompanion_LL13_18.fir.kt | 6 +- 8 files changed, 144 insertions(+), 236 deletions(-) delete mode 100644 compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/functions.fir.kt delete mode 100644 compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/functions_LL13.fir.kt diff --git a/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/declaration/FirJvmStaticChecker.kt b/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/declaration/FirJvmStaticChecker.kt index 63ca577a1c6..a748cb7043a 100644 --- a/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/declaration/FirJvmStaticChecker.kt +++ b/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/declaration/FirJvmStaticChecker.kt @@ -10,6 +10,7 @@ import org.jetbrains.kotlin.descriptors.ClassKind import org.jetbrains.kotlin.descriptors.Visibilities import org.jetbrains.kotlin.descriptors.Visibility import org.jetbrains.kotlin.descriptors.isInterface +import org.jetbrains.kotlin.fir.FirSourceElement import org.jetbrains.kotlin.fir.analysis.checkers.classKind import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext import org.jetbrains.kotlin.fir.analysis.checkers.declaration.FirBasicDeclarationChecker @@ -25,7 +26,7 @@ import org.jetbrains.kotlin.fir.symbols.SymbolInternals import org.jetbrains.kotlin.fir.symbols.impl.FirClassLikeSymbol import org.jetbrains.kotlin.fir.symbols.impl.FirRegularClassSymbol import org.jetbrains.kotlin.name.ClassId -import org.jetbrains.kotlin.name.Name +import org.jetbrains.kotlin.name.SpecialNames import org.jetbrains.kotlin.name.StandardClassIds import org.jetbrains.kotlin.utils.addToStdlib.safeAs @@ -40,70 +41,121 @@ object FirJvmStaticChecker : FirBasicDeclarationChecker() { return } + if (declaration is FirPropertyAccessor) { + return + } + + val declarationAnnotation = declaration.findAnnotation(StandardClassIds.JvmStatic) + + if (declarationAnnotation != null) { + checkAnnotated(declaration, context, reporter, declaration.source) + } + + fun checkIfAnnotated(it: FirAnnotatedDeclaration) { + if (!it.hasAnnotation(StandardClassIds.JvmStatic)) { + return + } + val targetSource = it.source ?: declaration.source + checkAnnotated(it, context, reporter, targetSource, declaration as? FirProperty) + } + + if (declaration is FirProperty) { + declaration.getter?.let { checkIfAnnotated(it) } + declaration.setter?.let { checkIfAnnotated(it) } + } + } + + private fun checkAnnotated( + declaration: FirAnnotatedDeclaration, + context: CheckerContext, + reporter: DiagnosticReporter, + targetSource: FirSourceElement?, + outerProperty: FirProperty? = null, + ) { if (declaration !is FirMemberDeclaration) { return } - val annotatedParts = declaration.getAnnotatedParts() + val container = context.getContainerAt(0) ?: return + val supportsJvmStaticInInterface = context.supports(LanguageFeature.JvmStaticInInterface) + val containerIsAnonymous = container.classId.shortClassName == SpecialNames.ANONYMOUS - checkOverrideCannotBeStatic(declaration, context, reporter, annotatedParts) - checkStaticNotInProperObject(context, reporter, annotatedParts) - checkStaticNonPublicOrExternal(declaration, context, reporter, annotatedParts) - checkStaticOnConstOrJvmField(context, reporter, annotatedParts) - } - - private fun checkStaticOnConstOrJvmField( - context: CheckerContext, - reporter: DiagnosticReporter, - annotatedParts: List, - ) { - annotatedParts.forEach { - if ( - it is FirProperty && it.isConst || - it.hasAnnotationNamedAs(StandardClassIds.JvmField) - ) { - reporter.reportOn(it.source, FirJvmErrors.JVM_STATIC_ON_CONST_OR_JVM_FIELD, context) + if ( + container.classKind != ClassKind.OBJECT || + !container.isCompanion() && containerIsAnonymous + ) { + reportStaticNotInProperObject(context, reporter, supportsJvmStaticInInterface, targetSource) + } else if ( + container.isCompanion() && + context.containerIsInterface(1) + ) { + if (supportsJvmStaticInInterface) { + checkForInterface(declaration, context, reporter, targetSource) + } else { + reportStaticNotInProperObject(context, reporter, supportsJvmStaticInInterface, targetSource) } } + + checkOverrideCannotBeStatic(declaration, context, reporter, targetSource, outerProperty) + checkStaticOnConstOrJvmField(declaration, context, reporter, targetSource) } - private fun checkStaticNonPublicOrExternal( - declaration: FirMemberDeclaration, + private fun reportStaticNotInProperObject( context: CheckerContext, reporter: DiagnosticReporter, - annotatedParts: List, + supportsJvmStaticInInterface: Boolean, + targetSource: FirSourceElement?, ) { - val containingClassSymbol = context.getContainerAt(0) ?: return - var shouldCheck = false - - if (containingClassSymbol.classKind != ClassKind.OBJECT) { - shouldCheck = true + val properDiagnostic = if (supportsJvmStaticInInterface) { + FirJvmErrors.JVM_STATIC_NOT_IN_OBJECT_OR_COMPANION } else { - if (context.containerIsInterface(1)) { - shouldCheck = true - } + FirJvmErrors.JVM_STATIC_NOT_IN_OBJECT_OR_CLASS_COMPANION } - if (!shouldCheck) { + reporter.reportOn(targetSource, properDiagnostic, context) + } + + private fun checkForInterface( + declaration: FirAnnotatedDeclaration, + context: CheckerContext, + reporter: DiagnosticReporter, + targetSource: FirSourceElement?, + ) { + if (declaration !is FirCallableDeclaration) { return } - val minVisibility = declaration.getMinimumVisibility() + val visibility = if (declaration is FirProperty) { + declaration.getMinimumVisibility() + } else { + declaration.visibility + } - if (minVisibility != Visibilities.Public) { - annotatedParts.forEach { - reporter.reportOn(it.source, FirJvmErrors.JVM_STATIC_ON_NON_PUBLIC_MEMBER, context) - } - } else if (declaration.isExternal) { - annotatedParts.forEach { - reporter.reportOn(it.source, FirJvmErrors.JVM_STATIC_ON_EXTERNAL_IN_INTERFACE, context) - } + val isExternal = if (declaration is FirProperty) { + declaration.hasExternalParts() + } else { + declaration.isExternal + } + + if (visibility != Visibilities.Public) { + reporter.reportOn(targetSource, FirJvmErrors.JVM_STATIC_ON_NON_PUBLIC_MEMBER, context) + } else if (isExternal) { + reporter.reportOn(targetSource, FirJvmErrors.JVM_STATIC_ON_EXTERNAL_IN_INTERFACE, context) } } - private fun FirMemberDeclaration.getMinimumVisibility() = when (this) { - is FirProperty -> getMinimumVisibility() - else -> this.visibility + private fun FirProperty.hasExternalParts(): Boolean { + var hasExternal = isExternal + + getter?.let { + hasExternal = hasExternal || it.isExternal + } + + setter?.let { + hasExternal = hasExternal || it.isExternal + } + + return hasExternal } private fun FirProperty.getMinimumVisibility(): Visibility { @@ -129,85 +181,36 @@ object FirJvmStaticChecker : FirBasicDeclarationChecker() { } } - private fun checkStaticNotInProperObject( - context: CheckerContext, - reporter: DiagnosticReporter, - annotatedParts: List, - ) { - val containingClassSymbol = context.getContainerAt(0) ?: return - val supportJvmStaticInInterface = context.session.languageVersionSettings.supportsFeature(LanguageFeature.JvmStaticInInterface) - - val properDiagnostic = if (supportJvmStaticInInterface) { - FirJvmErrors.JVM_STATIC_NOT_IN_OBJECT_OR_COMPANION - } else { - FirJvmErrors.JVM_STATIC_NOT_IN_OBJECT_OR_CLASS_COMPANION - } - - var shouldReport = false - - if (containingClassSymbol.classKind != ClassKind.OBJECT) { - shouldReport = true - } else if ( - containingClassSymbol is FirRegularClassSymbol && - containingClassSymbol.isCompanion && - context.containerIsInterface(1) && - !supportJvmStaticInInterface - ) { - shouldReport = true - } - - if (!shouldReport) { - return - } - - annotatedParts.forEach { - reporter.reportOn(it.source, properDiagnostic, context) - } - } - private fun checkOverrideCannotBeStatic( declaration: FirMemberDeclaration, context: CheckerContext, reporter: DiagnosticReporter, - annotatedParts: List, + targetSource: FirSourceElement?, + outerProperty: FirProperty? = null, ) { - if (!declaration.isOverride || !context.containerIsNonCompanionObject(0)) { + val isOverride = outerProperty?.isOverride ?: declaration.isOverride + + if (!isOverride || !context.containerIsNonCompanionObject(0)) { return } - annotatedParts.forEach { - reporter.reportOn(it.source, FirJvmErrors.OVERRIDE_CANNOT_BE_STATIC, context) - } + reporter.reportOn(targetSource, FirJvmErrors.OVERRIDE_CANNOT_BE_STATIC, context) } - private fun FirAnnotatedDeclaration.getAnnotatedParts(): List { - return getReportableParts().filter { - it.hasAnnotationNamedAs(StandardClassIds.JvmStatic) + private fun checkStaticOnConstOrJvmField( + declaration: FirAnnotatedDeclaration, + context: CheckerContext, + reporter: DiagnosticReporter, + targetSource: FirSourceElement?, + ) { + if ( + declaration is FirProperty && declaration.isConst || + declaration.hasAnnotationNamedAs(StandardClassIds.JvmField) + ) { + reporter.reportOn(targetSource, FirJvmErrors.JVM_STATIC_ON_CONST_OR_JVM_FIELD, context) } } - private fun FirAnnotatedDeclaration.getReportableParts(): List { - val parts = mutableListOf(this) - - if (this is FirProperty) { - fun takeIfNecessary(it: FirPropertyAccessor) { - if (it.visibility.compatibleAndLesser(this.visibility)) { - parts.add(it) - } - } - - this.getter?.let(::takeIfNecessary) - this.setter?.let(::takeIfNecessary) - } - - return parts - } - - private fun Visibility.compatibleAndLesser(other: Visibility): Boolean { - val difference = this.compareTo(other) ?: return false - return difference <= 0 - } - private fun CheckerContext.containerIsInterface(outerLevel: Int): Boolean { return this.getContainerAt(outerLevel)?.classKind?.isInterface == true } @@ -222,7 +225,12 @@ object FirJvmStaticChecker : FirBasicDeclarationChecker() { } private fun CheckerContext.getContainerAt(outerLevel: Int): FirClassLikeSymbol<*>? { - val last = this.containingDeclarations.asReversed().getOrNull(outerLevel) + val correction = if (this.containingDeclarations.lastOrNull() is FirProperty) { + 1 + } else { + 0 + } + val last = this.containingDeclarations.asReversed().getOrNull(outerLevel + correction) return if (last is FirClassLikeDeclaration) { last.symbol } else { @@ -230,13 +238,17 @@ object FirJvmStaticChecker : FirBasicDeclarationChecker() { } } + private fun CheckerContext.supports(feature: LanguageFeature) = session.languageVersionSettings.supportsFeature(feature) + + private fun FirClassLikeSymbol<*>.isCompanion() = safeAs()?.isCompanion == true + private fun FirAnnotatedDeclaration.hasAnnotationNamedAs(classId: ClassId): Boolean { - return findAnnotation(classId.shortClassName) != null + return findAnnotation(classId) != null } - private fun FirAnnotatedDeclaration.findAnnotation(name: Name): FirAnnotationCall? { + private fun FirAnnotatedDeclaration.findAnnotation(classId: ClassId): FirAnnotationCall? { return annotations.firstOrNull { - it.calleeReference.safeAs()?.name == name + it.calleeReference.safeAs()?.name == classId.shortClassName } } } diff --git a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/functions.fir.kt b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/functions.fir.kt deleted file mode 100644 index 160949cca94..00000000000 --- a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/functions.fir.kt +++ /dev/null @@ -1,53 +0,0 @@ -// !LANGUAGE: -JvmStaticInInterface -// !DIAGNOSTICS: -UNUSED_VARIABLE -class A { - companion object { - @JvmStatic fun a1() { - - } - } - - object A { - @JvmStatic fun a2() { - - } - } - - fun test() { - val s = object { - @JvmStatic fun a3() { - - } - } - } - - @JvmStatic fun a4() { - - } -} - -interface B { - companion object { - @JvmStatic fun a1() { - - } - } - - object A { - @JvmStatic fun a2() { - - } - } - - fun test() { - val s = object { - @JvmStatic fun a3() { - - } - } - } - - @JvmStatic fun a4() { - - } -} diff --git a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/functions.kt b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/functions.kt index fbbede0ab34..21d901ae2b4 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/functions.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/functions.kt @@ -1,3 +1,4 @@ +// FIR_IDENTICAL // !LANGUAGE: -JvmStaticInInterface // !DIAGNOSTICS: -UNUSED_VARIABLE class A { diff --git a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/functions_LL13.fir.kt b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/functions_LL13.fir.kt deleted file mode 100644 index fca77383441..00000000000 --- a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/functions_LL13.fir.kt +++ /dev/null @@ -1,53 +0,0 @@ -// !DIAGNOSTICS: -UNUSED_VARIABLE -// !LANGUAGE: +JvmStaticInInterface -class A { - companion object { - @JvmStatic fun a1() { - - } - } - - object A { - @JvmStatic fun a2() { - - } - } - - fun test() { - val s = object { - @JvmStatic fun a3() { - - } - } - } - - @JvmStatic fun a4() { - - } -} - -interface B { - companion object { - @JvmStatic fun a1() { - - } - } - - object A { - @JvmStatic fun a2() { - - } - } - - fun test() { - val s = object { - @JvmStatic fun a3() { - - } - } - } - - @JvmStatic fun a4() { - - } -} diff --git a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/functions_LL13.kt b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/functions_LL13.kt index 621f6dc93c7..a486df53fb9 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/functions_LL13.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/functions_LL13.kt @@ -1,3 +1,4 @@ +// FIR_IDENTICAL // !DIAGNOSTICS: -UNUSED_VARIABLE // !LANGUAGE: +JvmStaticInInterface class A { diff --git a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/interfaceCompanion_LL12.fir.kt b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/interfaceCompanion_LL12.fir.kt index 4f66bc393b4..450bfc69c6b 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/interfaceCompanion_LL12.fir.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/interfaceCompanion_LL12.fir.kt @@ -6,38 +6,38 @@ interface B { } - @JvmStatic private fun a2() { + @JvmStatic private fun a2() { } - @JvmStatic protected fun a3() { + @JvmStatic protected fun a3() { } - @JvmStatic internal fun a4() { + @JvmStatic internal fun a4() { } - @JvmStatic external fun a5() + @JvmStatic external fun a5() @JvmStatic var foo = 1 - @JvmStatic + @JvmStatic var foo1 = 1 protected set - @JvmStatic + @JvmStatic var foo2 = 1 private set - @JvmStatic + @JvmStatic private var foo3 = 1 - @JvmStatic + @JvmStatic protected var foo4 = 1 - @JvmStatic + @JvmStatic protected var foo5 = 1 @JvmStatic @@ -47,10 +47,10 @@ interface B { @JvmStatic get private var foo8 = 1 - @JvmStatic public set + @JvmStatic public set public var foo9 = 1 - @JvmStatic private set + @JvmStatic private set @JvmStatic val foo10: Int external get diff --git a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/interfaceCompanion_LL13_16.fir.kt b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/interfaceCompanion_LL13_16.fir.kt index a9f224f8a56..63f1ee5e26e 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/interfaceCompanion_LL13_16.fir.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/interfaceCompanion_LL13_16.fir.kt @@ -54,10 +54,10 @@ interface B { public var foo9 = 1 @JvmStatic private set - @JvmStatic - val foo10: Int external get + @JvmStatic + val foo10: Int external get - val foo11: Int @JvmStatic external get + val foo11: Int @JvmStatic external get } } diff --git a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/interfaceCompanion_LL13_18.fir.kt b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/interfaceCompanion_LL13_18.fir.kt index ca96f7f72f6..8200b39259f 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/interfaceCompanion_LL13_18.fir.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/interfaceCompanion_LL13_18.fir.kt @@ -53,10 +53,10 @@ interface B { public var foo9 = 1 @JvmStatic private set - @JvmStatic - val foo10: Int external get + @JvmStatic + val foo10: Int external get - val foo11: Int @JvmStatic external get + val foo11: Int @JvmStatic external get } }