From 5760a94ab461dad6d840538cec830e3c3d5e6675 Mon Sep 17 00:00:00 2001 From: Ivan Kochurkin Date: Fri, 10 Sep 2021 15:56:12 +0300 Subject: [PATCH] [FIR] Add CONCURRENT_HASH_MAP_CONTAINS_OPERATOR --- .../diagnostics/FirJvmDiagnosticsList.kt | 1 + .../analysis/diagnostics/jvm/FirJvmErrors.kt | 2 + .../jvm/checkers/JvmExpressionCheckers.kt | 3 +- ...InconsistentOperatorFromJavaCallChecker.kt | 64 +++++++++++++++++++ .../diagnostics/FirJvmDefaultErrorMessages.kt | 10 ++- .../fir/analysis/checkers/FirHelpers.kt | 11 +++- .../java/concurrentHashMapContains.fir.kt | 32 +++++----- .../concurrentHashMapContainsError.fir.kt | 33 +++++----- .../diagnostics/KtFirDataClassConverters.kt | 12 ++++ .../api/fir/diagnostics/KtFirDiagnostics.kt | 8 +++ .../fir/diagnostics/KtFirDiagnosticsImpl.kt | 14 ++++ 11 files changed, 152 insertions(+), 38 deletions(-) create mode 100644 compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/expression/FirJvmInconsistentOperatorFromJavaCallChecker.kt diff --git a/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirJvmDiagnosticsList.kt b/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirJvmDiagnosticsList.kt index a5839a52d51..51791ec72d4 100644 --- a/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirJvmDiagnosticsList.kt +++ b/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirJvmDiagnosticsList.kt @@ -173,5 +173,6 @@ object JVM_DIAGNOSTICS_LIST : DiagnosticList("FirJvmErrors") { PositioningStrategy.REFERENCE_BY_QUALIFIED ) val SUBCLASS_CANT_CALL_COMPANION_PROTECTED_NON_STATIC by error(PositioningStrategy.REFERENCED_NAME_BY_QUALIFIED) + val CONCURRENT_HASH_MAP_CONTAINS_OPERATOR by deprecationError(ProhibitConcurrentHashMapContains) } } diff --git a/compiler/fir/checkers/checkers.jvm/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/jvm/FirJvmErrors.kt b/compiler/fir/checkers/checkers.jvm/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/jvm/FirJvmErrors.kt index 5a0517fa0cb..02f4286efd6 100644 --- a/compiler/fir/checkers/checkers.jvm/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/jvm/FirJvmErrors.kt +++ b/compiler/fir/checkers/checkers.jvm/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/jvm/FirJvmErrors.kt @@ -7,6 +7,7 @@ package org.jetbrains.kotlin.fir.analysis.diagnostics.jvm import com.intellij.psi.PsiElement import org.jetbrains.kotlin.config.LanguageFeature.DefaultMethodsCallFromJava6TargetError +import org.jetbrains.kotlin.config.LanguageFeature.ProhibitConcurrentHashMapContains import org.jetbrains.kotlin.config.LanguageFeature.ProhibitJvmOverloadsOnConstructorsOfAnnotationClasses import org.jetbrains.kotlin.config.LanguageFeature.RepeatableAnnotationContainerConstraints import org.jetbrains.kotlin.fir.analysis.diagnostics.* @@ -116,5 +117,6 @@ object FirJvmErrors { val DEFAULT_METHOD_CALL_FROM_JAVA6_TARGET by deprecationError0(DefaultMethodsCallFromJava6TargetError, SourceElementPositioningStrategies.REFERENCE_BY_QUALIFIED) val INTERFACE_STATIC_METHOD_CALL_FROM_JAVA6_TARGET by deprecationError0(DefaultMethodsCallFromJava6TargetError, SourceElementPositioningStrategies.REFERENCE_BY_QUALIFIED) val SUBCLASS_CANT_CALL_COMPANION_PROTECTED_NON_STATIC by error0(SourceElementPositioningStrategies.REFERENCED_NAME_BY_QUALIFIED) + val CONCURRENT_HASH_MAP_CONTAINS_OPERATOR by deprecationError0(ProhibitConcurrentHashMapContains) } diff --git a/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/JvmExpressionCheckers.kt b/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/JvmExpressionCheckers.kt index c0d3607f63f..91d29ba7269 100644 --- a/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/JvmExpressionCheckers.kt +++ b/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/JvmExpressionCheckers.kt @@ -23,7 +23,8 @@ object JvmExpressionCheckers : ExpressionCheckers() { get() = setOf( FirJavaGenericVarianceViolationTypeChecker, FirSuperCallWithDefaultsChecker, - FirJvmSuspensionPointInsideMutexLockChecker + FirJvmSuspensionPointInsideMutexLockChecker, + FirJvmInconsistentOperatorFromJavaCallChecker ) override val annotationCheckers: Set diff --git a/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/expression/FirJvmInconsistentOperatorFromJavaCallChecker.kt b/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/expression/FirJvmInconsistentOperatorFromJavaCallChecker.kt new file mode 100644 index 00000000000..aef9b2806aa --- /dev/null +++ b/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/expression/FirJvmInconsistentOperatorFromJavaCallChecker.kt @@ -0,0 +1,64 @@ +/* + * Copyright 2010-2021 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.fir.analysis.jvm.checkers.expression + +import org.jetbrains.kotlin.fir.FirSourceElement +import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext +import org.jetbrains.kotlin.fir.analysis.checkers.expression.FirFunctionCallChecker +import org.jetbrains.kotlin.fir.analysis.checkers.overriddenFunctions +import org.jetbrains.kotlin.fir.analysis.diagnostics.DiagnosticReporter +import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors +import org.jetbrains.kotlin.fir.analysis.diagnostics.reportOn +import org.jetbrains.kotlin.fir.containingClass +import org.jetbrains.kotlin.fir.expressions.FirFunctionCall +import org.jetbrains.kotlin.fir.expressions.FirFunctionCallOrigin +import org.jetbrains.kotlin.fir.expressions.toResolvedCallableSymbol +import org.jetbrains.kotlin.fir.resolve.toFirRegularClassSymbol +import org.jetbrains.kotlin.fir.symbols.impl.FirNamedFunctionSymbol +import org.jetbrains.kotlin.fir.types.coneType +import org.jetbrains.kotlin.fir.types.isAny +import org.jetbrains.kotlin.fir.types.isNullableAny +import org.jetbrains.kotlin.fir.types.lowerBoundIfFlexible +import org.jetbrains.kotlin.name.CallableId +import org.jetbrains.kotlin.name.ClassId +import org.jetbrains.kotlin.name.Name +import org.jetbrains.kotlin.util.OperatorNameConventions + +object FirJvmInconsistentOperatorFromJavaCallChecker : FirFunctionCallChecker() { + private val CONCURRENT_HASH_MAP_CALLABLE_ID = CallableId( + ClassId.fromString("java/util/concurrent/ConcurrentHashMap"), + Name.identifier("contains") + ) + + override fun check(expression: FirFunctionCall, context: CheckerContext, reporter: DiagnosticReporter) { + val callableSymbol = expression.calleeReference.toResolvedCallableSymbol() as? FirNamedFunctionSymbol ?: return + if (callableSymbol.name != OperatorNameConventions.CONTAINS) return + val valueParameterSymbol = callableSymbol.valueParameterSymbols.singleOrNull() ?: return + val type = valueParameterSymbol.resolvedReturnTypeRef.coneType.lowerBoundIfFlexible() + if (!type.isAny && !type.isNullableAny) return + + if (expression.origin != FirFunctionCallOrigin.Operator || expression.origin.ordinal != 2) return + + callableSymbol.check(expression.calleeReference.source, context, reporter) + } + + fun FirNamedFunctionSymbol.check(source: FirSourceElement?, context: CheckerContext, reporter: DiagnosticReporter): Boolean { + if (callableId == CONCURRENT_HASH_MAP_CALLABLE_ID) { + reporter.reportOn(source, FirJvmErrors.CONCURRENT_HASH_MAP_CONTAINS_OPERATOR, context) + return true + } + + val containingClass = containingClass()?.toFirRegularClassSymbol(context.session) ?: return false + val overriddenFunctions = overriddenFunctions(containingClass, context) + for (overriddenFunction in overriddenFunctions) { + if (overriddenFunction is FirNamedFunctionSymbol && overriddenFunction.check(source, context, reporter)) { + return true + } + } + + return false + } +} \ No newline at end of file diff --git a/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/diagnostics/FirJvmDefaultErrorMessages.kt b/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/diagnostics/FirJvmDefaultErrorMessages.kt index 2ffa1e53409..53fb9360203 100644 --- a/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/diagnostics/FirJvmDefaultErrorMessages.kt +++ b/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/diagnostics/FirJvmDefaultErrorMessages.kt @@ -5,12 +5,12 @@ package org.jetbrains.kotlin.fir.analysis.jvm.diagnostics -import org.jetbrains.kotlin.diagnostics.rendering.Renderers.NAME import org.jetbrains.kotlin.diagnostics.rendering.CommonRenderers.STRING import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDefaultErrorMessages import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticRenderers.RENDER_TYPE import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticRenderers.SYMBOL import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticRenderers.TO_STRING +import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.CONCURRENT_HASH_MAP_CONTAINS_OPERATOR import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.CONFLICTING_JVM_DECLARATIONS import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.DEFAULT_METHOD_CALL_FROM_JAVA6_TARGET import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.DELEGATION_BY_IN_JVM_RECORD @@ -23,9 +23,9 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.EXTERNAL_D import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.FIELD_IN_JVM_RECORD import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.ILLEGAL_JAVA_LANG_RECORD_SUPERTYPE import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.ILLEGAL_JVM_NAME -import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.INAPPLICABLE_JVM_NAME import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.INAPPLICABLE_JVM_FIELD import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.INAPPLICABLE_JVM_FIELD_WARNING +import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.INAPPLICABLE_JVM_NAME import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.INNER_JVM_RECORD import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.INTERFACE_CANT_CALL_DEFAULT_METHOD_VIA_SUPER import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.INTERFACE_STATIC_METHOD_CALL_FROM_JAVA6_TARGET @@ -264,6 +264,12 @@ object FirJvmDefaultErrorMessages { "The ''{0}'' suspension point is inside a critical section", SYMBOL ) + map.put( + CONCURRENT_HASH_MAP_CONTAINS_OPERATOR, + "Method 'contains' from ConcurrentHashMap may have unexpected semantics: it calls 'containsValue' instead of 'containsKey'. " + + "Use explicit form of the call to 'containsKey'/'containsValue'/'contains' or cast the value to kotlin.collections.Map instead. " + + "See https://youtrack.jetbrains.com/issue/KT-18053 for more details" + ) } } } \ No newline at end of file diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirHelpers.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirHelpers.kt index eca6aa1d3e5..2df8c90e746 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirHelpers.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirHelpers.kt @@ -185,6 +185,13 @@ fun CheckerContext.findClosestClassOrObject(): FirClass? { fun FirSimpleFunction.overriddenFunctions( containingClass: FirClassSymbol<*>, context: CheckerContext +): List> { + return symbol.overriddenFunctions(containingClass, context) +} + +fun FirNamedFunctionSymbol.overriddenFunctions( + containingClass: FirClassSymbol<*>, + context: CheckerContext ): List> { val firTypeScope = containingClass.unsubstitutedScope( context.sessionHolder.session, @@ -193,8 +200,8 @@ fun FirSimpleFunction.overriddenFunctions( ) val overriddenFunctions = mutableListOf>() - firTypeScope.processFunctionsByName(symbol.callableId.callableName) { } - firTypeScope.processOverriddenFunctions(symbol) { + firTypeScope.processFunctionsByName(callableId.callableName) { } + firTypeScope.processOverriddenFunctions(this) { overriddenFunctions.add(it) ProcessorAction.NEXT } diff --git a/compiler/testData/diagnostics/testsWithStdLib/java/concurrentHashMapContains.fir.kt b/compiler/testData/diagnostics/testsWithStdLib/java/concurrentHashMapContains.fir.kt index 7f11960bfb7..52f24c84d08 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/java/concurrentHashMapContains.fir.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/java/concurrentHashMapContains.fir.kt @@ -18,10 +18,10 @@ class C : java.util.concurrent.ConcurrentHashMap() { fun main() { val hm = java.util.concurrent.ConcurrentHashMap() - "" in hm - "" !in hm - 1 !in hm - 2 in hm + "" in hm + "" !in hm + 1 !in hm + 2 in hm hm.contains("") hm.contains(1) @@ -32,10 +32,10 @@ fun main() { 1 !in (hm as Map) val a = A() - "" in a - "" !in a - 1 !in a - 2 in a + "" in a + "" !in a + 1 !in a + 2 in a ' ' in a ' ' !in a @@ -48,10 +48,10 @@ fun main() { 1 !in (a as Map) val b = B() - "" in b - "" !in b - 1 !in b - 2 in b + "" in b + "" !in b + 1 !in b + 2 in b b.contains("") b.contains(1) @@ -63,10 +63,10 @@ fun main() { // Actually, we could've allow calls here because the owner explicitly declared as operator, but semantics is still weird val c = C() - "" in c - "" !in c - 1 !in c - 2 in c + "" in c + "" !in c + 1 !in c + 2 in c c.contains("") c.contains(1) diff --git a/compiler/testData/diagnostics/testsWithStdLib/java/concurrentHashMapContainsError.fir.kt b/compiler/testData/diagnostics/testsWithStdLib/java/concurrentHashMapContainsError.fir.kt index 6c4d1f7418d..5308cee86ce 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/java/concurrentHashMapContainsError.fir.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/java/concurrentHashMapContainsError.fir.kt @@ -18,10 +18,10 @@ class C : java.util.concurrent.ConcurrentHashMap() { fun main() { val hm = java.util.concurrent.ConcurrentHashMap() - "" in hm - "" !in hm - 1 !in hm - 2 in hm + "" in hm + "" !in hm + 1 !in hm + 2 in hm hm.contains("") hm.contains(1) @@ -32,10 +32,10 @@ fun main() { 1 !in (hm as Map) val a = A() - "" in a - "" !in a - 1 !in a - 2 in a + "" in a + "" !in a + 1 !in a + 2 in a ' ' in a ' ' !in a @@ -48,10 +48,10 @@ fun main() { 1 !in (a as Map) val b = B() - "" in b - "" !in b - 1 !in b - 2 in b + "" in b + "" !in b + 1 !in b + 2 in b b.contains("") b.contains(1) @@ -63,10 +63,10 @@ fun main() { // Actually, we could've allow calls here because the owner explicitly declared as operator, but semantics is still weird val c = C() - "" in c - "" !in c - 1 !in c - 2 in c + "" in c + "" !in c + 1 !in c + 2 in c c.contains("") c.contains(1) @@ -76,4 +76,3 @@ fun main() { 1 in (c as Map) 1 !in (c as Map) } - diff --git a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDataClassConverters.kt b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDataClassConverters.kt index 51c521b57c8..d09cec96f48 100644 --- a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDataClassConverters.kt +++ b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDataClassConverters.kt @@ -4047,4 +4047,16 @@ internal val KT_DIAGNOSTIC_CONVERTER = KtDiagnosticConverterBuilder.buildConvert token, ) } + add(FirJvmErrors.CONCURRENT_HASH_MAP_CONTAINS_OPERATOR.errorFactory) { firDiagnostic -> + ConcurrentHashMapContainsOperatorErrorImpl( + firDiagnostic as FirPsiDiagnostic, + token, + ) + } + add(FirJvmErrors.CONCURRENT_HASH_MAP_CONTAINS_OPERATOR.warningFactory) { firDiagnostic -> + ConcurrentHashMapContainsOperatorWarningImpl( + firDiagnostic as FirPsiDiagnostic, + token, + ) + } } diff --git a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnostics.kt b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnostics.kt index 03026f4a3d9..a33efb46346 100644 --- a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnostics.kt +++ b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnostics.kt @@ -2816,4 +2816,12 @@ sealed class KtFirDiagnostic : KtDiagnosticWithPsi { override val diagnosticClass get() = SubclassCantCallCompanionProtectedNonStatic::class } + abstract class ConcurrentHashMapContainsOperatorError : KtFirDiagnostic() { + override val diagnosticClass get() = ConcurrentHashMapContainsOperatorError::class + } + + abstract class ConcurrentHashMapContainsOperatorWarning : KtFirDiagnostic() { + override val diagnosticClass get() = ConcurrentHashMapContainsOperatorWarning::class + } + } diff --git a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnosticsImpl.kt b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnosticsImpl.kt index 7ff7aa3efb9..b48329e6d5b 100644 --- a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnosticsImpl.kt +++ b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnosticsImpl.kt @@ -4581,3 +4581,17 @@ internal class SubclassCantCallCompanionProtectedNonStaticImpl( override val firDiagnostic: FirPsiDiagnostic by weakRef(firDiagnostic) } +internal class ConcurrentHashMapContainsOperatorErrorImpl( + firDiagnostic: FirPsiDiagnostic, + override val token: ValidityToken, +) : KtFirDiagnostic.ConcurrentHashMapContainsOperatorError(), KtAbstractFirDiagnostic { + override val firDiagnostic: FirPsiDiagnostic by weakRef(firDiagnostic) +} + +internal class ConcurrentHashMapContainsOperatorWarningImpl( + firDiagnostic: FirPsiDiagnostic, + override val token: ValidityToken, +) : KtFirDiagnostic.ConcurrentHashMapContainsOperatorWarning(), KtAbstractFirDiagnostic { + override val firDiagnostic: FirPsiDiagnostic by weakRef(firDiagnostic) +} +