diff --git a/idea/resources/inspectionDescriptions/FakeJvmFieldConstant.html b/idea/resources/inspectionDescriptions/FakeJvmFieldConstant.html new file mode 100644 index 00000000000..90de4cd24ad --- /dev/null +++ b/idea/resources/inspectionDescriptions/FakeJvmFieldConstant.html @@ -0,0 +1,17 @@ + + +

+ This inspection reports non-const Kotlin properties used as Java annotation arguments. + It's possible for property which is annotated by @JvmField, + has initializer that can be evaluated in compile-time, has primitive or String type. + Such properties have ConstantValue attribute in bytecode in Kotlin 1.1-1.2. + This attribute allows javac to fold usages of the corresponding field, and use that field in annotations. + This can cause incorrect behavior in case of separate or incremental compilation in mixed Java/Kotlin code. +

+

+ This behaviour is subject to change in Kotlin 1.3 (no ConstantValue attribute more). + Recommended action: replace JvmField annotation with const modifier on relevant Kotlin property or inline it. +

+ + + diff --git a/idea/src/META-INF/plugin.xml b/idea/src/META-INF/plugin.xml index d1d23c2cc8e..d34be2d324e 100644 --- a/idea/src/META-INF/plugin.xml +++ b/idea/src/META-INF/plugin.xml @@ -2721,6 +2721,15 @@ language="kotlin" /> + + diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/FakeJvmFieldConstantInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/FakeJvmFieldConstantInspection.kt new file mode 100644 index 00000000000..5cb16c1056a --- /dev/null +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/FakeJvmFieldConstantInspection.kt @@ -0,0 +1,62 @@ +/* + * Copyright 2010-2017 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.idea.inspections + +import com.intellij.codeInspection.IntentionWrapper +import com.intellij.codeInspection.LocalQuickFix +import com.intellij.codeInspection.ProblemHighlightType +import com.intellij.codeInspection.ProblemsHolder +import com.intellij.psi.* +import org.jetbrains.kotlin.asJava.elements.KtLightFieldImpl.KtLightFieldForDeclaration +import org.jetbrains.kotlin.idea.inspections.MayBeConstantInspection.Status.JVM_FIELD_MIGHT_BE_CONST +import org.jetbrains.kotlin.idea.inspections.MayBeConstantInspection.Status.JVM_FIELD_MIGHT_BE_CONST_NO_INITIALIZER +import org.jetbrains.kotlin.idea.quickfix.AddConstModifierFix +import org.jetbrains.kotlin.psi.KtProperty + +class FakeJvmFieldConstantInspection : AbstractKotlinInspection() { + override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor { + return object : JavaElementVisitor() { + override fun visitAnnotationParameterList(list: PsiAnnotationParameterList) { + super.visitAnnotationParameterList(list) + + for (attribute in list.attributes) { + val valueExpression = attribute.value as? PsiExpression ?: continue + val resolvedLightField = (valueExpression as? PsiReference)?.resolve() as? KtLightFieldForDeclaration ?: continue + val resolvedProperty = resolvedLightField.kotlinOrigin as? KtProperty ?: continue + with(MayBeConstantInspection) { + if (resolvedProperty.annotationEntries.isEmpty()) return@with + val resolvedPropertyStatus = resolvedProperty.getStatus() + if (resolvedPropertyStatus == JVM_FIELD_MIGHT_BE_CONST || + resolvedPropertyStatus == JVM_FIELD_MIGHT_BE_CONST_NO_INITIALIZER) { + val fixes = mutableListOf() + if (resolvedPropertyStatus == JVM_FIELD_MIGHT_BE_CONST) { + fixes += IntentionWrapper(AddConstModifierFix(resolvedProperty), resolvedProperty.containingFile) + } + holder.registerProblem( + valueExpression, + "Use of @JvmField non-const Kotlin property as annotation argument is incorrect." + + " Will be forbidden in 1.3", + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, + *fixes.toTypedArray() + ) + } + } + } + } + } + } +} \ No newline at end of file diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/MayBeConstantInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/MayBeConstantInspection.kt index 21d8dc609bb..b021856ce98 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/MayBeConstantInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/MayBeConstantInspection.kt @@ -22,6 +22,7 @@ import com.intellij.codeInspection.ProblemsHolder import com.intellij.psi.PsiElementVisitor import org.jetbrains.kotlin.descriptors.VariableDescriptor import org.jetbrains.kotlin.idea.caches.resolve.analyze +import org.jetbrains.kotlin.idea.inspections.MayBeConstantInspection.Status.* import org.jetbrains.kotlin.idea.quickfix.AddConstModifierFix import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.KtClass @@ -35,34 +36,55 @@ import org.jetbrains.kotlin.resolve.jvm.annotations.hasJvmFieldAnnotation import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode class MayBeConstantInspection : AbstractKotlinInspection() { + enum class Status { + NONE, + MIGHT_BE_CONST, + JVM_FIELD_MIGHT_BE_CONST, + JVM_FIELD_MIGHT_BE_CONST_NO_INITIALIZER + } + override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor { return object : KtVisitorVoid() { override fun visitProperty(property: KtProperty) { super.visitProperty(property) + val status = property.getStatus() + when (status) { + NONE, JVM_FIELD_MIGHT_BE_CONST_NO_INITIALIZER -> return + else -> { + holder.registerProblem( + property.nameIdentifier ?: property, + if (status == JVM_FIELD_MIGHT_BE_CONST) "'const' might be used instead of '@JvmField'" else "Might be 'const'", + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, + IntentionWrapper(AddConstModifierFix(property), property.containingFile) + ) + } + } + } + } + } - if (property.isLocal || property.isVar) return - val initializer = property.initializer ?: return - if (property.hasModifier(KtTokens.CONST_KEYWORD)) return - // Top-level or object only - if (property.containingClassOrObject is KtClass) return + companion object { + fun KtProperty.getStatus(): Status { + if (isLocal || isVar || hasModifier(KtTokens.CONST_KEYWORD) || containingClassOrObject is KtClass) { + return NONE + } + val initializer = initializer - // For some reason constant evaluation does not work for property.analyze() - val context = initializer.analyze(BodyResolveMode.PARTIAL) - val propertyDescriptor = context[BindingContext.DECLARATION_TO_DESCRIPTOR, property] as? VariableDescriptor ?: return - val withJvmField = propertyDescriptor.hasJvmFieldAnnotation() - if (property.annotationEntries.isNotEmpty() && !withJvmField) return + // For some reason constant evaluation does not work for property.analyze() + val context = (initializer ?: this).analyze(BodyResolveMode.PARTIAL) + val propertyDescriptor = context[BindingContext.DECLARATION_TO_DESCRIPTOR, this] as? VariableDescriptor ?: return NONE + val withJvmField = propertyDescriptor.hasJvmFieldAnnotation() + if (annotationEntries.isNotEmpty() && !withJvmField) return NONE - val initializerValue = ConstantExpressionEvaluator.getConstant( - initializer, context - )?.toConstantValue(propertyDescriptor.type) ?: return - if (initializerValue.isStandaloneOnlyConstant()) return - - holder.registerProblem( - property.nameIdentifier ?: property, - if (withJvmField) "'const' might be used instead of '@JvmField'" else "Might be 'const'", - ProblemHighlightType.GENERIC_ERROR_OR_WARNING, - IntentionWrapper(AddConstModifierFix(property), property.containingFile) - ) + return when { + initializer != null -> { + ConstantExpressionEvaluator.getConstant( + initializer, context + )?.toConstantValue(propertyDescriptor.type)?.takeIf { !it.isStandaloneOnlyConstant() } ?: return NONE + if (withJvmField) JVM_FIELD_MIGHT_BE_CONST else MIGHT_BE_CONST + } + withJvmField -> JVM_FIELD_MIGHT_BE_CONST_NO_INITIALIZER + else -> NONE } } } diff --git a/idea/src/org/jetbrains/kotlin/idea/quickfix/AddModifierFix.kt b/idea/src/org/jetbrains/kotlin/idea/quickfix/AddModifierFix.kt index bc3aa95fd37..cf671fb9e52 100644 --- a/idea/src/org/jetbrains/kotlin/idea/quickfix/AddModifierFix.kt +++ b/idea/src/org/jetbrains/kotlin/idea/quickfix/AddModifierFix.kt @@ -47,7 +47,7 @@ open class AddModifierFix( override fun getText(): String { val element = element ?: return "" - if (modifier in modalityModifiers || modifier in KtTokens.VISIBILITY_MODIFIERS) { + if (modifier in modalityModifiers || modifier in KtTokens.VISIBILITY_MODIFIERS || modifier == KtTokens.CONST_KEYWORD) { return "Make ${getElementName(element)} ${modifier.value}" } return "Add '${modifier.value}' modifier" diff --git a/idea/testData/inspectionsLocal/mayBeConstant/array.kt b/idea/testData/inspectionsLocal/mayBeConstant/array.kt new file mode 100644 index 00000000000..e2e54ed7f6e --- /dev/null +++ b/idea/testData/inspectionsLocal/mayBeConstant/array.kt @@ -0,0 +1,3 @@ +// WITH_RUNTIME + +val arr = arrayOf("alpha", "omega") \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/mayBeConstant/enum.kt b/idea/testData/inspectionsLocal/mayBeConstant/enum.kt new file mode 100644 index 00000000000..c820c8e3272 --- /dev/null +++ b/idea/testData/inspectionsLocal/mayBeConstant/enum.kt @@ -0,0 +1,7 @@ +// PROBLEM: none + +enum class E { + FIRST +} + +val e = E.FIRST \ No newline at end of file diff --git a/idea/testData/multiFileInspections/fakeJvmFieldConstant/before/First/First.iml b/idea/testData/multiFileInspections/fakeJvmFieldConstant/before/First/First.iml new file mode 100644 index 00000000000..6c1501649ec --- /dev/null +++ b/idea/testData/multiFileInspections/fakeJvmFieldConstant/before/First/First.iml @@ -0,0 +1,20 @@ + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/idea/testData/multiFileInspections/fakeJvmFieldConstant/before/First/lib/FakeJvmFieldConstant.jar b/idea/testData/multiFileInspections/fakeJvmFieldConstant/before/First/lib/FakeJvmFieldConstant.jar new file mode 100644 index 00000000000..7558ba93726 Binary files /dev/null and b/idea/testData/multiFileInspections/fakeJvmFieldConstant/before/First/lib/FakeJvmFieldConstant.jar differ diff --git a/idea/testData/multiFileInspections/fakeJvmFieldConstant/before/First/src/JavaUser.java b/idea/testData/multiFileInspections/fakeJvmFieldConstant/before/First/src/JavaUser.java new file mode 100644 index 00000000000..10506a95635 --- /dev/null +++ b/idea/testData/multiFileInspections/fakeJvmFieldConstant/before/First/src/JavaUser.java @@ -0,0 +1,13 @@ +public class JavaUser { + // Dangerous + @Ann(s = KotlinPropertiesKt.importantString) + public void foo() {} + + // Also dangerous + @AnnValue(OtherPropertiesKt.notSoImportantString) + public void bar() {} + + // Safe + @Ann(s = KotlinPropertiesKt.constantString) + public void baz() {} +} \ 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 new file mode 100644 index 00000000000..06593ca2455 --- /dev/null +++ b/idea/testData/multiFileInspections/fakeJvmFieldConstant/before/First/src/KotlinProperties.kt @@ -0,0 +1,7 @@ +annotation class Ann(val s: String) + +annotation class AnnValue(val value: String) + +@JvmField val importantString = "important" + +const val constantString = "constant" \ No newline at end of file diff --git a/idea/testData/multiFileInspections/fakeJvmFieldConstant/expected.xml b/idea/testData/multiFileInspections/fakeJvmFieldConstant/expected.xml new file mode 100644 index 00000000000..1e20bae89bf --- /dev/null +++ b/idea/testData/multiFileInspections/fakeJvmFieldConstant/expected.xml @@ -0,0 +1,20 @@ + + + JavaUser.java + 3 + First + <default> + + Kotlin non-const property used as annotation argument + Use of @JvmField non-const Kotlin property as annotation argument is incorrect. Will be forbidden in 1.3 + + + JavaUser.java + 7 + First + <default> + + Kotlin non-const property used as annotation argument + Use of @JvmField non-const Kotlin property as annotation argument is incorrect. Will be forbidden in 1.3 + + diff --git a/idea/testData/multiFileInspections/fakeJvmFieldConstant/fakeJvmFieldConstant.test b/idea/testData/multiFileInspections/fakeJvmFieldConstant/fakeJvmFieldConstant.test new file mode 100644 index 00000000000..2246059b4af --- /dev/null +++ b/idea/testData/multiFileInspections/fakeJvmFieldConstant/fakeJvmFieldConstant.test @@ -0,0 +1,5 @@ +{ + "inspectionClass": "org.jetbrains.kotlin.idea.inspections.FakeJvmFieldConstantInspection", + "withRuntime": "true", + "isMultiModule": "true" +} \ No newline at end of file diff --git a/idea/testData/quickfix/modifiers/constVal.after.kt b/idea/testData/quickfix/modifiers/constVal.after.kt index 51a147a3402..16ea1d430e4 100644 --- a/idea/testData/quickfix/modifiers/constVal.after.kt +++ b/idea/testData/quickfix/modifiers/constVal.after.kt @@ -1,4 +1,4 @@ -// "Add 'const' modifier" "true" +// "Make 'i' const" "true" // ERROR: Only 'const val' can be used in constant expressions package constVal diff --git a/idea/testData/quickfix/modifiers/constVal.before.Main.kt b/idea/testData/quickfix/modifiers/constVal.before.Main.kt index 14e16dd2a30..47f9a5cc296 100644 --- a/idea/testData/quickfix/modifiers/constVal.before.Main.kt +++ b/idea/testData/quickfix/modifiers/constVal.before.Main.kt @@ -1,4 +1,4 @@ -// "Add 'const' modifier" "true" +// "Make 'i' const" "true" // ERROR: Only 'const val' can be used in constant expressions package constVal diff --git a/idea/tests/org/jetbrains/kotlin/idea/codeInsight/MultiFileInspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/codeInsight/MultiFileInspectionTestGenerated.java index e23d6024167..57df749915e 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/codeInsight/MultiFileInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/codeInsight/MultiFileInspectionTestGenerated.java @@ -36,6 +36,12 @@ public class MultiFileInspectionTestGenerated extends AbstractMultiFileInspectio KotlinTestUtils.assertAllTestsPresentInSingleGeneratedClass(this.getClass(), new File("idea/testData/multiFileInspections"), Pattern.compile("^(.+)\\.test$"), TargetBackend.ANY); } + @TestMetadata("fakeJvmFieldConstant/fakeJvmFieldConstant.test") + public void testFakeJvmFieldConstant_FakeJvmFieldConstant() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/multiFileInspections/fakeJvmFieldConstant/fakeJvmFieldConstant.test"); + doTest(fileName); + } + @TestMetadata("invalidBundleOrProperty/invalidBundleOrProperty.test") public void testInvalidBundleOrProperty_InvalidBundleOrProperty() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/multiFileInspections/invalidBundleOrProperty/invalidBundleOrProperty.test");