From bc1b34a9898d4e1de3cd614766b955343b2cc7fe Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Thu, 31 Mar 2016 12:26:14 +0300 Subject: [PATCH] Add additional visibility check for synthetic extensions Use extension receiver as dispatch one, because it is effectively dispatch (after some desugaring) --- .../ProtectedSyntheticExtensionCallChecker.kt | 55 +++++++++++++++++++ .../jvm/platform/JvmPlatformConfigurator.kt | 3 +- .../syntheticPropertyExtensions.kt | 45 +++++++++++++++ .../syntheticPropertyExtensions.txt | 23 ++++++++ .../syntheticSAMExtensions.kt | 30 ++++++++++ .../syntheticSAMExtensions.txt | 20 +++++++ .../samAdapters/Protected.kt | 2 +- .../checkers/DiagnosticsTestGenerated.java | 12 ++++ 8 files changed, 188 insertions(+), 2 deletions(-) create mode 100644 compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/ProtectedSyntheticExtensionCallChecker.kt create mode 100644 compiler/testData/diagnostics/tests/scopes/protectedVisibility/syntheticPropertyExtensions.kt create mode 100644 compiler/testData/diagnostics/tests/scopes/protectedVisibility/syntheticPropertyExtensions.txt create mode 100644 compiler/testData/diagnostics/tests/scopes/protectedVisibility/syntheticSAMExtensions.kt create mode 100644 compiler/testData/diagnostics/tests/scopes/protectedVisibility/syntheticSAMExtensions.txt diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/ProtectedSyntheticExtensionCallChecker.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/ProtectedSyntheticExtensionCallChecker.kt new file mode 100644 index 00000000000..044394f5cfe --- /dev/null +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/ProtectedSyntheticExtensionCallChecker.kt @@ -0,0 +1,55 @@ +/* + * 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.jvm.checkers + +import org.jetbrains.kotlin.descriptors.Visibilities +import org.jetbrains.kotlin.diagnostics.Errors +import org.jetbrains.kotlin.resolve.calls.checkers.CallChecker +import org.jetbrains.kotlin.resolve.calls.context.BasicCallResolutionContext +import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall +import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowValueFactory +import org.jetbrains.kotlin.resolve.calls.smartcasts.getReceiverValueWithSmartCast +import org.jetbrains.kotlin.resolve.scopes.receivers.ReceiverValue +import org.jetbrains.kotlin.synthetic.SamAdapterExtensionFunctionDescriptor +import org.jetbrains.kotlin.synthetic.SyntheticJavaPropertyDescriptor + +object ProtectedSyntheticExtensionCallChecker : CallChecker { + override fun check(resolvedCall: ResolvedCall<*>, context: BasicCallResolutionContext) { + val descriptor = resolvedCall.resultingDescriptor + + val sourceFunction = when (descriptor) { + is SyntheticJavaPropertyDescriptor -> descriptor.getMethod + is SamAdapterExtensionFunctionDescriptor -> descriptor.sourceFunction + else -> return + } + + val from = context.scope.ownerDescriptor + + // Already reported + if (!Visibilities.isVisibleIgnoringReceiver(descriptor, from)) return + + if (resolvedCall.dispatchReceiver != null && resolvedCall.extensionReceiver !is ReceiverValue) return + + val receiverValue = resolvedCall.extensionReceiver as ReceiverValue + val receiverTypes = listOf(receiverValue.type) + context.dataFlowInfo.getPredictableTypes( + DataFlowValueFactory.createDataFlowValue(receiverValue, context)) + + if (receiverTypes.none { Visibilities.isVisible(getReceiverValueWithSmartCast(null, it), sourceFunction, from) }) { + context.trace.report(Errors.INVISIBLE_MEMBER.on(resolvedCall.call.callElement, descriptor, descriptor.visibility, from)) + } + } +} diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JvmPlatformConfigurator.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JvmPlatformConfigurator.kt index 3c7c34874f9..0f8f8cf9cad 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JvmPlatformConfigurator.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JvmPlatformConfigurator.kt @@ -51,7 +51,8 @@ object JvmPlatformConfigurator : PlatformConfigurator( ProtectedInSuperClassCompanionCallChecker(), UnsupportedSyntheticCallableReferenceChecker(), SuperCallWithDefaultArgumentsChecker(), - MissingDependencyClassChecker() + MissingDependencyClassChecker(), + ProtectedSyntheticExtensionCallChecker ), additionalTypeCheckers = listOf( diff --git a/compiler/testData/diagnostics/tests/scopes/protectedVisibility/syntheticPropertyExtensions.kt b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/syntheticPropertyExtensions.kt new file mode 100644 index 00000000000..f4701d86ecd --- /dev/null +++ b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/syntheticPropertyExtensions.kt @@ -0,0 +1,45 @@ +// FILE: abc/A.java +package abc; +public class A { + public int getAbc() {} + protected int getFoo() { return 1; } + + public String getBar() { return ""; } + protected void setBar(String x) { } +} + +// FILE: main.kt +import abc.A + +class Data(var x: A) + +class B : A() { + fun baz(a: A, b: B, d: Data) { + foo + bar = bar + "" + + b.foo + b.bar = b.bar + "" + + a.foo + // TODO: should be INVISIBLE_SETTER + a.bar = a.bar + "" + + if (a is B) { + a.foo + a.bar = a.bar + "" + } + + if (d.x is B) { + d.x.abc // Ok + d.x.foo + // TODO: should be INVISIBLE_SETTER + d.x.bar = d.x.bar + "" + } + } +} + +fun baz(a: A) { + a.foo + a.bar = a.bar + "" +} diff --git a/compiler/testData/diagnostics/tests/scopes/protectedVisibility/syntheticPropertyExtensions.txt b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/syntheticPropertyExtensions.txt new file mode 100644 index 00000000000..426f3c553b5 --- /dev/null +++ b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/syntheticPropertyExtensions.txt @@ -0,0 +1,23 @@ +package + +public fun baz(/*0*/ a: abc.A): kotlin.Unit + +public final class B : abc.A { + public constructor B() + public final fun baz(/*0*/ a: abc.A, /*1*/ b: B, /*2*/ d: Data): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun getAbc(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun getBar(): kotlin.String! + protected/*protected and package*/ open override /*1*/ /*fake_override*/ fun getFoo(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + protected/*protected and package*/ open override /*1*/ /*fake_override*/ fun setBar(/*0*/ x: kotlin.String!): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class Data { + public constructor Data(/*0*/ x: abc.A) + public final var x: abc.A + 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 +} diff --git a/compiler/testData/diagnostics/tests/scopes/protectedVisibility/syntheticSAMExtensions.kt b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/syntheticSAMExtensions.kt new file mode 100644 index 00000000000..dafd94ed2c9 --- /dev/null +++ b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/syntheticSAMExtensions.kt @@ -0,0 +1,30 @@ +// FILE: abc/A.java +package abc; +public class A { + protected void foo(Runnable x) {} +} + +// FILE: main.kt +import abc.A; + +class Data(var x: A) + +class B : A() { + fun baz(a: A, b: B, d: Data) { + a.foo { } + + b.foo { } + + if (a is B) { + a.foo {} + } + + if (d.x is B) { + d.x.foo {} + } + } +} + +fun baz(a: A) { + a.foo { } +} diff --git a/compiler/testData/diagnostics/tests/scopes/protectedVisibility/syntheticSAMExtensions.txt b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/syntheticSAMExtensions.txt new file mode 100644 index 00000000000..b0fe21a1f2d --- /dev/null +++ b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/syntheticSAMExtensions.txt @@ -0,0 +1,20 @@ +package + +public fun baz(/*0*/ a: abc.A): kotlin.Unit + +public final class B : abc.A { + public constructor B() + public final fun baz(/*0*/ a: abc.A, /*1*/ b: B, /*2*/ d: Data): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + protected/*protected and package*/ open override /*1*/ /*fake_override*/ fun foo(/*0*/ x: java.lang.Runnable!): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class Data { + public constructor Data(/*0*/ x: abc.A) + public final var x: abc.A + 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 +} diff --git a/compiler/testData/diagnostics/tests/syntheticExtensions/samAdapters/Protected.kt b/compiler/testData/diagnostics/tests/syntheticExtensions/samAdapters/Protected.kt index eb50ccfc8bc..e5c45e0be05 100644 --- a/compiler/testData/diagnostics/tests/syntheticExtensions/samAdapters/Protected.kt +++ b/compiler/testData/diagnostics/tests/syntheticExtensions/samAdapters/Protected.kt @@ -12,7 +12,7 @@ fun foo(javaClass: JavaClass) { class X : JavaClass() { fun foo(other: JavaClass) { doSomething { bar() } - other.doSomething { bar() } // currently not flagged as error - see KT-8654 + other.doSomething { bar() } } } diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index c47ffec4e96..9a5d55758aa 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -15536,6 +15536,18 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { doTest(fileName); } + @TestMetadata("syntheticPropertyExtensions.kt") + public void testSyntheticPropertyExtensions() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/scopes/protectedVisibility/syntheticPropertyExtensions.kt"); + doTest(fileName); + } + + @TestMetadata("syntheticSAMExtensions.kt") + public void testSyntheticSAMExtensions() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/scopes/protectedVisibility/syntheticSAMExtensions.kt"); + doTest(fileName); + } + @TestMetadata("unstableSmartCast.kt") public void testUnstableSmartCast() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/scopes/protectedVisibility/unstableSmartCast.kt");