diff --git a/idea/resources/inspectionDescriptions/SuspiciousVarProperty.html b/idea/resources/inspectionDescriptions/SuspiciousVarProperty.html new file mode 100644 index 00000000000..d9ac6d9b222 --- /dev/null +++ b/idea/resources/inspectionDescriptions/SuspiciousVarProperty.html @@ -0,0 +1,5 @@ + + +This inspection reports var properties with default setter and getter that doesn't reference backing field. + + \ No newline at end of file diff --git a/idea/src/META-INF/plugin-common.xml b/idea/src/META-INF/plugin-common.xml index 9861d410776..301be89cf49 100644 --- a/idea/src/META-INF/plugin-common.xml +++ b/idea/src/META-INF/plugin-common.xml @@ -3069,6 +3069,15 @@ language="kotlin" /> + + diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/SuspiciousVarPropertyInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/SuspiciousVarPropertyInspection.kt new file mode 100644 index 00000000000..15472874cd4 --- /dev/null +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/SuspiciousVarPropertyInspection.kt @@ -0,0 +1,56 @@ +/* + * Copyright 2010-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license + * that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.idea.inspections + +import com.intellij.codeInspection.IntentionWrapper +import com.intellij.codeInspection.ProblemHighlightType +import com.intellij.codeInspection.ProblemsHolder +import org.jetbrains.kotlin.descriptors.PropertyDescriptor +import org.jetbrains.kotlin.idea.caches.resolve.analyze +import org.jetbrains.kotlin.idea.quickfix.ChangeVariableMutabilityFix +import org.jetbrains.kotlin.idea.references.mainReference +import org.jetbrains.kotlin.lexer.KtTokens +import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.psi.psiUtil.anyDescendantOfType +import org.jetbrains.kotlin.resolve.BindingContext + +class SuspiciousVarPropertyInspection : AbstractKotlinInspection() { + override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean) = + propertyVisitor(fun(property: KtProperty) { + if (property.isLocal || !property.isVar || property.initializer == null || property.setter != null) return + val getter = property.getter ?: return + val context = property.analyze() + val descriptor = context[BindingContext.DECLARATION_TO_DESCRIPTOR, property] as? PropertyDescriptor ?: return + if (context[BindingContext.BACKING_FIELD_REQUIRED, descriptor] == false) return + if (getter.hasBackingFieldReference()) return + + holder.registerProblem( + property.valOrVarKeyword, + "Suspicious 'var' property: its setter does not influence its getter result", + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, + IntentionWrapper( + ChangeVariableMutabilityFix(property, makeVar = false, deleteInitializer = true), + property.containingFile + ) + ) + }) + + companion object { + private fun KtPropertyAccessor.hasBackingFieldReference(): Boolean { + val bodyExpression = this.bodyExpression ?: return false + return bodyExpression.isBackingFieldReference(property) || bodyExpression.anyDescendantOfType { + it.isBackingFieldReference(property) + } + } + + fun KtExpression.isBackingFieldReference(property: KtProperty): Boolean = + this is KtNameReferenceExpression && isBackingFieldReference(property) + + private fun KtNameReferenceExpression.isBackingFieldReference(property: KtProperty): Boolean { + return text == KtTokens.FIELD_KEYWORD.value && mainReference.resolve() == property + } + } +} \ No newline at end of file diff --git a/idea/src/org/jetbrains/kotlin/idea/quickfix/ChangeVariableMutabilityFix.kt b/idea/src/org/jetbrains/kotlin/idea/quickfix/ChangeVariableMutabilityFix.kt index 03c85c09e2f..8be736ad7b3 100644 --- a/idea/src/org/jetbrains/kotlin/idea/quickfix/ChangeVariableMutabilityFix.kt +++ b/idea/src/org/jetbrains/kotlin/idea/quickfix/ChangeVariableMutabilityFix.kt @@ -33,10 +33,12 @@ import org.jetbrains.kotlin.util.OperatorNameConventions class ChangeVariableMutabilityFix( element: KtValVarKeywordOwner, private val makeVar: Boolean, - private val actionText: String? = null + private val actionText: String? = null, + private val deleteInitializer: Boolean = false ) : KotlinQuickFixAction(element) { - override fun getText() = actionText ?: if (makeVar) "Change to var" else "Change to val" + override fun getText() = actionText + ?: (if (makeVar) "Change to var" else "Change to val") + (if (deleteInitializer) " and delete initializer" else "") override fun getFamilyName(): String = text @@ -51,6 +53,9 @@ class ChangeVariableMutabilityFix( val factory = KtPsiFactory(project) val newKeyword = if (makeVar) factory.createVarKeyword() else factory.createValKeyword() element.valOrVarKeyword!!.replace(newKeyword) + if (deleteInitializer) { + (element as? KtProperty)?.initializer = null + } } companion object { diff --git a/idea/testData/inspectionsLocal/suspiciousVarProperty/.inspection b/idea/testData/inspectionsLocal/suspiciousVarProperty/.inspection new file mode 100644 index 00000000000..b3c0d4c23ed --- /dev/null +++ b/idea/testData/inspectionsLocal/suspiciousVarProperty/.inspection @@ -0,0 +1 @@ +org.jetbrains.kotlin.idea.inspections.SuspiciousVarPropertyInspection \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/suspiciousVarProperty/hasBackingFieldRef.kt b/idea/testData/inspectionsLocal/suspiciousVarProperty/hasBackingFieldRef.kt new file mode 100644 index 00000000000..9b7812661a4 --- /dev/null +++ b/idea/testData/inspectionsLocal/suspiciousVarProperty/hasBackingFieldRef.kt @@ -0,0 +1,5 @@ +// PROBLEM: none +class Test { + var foo: Int = 0 + get() = field +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/suspiciousVarProperty/hasBackingFieldRef2.kt b/idea/testData/inspectionsLocal/suspiciousVarProperty/hasBackingFieldRef2.kt new file mode 100644 index 00000000000..db74d0bdb7b --- /dev/null +++ b/idea/testData/inspectionsLocal/suspiciousVarProperty/hasBackingFieldRef2.kt @@ -0,0 +1,7 @@ +// PROBLEM: none +class Test { + var foo: Int = 0 + get() { + return field + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/suspiciousVarProperty/hasSetter.kt b/idea/testData/inspectionsLocal/suspiciousVarProperty/hasSetter.kt new file mode 100644 index 00000000000..64ccc569c3e --- /dev/null +++ b/idea/testData/inspectionsLocal/suspiciousVarProperty/hasSetter.kt @@ -0,0 +1,7 @@ +// PROBLEM: none +class Test { + var foo: Int + get() = 1 + set(value) { + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/suspiciousVarProperty/noBackingFieldRef.kt b/idea/testData/inspectionsLocal/suspiciousVarProperty/noBackingFieldRef.kt new file mode 100644 index 00000000000..20ca7e97be1 --- /dev/null +++ b/idea/testData/inspectionsLocal/suspiciousVarProperty/noBackingFieldRef.kt @@ -0,0 +1,5 @@ +// FIX: Change to val and delete initializer +class Test { + var foo: Int = 0 + get() = 1 +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/suspiciousVarProperty/noBackingFieldRef.kt.after b/idea/testData/inspectionsLocal/suspiciousVarProperty/noBackingFieldRef.kt.after new file mode 100644 index 00000000000..1c606fa1989 --- /dev/null +++ b/idea/testData/inspectionsLocal/suspiciousVarProperty/noBackingFieldRef.kt.after @@ -0,0 +1,5 @@ +// FIX: Change to val and delete initializer +class Test { + val foo: Int + get() = 1 +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java index 8a6fa6b8c65..1a5b2b14aca 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java @@ -6228,6 +6228,39 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { } } + @TestMetadata("idea/testData/inspectionsLocal/suspiciousVarProperty") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class SuspiciousVarProperty extends AbstractLocalInspectionTest { + private void runTest(String testDataFilePath) throws Exception { + KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath); + } + + public void testAllFilesPresentInSuspiciousVarProperty() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/suspiciousVarProperty"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), TargetBackend.ANY, true); + } + + @TestMetadata("hasBackingFieldRef.kt") + public void testHasBackingFieldRef() throws Exception { + runTest("idea/testData/inspectionsLocal/suspiciousVarProperty/hasBackingFieldRef.kt"); + } + + @TestMetadata("hasBackingFieldRef2.kt") + public void testHasBackingFieldRef2() throws Exception { + runTest("idea/testData/inspectionsLocal/suspiciousVarProperty/hasBackingFieldRef2.kt"); + } + + @TestMetadata("hasSetter.kt") + public void testHasSetter() throws Exception { + runTest("idea/testData/inspectionsLocal/suspiciousVarProperty/hasSetter.kt"); + } + + @TestMetadata("noBackingFieldRef.kt") + public void testNoBackingFieldRef() throws Exception { + runTest("idea/testData/inspectionsLocal/suspiciousVarProperty/noBackingFieldRef.kt"); + } + } + @TestMetadata("idea/testData/inspectionsLocal/unlabeledReturnInsideLambda") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class)