FIR checker: warn unnecessary safe calls

This commit is contained in:
Jinseong Jeon
2021-04-01 11:55:05 -07:00
committed by Mikhail Glukhikh
parent 2ecb6733ed
commit 5a0b75bd89
65 changed files with 232 additions and 349 deletions
@@ -16,7 +16,7 @@ fun test(ordinal: Int) {
buildString {
insert(KDocTemplate()) {
definition {
ordinal?.let {}
ordinal<!UNNECESSARY_SAFE_CALL!>?.<!>let {}
}
}
}
@@ -69,8 +69,8 @@ fun test_6(d1: D) {
}
fun test_7(d1: D, d2: D) {
val a = d1?.any
val b = d2?.any
val a = d1<!UNNECESSARY_SAFE_CALL!>?.<!>any
val b = d2<!UNNECESSARY_SAFE_CALL!>?.<!>any
a as A
a.foo() // should be OK
b as B
@@ -51,6 +51,7 @@ enum class PositioningStrategy(private val strategy: String? = null) {
COMPANION_OBJECT,
CONST_MODIFIER,
ARRAY_ACCESS,
SAFE_ACCESS,
NAME_OF_NAMED_ARGUMENT,
VALUE_ARGUMENTS,
SUPERTYPES_LIST,
@@ -299,11 +299,11 @@ object DIAGNOSTICS_LIST : DiagnosticList() {
}
val UPPER_BOUND_IS_EXTENSION_FUNCTION_TYPE by error<FirSourceElement, KtTypeReference>()
val BOUNDS_NOT_ALLOWED_IF_BOUNDED_BY_TYPE_PARAMETER by error<FirSourceElement, KtElement>()
val ONLY_ONE_CLASS_BOUND_ALLOWED by error<FirSourceElement, KtTypeReference>()
val REPEATED_BOUND by error<FirSourceElement, KtTypeReference>()
val CONFLICTING_UPPER_BOUNDS by error<FirSourceElement, KtNamedDeclaration> {
@@ -559,7 +559,10 @@ object DIAGNOSTICS_LIST : DiagnosticList() {
parameter<String>("operator")
parameter<FirExpression>("rhs")
}
// TODO: val UNEXPECTED_SAFE_CALL by ...
val UNNECESSARY_SAFE_CALL by warning<FirSourceElement, PsiElement>(PositioningStrategy.SAFE_ACCESS) {
parameter<ConeKotlinType>("receiverType")
}
val UNEXPECTED_SAFE_CALL by error<FirSourceElement, PsiElement>(PositioningStrategy.SAFE_ACCESS)
val UNNECESSARY_NOT_NULL_ASSERTION by warning<FirSourceElement, KtExpression>(PositioningStrategy.OPERATOR) {
parameter<ConeKotlinType>("receiverType")
}
@@ -331,6 +331,8 @@ object FirErrors {
val UNSAFE_IMPLICIT_INVOKE_CALL by error1<FirSourceElement, PsiElement, ConeKotlinType>(SourceElementPositioningStrategies.REFERENCE_BY_QUALIFIED)
val UNSAFE_INFIX_CALL by error3<FirSourceElement, KtExpression, FirExpression, String, FirExpression>(SourceElementPositioningStrategies.REFERENCE_BY_QUALIFIED)
val UNSAFE_OPERATOR_CALL by error3<FirSourceElement, KtExpression, FirExpression, String, FirExpression>(SourceElementPositioningStrategies.REFERENCE_BY_QUALIFIED)
val UNNECESSARY_SAFE_CALL by warning1<FirSourceElement, PsiElement, ConeKotlinType>(SourceElementPositioningStrategies.SAFE_ACCESS)
val UNEXPECTED_SAFE_CALL by error0<FirSourceElement, PsiElement>(SourceElementPositioningStrategies.SAFE_ACCESS)
val UNNECESSARY_NOT_NULL_ASSERTION by warning1<FirSourceElement, KtExpression, ConeKotlinType>(SourceElementPositioningStrategies.OPERATOR)
val NOT_NULL_ASSERTION_ON_LAMBDA_EXPRESSION by warning0<FirSourceElement, KtExpression>(SourceElementPositioningStrategies.OPERATOR)
val NOT_NULL_ASSERTION_ON_CALLABLE_REFERENCE by warning0<FirSourceElement, KtExpression>(SourceElementPositioningStrategies.OPERATOR)
@@ -0,0 +1,32 @@
/*
* 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.fir.analysis.checkers.context.CheckerContext
import org.jetbrains.kotlin.fir.analysis.diagnostics.DiagnosticReporter
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors
import org.jetbrains.kotlin.fir.analysis.diagnostics.reportOn
import org.jetbrains.kotlin.fir.expressions.FirSafeCallExpression
import org.jetbrains.kotlin.fir.expressions.FirStatement
import org.jetbrains.kotlin.fir.resolve.fullyExpandedType
import org.jetbrains.kotlin.fir.types.canBeNull
import org.jetbrains.kotlin.fir.types.coneType
import org.jetbrains.kotlin.fir.types.isUnit
object FirSafeCallExpressionChecker : FirBasicExpressionChecker() {
override fun check(expression: FirStatement, context: CheckerContext, reporter: DiagnosticReporter) {
if (expression !is FirSafeCallExpression) return
val receiverType = expression.receiver.typeRef.coneType.fullyExpandedType(context.session)
if (receiverType.isUnit) {
reporter.reportOn(expression.source, FirErrors.UNEXPECTED_SAFE_CALL, context)
return
}
if (!receiverType.canBeNull) {
reporter.reportOn(expression.source, FirErrors.UNNECESSARY_SAFE_CALL, receiverType, context)
}
}
}
@@ -77,6 +77,10 @@ class ExpressionCheckersDiagnosticComponent(
checkers.allBasicExpressionCheckers.check(checkNotNullCall, data, reporter)
}
override fun visitSafeCallExpression(safeCallExpression: FirSafeCallExpression, data: CheckerContext) {
checkers.basicExpressionCheckers.check(safeCallExpression, data, reporter)
}
override fun visitTryExpression(tryExpression: FirTryExpression, data: CheckerContext) {
checkers.allTryExpressionCheckers.check(tryExpression, data, reporter)
}
@@ -217,9 +217,11 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.TYPE_PARAMETER_AS
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.TYPE_PARAMETER_IN_CATCH_CLAUSE
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.TYPE_PARAMETER_IS_NOT_AN_EXPRESSION
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.TYPE_PARAMETER_ON_LHS_OF_DOT
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.UNEXPECTED_SAFE_CALL
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.UNINITIALIZED_VARIABLE
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.UNNECESSARY_LATEINIT
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.UNNECESSARY_NOT_NULL_ASSERTION
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.UNNECESSARY_SAFE_CALL
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.UNRESOLVED_LABEL
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.UNRESOLVED_REFERENCE
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.UNSAFE_CALL
@@ -751,6 +753,8 @@ class FirDefaultErrorMessages : DefaultErrorMessages.Extension {
map.put(UNNECESSARY_NOT_NULL_ASSERTION, "Unnecessary non-null assertion (!!) on a non-null receiver of type {0}", RENDER_TYPE)
map.put(NOT_NULL_ASSERTION_ON_LAMBDA_EXPRESSION, "Non-null assertion (!!) is called on a lambda expression")
map.put(NOT_NULL_ASSERTION_ON_CALLABLE_REFERENCE, "Non-null assertion (!!) is called on a callable reference expression")
map.put(UNNECESSARY_SAFE_CALL, "Unnecessary safe call on a non-null receiver of type {0}", RENDER_TYPE)
map.put(UNEXPECTED_SAFE_CALL, "Safe-call is not allowed here")
// When expressions
map.put(NO_ELSE_IN_WHEN, "''when'' expression must be exhaustive, add necessary {0}", WHEN_MISSING_CASES)
@@ -497,6 +497,17 @@ object LightTreePositioningStrategies {
}
}
val SAFE_ACCESS = object : LightTreePositioningStrategy() {
override fun mark(
node: LighterASTNode,
startOffset: Int,
endOffset: Int,
tree: FlyweightCapableTreeStructure<LighterASTNode>
): List<TextRange> {
return markElement(tree.safeAccess(node) ?: node, startOffset, endOffset, tree, node)
}
}
val RETURN_WITH_LABEL = object : LightTreePositioningStrategy() {
override fun mark(
node: LighterASTNode,
@@ -531,6 +542,9 @@ private fun FlyweightCapableTreeStructure<LighterASTNode>.constructorKeyword(nod
private fun FlyweightCapableTreeStructure<LighterASTNode>.dotOperator(node: LighterASTNode): LighterASTNode? =
findChildByType(node, KtTokens.DOT)
private fun FlyweightCapableTreeStructure<LighterASTNode>.safeAccess(node: LighterASTNode): LighterASTNode? =
findChildByType(node, KtTokens.SAFE_ACCESS)
private fun FlyweightCapableTreeStructure<LighterASTNode>.initKeyword(node: LighterASTNode): LighterASTNode? =
findChildByType(node, KtTokens.INIT_KEYWORD)
@@ -168,6 +168,11 @@ object SourceElementPositioningStrategies {
PositioningStrategies.ARRAY_ACCESS
)
val SAFE_ACCESS = SourceElementPositioningStrategy(
LightTreePositioningStrategies.SAFE_ACCESS,
PositioningStrategies.SAFE_ACCESS
)
val RETURN_WITH_LABEL = SourceElementPositioningStrategy(
LightTreePositioningStrategies.RETURN_WITH_LABEL,
PositioningStrategies.RETURN_WITH_LABEL
@@ -12,6 +12,7 @@ object CommonExpressionCheckers : ExpressionCheckers() {
FirAnonymousFunctionChecker,
FirCheckNotNullCallChecker,
FirGetClassCallChecker,
FirSafeCallExpressionChecker,
)
override val qualifiedAccessCheckers: Set<FirQualifiedAccessChecker> = setOf(
@@ -490,7 +490,10 @@ class ExpressionsConverter(
(firSelector as? FirQualifiedAccess)?.let {
if (isSafe) {
return it.wrapWithSafeCall(firReceiver!!)
return it.wrapWithSafeCall(
firReceiver!!,
dotQualifiedExpression.toFirSourceElement(FirFakeSourceElementKind.DesugaredSafeCallExpression)
)
}
it.replaceExplicitReceiver(firReceiver)
@@ -1988,7 +1988,10 @@ open class RawFirBuilder(
val receiver = expression.receiverExpression.toFirExpression("Incorrect receiver expression")
if (expression is KtSafeQualifiedExpression) {
return firSelector.wrapWithSafeCall(receiver)
return firSelector.wrapWithSafeCall(
receiver,
expression.toFirSourceElement(FirFakeSourceElementKind.DesugaredSafeCallExpression)
)
}
firSelector.replaceExplicitReceiver(receiver)
@@ -37,6 +37,7 @@ import org.jetbrains.kotlin.fir.types.builder.*
import org.jetbrains.kotlin.fir.types.impl.*
import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.name.Name
import org.jetbrains.kotlin.psi.KtQualifiedExpression
import org.jetbrains.kotlin.types.ConstantValueKind
import org.jetbrains.kotlin.types.expressions.OperatorConventions
import org.jetbrains.kotlin.util.OperatorNameConventions
@@ -513,8 +514,7 @@ private fun FirExpression.checkReceiver(name: String?): Boolean {
return receiverName == name
}
fun FirQualifiedAccess.wrapWithSafeCall(receiver: FirExpression): FirSafeCallExpression {
fun FirQualifiedAccess.wrapWithSafeCall(receiver: FirExpression, source: FirSourceElement): FirSafeCallExpression {
val checkedSafeCallSubject = buildCheckedSafeCallSubject {
@OptIn(FirContractViolation::class)
this.originalReceiverRef = FirExpressionRef<FirExpression>().apply {
@@ -531,7 +531,7 @@ fun FirQualifiedAccess.wrapWithSafeCall(receiver: FirExpression): FirSafeCallExp
bind(checkedSafeCallSubject)
}
this.regularQualifiedAccess = this@wrapWithSafeCall
this.source = this@wrapWithSafeCall.source?.fakeElement(FirFakeSourceElementKind.DesugaredSafeCallExpression)
this.source = source
}
}