From 9a69b8b7b50eb67d4fb6eea615d448ca13c6e85e Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Tue, 24 May 2016 11:46:47 +0300 Subject: [PATCH] REDUNDANT_OPEN_FOR_INTERFACE warning: reported for open members without implementation #KT-12452 Fixed --- .../jetbrains/kotlin/diagnostics/Errors.java | 2 ++ .../diagnostics/PositioningStrategies.kt | 2 ++ .../rendering/DefaultErrorMessages.java | 1 + .../kotlin/resolve/DeclarationsChecker.kt | 30 ++++++++++++------- .../Delegation_MultipleDelegates.kt | 4 +-- .../tests/modifiers/openInInterface.kt | 10 +++++++ .../tests/modifiers/openInInterface.txt | 13 ++++++++ .../diagnostics/tests/override/kt880.kt | 2 +- .../checkers/DiagnosticsTestGenerated.java | 6 ++++ .../kotlin/idea/quickfix/QuickFixRegistrar.kt | 1 + 10 files changed, 58 insertions(+), 13 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/modifiers/openInInterface.kt create mode 100644 compiler/testData/diagnostics/tests/modifiers/openInInterface.txt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java index b4e3abd8c45..d4a9221b934 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java @@ -239,6 +239,8 @@ public interface Errors { // Interface-specific + DiagnosticFactory0 REDUNDANT_OPEN_IN_INTERFACE = DiagnosticFactory0.create(WARNING, OPEN_MODIFIER); + DiagnosticFactory0 CONSTRUCTOR_IN_INTERFACE = DiagnosticFactory0.create(ERROR, DECLARATION_SIGNATURE); DiagnosticFactory0 METHOD_OF_ANY_IMPLEMENTED_IN_INTERFACE = DiagnosticFactory0.create(ERROR); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/PositioningStrategies.kt b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/PositioningStrategies.kt index b7f16c2dd15..19e22b7e2b3 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/PositioningStrategies.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/PositioningStrategies.kt @@ -195,6 +195,8 @@ object PositioningStrategies { @JvmField val ABSTRACT_MODIFIER: PositioningStrategy = modifierSetPosition(KtTokens.ABSTRACT_KEYWORD) + @JvmField val OPEN_MODIFIER: PositioningStrategy = modifierSetPosition(KtTokens.OPEN_KEYWORD) + @JvmField val OVERRIDE_MODIFIER: PositioningStrategy = modifierSetPosition(KtTokens.OVERRIDE_KEYWORD) @JvmField val PRIVATE_MODIFIER: PositioningStrategy = modifierSetPosition(KtTokens.PRIVATE_KEYWORD) diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java index d581cccc011..da4a6403458 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java @@ -158,6 +158,7 @@ public class DefaultErrorMessages { MAP.put(REDUNDANT_ANNOTATION_TARGET, "Redundant annotation target ''{0}''", STRING); MAP.put(REDUNDANT_MODIFIER, "Modifier ''{0}'' is redundant because ''{1}'' is present", TO_STRING, TO_STRING); + MAP.put(REDUNDANT_OPEN_IN_INTERFACE, "Modifier 'open' is redundant for abstract interface members"); MAP.put(REDUNDANT_MODIFIER_IN_GETTER, "Visibility modifiers are redundant in getter"); MAP.put(TYPE_PARAMETERS_IN_ENUM, "Enum class cannot have type parameters"); MAP.put(TYPECHECKER_HAS_RUN_INTO_RECURSIVE_PROBLEM, diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/DeclarationsChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/DeclarationsChecker.kt index 17f33e7c131..67d4d3e422d 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/DeclarationsChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/DeclarationsChecker.kt @@ -409,7 +409,7 @@ class DeclarationsChecker( private fun checkProperty(property: KtProperty, propertyDescriptor: PropertyDescriptor) { val containingDeclaration = propertyDescriptor.containingDeclaration if (containingDeclaration is ClassDescriptor) { - checkPropertyAbstractness(property, propertyDescriptor, containingDeclaration) + checkMemberProperty(property, propertyDescriptor, containingDeclaration) } checkPropertyLateInit(property, propertyDescriptor) checkPropertyInitializer(property, propertyDescriptor) @@ -506,17 +506,24 @@ class DeclarationsChecker( } } - private fun checkPropertyAbstractness( + private fun checkMemberProperty( property: KtProperty, propertyDescriptor: PropertyDescriptor, classDescriptor: ClassDescriptor) { val modifierList = property.modifierList - if (modifierList != null && modifierList.hasModifier(KtTokens.ABSTRACT_KEYWORD)) { - //has abstract modifier - if (!classCanHaveAbstractMembers(classDescriptor)) { - trace.report(ABSTRACT_PROPERTY_IN_NON_ABSTRACT_CLASS.on(property, property.name ?: "", classDescriptor)) - return + if (modifierList != null) { + if (modifierList.hasModifier(KtTokens.ABSTRACT_KEYWORD)) { + //has abstract modifier + if (!classCanHaveAbstractMembers(classDescriptor)) { + trace.report(ABSTRACT_PROPERTY_IN_NON_ABSTRACT_CLASS.on(property, property.name ?: "", classDescriptor)) + return + } + } + else if (classDescriptor.kind == ClassKind.INTERFACE && + modifierList.hasModifier(KtTokens.OPEN_KEYWORD) && + propertyDescriptor.modality == Modality.ABSTRACT) { + trace.report(REDUNDANT_OPEN_IN_INTERFACE.on(property)) } } @@ -607,7 +614,7 @@ class DeclarationsChecker( val hasExternalModifier = function.hasModifier(KtTokens.EXTERNAL_KEYWORD) if (containingDescriptor is ClassDescriptor) { - val inTrait = containingDescriptor.kind == ClassKind.INTERFACE + val inInterface = containingDescriptor.kind == ClassKind.INTERFACE if (hasAbstractModifier && !classCanHaveAbstractMembers(containingDescriptor)) { trace.report(ABSTRACT_FUNCTION_IN_NON_ABSTRACT_CLASS.on(function, functionDescriptor.name.asString(), containingDescriptor)) } @@ -615,12 +622,15 @@ class DeclarationsChecker( if (hasBody && hasAbstractModifier) { trace.report(ABSTRACT_FUNCTION_WITH_BODY.on(function, functionDescriptor)) } - if (!hasBody && inTrait) { + if (!hasBody && inInterface) { if (function.hasModifier(KtTokens.PRIVATE_KEYWORD)) { trace.report(PRIVATE_FUNCTION_WITH_NO_BODY.on(function, functionDescriptor)) } + if (!hasAbstractModifier && function.hasModifier(KtTokens.OPEN_KEYWORD)) { + trace.report(REDUNDANT_OPEN_IN_INTERFACE.on(function)) + } } - if (!hasBody && !hasAbstractModifier && !hasExternalModifier && !inTrait) { + if (!hasBody && !hasAbstractModifier && !hasExternalModifier && !inInterface) { trace.report(NON_ABSTRACT_FUNCTION_WITH_NO_BODY.on(function, functionDescriptor)) } } diff --git a/compiler/testData/diagnostics/tests/delegation/Delegation_MultipleDelegates.kt b/compiler/testData/diagnostics/tests/delegation/Delegation_MultipleDelegates.kt index 5955a9e68af..49639167e4e 100644 --- a/compiler/testData/diagnostics/tests/delegation/Delegation_MultipleDelegates.kt +++ b/compiler/testData/diagnostics/tests/delegation/Delegation_MultipleDelegates.kt @@ -1,10 +1,10 @@ // !DIAGNOSTICS: -CONFLICTING_JVM_DECLARATIONS interface One { - public open fun foo() : Int + public open fun foo() : Int private fun boo() = 10 } interface Two { - public open fun foo() : Int + public open fun foo() : Int } interface OneImpl : One { diff --git a/compiler/testData/diagnostics/tests/modifiers/openInInterface.kt b/compiler/testData/diagnostics/tests/modifiers/openInInterface.kt new file mode 100644 index 00000000000..b2e1578e77c --- /dev/null +++ b/compiler/testData/diagnostics/tests/modifiers/openInInterface.kt @@ -0,0 +1,10 @@ +interface My { + open fun foo() + open fun bar() {} + open abstract fun baz(): Int + + open val x: Int + open val y: String + get() = "" + open abstract val z: Double +} diff --git a/compiler/testData/diagnostics/tests/modifiers/openInInterface.txt b/compiler/testData/diagnostics/tests/modifiers/openInInterface.txt new file mode 100644 index 00000000000..c593c97f3e1 --- /dev/null +++ b/compiler/testData/diagnostics/tests/modifiers/openInInterface.txt @@ -0,0 +1,13 @@ +package + +public interface My { + public abstract val x: kotlin.Int + public open val y: kotlin.String + public abstract val z: kotlin.Double + public open fun bar(): kotlin.Unit + public abstract fun baz(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public abstract fun foo(): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/testData/diagnostics/tests/override/kt880.kt b/compiler/testData/diagnostics/tests/override/kt880.kt index 814e808b9fe..5b6b9042761 100644 --- a/compiler/testData/diagnostics/tests/override/kt880.kt +++ b/compiler/testData/diagnostics/tests/override/kt880.kt @@ -1,7 +1,7 @@ // KT-880 Overload resolution ambiguity public interface I { - open fun test() : Unit + open fun test() : Unit } abstract public class A() { diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index aafbfad6166..7c7c993b746 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -10986,6 +10986,12 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { doTest(fileName); } + @TestMetadata("openInInterface.kt") + public void testOpenInInterface() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/modifiers/openInInterface.kt"); + doTest(fileName); + } + @TestMetadata("primaryConstructorMissingKeyword.kt") public void testPrimaryConstructorMissingKeyword() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/modifiers/primaryConstructorMissingKeyword.kt"); diff --git a/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt b/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt index 2c69b3faeee..72e73a75736 100644 --- a/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt +++ b/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt @@ -101,6 +101,7 @@ class QuickFixRegistrar : QuickFixContributor { val removeRedundantModifierFactory = RemoveModifierFix.createRemoveModifierFactory(true) REDUNDANT_MODIFIER.registerFactory(removeRedundantModifierFactory) + REDUNDANT_OPEN_IN_INTERFACE.registerFactory(removeRedundantModifierFactory) REDUNDANT_PROJECTION.registerFactory(RemoveModifierFix.createRemoveProjectionFactory(true)) INCOMPATIBLE_MODIFIERS.registerFactory(RemoveModifierFix.createRemoveModifierFactory(false))