From c0198a15fe3f4b8dcc68ef1f1a74d2300f38bbdd Mon Sep 17 00:00:00 2001 From: nik Date: Wed, 3 Apr 2019 14:13:14 +0300 Subject: [PATCH] kotlin: fix false positives for 'suspicious package-private access' inspection nested object literal expressions in Kotlin This is an improvement for cdd6055 which also fixes IDEA-210253. JavaResolveUtil.canAccessProtectedMember doesn't work properly for Kotlin because of KT-30759 and KT-30752, so its code is rewritten to use UAST to find outer class. --- .../dep/xxx/ProtectedMembersKotlin.kt | 7 ++++ .../src/AccessingProtectedMembers.kt | 14 +++++++ .../AccessingProtectedMembersFromKotlin.kt | 31 ++++++++++++++ ...ciousPackagePrivateAccessInspectionTest.kt | 4 ++ ...piciousPackagePrivateAccessInspection.java | 40 +++++++++++++------ .../src/AccessingProtectedMembers.java | 16 ++++++++ 6 files changed, 99 insertions(+), 13 deletions(-) create mode 100644 jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/ProtectedMembersKotlin.kt create mode 100644 jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/src/AccessingProtectedMembersFromKotlin.kt diff --git a/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/ProtectedMembersKotlin.kt b/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/ProtectedMembersKotlin.kt new file mode 100644 index 000000000000..4b919ebd9661 --- /dev/null +++ b/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/ProtectedMembersKotlin.kt @@ -0,0 +1,7 @@ +package xxx + +open class ProtectedMembersKotlin { + protected val property: String = "" + + protected fun foo() {} +} \ No newline at end of file diff --git a/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/src/AccessingProtectedMembers.kt b/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/src/AccessingProtectedMembers.kt index bb23857c7f2a..e910cc31e1cc 100644 --- a/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/src/AccessingProtectedMembers.kt +++ b/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/src/AccessingProtectedMembers.kt @@ -61,3 +61,17 @@ class AccessingProtectedConstructorFromSubclass : ProtectedConstructors(1) val objectAccessingDefaultProtectedConstructorFromSubclass = object : ProtectedConstructors() {} val objectAccessingProtectedConstructorFromSubclass = object : ProtectedConstructors(1) {} + +class AccessingProtectedMembersFromObjectLiteral { + fun bar1() { + object : ProtectedMembers() { + fun bar2() { + object : Runnable { + override fun run() { + method() + } + } + } + } + } +} diff --git a/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/src/AccessingProtectedMembersFromKotlin.kt b/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/src/AccessingProtectedMembersFromKotlin.kt new file mode 100644 index 000000000000..8d8a5c9def82 --- /dev/null +++ b/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/src/AccessingProtectedMembersFromKotlin.kt @@ -0,0 +1,31 @@ +package xxx + +@Suppress("UNUSED_VARIABLE") +class AccessingProtectedKotlinMembersFromObjectLiteral { + fun bar1() { + object : ProtectedMembersKotlin() { + fun bar2() { + object : Runnable { + override fun run() { + foo() + val s2 = property + } + } + } + } + } +} + +@Suppress("UNUSED_VARIABLE") +class AccessingProtectedMembersFromKotlin : ProtectedMembersKotlin() { + fun bar() { + foo() + val s = property + object : Runnable { + override fun run() { + foo() + val s2 = property + } + } + } +} \ No newline at end of file diff --git a/jvm/jvm-analysis-kotlin-tests/testSrc/com/intellij/codeInspection/tests/kotlin/KtSuspiciousPackagePrivateAccessInspectionTest.kt b/jvm/jvm-analysis-kotlin-tests/testSrc/com/intellij/codeInspection/tests/kotlin/KtSuspiciousPackagePrivateAccessInspectionTest.kt index 837a5bb49182..060b7a7e010b 100644 --- a/jvm/jvm-analysis-kotlin-tests/testSrc/com/intellij/codeInspection/tests/kotlin/KtSuspiciousPackagePrivateAccessInspectionTest.kt +++ b/jvm/jvm-analysis-kotlin-tests/testSrc/com/intellij/codeInspection/tests/kotlin/KtSuspiciousPackagePrivateAccessInspectionTest.kt @@ -14,5 +14,9 @@ class KtSuspiciousPackagePrivateAccessInspectionTest : SuspiciousPackagePrivateA doTestWithDependency() } + fun testAccessingProtectedMembersFromKotlin() { + doTestWithDependency() + } + override fun getBasePath() = "${JvmAnalysisKtTestsUtil.TEST_DATA_PROJECT_RELATIVE_BASE_PATH}/codeInspection/suspiciousPackagePrivateAccess" } \ No newline at end of file diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/dependency/SuspiciousPackagePrivateAccessInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/dependency/SuspiciousPackagePrivateAccessInspection.java index 294c4d66394b..098cb71d2058 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/dependency/SuspiciousPackagePrivateAccessInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/dependency/SuspiciousPackagePrivateAccessInspection.java @@ -15,7 +15,7 @@ import com.intellij.openapi.util.Key; import com.intellij.openapi.util.text.StringUtil; import com.intellij.profile.codeInspection.InspectionProfileManager; import com.intellij.psi.*; -import com.intellij.psi.impl.source.resolve.JavaResolveUtil; +import com.intellij.psi.util.InheritanceUtil; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiTypesUtil; import com.intellij.refactoring.util.RefactoringUIUtil; @@ -232,24 +232,38 @@ public class SuspiciousPackagePrivateAccessInspection extends AbstractBaseUastLo sourceClass); } - private static boolean canAccessProtectedMember(PsiMember member, PsiClass memberClass, PsiClass accessObjectType, - boolean isMemberStatic, UClass sourceClass) { - if (JavaResolveUtil.canAccessProtectedMember(member, memberClass, accessObjectType, sourceClass.getJavaPsi(), isMemberStatic)) { - return true; - } - if (sourceClass instanceof UAnonymousClass && sourceClass.getJavaPsi().getContext() == null) { - //workaround for KT-30752: KtLightClassForAnonymousDeclaration::getContext returns null for object literal expressions in some member initializers - UElement uastParent = sourceClass.getUastParent(); - if (uastParent != null) { - UClass parentClass = UastUtils.findContaining(uastParent.getSourcePsi(), UClass.class); - if (parentClass != null) { - return canAccessProtectedMember(member, memberClass, accessObjectType, isMemberStatic, parentClass); + /** + * The implementation was copied from {@link com.intellij.psi.impl.source.resolve.JavaResolveUtil#canAccessProtectedMember} but uses UAST + * to find outer class as a workaround for bugs in Kotlin Light PSI (KT-30759, KT-30752) + */ + private static boolean canAccessProtectedMember(@NotNull PsiMember member, @NotNull PsiClass memberClass, + @Nullable PsiClass accessObjectClass, boolean isStatic, @Nullable UClass contextClass) { + while (contextClass != null) { + PsiClass javaPsiClass = contextClass.getJavaPsi(); + if (InheritanceUtil.isInheritorOrSelf(javaPsiClass, memberClass, true)) { + if (member instanceof PsiClass || isStatic || accessObjectClass == null + || InheritanceUtil.isInheritorOrSelf(accessObjectClass, javaPsiClass, true)) { + return true; } } + + contextClass = getOuterClass(contextClass); } return false; } + private static UClass getOuterClass(@NotNull UClass aClass) { + UElement uastParent = aClass.getUastParent(); + if (uastParent == null) return null; + PsiElement sourcePsi = uastParent.getSourcePsi(); + while (sourcePsi == null) { + uastParent = uastParent.getUastParent(); + if (uastParent == null) return null; + sourcePsi = uastParent.getSourcePsi(); + } + return UastUtils.findContaining(sourcePsi, UClass.class); + } + private boolean isPackageLocalAccessSuspicious(Module sourceModule, Module targetModule) { if (targetModule == null || sourceModule == null || targetModule.equals(sourceModule)) { return false; diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/src/AccessingProtectedMembers.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/src/AccessingProtectedMembers.java index bc20f0d11cef..1950836066f6 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/src/AccessingProtectedMembers.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/src/AccessingProtectedMembers.java @@ -10,6 +10,15 @@ class AccessingProtectedMembersNotFromSubclass { new ProtectedConstructors() {}; new ProtectedConstructors(1) {}; } + + void baz() { + class LocalSubclass extends ProtectedMembers { + void bar() { + method(); + staticMethod(); + } + } + } } class AccessingProtectedMembersFromSubclass extends ProtectedMembers { @@ -32,6 +41,13 @@ class AccessingProtectedMembersFromSubclass extends ProtectedMembers { staticMethod(); } }; + + class LocalClass { + void baz() { + method(); + staticMethod(); + } + } } public static class StaticInnerImpl1 extends ProtectedMembers.StaticInner {