FIR checker: SENSELESS_(COMPARISON|NULL_IN_WHEN)
Currently DFA does not set "definitely equal to null" for access to variables that got assigned `null`. For example, FIR should mark the following line as SENSELESS_COMPARISON due to `s = null` above. https://github.com/JetBrains/kotlin/blob/d1531f9cdd5852352c0133198706125dc63b6007/compiler/testData/diagnostics/tests/smartCasts/alwaysNull.fir.kt#L6 The problem is at https://github.com/JetBrains/kotlin/blob/7e9f27436a77de1c76e3705da7aa1fbe8938336b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirDataFlowAnalyzer.kt#L1104 For null assignment, ideally the type should be `Nothing?`. This is addressed in a followup commit instead.
This commit is contained in:
committed by
teamcityserver
parent
4726dcce40
commit
c7272f6986
+1
-1
@@ -137,7 +137,7 @@ fun main(args: Array<String?>) {
|
||||
<!ASSIGNED_VALUE_IS_NEVER_READ!>a<!> = args[0]
|
||||
} else {
|
||||
a = args.toString()
|
||||
if (a != null && a.equals("cde")) return
|
||||
if (<!SENSELESS_COMPARISON!>a != null<!> && a.equals("cde")) return
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
compiler/fir/analysis-tests/testData/resolve/smartcasts/boundSmartcasts/boundSmartcastsInBranches.kt
Vendored
+1
-1
@@ -107,7 +107,7 @@ fun test_7() {
|
||||
y<!UNSAFE_CALL!>.<!>length // Bad
|
||||
z.length // OK
|
||||
}
|
||||
if (y != null) {
|
||||
if (<!SENSELESS_COMPARISON!>y != null<!>) {
|
||||
x<!UNSAFE_CALL!>.<!>length // Bad
|
||||
y.length // OK
|
||||
z<!UNSAFE_CALL!>.<!>length // Bad
|
||||
|
||||
+1
-1
@@ -14,7 +14,7 @@ var Any?.isNotNull: Boolean
|
||||
set(value) {
|
||||
contract {
|
||||
returns() implies (this@isNotNull != null)
|
||||
<!ERROR_IN_CONTRACT_DESCRIPTION!>require(this != null)<!>
|
||||
<!ERROR_IN_CONTRACT_DESCRIPTION!>require(<!SENSELESS_COMPARISON!>this != null<!>)<!>
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+5
-1
@@ -944,11 +944,15 @@ object DIAGNOSTICS_LIST : DiagnosticList("FirErrors") {
|
||||
val INITIALIZATION_BEFORE_DECLARATION by error<KtExpression>() {
|
||||
parameter<Symbol>("property")
|
||||
}
|
||||
|
||||
val UNREACHABLE_CODE by warning<KtElement>(PositioningStrategy.UNREACHABLE_CODE) {
|
||||
parameter<Set<FirSourceElement>>("reachable")
|
||||
parameter<Set<FirSourceElement>>("unreachable")
|
||||
}
|
||||
val SENSELESS_COMPARISON by warning<KtBinaryExpression> {
|
||||
parameter<FirExpression>("expression")
|
||||
parameter<Boolean>("compareResult")
|
||||
}
|
||||
val SENSELESS_NULL_IN_WHEN by warning<KtElement>()
|
||||
}
|
||||
|
||||
val NULLABILITY by object : DiagnosticGroup("Nullability") {
|
||||
|
||||
@@ -507,6 +507,8 @@ object FirErrors {
|
||||
val VARIABLE_WITH_NO_TYPE_NO_INITIALIZER by error0<KtVariableDeclaration>(SourceElementPositioningStrategies.DECLARATION_NAME)
|
||||
val INITIALIZATION_BEFORE_DECLARATION by error1<KtExpression, FirBasedSymbol<*>>()
|
||||
val UNREACHABLE_CODE by warning2<KtElement, Set<FirSourceElement>, Set<FirSourceElement>>(SourceElementPositioningStrategies.UNREACHABLE_CODE)
|
||||
val SENSELESS_COMPARISON by warning2<KtBinaryExpression, FirExpression, Boolean>()
|
||||
val SENSELESS_NULL_IN_WHEN by warning0<KtElement>()
|
||||
|
||||
// Nullability
|
||||
val UNSAFE_CALL by error2<PsiElement, ConeKotlinType, FirExpression?>(SourceElementPositioningStrategies.DOT_BY_QUALIFIED)
|
||||
|
||||
+50
-2
@@ -5,6 +5,7 @@
|
||||
|
||||
package org.jetbrains.kotlin.fir.analysis.checkers.expression
|
||||
|
||||
import org.jetbrains.kotlin.KtNodeTypes
|
||||
import org.jetbrains.kotlin.fir.FirRealSourceElementKind
|
||||
import org.jetbrains.kotlin.fir.analysis.checkers.ConeTypeCompatibilityChecker
|
||||
import org.jetbrains.kotlin.fir.analysis.checkers.ConeTypeCompatibilityChecker.isCompatible
|
||||
@@ -15,17 +16,34 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.reportOn
|
||||
import org.jetbrains.kotlin.fir.declarations.FirFile
|
||||
import org.jetbrains.kotlin.fir.declarations.utils.isEnumClass
|
||||
import org.jetbrains.kotlin.fir.expressions.FirEqualityOperatorCall
|
||||
import org.jetbrains.kotlin.fir.expressions.FirExpression
|
||||
import org.jetbrains.kotlin.fir.expressions.FirExpressionWithSmartcastToNull
|
||||
import org.jetbrains.kotlin.fir.expressions.FirOperation
|
||||
import org.jetbrains.kotlin.fir.render
|
||||
import org.jetbrains.kotlin.fir.resolve.inference.inferenceComponents
|
||||
import org.jetbrains.kotlin.fir.resolve.toFirRegularClass
|
||||
import org.jetbrains.kotlin.fir.typeContext
|
||||
import org.jetbrains.kotlin.fir.types.*
|
||||
|
||||
object FirEqualityCompatibilityChecker : FirEqualityOperatorCallChecker() {
|
||||
override fun check(expression: FirEqualityOperatorCall, context: CheckerContext, reporter: DiagnosticReporter) {
|
||||
val arguments = expression.argumentList.arguments
|
||||
if (arguments.size != 2) return
|
||||
val lType = arguments[0].typeRef.coneType
|
||||
val rType = arguments[1].typeRef.coneType
|
||||
val lExpr = arguments[0]
|
||||
val rExpr = arguments[1]
|
||||
checkCompatibility(lExpr, rExpr, context, expression, reporter)
|
||||
checkSensibleness(lExpr, rExpr, context, expression, reporter)
|
||||
}
|
||||
|
||||
private fun checkCompatibility(
|
||||
lExpr: FirExpression,
|
||||
rExpr: FirExpression,
|
||||
context: CheckerContext,
|
||||
expression: FirEqualityOperatorCall,
|
||||
reporter: DiagnosticReporter
|
||||
) {
|
||||
val lType = lExpr.typeRef.coneType
|
||||
val rType = rExpr.typeRef.coneType
|
||||
// If one of the type is already `Nothing?`, we skip reporting further comparison. This is to allow comparing with `null`, which has
|
||||
// type `Nothing?`
|
||||
if (lType.isNullableNothing || rType.isNullableNothing) return
|
||||
@@ -95,4 +113,34 @@ object FirEqualityCompatibilityChecker : FirEqualityOperatorCallChecker() {
|
||||
val firRegularClass = (this as? ConeClassLikeType)?.lookupTag?.toFirRegularClass(context.session) ?: return false
|
||||
return firRegularClass.isEnumClass
|
||||
}
|
||||
|
||||
private fun checkSensibleness(
|
||||
lExpr: FirExpression,
|
||||
rExpr: FirExpression,
|
||||
context: CheckerContext,
|
||||
expression: FirEqualityOperatorCall,
|
||||
reporter: DiagnosticReporter
|
||||
) {
|
||||
val expressionComparedWithNull = when {
|
||||
lExpr.isNullLiteral -> rExpr
|
||||
rExpr.isNullLiteral -> lExpr
|
||||
else -> return
|
||||
}
|
||||
val type = expressionComparedWithNull.typeRef.coneType
|
||||
if (type is ConeKotlinErrorType) return
|
||||
val isPositiveCompare = expression.operation == FirOperation.EQ || expression.operation == FirOperation.IDENTITY
|
||||
val compareResult = with(context.session.typeContext) {
|
||||
when {
|
||||
// `null` literal has type `Nothing?`
|
||||
type.isNullableNothing || (expressionComparedWithNull is FirExpressionWithSmartcastToNull && expressionComparedWithNull.isStable) -> isPositiveCompare
|
||||
!type.isNullableType() -> !isPositiveCompare
|
||||
else -> return
|
||||
}
|
||||
}
|
||||
if (expression.source?.elementType == KtNodeTypes.BINARY_EXPRESSION) {
|
||||
reporter.reportOn(expression.source, FirErrors.SENSELESS_COMPARISON, expression, compareResult, context)
|
||||
} else {
|
||||
reporter.reportOn(expression.source, FirErrors.SENSELESS_NULL_IN_WHEN, context)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+4
@@ -363,6 +363,8 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.SEALED_CLASS_CONS
|
||||
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.SEALED_SUPERTYPE
|
||||
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.SEALED_SUPERTYPE_IN_LOCAL_CLASS
|
||||
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.SECONDARY_CONSTRUCTOR_WITH_BODY_INSIDE_INLINE_CLASS
|
||||
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.SENSELESS_COMPARISON
|
||||
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.SENSELESS_NULL_IN_WHEN
|
||||
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.SETTER_PROJECTED_OUT
|
||||
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.SETTER_VISIBILITY_INCONSISTENT_WITH_PROPERTY_VISIBILITY
|
||||
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.SINGLETON_IN_SUPERTYPE
|
||||
@@ -1308,6 +1310,8 @@ class FirDefaultErrorMessages {
|
||||
map.put(LEAKED_IN_PLACE_LAMBDA, "Leaked in-place lambda: {2}", SYMBOL)
|
||||
map.put(FirErrors.WRONG_IMPLIES_CONDITION, "Wrong implies condition")
|
||||
map.put(UNREACHABLE_CODE, "Unreachable code", NOT_RENDERED, NOT_RENDERED)
|
||||
map.put(SENSELESS_COMPARISON, "Condition ''{0}'' is always ''{1}''", FIR, TO_STRING)
|
||||
map.put(SENSELESS_NULL_IN_WHEN, "Expression under 'when' is never equal to null")
|
||||
|
||||
// Nullability
|
||||
map.put(
|
||||
|
||||
+1
@@ -5,6 +5,7 @@
|
||||
|
||||
package org.jetbrains.kotlin.fir.analysis.diagnostics
|
||||
|
||||
import com.intellij.psi.PsiElement
|
||||
import org.jetbrains.kotlin.descriptors.ClassKind
|
||||
import org.jetbrains.kotlin.descriptors.Visibility
|
||||
import org.jetbrains.kotlin.diagnostics.WhenMissingCase
|
||||
|
||||
Reference in New Issue
Block a user