From 779aa75855d139b3b8cb6d4890a81cff31f74928 Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Thu, 3 Mar 2022 10:49:56 +0300 Subject: [PATCH] Make FE1.0/FIR OptIn override message more clear --- .../expression/FirOptInUsageBaseChecker.kt | 55 +++++++++++++++---- .../checkers/ExperimentalUsageChecker.kt | 10 ++-- .../testData/cli/jvm/optInEmptyMessageFir.out | 2 +- .../cli/jvm/optInOverrideMessage.args | 4 ++ .../testData/cli/jvm/optInOverrideMessage.kt | 19 +++++++ .../testData/cli/jvm/optInOverrideMessage.out | 7 +++ .../cli/jvm/optInOverrideMessageFir.args | 5 ++ .../cli/jvm/optInOverrideMessageFir.kt | 19 +++++++ .../cli/jvm/optInOverrideMessageFir.out | 10 ++++ .../kotlin/cli/CliTestGenerated.java | 10 ++++ .../kotlin/resolve/checkers/OptInNames.kt | 8 +++ 11 files changed, 133 insertions(+), 16 deletions(-) create mode 100644 compiler/testData/cli/jvm/optInOverrideMessage.args create mode 100644 compiler/testData/cli/jvm/optInOverrideMessage.kt create mode 100644 compiler/testData/cli/jvm/optInOverrideMessage.out create mode 100644 compiler/testData/cli/jvm/optInOverrideMessageFir.args create mode 100644 compiler/testData/cli/jvm/optInOverrideMessageFir.kt create mode 100644 compiler/testData/cli/jvm/optInOverrideMessageFir.out diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirOptInUsageBaseChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirOptInUsageBaseChecker.kt index 896348d86f4..deb049328b1 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirOptInUsageBaseChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirOptInUsageBaseChecker.kt @@ -37,17 +37,41 @@ import org.jetbrains.kotlin.utils.SmartSet import org.jetbrains.kotlin.utils.addIfNotNull object FirOptInUsageBaseChecker { - data class Experimentality(val annotationClassId: ClassId, val severity: Severity, val message: String?) { + data class Experimentality( + val annotationClassId: ClassId, + val severity: Severity, + val message: String?, + val supertypeName: String? = null + ) { enum class Severity { WARNING, ERROR } companion object { val DEFAULT_SEVERITY = Severity.ERROR } + + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is Experimentality) return false + + if (annotationClassId != other.annotationClassId) return false + if (severity != other.severity) return false + if (message != other.message) return false + + return true + } + + override fun hashCode(): Int { + var result = annotationClassId.hashCode() + result = 31 * result + severity.hashCode() + result = 31 * result + (message?.hashCode() ?: 0) + return result + } } - fun FirRegularClassSymbol.loadExperimentalityForMarkerAnnotation(): Experimentality? { + // Note: receiver is an OptIn marker class and parameter is an annotated member owner class / self class name + fun FirRegularClassSymbol.loadExperimentalityForMarkerAnnotation(annotatedOwnerClassName: String? = null): Experimentality? { ensureResolved(FirResolvePhase.BODY_RESOLVE) @OptIn(SymbolInternals::class) - return fir.loadExperimentalityForMarkerAnnotation() + return fir.loadExperimentalityForMarkerAnnotation(annotatedOwnerClassName) } fun FirBasedSymbol<*>.loadExperimentalitiesFromAnnotationTo(session: FirSession, result: MutableCollection) { @@ -62,10 +86,15 @@ object FirOptInUsageBaseChecker { ) { for (annotation in annotations) { val annotationType = annotation.annotationTypeRef.coneTypeSafe() + val className = when (this) { + is FirRegularClass -> name.asString() + is FirCallableDeclaration -> symbol.callableId.className?.shortName()?.asString() + else -> null + } result.addIfNotNull( annotationType?.lookupTag?.toFirRegularClassSymbol( session - )?.loadExperimentalityForMarkerAnnotation() + )?.loadExperimentalityForMarkerAnnotation(className) ) } } @@ -183,7 +212,8 @@ object FirOptInUsageBaseChecker { } } - private fun FirRegularClass.loadExperimentalityForMarkerAnnotation(): Experimentality? { + // Note: receiver is an OptIn marker class and parameter is an annotated member owner class / self class name + private fun FirRegularClass.loadExperimentalityForMarkerAnnotation(annotatedOwnerClassName: String? = null): Experimentality? { val experimental = getAnnotationByClassId(OptInNames.REQUIRES_OPT_IN_CLASS_ID) ?: return null @@ -191,7 +221,7 @@ object FirOptInUsageBaseChecker { val levelName = levelArgument?.calleeReference?.resolved?.name?.asString() val level = OptInLevel.values().firstOrNull { it.name == levelName } ?: OptInLevel.DEFAULT val message = (experimental.findArgumentByName(MESSAGE) as? FirConstExpression<*>)?.value as? String - return Experimentality(symbol.classId, level.severity, message) + return Experimentality(symbol.classId, level.severity, message, annotatedOwnerClassName) } fun reportNotAcceptedExperimentalities( @@ -221,7 +251,7 @@ object FirOptInUsageBaseChecker { context: CheckerContext, reporter: DiagnosticReporter ) { - for ((annotationClassId, severity, _) in experimentalities) { + for ((annotationClassId, severity, markerMessage, supertypeName) in experimentalities) { if (!symbol.fir.isExperimentalityAcceptable(annotationClassId) && !isExperimentalityAcceptableInContext(annotationClassId, context) ) { @@ -229,10 +259,13 @@ object FirOptInUsageBaseChecker { Experimentality.Severity.WARNING -> FirErrors.OPT_IN_OVERRIDE to "should" Experimentality.Severity.ERROR -> FirErrors.OPT_IN_OVERRIDE_ERROR to "must" } - val reportedMessage = "This declaration overrides experimental member of supertype " + - "'${symbol.callableId.className?.shortName()?.asString()}' and $verb be annotated " + - "with '@${annotationClassId.asFqNameString()}'" - reporter.reportOn(symbol.source, diagnostic, annotationClassId.asSingleFqName(), reportedMessage, context) + val message = OptInNames.buildOverrideMessage( + supertypeName ?: "???", + markerMessage, + verb, + markerName = annotationClassId.asFqNameString() + ) + reporter.reportOn(symbol.source, diagnostic, annotationClassId.asSingleFqName(), message, context) } } } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ExperimentalUsageChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ExperimentalUsageChecker.kt index 5a5855d581c..4a3c90f5108 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ExperimentalUsageChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ExperimentalUsageChecker.kt @@ -480,10 +480,12 @@ class ExperimentalUsageChecker(project: Project) : CallChecker { Experimentality.Severity.ERROR -> Errors.OPT_IN_OVERRIDE_ERROR to "must" Experimentality.Severity.FUTURE_ERROR -> Errors.OPT_IN_OVERRIDE_ERROR to "must" } - val message = experimentality.message?.takeIf { it.isNotBlank() } - ?: "This declaration overrides experimental member of supertype " + - "'${member.containingDeclaration.name.asString()}' and $defaultMessageVerb be annotated " + - "with '@${experimentality.annotationFqName.asString()}'" + val message = OptInNames.buildOverrideMessage( + supertypeName = member.containingDeclaration.name.asString(), + markerMessage = experimentality.message, + verb = defaultMessageVerb, + markerName = experimentality.annotationFqName.asString() + ) context.trace.report(diagnostic.on(reportOn, experimentality.annotationFqName, message)) } } diff --git a/compiler/testData/cli/jvm/optInEmptyMessageFir.out b/compiler/testData/cli/jvm/optInEmptyMessageFir.out index a3a60f4d360..93e72cd154c 100644 --- a/compiler/testData/cli/jvm/optInEmptyMessageFir.out +++ b/compiler/testData/cli/jvm/optInEmptyMessageFir.out @@ -1,7 +1,7 @@ warning: ATTENTION! This build uses in-dev FIR: -Xuse-fir -compiler/testData/cli/jvm/optInEmptyMessage.kt:8:5: error: this declaration needs OptIn. Its usage must be marked with '@EmptyMarker' or '@OptIn(EmptyMarker::class)' +compiler/testData/cli/jvm/optInEmptyMessageFir.kt:8:5: error: this declaration needs OptIn. Its usage must be marked with '@EmptyMarker' or '@OptIn(EmptyMarker::class)' foo() ^ COMPILATION_ERROR diff --git a/compiler/testData/cli/jvm/optInOverrideMessage.args b/compiler/testData/cli/jvm/optInOverrideMessage.args new file mode 100644 index 00000000000..868b878c363 --- /dev/null +++ b/compiler/testData/cli/jvm/optInOverrideMessage.args @@ -0,0 +1,4 @@ +$TESTDATA_DIR$/optInOverrideMessage.kt +-d +$TEMP_DIR$ +-opt-in=kotlin.RequiresOptIn \ No newline at end of file diff --git a/compiler/testData/cli/jvm/optInOverrideMessage.kt b/compiler/testData/cli/jvm/optInOverrideMessage.kt new file mode 100644 index 00000000000..146b3567e77 --- /dev/null +++ b/compiler/testData/cli/jvm/optInOverrideMessage.kt @@ -0,0 +1,19 @@ +@RequiresOptIn(message = "This API is experimental and can change at any time, please use with care") +annotation class Marker + +@RequiresOptIn +annotation class EmptyMarker + +interface Base { + @Marker + fun foo() + + @EmptyMarker + fun bar() +} + +class Derived : Base { + override fun foo() {} + + override fun bar() {} +} diff --git a/compiler/testData/cli/jvm/optInOverrideMessage.out b/compiler/testData/cli/jvm/optInOverrideMessage.out new file mode 100644 index 00000000000..78c82f2c1ff --- /dev/null +++ b/compiler/testData/cli/jvm/optInOverrideMessage.out @@ -0,0 +1,7 @@ +compiler/testData/cli/jvm/optInOverrideMessage.kt:16:18: error: base declaration of supertype 'Base' needs OptIn. This API is experimental and can change at any time, please use with care. The declaration override must be annotated with '@Marker' + override fun foo() {} + ^ +compiler/testData/cli/jvm/optInOverrideMessage.kt:18:18: error: base declaration of supertype 'Base' needs OptIn. The declaration override must be annotated with '@EmptyMarker' + override fun bar() {} + ^ +COMPILATION_ERROR diff --git a/compiler/testData/cli/jvm/optInOverrideMessageFir.args b/compiler/testData/cli/jvm/optInOverrideMessageFir.args new file mode 100644 index 00000000000..75ec0f2cd3e --- /dev/null +++ b/compiler/testData/cli/jvm/optInOverrideMessageFir.args @@ -0,0 +1,5 @@ +$TESTDATA_DIR$/optInOverrideMessageFir.kt +-d +$TEMP_DIR$ +-opt-in=kotlin.RequiresOptIn +-Xuse-fir diff --git a/compiler/testData/cli/jvm/optInOverrideMessageFir.kt b/compiler/testData/cli/jvm/optInOverrideMessageFir.kt new file mode 100644 index 00000000000..146b3567e77 --- /dev/null +++ b/compiler/testData/cli/jvm/optInOverrideMessageFir.kt @@ -0,0 +1,19 @@ +@RequiresOptIn(message = "This API is experimental and can change at any time, please use with care") +annotation class Marker + +@RequiresOptIn +annotation class EmptyMarker + +interface Base { + @Marker + fun foo() + + @EmptyMarker + fun bar() +} + +class Derived : Base { + override fun foo() {} + + override fun bar() {} +} diff --git a/compiler/testData/cli/jvm/optInOverrideMessageFir.out b/compiler/testData/cli/jvm/optInOverrideMessageFir.out new file mode 100644 index 00000000000..54278f08d8e --- /dev/null +++ b/compiler/testData/cli/jvm/optInOverrideMessageFir.out @@ -0,0 +1,10 @@ +warning: ATTENTION! + This build uses in-dev FIR: + -Xuse-fir +compiler/testData/cli/jvm/optInOverrideMessageFir.kt:16:18: error: base declaration of supertype 'Base' needs OptIn. This API is experimental and can change at any time, please use with care. The declaration override must be annotated with '@Marker' + override fun foo() {} + ^ +compiler/testData/cli/jvm/optInOverrideMessageFir.kt:18:18: error: base declaration of supertype 'Base' needs OptIn. The declaration override must be annotated with '@EmptyMarker' + override fun bar() {} + ^ +COMPILATION_ERROR diff --git a/compiler/tests-gen/org/jetbrains/kotlin/cli/CliTestGenerated.java b/compiler/tests-gen/org/jetbrains/kotlin/cli/CliTestGenerated.java index ee290991dde..0d61a34eedb 100644 --- a/compiler/tests-gen/org/jetbrains/kotlin/cli/CliTestGenerated.java +++ b/compiler/tests-gen/org/jetbrains/kotlin/cli/CliTestGenerated.java @@ -809,6 +809,16 @@ public class CliTestGenerated extends AbstractCliTest { runTest("compiler/testData/cli/jvm/optInEmptyMessageFir.args"); } + @TestMetadata("optInOverrideMessage.args") + public void testOptInOverrideMessage() throws Exception { + runTest("compiler/testData/cli/jvm/optInOverrideMessage.args"); + } + + @TestMetadata("optInOverrideMessageFir.args") + public void testOptInOverrideMessageFir() throws Exception { + runTest("compiler/testData/cli/jvm/optInOverrideMessageFir.args"); + } + @TestMetadata("pluginSimple.args") public void testPluginSimple() throws Exception { runTest("compiler/testData/cli/jvm/pluginSimple.args"); diff --git a/core/compiler.common/src/org/jetbrains/kotlin/resolve/checkers/OptInNames.kt b/core/compiler.common/src/org/jetbrains/kotlin/resolve/checkers/OptInNames.kt index 2ef0955a402..cb15382d821 100644 --- a/core/compiler.common/src/org/jetbrains/kotlin/resolve/checkers/OptInNames.kt +++ b/core/compiler.common/src/org/jetbrains/kotlin/resolve/checkers/OptInNames.kt @@ -45,4 +45,12 @@ object OptInNames { fun buildMessagePrefix(verb: String): String = "This declaration needs OptIn. Its usage $verb be marked" + + fun buildOverrideMessage(supertypeName: String, markerMessage: String?, verb: String, markerName: String): String { + val basePrefix = "Base declaration of supertype '$supertypeName' needs OptIn. " + val markerMessageOrStub = markerMessage + ?.takeIf { it.isNotBlank() }?.let { if (it.endsWith(".")) "$it " else "$it. " } ?: "" + val baseSuffix = "The declaration override $verb be annotated with '@$markerName'" + return basePrefix + markerMessageOrStub + baseSuffix + } } \ No newline at end of file