FIR checker: introduce NO_(GET|SET)_METHOD

Besides introducing new diagnostics, this commit unifies source usages
for array accesses in PSI & LT.
This commit is contained in:
Tianyu Geng
2021-03-11 16:01:58 -08:00
committed by Mikhail Glukhikh
parent 651fd4ad9f
commit e1c80ac75c
32 changed files with 137 additions and 100 deletions
@@ -32,5 +32,5 @@ fun test_3(a: A<D>) {
}
fun test_4(b: B) {
<!UNRESOLVED_REFERENCE!><!UNRESOLVED_REFERENCE!>b[0]<!> += B()<!> // unresolved
<!UNRESOLVED_REFERENCE!>b<!NO_GET_METHOD!>[0]<!> += B()<!> // unresolved
}
@@ -49,6 +49,7 @@ enum class PositioningStrategy(private val strategy: String? = null) {
PRIVATE_MODIFIER,
COMPANION_OBJECT,
CONST_MODIFIER,
ARRAY_ACCESS
;
@@ -515,6 +515,11 @@ object DIAGNOSTICS_LIST : DiagnosticList() {
}
}
val CONVENTIONS by object : DiagnosticGroup("Conventions") {
val NO_GET_METHOD by error<FirSourceElement, KtArrayAccessExpression>(PositioningStrategy.ARRAY_ACCESS)
val NO_SET_METHOD by error<FirSourceElement, KtArrayAccessExpression>(PositioningStrategy.ARRAY_ACCESS)
}
val EXTENDED_CHECKERS by object : DiagnosticGroup("Extended checkers") {
val REDUNDANT_VISIBILITY_MODIFIER by warning<FirSourceElement, KtModifierListOwner>(PositioningStrategy.VISIBILITY_MODIFIER)
val REDUNDANT_MODALITY_MODIFIER by warning<FirSourceElement, KtModifierListOwner>(PositioningStrategy.MODALITY_MODIFIER)
@@ -24,6 +24,7 @@ import org.jetbrains.kotlin.fir.symbols.impl.FirTypeParameterSymbol
import org.jetbrains.kotlin.fir.types.ConeKotlinType
import org.jetbrains.kotlin.lexer.KtModifierKeywordToken
import org.jetbrains.kotlin.name.Name
import org.jetbrains.kotlin.psi.KtArrayAccessExpression
import org.jetbrains.kotlin.psi.KtClassOrObject
import org.jetbrains.kotlin.psi.KtDeclaration
import org.jetbrains.kotlin.psi.KtDestructuringDeclaration
@@ -307,6 +308,10 @@ object FirErrors {
// Function contracts
val ERROR_IN_CONTRACT_DESCRIPTION by error1<FirSourceElement, KtElement, String>(SourceElementPositioningStrategies.SELECTOR_BY_QUALIFIED)
// Conventions
val NO_GET_METHOD by error0<FirSourceElement, KtArrayAccessExpression>(SourceElementPositioningStrategies.ARRAY_ACCESS)
val NO_SET_METHOD by error0<FirSourceElement, KtArrayAccessExpression>(SourceElementPositioningStrategies.ARRAY_ACCESS)
// Extended checkers
val REDUNDANT_VISIBILITY_MODIFIER by warning0<FirSourceElement, KtModifierListOwner>(SourceElementPositioningStrategies.VISIBILITY_MODIFIER)
val REDUNDANT_MODALITY_MODIFIER by warning0<FirSourceElement, KtModifierListOwner>(SourceElementPositioningStrategies.MODALITY_MODIFIER)
@@ -0,0 +1,30 @@
/*
* 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.FirFakeSourceElementKind
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.FirFunctionCall
import org.jetbrains.kotlin.fir.references.FirErrorNamedReference
import org.jetbrains.kotlin.fir.resolve.diagnostics.ConeUnresolvedNameError
import org.jetbrains.kotlin.util.OperatorNameConventions
object FirConventionFunctionCallChecker : FirFunctionCallChecker() {
override fun check(expression: FirFunctionCall, context: CheckerContext, reporter: DiagnosticReporter) {
val calleeReference = expression.calleeReference as? FirErrorNamedReference ?: return
val diagnostic = calleeReference.diagnostic as? ConeUnresolvedNameError ?: return
if (expression.calleeReference.source?.kind == FirFakeSourceElementKind.ArrayAccessNameReference) {
when (diagnostic.name) {
OperatorNameConventions.GET -> reporter.reportOn(calleeReference.source, FirErrors.NO_GET_METHOD, context)
OperatorNameConventions.SET -> reporter.reportOn(calleeReference.source, FirErrors.NO_SET_METHOD, context)
}
}
}
}
@@ -39,6 +39,10 @@ class ErrorNodeDiagnosticCollectorComponent(collector: AbstractDiagnosticCollect
?: errorNamedReference.source ?: return
// Don't report duplicated unresolved reference on annotation entry (already reported on its type)
if (source.elementType == KtNodeTypes.ANNOTATION_ENTRY && errorNamedReference.diagnostic is ConeUnresolvedNameError) return
// Already reported in FirConventionFunctionCallChecker
if (errorNamedReference.source?.kind == FirFakeSourceElementKind.ArrayAccessNameReference &&
errorNamedReference.diagnostic is ConeUnresolvedNameError
) return
reportFirDiagnostic(errorNamedReference.diagnostic, source, reporter, data)
}
@@ -139,6 +139,8 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.NOT_AN_ANNOTATION
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.NOT_A_LOOP_LABEL
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.NOT_A_SUPERTYPE
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.NO_ELSE_IN_WHEN
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.NO_GET_METHOD
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.NO_SET_METHOD
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.NO_THIS
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.NULLABLE_TYPE_IN_CLASS_LITERAL_LHS
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.NULLABLE_TYPE_OF_ANNOTATION_MEMBER
@@ -679,6 +681,10 @@ class FirDefaultErrorMessages : DefaultErrorMessages.Extension {
// Function contracts
map.put(ERROR_IN_CONTRACT_DESCRIPTION, "Error in contract description", TO_STRING)
// Conventions
map.put(NO_GET_METHOD, "No get method providing array access")
map.put(NO_SET_METHOD, "No set method providing array access")
// Extended checkers group
map.put(REDUNDANT_VISIBILITY_MODIFIER, "Redundant visibility modifier")
map.put(REDUNDANT_MODALITY_MODIFIER, "Redundant modality modifier")
@@ -415,6 +415,17 @@ object LightTreePositioningStrategies {
return markElement(tree.ifKeyword(node) ?: node, startOffset, endOffset, tree, node)
}
}
val ARRAY_ACCESS = object : LightTreePositioningStrategy() {
override fun mark(
node: LighterASTNode,
startOffset: Int,
endOffset: Int,
tree: FlyweightCapableTreeStructure<LighterASTNode>
): List<TextRange> {
return markElement(tree.findChildByType(node, KtNodeTypes.INDICES)!!, startOffset, endOffset, tree, node)
}
}
}
fun FirSourceElement.hasValOrVar(): Boolean =
@@ -143,4 +143,8 @@ object SourceElementPositioningStrategies {
PositioningStrategies.IF_EXPRESSION
)
val ARRAY_ACCESS = SourceElementPositioningStrategy(
LightTreePositioningStrategies.ARRAY_ACCESS,
PositioningStrategies.ARRAY_ACCESS
)
}
@@ -26,7 +26,9 @@ object CommonExpressionCheckers : ExpressionCheckers() {
FirSealedClassConstructorCallChecker,
)
override val functionCallCheckers: Set<FirFunctionCallChecker> = setOf()
override val functionCallCheckers: Set<FirFunctionCallChecker> = setOf(
FirConventionFunctionCallChecker,
)
override val tryExpressionCheckers: Set<FirTryExpressionChecker> = setOf(
FirCatchParameterChecker
@@ -825,10 +825,11 @@ class ExpressionsConverter(
}
val getArgument = context.arraySetArgument.remove(arrayAccess)
return buildFunctionCall {
source = arrayAccess.toFirSourceElement()
val isGet = getArgument == null
source = (if (isGet) arrayAccess else arrayAccess.getParent()!!).toFirSourceElement()
calleeReference = buildSimpleNamedReference {
source = this@buildFunctionCall.source
name = if (getArgument == null) OperatorNameConventions.GET else OperatorNameConventions.SET
source = arrayAccess.toFirSourceElement().fakeElement(FirFakeSourceElementKind.ArrayAccessNameReference)
name = if (isGet) OperatorNameConventions.GET else OperatorNameConventions.SET
}
explicitReceiver = firExpression
argumentList = buildArgumentList {
@@ -28,8 +28,6 @@ import org.jetbrains.kotlin.fir.expressions.builder.*
import org.jetbrains.kotlin.fir.expressions.impl.FirSingleExpressionBlock
import org.jetbrains.kotlin.fir.references.builder.*
import org.jetbrains.kotlin.fir.scopes.FirScopeProvider
import org.jetbrains.kotlin.name.CallableId
import org.jetbrains.kotlin.name.LocalCallableIdConstructor
import org.jetbrains.kotlin.fir.symbols.impl.*
import org.jetbrains.kotlin.fir.types.*
import org.jetbrains.kotlin.fir.types.builder.*
@@ -38,7 +36,9 @@ import org.jetbrains.kotlin.fir.types.impl.FirQualifierPartImpl
import org.jetbrains.kotlin.fir.types.impl.FirTypeArgumentListImpl
import org.jetbrains.kotlin.fir.types.impl.FirTypePlaceholderProjection
import org.jetbrains.kotlin.lexer.KtTokens.*
import org.jetbrains.kotlin.name.CallableId
import org.jetbrains.kotlin.name.ClassId
import org.jetbrains.kotlin.name.LocalCallableIdConstructor
import org.jetbrains.kotlin.name.Name
import org.jetbrains.kotlin.psi.*
import org.jetbrains.kotlin.psi.psiUtil.*
@@ -1931,23 +1931,14 @@ open class RawFirBuilder(
override fun visitArrayAccessExpression(expression: KtArrayAccessExpression, data: Unit): FirElement {
val arrayExpression = expression.arrayExpression
val getArgument = context.arraySetArgument.remove(expression)
return buildFunctionCall {
val source: FirPsiSourceElement<*>
val getArgument = context.arraySetArgument.remove(expression)
if (getArgument != null) {
calleeReference = buildSimpleNamedReference {
source = expression.parent.toFirSourceElement()
this.source = source.fakeElement(FirFakeSourceElementKind.ArrayAccessNameReference)
name = OperatorNameConventions.SET
}
} else {
source = expression.toFirSourceElement()
calleeReference = buildSimpleNamedReference {
this.source = source.fakeElement(FirFakeSourceElementKind.ArrayAccessNameReference)
name = OperatorNameConventions.GET
}
val isGet = getArgument == null
source = (if (isGet) expression else expression.parent).toFirSourceElement()
calleeReference = buildSimpleNamedReference {
source = expression.toFirSourceElement().fakeElement(FirFakeSourceElementKind.ArrayAccessNameReference)
name = if (isGet) OperatorNameConventions.GET else OperatorNameConventions.SET
}
this.source = source
explicitReceiver = arrayExpression.toFirExpression("No array expression")
argumentList = buildArgumentList {
for (indexExpression in expression.indexExpressions) {
@@ -128,7 +128,7 @@ class Test() {
(f@ a)[3] = 4
<!UNRESOLVED_REFERENCE!>this[54] = 34<!>
this<!NO_SET_METHOD!>[54]<!> = 34
}
}
@@ -12,5 +12,5 @@ class C<T> {
}
fun test(a: C<out CharSequence>) {
<!INAPPLICABLE_CANDIDATE!>a[1] = 25<!>
<!INAPPLICABLE_CANDIDATE!>a[1]<!> = 25
}
@@ -31,5 +31,5 @@ fun test() {
D[0] = ""
D[0] = 2.72
<!INAPPLICABLE_CANDIDATE!>Z[0] = ""<!>
<!INAPPLICABLE_CANDIDATE!>Z[0]<!> = ""
}
@@ -6,6 +6,6 @@ class A {
operator fun set(x: String, value: Int) {}
fun d(x: Int) {
<!INAPPLICABLE_CANDIDATE!>this["", 1] = 1<!>
<!INAPPLICABLE_CANDIDATE!>this["", 1]<!> = 1
}
}
@@ -1,7 +1,7 @@
// !WITH_NEW_INFERENCE
fun test(): Array<Int> {
[1, 2]
<!UNRESOLVED_REFERENCE!>[1, 2][0]<!>
[1, 2]<!NO_GET_METHOD!>[0]<!>
[1, 2].<!UNRESOLVED_REFERENCE!>get<!>(0)
foo([""])
@@ -9,6 +9,6 @@ class A<T> {
fun test(a: A<out CharSequence>) {
a <!INAPPLICABLE_CANDIDATE!>+<!> ""
<!INAPPLICABLE_CANDIDATE!>a[1] = ""<!>
<!INAPPLICABLE_CANDIDATE!>a[1]<!> = ""
<!INAPPLICABLE_CANDIDATE!>a[""]<!>
}
@@ -11,7 +11,7 @@ class Out<out F>
fun test(a: A<out CharSequence>, y: Out<CharSequence>) {
a <!INAPPLICABLE_CANDIDATE!>+<!> y
<!INAPPLICABLE_CANDIDATE!>a[1] = y<!>
<!INAPPLICABLE_CANDIDATE!>a[1]<!> = y
<!INAPPLICABLE_CANDIDATE!>a[y]<!>
a + Out<Nothing>()
@@ -4,5 +4,5 @@ package bar
fun main() {
class Some
Some[<!SYNTAX!><!>] <!UNRESOLVED_REFERENCE!>names<!> <!UNRESOLVED_REFERENCE!><!SYNTAX!>=<!> ["ads"]<!>
Some[<!SYNTAX!><!>] <!UNRESOLVED_REFERENCE!>names<!> <!SYNTAX!>=<!> <!NO_GET_METHOD!>["ads"]<!>
}
@@ -1,5 +1,5 @@
//KT-4866 Resolve does not work inside brackets with unresolved reference before
fun test(i: Int, j: Int) {
<!UNRESOLVED_REFERENCE!><!UNRESOLVED_REFERENCE!>foo<!>[i, j]<!>
}
<!UNRESOLVED_REFERENCE!>foo<!><!NO_GET_METHOD!>[i, j]<!>
}
@@ -15,7 +15,7 @@ fun case_1(a: MutableList<out MutableList<MutableList<MutableList<MutableList<Mu
if (f != null) {
val g = f[0]
if (g != null) {
val h = <!UNRESOLVED_REFERENCE!>g[0]<!>
val h = g<!NO_GET_METHOD!>[0]<!>
if (h != null) {
h.<!UNRESOLVED_REFERENCE!>inc<!>()
}
@@ -43,7 +43,7 @@ fun case_2(a: MutableList<out MutableList<MutableList<MutableList<out MutableLis
if (f != null) {
val g = f[0]
if (g != null) {
val h = <!UNRESOLVED_REFERENCE!>g[0]<!> // no SMARTCAST diagnostic
val h = g<!NO_GET_METHOD!>[0]<!> // no SMARTCAST diagnostic
if (h != null) {
h.<!UNRESOLVED_REFERENCE!>inc<!>()
}
@@ -1,31 +0,0 @@
// FILE: J.java
import org.jetbrains.annotations.*;
public class J {
@NotNull
public static Integer[] staticNN;
@Nullable
public static Integer[] staticN;
public static Integer[] staticJ;
}
// FILE: k.kt
fun test() {
// @NotNull platform type
val platformNN = J.staticNN
// @Nullable platform type
val platformN = J.staticN
// platform type with no annotation
val platformJ = J.staticJ
platformNN[0]
<!UNSAFE_CALL!>platformN[0]<!>
platformJ[0]
platformNN[0] = 1
<!UNSAFE_CALL!>platformN[0] = 1<!>
platformJ[0] = 1
}
@@ -1,3 +1,4 @@
// FIR_IDENTICAL
// FILE: J.java
import org.jetbrains.annotations.*;
@@ -1,31 +0,0 @@
// FILE: J.java
import org.jetbrains.annotations.*;
public class J {
@NotNull
public static int[] staticNN;
@Nullable
public static int[] staticN;
public static int[] staticJ;
}
// FILE: k.kt
fun test() {
// @NotNull platform type
val platformNN = J.staticNN
// @Nullable platform type
val platformN = J.staticN
// platform type with no annotation
val platformJ = J.staticJ
platformNN[0]
<!UNSAFE_CALL!>platformN[0]<!>
platformJ[0]
platformNN[0] = 1
<!UNSAFE_CALL!>platformN[0] = 1<!>
platformJ[0] = 1
}
@@ -1,3 +1,4 @@
// FIR_IDENTICAL
// FILE: J.java
import org.jetbrains.annotations.*;
@@ -7,6 +7,6 @@ fun foo(a: MutableMap<String, String>, x: String?) {
}
fun foo1(a: MutableMap<String, String>, x: String?) {
<!INAPPLICABLE_CANDIDATE!>a[x] = x!!<!>
<!INAPPLICABLE_CANDIDATE!>a[x]<!> = x!!
a[x!!] = x
}
@@ -96,7 +96,7 @@ fun case_7() {
fun case_8(y: MutableList<Boolean>) {
var x: Boolean? = true
if (x != null) {
<!INAPPLICABLE_CANDIDATE!>y[if (true) {x = null;0} else 0] = <!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Boolean?")!>x<!><!>
<!INAPPLICABLE_CANDIDATE!>y[if (true) {x = null;0} else 0]<!> = <!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Boolean?")!>x<!>
}
}
@@ -130,6 +130,6 @@ fun case_10() {
fun case_11(y: MutableList<MutableList<Int>>) {
var x: Int? = 10
if (x != null) {
<!INAPPLICABLE_CANDIDATE!>y[if (true) {x = null;0} else 0][x] = 10<!>
<!INAPPLICABLE_CANDIDATE!>y[if (true) {x = null;0} else 0][x]<!> = 10
}
}
@@ -6,6 +6,6 @@
fun <T : List<T>> Inv<out T>.case_1() {
if (this is MutableList<*>) {
<!DEBUG_INFO_EXPRESSION_TYPE("Inv<out T> & kotlin.collections.MutableList<*> & Inv<out T>")!>this<!>
<!INAPPLICABLE_CANDIDATE!><!DEBUG_INFO_EXPRESSION_TYPE("Inv<out T> & kotlin.collections.MutableList<*> & Inv<out T>")!>this<!>[0] = <!DEBUG_INFO_EXPRESSION_TYPE("Inv<out T> & kotlin.collections.MutableList<*> & Inv<out T>")!>this<!>[1]<!>
<!INAPPLICABLE_CANDIDATE!><!DEBUG_INFO_EXPRESSION_TYPE("Inv<out T> & kotlin.collections.MutableList<*> & Inv<out T>")!>this<!>[0]<!> = <!DEBUG_INFO_EXPRESSION_TYPE("Inv<out T> & kotlin.collections.MutableList<*> & Inv<out T>")!>this<!>[1]
}
}
@@ -15,6 +15,7 @@ import org.jetbrains.kotlin.fir.declarations.FirDeclaration
import org.jetbrains.kotlin.fir.declarations.FirProperty
import org.jetbrains.kotlin.fir.declarations.FirTypeParameter
import org.jetbrains.kotlin.fir.psi
import org.jetbrains.kotlin.psi.KtArrayAccessExpression
import org.jetbrains.kotlin.psi.KtClassOrObject
import org.jetbrains.kotlin.psi.KtDeclaration
import org.jetbrains.kotlin.psi.KtDestructuringDeclaration
@@ -1402,6 +1403,18 @@ internal val KT_DIAGNOSTIC_CONVERTER = KtDiagnosticConverterBuilder.buildConvert
token,
)
}
add(FirErrors.NO_GET_METHOD) { firDiagnostic ->
NoGetMethodImpl(
firDiagnostic as FirPsiDiagnostic<*>,
token,
)
}
add(FirErrors.NO_SET_METHOD) { firDiagnostic ->
NoSetMethodImpl(
firDiagnostic as FirPsiDiagnostic<*>,
token,
)
}
add(FirErrors.REDUNDANT_VISIBILITY_MODIFIER) { firDiagnostic ->
RedundantVisibilityModifierImpl(
firDiagnostic as FirPsiDiagnostic<*>,
@@ -19,6 +19,7 @@ import org.jetbrains.kotlin.idea.frontend.api.symbols.KtVariableSymbol
import org.jetbrains.kotlin.idea.frontend.api.types.KtType
import org.jetbrains.kotlin.lexer.KtModifierKeywordToken
import org.jetbrains.kotlin.name.Name
import org.jetbrains.kotlin.psi.KtArrayAccessExpression
import org.jetbrains.kotlin.psi.KtClassOrObject
import org.jetbrains.kotlin.psi.KtDeclaration
import org.jetbrains.kotlin.psi.KtDestructuringDeclaration
@@ -987,6 +988,14 @@ sealed class KtFirDiagnostic<PSI: PsiElement> : KtDiagnosticWithPsi<PSI> {
abstract val reason: String
}
abstract class NoGetMethod : KtFirDiagnostic<KtArrayAccessExpression>() {
override val diagnosticClass get() = NoGetMethod::class
}
abstract class NoSetMethod : KtFirDiagnostic<KtArrayAccessExpression>() {
override val diagnosticClass get() = NoSetMethod::class
}
abstract class RedundantVisibilityModifier : KtFirDiagnostic<KtModifierListOwner>() {
override val diagnosticClass get() = RedundantVisibilityModifier::class
}
@@ -21,6 +21,7 @@ import org.jetbrains.kotlin.idea.frontend.api.symbols.KtVariableSymbol
import org.jetbrains.kotlin.idea.frontend.api.types.KtType
import org.jetbrains.kotlin.lexer.KtModifierKeywordToken
import org.jetbrains.kotlin.name.Name
import org.jetbrains.kotlin.psi.KtArrayAccessExpression
import org.jetbrains.kotlin.psi.KtClassOrObject
import org.jetbrains.kotlin.psi.KtDeclaration
import org.jetbrains.kotlin.psi.KtDestructuringDeclaration
@@ -1597,6 +1598,20 @@ internal class ErrorInContractDescriptionImpl(
override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic)
}
internal class NoGetMethodImpl(
firDiagnostic: FirPsiDiagnostic<*>,
override val token: ValidityToken,
) : KtFirDiagnostic.NoGetMethod(), KtAbstractFirDiagnostic<KtArrayAccessExpression> {
override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic)
}
internal class NoSetMethodImpl(
firDiagnostic: FirPsiDiagnostic<*>,
override val token: ValidityToken,
) : KtFirDiagnostic.NoSetMethod(), KtAbstractFirDiagnostic<KtArrayAccessExpression> {
override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic)
}
internal class RedundantVisibilityModifierImpl(
firDiagnostic: FirPsiDiagnostic<*>,
override val token: ValidityToken,