From cb7f7b1de5c4c3506559c81dc6e10abc01b8f646 Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Fri, 23 Nov 2018 17:02:41 +0300 Subject: [PATCH] Report "use of non-const Kotlin property" also on some assignments #KT-25536 Fixed --- .../FakeJvmFieldConstantInspection.kt | 38 +++++++++++++++- .../before/First/src/JavaUser.java | 20 +++++++++ .../before/First/src/KotlinProperties.kt | 6 ++- .../fakeJvmFieldConstant/expected.xml | 45 +++++++++++++++++++ 4 files changed, 106 insertions(+), 3 deletions(-) diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/FakeJvmFieldConstantInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/FakeJvmFieldConstantInspection.kt index 92829a6cb44..6c149027800 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/FakeJvmFieldConstantInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/FakeJvmFieldConstantInspection.kt @@ -25,6 +25,7 @@ import org.jetbrains.kotlin.asJava.elements.KtLightFieldImpl.KtLightFieldForDecl import org.jetbrains.kotlin.idea.inspections.MayBeConstantInspection.Status.* import org.jetbrains.kotlin.idea.quickfix.AddConstModifierFix import org.jetbrains.kotlin.psi.KtProperty +import org.jetbrains.kotlin.psi.psiUtil.forEachDescendantOfType class FakeJvmFieldConstantInspection : AbstractKotlinInspection() { override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor { @@ -44,19 +45,52 @@ class FakeJvmFieldConstantInspection : AbstractKotlinInspection() { val valueExpression = statement.caseValue ?: return checkExpression(valueExpression, holder) } + + override fun visitAssignmentExpression(expression: PsiAssignmentExpression) { + super.visitAssignmentExpression(expression) + + if (expression.operationTokenType != JavaTokenType.EQ) return + val leftType = expression.lExpression.type as? PsiPrimitiveType ?: return + checkAssignmentChildren(expression.rExpression ?: return, leftType, holder) + } + + override fun visitVariable(variable: PsiVariable) { + super.visitVariable(variable) + + val leftType = variable.type as? PsiPrimitiveType ?: return + val initializer = variable.initializer ?: return + checkAssignmentChildren(initializer, leftType, holder) + } } } - private fun checkExpression(valueExpression: PsiExpression, holder: ProblemsHolder) { + private fun checkAssignmentChildren(right: PsiExpression, leftType: PsiPrimitiveType, holder: ProblemsHolder) { + if (leftType == PsiType.BOOLEAN || leftType == PsiType.CHAR || leftType == PsiType.VOID) return + right.forEachDescendantOfType(canGoInside = { parentElement -> + parentElement !is PsiCallExpression && parentElement !is PsiTypeCastExpression + }) { rightPart -> + checkExpression(rightPart, holder) { resolvedPropertyType -> + leftType != resolvedPropertyType && !leftType.isAssignableFrom(resolvedPropertyType) + } + } + } + + private fun checkExpression( + valueExpression: PsiExpression, + holder: ProblemsHolder, + additionalTypeCheck: (PsiType) -> Boolean = { true } + ) { val resolvedLightField = (valueExpression as? PsiReference)?.resolve() as? KtLightFieldForDeclaration ?: return val resolvedProperty = resolvedLightField.kotlinOrigin as? KtProperty ?: return with(MayBeConstantInspection) { - if (resolvedProperty.annotationEntries.isEmpty()) return@with + if (resolvedProperty.annotationEntries.isEmpty()) return val resolvedPropertyStatus = resolvedProperty.getStatus() if (resolvedPropertyStatus == JVM_FIELD_MIGHT_BE_CONST || resolvedPropertyStatus == JVM_FIELD_MIGHT_BE_CONST_NO_INITIALIZER || resolvedPropertyStatus == JVM_FIELD_MIGHT_BE_CONST_ERRONEOUS ) { + val resolvedPropertyType = resolvedLightField.type + if (!additionalTypeCheck(resolvedPropertyType)) return val fixes = mutableListOf() if (resolvedPropertyStatus == JVM_FIELD_MIGHT_BE_CONST) { fixes += IntentionWrapper(AddConstModifierFix(resolvedProperty), resolvedProperty.containingFile) diff --git a/idea/testData/multiFileInspections/fakeJvmFieldConstant/before/First/src/JavaUser.java b/idea/testData/multiFileInspections/fakeJvmFieldConstant/before/First/src/JavaUser.java index 0507ce3cceb..914e5938881 100644 --- a/idea/testData/multiFileInspections/fakeJvmFieldConstant/before/First/src/JavaUser.java +++ b/idea/testData/multiFileInspections/fakeJvmFieldConstant/before/First/src/JavaUser.java @@ -21,4 +21,24 @@ public class JavaUser { break; } } + + public void fau() { + byte b1 = KotlinPropertiesKt.importantNumber; // Dangerous + byte b2 = KotlinPropertiesKt.constantNumber; // Safe (const) + byte b3 = 0 + KotlinPropertiesKt.importantNumber; // Dangerous + int b4 = 0 + KotlinPropertiesKt.importantNumber; // Safe (types are the same) + b4 += KotlinPropertiesKt.importantNumber; // Safe (modification) + long l1 = KotlinPropertiesKt.importantNumber; // Safe (type widening) + b2 = KotlinPropertiesKt.importantNumber; // Dangerous + b1 = KotlinPropertiesKt.constantNumber; // Safe (const) + b3 = (0 + KotlinPropertiesKt.importantNumber); // Dangerous + b3 = (byte) (KotlinPropertiesKt.importantNumber); // Safe (explicit cast) + b3 = convert(KotlinPropertiesKt.importantNumber); // Safe (explicit cast) + } + + public byte convert(int x) { + return (byte) x; + } + + byte importantNumber = KotlinPropertiesKt.importantNumber; // Also dangerous } \ No newline at end of file diff --git a/idea/testData/multiFileInspections/fakeJvmFieldConstant/before/First/src/KotlinProperties.kt b/idea/testData/multiFileInspections/fakeJvmFieldConstant/before/First/src/KotlinProperties.kt index 06593ca2455..16b674a7a53 100644 --- a/idea/testData/multiFileInspections/fakeJvmFieldConstant/before/First/src/KotlinProperties.kt +++ b/idea/testData/multiFileInspections/fakeJvmFieldConstant/before/First/src/KotlinProperties.kt @@ -4,4 +4,8 @@ annotation class AnnValue(val value: String) @JvmField val importantString = "important" -const val constantString = "constant" \ No newline at end of file +const val constantString = "constant" + +@JvmField val importantNumber = 42 + +const val constantNumber = 42 \ No newline at end of file diff --git a/idea/testData/multiFileInspections/fakeJvmFieldConstant/expected.xml b/idea/testData/multiFileInspections/fakeJvmFieldConstant/expected.xml index c1266c5029a..2f5ffec959a 100644 --- a/idea/testData/multiFileInspections/fakeJvmFieldConstant/expected.xml +++ b/idea/testData/multiFileInspections/fakeJvmFieldConstant/expected.xml @@ -26,4 +26,49 @@ Kotlin non-const property used as Java constant Use of non-const Kotlin property as Java constant is incorrect. Will be forbidden in 1.4 + + JavaUser.java + 26 + First + <default> + + Kotlin non-const property used as Java constant + Use of non-const Kotlin property as Java constant is incorrect. Will be forbidden in 1.4 + + + JavaUser.java + 28 + First + <default> + + Kotlin non-const property used as Java constant + Use of non-const Kotlin property as Java constant is incorrect. Will be forbidden in 1.4 + + + JavaUser.java + 32 + First + <default> + + Kotlin non-const property used as Java constant + Use of non-const Kotlin property as Java constant is incorrect. Will be forbidden in 1.4 + + + JavaUser.java + 34 + First + <default> + + Kotlin non-const property used as Java constant + Use of non-const Kotlin property as Java constant is incorrect. Will be forbidden in 1.4 + + + JavaUser.java + 43 + First + <default> + + Kotlin non-const property used as Java constant + Use of non-const Kotlin property as Java constant is incorrect. Will be forbidden in 1.4 +