From 4e91f2ffb7284d9d0fe74056ab03aa7a4ac24fd7 Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Mon, 28 Sep 2015 13:29:06 +0300 Subject: [PATCH] 'open' + 'private' and 'abstract' + 'private' are now incompatible for functions and properties #KT-9324 Fixed --- .../kotlin/resolve/ModifiersChecker.kt | 30 ++++++++++++++++--- .../tests/modifiers/IllegalModifiers.kt | 4 +-- .../kt4204-completeNestedCallsForInvoke.kt | 2 +- .../tests/scopes/VisibilityInheritModifier.kt | 2 +- .../overrideImplement/privateToThis.kt | 7 ----- .../overrideImplement/privateToThis.kt.after | 17 ----------- .../idea/codeInsight/OverrideImplementTest.kt | 4 --- 7 files changed, 30 insertions(+), 36 deletions(-) delete mode 100644 idea/testData/codeInsight/overrideImplement/privateToThis.kt delete mode 100644 idea/testData/codeInsight/overrideImplement/privateToThis.kt.after diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/ModifiersChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/ModifiersChecker.kt index f08d5218ed0..447fcbdef04 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/ModifiersChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/ModifiersChecker.kt @@ -17,6 +17,7 @@ package org.jetbrains.kotlin.resolve import com.intellij.lang.ASTNode +import com.intellij.psi.PsiElement import com.intellij.psi.tree.TokenSet import org.jetbrains.kotlin.descriptors.* import org.jetbrains.kotlin.descriptors.annotations.KotlinTarget @@ -30,12 +31,20 @@ import org.jetbrains.kotlin.psi.* public object ModifierCheckerCore { private enum class Compatibility { + // modifier pair is compatible: ok (default) COMPATIBLE, + // second is redundant to first: warning REDUNDANT, + // first is redundant to second: warning REVERSE_REDUNDANT, + // error REPEATED, + // pair is deprecated, will become incompatible: warning DEPRECATED, - INCOMPATIBLE + // pair is incompatible: error + INCOMPATIBLE, + // same but only for functions / properties: error + COMPATIBLE_FOR_CLASSES_ONLY } private val possibleTargetMap = mapOf>( @@ -121,6 +130,9 @@ public object ModifierCheckerCore { // private is incompatible with override result += incompatibilityRegister(PRIVATE_KEYWORD, OVERRIDE_KEYWORD) + // private is compatible with open / abstract only for classes + result += compatibilityForClassesRegister(PRIVATE_KEYWORD, OPEN_KEYWORD) + result += compatibilityForClassesRegister(PRIVATE_KEYWORD, ABSTRACT_KEYWORD) return result } @@ -146,6 +158,9 @@ public object ModifierCheckerCore { return result } + private fun compatibilityForClassesRegister(vararg list: JetModifierKeywordToken) = + compatibilityRegister(Compatibility.COMPATIBLE_FOR_CLASSES_ONLY, *list) + private fun incompatibilityRegister(vararg list: JetModifierKeywordToken) = compatibilityRegister(Compatibility.INCOMPATIBLE, *list) private fun deprecationRegister(vararg list: JetModifierKeywordToken) = compatibilityRegister(Compatibility.DEPRECATED, *list) @@ -159,7 +174,11 @@ public object ModifierCheckerCore { } } - private fun checkCompatibility(trace: BindingTrace, firstNode: ASTNode, secondNode: ASTNode, incorrectNodes: MutableSet) { + private fun checkCompatibility(trace: BindingTrace, + firstNode: ASTNode, + secondNode: ASTNode, + owner: PsiElement, + incorrectNodes: MutableSet) { val first = firstNode.elementType as JetModifierKeywordToken val second = secondNode.elementType as JetModifierKeywordToken val compatibility = compatibility(first, second) @@ -176,7 +195,10 @@ public object ModifierCheckerCore { trace.report(Errors.DEPRECATED_MODIFIER_PAIR.on(firstNode.psi, first, second)) trace.report(Errors.DEPRECATED_MODIFIER_PAIR.on(secondNode.psi, second, first)) } - Compatibility.INCOMPATIBLE -> { + Compatibility.COMPATIBLE_FOR_CLASSES_ONLY, Compatibility.INCOMPATIBLE -> { + if (compatibility == Compatibility.COMPATIBLE_FOR_CLASSES_ONLY) { + if (owner is JetClassOrObject) return + } if (incorrectNodes.add(firstNode)) { trace.report(Errors.INCOMPATIBLE_MODIFIERS.on(firstNode.psi, first, second)) } @@ -234,7 +256,7 @@ public object ModifierCheckerCore { if (first == second) { break; } - checkCompatibility(trace, first, second, incorrectNodes) + checkCompatibility(trace, first, second, list.owner, incorrectNodes) } if (second !in incorrectNodes) { if (!checkTarget(trace, second, actualTargets)) { diff --git a/compiler/testData/diagnostics/tests/modifiers/IllegalModifiers.kt b/compiler/testData/diagnostics/tests/modifiers/IllegalModifiers.kt index 331ba1c8676..2b76219b0e1 100644 --- a/compiler/testData/diagnostics/tests/modifiers/IllegalModifiers.kt +++ b/compiler/testData/diagnostics/tests/modifiers/IllegalModifiers.kt @@ -40,7 +40,7 @@ class IllegalModifiers1( a: Int) //Check multiple illegal modifiers in constructor -class IllegalModifiers2(private abstract a: Int) +class IllegalModifiers2(private abstract a: Int) //Check annotations with illegal modifiers in constructor @@ -131,7 +131,7 @@ class IllegalModifiers8 { const constructor() {} - constructor(private enum abstract x: Int) {} + constructor(private enum abstract x: Int) {} } class IllegalModifiers9 { diff --git a/compiler/testData/diagnostics/tests/resolve/invoke/kt4204-completeNestedCallsForInvoke.kt b/compiler/testData/diagnostics/tests/resolve/invoke/kt4204-completeNestedCallsForInvoke.kt index 5fd22fd97d6..c7bed6c0bcb 100644 --- a/compiler/testData/diagnostics/tests/resolve/invoke/kt4204-completeNestedCallsForInvoke.kt +++ b/compiler/testData/diagnostics/tests/resolve/invoke/kt4204-completeNestedCallsForInvoke.kt @@ -11,7 +11,7 @@ public abstract class TestBug1() { position(m1().second) } - private abstract fun m1(): Pair + private abstract fun m1(): Pair private fun position(p: Int) {} diff --git a/compiler/testData/diagnostics/tests/scopes/VisibilityInheritModifier.kt b/compiler/testData/diagnostics/tests/scopes/VisibilityInheritModifier.kt index 2f86e1caab2..4f0097fbeb8 100644 --- a/compiler/testData/diagnostics/tests/scopes/VisibilityInheritModifier.kt +++ b/compiler/testData/diagnostics/tests/scopes/VisibilityInheritModifier.kt @@ -14,7 +14,7 @@ class C : A() { //------------ open class D { - private open fun self() : D = this + private open fun self() : D = this } class E : D() { diff --git a/idea/testData/codeInsight/overrideImplement/privateToThis.kt b/idea/testData/codeInsight/overrideImplement/privateToThis.kt deleted file mode 100644 index 420fb1b144c..00000000000 --- a/idea/testData/codeInsight/overrideImplement/privateToThis.kt +++ /dev/null @@ -1,7 +0,0 @@ -open class A { - private open fun foo(t: T) {} -} - -class B : A() { - -} \ No newline at end of file diff --git a/idea/testData/codeInsight/overrideImplement/privateToThis.kt.after b/idea/testData/codeInsight/overrideImplement/privateToThis.kt.after deleted file mode 100644 index bba7fbcbd31..00000000000 --- a/idea/testData/codeInsight/overrideImplement/privateToThis.kt.after +++ /dev/null @@ -1,17 +0,0 @@ -open class A { - private open fun foo(t: T) {} -} - -class B : A() { - override fun equals(other: Any?): Boolean { - return super.equals(other) - } - - override fun hashCode(): Int { - return super.hashCode() - } - - override fun toString(): String { - return super.toString() - } -} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/codeInsight/OverrideImplementTest.kt b/idea/tests/org/jetbrains/kotlin/idea/codeInsight/OverrideImplementTest.kt index d99c8e40f49..14a1d2e4b1f 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/codeInsight/OverrideImplementTest.kt +++ b/idea/tests/org/jetbrains/kotlin/idea/codeInsight/OverrideImplementTest.kt @@ -152,10 +152,6 @@ public class OverrideImplementTest : AbstractOverrideImplementTest() { doMultiOverrideDirectoryTest() } - public fun testPrivateToThis() { - doMultiOverrideFileTest() - } - public fun testInheritVisibilities() { doMultiOverrideFileTest() }