[FIR] Fix missing USELESS_CAST

`FirUselessTypeOperationCallChecker` always checks exact types matching for `as` operator

Simplify code of cast checker and utils

^KT-56629 Fixed
^KT-56615 Fixed
^KT-59820 Fixed
This commit is contained in:
Ivan Kochurkin
2023-10-19 20:35:03 +02:00
committed by Space Team
parent 2f8026f335
commit d50c6f1b6d
21 changed files with 209 additions and 121 deletions
@@ -5556,6 +5556,12 @@ public class DiagnosticCompilerTestFE10TestdataTestGenerated extends AbstractDia
runTest("compiler/testData/diagnostics/tests/cast/kt47685.kt");
}
@Test
@TestMetadata("kt56629.kt")
public void testKt56629() throws Exception {
runTest("compiler/testData/diagnostics/tests/cast/kt56629.kt");
}
@Test
@TestMetadata("kt614.kt")
public void testKt614() throws Exception {
@@ -5556,6 +5556,12 @@ public class LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated
runTest("compiler/testData/diagnostics/tests/cast/kt47685.kt");
}
@Test
@TestMetadata("kt56629.kt")
public void testKt56629() throws Exception {
runTest("compiler/testData/diagnostics/tests/cast/kt56629.kt");
}
@Test
@TestMetadata("kt614.kt")
public void testKt614() throws Exception {
@@ -2,7 +2,7 @@
fun test_1(a: Any?) {
(a as String?)!!
(a as? String)!!
(a <!USELESS_CAST!>as? String<!>)!!
}
fun test_2(a: Any?) {
@@ -14,7 +14,7 @@ interface FirTarget<E : FirTargetElement> {
fun foo(target: FirTarget<FirFunction<*>>, property: FirProperty) {
val functionTarget = target.labeledElement
val x = (functionTarget as? FirFunction)?.let {
val x = (functionTarget <!USELESS_CAST!>as? FirFunction<!>)?.let {
if (property.getter === functionTarget) {
return@let 1
}
@@ -5550,6 +5550,12 @@ public class FirLightTreeOldFrontendDiagnosticsTestGenerated extends AbstractFir
runTest("compiler/testData/diagnostics/tests/cast/kt47685.kt");
}
@Test
@TestMetadata("kt56629.kt")
public void testKt56629() throws Exception {
runTest("compiler/testData/diagnostics/tests/cast/kt56629.kt");
}
@Test
@TestMetadata("kt614.kt")
public void testKt614() throws Exception {
@@ -5556,6 +5556,12 @@ public class FirPsiOldFrontendDiagnosticsTestGenerated extends AbstractFirPsiDia
runTest("compiler/testData/diagnostics/tests/cast/kt47685.kt");
}
@Test
@TestMetadata("kt56629.kt")
public void testKt56629() throws Exception {
runTest("compiler/testData/diagnostics/tests/cast/kt56629.kt");
}
@Test
@TestMetadata("kt614.kt")
public void testKt614() throws Exception {
@@ -8,11 +8,9 @@ package org.jetbrains.kotlin.fir.analysis.checkers
import org.jetbrains.kotlin.fir.FirSession
import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext
import org.jetbrains.kotlin.fir.declarations.utils.isInterface
import org.jetbrains.kotlin.fir.expressions.*
import org.jetbrains.kotlin.fir.resolve.defaultType
import org.jetbrains.kotlin.fir.resolve.substitution.ConeSubstitutorByMap
import org.jetbrains.kotlin.fir.scopes.platformClassMapper
import org.jetbrains.kotlin.fir.symbols.impl.FirFunctionSymbol
import org.jetbrains.kotlin.fir.symbols.impl.FirRegularClassSymbol
import org.jetbrains.kotlin.fir.symbols.impl.FirTypeParameterSymbol
import org.jetbrains.kotlin.fir.types.*
@@ -254,46 +252,7 @@ fun ConeKotlinType.isNonReifiedTypeParameter(): Boolean {
return this is ConeTypeParameterType && !this.lookupTag.typeParameterSymbol.isReified
}
@Suppress("UNUSED_PARAMETER")
fun shouldCheckForExactType(expression: FirTypeOperatorCall, context: CheckerContext): Boolean {
return when (expression.operation) {
FirOperation.IS, FirOperation.NOT_IS -> false
// TODO, KT-59820: differentiate if this expression defines the enclosing thing's type
// e.g.,
// val c1 get() = 1 as Number
// val c2: Number get() = 1 <!USELESS_CAST!>as Number<!>
FirOperation.AS, FirOperation.SAFE_AS -> true
else -> throw AssertionError("Should not be here: ${expression.operation}")
}
}
fun isRefinementUseless(
context: CheckerContext,
candidateType: ConeSimpleKotlinType,
targetType: ConeKotlinType,
shouldCheckForExactType: Boolean,
arg: FirExpression,
): Boolean {
return if (shouldCheckForExactType) {
if (arg is FirFunctionCall) {
val functionSymbol = arg.toResolvedCallableSymbol() as? FirFunctionSymbol<*>
if (functionSymbol != null && functionSymbol.isFunctionForExpectTypeFromCastFeature()) return false
}
isExactTypeCast(context, candidateType, targetType)
} else {
isUpcast(context, candidateType, targetType)
}
}
private fun isExactTypeCast(context: CheckerContext, candidateType: ConeSimpleKotlinType, targetType: ConeKotlinType): Boolean {
if (!AbstractTypeChecker.equalTypes(context.session.typeContext, candidateType, targetType, stubTypesEqualToAnything = false))
return false
// See comments at [isUpcast] why we need to check the existence of @ExtensionFunctionType
return candidateType.isExtensionFunctionType == targetType.isExtensionFunctionType
}
private fun isUpcast(context: CheckerContext, candidateType: ConeKotlinType, targetType: ConeKotlinType): Boolean {
fun isUpcast(context: CheckerContext, candidateType: ConeKotlinType, targetType: ConeKotlinType): Boolean {
if (!AbstractTypeChecker.isSubtypeOf(context.session.typeContext, candidateType, targetType, stubTypesEqualToAnything = false))
return false
@@ -8,35 +8,27 @@ 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.context.CheckerContext
import org.jetbrains.kotlin.fir.analysis.checkers.isRefinementUseless
import org.jetbrains.kotlin.fir.analysis.checkers.shouldCheckForExactType
import org.jetbrains.kotlin.fir.analysis.checkers.isFunctionForExpectTypeFromCastFeature
import org.jetbrains.kotlin.fir.analysis.checkers.isUpcast
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.expressions.*
import org.jetbrains.kotlin.fir.resolve.fullyExpandedType
import org.jetbrains.kotlin.fir.symbols.impl.FirFunctionSymbol
import org.jetbrains.kotlin.fir.types.*
import org.jetbrains.kotlin.types.AbstractTypeChecker
// See .../types/CastDiagnosticsUtil.kt for counterparts, including isRefinementUseless, isExactTypeCast, isUpcast.
object FirUselessTypeOperationCallChecker : FirTypeOperatorCallChecker() {
override fun check(expression: FirTypeOperatorCall, context: CheckerContext, reporter: DiagnosticReporter) {
if (expression.operation !in FirOperation.TYPES) return
val arg = expression.argument
val candidateType = arg.resolvedType.upperBoundIfFlexible().fullyExpandedType(context.session)
if (candidateType is ConeErrorType) return
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
// x as? Type <=> x as Type?
val refinedTargetType =
if (expression.operation == FirOperation.SAFE_AS) {
targetType.withNullability(ConeNullability.NULLABLE, context.session.typeContext)
} else {
targetType
}
if (isRefinementUseless(context, candidateType, refinedTargetType, shouldCheckForExactType(expression, context), arg)) {
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)
@@ -49,4 +41,42 @@ object FirUselessTypeOperationCallChecker : FirTypeOperatorCallChecker() {
}
}
}
private fun isRefinementUseless(
context: CheckerContext,
lhsType: ConeSimpleKotlinType,
targetType: ConeKotlinType,
expression: FirTypeOperatorCall,
arg: FirExpression,
): Boolean {
return when (expression.operation) {
FirOperation.AS, FirOperation.SAFE_AS -> {
if (arg is FirFunctionCall) {
val functionSymbol = arg.toResolvedCallableSymbol() as? FirFunctionSymbol<*>
if (functionSymbol != null && functionSymbol.isFunctionForExpectTypeFromCastFeature()) return false
}
// Normalize `targetType` for cases like the following:
// fun f(x: Int?) { x as? Int } // USELESS_CAST is reasonable here
val refinedTargetType =
if (expression.operation == FirOperation.SAFE_AS && lhsType.isNullable) {
targetType.withNullability(ConeNullability.NULLABLE, context.session.typeContext)
} else {
targetType
}
isExactTypeCast(context, lhsType, refinedTargetType)
}
FirOperation.IS, FirOperation.NOT_IS -> {
isUpcast(context, lhsType, targetType)
}
else -> throw AssertionError("Should not be here: ${expression.operation}")
}
}
private fun isExactTypeCast(context: CheckerContext, lhsType: ConeSimpleKotlinType, targetType: ConeKotlinType): Boolean {
if (!AbstractTypeChecker.equalTypes(context.session.typeContext, lhsType, targetType, stubTypesEqualToAnything = false))
return false
// See comments at [isUpcast] why we need to check the existence of @ExtensionFunctionType
return lhsType.isExtensionFunctionType == targetType.isExtensionFunctionType
}
}
+2 -2
View File
@@ -10,8 +10,8 @@ fun test() : Unit {
checkSubtype<Int>(y <!USELESS_CAST!>as Int<!>)
checkSubtype<Int?>(x as Int?)
checkSubtype<Int?>(y as Int?)
checkSubtype<Int?>(x as? Int)
checkSubtype<Int?>(y as? Int)
checkSubtype<Int?>(x <!USELESS_CAST!>as? Int<!>)
checkSubtype<Int?>(y <!USELESS_CAST!>as? Int<!>)
checkSubtype<Int?>(x as? Int?)
checkSubtype<Int?>(y as? Int?)
@@ -15,7 +15,7 @@ fun test(x: Int?) {
foo(x as? Number)
if (x is Int) {
val b = x as? Int
val b = x <!USELESS_CAST!>as? Int<!>
}
}
@@ -1,51 +0,0 @@
fun asCall() {
1 <!USELESS_CAST!>as Int<!>
1 <!CAST_NEVER_SUCCEEDS!>as<!> Byte
1 <!CAST_NEVER_SUCCEEDS!>as<!> Short
1 <!CAST_NEVER_SUCCEEDS!>as<!> Long
1 <!CAST_NEVER_SUCCEEDS!>as<!> Char
1 <!CAST_NEVER_SUCCEEDS!>as<!> Double
1 <!CAST_NEVER_SUCCEEDS!>as<!> Float
1.0 <!CAST_NEVER_SUCCEEDS!>as<!> Int
1.0 <!CAST_NEVER_SUCCEEDS!>as<!> Byte
1.0 <!CAST_NEVER_SUCCEEDS!>as<!> Short
1.0 <!CAST_NEVER_SUCCEEDS!>as<!> Long
1.0 <!CAST_NEVER_SUCCEEDS!>as<!> Char
1.0 <!USELESS_CAST!>as Double<!>
1.0 <!CAST_NEVER_SUCCEEDS!>as<!> Float
1f <!CAST_NEVER_SUCCEEDS!>as<!> Int
1f <!CAST_NEVER_SUCCEEDS!>as<!> Byte
1f <!CAST_NEVER_SUCCEEDS!>as<!> Short
1f <!CAST_NEVER_SUCCEEDS!>as<!> Long
1f <!CAST_NEVER_SUCCEEDS!>as<!> Char
1f <!CAST_NEVER_SUCCEEDS!>as<!> Double
1f <!USELESS_CAST!>as Float<!>
}
fun asSafe() {
1 as? Int
1 <!CAST_NEVER_SUCCEEDS!>as?<!> Byte
1 <!CAST_NEVER_SUCCEEDS!>as?<!> Short
1 <!CAST_NEVER_SUCCEEDS!>as?<!> Long
1 <!CAST_NEVER_SUCCEEDS!>as?<!> Char
1 <!CAST_NEVER_SUCCEEDS!>as?<!> Double
1 <!CAST_NEVER_SUCCEEDS!>as?<!> Float
1.0 <!CAST_NEVER_SUCCEEDS!>as?<!> Int
1.0 <!CAST_NEVER_SUCCEEDS!>as?<!> Byte
1.0 <!CAST_NEVER_SUCCEEDS!>as?<!> Short
1.0 <!CAST_NEVER_SUCCEEDS!>as?<!> Long
1.0 <!CAST_NEVER_SUCCEEDS!>as?<!> Char
1.0 as? Double
1.0 <!CAST_NEVER_SUCCEEDS!>as?<!> Float
1f <!CAST_NEVER_SUCCEEDS!>as?<!> Int
1f <!CAST_NEVER_SUCCEEDS!>as?<!> Byte
1f <!CAST_NEVER_SUCCEEDS!>as?<!> Short
1f <!CAST_NEVER_SUCCEEDS!>as?<!> Long
1f <!CAST_NEVER_SUCCEEDS!>as?<!> Char
1f <!CAST_NEVER_SUCCEEDS!>as?<!> Double
1f as? Float
}
+1
View File
@@ -1,3 +1,4 @@
// FIR_IDENTICAL
fun asCall() {
1 <!USELESS_CAST!>as Int<!>
1 <!CAST_NEVER_SUCCEEDS!>as<!> Byte
+36
View File
@@ -0,0 +1,36 @@
// ISSUE: KT-56629, KT-56615
class Klass
fun foo(arg: Klass) {
arg <!USELESS_CAST!>as Klass<!>
arg <!USELESS_CAST!>as? Klass<!>
}
fun test_1(a: Any?) {
(a as String?)!!
a.length
(a <!USELESS_CAST!>as? String<!>)!!
a.length
}
fun test_3(a: Any?) {
a as String
a <!USELESS_CAST!>as String<!>
}
fun test_4(a: Any?) {
a as String
a <!USELESS_CAST!>as? String<!>
}
fun test_5(a: Any?) {
(a as? String)!!
a.length
(a <!USELESS_CAST!>as String<!>)
}
fun test_6(a: Any?) {
(a as? String)!!
a.length
(a <!USELESS_CAST!>as? String<!>)
}
+36
View File
@@ -0,0 +1,36 @@
// ISSUE: KT-56629, KT-56615
class Klass
fun foo(arg: Klass) {
arg <!USELESS_CAST!>as Klass<!>
arg <!USELESS_CAST!>as? Klass<!>
}
fun test_1(a: Any?) {
(a as String?)!!
a<!UNSAFE_CALL!>.<!>length
(a <!USELESS_CAST!>as? String<!>)!!
<!DEBUG_INFO_SMARTCAST!>a<!>.length
}
fun test_3(a: Any?) {
a as String
a <!USELESS_CAST!>as String<!>
}
fun test_4(a: Any?) {
a as String
a <!USELESS_CAST!>as? String<!>
}
fun test_5(a: Any?) {
(a as? String)!!
<!DEBUG_INFO_SMARTCAST!>a<!>.length
(a <!USELESS_CAST!>as String<!>)
}
fun test_6(a: Any?) {
(a as? String)!!
<!DEBUG_INFO_SMARTCAST!>a<!>.length
(a <!USELESS_CAST!>as? String<!>)
}
@@ -0,0 +1,48 @@
// !DIAGNOSTICS: -UNUSED_PARAMETER, -SENSELESS_COMPARISON, -DEBUG_INFO_SMARTCAST
fun <T: Any?> test1(t: Any?): Any {
return t <!UNCHECKED_CAST!>as T<!> ?: ""
}
fun <T: Any> test2(t: Any?): Any {
return t <!UNCHECKED_CAST!>as T<!> <!USELESS_ELVIS!>?: ""<!>
}
fun <T: Any?> test3(t: Any?): Any {
if (t != null) {
return t <!USELESS_ELVIS!>?: ""<!>
}
return 1
}
fun takeNotNull(s: String) {}
fun <T> notNull(): T = TODO()
fun <T> nullable(): T? = null
fun <T> dependOn(x: T) = x
fun test() {
takeNotNull(notNull() ?: "")
takeNotNull(nullable() ?: "")
val x: String? = null
takeNotNull(dependOn(x) ?: "")
takeNotNull(dependOn(dependOn(x)) ?: "")
takeNotNull(dependOn(dependOn(x as String)) <!USELESS_ELVIS!>?: ""<!>)
if (x != null) {
takeNotNull(dependOn(x) <!USELESS_ELVIS!>?: ""<!>)
takeNotNull(dependOn(dependOn(x)) <!USELESS_ELVIS!>?: ""<!>)
takeNotNull(dependOn(dependOn(x) <!USELESS_CAST!>as? String<!>) ?: "")
}
takeNotNull(bar()!!)
}
inline fun <reified T : Any> reifiedNull(): T? = null
fun testFrom13648() {
takeNotNull(reifiedNull() ?: "")
}
fun bar() = <!UNRESOLVED_REFERENCE!>unresolved<!>
@@ -1,4 +1,3 @@
// FIR_IDENTICAL
// !DIAGNOSTICS: -UNUSED_PARAMETER, -SENSELESS_COMPARISON, -DEBUG_INFO_SMARTCAST
fun <T: Any?> test1(t: Any?): Any {
@@ -13,7 +13,7 @@ fun g(a: SomeClass?) {
// 'a' can be cast to SomeSubClass
a.hashCode()
a.foo
(a as? SomeSubClass)<!UNSAFE_CALL!>.<!>foo
(a <!USELESS_CAST!>as? SomeSubClass<!>)<!UNSAFE_CALL!>.<!>foo
(a <!USELESS_CAST!>as SomeSubClass<!>).foo
}
val b = (a as? SomeSubClass)?.foo
@@ -21,7 +21,7 @@ fun g(a: SomeClass?) {
// 'a' can be cast to SomeSubClass
a.hashCode()
a.foo
(a as? SomeSubClass)<!UNSAFE_CALL!>.<!>foo
(a <!USELESS_CAST!>as? SomeSubClass<!>)<!UNSAFE_CALL!>.<!>foo
(a <!USELESS_CAST!>as SomeSubClass<!>).foo
}
val c = a as? SomeSubClass
@@ -29,7 +29,7 @@ fun g(a: SomeClass?) {
// 'a' and 'c' can be cast to SomeSubClass
a.hashCode()
a.foo
(a as? SomeSubClass)<!UNSAFE_CALL!>.<!>foo
(a <!USELESS_CAST!>as? SomeSubClass<!>)<!UNSAFE_CALL!>.<!>foo
c.hashCode()
c.foo
}
@@ -60,7 +60,7 @@ fun f(a: SomeClass?) {
// 'c' can be cast to SomeSubClass
aa.hashCode()
aa.foo
(aa as? SomeSubClass)<!UNSAFE_CALL!>.<!>foo
(aa <!USELESS_CAST!>as? SomeSubClass<!>)<!UNSAFE_CALL!>.<!>foo
c.hashCode()
c.foo
}
@@ -5556,6 +5556,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest {
runTest("compiler/testData/diagnostics/tests/cast/kt47685.kt");
}
@Test
@TestMetadata("kt56629.kt")
public void testKt56629() throws Exception {
runTest("compiler/testData/diagnostics/tests/cast/kt56629.kt");
}
@Test
@TestMetadata("kt614.kt")
public void testKt614() throws Exception {
@@ -111,7 +111,7 @@ fun case_9(value_1: Any) {
fun case_10(value_1: Collection<Int>, value_2: Collection<Int>, value_3: Collection<Int>?) {
when (value_1) {
value_2 as List<Int> -> {}
value_2 as? List<Int> -> {}
value_2 <!USELESS_CAST!>as? List<Int><!> -> {}
value_3 <!UNCHECKED_CAST!>as? MutableMap<Int, Int><!> -> {}
(value_2 <!UNCHECKED_CAST!>as? Map<Int, Int><!>) as MutableMap<Int, Int> -> {}
}
@@ -82,7 +82,7 @@ fun case_9(value_1: Any) {
// TESTCASE NUMBER: 10
fun case_10(value_1: Collection<Int>, value_2: Collection<Int>, value_3: Collection<Int>?) {
when (value_1) {
value_2 as List<Int>, value_2 as? List<Int> -> {}
value_2 as List<Int>, value_2 <!USELESS_CAST!>as? List<Int><!> -> {}
value_3 <!UNCHECKED_CAST!>as? MutableMap<Int, Int><!>, (value_2 <!UNCHECKED_CAST!>as? Map<Int, Int><!>) as MutableMap<Int, Int> -> {}
}
}