From 2a03e1b3dae803cce00d2c47011cfa72733d641e Mon Sep 17 00:00:00 2001 From: Toshiaki Kameyama Date: Wed, 14 Nov 2018 18:49:25 +0900 Subject: [PATCH] Leaking this: fix for 'this' in enum class #KT-15835 Fixed --- .../idea/inspections/LeakingThisInspection.kt | 17 +++++++++++--- .../inspectionsLocal/leakingThis/enum.kt | 8 +++++++ .../enumEntryHasOverriddenMember.kt | 20 ++++++++++++++++ .../enumEntryHasOverriddenMember2.kt | 23 +++++++++++++++++++ .../LocalInspectionTestGenerated.java | 15 ++++++++++++ 5 files changed, 80 insertions(+), 3 deletions(-) create mode 100644 idea/testData/inspectionsLocal/leakingThis/enum.kt create mode 100644 idea/testData/inspectionsLocal/leakingThis/enumEntryHasOverriddenMember.kt create mode 100644 idea/testData/inspectionsLocal/leakingThis/enumEntryHasOverriddenMember2.kt diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/LeakingThisInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/LeakingThisInspection.kt index 4171dcf2d9e..b252f2a1a3b 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/LeakingThisInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/LeakingThisInspection.kt @@ -19,6 +19,7 @@ import org.jetbrains.kotlin.idea.quickfix.AddModifierFix import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.psiUtil.containingClassOrObject +import org.jetbrains.kotlin.psi.psiUtil.getChildrenOfType import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType import org.jetbrains.kotlin.resolve.BindingContext.LEAKING_THIS import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils @@ -36,10 +37,17 @@ class LeakingThisInspection : AbstractKotlinInspection() { if (leakingThisDescriptor.classOrObject != klass) continue@these val description = when (leakingThisDescriptor) { is NonFinalClass -> - if (expression is KtThisExpression && expression.getStrictParentOfType() == null) - "Leaking 'this' in constructor of non-final class ${leakingThisDescriptor.klass.name}" - else + if (expression is KtThisExpression && expression.getStrictParentOfType() == null) { + if (klass.isEnum()) { + val enumEntries = klass.body?.getChildrenOfType() ?: continue@these + if (enumEntries.none { it.hasOverriddenMember() }) continue@these + "Leaking 'this' in constructor of enum class ${leakingThisDescriptor.klass.name} (with overridable members)" + } else { + "Leaking 'this' in constructor of non-final class ${leakingThisDescriptor.klass.name}" + } + } else { continue@these // Not supported yet + } is NonFinalProperty -> "Accessing non-final property ${leakingThisDescriptor.property.name} in constructor" is NonFinalFunction -> @@ -75,6 +83,9 @@ class LeakingThisInspection : AbstractKotlinInspection() { } } + private fun KtEnumEntry.hasOverriddenMember(): Boolean { + return body?.getChildrenOfType()?.any { it.hasModifier(KtTokens.OVERRIDE_KEYWORD) } == true + } companion object { private fun createMakeFinalFix(declaration: KtDeclaration?): IntentionWrapper? { diff --git a/idea/testData/inspectionsLocal/leakingThis/enum.kt b/idea/testData/inspectionsLocal/leakingThis/enum.kt new file mode 100644 index 00000000000..6cea5f2b826 --- /dev/null +++ b/idea/testData/inspectionsLocal/leakingThis/enum.kt @@ -0,0 +1,8 @@ +// PROBLEM: none +enum class Foo { + ONE, + TWO; + val bar = Bar(this) +} + +class Bar(val foo: Foo) \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/leakingThis/enumEntryHasOverriddenMember.kt b/idea/testData/inspectionsLocal/leakingThis/enumEntryHasOverriddenMember.kt new file mode 100644 index 00000000000..7c4ee284ee5 --- /dev/null +++ b/idea/testData/inspectionsLocal/leakingThis/enumEntryHasOverriddenMember.kt @@ -0,0 +1,20 @@ +// PROBLEM: Leaking 'this' in constructor of enum class Foo (with overridable members) +// FIX: none +enum class Foo { + ONE { + override val x = 1 + }, + TWO { + override val x = 2 + }; + + abstract val x: Int + val double = double(this) +} + +fun double(foo: Foo) = foo.x * foo.x + +fun test() { + Foo.ONE.double + Foo.TWO.double +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/leakingThis/enumEntryHasOverriddenMember2.kt b/idea/testData/inspectionsLocal/leakingThis/enumEntryHasOverriddenMember2.kt new file mode 100644 index 00000000000..4d6f132e1a8 --- /dev/null +++ b/idea/testData/inspectionsLocal/leakingThis/enumEntryHasOverriddenMember2.kt @@ -0,0 +1,23 @@ +// PROBLEM: Leaking 'this' in constructor of enum class Foo (with overridable members) +// FIX: none +enum class Foo : Bar { + ONE { + override val x = 1 + }, + TWO { + override val x = 2 + }; + + val double = double(this) +} + +interface Bar { + val x: Int +} + +fun double(foo: Foo) = foo.x * foo.x + +fun test() { + Foo.ONE.double + Foo.TWO.double +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java index 96ae1ea4e35..8b993bd1056 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java @@ -2705,6 +2705,21 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/leakingThis"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), TargetBackend.ANY, true); } + @TestMetadata("enum.kt") + public void testEnum() throws Exception { + runTest("idea/testData/inspectionsLocal/leakingThis/enum.kt"); + } + + @TestMetadata("enumEntryHasOverriddenMember.kt") + public void testEnumEntryHasOverriddenMember() throws Exception { + runTest("idea/testData/inspectionsLocal/leakingThis/enumEntryHasOverriddenMember.kt"); + } + + @TestMetadata("enumEntryHasOverriddenMember2.kt") + public void testEnumEntryHasOverriddenMember2() throws Exception { + runTest("idea/testData/inspectionsLocal/leakingThis/enumEntryHasOverriddenMember2.kt"); + } + @TestMetadata("inClassLiteral.kt") public void testInClassLiteral() throws Exception { runTest("idea/testData/inspectionsLocal/leakingThis/inClassLiteral.kt");