Skip to content

Commit

Permalink
Report invisible setter error if it's resolved to synthetic property …
Browse files Browse the repository at this point in the history
…of base class with public getter and protected setter

^KT-11713 Fixed
  • Loading branch information
petukhovv committed Sep 30, 2020
1 parent fdd71c0 commit fcfabb7
Show file tree
Hide file tree
Showing 21 changed files with 372 additions and 36 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Expand Up @@ -17,28 +17,47 @@
package org.jetbrains.kotlin.resolve.jvm.checkers

import com.intellij.psi.PsiElement
import org.jetbrains.kotlin.config.LanguageFeature
import org.jetbrains.kotlin.descriptors.DeclarationDescriptor
import org.jetbrains.kotlin.descriptors.DescriptorVisibilities
import org.jetbrains.kotlin.descriptors.DescriptorVisibility
import org.jetbrains.kotlin.descriptors.FunctionDescriptor
import org.jetbrains.kotlin.diagnostics.DiagnosticFactory3
import org.jetbrains.kotlin.diagnostics.Errors
import org.jetbrains.kotlin.psi.KtDotQualifiedExpression
import org.jetbrains.kotlin.resolve.calls.checkers.CallChecker
import org.jetbrains.kotlin.resolve.calls.checkers.CallCheckerContext
import org.jetbrains.kotlin.resolve.calls.context.CallPosition
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 {
fun computeSuitableDescriptorAndError(
descriptor: SyntheticJavaPropertyDescriptor,
reportOn: PsiElement,
context: CallCheckerContext
): Pair<FunctionDescriptor, DiagnosticFactory3<PsiElement, DeclarationDescriptor, DescriptorVisibility, DeclarationDescriptor>> {
val callPosition = context.resolutionContext.callPosition
val isLeftSide = callPosition is CallPosition.PropertyAssignment
&& (callPosition.leftPart as? KtDotQualifiedExpression)?.selectorExpression == reportOn
val getMethod = descriptor.getMethod
val setMethod = descriptor.setMethod
val isImprovingDiagnosticsEnabled =
context.languageVersionSettings.supportsFeature(LanguageFeature.ImproveReportingDiagnosticsOnProtectedMembersOfBaseClass)
val needToTakeSetter = isImprovingDiagnosticsEnabled && isLeftSide
val suitableDescriptor = if (needToTakeSetter && setMethod != null) setMethod else getMethod

return suitableDescriptor to if (needToTakeSetter && setMethod != null) Errors.INVISIBLE_SETTER else Errors.INVISIBLE_MEMBER
}

override fun check(resolvedCall: ResolvedCall<*>, reportOn: PsiElement, context: CallCheckerContext) {
val descriptor = resolvedCall.resultingDescriptor

val sourceFunction = when (descriptor) {
is SyntheticJavaPropertyDescriptor -> descriptor.getMethod
// TODO: this branch becomes unnecessary, because common checks are applied to SAM adapters being resolved as common members
// But this part may be still useful when we enable backward compatibility mode and SAM adapters become extensions again
is SamAdapterExtensionFunctionDescriptor -> descriptor.baseDescriptorForSynthetic
else -> return
}
if (descriptor !is SyntheticJavaPropertyDescriptor) return

val (sourceFunction, error) = computeSuitableDescriptorAndError(descriptor, reportOn, context)

val from = context.scope.ownerDescriptor

Expand All @@ -49,12 +68,14 @@ object ProtectedSyntheticExtensionCallChecker : CallChecker {

val receiverValue = resolvedCall.extensionReceiver as ReceiverValue
val receiverTypes = listOf(receiverValue.type) + context.dataFlowInfo.getStableTypes(
context.dataFlowValueFactory.createDataFlowValue(receiverValue, context.trace.bindingContext, context.scope.ownerDescriptor),
context.languageVersionSettings
context.dataFlowValueFactory.createDataFlowValue(
receiverValue, context.trace.bindingContext, context.scope.ownerDescriptor
),
context.languageVersionSettings
)

if (receiverTypes.none { DescriptorVisibilities.isVisible(getReceiverValueWithSmartCast(null, it), sourceFunction, from) }) {
context.trace.report(Errors.INVISIBLE_MEMBER.on(reportOn, descriptor, descriptor.visibility, from))
context.trace.report(error.on(reportOn, descriptor, descriptor.visibility, from))
}
}
}
Expand Up @@ -1033,7 +1033,7 @@ enum BadNamedArgumentsTarget {
DiagnosticFactory0<KtTypeParameterList> LOCAL_VARIABLE_WITH_TYPE_PARAMETERS_WARNING = DiagnosticFactory0.create(WARNING);
DiagnosticFactory0<KtTypeParameterList> LOCAL_VARIABLE_WITH_TYPE_PARAMETERS = DiagnosticFactory0.create(ERROR);

DiagnosticFactory3<KtExpression, DeclarationDescriptor, DescriptorVisibility, DeclarationDescriptor> INVISIBLE_SETTER = DiagnosticFactory3.create(ERROR);
DiagnosticFactory3<PsiElement, DeclarationDescriptor, DescriptorVisibility, DeclarationDescriptor> INVISIBLE_SETTER = DiagnosticFactory3.create(ERROR);

DiagnosticFactory1<PsiElement, KtKeywordToken> VAL_OR_VAR_ON_LOOP_PARAMETER = DiagnosticFactory1.create(ERROR);
DiagnosticFactory1<PsiElement, KtKeywordToken> VAL_OR_VAR_ON_FUN_PARAMETER = DiagnosticFactory1.create(ERROR);
Expand Down
Expand Up @@ -51,25 +51,23 @@ fun ResolutionContext<*>.reportTypeMismatchDueToTypeProjection(
): Boolean {
if (!TypeUtils.contains(expectedType) { it.isAnyOrNullableAny() || it.isNothing() || it.isNullableNothing() }) return false

val callPosition = this.callPosition
val (resolvedCall, correspondingNotApproximatedTypeByDescriptor: (CallableDescriptor) -> KotlinType?) = when (callPosition) {
is CallPosition.ValueArgumentPosition -> Pair(
callPosition.resolvedCall, { f: CallableDescriptor ->
is CallPosition.ValueArgumentPosition ->
callPosition.resolvedCall to { f: CallableDescriptor ->
getEffectiveExpectedType(f.valueParameters[callPosition.valueParameter.index], callPosition.valueArgument, this)
})
is CallPosition.ExtensionReceiverPosition -> Pair<ResolvedCall<*>, (CallableDescriptor) -> KotlinType?>(
callPosition.resolvedCall, { f: CallableDescriptor ->
f.extensionReceiverParameter?.type
})
is CallPosition.PropertyAssignment -> Pair<ResolvedCall<out CallableDescriptor>, (CallableDescriptor) -> KotlinType?>(
callPosition.leftPart.getResolvedCall(trace.bindingContext) ?: return false, { f: CallableDescriptor ->
(f as? PropertyDescriptor)?.setter?.valueParameters?.get(0)?.type
})
}
is CallPosition.ExtensionReceiverPosition ->
callPosition.resolvedCall to { f: CallableDescriptor -> f.extensionReceiverParameter?.type }
is CallPosition.PropertyAssignment -> {
if (callPosition.isLeft) return false
val resolvedCall = callPosition.leftPart.getResolvedCall(trace.bindingContext) ?: return false
resolvedCall to { f: CallableDescriptor -> (f as? PropertyDescriptor)?.setter?.valueParameters?.get(0)?.type }
}
is CallPosition.Unknown -> return false
}

val receiverType = resolvedCall.smartCastDispatchReceiverType
?: (resolvedCall.dispatchReceiver ?: return false).type
?: (resolvedCall.dispatchReceiver ?: return false).type

val callableDescriptor = resolvedCall.resultingDescriptor.original

Expand Down
Expand Up @@ -33,5 +33,5 @@ sealed class CallPosition {
val valueArgument: ValueArgument
) : CallPosition()

class PropertyAssignment(val leftPart: KtExpression?) : CallPosition()
class PropertyAssignment(val leftPart: KtExpression?, val isLeft: Boolean) : CallPosition()
}
Expand Up @@ -304,7 +304,7 @@ else if (assignmentOperationType != null &&
KotlinType expectedType = refineTypeFromPropertySetterIfPossible(context.trace.getBindingContext(), leftOperand, leftType);

components.dataFlowAnalyzer.checkType(binaryOperationType, expression, context.replaceExpectedType(expectedType)
.replaceDataFlowInfo(rightInfo.getDataFlowInfo()).replaceCallPosition(new CallPosition.PropertyAssignment(left)));
.replaceDataFlowInfo(rightInfo.getDataFlowInfo()).replaceCallPosition(new CallPosition.PropertyAssignment(left, false)));
basic.checkLValue(context.trace, context, leftOperand, right, expression, false);
}
temporary.commit();
Expand Down Expand Up @@ -354,14 +354,21 @@ protected KotlinTypeInfo visitAssignment(KtBinaryExpression expression, Expressi
basic.checkLValue(context.trace, context, arrayAccessExpression, right, expression, true);
return typeInfo.replaceType(checkAssignmentType(typeInfo.getType(), expression, contextWithExpectedType));
}
KotlinTypeInfo leftInfo = ExpressionTypingUtils.getTypeInfoOrNullType(left, context, facade);
KotlinTypeInfo leftInfo = ExpressionTypingUtils.getTypeInfoOrNullType(
left,
context.replaceCallPosition(new CallPosition.PropertyAssignment(left, true)),
facade
);
KotlinType expectedType = refineTypeFromPropertySetterIfPossible(context.trace.getBindingContext(), leftOperand, leftInfo.getType());
DataFlowInfo dataFlowInfo = leftInfo.getDataFlowInfo();
KotlinTypeInfo resultInfo;
if (right != null) {
resultInfo = facade.getTypeInfo(
right, context.replaceDataFlowInfo(dataFlowInfo).replaceExpectedType(expectedType).replaceCallPosition(
new CallPosition.PropertyAssignment(leftOperand)));
right,
context.replaceDataFlowInfo(dataFlowInfo)
.replaceExpectedType(expectedType)
.replaceCallPosition(new CallPosition.PropertyAssignment(leftOperand, false))
);

dataFlowInfo = resultInfo.getDataFlowInfo();
KotlinType rightType = resultInfo.getType();
Expand Down
Expand Up @@ -23,7 +23,7 @@ class B : A() {

a.<!INVISIBLE_MEMBER!>foo<!>
// TODO: should be INVISIBLE_SETTER
a.bar = a.bar + ""
a.<!INVISIBLE_SETTER!>bar<!> = a.bar + ""

if (a is B) {
<!DEBUG_INFO_SMARTCAST!>a<!>.foo
Expand All @@ -34,12 +34,12 @@ class B : A() {
d.x.abc // Ok
d.x.<!INVISIBLE_MEMBER!>foo<!>
// TODO: should be INVISIBLE_SETTER
d.x.bar = d.x.bar + ""
d.x.<!INVISIBLE_SETTER!>bar<!> = d.x.bar + ""
}
}
}

fun baz(a: A) {
a.<!INVISIBLE_MEMBER!>foo<!>
<!INVISIBLE_SETTER!>a.bar<!> = a.bar + ""
<!INVISIBLE_SETTER!>a.<!INVISIBLE_SETTER!>bar<!><!> = a.bar + ""
}
Expand Up @@ -17,7 +17,7 @@ class B : A() {
b.foo { }

if (a is B) {
<!DEBUG_INFO_SMARTCAST!>a<!>.foo {}
<!OI;DEBUG_INFO_SMARTCAST!>a<!>.<!NI;INVISIBLE_MEMBER!>foo<!> {}
}

if (d.x is B) {
Expand Down
Expand Up @@ -8,7 +8,7 @@ fun foo(javaClass: JavaClass) {
javaClass.<!INVISIBLE_MEMBER!>somethingProtected<!>
javaClass.<!UNRESOLVED_REFERENCE!>somethingPrivate<!>
javaClass.<!INVISIBLE_MEMBER!>somethingPackage<!>
<!INVISIBLE_SETTER!>javaClass.somethingPublic<!> = 1
<!INVISIBLE_SETTER!>javaClass.<!INVISIBLE_SETTER!>somethingPublic<!><!> = 1
}

// FILE: JavaClass.java
Expand Down
@@ -0,0 +1,29 @@
// !LANGUAGE: +ImproveReportingDiagnosticsOnProtectedMembersOfBaseClass

// FILE: abc/Foo.java
package abc;

public class Foo {
public String getBar() { return ""; }
protected void setBar(String x) { }
public String getFoo() { return ""; }
private void setFoo(String x) { }
}

// FILE: main.kt

import abc.Foo

class Data(var x: Foo)

class B : Foo() {
fun baz(a: Foo, t: Foo, d: Data) {
a.bar = t.bar
a.foo = t.foo

if (d.x is B) {
d.x.bar = d.x.bar + ""
d.x.foo = d.x.foo + ""
}
}
}
@@ -0,0 +1,29 @@
// !LANGUAGE: +ImproveReportingDiagnosticsOnProtectedMembersOfBaseClass

// FILE: abc/Foo.java
package abc;

public class Foo {
public String getBar() { return ""; }
protected void setBar(String x) { }
public String getFoo() { return ""; }
private void setFoo(String x) { }
}

// FILE: main.kt

import abc.Foo

class Data(var x: Foo)

class B : Foo() {
fun baz(a: Foo, t: Foo, d: Data) {
a.<!INVISIBLE_SETTER!>bar<!> = t.bar
<!VAL_REASSIGNMENT!>a.foo<!> = t.foo

if (d.x is B) {
d.x.<!INVISIBLE_SETTER!>bar<!> = d.x.bar + ""
d.x.foo = d.x.foo + ""
}
}
}
@@ -0,0 +1,35 @@
package

public final class B : abc.Foo {
public constructor B()
public final fun baz(/*0*/ a: abc.Foo, /*1*/ t: abc.Foo, /*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 getBar(): kotlin.String!
public open override /*1*/ /*fake_override*/ fun getFoo(): kotlin.String!
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
invisible_fake open override /*1*/ /*fake_override*/ fun setFoo(/*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.Foo)
public final var x: abc.Foo
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
}

package abc {

public open class Foo {
public constructor Foo()
public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
public open fun getBar(): kotlin.String!
public open fun getFoo(): kotlin.String!
public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
protected/*protected and package*/ open fun setBar(/*0*/ x: kotlin.String!): kotlin.Unit
private open fun setFoo(/*0*/ x: kotlin.String!): kotlin.Unit
public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
}
}
@@ -0,0 +1,29 @@
// !LANGUAGE: -ImproveReportingDiagnosticsOnProtectedMembersOfBaseClass

// FILE: abc/Foo.java
package abc;

public class Foo {
public String getBar() { return ""; }
protected void setBar(String x) { }
public String getFoo() { return ""; }
private void setFoo(String x) { }
}

// FILE: main.kt

import abc.Foo

class Data(var x: Foo)

class B : Foo() {
fun baz(a: Foo, t: Foo, d: Data) {
a.bar = t.bar
a.foo = t.foo

if (d.x is B) {
d.x.bar = d.x.bar + ""
d.x.foo = d.x.foo + ""
}
}
}
@@ -0,0 +1,29 @@
// !LANGUAGE: -ImproveReportingDiagnosticsOnProtectedMembersOfBaseClass

// FILE: abc/Foo.java
package abc;

public class Foo {
public String getBar() { return ""; }
protected void setBar(String x) { }
public String getFoo() { return ""; }
private void setFoo(String x) { }
}

// FILE: main.kt

import abc.Foo

class Data(var x: Foo)

class B : Foo() {
fun baz(a: Foo, t: Foo, d: Data) {
a.bar = t.bar
<!VAL_REASSIGNMENT!>a.foo<!> = t.foo

if (d.x is B) {
d.x.bar = d.x.bar + ""
d.x.foo = d.x.foo + ""
}
}
}

0 comments on commit fcfabb7

Please sign in to comment.