Check backing field availability for AnnotationTarget.FIELD #KT-10387 Fixed

Some duplicated checks deleted (UseSiteTargetChecker / JvmFieldApplicabilityChecker)
This commit is contained in:
Mikhail Glukhikh
2015-12-15 14:13:36 +03:00
parent f55574df36
commit 3fb04aceb9
13 changed files with 154 additions and 54 deletions
@@ -32,12 +32,10 @@ import org.jetbrains.kotlin.resolve.jvm.diagnostics.ErrorsJvm
class JvmFieldApplicabilityChecker : DeclarationChecker {
enum class Problem(val errorMessage: String) {
NOT_A_PROPERTY("JvmField can only be applied to a property"),
internal enum class Problem(val errorMessage: String) {
NOT_FINAL("JvmField can only be applied to final property"),
PRIVATE("JvmField has no effect on a private property"),
CUSTOM_ACCESSOR("JvmField cannot be applied to a property with a custom accessor"),
NO_BACKING_FIELD("JvmField can only be applied to a property with backing field"),
OVERRIDES("JvmField cannot be applied to a property that overrides some other property"),
LATEINIT("JvmField cannot be applied to lateinit property"),
CONST("JvmField cannot be applied to const property"),
@@ -54,10 +52,11 @@ class JvmFieldApplicabilityChecker : DeclarationChecker {
val annotation = descriptor.findJvmFieldAnnotation() ?: return
val problem = when {
descriptor !is PropertyDescriptor -> NOT_A_PROPERTY
// First two cases just prevent duplication of WRONG_ANNOTATION_TARGET
descriptor !is PropertyDescriptor -> return
!descriptor.hasBackingField(bindingContext) -> return
descriptor.isOverridable -> NOT_FINAL
Visibilities.isPrivate(descriptor.visibility) -> PRIVATE
!descriptor.hasBackingField(bindingContext) -> NO_BACKING_FIELD
descriptor.hasCustomAccessor() -> CUSTOM_ACCESSOR
descriptor.overriddenDescriptors.isNotEmpty() -> OVERRIDES
descriptor.isLateInit -> LATEINIT
@@ -19,6 +19,7 @@ package org.jetbrains.kotlin.resolve
import org.jetbrains.kotlin.builtins.KotlinBuiltIns
import org.jetbrains.kotlin.descriptors.ClassDescriptor
import org.jetbrains.kotlin.descriptors.DeclarationDescriptor
import org.jetbrains.kotlin.descriptors.PropertyDescriptor
import org.jetbrains.kotlin.descriptors.annotations.AnnotationDescriptor
import org.jetbrains.kotlin.descriptors.annotations.AnnotationUseSiteTarget
import org.jetbrains.kotlin.descriptors.annotations.KotlinRetention
@@ -39,7 +40,7 @@ import org.jetbrains.kotlin.types.expressions.ExpressionTypingUtils
public class AnnotationChecker(private val additionalCheckers: Iterable<AdditionalAnnotationChecker>) {
public fun check(annotated: KtAnnotated, trace: BindingTrace, descriptor: DeclarationDescriptor? = null) {
val actualTargets = getActualTargetList(annotated, descriptor)
val actualTargets = getActualTargetList(annotated, descriptor, trace)
checkEntries(annotated.annotationEntries, actualTargets, trace)
if (annotated is KtCallableDeclaration) {
annotated.typeReference?.let { check(it, trace) }
@@ -66,7 +67,7 @@ public class AnnotationChecker(private val additionalCheckers: Iterable<Addition
}
public fun checkExpression(expression: KtExpression, trace: BindingTrace) {
checkEntries(expression.getAnnotationEntries(), getActualTargetList(expression, null), trace)
checkEntries(expression.getAnnotationEntries(), getActualTargetList(expression, null, trace), trace)
if (expression is KtLambdaExpression) {
for (parameter in expression.valueParameters) {
parameter.typeReference?.let { check(it, trace) }
@@ -162,11 +163,15 @@ public class AnnotationChecker(private val additionalCheckers: Iterable<Addition
}.toSet()
}
public fun getDeclarationSiteActualTargetList(annotated: KtElement, descriptor: ClassDescriptor?): List<KotlinTarget> {
return getActualTargetList(annotated, descriptor).defaultTargets
public fun getDeclarationSiteActualTargetList(annotated: KtElement, descriptor: ClassDescriptor?, trace: BindingTrace):
List<KotlinTarget> {
return getActualTargetList(annotated, descriptor, trace).defaultTargets
}
private fun getActualTargetList(annotated: KtElement, descriptor: DeclarationDescriptor?): TargetList {
private fun DeclarationDescriptor?.hasBackingField(bindingTrace: BindingTrace)
= (this as? PropertyDescriptor)?.let { bindingTrace.get(BindingContext.BACKING_FIELD_REQUIRED, it) } ?: false
private fun getActualTargetList(annotated: KtElement, descriptor: DeclarationDescriptor?, trace: BindingTrace): TargetList {
return when (annotated) {
is KtClassOrObject ->
(descriptor as? ClassDescriptor)?.let { TargetList(KotlinTarget.classActualTargets(it)) } ?: TargetLists.T_CLASSIFIER
@@ -175,9 +180,9 @@ public class AnnotationChecker(private val additionalCheckers: Iterable<Addition
if (annotated.isLocal)
TargetLists.T_LOCAL_VARIABLE
else if (annotated.parent is KtClassOrObject || annotated.parent is KtClassBody)
TargetLists.T_MEMBER_PROPERTY
TargetLists.T_MEMBER_PROPERTY(descriptor.hasBackingField(trace))
else
TargetLists.T_TOP_LEVEL_PROPERTY
TargetLists.T_TOP_LEVEL_PROPERTY(descriptor.hasBackingField(trace))
}
is KtParameter -> {
if (annotated.hasValOrVar())
@@ -220,13 +225,17 @@ public class AnnotationChecker(private val additionalCheckers: Iterable<Addition
val T_DESTRUCTURING_DECLARATION = targetList(DESTRUCTURING_DECLARATION)
val T_MEMBER_PROPERTY = targetList(MEMBER_PROPERTY, PROPERTY) {
extraTargets(FIELD)
fun T_MEMBER_PROPERTY(backingField: Boolean) =
targetList(if (backingField) MEMBER_PROPERTY_WITH_FIELD else MEMBER_PROPERTY_WITHOUT_FIELD,
MEMBER_PROPERTY, PROPERTY) {
if (backingField) extraTargets(FIELD)
onlyWithUseSiteTarget(VALUE_PARAMETER, PROPERTY_GETTER, PROPERTY_SETTER)
}
val T_TOP_LEVEL_PROPERTY = targetList(TOP_LEVEL_PROPERTY, PROPERTY) {
extraTargets(FIELD)
fun T_TOP_LEVEL_PROPERTY(backingField: Boolean) =
targetList(if (backingField) TOP_LEVEL_PROPERTY_WITH_FIELD else TOP_LEVEL_PROPERTY_WITHOUT_FIELD,
TOP_LEVEL_PROPERTY, PROPERTY) {
if (backingField) extraTargets(FIELD)
onlyWithUseSiteTarget(VALUE_PARAMETER, PROPERTY_GETTER, PROPERTY_SETTER)
}
@@ -62,7 +62,7 @@ public object AnnotationUseSiteTargetChecker {
val target = annotation.useSiteTarget?.getAnnotationUseSiteTarget() ?: continue
when (target) {
AnnotationUseSiteTarget.FIELD -> checkFieldTargetApplicability(annotated, annotation, descriptor)
AnnotationUseSiteTarget.FIELD -> checkIfProperty(annotated, annotation)
AnnotationUseSiteTarget.PROPERTY -> checkIfProperty(annotated, annotation)
AnnotationUseSiteTarget.PROPERTY_GETTER -> checkIfProperty(annotated, annotation)
AnnotationUseSiteTarget.PROPERTY_SETTER -> checkIfMutableProperty(annotated, annotation)
@@ -87,20 +87,6 @@ public object AnnotationUseSiteTargetChecker {
}
}
private fun BindingTrace.checkFieldTargetApplicability(
annotated: KtAnnotated,
annotation: KtAnnotationEntry,
descriptor: DeclarationDescriptor
) {
if (!checkIfProperty(annotated, annotation)) return
if (annotated is KtProperty && descriptor is PropertyDescriptor) {
if (!annotated.hasDelegate() && !(bindingContext.get(BindingContext.BACKING_FIELD_REQUIRED, descriptor) ?: false)) {
report(INAPPLICABLE_FIELD_TARGET_NO_BACKING_FIELD.on(annotation))
}
}
}
private fun BindingTrace.checkIfMutableProperty(annotated: KtAnnotated, annotation: KtAnnotationEntry) {
if (!checkIfProperty(annotated, annotation)) return
@@ -295,7 +295,7 @@ public object ModifierCheckerCore {
}
}
}
val actualTargets = AnnotationChecker.getDeclarationSiteActualTargetList(listOwner, descriptor as? ClassDescriptor)
val actualTargets = AnnotationChecker.getDeclarationSiteActualTargetList(listOwner, descriptor as? ClassDescriptor, trace)
val list = listOwner.modifierList ?: return
checkModifierList(list, trace, descriptor?.containingDeclaration, actualTargets)
}
@@ -29,10 +29,6 @@ public class A(
@Target(AnnotationTarget.FIELD)
annotation class Anno
@Anno
val p: Int
get() = 5
@Anno
val p2: Int = 4
get() = field
@@ -55,6 +55,5 @@ public abstract class Anno
public final class DefaultTargetsKt {
private final static @Anno field p2: int
static method <clinit>(): void
public final static method getP(): int
public final static method getP2(): int
}
@@ -0,0 +1,44 @@
@Target(AnnotationTarget.FIELD)
annotation class Field
<!WRONG_ANNOTATION_TARGET!>@Field<!>
annotation class Another
@Field
val x: Int = 42
<!WRONG_ANNOTATION_TARGET!>@Field<!>
val y: Int
get() = 13
<!WRONG_ANNOTATION_TARGET!>@Field<!>
abstract class My(<!WRONG_ANNOTATION_TARGET!>@Field<!> arg: Int, @Field val w: Int) {
@Field
val x: Int = arg
<!WRONG_ANNOTATION_TARGET!>@Field<!>
val y: Int
get() = 0
<!WRONG_ANNOTATION_TARGET!>@Field<!>
abstract val z: Int
<!WRONG_ANNOTATION_TARGET!>@Field<!>
fun foo() {}
<!WRONG_ANNOTATION_TARGET!>@Field<!>
val v: Int by <!UNRESOLVED_REFERENCE!>Delegates<!>.<!DEBUG_INFO_ELEMENT_WITH_ERROR_TYPE!>lazy<!> { 42 }
}
enum class Your {
@Field FIRST
}
interface His {
<!WRONG_ANNOTATION_TARGET!>@Field<!>
val x: Int
<!WRONG_ANNOTATION_TARGET!>@Field<!>
val y: Int
get() = 42
}
@@ -0,0 +1,57 @@
package
@Field() public val x: kotlin.Int = 42
@Field() public val y: kotlin.Int
@Field() public final annotation class Another : kotlin.Annotation {
public constructor Another()
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
}
@kotlin.annotation.Target(allowedTargets = {AnnotationTarget.FIELD}) public final annotation class Field : kotlin.Annotation {
public constructor Field()
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 interface His {
@Field() public abstract val x: kotlin.Int
@Field() public open val y: 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
}
@Field() public abstract class My {
public constructor My(/*0*/ @Field() arg: kotlin.Int, /*1*/ w: kotlin.Int)
@Field() public final val v: kotlin.Int
@Field() public final val w: kotlin.Int
@Field() public final val x: kotlin.Int
@Field() public final val y: kotlin.Int
@Field() public abstract val z: kotlin.Int
public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
@Field() 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
}
public final enum class Your : kotlin.Enum<Your> {
@Field() enum entry FIRST
private constructor Your()
public final override /*1*/ /*fake_override*/ val name: kotlin.String
public final override /*1*/ /*fake_override*/ val ordinal: kotlin.Int
protected final override /*1*/ /*fake_override*/ fun clone(): kotlin.Any
public final override /*1*/ /*fake_override*/ fun compareTo(/*0*/ other: Your): kotlin.Int
public final override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
public final override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
// Static members
@kotlin.Deprecated(level = DeprecationLevel.ERROR, message = "Use 'values()' function instead", replaceWith = kotlin.ReplaceWith(expression = "this.values()", imports = {})) public final /*synthesized*/ val values: kotlin.Array<Your>
public final /*synthesized*/ fun valueOf(/*0*/ value: kotlin.String): Your
public final /*synthesized*/ fun values(): kotlin.Array<Your>
}
@@ -18,10 +18,10 @@ class SomeClass {
@field:[Ann]
protected val simplePropertyWithAnnotationList: String = "text"
@field:Ann
<!WRONG_ANNOTATION_TARGET_WITH_USE_SITE_TARGET!>@field:Ann<!>
protected val delegatedProperty: String by CustomDelegate()
<!INAPPLICABLE_FIELD_TARGET_NO_BACKING_FIELD!>@field:Ann<!>
<!WRONG_ANNOTATION_TARGET_WITH_USE_SITE_TARGET!>@field:Ann<!>
val propertyWithCustomGetter: Int
get() = 5
@@ -1,23 +1,23 @@
// !DIAGNOSTICS: -UNUSED_PARAMETER -UNUSED_VARIABLE
<!INAPPLICABLE_JVM_FIELD, WRONG_ANNOTATION_TARGET!>@kotlin.jvm.JvmField<!>
<!WRONG_ANNOTATION_TARGET!>@kotlin.jvm.JvmField<!>
fun foo() {
<!INAPPLICABLE_JVM_FIELD, WRONG_ANNOTATION_TARGET!>@kotlin.jvm.JvmField<!> val x = "A"
<!WRONG_ANNOTATION_TARGET!>@kotlin.jvm.JvmField<!> val x = "A"
}
<!INAPPLICABLE_JVM_FIELD, WRONG_ANNOTATION_TARGET!>@JvmField<!>
<!WRONG_ANNOTATION_TARGET!>@JvmField<!>
abstract class C : I{
<!INAPPLICABLE_JVM_FIELD, WRONG_ANNOTATION_TARGET!>@kotlin.jvm.JvmField<!> constructor(s: String) {
<!WRONG_ANNOTATION_TARGET!>@kotlin.jvm.JvmField<!> constructor(s: String) {
}
<!INAPPLICABLE_JVM_FIELD, WRONG_ANNOTATION_TARGET!>@kotlin.jvm.JvmField<!> private fun foo(s: String = "OK") {
<!WRONG_ANNOTATION_TARGET!>@kotlin.jvm.JvmField<!> private fun foo(s: String = "OK") {
}
<!INAPPLICABLE_JVM_FIELD!>@JvmField<!> val a: String by lazy { "A" }
<!WRONG_ANNOTATION_TARGET!>@JvmField<!> val a: String by lazy { "A" }
<!INAPPLICABLE_JVM_FIELD!>@JvmField<!> open val b: Int = 3
<!INAPPLICABLE_JVM_FIELD!>@JvmField<!> abstract val c: Int
<!WRONG_ANNOTATION_TARGET!>@JvmField<!> abstract val c: Int
<!INAPPLICABLE_JVM_FIELD!>@JvmField<!>
val customGetter: String = ""
@@ -29,7 +29,7 @@ abstract class C : I{
field = s
}
<!INAPPLICABLE_JVM_FIELD!>@JvmField<!>
<!WRONG_ANNOTATION_TARGET!>@JvmField<!>
val noBackingField: String
get() = "a"
@@ -41,8 +41,8 @@ abstract class C : I{
}
interface I {
<!INAPPLICABLE_JVM_FIELD!>@JvmField<!> val ai: Int
<!INAPPLICABLE_JVM_FIELD!>@JvmField<!> val bi: Int
<!WRONG_ANNOTATION_TARGET!>@JvmField<!> val ai: Int
<!WRONG_ANNOTATION_TARGET!>@JvmField<!> val bi: Int
get() = 5
}
@@ -1300,6 +1300,12 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest {
doTest(fileName);
}
@TestMetadata("field.kt")
public void testField() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/annotations/options/targets/field.kt");
doTest(fileName);
}
@TestMetadata("file.kt")
public void testFile() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/annotations/options/targets/file.kt");
@@ -27,8 +27,8 @@ public enum class KotlinTarget(val description: String, val isDefault: Boolean =
CLASS("class"), // includes CLASS_ONLY, OBJECT, COMPANION_OBJECT, OBJECT_LITERAL, INTERFACE, *_CLASS but not ENUM_ENTRY
ANNOTATION_CLASS("annotation class"),
TYPE_PARAMETER("type parameter", false),
PROPERTY("property"), // includes *_PROPERTY, PROPERTY_PARAMETER, ENUM_ENTRY
FIELD("field"),
PROPERTY("property"), // includes *_PROPERTY (with and without backing field), PROPERTY_PARAMETER, ENUM_ENTRY
FIELD("field"), // includes MEMBER_PROPERTY_WITH_FIELD, TOP_LEVEL_PROPERTY_WITH_FIELD, PROPERTY_PARAMETER, ENUM_ENTRY
LOCAL_VARIABLE("local variable"),
VALUE_PARAMETER("value parameter"),
CONSTRUCTOR("constructor"),
@@ -57,8 +57,12 @@ public enum class KotlinTarget(val description: String, val isDefault: Boolean =
MEMBER_FUNCTION("member function", false),
TOP_LEVEL_FUNCTION("top level function", false),
MEMBER_PROPERTY("member property", false), // includes PROPERTY_PARAMETER
TOP_LEVEL_PROPERTY("top level property", false),
MEMBER_PROPERTY("member property", false), // includes PROPERTY_PARAMETER, with and without field
MEMBER_PROPERTY_WITH_FIELD("member property with backing field", false),
MEMBER_PROPERTY_WITHOUT_FIELD("member property without backing field", false),
TOP_LEVEL_PROPERTY("top level property", false), // with and without field
TOP_LEVEL_PROPERTY_WITH_FIELD("top level property with backing field", false),
TOP_LEVEL_PROPERTY_WITHOUT_FIELD("top level property without backing field", false),
INITIALIZER("initializer", false),
DESTRUCTURING_DECLARATION("destructuring declaration", false),
@@ -1,4 +1,4 @@
// ERROR: This annotation is not applicable to target 'member property'
// ERROR: This annotation is not applicable to target 'member property with backing field'
internal class A {
@Deprecated("")
@Volatile var field1 = 0