diff --git a/idea/resources/inspectionDescriptions/CanBePrimaryConstructorProperty.html b/idea/resources/inspectionDescriptions/CanBePrimaryConstructorProperty.html new file mode 100644 index 00000000000..680255d8b8c --- /dev/null +++ b/idea/resources/inspectionDescriptions/CanBePrimaryConstructorProperty.html @@ -0,0 +1,5 @@ + + +This inspection reports properties which are explicitly assigned by constructor arguments and can be declared directly in constructor instead. + + \ No newline at end of file diff --git a/idea/src/META-INF/plugin.xml b/idea/src/META-INF/plugin.xml index 12bcaf5bf3b..eba7789b6d1 100644 --- a/idea/src/META-INF/plugin.xml +++ b/idea/src/META-INF/plugin.xml @@ -1569,6 +1569,14 @@ language="kotlin" /> + + diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/CanBePrimaryConstructorPropertyInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/CanBePrimaryConstructorPropertyInspection.kt new file mode 100644 index 00000000000..6ceda4bcb34 --- /dev/null +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/CanBePrimaryConstructorPropertyInspection.kt @@ -0,0 +1,88 @@ +/* + * Copyright 2010-2016 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.LocalQuickFix +import com.intellij.codeInspection.ProblemDescriptor +import com.intellij.codeInspection.ProblemHighlightType +import com.intellij.codeInspection.ProblemsHolder +import com.intellij.openapi.project.Project +import com.intellij.psi.PsiElementVisitor +import org.jetbrains.kotlin.descriptors.ConstructorDescriptor +import org.jetbrains.kotlin.descriptors.ValueParameterDescriptor +import org.jetbrains.kotlin.idea.caches.resolve.analyzeFully +import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.psi.psiUtil.getAnnotationEntries +import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils + +class CanBePrimaryConstructorPropertyInspection : AbstractKotlinInspection() { + + override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor { + return object : KtVisitorVoid() { + override fun visitProperty(property: KtProperty) { + if (property.isLocal) return + if (property.getter != null || property.setter != null || property.delegate != null) return + val assigned = property.initializer as? KtReferenceExpression ?: return + + val context = property.analyzeFully() + val assignedDescriptor = context.get(BindingContext.REFERENCE_TARGET, assigned) as? ValueParameterDescriptor ?: return + // to prevent some exotic situations + if (!assignedDescriptor.annotations.isEmpty() || !assigned.getAnnotationEntries().isEmpty()) return + + val containingConstructor = assignedDescriptor.containingDeclaration as? ConstructorDescriptor ?: return + if (containingConstructor.containingDeclaration.isData) return + + val propertyTypeReference = property.typeReference + val propertyType = context.get(BindingContext.TYPE, propertyTypeReference) + if (propertyType != null && propertyType != assignedDescriptor.type) return + + val nameIdentifier = property.nameIdentifier ?: return + if (nameIdentifier.text != assignedDescriptor.name.asString()) return + + val assignedParameter = DescriptorToSourceUtils.descriptorToDeclaration(assignedDescriptor) as? KtParameter ?: return + holder.registerProblem(holder.manager.createProblemDescriptor( + nameIdentifier, + nameIdentifier, + "Property is explicitly assigned by parameter ${assignedDescriptor.name}, can be declared directly in constructor", + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, + isOnTheFly, + MakeConstructorPropertyFix(property, assignedParameter) + )) + } + } + } + + class MakeConstructorPropertyFix(val original: KtProperty, val parameter: KtParameter) : LocalQuickFix { + override fun getName() = "Move to constructor" + + override fun getFamilyName() = "Move to constructor" + + override fun applyFix(project: Project, descriptor: ProblemDescriptor) { + val isVar = original.isVar + val modifiers = original.modifierList?.text + val factory = KtPsiFactory(project) + val valOrVar = if (isVar) factory.createVarKeyword() else factory.createValKeyword() + parameter.addBefore(valOrVar, parameter.nameIdentifier) + if (modifiers != null) { + val newModifiers = factory.createModifierList(modifiers) + parameter.addBefore(newModifiers, parameter.valOrVarKeyword) + } + original.delete() + } + } +} \ No newline at end of file diff --git a/idea/testData/inspections/canBePrimaryConstructorProperty/inspectionData/expected.xml b/idea/testData/inspections/canBePrimaryConstructorProperty/inspectionData/expected.xml new file mode 100644 index 00000000000..bb71e38ae4a --- /dev/null +++ b/idea/testData/inspections/canBePrimaryConstructorProperty/inspectionData/expected.xml @@ -0,0 +1,18 @@ + + + properties.kt + 3 + light_idea_test_case + + Property is explicitly assigned to constructor parameter + Property is explicitly assigned by parameter simple, can be declared directly in constructor + + + properties.kt + 5 + light_idea_test_case + + Property is explicitly assigned to constructor parameter + Property is explicitly assigned by parameter withType, can be declared directly in constructor + + \ No newline at end of file diff --git a/idea/testData/inspections/canBePrimaryConstructorProperty/inspectionData/inspections.test b/idea/testData/inspections/canBePrimaryConstructorProperty/inspectionData/inspections.test new file mode 100644 index 00000000000..53652f7dfa7 --- /dev/null +++ b/idea/testData/inspections/canBePrimaryConstructorProperty/inspectionData/inspections.test @@ -0,0 +1 @@ +// INSPECTION_CLASS: org.jetbrains.kotlin.idea.inspections.CanBePrimaryConstructorPropertyInspection diff --git a/idea/testData/inspections/canBePrimaryConstructorProperty/properties.kt b/idea/testData/inspections/canBePrimaryConstructorProperty/properties.kt new file mode 100644 index 00000000000..4f91ce95c76 --- /dev/null +++ b/idea/testData/inspections/canBePrimaryConstructorProperty/properties.kt @@ -0,0 +1,34 @@ +class Correct(simple: String, withType: Int, otherName: Double) { + + val simple = simple + + val withType: Int = withType + + // Questionable case (due to possible named parameters), not allowed now + val anotherName = otherName +} + +// Inspection should not work here anywhere +class Incorrect(val property: String, withGetter: Double, withSetter: Int, differentType: String) { + + val another = property + + val fromProperty = another + + val withGetter = withGetter + get() = 2 * field + + val withSetter = withSetter + private set + + val differentType: String? = differentType + + constructor(param: Int): this("", 0.0, param, "") { + val local = param + } +} + +// For data class inspection also should not work +data class Data(name: String) { + val name = name +} \ No newline at end of file diff --git a/idea/testData/quickfix/canBePrimaryConstructorProperty/.inspection b/idea/testData/quickfix/canBePrimaryConstructorProperty/.inspection new file mode 100644 index 00000000000..9a3b7fcc327 --- /dev/null +++ b/idea/testData/quickfix/canBePrimaryConstructorProperty/.inspection @@ -0,0 +1 @@ +org.jetbrains.kotlin.idea.inspections.CanBePrimaryConstructorPropertyInspection \ No newline at end of file diff --git a/idea/testData/quickfix/canBePrimaryConstructorProperty/protectedOpenVar.kt b/idea/testData/quickfix/canBePrimaryConstructorProperty/protectedOpenVar.kt new file mode 100644 index 00000000000..7e35b63c8ef --- /dev/null +++ b/idea/testData/quickfix/canBePrimaryConstructorProperty/protectedOpenVar.kt @@ -0,0 +1,4 @@ +// "Move to constructor" "true" +class Container(index: Int) { + protected open var index = index +} \ No newline at end of file diff --git a/idea/testData/quickfix/canBePrimaryConstructorProperty/protectedOpenVar.kt.after b/idea/testData/quickfix/canBePrimaryConstructorProperty/protectedOpenVar.kt.after new file mode 100644 index 00000000000..c83e586dead --- /dev/null +++ b/idea/testData/quickfix/canBePrimaryConstructorProperty/protectedOpenVar.kt.after @@ -0,0 +1,3 @@ +// "Move to constructor" "true" +class Container(protected open var index: Int) { +} \ No newline at end of file diff --git a/idea/testData/quickfix/canBePrimaryConstructorProperty/simple.kt b/idea/testData/quickfix/canBePrimaryConstructorProperty/simple.kt new file mode 100644 index 00000000000..d7cc8de7bb5 --- /dev/null +++ b/idea/testData/quickfix/canBePrimaryConstructorProperty/simple.kt @@ -0,0 +1,4 @@ +// "Move to constructor" "true" +class Correct(name: String) { + val name: String = name +} \ No newline at end of file diff --git a/idea/testData/quickfix/canBePrimaryConstructorProperty/simple.kt.after b/idea/testData/quickfix/canBePrimaryConstructorProperty/simple.kt.after new file mode 100644 index 00000000000..fb5ae10ab77 --- /dev/null +++ b/idea/testData/quickfix/canBePrimaryConstructorProperty/simple.kt.after @@ -0,0 +1,3 @@ +// "Move to constructor" "true" +class Correct(val name: String) { +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java index 06a94bae274..4c137231dd8 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java @@ -130,6 +130,12 @@ public class InspectionTestGenerated extends AbstractInspectionTest { doTest(fileName); } + @TestMetadata("canBePrimaryConstructorProperty/inspectionData/inspections.test") + public void testCanBePrimaryConstructorProperty_inspectionData_Inspections_test() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/canBePrimaryConstructorProperty/inspectionData/inspections.test"); + doTest(fileName); + } + @TestMetadata("canBeVal/inspectionData/inspections.test") public void testCanBeVal_inspectionData_Inspections_test() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/canBeVal/inspectionData/inspections.test"); diff --git a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java index 2f033ee4135..6155a9b6e11 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java @@ -716,6 +716,27 @@ public class QuickFixTestGenerated extends AbstractQuickFixTest { } } + @TestMetadata("idea/testData/quickfix/canBePrimaryConstructorProperty") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class CanBePrimaryConstructorProperty extends AbstractQuickFixTest { + public void testAllFilesPresentInCanBePrimaryConstructorProperty() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/quickfix/canBePrimaryConstructorProperty"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), true); + } + + @TestMetadata("protectedOpenVar.kt") + public void testProtectedOpenVar() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/canBePrimaryConstructorProperty/protectedOpenVar.kt"); + doTest(fileName); + } + + @TestMetadata("simple.kt") + public void testSimple() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/canBePrimaryConstructorProperty/simple.kt"); + doTest(fileName); + } + } + @TestMetadata("idea/testData/quickfix/changeSignature") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class)