From 54c58671fb6fd9b416105d065ab22025cd32b362 Mon Sep 17 00:00:00 2001 From: Sergej Jaskiewicz Date: Thu, 29 Feb 2024 17:49:42 +0100 Subject: [PATCH] [PowerAssert] Correctly align infix calls for built-in operators Instead of searching for the operator in the string representation of the whole expression, consider the operator's start to be the first non-whitespace non-dot character _after_ the LHS of the infix expression. This fixes cases like this: ``` assert("Name in " in listOf("Hello", "World")) | | | [Hello, World] false ``` ^KT-66208 Fixed --- .../powerassert/diagram/ExpressionTree.kt | 9 +- .../kotlin/powerassert/diagram/IrDiagram.kt | 94 +++++++++++++------ .../codegen/cast/InstanceEquals.box.txt | 16 ++-- .../codegen/cast/InstanceNotEquals.box.txt | 16 ++-- .../codegen/operator/ContainsOperator.box.txt | 22 ++--- .../codegen/operator/ExcleqOperator.box.txt | 16 ++-- .../codegen/operator/ExcleqeqOperator.box.txt | 24 ++--- .../operator/NegativeContainsOperator.box.txt | 22 ++--- 8 files changed, 125 insertions(+), 94 deletions(-) diff --git a/plugins/power-assert/power-assert.backend/src/org/jetbrains/kotlin/powerassert/diagram/ExpressionTree.kt b/plugins/power-assert/power-assert.backend/src/org/jetbrains/kotlin/powerassert/diagram/ExpressionTree.kt index 5e5fe734325..ead9caec30d 100644 --- a/plugins/power-assert/power-assert.backend/src/org/jetbrains/kotlin/powerassert/diagram/ExpressionTree.kt +++ b/plugins/power-assert/power-assert.backend/src/org/jetbrains/kotlin/powerassert/diagram/ExpressionTree.kt @@ -19,6 +19,7 @@ package org.jetbrains.kotlin.powerassert.diagram +import org.jetbrains.kotlin.ir.BuiltInOperatorNames import org.jetbrains.kotlin.ir.IrElement import org.jetbrains.kotlin.ir.expressions.* import org.jetbrains.kotlin.ir.util.dumpKotlinLike @@ -105,8 +106,12 @@ fun buildTree(expression: IrExpression): Node? { } override fun visitCall(expression: IrCall, data: Node) { - if (expression.symbol.owner.name.asString() == "EQEQ" && expression.origin == IrStatementOrigin.EXCLEQ) { - // Skip the EQEQ part of a EXCLEQ call + val isExcleq = expression.symbol.owner.name.asString() == BuiltInOperatorNames.EQEQ + && expression.origin == IrStatementOrigin.EXCLEQ + val isExcleqeq = expression.symbol.owner.name.asString() == BuiltInOperatorNames.EQEQEQ + && expression.origin == IrStatementOrigin.EXCLEQEQ + if (isExcleq || isExcleqeq) { + // Skip the EQEQ/EQEQEQ part of a EXCLEQ/EXCLEQEQ call expression.acceptChildren(this, data) } else if (expression.origin == IrStatementOrigin.NOT_IN) { // Exclude the wrapped "contains" call for `!in` operator expressions and only display the final result diff --git a/plugins/power-assert/power-assert.backend/src/org/jetbrains/kotlin/powerassert/diagram/IrDiagram.kt b/plugins/power-assert/power-assert.backend/src/org/jetbrains/kotlin/powerassert/diagram/IrDiagram.kt index 7cf7c16d8a8..2aed37ccb35 100644 --- a/plugins/power-assert/power-assert.backend/src/org/jetbrains/kotlin/powerassert/diagram/IrDiagram.kt +++ b/plugins/power-assert/power-assert.backend/src/org/jetbrains/kotlin/powerassert/diagram/IrDiagram.kt @@ -154,7 +154,7 @@ private fun findDisplayOffset( ): Int { return when (expression) { is IrMemberAccessExpression<*> -> memberAccessOffset(expression, sourceRangeInfo, source) - is IrTypeOperatorCall -> typeOperatorOffset(expression, source) + is IrTypeOperatorCall -> typeOperatorOffset(expression, sourceRangeInfo, source) else -> 0 } } @@ -164,38 +164,12 @@ private fun memberAccessOffset( sourceRangeInfo: SourceRangeInfo, source: String, ): Int { - when (expression.origin) { - // special case to handle `value != null` - IrStatementOrigin.EXCLEQ, IrStatementOrigin.EXCLEQEQ -> return source.indexOf("!=") - // special case to handle `in` operator - IrStatementOrigin.IN -> return source.indexOf(" in ") + 1 - // special case to handle `in` operator - IrStatementOrigin.NOT_IN -> return source.indexOf(" !in ") + 1 - else -> Unit - } - val owner = expression.symbol.owner if (owner !is IrSimpleFunction) return 0 if (owner.isInfix || owner.isOperator || owner.origin == IrBuiltIns.BUILTIN_OPERATOR) { - // Ignore single value operators - val singleReceiver = (expression.dispatchReceiver != null) xor (expression.extensionReceiver != null) - if (singleReceiver && expression.valueArgumentsCount == 0) return 0 - - // Start after the dispatcher or first argument - val receiver = expression.dispatchReceiver - ?: expression.extensionReceiver - ?: expression.getValueArgument(0).takeIf { owner.origin == IrBuiltIns.BUILTIN_OPERATOR } - ?: return 0 - var offset = receiver.endOffset - sourceRangeInfo.startOffset + 1 - if (offset < 0 || offset >= source.length) return 0 // infix function called using non-infix syntax - - // Continue until there is a non-whitespace character - while (source[offset].isWhitespace() || source[offset] == '.') { - offset++ - if (offset >= source.length) return 0 - } - return offset + val lhs = expression.binaryOperatorLhs() ?: return 0 + return binaryOperatorOffset(lhs, sourceRangeInfo, source) } return 0 @@ -203,15 +177,73 @@ private fun memberAccessOffset( private fun typeOperatorOffset( expression: IrTypeOperatorCall, + sourceRangeInfo: SourceRangeInfo, source: String, ): Int { return when (expression.operator) { - IrTypeOperator.INSTANCEOF -> source.indexOf(" is ") + 1 - IrTypeOperator.NOT_INSTANCEOF -> source.indexOf(" !is ") + 1 + IrTypeOperator.INSTANCEOF, IrTypeOperator.NOT_INSTANCEOF -> binaryOperatorOffset(expression.argument, sourceRangeInfo, source) else -> 0 } } +/** + * The offset of the infix operator/function itself. + * + * @param lhs The left-hand side expression of the operator. + * @param wholeOperatorSourceRangeInfo The source range of the whole operator expression. + * @param wholeOperatorSource The source text of the whole operator expression. + */ +private fun binaryOperatorOffset(lhs: IrExpression, wholeOperatorSourceRangeInfo: SourceRangeInfo, wholeOperatorSource: String): Int { + var offset = lhs.endOffset - wholeOperatorSourceRangeInfo.startOffset + 1 + if (offset < 0 || offset >= wholeOperatorSource.length) return 0 // infix function called using non-infix syntax + + // Continue until there is a non-whitespace character + while (wholeOperatorSource[offset].isWhitespace() || wholeOperatorSource[offset] == '.') { + offset++ + if (offset >= wholeOperatorSource.length) return 0 + } + return offset +} + +/** + * The left-hand side expression of an infix operator/function that takes into account special cases like `in`, `!in` and `!=` operators + * that have a more complex structure than just a single call with two arguments. + */ +private fun IrMemberAccessExpression<*>.binaryOperatorLhs(): IrExpression? = when (origin) { + IrStatementOrigin.EXCLEQ -> { + // The `!=` operator call is actually a sugar for `lhs.equals(rhs).not()`. + (dispatchReceiver as? IrCall)?.simpleBinaryOperatorLhs() + } + IrStatementOrigin.EXCLEQEQ -> { + // The `!==` operator call is actually a sugar for `(lhs === rhs).not()`. + (dispatchReceiver as? IrCall)?.simpleBinaryOperatorLhs() + } + IrStatementOrigin.IN -> { + // The `in` operator call is actually a sugar for `rhs.contains(lhs)`. + getValueArgument(0) + } + IrStatementOrigin.NOT_IN -> { + // The `!in` operator call is actually a sugar for `rhs.contains(lhs).not()`. + (dispatchReceiver as? IrCall)?.getValueArgument(0) + } + else -> simpleBinaryOperatorLhs() +} + +/** + * The left-hand side expression of an infix operator/function. + * For single-value operators returns `null`, for all other infix operators/functions, returns the receiver or the first value argument. + */ +private fun IrMemberAccessExpression<*>.simpleBinaryOperatorLhs(): IrExpression? { + val singleReceiver = (dispatchReceiver != null) xor (extensionReceiver != null) + return if (singleReceiver && valueArgumentsCount == 0) { + null + } else { + dispatchReceiver + ?: extensionReceiver + ?: getValueArgument(0).takeIf { (symbol.owner as? IrSimpleFunction)?.origin == IrBuiltIns.BUILTIN_OPERATOR } + } +} + fun StringBuilder.indent(indentation: Int): StringBuilder { repeat(indentation) { append(" ") } return this diff --git a/plugins/power-assert/testData/codegen/cast/InstanceEquals.box.txt b/plugins/power-assert/testData/codegen/cast/InstanceEquals.box.txt index 54859abc7be..5a3529dd233 100644 --- a/plugins/power-assert/testData/codegen/cast/InstanceEquals.box.txt +++ b/plugins/power-assert/testData/codegen/cast/InstanceEquals.box.txt @@ -5,8 +5,8 @@ assert(null is String) Assertion failed assert(!(" is " is String)) - | | - | true + | | + | true false Assertion failed @@ -14,22 +14,22 @@ assert(!( | false " is " - | - true is + | + true String )) Assertion failed assert(null/*is*/is/*is*/String) - | - false + | + false Assertion failed assert(!((null is String) is Boolean)) - | | - | true + | | | + | | true | false false \ No newline at end of file diff --git a/plugins/power-assert/testData/codegen/cast/InstanceNotEquals.box.txt b/plugins/power-assert/testData/codegen/cast/InstanceNotEquals.box.txt index e97491bb743..0d7015930e8 100644 --- a/plugins/power-assert/testData/codegen/cast/InstanceNotEquals.box.txt +++ b/plugins/power-assert/testData/codegen/cast/InstanceNotEquals.box.txt @@ -5,27 +5,27 @@ assert("Hello, world!" !is String) Assertion failed assert(" !is " !is String) - | - false + | + false Assertion failed assert( " !is " - | - false !is + | + false String ) Assertion failed assert("Hello, world!"/*!is*/!is/*!is*/String) - | - false + | + false Assertion failed assert(("Hello, world!" !is String) !is Boolean) - | - false + | | + | false false \ No newline at end of file diff --git a/plugins/power-assert/testData/codegen/operator/ContainsOperator.box.txt b/plugins/power-assert/testData/codegen/operator/ContainsOperator.box.txt index 9fdf315ea61..0d45e4f9bd5 100644 --- a/plugins/power-assert/testData/codegen/operator/ContainsOperator.box.txt +++ b/plugins/power-assert/testData/codegen/operator/ContainsOperator.box.txt @@ -6,17 +6,17 @@ assert("Name" in listOf("Hello", "World")) Assertion failed assert(" in " in listOf("Hello", "World")) - | | - | [Hello, World] - false + | | + | [Hello, World] + false Assertion failed assert( " in " - | - false in + | + false listOf("Hello", "World") | @@ -25,14 +25,14 @@ assert( Assertion failed assert("Name"/*in*/in/*in*/listOf("Hello", "World")) - | | - | [Hello, World] - false + | | + | [Hello, World] + false Assertion failed assert(("Name" in listOf("Hello", "World")) in listOf(true)) - | | | - | | [true] + | | | | + | | | [true] + | | false | [Hello, World] - false false \ No newline at end of file diff --git a/plugins/power-assert/testData/codegen/operator/ExcleqOperator.box.txt b/plugins/power-assert/testData/codegen/operator/ExcleqOperator.box.txt index da3c4816043..503c23379a1 100644 --- a/plugins/power-assert/testData/codegen/operator/ExcleqOperator.box.txt +++ b/plugins/power-assert/testData/codegen/operator/ExcleqOperator.box.txt @@ -5,27 +5,27 @@ assert(1 != 1) Assertion failed assert(" != " != " != ") - | - false + | + false Assertion failed assert( " != " - | - false != + | + false " != " ) Assertion failed assert(1/*!=*/!=/*!=*/1) - | - false + | + false Assertion failed assert((1 != 1) != false) - | - false + | | + | false false \ No newline at end of file diff --git a/plugins/power-assert/testData/codegen/operator/ExcleqeqOperator.box.txt b/plugins/power-assert/testData/codegen/operator/ExcleqeqOperator.box.txt index 80b810e55b6..9ed3fa9c48b 100644 --- a/plugins/power-assert/testData/codegen/operator/ExcleqeqOperator.box.txt +++ b/plugins/power-assert/testData/codegen/operator/ExcleqeqOperator.box.txt @@ -2,36 +2,30 @@ Assertion failed assert(1 !== 1) | false - true Assertion failed assert(" !== " !== " !== ") - | - false - true + | + false Assertion failed assert( " !== " - | - false - true !== + | + false " !== " ) Assertion failed assert(1/*!==*/!==/*!==*/1) - | - false - true + | + false Assertion failed assert((1 !== 1) !== false) - | - false - true - false - true \ No newline at end of file + | | + | false + false \ No newline at end of file diff --git a/plugins/power-assert/testData/codegen/operator/NegativeContainsOperator.box.txt b/plugins/power-assert/testData/codegen/operator/NegativeContainsOperator.box.txt index 0db6e650729..af414cbfe50 100644 --- a/plugins/power-assert/testData/codegen/operator/NegativeContainsOperator.box.txt +++ b/plugins/power-assert/testData/codegen/operator/NegativeContainsOperator.box.txt @@ -6,17 +6,17 @@ assert("Hello" !in listOf("Hello", "World")) Assertion failed assert(" !in " !in listOf(" !in ")) - | | - | [ !in ] - false + | | + | [ !in ] + false Assertion failed assert( " !in " - | - false !in + | + false listOf(" !in ") | @@ -25,14 +25,14 @@ assert( Assertion failed assert("Hello"/*!in*/!in/*!in*/listOf("Hello", "World")) - | | - | [Hello, World] - false + | | + | [Hello, World] + false Assertion failed assert(("Hello" !in listOf("Hello", "World")) !in listOf(false)) - | | | - | | [false] + | | | | + | | | [false] + | | false | [Hello, World] - false false \ No newline at end of file