From a818c543a9cba2068dfe5a3b534a9724316838d7 Mon Sep 17 00:00:00 2001 From: Kirill Rakhman Date: Wed, 26 Apr 2023 16:19:27 +0200 Subject: [PATCH] [FIR] Verify diagnostic messages don't reference non-existent parameters --- .../diagnostics/FirErrorsDefaultMessages.kt | 2 +- .../diagnostics/KtDiagnosticRenderer.kt | 6 ++- .../utils/FirErrorsDefaultMessagesHelper.kt | 38 +++++++++++++------ .../kotlin/checkers/DefaultMessagesTest.kt | 12 +++--- .../test/ParcelizeDefaultMessagesTest.kt | 4 +- 5 files changed, 39 insertions(+), 23 deletions(-) diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrorsDefaultMessages.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrorsDefaultMessages.kt index c6b57d55842..25972c6043e 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrorsDefaultMessages.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrorsDefaultMessages.kt @@ -1843,7 +1843,7 @@ object FirErrorsDefaultMessages : BaseDiagnosticRendererFactory() { ) map.put( WRONG_INVOCATION_KIND, - "{2} wrong invocation kind: given {3} case, but {4} case is possible", + "{0} has wrong invocation kind: given {1} case, but {2} case is possible", SYMBOL, TO_STRING, TO_STRING diff --git a/compiler/frontend.common/src/org/jetbrains/kotlin/diagnostics/KtDiagnosticRenderer.kt b/compiler/frontend.common/src/org/jetbrains/kotlin/diagnostics/KtDiagnosticRenderer.kt index 496ae7861a8..1adabf72d95 100644 --- a/compiler/frontend.common/src/org/jetbrains/kotlin/diagnostics/KtDiagnosticRenderer.kt +++ b/compiler/frontend.common/src/org/jetbrains/kotlin/diagnostics/KtDiagnosticRenderer.kt @@ -5,17 +5,19 @@ package org.jetbrains.kotlin.diagnostics +import com.google.common.annotations.VisibleForTesting import org.jetbrains.kotlin.diagnostics.rendering.DiagnosticParameterRenderer import org.jetbrains.kotlin.diagnostics.rendering.RenderingContext import org.jetbrains.kotlin.diagnostics.rendering.renderParameter import java.text.MessageFormat sealed interface KtDiagnosticRenderer { + @VisibleForTesting val message: String fun render(diagnostic: KtDiagnostic): String fun renderParameters(diagnostic: KtDiagnostic): Array } -class SimpleKtDiagnosticRenderer(private val message: String) : KtDiagnosticRenderer { +class SimpleKtDiagnosticRenderer(override val message: String) : KtDiagnosticRenderer { override fun render(diagnostic: KtDiagnostic): String { require(diagnostic is KtSimpleDiagnostic) return message @@ -28,7 +30,7 @@ class SimpleKtDiagnosticRenderer(private val message: String) : KtDiagnosticRend } sealed class AbstractKtDiagnosticWithParametersRenderer( - protected val message: String + final override val message: String ) : KtDiagnosticRenderer { private val messageFormat = MessageFormat(message) diff --git a/compiler/tests-common-new/tests/org/jetbrains/kotlin/test/utils/FirErrorsDefaultMessagesHelper.kt b/compiler/tests-common-new/tests/org/jetbrains/kotlin/test/utils/FirErrorsDefaultMessagesHelper.kt index 683dd9b8cd2..d324dc60566 100644 --- a/compiler/tests-common-new/tests/org/jetbrains/kotlin/test/utils/FirErrorsDefaultMessagesHelper.kt +++ b/compiler/tests-common-new/tests/org/jetbrains/kotlin/test/utils/FirErrorsDefaultMessagesHelper.kt @@ -5,29 +5,43 @@ package org.jetbrains.kotlin.test.utils -import org.jetbrains.kotlin.diagnostics.AbstractKtDiagnosticFactory -import org.jetbrains.kotlin.diagnostics.KtDiagnosticFactoryForDeprecation -import org.jetbrains.kotlin.diagnostics.KtDiagnosticFactoryToRendererMap +import org.jetbrains.kotlin.diagnostics.* +import org.junit.Assert import kotlin.reflect.KProperty import kotlin.reflect.full.memberProperties -fun KtDiagnosticFactoryToRendererMap.checkMissingMessages(objectWithErrors: Any) { - for (property in objectWithErrors.javaClass.kotlin.memberProperties) { +fun KtDiagnosticFactoryToRendererMap.verifyMessages(objectWithErrors: Any) { + for (property in objectWithErrors::class.memberProperties) { when (val factory = property.getter.call(objectWithErrors)) { is AbstractKtDiagnosticFactory -> { - checkMissingMessagesForFactory(factory, property) + verifyMessageForFactory(factory, property) } is KtDiagnosticFactoryForDeprecation<*> -> { - checkMissingMessagesForFactory(factory.warningFactory, property) - checkMissingMessagesForFactory(factory.errorFactory, property) + verifyMessageForFactory(factory.warningFactory, property) + verifyMessageForFactory(factory.errorFactory, property) } else -> {} } } } -fun KtDiagnosticFactoryToRendererMap.checkMissingMessagesForFactory(factory: AbstractKtDiagnosticFactory, property: KProperty<*>) { - if (!containsKey(factory)) { - throw IllegalStateException("No default diagnostic renderer is provided for ${property.name}") +private val messageParameterRegex = """\{\d.*?}""".toRegex() + +fun KtDiagnosticFactoryToRendererMap.verifyMessageForFactory(factory: AbstractKtDiagnosticFactory, property: KProperty<*>) { + Assert.assertTrue("No default diagnostic renderer is provided for ${property.name}", containsKey(factory)) + + val renderer = get(factory)!! + + val parameterCount = when (renderer) { + is KtDiagnosticWithParameters4Renderer<*, *, *, *> -> 4 + is KtDiagnosticWithParameters3Renderer<*, *, *> -> 3 + is KtDiagnosticWithParameters2Renderer<*, *> -> 2 + is KtDiagnosticWithParameters1Renderer<*> -> 1 + else -> 0 } -} + + for (parameter in messageParameterRegex.findAll(renderer.message)) { + val index = parameter.value.substring(1, 2).toInt() + Assert.assertTrue("Message for ${property.name} references wrong parameter {$index}", index < parameterCount) + } +} \ No newline at end of file diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DefaultMessagesTest.kt b/compiler/tests/org/jetbrains/kotlin/checkers/DefaultMessagesTest.kt index 8edfa3738fe..283ca1563eb 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DefaultMessagesTest.kt +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DefaultMessagesTest.kt @@ -15,16 +15,16 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.native.FirNativeErrors import org.jetbrains.kotlin.fir.analysis.diagnostics.native.FirNativeErrorsDefaultMessages import org.jetbrains.kotlin.fir.builder.FirSyntaxErrors import org.jetbrains.kotlin.fir.builder.FirSyntaxErrorsDefaultMessages -import org.jetbrains.kotlin.test.utils.checkMissingMessages +import org.jetbrains.kotlin.test.utils.verifyMessages import org.junit.Test class DefaultMessagesTest { @Test fun ensureAllMessagesPresent() { - FirErrorsDefaultMessages.MAP.checkMissingMessages(FirErrors) - FirJvmErrorsDefaultMessages.MAP.checkMissingMessages(FirJvmErrors) - FirJsErrorsDefaultMessages.MAP.checkMissingMessages(FirJsErrors) - FirNativeErrorsDefaultMessages.MAP.checkMissingMessages(FirNativeErrors) - FirSyntaxErrorsDefaultMessages.MAP.checkMissingMessages(FirSyntaxErrors) + FirErrorsDefaultMessages.MAP.verifyMessages(FirErrors) + FirJvmErrorsDefaultMessages.MAP.verifyMessages(FirJvmErrors) + FirJsErrorsDefaultMessages.MAP.verifyMessages(FirJsErrors) + FirNativeErrorsDefaultMessages.MAP.verifyMessages(FirNativeErrors) + FirSyntaxErrorsDefaultMessages.MAP.verifyMessages(FirSyntaxErrors) } } diff --git a/plugins/parcelize/parcelize-compiler/tests/org/jetbrains/kotlin/parcelize/test/ParcelizeDefaultMessagesTest.kt b/plugins/parcelize/parcelize-compiler/tests/org/jetbrains/kotlin/parcelize/test/ParcelizeDefaultMessagesTest.kt index fd862736c23..f31e1717bbc 100644 --- a/plugins/parcelize/parcelize-compiler/tests/org/jetbrains/kotlin/parcelize/test/ParcelizeDefaultMessagesTest.kt +++ b/plugins/parcelize/parcelize-compiler/tests/org/jetbrains/kotlin/parcelize/test/ParcelizeDefaultMessagesTest.kt @@ -7,12 +7,12 @@ package org.jetbrains.kotlin.parcelize.test import org.jetbrains.kotlin.parcelize.fir.diagnostics.KtDefaultErrorMessagesParcelize import org.jetbrains.kotlin.parcelize.fir.diagnostics.KtErrorsParcelize -import org.jetbrains.kotlin.test.utils.checkMissingMessages +import org.jetbrains.kotlin.test.utils.verifyMessages import org.junit.jupiter.api.Test class ParcelizeDefaultMessagesTest { @Test fun ensureAllMessagesPresent() { - KtDefaultErrorMessagesParcelize.MAP.checkMissingMessages(KtErrorsParcelize) + KtDefaultErrorMessagesParcelize.MAP.verifyMessages(KtErrorsParcelize) } }