From 5ede37d6ab9a3e09828d5012b6c2e52e60764751 Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Fri, 14 Aug 2020 11:12:13 +0300 Subject: [PATCH] Report warnings on safe call + nullable extension operator ^KT-41034 In Progress --- ...irOldFrontendDiagnosticsTestGenerated.java | 5 + .../jetbrains/kotlin/diagnostics/Errors.java | 2 + .../rendering/DefaultErrorMessages.java | 3 + .../resolve/PlatformConfiguratorBase.kt | 2 +- ...bleExtensionOperatorWithSafeCallChecker.kt | 60 ++++++++++++ .../nullableTypes/safeCallOperators.fir.kt | 91 +++++++++++++++++++ .../tests/nullableTypes/safeCallOperators.kt | 91 +++++++++++++++++++ .../operator-call/p-1/pos/2.1.kt | 14 +-- .../checkers/DiagnosticsTestGenerated.java | 5 + .../DiagnosticsUsingJavacTestGenerated.java | 5 + 10 files changed, 270 insertions(+), 8 deletions(-) create mode 100644 compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/NullableExtensionOperatorWithSafeCallChecker.kt create mode 100644 compiler/testData/diagnostics/tests/nullableTypes/safeCallOperators.fir.kt create mode 100644 compiler/testData/diagnostics/tests/nullableTypes/safeCallOperators.kt diff --git a/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirOldFrontendDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirOldFrontendDiagnosticsTestGenerated.java index 873b00f97d0..808e1c71be3 100644 --- a/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirOldFrontendDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirOldFrontendDiagnosticsTestGenerated.java @@ -15821,6 +15821,11 @@ public class FirOldFrontendDiagnosticsTestGenerated extends AbstractFirOldFronte runTest("compiler/testData/diagnostics/tests/nullableTypes/safeCallOnTypeWithNullableUpperBound.kt"); } + @TestMetadata("safeCallOperators.kt") + public void testSafeCallOperators() throws Exception { + runTest("compiler/testData/diagnostics/tests/nullableTypes/safeCallOperators.kt"); + } + @TestMetadata("safeCallWithInvoke.kt") public void testSafeCallWithInvoke() throws Exception { runTest("compiler/testData/diagnostics/tests/nullableTypes/safeCallWithInvoke.kt"); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java index d86ca1ec5ab..d25d2a24786 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java @@ -901,6 +901,8 @@ public interface Errors { DiagnosticFactory1 DSL_SCOPE_VIOLATION = DiagnosticFactory1.create(ERROR); DiagnosticFactory1 DSL_SCOPE_VIOLATION_WARNING = DiagnosticFactory1.create(WARNING); + DiagnosticFactory0 NULLABLE_EXTENSION_OPERATOR_WITH_SAFE_CALL_RECEIVER = DiagnosticFactory0.create(WARNING); + // Labels DiagnosticFactory0 LABEL_NAME_CLASH = DiagnosticFactory0.create(WARNING); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java index c783f9fe505..b5cfe358cce 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java @@ -528,6 +528,9 @@ public class DefaultErrorMessages { MAP.put(DSL_SCOPE_VIOLATION_WARNING, "''{0}'' shouldn't be called in this context by implicit receiver, it will become an error soon. " + "Use the explicit one if necessary", COMPACT); + MAP.put(NULLABLE_EXTENSION_OPERATOR_WITH_SAFE_CALL_RECEIVER, "Semantics of such combination of safe call and operator will change in next compiler version. " + + "Namely, the right part of the safe call will not be evaluated if receiver is null"); + MAP.put(RETURN_IN_FUNCTION_WITH_EXPRESSION_BODY, "Returns are not allowed for functions with expression body. Use block body in '{...}'"); MAP.put(NO_RETURN_IN_FUNCTION_WITH_BLOCK_BODY, "A 'return' expression required in a function with a block body ('{...}')"); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/PlatformConfiguratorBase.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/PlatformConfiguratorBase.kt index 6fdc43476fc..a5ce145381d 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/PlatformConfiguratorBase.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/PlatformConfiguratorBase.kt @@ -51,7 +51,7 @@ private val DEFAULT_CALL_CHECKERS = listOf( UselessElvisCallChecker(), ResultTypeWithNullableOperatorsChecker(), NullableVarargArgumentCallChecker, NamedFunAsExpressionChecker, ContractNotAllowedCallChecker, ReifiedTypeParameterSubstitutionChecker(), MissingDependencySupertypeChecker.ForCalls, AbstractClassInstantiationChecker, SuspendConversionCallChecker, - UnitConversionCallChecker, FunInterfaceConstructorReferenceChecker + UnitConversionCallChecker, FunInterfaceConstructorReferenceChecker, NullableExtensionOperatorWithSafeCallChecker ) private val DEFAULT_TYPE_CHECKERS = emptyList() private val DEFAULT_CLASSIFIER_USAGE_CHECKERS = listOf( diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/NullableExtensionOperatorWithSafeCallChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/NullableExtensionOperatorWithSafeCallChecker.kt new file mode 100644 index 00000000000..7df25947701 --- /dev/null +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/NullableExtensionOperatorWithSafeCallChecker.kt @@ -0,0 +1,60 @@ +/* + * Copyright 2010-2020 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. + */ + +package org.jetbrains.kotlin.resolve.checkers + +import com.intellij.psi.PsiElement +import org.jetbrains.kotlin.diagnostics.Errors +import org.jetbrains.kotlin.name.Name +import org.jetbrains.kotlin.psi.KtCallExpression +import org.jetbrains.kotlin.psi.KtNameReferenceExpression +import org.jetbrains.kotlin.psi.KtParenthesizedExpression +import org.jetbrains.kotlin.psi.KtSafeQualifiedExpression +import org.jetbrains.kotlin.resolve.calls.checkers.CallChecker +import org.jetbrains.kotlin.resolve.calls.checkers.CallCheckerContext +import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall +import org.jetbrains.kotlin.resolve.calls.model.isReallySuccess +import org.jetbrains.kotlin.resolve.scopes.receivers.ExpressionReceiver +import org.jetbrains.kotlin.resolve.scopes.receivers.ReceiverValue +import org.jetbrains.kotlin.types.isFlexible +import org.jetbrains.kotlin.types.isNullable +import org.jetbrains.kotlin.util.OperatorNameConventions + +object NullableExtensionOperatorWithSafeCallChecker : CallChecker { + private val RELEVANT_OPERATORS = mutableSetOf().apply { + addAll(OperatorNameConventions.ASSIGNMENT_OPERATIONS) + add(OperatorNameConventions.INC) + add(OperatorNameConventions.DEC) + add(OperatorNameConventions.GET) + add(OperatorNameConventions.SET) + } + + override fun check(resolvedCall: ResolvedCall<*>, reportOn: PsiElement, context: CallCheckerContext) { + if (!resolvedCall.isReallySuccess()) return + val name = resolvedCall.resultingDescriptor.name + if (name !in RELEVANT_OPERATORS) return + + if (!isNullableSafeCallReceiver(resolvedCall.extensionReceiver)) return + + val callElement = resolvedCall.call.callElement + + // It's an operator call, not a regular one + if (callElement is KtCallExpression && name.identifier == (callElement.calleeExpression as? KtNameReferenceExpression)?.getReferencedName()) return + + context.trace.report(Errors.NULLABLE_EXTENSION_OPERATOR_WITH_SAFE_CALL_RECEIVER.on(reportOn)) + } + + private fun isNullableSafeCallReceiver(receiverValue: ReceiverValue?): Boolean { + if (receiverValue !is ExpressionReceiver) return false + if (!receiverValue.type.isNullable() || receiverValue.type.isFlexible()) return false + + val expression = receiverValue.expression + + if (expression !is KtSafeQualifiedExpression) return false + if (expression.parent is KtParenthesizedExpression) return false + + return true + } +} diff --git a/compiler/testData/diagnostics/tests/nullableTypes/safeCallOperators.fir.kt b/compiler/testData/diagnostics/tests/nullableTypes/safeCallOperators.fir.kt new file mode 100644 index 00000000000..d437896e0d8 --- /dev/null +++ b/compiler/testData/diagnostics/tests/nullableTypes/safeCallOperators.fir.kt @@ -0,0 +1,91 @@ +// SKIP_TXT +// !DIAGNOSTICS: -UNUSED_PARAMETER + +class A( + val l: MutableList, + val ll: MutableList>, + var w: Int, + val q: () -> Unit +) + +operator fun MutableList?.plusAssign(t: Any?) {} +operator fun MutableList?.set(t: Int, w: Any?) {} +operator fun List?.get(t: Int): T? = this?.get(t) +operator fun Int?.inc(): Int = 1 +operator fun (() -> Unit)?.invoke() {} + +fun foo(a: A?) { + a?.l += 1 + a?.l[0] + a?.l[0]++ + a?.l[0] = 1 + + a?.ll[0][0] + a?.ll[0][0]++ + a?.ll[0][0] = 1 + // No warning is reported because + // 1. All kinds of green code with safe+call + invoke we identified fails with CCE if `a != null`, anyway + // 2. In case of null value, the behavior is intended (no call performed) + a?.q() + a?.w++ + + (a?.l) += 1 + (a?.l)[0] + (a?.l)[0]++ + (a?.l)[0] = 1 + + (a?.ll)[0][0] + (a?.ll)[0][0]++ + (a?.ll)[0][0] = 1 + (a?.q)() + (a?.w)++ + + a?.l.plusAssign(1) + a?.l.get(0) + a?.l.get(0).inc() + a?.l.set(0, 1) + + a?.ll.get(0).get(0) + a?.ll.get(0).get(0).inc() + a?.ll.get(0).set(0, 1) + a?.q.invoke() + a?.w.inc() + + if (a != null) { + a?.l += 1 + a?.l[0] + a?.l[0]++ + a?.l[0] = 1 + + a?.ll[0][0] + a?.ll[0][0]++ + a?.ll[0][0] = 1 + // No warning is reported because + // 1. All kinds of green code with safe+call + invoke we identified fails with CCE if `a != null`, anyway + // 2. In case of null value, the behavior is intended (no call performed) + a?.q() + a?.w++ + + (a?.l) += 1 + (a?.l)[0] + (a?.l)[0]++ + (a?.l)[0] = 1 + + (a?.ll)[0][0] + (a?.ll)[0][0]++ + (a?.ll)[0][0] = 1 + (a?.q)() + (a?.w)++ + + a?.l.plusAssign(1) + a?.l.get(0) + a?.l.get(0).inc() + a?.l.set(0, 1) + + a?.ll.get(0).get(0) + a?.ll.get(0).get(0).inc() + a?.ll.get(0).set(0, 1) + a?.q.invoke() + a?.w.inc() + } +} diff --git a/compiler/testData/diagnostics/tests/nullableTypes/safeCallOperators.kt b/compiler/testData/diagnostics/tests/nullableTypes/safeCallOperators.kt new file mode 100644 index 00000000000..2de57b312cc --- /dev/null +++ b/compiler/testData/diagnostics/tests/nullableTypes/safeCallOperators.kt @@ -0,0 +1,91 @@ +// SKIP_TXT +// !DIAGNOSTICS: -UNUSED_PARAMETER + +class A( + val l: MutableList, + val ll: MutableList>, + var w: Int, + val q: () -> Unit +) + +operator fun MutableList?.plusAssign(t: Any?) {} +operator fun MutableList?.set(t: Int, w: Any?) {} +operator fun List?.get(t: Int): T? = this?.get(t) +operator fun Int?.inc(): Int = 1 +operator fun (() -> Unit)?.invoke() {} + +fun foo(a: A?) { + a?.l += 1 + a?.l[0] + a?.l[0]++ + a?.l[0] = 1 + + a?.ll[0][0] + a?.ll[0][0]++ + a?.ll[0][0] = 1 + // No warning is reported because + // 1. All kinds of green code with safe+call + invoke we identified fails with CCE if `a != null`, anyway + // 2. In case of null value, the behavior is intended (no call performed) + a?.q() + a?.w++ + + (a?.l) += 1 + (a?.l)[0] + (a?.l)[0]++ + (a?.l)[0] = 1 + + (a?.ll)[0][0] + (a?.ll)[0][0]++ + (a?.ll)[0][0] = 1 + (a?.q)() + (a?.w)++ + + a?.l.plusAssign(1) + a?.l.get(0) + a?.l.get(0).inc() + a?.l.set(0, 1) + + a?.ll.get(0).get(0) + a?.ll.get(0).get(0).inc() + a?.ll.get(0).set(0, 1) + a?.q.invoke() + a?.w.inc() + + if (a != null) { + a?.l += 1 + a?.l[0] + a?.l[0]++ + a?.l[0] = 1 + + a?.ll[0][0] + a?.ll[0][0]++ + a?.ll[0][0] = 1 + // No warning is reported because + // 1. All kinds of green code with safe+call + invoke we identified fails with CCE if `a != null`, anyway + // 2. In case of null value, the behavior is intended (no call performed) + a?.q() + a?.w++ + + (a?.l) += 1 + (a?.l)[0] + (a?.l)[0]++ + (a?.l)[0] = 1 + + (a?.ll)[0][0] + (a?.ll)[0][0]++ + (a?.ll)[0][0] = 1 + (a?.q)() + (a?.w)++ + + a?.l.plusAssign(1) + a?.l.get(0) + a?.l.get(0).inc() + a?.l.set(0, 1) + + a?.ll.get(0).get(0) + a?.ll.get(0).get(0).inc() + a?.ll.get(0).set(0, 1) + a?.q.invoke() + a?.w.inc() + } +} diff --git a/compiler/tests-spec/testData/diagnostics/linked/overload-resolution/building-the-overload-candidate-set-ocs/operator-call/p-1/pos/2.1.kt b/compiler/tests-spec/testData/diagnostics/linked/overload-resolution/building-the-overload-candidate-set-ocs/operator-call/p-1/pos/2.1.kt index b31a0d68b0e..0829021d30f 100644 --- a/compiler/tests-spec/testData/diagnostics/linked/overload-resolution/building-the-overload-candidate-set-ocs/operator-call/p-1/pos/2.1.kt +++ b/compiler/tests-spec/testData/diagnostics/linked/overload-resolution/building-the-overload-candidate-set-ocs/operator-call/p-1/pos/2.1.kt @@ -54,15 +54,15 @@ package testPackCase2 fun case2(a: A?, c: C) { - a?.b += c + a?.b += c a?.b.plusAssign(c) val x = { - a?.b += c + a?.b += c a?.b.plusAssign(c) }() - a?.b += { c }() + a?.b += { c }() a?.b.plusAssign({ c }()) @@ -92,15 +92,15 @@ package testPackCase3 fun case3(a: A?, c: C) { - a?.b += c + a?.b += c a?.b.plusAssign(c) val x = { - a?.b += c + a?.b += c a?.b.plusAssign(c) }() - a?.b += { c }() + a?.b += { c }() a?.b.plusAssign({ c }()) @@ -128,4 +128,4 @@ operator fun B?.plusAssign(c: Any) { } -class C \ No newline at end of file +class C diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index 6edeccc5e49..1542c3b5490 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -15828,6 +15828,11 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTestWithFirVali runTest("compiler/testData/diagnostics/tests/nullableTypes/safeCallOnTypeWithNullableUpperBound.kt"); } + @TestMetadata("safeCallOperators.kt") + public void testSafeCallOperators() throws Exception { + runTest("compiler/testData/diagnostics/tests/nullableTypes/safeCallOperators.kt"); + } + @TestMetadata("safeCallWithInvoke.kt") public void testSafeCallWithInvoke() throws Exception { runTest("compiler/testData/diagnostics/tests/nullableTypes/safeCallWithInvoke.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java index d120607577d..282110adefc 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java @@ -15823,6 +15823,11 @@ public class DiagnosticsUsingJavacTestGenerated extends AbstractDiagnosticsUsing runTest("compiler/testData/diagnostics/tests/nullableTypes/safeCallOnTypeWithNullableUpperBound.kt"); } + @TestMetadata("safeCallOperators.kt") + public void testSafeCallOperators() throws Exception { + runTest("compiler/testData/diagnostics/tests/nullableTypes/safeCallOperators.kt"); + } + @TestMetadata("safeCallWithInvoke.kt") public void testSafeCallWithInvoke() throws Exception { runTest("compiler/testData/diagnostics/tests/nullableTypes/safeCallWithInvoke.kt");