[FIR] Merge the two type operator checkers into a single one
This way it's easier to reason about where useless casts/is checks come from, because everything is on the same screen. `USELESS_CAST` disappeared from `FirPsiJsOldFrontendDiagnosticsTestGenerated.testDynamicCastTarget` because `LanguageFeature.EnableDfaWarningsInK2` is disabled, and previously it only affected `FirCastOperatorsChecker`, but not `FirUselessTypeOperationCallChecker`, which felt like an unintended mistake. A related issue: KT-50965
This commit is contained in:
committed by
Space Team
parent
116ff60325
commit
92d8da621e
-1
@@ -154,7 +154,6 @@ object CommonExpressionCheckers : ExpressionCheckers() {
|
|||||||
|
|
||||||
override val typeOperatorCallCheckers: Set<FirTypeOperatorCallChecker>
|
override val typeOperatorCallCheckers: Set<FirTypeOperatorCallChecker>
|
||||||
get() = setOf(
|
get() = setOf(
|
||||||
FirUselessTypeOperationCallChecker,
|
|
||||||
FirCastOperatorsChecker
|
FirCastOperatorsChecker
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|||||||
+8
-3
@@ -182,11 +182,16 @@ fun isUpcast(context: CheckerContext, candidateType: ConeKotlinType, targetType:
|
|||||||
|
|
||||||
internal fun isRefinementUseless(
|
internal fun isRefinementUseless(
|
||||||
context: CheckerContext,
|
context: CheckerContext,
|
||||||
lhsType: ConeSimpleKotlinType,
|
lhsType: ConeKotlinType,
|
||||||
targetType: ConeKotlinType,
|
targetType: ConeKotlinType,
|
||||||
expression: FirTypeOperatorCall,
|
expression: FirTypeOperatorCall,
|
||||||
arg: FirExpression,
|
|
||||||
): Boolean {
|
): Boolean {
|
||||||
|
if (lhsType is ConeErrorType || targetType is ConeErrorType) {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
|
val arg = expression.argument
|
||||||
|
|
||||||
return when (expression.operation) {
|
return when (expression.operation) {
|
||||||
FirOperation.AS, FirOperation.SAFE_AS -> {
|
FirOperation.AS, FirOperation.SAFE_AS -> {
|
||||||
if (arg is FirFunctionCall) {
|
if (arg is FirFunctionCall) {
|
||||||
@@ -211,7 +216,7 @@ internal fun isRefinementUseless(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun isExactTypeCast(context: CheckerContext, lhsType: ConeSimpleKotlinType, targetType: ConeKotlinType): Boolean {
|
private fun isExactTypeCast(context: CheckerContext, lhsType: ConeKotlinType, targetType: ConeKotlinType): Boolean {
|
||||||
if (!AbstractTypeChecker.equalTypes(context.session.typeContext, lhsType, targetType, stubTypesEqualToAnything = false))
|
if (!AbstractTypeChecker.equalTypes(context.session.typeContext, lhsType, targetType, stubTypesEqualToAnything = false))
|
||||||
return false
|
return false
|
||||||
// See comments at [isUpcast] why we need to check the existence of @ExtensionFunctionType
|
// See comments at [isUpcast] why we need to check the existence of @ExtensionFunctionType
|
||||||
|
|||||||
+10
-6
@@ -45,10 +45,9 @@ object FirCastOperatorsChecker : FirTypeOperatorCallChecker(MppCheckerKind.Commo
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// IDE doesn't care this function is referenced as :: and then used polymorphicly
|
|
||||||
@Suppress("UNUSED_PARAMETER")
|
|
||||||
private fun checkIsApplicability(l: TypeInfo, r: TypeInfo, expression: FirTypeOperatorCall, context: CheckerContext): Applicability {
|
private fun checkIsApplicability(l: TypeInfo, r: TypeInfo, expression: FirTypeOperatorCall, context: CheckerContext): Applicability {
|
||||||
return when {
|
return when {
|
||||||
|
isRefinementUseless(context, l.directType.upperBoundIfFlexible(), r.directType, expression) -> Applicability.USELESS_IS_CHECK
|
||||||
isCastErased(l.directType, r.directType, context) -> Applicability.CAST_ERASED
|
isCastErased(l.directType, r.directType, context) -> Applicability.CAST_ERASED
|
||||||
else -> Applicability.APPLICABLE
|
else -> Applicability.APPLICABLE
|
||||||
}
|
}
|
||||||
@@ -61,11 +60,12 @@ object FirCastOperatorsChecker : FirTypeOperatorCallChecker(MppCheckerKind.Commo
|
|||||||
|| l.type.isNullableNothing && !r.type.isNullable
|
|| l.type.isNullableNothing && !r.type.isNullable
|
||||||
|
|
||||||
return when {
|
return when {
|
||||||
|
isRefinementUseless(context, l.directType.upperBoundIfFlexible(), r.directType, expression) -> Applicability.USELESS_CAST
|
||||||
l.type.isNothing && r.type.isNothingOrNullableNothing -> Applicability.APPLICABLE
|
l.type.isNothing && r.type.isNothingOrNullableNothing -> Applicability.APPLICABLE
|
||||||
r.type.isNothing -> Applicability.IMPOSSIBLE
|
r.type.isNothing -> Applicability.IMPOSSIBLE
|
||||||
isNullableNothingWithNotNull -> when (expression.operation) {
|
isNullableNothingWithNotNull -> when (expression.operation) {
|
||||||
// (null as? WhatEver) == null
|
// (null as? WhatEver) == null
|
||||||
FirOperation.SAFE_AS -> Applicability.USELESS
|
FirOperation.SAFE_AS -> Applicability.USELESS_CAST
|
||||||
else -> Applicability.IMPOSSIBLE
|
else -> Applicability.IMPOSSIBLE
|
||||||
}
|
}
|
||||||
oneIsNotNull && oneIsFinal && areUnrelated(l, r, context) -> Applicability.IMPOSSIBLE
|
oneIsNotNull && oneIsFinal && areUnrelated(l, r, context) -> Applicability.IMPOSSIBLE
|
||||||
@@ -92,7 +92,8 @@ object FirCastOperatorsChecker : FirTypeOperatorCallChecker(MppCheckerKind.Commo
|
|||||||
private enum class Applicability {
|
private enum class Applicability {
|
||||||
APPLICABLE,
|
APPLICABLE,
|
||||||
IMPOSSIBLE,
|
IMPOSSIBLE,
|
||||||
USELESS,
|
USELESS_CAST,
|
||||||
|
USELESS_IS_CHECK,
|
||||||
CAST_ERASED,
|
CAST_ERASED,
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -114,9 +115,12 @@ object FirCastOperatorsChecker : FirTypeOperatorCallChecker(MppCheckerKind.Commo
|
|||||||
Applicability.IMPOSSIBLE -> getImpossibilityDiagnostic(l, r, context)?.let {
|
Applicability.IMPOSSIBLE -> getImpossibilityDiagnostic(l, r, context)?.let {
|
||||||
reportOn(expression.source, it, context)
|
reportOn(expression.source, it, context)
|
||||||
}
|
}
|
||||||
Applicability.USELESS -> getUselessnessDiagnostic(context)?.let {
|
Applicability.USELESS_CAST -> getUselessCastDiagnostic(context)?.let {
|
||||||
reportOn(expression.source, it, context)
|
reportOn(expression.source, it, context)
|
||||||
}
|
}
|
||||||
|
Applicability.USELESS_IS_CHECK -> reportOn(
|
||||||
|
expression.source, FirErrors.USELESS_IS_CHECK, expression.operation == FirOperation.IS, context,
|
||||||
|
)
|
||||||
Applicability.CAST_ERASED -> when {
|
Applicability.CAST_ERASED -> when {
|
||||||
expression.operation == FirOperation.AS || expression.operation == FirOperation.SAFE_AS -> {
|
expression.operation == FirOperation.AS || expression.operation == FirOperation.SAFE_AS -> {
|
||||||
reportOn(expression.source, FirErrors.UNCHECKED_CAST, lUserType, rUserType, context)
|
reportOn(expression.source, FirErrors.UNCHECKED_CAST, lUserType, rUserType, context)
|
||||||
@@ -133,7 +137,7 @@ object FirCastOperatorsChecker : FirTypeOperatorCallChecker(MppCheckerKind.Commo
|
|||||||
else -> FirErrors.CAST_NEVER_SUCCEEDS
|
else -> FirErrors.CAST_NEVER_SUCCEEDS
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun getUselessnessDiagnostic(context: CheckerContext) = when {
|
private fun getUselessCastDiagnostic(context: CheckerContext) = when {
|
||||||
!context.languageVersionSettings.supportsFeature(LanguageFeature.EnableDfaWarningsInK2) -> null
|
!context.languageVersionSettings.supportsFeature(LanguageFeature.EnableDfaWarningsInK2) -> null
|
||||||
else -> FirErrors.USELESS_CAST
|
else -> FirErrors.USELESS_CAST
|
||||||
}
|
}
|
||||||
|
|||||||
-47
@@ -1,47 +0,0 @@
|
|||||||
/*
|
|
||||||
* 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.checkers.expression
|
|
||||||
|
|
||||||
import org.jetbrains.kotlin.diagnostics.DiagnosticReporter
|
|
||||||
import org.jetbrains.kotlin.diagnostics.reportOn
|
|
||||||
import org.jetbrains.kotlin.fir.analysis.checkers.MppCheckerKind
|
|
||||||
import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext
|
|
||||||
import org.jetbrains.kotlin.fir.analysis.checkers.isRefinementUseless
|
|
||||||
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors
|
|
||||||
import org.jetbrains.kotlin.fir.expressions.FirOperation
|
|
||||||
import org.jetbrains.kotlin.fir.expressions.FirTypeOperatorCall
|
|
||||||
import org.jetbrains.kotlin.fir.expressions.argument
|
|
||||||
import org.jetbrains.kotlin.fir.resolve.fullyExpandedType
|
|
||||||
import org.jetbrains.kotlin.fir.types.ConeErrorType
|
|
||||||
import org.jetbrains.kotlin.fir.types.coneType
|
|
||||||
import org.jetbrains.kotlin.fir.types.resolvedType
|
|
||||||
import org.jetbrains.kotlin.fir.types.upperBoundIfFlexible
|
|
||||||
|
|
||||||
object FirUselessTypeOperationCallChecker : FirTypeOperatorCallChecker(MppCheckerKind.Common) {
|
|
||||||
override fun check(expression: FirTypeOperatorCall, context: CheckerContext, reporter: DiagnosticReporter) {
|
|
||||||
if (expression.operation !in FirOperation.TYPES) return
|
|
||||||
val arg = expression.argument
|
|
||||||
|
|
||||||
val lhsType = arg.resolvedType.upperBoundIfFlexible().fullyExpandedType(context.session)
|
|
||||||
if (lhsType is ConeErrorType) return
|
|
||||||
|
|
||||||
val targetType = expression.conversionTypeRef.coneType.fullyExpandedType(context.session)
|
|
||||||
if (targetType is ConeErrorType) return
|
|
||||||
|
|
||||||
if (isRefinementUseless(context, lhsType, targetType, expression, arg)) {
|
|
||||||
when (expression.operation) {
|
|
||||||
FirOperation.IS -> reporter.reportOn(expression.source, FirErrors.USELESS_IS_CHECK, true, context)
|
|
||||||
FirOperation.NOT_IS -> reporter.reportOn(expression.source, FirErrors.USELESS_IS_CHECK, false, context)
|
|
||||||
FirOperation.AS, FirOperation.SAFE_AS -> {
|
|
||||||
if (!expression.argFromStubType) {
|
|
||||||
reporter.reportOn(expression.source, FirErrors.USELESS_CAST, context)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
else -> throw AssertionError("Should not be here: ${expression.operation}")
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
+4
-4
@@ -1,11 +1,11 @@
|
|||||||
// !DIAGNOSTICS: -REDUNDANT_NULLABLE
|
// !DIAGNOSTICS: -REDUNDANT_NULLABLE
|
||||||
|
|
||||||
fun test(d: Any, dl: Collection<dynamic>) {
|
fun test(d: Any, dl: Collection<dynamic>) {
|
||||||
d <!USELESS_CAST!>as <!DYNAMIC_NOT_ALLOWED!>dynamic<!><!>
|
d as <!DYNAMIC_NOT_ALLOWED!>dynamic<!>
|
||||||
d <!USELESS_CAST!>as <!DYNAMIC_NOT_ALLOWED!>dynamic?<!><!>
|
d as <!DYNAMIC_NOT_ALLOWED!>dynamic?<!>
|
||||||
|
|
||||||
d <!USELESS_CAST!>as? <!DYNAMIC_NOT_ALLOWED!>dynamic<!><!>
|
d as? <!DYNAMIC_NOT_ALLOWED!>dynamic<!>
|
||||||
d <!USELESS_CAST!>as? <!DYNAMIC_NOT_ALLOWED!>dynamic?<!><!>
|
d as? <!DYNAMIC_NOT_ALLOWED!>dynamic?<!>
|
||||||
|
|
||||||
<!USELESS_IS_CHECK!>d is <!DYNAMIC_NOT_ALLOWED!>dynamic<!><!>
|
<!USELESS_IS_CHECK!>d is <!DYNAMIC_NOT_ALLOWED!>dynamic<!><!>
|
||||||
<!USELESS_IS_CHECK!>d is <!DYNAMIC_NOT_ALLOWED!>dynamic?<!><!>
|
<!USELESS_IS_CHECK!>d is <!DYNAMIC_NOT_ALLOWED!>dynamic?<!><!>
|
||||||
|
|||||||
Reference in New Issue
Block a user