K2: add checker detecting protected Java field shadowed by a property

2022-11-25 15:30:13 +01:00
parent 6234da4c86
commit 949a39b80f
25 changed files with 717 additions and 12 deletions
@@ -405,6 +405,24 @@ public class FirOldFrontendDiagnosticsTestGenerated extends AbstractFirDiagnosti
            runTest("compiler/testData/diagnostics/tests/IsExpressions.kt");
        }

+        @Test
+        @TestMetadata("javaFieldKotlinPropertyJavaFieldInPackagePrivate.kt")
+        public void testJavaFieldKotlinPropertyJavaFieldInPackagePrivate() throws Exception {
+            runTest("compiler/testData/diagnostics/tests/javaFieldKotlinPropertyJavaFieldInPackagePrivate.kt");
+        }
+
+        @Test
+        @TestMetadata("javaFieldKotlinPropertyJavaPackagePrivateField.kt")
+        public void testJavaFieldKotlinPropertyJavaPackagePrivateField() throws Exception {
+            runTest("compiler/testData/diagnostics/tests/javaFieldKotlinPropertyJavaPackagePrivateField.kt");
+        }
+
+        @Test
+        @TestMetadata("javaProtectedFieldAndKotlinInvisiblePropertyReference.kt")
+        public void testJavaProtectedFieldAndKotlinInvisiblePropertyReference() throws Exception {
+            runTest("compiler/testData/diagnostics/tests/javaProtectedFieldAndKotlinInvisiblePropertyReference.kt");
+        }
+
        @Test
        @TestMetadata("kt11167.kt")
        public void testKt11167() throws Exception {
@@ -405,6 +405,24 @@ public class FirOldFrontendDiagnosticsWithLightTreeTestGenerated extends Abstrac
            runTest("compiler/testData/diagnostics/tests/IsExpressions.kt");
        }

+        @Test
+        @TestMetadata("javaFieldKotlinPropertyJavaFieldInPackagePrivate.kt")
+        public void testJavaFieldKotlinPropertyJavaFieldInPackagePrivate() throws Exception {
+            runTest("compiler/testData/diagnostics/tests/javaFieldKotlinPropertyJavaFieldInPackagePrivate.kt");
+        }
+
+        @Test
+        @TestMetadata("javaFieldKotlinPropertyJavaPackagePrivateField.kt")
+        public void testJavaFieldKotlinPropertyJavaPackagePrivateField() throws Exception {
+            runTest("compiler/testData/diagnostics/tests/javaFieldKotlinPropertyJavaPackagePrivateField.kt");
+        }
+
+        @Test
+        @TestMetadata("javaProtectedFieldAndKotlinInvisiblePropertyReference.kt")
+        public void testJavaProtectedFieldAndKotlinInvisiblePropertyReference() throws Exception {
+            runTest("compiler/testData/diagnostics/tests/javaProtectedFieldAndKotlinInvisiblePropertyReference.kt");
+        }
+
        @Test
        @TestMetadata("kt11167.kt")
        public void testKt11167() throws Exception {
@@ -6,7 +6,6 @@
 package org.jetbrains.kotlin.fir.checkers.generator.diagnostics

 import com.intellij.psi.PsiElement
-import org.jetbrains.kotlin.config.LanguageFeature
 import org.jetbrains.kotlin.config.LanguageFeature.*
 import org.jetbrains.kotlin.fir.PrivateForInline
 import org.jetbrains.kotlin.fir.checkers.generator.diagnostics.model.DiagnosticList
@@ -21,7 +20,7 @@ import org.jetbrains.kotlin.psi.KtDeclaration
 import org.jetbrains.kotlin.psi.KtElement
 import org.jetbrains.kotlin.psi.KtExpression

-@Suppress("UNUSED_VARIABLE", "LocalVariableName", "ClassName", "unused")
+@Suppress("ClassName", "unused")
@OptIn(PrivateForInline::class)
 object JVM_DIAGNOSTICS_LIST : DiagnosticList("FirJvmErrors") {
    val DECLARATIONS by object : DiagnosticGroup("Declarations") {
@@ -70,7 +69,7 @@ object JVM_DIAGNOSTICS_LIST : DiagnosticList("FirJvmErrors") {
            ProhibitJvmOverloadsOnConstructorsOfAnnotationClasses
        )
        val OVERLOADS_PRIVATE by warning<KtAnnotationEntry>()
-        val DEPRECATED_JAVA_ANNOTATION by warning<KtAnnotationEntry>() {
+        val DEPRECATED_JAVA_ANNOTATION by warning<KtAnnotationEntry> {
            parameter<FqName>("kotlinName")
        }

@@ -87,7 +86,7 @@ object JVM_DIAGNOSTICS_LIST : DiagnosticList("FirJvmErrors") {
    }

    val SUPER by object : DiagnosticGroup("Super") {
-        val SUPER_CALL_WITH_DEFAULT_PARAMETERS by error<PsiElement>() {
+        val SUPER_CALL_WITH_DEFAULT_PARAMETERS by error<PsiElement> {
            parameter<String>("name")
        }
        val INTERFACE_CANT_CALL_DEFAULT_METHOD_VIA_SUPER by error<PsiElement>(PositioningStrategy.REFERENCE_BY_QUALIFIED)
@@ -191,5 +190,9 @@ object JVM_DIAGNOSTICS_LIST : DiagnosticList("FirJvmErrors") {
            PositioningStrategy.SPREAD_OPERATOR
        )
        val JAVA_SAM_INTERFACE_CONSTRUCTOR_REFERENCE by error<PsiElement>()
+        val JAVA_SHADOWED_PROTECTED_FIELD_REFERENCE by error<PsiElement> {
+            parameter<ClassId>("containerClass")
+            parameter<ClassId>("shadowingClass")
+        }
    }
 }
@@ -129,6 +129,7 @@ object FirJvmErrors {
    val CONCURRENT_HASH_MAP_CONTAINS_OPERATOR by deprecationError0<PsiElement>(ProhibitConcurrentHashMapContains)
    val SPREAD_ON_SIGNATURE_POLYMORPHIC_CALL by deprecationError0<PsiElement>(ProhibitSpreadOnSignaturePolymorphicCall, SourceElementPositioningStrategies.SPREAD_OPERATOR)
    val JAVA_SAM_INTERFACE_CONSTRUCTOR_REFERENCE by error0<PsiElement>()
+    val JAVA_SHADOWED_PROTECTED_FIELD_REFERENCE by error2<PsiElement, ClassId, ClassId>()

    init {
        RootDiagnosticRendererFactory.registerFactory(FirJvmErrorsDefaultMessages)
@@ -33,6 +33,7 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.INNER_JVM_
 import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.INTERFACE_CANT_CALL_DEFAULT_METHOD_VIA_SUPER
 import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.INTERFACE_STATIC_METHOD_CALL_FROM_JAVA6_TARGET
 import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.JAVA_SAM_INTERFACE_CONSTRUCTOR_REFERENCE
+import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.JAVA_SHADOWED_PROTECTED_FIELD_REFERENCE
 import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.JAVA_TYPE_MISMATCH
 import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.JVM_DEFAULT_IN_DECLARATION
 import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.JVM_DEFAULT_IN_JVM6_TARGET
@@ -286,6 +287,14 @@ object FirJvmErrorsDefaultMessages : BaseDiagnosticRendererFactory() {
            JAVA_SAM_INTERFACE_CONSTRUCTOR_REFERENCE,
            "Java SAM interface constructor references are prohibited"
        )
+        map.put(
+            JAVA_SHADOWED_PROTECTED_FIELD_REFERENCE,
+            "It's impossible to refer here a protected Java field from class ''{0}'', " +
+                    "because an invisible property from class ''{1}'' shadows it. " +
+                    "Rename the property from class ''{1}'' or change visibility of the field from class ''{0}''",
+            TO_STRING,
+            TO_STRING
+        )
        map.put(
            REDUNDANT_REPEATABLE_ANNOTATION,
            "Please, remove the ''{0}'' annotation, as ''{1}'' is already enough",
@@ -17,7 +17,12 @@ object JvmExpressionCheckers : ExpressionCheckers() {
    override val qualifiedAccessExpressionCheckers: Set<FirQualifiedAccessExpressionChecker>
        get() = setOf(
            FirInterfaceDefaultMethodCallChecker,
-            FirJavaSamInterfaceConstructorReferenceChecker
+            FirJavaSamInterfaceConstructorReferenceChecker,
+        )
+
+    override val callableReferenceAccessCheckers: Set<FirCallableReferenceAccessChecker>
+        get() = setOf(
+            FirJavaShadowedFieldReferenceChecker,
        )

    override val functionCallCheckers: Set<FirFunctionCallChecker>
@@ -0,0 +1,69 @@
+/*
+ * Copyright 2010-2022 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.jvm.checkers.expression
+
+import org.jetbrains.kotlin.descriptors.java.JavaVisibilities
+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.expression.FirCallableReferenceAccessChecker
+import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors
+import org.jetbrains.kotlin.fir.containingClassLookupTag
+import org.jetbrains.kotlin.fir.declarations.utils.hasBackingField
+import org.jetbrains.kotlin.fir.declarations.utils.visibility
+import org.jetbrains.kotlin.fir.expressions.FirCallableReferenceAccess
+import org.jetbrains.kotlin.fir.expressions.impl.FirNoReceiverExpression
+import org.jetbrains.kotlin.fir.packageFqName
+import org.jetbrains.kotlin.fir.references.FirResolvedNamedReference
+import org.jetbrains.kotlin.fir.resolve.*
+import org.jetbrains.kotlin.fir.scopes.FakeOverrideTypeCalculator
+import org.jetbrains.kotlin.fir.symbols.impl.FirFieldSymbol
+import org.jetbrains.kotlin.fir.symbols.impl.FirPropertySymbol
+import org.jetbrains.kotlin.fir.types.coneType
+import org.jetbrains.kotlin.name.ClassId
+
+object FirJavaShadowedFieldReferenceChecker : FirCallableReferenceAccessChecker() {
+    override fun check(expression: FirCallableReferenceAccess, context: CheckerContext, reporter: DiagnosticReporter) {
+        val reference = expression.calleeReference as? FirResolvedNamedReference ?: return
+        val referredSymbol = reference.resolvedSymbol as? FirFieldSymbol ?: return
+        if (referredSymbol.visibility != JavaVisibilities.ProtectedAndPackage) return
+        val session = context.session
+        val fieldContainingClassSymbol = referredSymbol.containingClassLookupTag()?.toFirRegularClassSymbol(session) ?: return
+        // Would it be visible, if it would be package private instead of protected-and-package?
+        if (context.containingFile?.packageFqName == fieldContainingClassSymbol.classId.packageFqName) {
+            return
+        }
+
+        val dispatchReceiver = expression.dispatchReceiver.takeIf { it !is FirNoReceiverExpression } ?: return
+        val scope = dispatchReceiver.typeRef.coneType.scope(
+            session, context.sessionHolder.scopeSession, FakeOverrideTypeCalculator.DoNothing
+        ) ?: return
+
+        var shadowingPropertyClassId: ClassId? = null
+        scope.processPropertiesByName(referredSymbol.name) {
+            if (it !is FirPropertySymbol) return@processPropertiesByName
+            if (!it.hasBackingField) return@processPropertiesByName
+            val propertyContainingClassSymbol = it.containingClassLookupTag()?.toFirRegularClassSymbol(session)
+                ?: return@processPropertiesByName
+            if (propertyContainingClassSymbol.isSubclassOf(
+                    fieldContainingClassSymbol.toLookupTag(), session, isStrict = true, lookupInterfaces = false
+                )
+            ) {
+                shadowingPropertyClassId = propertyContainingClassSymbol.classId
+            }
+        }
+
+        shadowingPropertyClassId?.let {
+            reporter.reportOn(
+                reference.source,
+                FirJvmErrors.JAVA_SHADOWED_PROTECTED_FIELD_REFERENCE,
+                fieldContainingClassSymbol.classId,
+                it,
+                context
+            )
+        }
+    }
+}
@@ -11,7 +11,6 @@ import org.jetbrains.kotlin.diagnostics.DiagnosticReporter
 import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors
 import org.jetbrains.kotlin.diagnostics.reportOn
 import org.jetbrains.kotlin.fir.declarations.FirClass
-import org.jetbrains.kotlin.fir.declarations.FirFile
 import org.jetbrains.kotlin.fir.declarations.getStringArgument
 import org.jetbrains.kotlin.fir.expressions.FirAnnotation
 import org.jetbrains.kotlin.fir.types.ConeClassLikeType
@@ -37,7 +36,7 @@ object FirJvmPackageNameAnnotationsChecker : FirAnnotationChecker() {
            reporter.reportOn(expression.source, FirJvmErrors.JVM_PACKAGE_NAME_MUST_BE_VALID_NAME, context)
        }

-        val file = context.containingDeclarations.firstOrNull() as? FirFile ?: return
+        val file = context.containingFile ?: return
        if (file.declarations.any { it is FirClass }) {
            reporter.reportOn(expression.source, FirJvmErrors.JVM_PACKAGE_NAME_NOT_SUPPORTED_IN_FILES_WITH_CLASSES, context)
        }
@@ -91,8 +91,11 @@ abstract class CheckerContext : MutableDiagnosticContext() {
    override val languageVersionSettings: LanguageVersionSettings
        get() = session.languageVersionSettings

+    val containingFile: FirFile?
+        get() = if (containingDeclarations.isEmpty()) null else containingDeclarations.first() as FirFile
+
    override val containingFilePath: String?
-        get() = containingDeclarations.firstOrNull()?.let { (it as? FirFile)?.sourceFile?.path }
+        get() = containingFile?.sourceFile?.path
 }

 /**
@@ -9,7 +9,6 @@ import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext
 import org.jetbrains.kotlin.diagnostics.DiagnosticReporter
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors
 import org.jetbrains.kotlin.diagnostics.reportOn
-import org.jetbrains.kotlin.fir.declarations.FirFile
 import org.jetbrains.kotlin.fir.declarations.FirTypeAlias
 import org.jetbrains.kotlin.fir.declarations.utils.expandedConeType
 import org.jetbrains.kotlin.fir.expressions.FirResolvedQualifier
@@ -32,7 +31,7 @@ object FirVisibilityQualifierChecker : FirResolvedQualifierChecker() {
        context: CheckerContext,
        reporter: DiagnosticReporter
    ) {
-        val firFile = context.containingDeclarations.firstOrNull() as? FirFile ?: return
+        val firFile = context.containingFile ?: return
        val firClassLikeDeclaration = symbol.fir

        if (!context.session.visibilityChecker.isClassLikeVisible(
@@ -92,11 +92,22 @@ fun lookupSuperTypes(
    }
 }

+fun FirClassSymbol<*>.isSubclassOf(
+    ownerLookupTag: ConeClassLikeLookupTag,
+    session: FirSession,
+    isStrict: Boolean,
+    lookupInterfaces: Boolean
+): Boolean {
+    lazyResolveToPhase(FirResolvePhase.SUPER_TYPES)
+    return fir.isSubclassOf(ownerLookupTag, session, isStrict, SupertypeSupplier.Default, lookupInterfaces)
+}
+
 fun FirClass.isSubclassOf(
    ownerLookupTag: ConeClassLikeLookupTag,
    session: FirSession,
    isStrict: Boolean,
-    supertypeSupplier: SupertypeSupplier = SupertypeSupplier.Default
+    supertypeSupplier: SupertypeSupplier = SupertypeSupplier.Default,
+    lookupInterfaces: Boolean = true,
 ): Boolean {
    if (classId.isSame(ownerLookupTag.classId)) {
        return !isStrict
@@ -104,7 +115,7 @@ fun FirClass.isSubclassOf(

    return lookupSuperTypes(
        this,
-        lookupInterfaces = true,
+        lookupInterfaces = lookupInterfaces,
        deep = true,
        session,
        substituteTypes = false,