From 4c29d2dbc7104c3afa013961419ceaeb839c0ec2 Mon Sep 17 00:00:00 2001 From: Brian Norman Date: Fri, 25 Sep 2020 12:21:04 -0500 Subject: [PATCH 1/4] Allow constants expression as argument to assertion function --- .../com/bnorm/power/PowerAssertCallTransformer.kt | 7 ++++--- .../src/test/kotlin/com/bnorm/power/test.kt | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/kotlin-power-assert-plugin/src/main/kotlin/com/bnorm/power/PowerAssertCallTransformer.kt b/kotlin-power-assert-plugin/src/main/kotlin/com/bnorm/power/PowerAssertCallTransformer.kt index 9558934f06a..d24fdd01194 100644 --- a/kotlin-power-assert-plugin/src/main/kotlin/com/bnorm/power/PowerAssertCallTransformer.kt +++ b/kotlin-power-assert-plugin/src/main/kotlin/com/bnorm/power/PowerAssertCallTransformer.kt @@ -105,6 +105,10 @@ class PowerAssertCallTransformer( val assertionArgument = expression.getValueArgument(0)!! val messageArgument = if (function.valueParameters.size == 2) expression.getValueArgument(1) else null + // If the tree does not contain any children, the expression is not transformable + val tree = buildAssertTree(assertionArgument) + val root = tree.children.singleOrNull() ?: return super.visitCall(expression) + val symbol = currentScope!!.scope.scopeOwnerSymbol DeclarationIrBuilder(context, symbol).run { at(expression) @@ -129,9 +133,6 @@ class PowerAssertCallTransformer( } } - val tree = buildAssertTree(assertionArgument) - val root = tree.children.single() - // println(assertionArgument.dump()) // println(tree.dump()) diff --git a/kotlin-power-assert-plugin/src/test/kotlin/com/bnorm/power/test.kt b/kotlin-power-assert-plugin/src/test/kotlin/com/bnorm/power/test.kt index e088ad45dbc..86c8b0d0306 100644 --- a/kotlin-power-assert-plugin/src/test/kotlin/com/bnorm/power/test.kt +++ b/kotlin-power-assert-plugin/src/test/kotlin/com/bnorm/power/test.kt @@ -399,6 +399,20 @@ assert(a == 42) 0 """.trimIndent()) } + + @Test + fun constantExpression() { + assertMessage( + """ +fun main() { + assert(true) + assert(false) +}""", + """ +Assertion failed +""".trimIndent() + ) + } } fun assertMessage( From 820ea18ebc9715792dc156fa719c0f6d062f22e3 Mon Sep 17 00:00:00 2001 From: Brian Norman Date: Fri, 25 Sep 2020 15:15:36 -0500 Subject: [PATCH 2/4] Add a reproduction of #22 to the sample project --- sample/build.gradle.kts | 2 +- .../com/bnorm/power/JsPowerAssertTest.kt | 26 +++++++++++++++++++ .../com/bnorm/power/JvmPowerAssertTest.kt | 2 +- .../com/bnorm/power/NativePowerAssertTest.kt | 26 +++++++++++++++++++ 4 files changed, 54 insertions(+), 2 deletions(-) create mode 100644 sample/src/jsTest/kotlin/com/bnorm/power/JsPowerAssertTest.kt create mode 100644 sample/src/nativeTest/kotlin/com/bnorm/power/NativePowerAssertTest.kt diff --git a/sample/build.gradle.kts b/sample/build.gradle.kts index 7d95a28c4ed..b366bbe7f47 100644 --- a/sample/build.gradle.kts +++ b/sample/build.gradle.kts @@ -57,5 +57,5 @@ kotlin { } configure { - functions = listOf("kotlin.test.assertTrue", "kotlin.require") + functions = listOf("kotlin.assert", "kotlin.test.assertTrue", "kotlin.require") } diff --git a/sample/src/jsTest/kotlin/com/bnorm/power/JsPowerAssertTest.kt b/sample/src/jsTest/kotlin/com/bnorm/power/JsPowerAssertTest.kt new file mode 100644 index 00000000000..111d332d15c --- /dev/null +++ b/sample/src/jsTest/kotlin/com/bnorm/power/JsPowerAssertTest.kt @@ -0,0 +1,26 @@ +/* + * Copyright (C) 2020 Brian Norman + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.bnorm.power + +import kotlin.test.Test + +class JsPowerAssertTest { + @Test + fun assert() { + require(Person.UNKNOWN.size == 1) { "unknown persons: ${Person.UNKNOWN}" } + } +} diff --git a/sample/src/jvmTest/kotlin/com/bnorm/power/JvmPowerAssertTest.kt b/sample/src/jvmTest/kotlin/com/bnorm/power/JvmPowerAssertTest.kt index 233d3de951a..2a0f2071f28 100644 --- a/sample/src/jvmTest/kotlin/com/bnorm/power/JvmPowerAssertTest.kt +++ b/sample/src/jvmTest/kotlin/com/bnorm/power/JvmPowerAssertTest.kt @@ -21,6 +21,6 @@ import kotlin.test.Test class JvmPowerAssertTest { @Test fun assert() { - assert(Person.UNKNOWN.size == 1) + assert(Person.UNKNOWN.size == 1) { "unknown persons: ${Person.UNKNOWN}" } } } diff --git a/sample/src/nativeTest/kotlin/com/bnorm/power/NativePowerAssertTest.kt b/sample/src/nativeTest/kotlin/com/bnorm/power/NativePowerAssertTest.kt new file mode 100644 index 00000000000..267dcbb0fd0 --- /dev/null +++ b/sample/src/nativeTest/kotlin/com/bnorm/power/NativePowerAssertTest.kt @@ -0,0 +1,26 @@ +/* + * Copyright (C) 2020 Brian Norman + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.bnorm.power + +import kotlin.test.Test + +class NativePowerAssertTest { + @Test + fun assert() { + assert(Person.UNKNOWN.size == 1) { "unknown persons: ${Person.UNKNOWN}" } + } +} From 3eebeca75322e210c2ed5a360789428ed68698b6 Mon Sep 17 00:00:00 2001 From: Brian Norman Date: Fri, 25 Sep 2020 15:23:52 -0500 Subject: [PATCH 3/4] Transform returnable blocks after inlining --- .../bnorm/power/PowerAssertCallTransformer.kt | 3 +- .../internal/ReturnableBlockTransformer.kt | 97 +++++++++++++++++++ 2 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 kotlin-power-assert-plugin/src/main/kotlin/com/bnorm/power/internal/ReturnableBlockTransformer.kt diff --git a/kotlin-power-assert-plugin/src/main/kotlin/com/bnorm/power/PowerAssertCallTransformer.kt b/kotlin-power-assert-plugin/src/main/kotlin/com/bnorm/power/PowerAssertCallTransformer.kt index d24fdd01194..241f81562e3 100644 --- a/kotlin-power-assert-plugin/src/main/kotlin/com/bnorm/power/PowerAssertCallTransformer.kt +++ b/kotlin-power-assert-plugin/src/main/kotlin/com/bnorm/power/PowerAssertCallTransformer.kt @@ -16,6 +16,7 @@ package com.bnorm.power +import com.bnorm.power.internal.ReturnableBlockTransformer import org.jetbrains.kotlin.backend.common.FileLoweringPass import org.jetbrains.kotlin.backend.common.IrElementTransformerVoidWithContext import org.jetbrains.kotlin.backend.common.extensions.IrPluginContext @@ -120,7 +121,7 @@ class PowerAssertCallTransformer( val title = when { messageArgument is IrConst<*> -> messageArgument messageArgument is IrStringConcatenation -> messageArgument - lambda != null -> lambda.inline(parent) + lambda != null -> lambda.inline(parent).transform(ReturnableBlockTransformer(context, symbol), null) messageArgument != null -> { val invoke = messageArgument.type.getClass()!!.functions.single { it.name == OperatorNameConventions.INVOKE } irCallOp(invoke.symbol, invoke.returnType, messageArgument) diff --git a/kotlin-power-assert-plugin/src/main/kotlin/com/bnorm/power/internal/ReturnableBlockTransformer.kt b/kotlin-power-assert-plugin/src/main/kotlin/com/bnorm/power/internal/ReturnableBlockTransformer.kt new file mode 100644 index 00000000000..13f85e47e20 --- /dev/null +++ b/kotlin-power-assert-plugin/src/main/kotlin/com/bnorm/power/internal/ReturnableBlockTransformer.kt @@ -0,0 +1,97 @@ +/* + * Copyright 2010-2018 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. + * + * Copied from https://github.com/JetBrains/kotlin/blob/1.4.0/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/ReturnableBlockLowering.kt + */ + +package com.bnorm.power.internal + +import org.jetbrains.kotlin.backend.common.IrElementTransformerVoidWithContext +import org.jetbrains.kotlin.backend.common.lower.DeclarationIrBuilder +import org.jetbrains.kotlin.backend.common.lower.irComposite +import org.jetbrains.kotlin.ir.builders.* +import org.jetbrains.kotlin.ir.expressions.* +import org.jetbrains.kotlin.ir.expressions.impl.* +import org.jetbrains.kotlin.ir.symbols.IrReturnableBlockSymbol +import org.jetbrains.kotlin.ir.symbols.IrSymbol + +// TODO Remove when inlining works correctly on Kotlin/JS and Kotlin/Native +class ReturnableBlockTransformer(val context: IrGeneratorContext, val containerSymbol: IrSymbol? = null) : IrElementTransformerVoidWithContext() { + private var labelCnt = 0 + private val returnMap = mutableMapOf IrExpression>() + + override fun visitReturn(expression: IrReturn): IrExpression { + expression.transformChildrenVoid() + return returnMap[expression.returnTargetSymbol]?.invoke(expression) ?: expression + } + + override fun visitContainerExpression(expression: IrContainerExpression): IrExpression { + if (expression !is IrReturnableBlock) return super.visitContainerExpression(expression) + + val scopeSymbol = currentScope?.scope?.scopeOwnerSymbol ?: containerSymbol + val builder = DeclarationIrBuilder(context, scopeSymbol!!) + val variable by lazy { + builder.scope.createTmpVariable(expression.type, "tmp\$ret\$${labelCnt++}", true) + } + + val loop by lazy { + IrDoWhileLoopImpl( + expression.startOffset, + expression.endOffset, + context.irBuiltIns.unitType, + expression.origin + ).apply { + label = "l\$ret\$${labelCnt++}" + condition = builder.irBoolean(false) + } + } + + var hasReturned = false + + returnMap[expression.symbol] = { returnExpression -> + hasReturned = true + builder.irComposite(returnExpression) { + +irSetVar(variable.symbol, returnExpression.value) + +irBreak(loop) + } + } + + val newStatements = expression.statements.mapIndexed { i, s -> + if (i == expression.statements.lastIndex && s is IrReturn && s.returnTargetSymbol == expression.symbol) { + s.transformChildrenVoid() + if (!hasReturned) s.value else { + builder.irSetVar(variable.symbol, s.value) + } + } else { + s.transform(this, null) + } + } + + returnMap.remove(expression.symbol) + + if (!hasReturned) { + return IrCompositeImpl( + expression.startOffset, + expression.endOffset, + expression.type, + expression.origin, + newStatements + ) + } else { + loop.body = IrBlockImpl( + expression.startOffset, + expression.endOffset, + context.irBuiltIns.unitType, + expression.origin, + newStatements + ) + + return builder.irComposite(expression, expression.origin) { + +variable + +loop + +irGet(variable) + } + } + } +} From 96f5cba332c412ead805ef30cc53d52b2992764c Mon Sep 17 00:00:00 2001 From: Brian Norman Date: Fri, 25 Sep 2020 15:59:19 -0500 Subject: [PATCH 4/4] Log messages when expressions can not be transformed --- .../bnorm/power/PowerAssertCallTransformer.kt | 25 ++++++++++++++++--- .../power/PowerAssertComponentRegistrar.kt | 6 ++++- .../power/PowerAssertIrGenerationExtension.kt | 4 ++- 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/kotlin-power-assert-plugin/src/main/kotlin/com/bnorm/power/PowerAssertCallTransformer.kt b/kotlin-power-assert-plugin/src/main/kotlin/com/bnorm/power/PowerAssertCallTransformer.kt index 241f81562e3..e5d06abe7f0 100644 --- a/kotlin-power-assert-plugin/src/main/kotlin/com/bnorm/power/PowerAssertCallTransformer.kt +++ b/kotlin-power-assert-plugin/src/main/kotlin/com/bnorm/power/PowerAssertCallTransformer.kt @@ -24,6 +24,7 @@ import org.jetbrains.kotlin.backend.common.ir.asSimpleLambda import org.jetbrains.kotlin.backend.common.ir.inline import org.jetbrains.kotlin.backend.common.lower.DeclarationIrBuilder import org.jetbrains.kotlin.backend.common.lower.at +import org.jetbrains.kotlin.cli.common.messages.* import org.jetbrains.kotlin.descriptors.Visibilities import org.jetbrains.kotlin.ir.IrElement import org.jetbrains.kotlin.ir.builders.IrBuilderWithScope @@ -78,6 +79,7 @@ fun FileLoweringPass.runOnFileInOrder(irFile: IrFile) { class PowerAssertCallTransformer( private val context: IrPluginContext, + private val messageCollector: MessageCollector, private val functions: Set ) : IrElementTransformerVoidWithContext(), FileLoweringPass { private lateinit var file: IrFile @@ -96,9 +98,15 @@ class PowerAssertCallTransformer( if (functions.none { fqName == it }) return super.visitCall(expression) + // Find a valid delegate function or do not translate val delegate = findDelegate(fqName) ?: run { - // Find a valid delegate function or do not translate - // TODO log a warning + val line = fileSource.substring(expression.startOffset).count { it == '\n' } + 1 + val location = CompilerMessageLocation.create(file.path, line, -1, null) + messageCollector.report( + CompilerMessageSeverity.WARNING, + "Unable to find overload for function $fqName callable as $fqName(Boolean, String) or $fqName(Boolean, () -> String) for power-assertion transformation", + location + ) return super.visitCall(expression) } @@ -108,7 +116,16 @@ class PowerAssertCallTransformer( // If the tree does not contain any children, the expression is not transformable val tree = buildAssertTree(assertionArgument) - val root = tree.children.singleOrNull() ?: return super.visitCall(expression) + val root = tree.children.singleOrNull() ?: run { + val line = fileSource.substring(expression.startOffset).count { it == '\n' } + 1 + val location = CompilerMessageLocation.create(file.path, line, -1, null) + messageCollector.report( + CompilerMessageSeverity.INFO, + "Expression is constant and will not be power-assertion transformed", + location + ) + return super.visitCall(expression) + } val symbol = currentScope!!.scope.scopeOwnerSymbol DeclarationIrBuilder(context, symbol).run { @@ -149,7 +166,7 @@ class PowerAssertCallTransformer( private fun findDelegate(fqName: FqName): FunctionDelegate? { return context.findOverloads(fqName) .mapNotNull { overload -> - // TODO allow other signatures than (Boolean, String) and (Boolean, () -> String)) + // TODO allow other signatures than (Boolean, String) and (Boolean, () -> String) val parameters = overload.owner.valueParameters if (parameters.size != 2) return@mapNotNull null if (!parameters[0].type.isBoolean()) return@mapNotNull null diff --git a/kotlin-power-assert-plugin/src/main/kotlin/com/bnorm/power/PowerAssertComponentRegistrar.kt b/kotlin-power-assert-plugin/src/main/kotlin/com/bnorm/power/PowerAssertComponentRegistrar.kt index 8e7f37d308e..1cbd298ea12 100644 --- a/kotlin-power-assert-plugin/src/main/kotlin/com/bnorm/power/PowerAssertComponentRegistrar.kt +++ b/kotlin-power-assert-plugin/src/main/kotlin/com/bnorm/power/PowerAssertComponentRegistrar.kt @@ -18,6 +18,8 @@ package com.bnorm.power import com.google.auto.service.AutoService import org.jetbrains.kotlin.backend.common.extensions.IrGenerationExtension +import org.jetbrains.kotlin.cli.common.CLIConfigurationKeys +import org.jetbrains.kotlin.cli.common.messages.MessageCollector import org.jetbrains.kotlin.com.intellij.mock.MockProject import org.jetbrains.kotlin.compiler.plugin.ComponentRegistrar import org.jetbrains.kotlin.config.CompilerConfiguration @@ -38,7 +40,9 @@ class PowerAssertComponentRegistrar( ) { val functions = configuration[KEY_FUNCTIONS]?.map { FqName(it) } ?: functions if (functions.isEmpty()) return - IrGenerationExtension.registerExtension(project, PowerAssertIrGenerationExtension(functions.toSet())) + + val messageCollector = configuration.get(CLIConfigurationKeys.MESSAGE_COLLECTOR_KEY, MessageCollector.NONE) + IrGenerationExtension.registerExtension(project, PowerAssertIrGenerationExtension(messageCollector, functions.toSet())) } } diff --git a/kotlin-power-assert-plugin/src/main/kotlin/com/bnorm/power/PowerAssertIrGenerationExtension.kt b/kotlin-power-assert-plugin/src/main/kotlin/com/bnorm/power/PowerAssertIrGenerationExtension.kt index 85d4c486742..3d05f8679e0 100644 --- a/kotlin-power-assert-plugin/src/main/kotlin/com/bnorm/power/PowerAssertIrGenerationExtension.kt +++ b/kotlin-power-assert-plugin/src/main/kotlin/com/bnorm/power/PowerAssertIrGenerationExtension.kt @@ -18,15 +18,17 @@ package com.bnorm.power import org.jetbrains.kotlin.backend.common.extensions.IrGenerationExtension import org.jetbrains.kotlin.backend.common.extensions.IrPluginContext +import org.jetbrains.kotlin.cli.common.messages.MessageCollector import org.jetbrains.kotlin.ir.declarations.IrModuleFragment import org.jetbrains.kotlin.name.FqName class PowerAssertIrGenerationExtension( + private val messageCollector: MessageCollector, private val functions: Set ) : IrGenerationExtension { override fun generate(moduleFragment: IrModuleFragment, pluginContext: IrPluginContext) { for (file in moduleFragment.files) { - PowerAssertCallTransformer(pluginContext, functions).runOnFileInOrder(file) + PowerAssertCallTransformer(pluginContext, messageCollector, functions).runOnFileInOrder(file) } } }