From ca7649edbbda17c79dc3af10d239dbfd75a9714e Mon Sep 17 00:00:00 2001 From: Mark Punzalan Date: Mon, 24 May 2021 17:58:34 +0000 Subject: [PATCH] FIR IDE: Enable ReplaceInfixOrOperatorCallFix for UNSAFE_CALL, UNSAFE_INFIX_CALL, UNSAFE_OPERATOR_CALL. --- .../idea/quickfix/MainKtQuickFixRegistrar.kt | 2 + .../quickfix/fixes/ReplaceCallFixFactories.kt | 44 +++++++++++++++---- .../unsafeInfixCall/unsafeGet.fir.kt.after | 4 ++ .../nullables/unsafeInfixCall/unsafeGet.kt | 4 +- .../unsafeInfixCall/unsafeGet.kt.after | 4 +- .../nullables/unsafeInfixCall/unsafePlus.kt | 4 +- .../unsafeInfixCall/unsafePlus.kt.after | 4 +- .../nullables/unsafeInfixCall/unsafeSet.kt | 4 +- .../unsafeInfixCall/unsafeSet.kt.after | 4 +- .../replaceInfixOrOperatorCall/array.kt | 4 +- .../replaceInfixOrOperatorCall/array.kt.after | 4 +- .../replaceInfixOrOperatorCall/arraySet.kt | 4 +- .../arraySet.kt.after | 4 +- .../assignmentArray.kt | 4 +- .../assignmentArray.kt.after | 4 +- .../assignmentBinaryOperator.kt | 4 +- .../assignmentBinaryOperator.kt.after | 4 +- .../assignmentList.kt | 4 +- .../assignmentList.kt.after | 4 +- .../binaryOperator.kt | 4 +- .../binaryOperator.kt.after | 4 +- .../replaceInfixOrOperatorCall/hasElvis.kt | 4 +- .../hasElvis.kt.after | 4 +- .../replaceInfixOrOperatorCall/list.kt | 4 +- .../replaceInfixOrOperatorCall/list.kt.after | 4 +- 25 files changed, 63 insertions(+), 75 deletions(-) create mode 100644 idea/testData/quickfix/nullables/unsafeInfixCall/unsafeGet.fir.kt.after diff --git a/idea/idea-fir/src/org/jetbrains/kotlin/idea/quickfix/MainKtQuickFixRegistrar.kt b/idea/idea-fir/src/org/jetbrains/kotlin/idea/quickfix/MainKtQuickFixRegistrar.kt index 3b9cbd099ce..4f473447136 100644 --- a/idea/idea-fir/src/org/jetbrains/kotlin/idea/quickfix/MainKtQuickFixRegistrar.kt +++ b/idea/idea-fir/src/org/jetbrains/kotlin/idea/quickfix/MainKtQuickFixRegistrar.kt @@ -109,6 +109,8 @@ class MainKtQuickFixRegistrar : KtQuickFixRegistrar() { registerPsiQuickFixes(KtFirDiagnostic.UselessCast::class, RemoveUselessCastFix) registerPsiQuickFixes(KtFirDiagnostic.UselessIsCheck::class, RemoveUselessIsCheckFix, RemoveUselessIsCheckFixForWhen) registerApplicator(ReplaceCallFixFactories.unsafeCallFactory) + registerApplicator(ReplaceCallFixFactories.unsafeInfixCallFactory) + registerApplicator(ReplaceCallFixFactories.unsafeOperatorCallFactory) registerApplicator(AddExclExclCallFixFactories.unsafeCallFactory) registerApplicator(AddExclExclCallFixFactories.unsafeInfixCallFactory) registerApplicator(AddExclExclCallFixFactories.unsafeOperatorCallFactory) diff --git a/idea/idea-fir/src/org/jetbrains/kotlin/idea/quickfix/fixes/ReplaceCallFixFactories.kt b/idea/idea-fir/src/org/jetbrains/kotlin/idea/quickfix/fixes/ReplaceCallFixFactories.kt index 96892fb81b0..3608f1f2742 100644 --- a/idea/idea-fir/src/org/jetbrains/kotlin/idea/quickfix/fixes/ReplaceCallFixFactories.kt +++ b/idea/idea-fir/src/org/jetbrains/kotlin/idea/quickfix/fixes/ReplaceCallFixFactories.kt @@ -6,25 +6,22 @@ package org.jetbrains.kotlin.idea.quickfix.fixes import org.jetbrains.kotlin.idea.fir.api.fixes.diagnosticFixFactory +import org.jetbrains.kotlin.idea.frontend.api.KtAnalysisSession import org.jetbrains.kotlin.idea.frontend.api.fir.diagnostics.KtFirDiagnostic import org.jetbrains.kotlin.idea.frontend.api.types.KtTypeNullability import org.jetbrains.kotlin.idea.frontend.api.types.KtTypeWithNullability import org.jetbrains.kotlin.idea.quickfix.ReplaceImplicitReceiverCallFix +import org.jetbrains.kotlin.idea.quickfix.ReplaceInfixOrOperatorCallFix import org.jetbrains.kotlin.idea.quickfix.ReplaceWithSafeCallFix import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.psiUtil.unwrapParenthesesLabelsAndAnnotations +import org.jetbrains.kotlin.types.expressions.OperatorConventions +import org.jetbrains.kotlin.utils.addToStdlib.safeAs object ReplaceCallFixFactories { val unsafeCallFactory = diagnosticFixFactory { diagnostic -> - fun KtExpression.shouldHaveNotNullType(): Boolean { - // This function is used to determine if we may need to add an elvis operator after the safe call. For example, to replace - // `s.length` in `val x: Int = s.length` with a safe call, it should be replaced with `s.length ?: `. - val expectedType = getExpectedType() as? KtTypeWithNullability - return expectedType?.nullability == KtTypeNullability.NON_NULLABLE - } - val psi = diagnostic.psi val target = if (psi is KtBinaryExpression && psi.operationToken in KtTokens.ALL_ASSIGNMENTS) { // UNSAFE_CALL for assignments (e.g., `foo.bar = value`) is reported on the entire statement (KtBinaryExpression). @@ -34,16 +31,45 @@ object ReplaceCallFixFactories { psi }.unwrapParenthesesLabelsAndAnnotations() + val shouldHaveNotNullType = target.safeAs()?.let { shouldHaveNotNullType(it) } ?: false when (target) { - is KtDotQualifiedExpression -> listOf(ReplaceWithSafeCallFix(target, target.shouldHaveNotNullType())) + is KtDotQualifiedExpression -> listOf(ReplaceWithSafeCallFix(target, shouldHaveNotNullType)) is KtNameReferenceExpression -> { // TODO: As a safety precaution, resolve the expression to determine if it is a call with an implicit receiver. // This is a defensive check to ensure that the diagnostic was reported on such a call and not some other name reference. // This isn't strictly needed because FIR checkers aren't reporting on wrong elements, but ReplaceWithSafeCallFixFactory // in FE1.0 does so. - listOf(ReplaceImplicitReceiverCallFix(target, target.shouldHaveNotNullType())) + listOf(ReplaceImplicitReceiverCallFix(target, shouldHaveNotNullType)) } + is KtArrayAccessExpression -> listOf(ReplaceInfixOrOperatorCallFix(target, shouldHaveNotNullType)) else -> emptyList() } } + + val unsafeInfixCallFactory = + diagnosticFixFactory { diagnostic -> + val psi = diagnostic.psi + val target = psi.parent as? KtBinaryExpression ?: return@diagnosticFixFactory emptyList() + listOf(ReplaceInfixOrOperatorCallFix(target, shouldHaveNotNullType(target), diagnostic.operator)) + } + + val unsafeOperatorCallFactory = + diagnosticFixFactory { diagnostic -> + val psi = diagnostic.psi + val operationToken = psi.safeAs()?.getReferencedNameElementType() + if (operationToken == KtTokens.EQ || operationToken in OperatorConventions.COMPARISON_OPERATIONS) { + // This matches FE1.0 behavior; see ReplaceInfixOrOperatorCallFixFactory.kt + return@diagnosticFixFactory emptyList() + } + + val target = psi.parent as? KtBinaryExpression ?: return@diagnosticFixFactory emptyList() + listOf(ReplaceInfixOrOperatorCallFix(target, shouldHaveNotNullType(target), diagnostic.operator)) + } + + private fun KtAnalysisSession.shouldHaveNotNullType(expression: KtExpression): Boolean { + // This function is used to determine if we may need to add an elvis operator after the safe call. For example, to replace + // `s.length` in `val x: Int = s.length` with a safe call, it should be replaced with `s.length ?: `. + val expectedType = expression.getExpectedType() as? KtTypeWithNullability + return expectedType?.nullability == KtTypeNullability.NON_NULLABLE + } } diff --git a/idea/testData/quickfix/nullables/unsafeInfixCall/unsafeGet.fir.kt.after b/idea/testData/quickfix/nullables/unsafeInfixCall/unsafeGet.fir.kt.after new file mode 100644 index 00000000000..f33454e9f22 --- /dev/null +++ b/idea/testData/quickfix/nullables/unsafeInfixCall/unsafeGet.fir.kt.after @@ -0,0 +1,4 @@ +// "Replace with safe (?.) call" "true" + +operator fun Int.get(row: Int, column: Int) = this +fun foo(arg: Int?) = arg?.get(42, 13) ?: \ No newline at end of file diff --git a/idea/testData/quickfix/nullables/unsafeInfixCall/unsafeGet.kt b/idea/testData/quickfix/nullables/unsafeInfixCall/unsafeGet.kt index 685ecaa40ef..a39b9af97b9 100644 --- a/idea/testData/quickfix/nullables/unsafeInfixCall/unsafeGet.kt +++ b/idea/testData/quickfix/nullables/unsafeInfixCall/unsafeGet.kt @@ -1,6 +1,4 @@ // "Replace with safe (?.) call" "true" operator fun Int.get(row: Int, column: Int) = this -fun foo(arg: Int?) = arg[42, 13] - -/* IGNORE_FIR */ +fun foo(arg: Int?) = arg[42, 13] \ No newline at end of file diff --git a/idea/testData/quickfix/nullables/unsafeInfixCall/unsafeGet.kt.after b/idea/testData/quickfix/nullables/unsafeInfixCall/unsafeGet.kt.after index 8c146c0c154..cc367c7c68c 100644 --- a/idea/testData/quickfix/nullables/unsafeInfixCall/unsafeGet.kt.after +++ b/idea/testData/quickfix/nullables/unsafeInfixCall/unsafeGet.kt.after @@ -1,6 +1,4 @@ // "Replace with safe (?.) call" "true" operator fun Int.get(row: Int, column: Int) = this -fun foo(arg: Int?) = arg?.get(42, 13) - -/* IGNORE_FIR */ +fun foo(arg: Int?) = arg?.get(42, 13) \ No newline at end of file diff --git a/idea/testData/quickfix/nullables/unsafeInfixCall/unsafePlus.kt b/idea/testData/quickfix/nullables/unsafeInfixCall/unsafePlus.kt index 0f05fde8352..cf60618231c 100644 --- a/idea/testData/quickfix/nullables/unsafeInfixCall/unsafePlus.kt +++ b/idea/testData/quickfix/nullables/unsafeInfixCall/unsafePlus.kt @@ -1,6 +1,4 @@ // "Replace with safe (?.) call" "true" operator fun Int.plus(index: Int) = this -fun fox(arg: Int?) = arg + 42 - -/* IGNORE_FIR */ +fun fox(arg: Int?) = arg + 42 \ No newline at end of file diff --git a/idea/testData/quickfix/nullables/unsafeInfixCall/unsafePlus.kt.after b/idea/testData/quickfix/nullables/unsafeInfixCall/unsafePlus.kt.after index 145fe220068..be5dda9f042 100644 --- a/idea/testData/quickfix/nullables/unsafeInfixCall/unsafePlus.kt.after +++ b/idea/testData/quickfix/nullables/unsafeInfixCall/unsafePlus.kt.after @@ -1,6 +1,4 @@ // "Replace with safe (?.) call" "true" operator fun Int.plus(index: Int) = this -fun fox(arg: Int?) = arg?.plus(42) - -/* IGNORE_FIR */ +fun fox(arg: Int?) = arg?.plus(42) \ No newline at end of file diff --git a/idea/testData/quickfix/nullables/unsafeInfixCall/unsafeSet.kt b/idea/testData/quickfix/nullables/unsafeInfixCall/unsafeSet.kt index 8ae8a5c9aff..99f4c3ecdfe 100644 --- a/idea/testData/quickfix/nullables/unsafeInfixCall/unsafeSet.kt +++ b/idea/testData/quickfix/nullables/unsafeInfixCall/unsafeSet.kt @@ -3,6 +3,4 @@ operator fun Int.set(row: Int, column: Int, value: Int) {} fun foo(arg: Int?) { arg[42, 13] = 0 -} - -/* IGNORE_FIR */ +} \ No newline at end of file diff --git a/idea/testData/quickfix/nullables/unsafeInfixCall/unsafeSet.kt.after b/idea/testData/quickfix/nullables/unsafeInfixCall/unsafeSet.kt.after index f12d6a643cb..ce2ed0ba5e0 100644 --- a/idea/testData/quickfix/nullables/unsafeInfixCall/unsafeSet.kt.after +++ b/idea/testData/quickfix/nullables/unsafeInfixCall/unsafeSet.kt.after @@ -3,6 +3,4 @@ operator fun Int.set(row: Int, column: Int, value: Int) {} fun foo(arg: Int?) { arg?.set(42, 13, 0) -} - -/* IGNORE_FIR */ +} \ No newline at end of file diff --git a/idea/testData/quickfix/replaceInfixOrOperatorCall/array.kt b/idea/testData/quickfix/replaceInfixOrOperatorCall/array.kt index 887fefb15d9..0025d4e9dcd 100644 --- a/idea/testData/quickfix/replaceInfixOrOperatorCall/array.kt +++ b/idea/testData/quickfix/replaceInfixOrOperatorCall/array.kt @@ -3,6 +3,4 @@ fun foo(array: Array?) { array[0] -} - -/* IGNORE_FIR */ +} \ No newline at end of file diff --git a/idea/testData/quickfix/replaceInfixOrOperatorCall/array.kt.after b/idea/testData/quickfix/replaceInfixOrOperatorCall/array.kt.after index 7058e19ba5e..396d3c95181 100644 --- a/idea/testData/quickfix/replaceInfixOrOperatorCall/array.kt.after +++ b/idea/testData/quickfix/replaceInfixOrOperatorCall/array.kt.after @@ -3,6 +3,4 @@ fun foo(array: Array?) { array?.get(0) -} - -/* IGNORE_FIR */ +} \ No newline at end of file diff --git a/idea/testData/quickfix/replaceInfixOrOperatorCall/arraySet.kt b/idea/testData/quickfix/replaceInfixOrOperatorCall/arraySet.kt index 40419b6e110..7392346d0cd 100644 --- a/idea/testData/quickfix/replaceInfixOrOperatorCall/arraySet.kt +++ b/idea/testData/quickfix/replaceInfixOrOperatorCall/arraySet.kt @@ -3,6 +3,4 @@ fun foo(array: Array?) { array[0] = "" -} - -/* IGNORE_FIR */ +} \ No newline at end of file diff --git a/idea/testData/quickfix/replaceInfixOrOperatorCall/arraySet.kt.after b/idea/testData/quickfix/replaceInfixOrOperatorCall/arraySet.kt.after index a00d4b46be1..2e26bfb9b46 100644 --- a/idea/testData/quickfix/replaceInfixOrOperatorCall/arraySet.kt.after +++ b/idea/testData/quickfix/replaceInfixOrOperatorCall/arraySet.kt.after @@ -3,6 +3,4 @@ fun foo(array: Array?) { array?.set(0, "") -} - -/* IGNORE_FIR */ +} \ No newline at end of file diff --git a/idea/testData/quickfix/replaceInfixOrOperatorCall/assignmentArray.kt b/idea/testData/quickfix/replaceInfixOrOperatorCall/assignmentArray.kt index 5098dda0737..45596a8c84c 100644 --- a/idea/testData/quickfix/replaceInfixOrOperatorCall/assignmentArray.kt +++ b/idea/testData/quickfix/replaceInfixOrOperatorCall/assignmentArray.kt @@ -4,6 +4,4 @@ fun foo(array: Array?) { var s = "" s = array[0] -} - -/* IGNORE_FIR */ +} \ No newline at end of file diff --git a/idea/testData/quickfix/replaceInfixOrOperatorCall/assignmentArray.kt.after b/idea/testData/quickfix/replaceInfixOrOperatorCall/assignmentArray.kt.after index 3df3fc30e3e..b6e99efceed 100644 --- a/idea/testData/quickfix/replaceInfixOrOperatorCall/assignmentArray.kt.after +++ b/idea/testData/quickfix/replaceInfixOrOperatorCall/assignmentArray.kt.after @@ -4,6 +4,4 @@ fun foo(array: Array?) { var s = "" s = array?.get(0) ?: -} - -/* IGNORE_FIR */ +} \ No newline at end of file diff --git a/idea/testData/quickfix/replaceInfixOrOperatorCall/assignmentBinaryOperator.kt b/idea/testData/quickfix/replaceInfixOrOperatorCall/assignmentBinaryOperator.kt index d62f9e08e55..e50130ca160 100644 --- a/idea/testData/quickfix/replaceInfixOrOperatorCall/assignmentBinaryOperator.kt +++ b/idea/testData/quickfix/replaceInfixOrOperatorCall/assignmentBinaryOperator.kt @@ -4,6 +4,4 @@ fun foo(bar: Int?) { var i: Int = 1 i = bar + 1 -} - -/* IGNORE_FIR */ +} \ No newline at end of file diff --git a/idea/testData/quickfix/replaceInfixOrOperatorCall/assignmentBinaryOperator.kt.after b/idea/testData/quickfix/replaceInfixOrOperatorCall/assignmentBinaryOperator.kt.after index 981abb144ec..c20ab0cdd5d 100644 --- a/idea/testData/quickfix/replaceInfixOrOperatorCall/assignmentBinaryOperator.kt.after +++ b/idea/testData/quickfix/replaceInfixOrOperatorCall/assignmentBinaryOperator.kt.after @@ -4,6 +4,4 @@ fun foo(bar: Int?) { var i: Int = 1 i = bar?.plus(1) ?: -} - -/* IGNORE_FIR */ +} \ No newline at end of file diff --git a/idea/testData/quickfix/replaceInfixOrOperatorCall/assignmentList.kt b/idea/testData/quickfix/replaceInfixOrOperatorCall/assignmentList.kt index 702e1e7b444..a09c7865d4d 100644 --- a/idea/testData/quickfix/replaceInfixOrOperatorCall/assignmentList.kt +++ b/idea/testData/quickfix/replaceInfixOrOperatorCall/assignmentList.kt @@ -4,6 +4,4 @@ fun foo(list: List?) { var s = "" s = list[0] -} - -/* IGNORE_FIR */ +} \ No newline at end of file diff --git a/idea/testData/quickfix/replaceInfixOrOperatorCall/assignmentList.kt.after b/idea/testData/quickfix/replaceInfixOrOperatorCall/assignmentList.kt.after index a9e4978985f..e140aa8677b 100644 --- a/idea/testData/quickfix/replaceInfixOrOperatorCall/assignmentList.kt.after +++ b/idea/testData/quickfix/replaceInfixOrOperatorCall/assignmentList.kt.after @@ -4,6 +4,4 @@ fun foo(list: List?) { var s = "" s = list?.get(0) ?: -} - -/* IGNORE_FIR */ +} \ No newline at end of file diff --git a/idea/testData/quickfix/replaceInfixOrOperatorCall/binaryOperator.kt b/idea/testData/quickfix/replaceInfixOrOperatorCall/binaryOperator.kt index 1fb4cb003b1..908b407ad11 100644 --- a/idea/testData/quickfix/replaceInfixOrOperatorCall/binaryOperator.kt +++ b/idea/testData/quickfix/replaceInfixOrOperatorCall/binaryOperator.kt @@ -3,6 +3,4 @@ fun foo(bar: Int?) { bar + 1 -} - -/* IGNORE_FIR */ +} \ No newline at end of file diff --git a/idea/testData/quickfix/replaceInfixOrOperatorCall/binaryOperator.kt.after b/idea/testData/quickfix/replaceInfixOrOperatorCall/binaryOperator.kt.after index 9978566f92b..9863dbfce16 100644 --- a/idea/testData/quickfix/replaceInfixOrOperatorCall/binaryOperator.kt.after +++ b/idea/testData/quickfix/replaceInfixOrOperatorCall/binaryOperator.kt.after @@ -3,6 +3,4 @@ fun foo(bar: Int?) { bar?.plus(1) -} - -/* IGNORE_FIR */ +} \ No newline at end of file diff --git a/idea/testData/quickfix/replaceInfixOrOperatorCall/hasElvis.kt b/idea/testData/quickfix/replaceInfixOrOperatorCall/hasElvis.kt index 42639d8732f..dbf659385c4 100644 --- a/idea/testData/quickfix/replaceInfixOrOperatorCall/hasElvis.kt +++ b/idea/testData/quickfix/replaceInfixOrOperatorCall/hasElvis.kt @@ -4,6 +4,4 @@ fun foo(list: List?) { var s = "" s = list[0] ?: "" -} - -/* IGNORE_FIR */ +} \ No newline at end of file diff --git a/idea/testData/quickfix/replaceInfixOrOperatorCall/hasElvis.kt.after b/idea/testData/quickfix/replaceInfixOrOperatorCall/hasElvis.kt.after index 5db3803be5b..a22be396f5d 100644 --- a/idea/testData/quickfix/replaceInfixOrOperatorCall/hasElvis.kt.after +++ b/idea/testData/quickfix/replaceInfixOrOperatorCall/hasElvis.kt.after @@ -4,6 +4,4 @@ fun foo(list: List?) { var s = "" s = list?.get(0) ?: "" -} - -/* IGNORE_FIR */ +} \ No newline at end of file diff --git a/idea/testData/quickfix/replaceInfixOrOperatorCall/list.kt b/idea/testData/quickfix/replaceInfixOrOperatorCall/list.kt index bffa1677fa8..6d9f7595d07 100644 --- a/idea/testData/quickfix/replaceInfixOrOperatorCall/list.kt +++ b/idea/testData/quickfix/replaceInfixOrOperatorCall/list.kt @@ -3,6 +3,4 @@ fun foo(list: List?) { list[0] -} - -/* IGNORE_FIR */ +} \ No newline at end of file diff --git a/idea/testData/quickfix/replaceInfixOrOperatorCall/list.kt.after b/idea/testData/quickfix/replaceInfixOrOperatorCall/list.kt.after index 91784c8f413..e89f33e5cd4 100644 --- a/idea/testData/quickfix/replaceInfixOrOperatorCall/list.kt.after +++ b/idea/testData/quickfix/replaceInfixOrOperatorCall/list.kt.after @@ -3,6 +3,4 @@ fun foo(list: List?) { list?.get(0) -} - -/* IGNORE_FIR */ +} \ No newline at end of file