From 92be4aee9c64892ac857df3ba88699713b84e726 Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Mon, 28 Mar 2016 18:22:08 +0300 Subject: [PATCH] Prohibit protected constructor calls that are not super-calls in subtypes #KT-11649 Fixed --- .../jetbrains/kotlin/diagnostics/Errors.java | 2 + .../rendering/DefaultErrorMessages.java | 2 + .../kotlin/resolve/TargetPlatform.kt | 2 +- .../ProtectedConstructorCallChecker.kt | 59 +++++++++++++++++++ .../nonSuperCallConstructor.kt | 26 ++++++++ .../nonSuperCallConstructor.txt | 18 ++++++ ...uperCallConstructorJavaDifferentPackage.kt | 32 ++++++++++ ...perCallConstructorJavaDifferentPackage.txt | 9 +++ .../nonSuperCallConstructorJavaSamePackage.kt | 30 ++++++++++ ...nonSuperCallConstructorJavaSamePackage.txt | 18 ++++++ .../checkers/DiagnosticsTestGenerated.java | 18 ++++++ .../kotlin/descriptors/Visibilities.java | 14 ++++- 12 files changed, 228 insertions(+), 2 deletions(-) create mode 100644 compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/checkers/ProtectedConstructorCallChecker.kt create mode 100644 compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructor.kt create mode 100644 compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructor.txt create mode 100644 compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructorJavaDifferentPackage.kt create mode 100644 compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructorJavaDifferentPackage.txt create mode 100644 compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructorJavaSamePackage.kt create mode 100644 compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructorJavaSamePackage.txt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java index afc6aacc356..2a9358c2533 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java @@ -73,6 +73,8 @@ public interface Errors { DiagnosticFactory3.create(ERROR); DiagnosticFactory3 INVISIBLE_MEMBER = DiagnosticFactory3.create(ERROR, CALL_ELEMENT); + DiagnosticFactory1 PROTECTED_CONSTRUCTOR_NOT_IN_SUPER_CALL = DiagnosticFactory1.create(ERROR); + // Exposed visibility group DiagnosticFactory3 EXPOSED_PROPERTY_TYPE = DiagnosticFactory3.create(ERROR); DiagnosticFactory3 EXPOSED_FUNCTION_RETURN_TYPE = DiagnosticFactory3.create(ERROR); 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 310905bbe01..d65cea20f61 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java @@ -97,6 +97,8 @@ public class DefaultErrorMessages { MAP.put(INVISIBLE_REFERENCE, "Cannot access ''{0}'': it is ''{1}'' in {2}", NAME, TO_STRING, NAME_OF_PARENT_OR_FILE); MAP.put(INVISIBLE_MEMBER, "Cannot access ''{0}'': it is ''{1}'' in {2}", NAME, TO_STRING, NAME_OF_PARENT_OR_FILE); + MAP.put(PROTECTED_CONSTRUCTOR_NOT_IN_SUPER_CALL, "Protected constructor ''{0}'' from other classes can only be used in super-call", Renderers.SHORT_NAMES_IN_TYPES); + MAP.put(EXPOSED_PROPERTY_TYPE, "''{0}'' property exposes its ''{2}'' type{1}", TO_STRING, TO_STRING, TO_STRING); MAP.put(EXPOSED_FUNCTION_RETURN_TYPE, "''{0}'' function exposes its ''{2}'' return type{1}", TO_STRING, TO_STRING, TO_STRING); MAP.put(EXPOSED_PARAMETER_TYPE, "''{0}'' function exposes its ''{2}'' parameter type{1}", TO_STRING, TO_STRING, TO_STRING); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/TargetPlatform.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/TargetPlatform.kt index 0e642f63da7..72894fc5e91 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/TargetPlatform.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/TargetPlatform.kt @@ -69,7 +69,7 @@ private val DEFAULT_DECLARATION_CHECKERS = listOf( private val DEFAULT_CALL_CHECKERS = listOf(CapturingInClosureChecker(), InlineCheckerWrapper(), ReifiedTypeParameterSubstitutionChecker(), SafeCallChecker(), InvokeConventionChecker(), CallReturnsArrayOfNothingChecker(), - ConstructorHeaderCallChecker) + ConstructorHeaderCallChecker, ProtectedConstructorCallChecker) private val DEFAULT_TYPE_CHECKERS = emptyList() private val DEFAULT_VALIDATORS = listOf(DeprecatedSymbolValidator(), OperatorValidator(), InfixValidator()) diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/checkers/ProtectedConstructorCallChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/checkers/ProtectedConstructorCallChecker.kt new file mode 100644 index 00000000000..37df34e8fca --- /dev/null +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/checkers/ProtectedConstructorCallChecker.kt @@ -0,0 +1,59 @@ +/* + * Copyright 2010-2016 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jetbrains.kotlin.resolve.calls.checkers + +import org.jetbrains.kotlin.descriptors.ConstructorDescriptor +import org.jetbrains.kotlin.descriptors.Visibilities +import org.jetbrains.kotlin.diagnostics.Errors +import org.jetbrains.kotlin.psi.KtConstructorCalleeExpression +import org.jetbrains.kotlin.psi.KtConstructorDelegationReferenceExpression +import org.jetbrains.kotlin.psi.KtSuperTypeCallEntry +import org.jetbrains.kotlin.resolve.calls.context.BasicCallResolutionContext +import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall +import org.jetbrains.kotlin.resolve.descriptorUtil.parentsWithSelf + +object ProtectedConstructorCallChecker : CallChecker { + override fun check(resolvedCall: ResolvedCall<*>, context: BasicCallResolutionContext) { + val descriptor = resolvedCall.resultingDescriptor as? ConstructorDescriptor ?: return + val constructorOwner = descriptor.containingDeclaration.original + val scopeOwner = context.scope.ownerDescriptor + + if (descriptor.visibility.normalize() != Visibilities.PROTECTED) return + // Error already reported + if (!Visibilities.isVisibleWithAnyReceiver(descriptor, scopeOwner)) return + + val calleeExpression = resolvedCall.call.calleeExpression ?: return + + // Permit constructor super-calls + when (calleeExpression) { + is KtConstructorCalleeExpression -> if (calleeExpression.parent is KtSuperTypeCallEntry) return + is KtConstructorDelegationReferenceExpression -> return + } + + // Permit calls within class + if (scopeOwner.parentsWithSelf.any { it.original === constructorOwner }) return + + // Using FALSE_IF_PROTECTED helps us to check that descriptor doesn't meet conditions of java package/static-protected + // (i.e. being in the same package) + // And without ProtectedConstructorCallChecker such calls would be allowed only because they are performed within subclass + // of constructor owner + @Suppress("DEPRECATION") + if (Visibilities.findInvisibleMember(Visibilities.FALSE_IF_PROTECTED, descriptor, scopeOwner) == descriptor) { + context.trace.report(Errors.PROTECTED_CONSTRUCTOR_NOT_IN_SUPER_CALL.on(calleeExpression, descriptor)) + } + } +} diff --git a/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructor.kt b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructor.kt new file mode 100644 index 00000000000..65fb3936ae6 --- /dev/null +++ b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructor.kt @@ -0,0 +1,26 @@ +// !DIAGNOSTICS: -UNUSED_PARAMETER + +open class A protected constructor(x: Int) { + protected constructor() : this(1) + public constructor(x: Double) : this(3) +} + +class B4 : A(1) { + init { + A() + A(1) + A(5.0) + } + + fun foo() { + A() + A(1) + A(5.0) + + object : A() {} + object : A(1) {} + object : A(5.0) {} + + class Local : A() + } +} diff --git a/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructor.txt b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructor.txt new file mode 100644 index 00000000000..cb04e17675e --- /dev/null +++ b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructor.txt @@ -0,0 +1,18 @@ +package + +public open class A { + protected constructor A() + public constructor A(/*0*/ x: kotlin.Double) + protected constructor A(/*0*/ x: kotlin.Int) + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class B4 : A { + public constructor B4() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final 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/scopes/protectedVisibility/nonSuperCallConstructorJavaDifferentPackage.kt b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructorJavaDifferentPackage.kt new file mode 100644 index 00000000000..8aaef71defa --- /dev/null +++ b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructorJavaDifferentPackage.kt @@ -0,0 +1,32 @@ +// !DIAGNOSTICS: -UNUSED_PARAMETER + +// FILE: abc/A.java +package abc; +public class A { + protected A() {} + protected A(int x) {} + public A(double x) {} +} + +// FILE: main.kt +import abc.* + +class B4 : A(1) { + init { + A() + A(1) + A(5.0) + } + + fun foo() { + A() + A(1) + A(5.0) + + object : A() {} + object : A(1) {} + object : A(5.0) {} + + class Local : A() + } +} diff --git a/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructorJavaDifferentPackage.txt b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructorJavaDifferentPackage.txt new file mode 100644 index 00000000000..912922e2ac4 --- /dev/null +++ b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructorJavaDifferentPackage.txt @@ -0,0 +1,9 @@ +package + +public final class B4 : abc.A { + public constructor B4() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final 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/scopes/protectedVisibility/nonSuperCallConstructorJavaSamePackage.kt b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructorJavaSamePackage.kt new file mode 100644 index 00000000000..bf4789a773e --- /dev/null +++ b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructorJavaSamePackage.kt @@ -0,0 +1,30 @@ +// !DIAGNOSTICS: -UNUSED_PARAMETER + +// FILE: A.java +public class A { + protected A() {} + protected A(int x) {} + public A(double x) {} +} + +// FILE: main.kt + +class B4 : A(1) { + init { + A() + A(1) + A(5.0) + } + + fun foo() { + A() + A(1) + A(5.0) + + object : A() {} + object : A(1) {} + object : A(5.0) {} + + class Local : A() + } +} diff --git a/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructorJavaSamePackage.txt b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructorJavaSamePackage.txt new file mode 100644 index 00000000000..9ab5d4681fa --- /dev/null +++ b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructorJavaSamePackage.txt @@ -0,0 +1,18 @@ +package + +public open class A { + protected/*protected and package*/ constructor A() + public constructor A(/*0*/ x: kotlin.Double) + protected/*protected and package*/ constructor A(/*0*/ x: kotlin.Int) + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class B4 : A { + public constructor B4() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final 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/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index cb57d310cba..c47ffec4e96 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -15506,6 +15506,24 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { doTest(fileName); } + @TestMetadata("nonSuperCallConstructor.kt") + public void testNonSuperCallConstructor() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructor.kt"); + doTest(fileName); + } + + @TestMetadata("nonSuperCallConstructorJavaDifferentPackage.kt") + public void testNonSuperCallConstructorJavaDifferentPackage() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructorJavaDifferentPackage.kt"); + doTest(fileName); + } + + @TestMetadata("nonSuperCallConstructorJavaSamePackage.kt") + public void testNonSuperCallConstructorJavaSamePackage() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/scopes/protectedVisibility/nonSuperCallConstructorJavaSamePackage.kt"); + doTest(fileName); + } + @TestMetadata("protectedCallOnSubClass.kt") public void testProtectedCallOnSubClass() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/scopes/protectedVisibility/protectedCallOnSubClass.kt"); diff --git a/core/descriptors/src/org/jetbrains/kotlin/descriptors/Visibilities.java b/core/descriptors/src/org/jetbrains/kotlin/descriptors/Visibilities.java index afa807a830c..c9668e84d2f 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/descriptors/Visibilities.java +++ b/core/descriptors/src/org/jetbrains/kotlin/descriptors/Visibilities.java @@ -160,6 +160,9 @@ public class Visibilities { @NotNull DeclarationDescriptorWithVisibility whatDeclaration, @NotNull ClassDescriptor fromClass ) { + //noinspection deprecation + if (receiver == FALSE_IF_PROTECTED) return false; + // Do not check receiver for non-callable declarations if (!(whatDeclaration instanceof CallableMemberDescriptor)) return true; // Constructor accessibility check is performed manually @@ -282,7 +285,6 @@ public class Visibilities { return findInvisibleMember(IRRELEVANT_RECEIVER, what, from) == null; } - @Nullable public static DeclarationDescriptorWithVisibility findInvisibleMember( @Nullable ReceiverValue receiver, @@ -362,6 +364,16 @@ public class Visibilities { } }; + // This constant is not intended to use somewhere else from + @Deprecated + public static final ReceiverValue FALSE_IF_PROTECTED = new ReceiverValue() { + @NotNull + @Override + public KotlinType getType() { + throw new IllegalStateException("This method should not be called"); + } + }; + public static boolean isPrivate(@NotNull Visibility visibility) { return visibility == PRIVATE || visibility == PRIVATE_TO_THIS; }