From 32bb64a225b5231b9932db53dc2d7b6826e59914 Mon Sep 17 00:00:00 2001 From: Mark Punzalan Date: Tue, 25 May 2021 17:56:59 +0000 Subject: [PATCH] FIR: Report UNSAFE_OPERATOR_CALL for augmented assignments (was reporting UNSAFE_CALL). --- ...irOldFrontendDiagnosticsTestGenerated.java | 6 ++++++ ...DiagnosticsWithLightTreeTestGenerated.java | 6 ++++++ .../coneDiagnosticToFirDiagnostic.kt | 13 +++++++++--- .../FirExpressionsResolveTransformer.kt | 4 ++-- .../augmentedAssignment.fir.kt | 17 ++++++++++++++++ .../augmentedAssignment.kt | 17 ++++++++++++++++ .../augmentedAssignment.txt | 20 +++++++++++++++++++ .../test/runners/DiagnosticTestGenerated.java | 6 ++++++ .../quickfix/fixes/ReplaceCallFixFactories.kt | 3 ++- ...CompilerTestFE10TestdataTestGenerated.java | 5 +++++ .../augmentedAssignmentAvailable.kt | 4 +--- .../augmentedAssignmentAvailable.kt.after | 4 +--- 12 files changed, 93 insertions(+), 12 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/augmentedAssignment.fir.kt create mode 100644 compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/augmentedAssignment.kt create mode 100644 compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/augmentedAssignment.txt diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java index 7a4c916782a..0fb8136376c 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java @@ -19087,6 +19087,12 @@ public class FirOldFrontendDiagnosticsTestGenerated extends AbstractFirDiagnosti runTest("compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/AssertNotNull.kt"); } + @Test + @TestMetadata("augmentedAssignment.kt") + public void testAugmentedAssignment() throws Exception { + runTest("compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/augmentedAssignment.kt"); + } + @Test @TestMetadata("dataFlowInfoAfterExclExcl.kt") public void testDataFlowInfoAfterExclExcl() throws Exception { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java index 6b1253bc4c3..c8ab746a16d 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java @@ -19087,6 +19087,12 @@ public class FirOldFrontendDiagnosticsWithLightTreeTestGenerated extends Abstrac runTest("compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/AssertNotNull.kt"); } + @Test + @TestMetadata("augmentedAssignment.kt") + public void testAugmentedAssignment() throws Exception { + runTest("compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/augmentedAssignment.kt"); + } + @Test @TestMetadata("dataFlowInfoAfterExclExcl.kt") public void testDataFlowInfoAfterExclExcl() throws Exception { diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/coneDiagnosticToFirDiagnostic.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/coneDiagnosticToFirDiagnostic.kt index 1661c7ddad2..19585133cb7 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/coneDiagnosticToFirDiagnostic.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/coneDiagnosticToFirDiagnostic.kt @@ -100,11 +100,18 @@ private fun mapUnsafeCallError( val receiverExpression = candidate.callInfo.explicitReceiver val singleArgument = candidate.callInfo.argumentList.arguments.singleOrNull() if (receiverExpression != null && singleArgument != null && - source.elementType == KtNodeTypes.OPERATION_REFERENCE && + (source.elementType == KtNodeTypes.OPERATION_REFERENCE || source.elementType == KtNodeTypes.BINARY_EXPRESSION) && (candidateFunction?.isOperator == true || candidateFunction?.isInfix == true) ) { - val operationToken = source.getChild(KtTokens.IDENTIFIER) - return if (operationToken != null) { + // For augmented assignment operations (e.g., `a += b`), the source is the entire binary expression (BINARY_EXPRESSION). + // TODO: No need to check for source.elementType == BINARY_EXPRESSION if we use operator as callee reference source + // (see FirExpressionsResolveTransformer.transformAssignmentOperatorStatement) + val operationSource = if (source.elementType == KtNodeTypes.BINARY_EXPRESSION) { + source.getChild(KtNodeTypes.OPERATION_REFERENCE) + } else { + source + } + return if (operationSource?.getChild(KtTokens.IDENTIFIER) != null) { FirErrors.UNSAFE_INFIX_CALL.on( source, receiverExpression, diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirExpressionsResolveTransformer.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirExpressionsResolveTransformer.kt index 1878ab11080..6de302c4a19 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirExpressionsResolveTransformer.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirExpressionsResolveTransformer.kt @@ -26,8 +26,6 @@ import org.jetbrains.kotlin.fir.resolve.diagnostics.* import org.jetbrains.kotlin.fir.resolve.inference.FirStubInferenceSession import org.jetbrains.kotlin.fir.resolve.inference.inferenceComponents import org.jetbrains.kotlin.fir.resolve.substitution.ConeSubstitutor -import org.jetbrains.kotlin.fir.resolve.substitution.substituteOrNull -import org.jetbrains.kotlin.fir.resolve.substitution.substitutorByMap import org.jetbrains.kotlin.fir.resolve.transformers.* import org.jetbrains.kotlin.fir.symbols.impl.FirVariableSymbol import org.jetbrains.kotlin.fir.types.* @@ -41,6 +39,7 @@ import org.jetbrains.kotlin.resolve.calls.tower.CandidateApplicability import org.jetbrains.kotlin.types.AbstractTypeChecker import org.jetbrains.kotlin.types.ConstantValueKind import org.jetbrains.kotlin.types.TypeApproximatorConfiguration +import org.jetbrains.kotlin.utils.addToStdlib.firstIsInstanceOrNull open class FirExpressionsResolveTransformer(transformer: FirBodyResolveTransformer) : FirPartialBodyResolveTransformer(transformer) { private inline val builtinTypes: BuiltinTypes get() = session.builtinTypes @@ -372,6 +371,7 @@ open class FirExpressionsResolveTransformer(transformer: FirBodyResolveTransform explicitReceiver = leftArgument argumentList = buildUnaryArgumentList(rightArgument) calleeReference = buildSimpleNamedReference { + // TODO: Use source of operator for callee reference source source = assignmentOperatorStatement.source?.fakeElement(FirFakeSourceElementKind.DesugaredCompoundAssignment) this.name = name candidateSymbol = null diff --git a/compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/augmentedAssignment.fir.kt b/compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/augmentedAssignment.fir.kt new file mode 100644 index 00000000000..a2c0dc24b08 --- /dev/null +++ b/compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/augmentedAssignment.fir.kt @@ -0,0 +1,17 @@ +class A { + operator fun plusAssign(s: String) {} +} + +fun test() { + var a: A? = A() + a += "" +} + +class B { + operator fun plus(other: B) = this +} + +fun test2() { + var b: B? = B() + b += B() +} diff --git a/compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/augmentedAssignment.kt b/compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/augmentedAssignment.kt new file mode 100644 index 00000000000..0231a992bff --- /dev/null +++ b/compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/augmentedAssignment.kt @@ -0,0 +1,17 @@ +class A { + operator fun plusAssign(s: String) {} +} + +fun test() { + var a: A? = A() + a += "" +} + +class B { + operator fun plus(other: B) = this +} + +fun test2() { + var b: B? = B() + b += B() +} diff --git a/compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/augmentedAssignment.txt b/compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/augmentedAssignment.txt new file mode 100644 index 00000000000..4f2a87e8db0 --- /dev/null +++ b/compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/augmentedAssignment.txt @@ -0,0 +1,20 @@ +package + +public fun test(): kotlin.Unit +public fun test2(): kotlin.Unit + +public final class A { + public constructor A() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final operator fun plusAssign(/*0*/ s: kotlin.String): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class B { + public constructor B() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final operator fun plus(/*0*/ other: B): B + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java index cf2f301e7e9..01cc18b02a4 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java @@ -19093,6 +19093,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest { runTest("compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/AssertNotNull.kt"); } + @Test + @TestMetadata("augmentedAssignment.kt") + public void testAugmentedAssignment() throws Exception { + runTest("compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/augmentedAssignment.kt"); + } + @Test @TestMetadata("dataFlowInfoAfterExclExcl.kt") public void testDataFlowInfoAfterExclExcl() throws Exception { 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 9fdff3e9163..82e5203de67 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 @@ -15,6 +15,7 @@ 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.getNonStrictParentOfType import org.jetbrains.kotlin.psi.psiUtil.unwrapParenthesesLabelsAndAnnotations import org.jetbrains.kotlin.types.expressions.OperatorConventions import org.jetbrains.kotlin.utils.addToStdlib.safeAs @@ -62,7 +63,7 @@ object ReplaceCallFixFactories { return@diagnosticFixFactory emptyList() } - val target = psi.parent as? KtBinaryExpression ?: return@diagnosticFixFactory emptyList() + val target = psi.getNonStrictParentOfType() ?: return@diagnosticFixFactory emptyList() listOf(ReplaceInfixOrOperatorCallFix(target, shouldHaveNotNullType(target), diagnostic.operator)) } diff --git a/idea/idea-frontend-fir/idea-fir-low-level-api/tests/org/jetbrains/kotlin/idea/fir/low/level/api/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java b/idea/idea-frontend-fir/idea-fir-low-level-api/tests/org/jetbrains/kotlin/idea/fir/low/level/api/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java index f14e63b2f1f..125b0622607 100644 --- a/idea/idea-frontend-fir/idea-fir-low-level-api/tests/org/jetbrains/kotlin/idea/fir/low/level/api/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java +++ b/idea/idea-frontend-fir/idea-fir-low-level-api/tests/org/jetbrains/kotlin/idea/fir/low/level/api/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java @@ -16741,6 +16741,11 @@ public class DiagnosisCompilerTestFE10TestdataTestGenerated extends AbstractDiag runTest("compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/AssertNotNull.kt"); } + @TestMetadata("augmentedAssignment.kt") + public void testAugmentedAssignment() throws Exception { + runTest("compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/augmentedAssignment.kt"); + } + @TestMetadata("dataFlowInfoAfterExclExcl.kt") public void testDataFlowInfoAfterExclExcl() throws Exception { runTest("compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/dataFlowInfoAfterExclExcl.kt"); diff --git a/idea/testData/quickfix/replaceInfixOrOperatorCall/augmentedAssignmentAvailable.kt b/idea/testData/quickfix/replaceInfixOrOperatorCall/augmentedAssignmentAvailable.kt index 1d3a88f0689..d2de26ccfe2 100644 --- a/idea/testData/quickfix/replaceInfixOrOperatorCall/augmentedAssignmentAvailable.kt +++ b/idea/testData/quickfix/replaceInfixOrOperatorCall/augmentedAssignmentAvailable.kt @@ -6,6 +6,4 @@ class A { fun foo(b: A) { var a: A? = A() a += b -} - -/* IGNORE_FIR */ +} \ No newline at end of file diff --git a/idea/testData/quickfix/replaceInfixOrOperatorCall/augmentedAssignmentAvailable.kt.after b/idea/testData/quickfix/replaceInfixOrOperatorCall/augmentedAssignmentAvailable.kt.after index cd4c520d4fa..f90ca8a85d1 100644 --- a/idea/testData/quickfix/replaceInfixOrOperatorCall/augmentedAssignmentAvailable.kt.after +++ b/idea/testData/quickfix/replaceInfixOrOperatorCall/augmentedAssignmentAvailable.kt.after @@ -6,6 +6,4 @@ class A { fun foo(b: A) { var a: A? = A() a?.plusAssign(b) -} - -/* IGNORE_FIR */ +} \ No newline at end of file