diff --git a/idea/resources/inspectionDescriptions/SetterBackingFieldAssignment.html b/idea/resources/inspectionDescriptions/SetterBackingFieldAssignment.html new file mode 100644 index 00000000000..fbdf8baaa29 --- /dev/null +++ b/idea/resources/inspectionDescriptions/SetterBackingFieldAssignment.html @@ -0,0 +1,5 @@ + + +This inspection reports a setter of a property with a backing field that doesn't update the 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 76eee45db2b..9861d410776 100644 --- a/idea/src/META-INF/plugin-common.xml +++ b/idea/src/META-INF/plugin-common.xml @@ -3060,6 +3060,15 @@ language="kotlin" /> + + diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/SetterBackingFieldAssignmentInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/SetterBackingFieldAssignmentInspection.kt new file mode 100644 index 00000000000..4117abbecd3 --- /dev/null +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/SetterBackingFieldAssignmentInspection.kt @@ -0,0 +1,78 @@ +/* + * 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.* +import com.intellij.openapi.project.Project +import com.intellij.psi.PsiElementVisitor +import org.jetbrains.kotlin.descriptors.PropertyDescriptor +import org.jetbrains.kotlin.descriptors.ValueParameterDescriptor +import org.jetbrains.kotlin.idea.caches.resolve.analyze +import org.jetbrains.kotlin.lexer.KtTokens +import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.psi.psiUtil.anyDescendantOfType +import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall + +class SetterBackingFieldAssignmentInspection : AbstractKotlinInspection(), CleanupLocalInspectionTool { + override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession): PsiElementVisitor { + return propertyAccessorVisitor(fun(accessor) { + if (!accessor.isSetter) return + val bodyExpression = accessor.bodyExpression as? KtBlockExpression ?: return + + val property = accessor.property + val propertyContext = property.analyze() + val propertyDescriptor = propertyContext[BindingContext.DECLARATION_TO_DESCRIPTOR, property] as? PropertyDescriptor ?: return + if (propertyContext[BindingContext.BACKING_FIELD_REQUIRED, propertyDescriptor] == false) return + + val accessorContext = accessor.analyze() + val parameter = accessor.valueParameters.singleOrNull() + val parameterDescriptor = accessorContext[BindingContext.VALUE_PARAMETER, parameter] as? ValueParameterDescriptor + if (bodyExpression.anyDescendantOfType { + when (it) { + is KtBinaryExpression -> + it.left?.text == KtTokens.FIELD_KEYWORD.value && it.operationToken in assignmentOperators + is KtUnaryExpression -> + it.baseExpression?.text == KtTokens.FIELD_KEYWORD.value && it.operationToken in incrementAndDecrementOperators + is KtCallExpression -> + it.valueArguments.any { arg -> + arg.text == parameter?.text + && arg.getArgumentExpression().getResolvedCall(accessorContext)?.resultingDescriptor == parameterDescriptor + } + else -> false + } + }) return + + holder.registerProblem( + accessor, + "Existing backing field is not assigned by the setter", + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, + AssignBackingFieldFix() + ) + }) + } +} + +private val assignmentOperators = listOf(KtTokens.EQ, KtTokens.PLUSEQ, KtTokens.MINUSEQ, KtTokens.MULTEQ, KtTokens.DIVEQ, KtTokens.PERCEQ) + +private val incrementAndDecrementOperators = listOf(KtTokens.PLUSPLUS, KtTokens.MINUSMINUS) + +private class AssignBackingFieldFix : LocalQuickFix { + override fun getName() = "Assign backing filed" + + override fun getFamilyName() = name + + override fun applyFix(project: Project, descriptor: ProblemDescriptor) { + val setter = descriptor.psiElement as? KtPropertyAccessor ?: return + val parameter = setter.valueParameters.firstOrNull() ?: return + val bodyExpression = setter.bodyExpression as? KtBlockExpression ?: return + setter.hasBlockBody() + bodyExpression.addBefore( + KtPsiFactory(setter).createExpression("field = ${parameter.text}"), + bodyExpression.rBrace + ) + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/setterBackingFieldAssignment/.inspection b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/.inspection new file mode 100644 index 00000000000..537b6cdb2e9 --- /dev/null +++ b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/.inspection @@ -0,0 +1 @@ +org.jetbrains.kotlin.idea.inspections.SetterBackingFieldAssignmentInspection \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/setterBackingFieldAssignment/assignment.kt b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/assignment.kt new file mode 100644 index 00000000000..4664cfe9ef9 --- /dev/null +++ b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/assignment.kt @@ -0,0 +1,10 @@ +// PROBLEM: none +class Test { + var foo: Int = 1 + set(value) { + bar() + field = value + } + + fun bar() {} +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/setterBackingFieldAssignment/decrement.kt b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/decrement.kt new file mode 100644 index 00000000000..dd1374cf2ec --- /dev/null +++ b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/decrement.kt @@ -0,0 +1,7 @@ +// PROBLEM: none +class Test { + var foo: Int = 10 + set(value) { + field-- + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/setterBackingFieldAssignment/decrement2.kt b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/decrement2.kt new file mode 100644 index 00000000000..65dafcd350f --- /dev/null +++ b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/decrement2.kt @@ -0,0 +1,7 @@ +// PROBLEM: none +class Test { + var foo: Int = 10 + set(value) { + --field + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/setterBackingFieldAssignment/divAssign.kt b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/divAssign.kt new file mode 100644 index 00000000000..e3d16f128d0 --- /dev/null +++ b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/divAssign.kt @@ -0,0 +1,7 @@ +// PROBLEM: none +class Test { + var foo: Int = 10 + set(value) { + field /= value + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/setterBackingFieldAssignment/functionCallWithSetterParam.kt b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/functionCallWithSetterParam.kt new file mode 100644 index 00000000000..68e569cc465 --- /dev/null +++ b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/functionCallWithSetterParam.kt @@ -0,0 +1,9 @@ +// PROBLEM: none +class Test { + var foo: Int = 10 + set(value) { + bar(value) + } + + fun bar(value: Int) {} +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/setterBackingFieldAssignment/increment.kt b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/increment.kt new file mode 100644 index 00000000000..3cff3ee8b87 --- /dev/null +++ b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/increment.kt @@ -0,0 +1,7 @@ +// PROBLEM: none +class Test { + var foo: Int = 10 + set(value) { + field++ + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/setterBackingFieldAssignment/increment2.kt b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/increment2.kt new file mode 100644 index 00000000000..3a0ccaac6b9 --- /dev/null +++ b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/increment2.kt @@ -0,0 +1,7 @@ +// PROBLEM: none +class Test { + var foo: Int = 10 + set(value) { + ++field + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/setterBackingFieldAssignment/minusAssign.kt b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/minusAssign.kt new file mode 100644 index 00000000000..39db2789954 --- /dev/null +++ b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/minusAssign.kt @@ -0,0 +1,7 @@ +// PROBLEM: none +class Test { + var foo: Int = 10 + set(value) { + field -= value + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/setterBackingFieldAssignment/modAssign.kt b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/modAssign.kt new file mode 100644 index 00000000000..5c2765c6643 --- /dev/null +++ b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/modAssign.kt @@ -0,0 +1,7 @@ +// PROBLEM: none +class Test { + var foo: Int = 10 + set(value) { + field %= value + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/setterBackingFieldAssignment/noAssignment.kt b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/noAssignment.kt new file mode 100644 index 00000000000..ba9e0ee8805 --- /dev/null +++ b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/noAssignment.kt @@ -0,0 +1,5 @@ +class Test { + var foo: Int = 1 + set(value) { + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/setterBackingFieldAssignment/noAssignment.kt.after b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/noAssignment.kt.after new file mode 100644 index 00000000000..edde0232452 --- /dev/null +++ b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/noAssignment.kt.after @@ -0,0 +1,6 @@ +class Test { + var foo: Int = 1 + set(value) { + field = value + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/setterBackingFieldAssignment/noAssignment2.kt b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/noAssignment2.kt new file mode 100644 index 00000000000..9a95f3cd38e --- /dev/null +++ b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/noAssignment2.kt @@ -0,0 +1,8 @@ +class Test { + var foo: Int = 1 + set(value) { + bar(field) + } + + fun bar(i: Int) {} +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/setterBackingFieldAssignment/noAssignment2.kt.after b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/noAssignment2.kt.after new file mode 100644 index 00000000000..09218419336 --- /dev/null +++ b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/noAssignment2.kt.after @@ -0,0 +1,9 @@ +class Test { + var foo: Int = 1 + set(value) { + bar(field) + field = value + } + + fun bar(i: Int) {} +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/setterBackingFieldAssignment/noBackingField.kt b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/noBackingField.kt new file mode 100644 index 00000000000..a783ddfddfb --- /dev/null +++ b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/noBackingField.kt @@ -0,0 +1,7 @@ +// PROBLEM: none +class Test { + var foo: Int + get() = 1 + set(value) { + } +} diff --git a/idea/testData/inspectionsLocal/setterBackingFieldAssignment/plusAssign.kt b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/plusAssign.kt new file mode 100644 index 00000000000..16cde20c8eb --- /dev/null +++ b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/plusAssign.kt @@ -0,0 +1,7 @@ +// PROBLEM: none +class Test { + var foo: Int = 10 + set(value) { + field += value + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/setterBackingFieldAssignment/timesAssign.kt b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/timesAssign.kt new file mode 100644 index 00000000000..fc16dddba50 --- /dev/null +++ b/idea/testData/inspectionsLocal/setterBackingFieldAssignment/timesAssign.kt @@ -0,0 +1,7 @@ +// PROBLEM: none +class Test { + var foo: Int = 10 + set(value) { + field *= value + } +} \ 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 772631e6965..8a6fa6b8c65 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java @@ -5847,6 +5847,89 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { } } + @TestMetadata("idea/testData/inspectionsLocal/setterBackingFieldAssignment") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class SetterBackingFieldAssignment extends AbstractLocalInspectionTest { + private void runTest(String testDataFilePath) throws Exception { + KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath); + } + + public void testAllFilesPresentInSetterBackingFieldAssignment() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/setterBackingFieldAssignment"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), TargetBackend.ANY, true); + } + + @TestMetadata("assignment.kt") + public void testAssignment() throws Exception { + runTest("idea/testData/inspectionsLocal/setterBackingFieldAssignment/assignment.kt"); + } + + @TestMetadata("decrement.kt") + public void testDecrement() throws Exception { + runTest("idea/testData/inspectionsLocal/setterBackingFieldAssignment/decrement.kt"); + } + + @TestMetadata("decrement2.kt") + public void testDecrement2() throws Exception { + runTest("idea/testData/inspectionsLocal/setterBackingFieldAssignment/decrement2.kt"); + } + + @TestMetadata("divAssign.kt") + public void testDivAssign() throws Exception { + runTest("idea/testData/inspectionsLocal/setterBackingFieldAssignment/divAssign.kt"); + } + + @TestMetadata("functionCallWithSetterParam.kt") + public void testFunctionCallWithSetterParam() throws Exception { + runTest("idea/testData/inspectionsLocal/setterBackingFieldAssignment/functionCallWithSetterParam.kt"); + } + + @TestMetadata("increment.kt") + public void testIncrement() throws Exception { + runTest("idea/testData/inspectionsLocal/setterBackingFieldAssignment/increment.kt"); + } + + @TestMetadata("increment2.kt") + public void testIncrement2() throws Exception { + runTest("idea/testData/inspectionsLocal/setterBackingFieldAssignment/increment2.kt"); + } + + @TestMetadata("minusAssign.kt") + public void testMinusAssign() throws Exception { + runTest("idea/testData/inspectionsLocal/setterBackingFieldAssignment/minusAssign.kt"); + } + + @TestMetadata("modAssign.kt") + public void testModAssign() throws Exception { + runTest("idea/testData/inspectionsLocal/setterBackingFieldAssignment/modAssign.kt"); + } + + @TestMetadata("noAssignment.kt") + public void testNoAssignment() throws Exception { + runTest("idea/testData/inspectionsLocal/setterBackingFieldAssignment/noAssignment.kt"); + } + + @TestMetadata("noAssignment2.kt") + public void testNoAssignment2() throws Exception { + runTest("idea/testData/inspectionsLocal/setterBackingFieldAssignment/noAssignment2.kt"); + } + + @TestMetadata("noBackingField.kt") + public void testNoBackingField() throws Exception { + runTest("idea/testData/inspectionsLocal/setterBackingFieldAssignment/noBackingField.kt"); + } + + @TestMetadata("plusAssign.kt") + public void testPlusAssign() throws Exception { + runTest("idea/testData/inspectionsLocal/setterBackingFieldAssignment/plusAssign.kt"); + } + + @TestMetadata("timesAssign.kt") + public void testTimesAssign() throws Exception { + runTest("idea/testData/inspectionsLocal/setterBackingFieldAssignment/timesAssign.kt"); + } + } + @TestMetadata("idea/testData/inspectionsLocal/simplifyAssertNotNull") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class)