diff --git a/compiler/frontend/src/org/jetbrains/kotlin/psi/psiUtil/ktPsiUtil.kt b/compiler/frontend/src/org/jetbrains/kotlin/psi/psiUtil/ktPsiUtil.kt index 886ac10ba02..9bb1388b012 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/psi/psiUtil/ktPsiUtil.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/psi/psiUtil/ktPsiUtil.kt @@ -394,17 +394,26 @@ fun KtLambdaArgument.getLambdaArgumentName(bindingContext: BindingContext): Name fun KtExpression.asAssignment(): KtBinaryExpression? = if (KtPsiUtil.isAssignment(this)) this as KtBinaryExpression else null -fun KtDeclaration.visibilityModifier(): PsiElement? { +private fun KtDeclaration.modifierFromTokenSet(set: TokenSet): PsiElement? { val modifierList = modifierList ?: return null - return KtTokens.VISIBILITY_MODIFIERS.types - .asSequence() - .map { modifierList.getModifier(it as KtModifierKeywordToken) } - .firstOrNull { it != null } + return set.types + .asSequence() + .map { modifierList.getModifier(it as KtModifierKeywordToken) } + .firstOrNull { it != null } + } +fun KtDeclaration.visibilityModifier() = modifierFromTokenSet(KtTokens.VISIBILITY_MODIFIERS) + fun KtDeclaration.visibilityModifierType(): KtModifierKeywordToken? = visibilityModifier()?.node?.elementType as KtModifierKeywordToken? +private val MODALITY_MODIFIERS = TokenSet.create( + KtTokens.ABSTRACT_KEYWORD, KtTokens.FINAL_KEYWORD, KtTokens.SEALED_KEYWORD, KtTokens.OPEN_KEYWORD +) + +fun KtDeclaration.modalityModifier() = modifierFromTokenSet(MODALITY_MODIFIERS) + fun KtStringTemplateExpression.isPlain() = entries.all { it is KtLiteralStringTemplateEntry } val KtDeclaration.containingClassOrObject: KtClassOrObject? diff --git a/idea/resources/inspectionDescriptions/CanBeParameter.html b/idea/resources/inspectionDescriptions/CanBeParameter.html new file mode 100644 index 00000000000..16b46868611 --- /dev/null +++ b/idea/resources/inspectionDescriptions/CanBeParameter.html @@ -0,0 +1,6 @@ + + +This inspection reports primary constructor parameters that can have 'val' or 'var' removed. +Unnecessary usage of 'val' and 'var' in primary constructor consumes unnecessary memory. + + diff --git a/idea/src/META-INF/plugin.xml b/idea/src/META-INF/plugin.xml index eec930825c4..15d31cd620c 100644 --- a/idea/src/META-INF/plugin.xml +++ b/idea/src/META-INF/plugin.xml @@ -1436,6 +1436,13 @@ level="WEAK WARNING" /> + + diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/CanBeParameterInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/CanBeParameterInspection.kt new file mode 100644 index 00000000000..0b19a1de957 --- /dev/null +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/CanBeParameterInspection.kt @@ -0,0 +1,124 @@ +/* + * 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.PsiElement +import com.intellij.psi.PsiElementVisitor +import com.intellij.psi.PsiReference +import com.intellij.psi.search.GlobalSearchScope +import com.intellij.psi.search.PsiSearchHelper +import com.intellij.psi.search.PsiSearchHelper.SearchCostResult.FEW_OCCURRENCES +import com.intellij.psi.search.PsiSearchHelper.SearchCostResult.ZERO_OCCURRENCES +import com.intellij.psi.search.searches.ReferencesSearch +import com.intellij.psi.util.PsiTreeUtil +import org.jetbrains.kotlin.idea.quickfix.RemoveValVarFromParameterFix +import org.jetbrains.kotlin.idea.references.KtSimpleNameReference +import org.jetbrains.kotlin.idea.search.usagesSearch.getAccessorNames +import org.jetbrains.kotlin.lexer.KtTokens +import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.psi.psiUtil.containingClassOrObject + +class CanBeParameterInspection : AbstractKotlinInspection() { + private fun PsiReference.usedAsPropertyIn(klass: KtClass): Boolean { + if (this !is KtSimpleNameReference) return true + val nameExpression = element + // this.x + val parent = element.parent + if (parent is KtQualifiedExpression) { + if (parent.receiverExpression is KtThisExpression) return true + } + // x += something + if (parent is KtBinaryExpression && + parent.left == element && + KtPsiUtil.isAssignment(parent)) return true + // init / constructor / non-local property? + var parameterUser: PsiElement = nameExpression + do { + parameterUser = PsiTreeUtil.getParentOfType(parameterUser, KtProperty::class.java, KtPropertyAccessor::class.java, + KtClassInitializer::class.java, KtSecondaryConstructor::class.java) ?: return true + } while (parameterUser is KtProperty && parameterUser.isLocal) + return when (parameterUser) { + is KtProperty -> parameterUser.containingClassOrObject !== klass + is KtPropertyAccessor -> true + is KtClassInitializer -> parameterUser.containingDeclaration !== klass + is KtSecondaryConstructor -> parameterUser.getContainingClassOrObject() !== klass + else -> true + } + } + + override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor { + return object : KtVisitorVoid() { + + override fun visitParameter(parameter: KtParameter) { + // Applicable to val / var parameters of a class / object primary constructors + val valOrVar = parameter.valOrVarKeyword ?: return + val name = parameter.name ?: return + if (parameter.hasModifier(KtTokens.OVERRIDE_KEYWORD)) return + if (parameter.annotationEntries.isNotEmpty()) return + val constructor = parameter.parent.parent as? KtPrimaryConstructor ?: return + val klass = constructor.getContainingClassOrObject() as? KtClass ?: return + if (klass.isData()) return + + val useScope = parameter.useScope + if (useScope is GlobalSearchScope) { + val psiSearchHelper = PsiSearchHelper.SERVICE.getInstance(parameter.project) + for (accessorName in parameter.getAccessorNames()) { + when (psiSearchHelper.isCheapEnoughToSearch(accessorName, useScope, null, null)) { + ZERO_OCCURRENCES -> { + } // go on + else -> return // accessor in use: should remain a property + } + } + // TOO_MANY_OCCURRENCES: too expensive + // ZERO_OCCURRENCES: unused at all, reported elsewhere + if (psiSearchHelper.isCheapEnoughToSearch(name, useScope, null, null) != FEW_OCCURRENCES) return + } + // Find all references and check them + val references = ReferencesSearch.search(parameter, useScope) + if (references.none()) return + if (references.any { it.usedAsPropertyIn(klass) }) return + holder.registerProblem( + valOrVar, + "Constructor parameter is never used as a property", + ProblemHighlightType.LIKE_UNUSED_SYMBOL, + RemoveValVarFix(parameter) + ) + } + } + } + + class RemoveValVarFix(val parameter: KtParameter) : LocalQuickFix { + + private val fix = RemoveValVarFromParameterFix(parameter) + + override fun getName() = fix.text + + override fun getFamilyName() = fix.familyName + + override fun applyFix(project: Project, descriptor: ProblemDescriptor) { + parameter.valOrVarKeyword?.delete() + // Delete visibility / modality, if any + parameter.modifierList?.delete() + } + } +} + diff --git a/idea/testData/inspections/canBeParameter/inspectionData/expected.xml b/idea/testData/inspections/canBeParameter/inspectionData/expected.xml new file mode 100644 index 00000000000..a05984c4175 --- /dev/null +++ b/idea/testData/inspections/canBeParameter/inspectionData/expected.xml @@ -0,0 +1,50 @@ + + + test.kt + 5 + light_idea_test_case + + Constructor parameter is never used as a property + Constructor parameter is never used as a property + + + test.kt + 9 + light_idea_test_case + + Constructor parameter is never used as a property + Constructor parameter is never used as a property + + + test.kt + 16 + light_idea_test_case + + Constructor parameter is never used as a property + Constructor parameter is never used as a property + + + test.kt + 52 + light_idea_test_case + + Constructor parameter is never used as a property + Constructor parameter is never used as a property + + + test.kt + 62 + light_idea_test_case + + Constructor parameter is never used as a property + Constructor parameter is never used as a property + + + test.kt + 70 + light_idea_test_case + + Constructor parameter is never used as a property + Constructor parameter is never used as a property + + \ No newline at end of file diff --git a/idea/testData/inspections/canBeParameter/inspectionData/inspections.test b/idea/testData/inspections/canBeParameter/inspectionData/inspections.test new file mode 100644 index 00000000000..7e514a40604 --- /dev/null +++ b/idea/testData/inspections/canBeParameter/inspectionData/inspections.test @@ -0,0 +1 @@ +// INSPECTION_CLASS: org.jetbrains.kotlin.idea.inspections.CanBeParameterInspection \ No newline at end of file diff --git a/idea/testData/inspections/canBeParameter/test.kt b/idea/testData/inspections/canBeParameter/test.kt new file mode 100644 index 00000000000..83a6c806946 --- /dev/null +++ b/idea/testData/inspections/canBeParameter/test.kt @@ -0,0 +1,105 @@ +class NonUsed(val x: Int) // NO +// NO +data class UsedInData(val x: Int) +// YES +class UsedInProperty(val x: Int) { + val y = x +} +// YES +class UsedInInitializer(val x: Int) { + val y: Int + init { + y = x + } +} +// YES +class UsedInConstructor(val x: Int) { + fun foo(arg: Int) = arg + + constructor(): this(42) { + foo(x) + } +} +// NO +class UsedInFunction(val x: Int) { + fun get() = x +} +// NO +class UsedInGetter(val x: Int) { + val y: Int + get() = x +} +// NO +class UsedInSetter(val x: Int) { + var y: Int + get() = field + set(arg) { field = x + arg } +} +// NO +class UsedInInnerClass(val x: Int) { + inner class Inner { + fun foo() = x + } +} +// NO +class UsedOutside(val x: Int) + +fun use(): Int { + val used = UsedOutside(30) + return used.x +} +// YES +class PrivateUsedInProperty(private val x: Int) { + val y = x +} +// NO +open class Base(protected open val x: Int) +// NO +override class UsedOverridden(override val x: Int) { + val y = x +} +// YES +class UsedInPropertyVar(var x: Int) { + var y = x +} +// NO +class UsedInPropertyAnnotated(@JvmField val x: Int) { + val y = x +} +// YES +class UsedWithoutThisInInitProperty(val x: Int) { + init { + val y = x + } +} +// NO +class UsedWithThisInInitProperty(val x: Int) { + init { + val y = this.x + } +} +// NO +class UsedWithLabeledThisInInitProperty(val x: Int) { + init { + run { + val y = this@UsedWithLabeledThisInInitProperty.x + } + } +} +// NO +class UsedInFunctionProperty(val x: Int) { + fun get() { + val y = x + return y + } +} +// NO +class ModifiedInInit(var x: Int) { + init { + x += 2 + } +} +// NO +open class UsedInOverride(val x: Int) + +class UserInOverride(override val x: Int) : UsedInOverride(x) diff --git a/idea/testData/inspections/canBeParameter/usedFromJava.kt b/idea/testData/inspections/canBeParameter/usedFromJava.kt new file mode 100644 index 00000000000..7ef0b1cb017 --- /dev/null +++ b/idea/testData/inspections/canBeParameter/usedFromJava.kt @@ -0,0 +1,2 @@ +// NO +class UsedFromJava(val z: Int, val w: Int) \ No newline at end of file diff --git a/idea/testData/inspections/canBeParameter/user/User.java b/idea/testData/inspections/canBeParameter/user/User.java new file mode 100644 index 00000000000..074d3f86703 --- /dev/null +++ b/idea/testData/inspections/canBeParameter/user/User.java @@ -0,0 +1,9 @@ +package user; + +class User { + public static void main(String[] args) { + UsedFromJava used = new UsedFromJava(1, 2); + System.out.println(used.getZ()); + System.out.println(used.getW()); + } +} \ No newline at end of file diff --git a/idea/testData/quickfix/canBeParameter/.inspection b/idea/testData/quickfix/canBeParameter/.inspection new file mode 100644 index 00000000000..5bd274a3da9 --- /dev/null +++ b/idea/testData/quickfix/canBeParameter/.inspection @@ -0,0 +1 @@ +org.jetbrains.kotlin.idea.inspections.CanBeParameterInspection \ No newline at end of file diff --git a/idea/testData/quickfix/canBeParameter/usedInDerivedClass.kt b/idea/testData/quickfix/canBeParameter/usedInDerivedClass.kt new file mode 100644 index 00000000000..fdc257675a4 --- /dev/null +++ b/idea/testData/quickfix/canBeParameter/usedInDerivedClass.kt @@ -0,0 +1,6 @@ +// "Remove 'val' from parameter" "true" +open class Base(open val x: Int) { + val y = x +} + +class Derived(y: Int) : Base(y) \ No newline at end of file diff --git a/idea/testData/quickfix/canBeParameter/usedInDerivedClass.kt.after b/idea/testData/quickfix/canBeParameter/usedInDerivedClass.kt.after new file mode 100644 index 00000000000..dd06fc97890 --- /dev/null +++ b/idea/testData/quickfix/canBeParameter/usedInDerivedClass.kt.after @@ -0,0 +1,6 @@ +// "Remove 'val' from parameter" "true" +open class Base(x: Int) { + val y = x +} + +class Derived(y: Int) : Base(y) \ No newline at end of file diff --git a/idea/testData/quickfix/canBeParameter/usedInProperty.kt b/idea/testData/quickfix/canBeParameter/usedInProperty.kt new file mode 100644 index 00000000000..1f6b90baef0 --- /dev/null +++ b/idea/testData/quickfix/canBeParameter/usedInProperty.kt @@ -0,0 +1,4 @@ +// "Remove 'var' from parameter" "true" +class UsedInProperty(var x: Int) { + var y = x +} \ No newline at end of file diff --git a/idea/testData/quickfix/canBeParameter/usedInProperty.kt.after b/idea/testData/quickfix/canBeParameter/usedInProperty.kt.after new file mode 100644 index 00000000000..f3e23a311c2 --- /dev/null +++ b/idea/testData/quickfix/canBeParameter/usedInProperty.kt.after @@ -0,0 +1,4 @@ +// "Remove 'var' from parameter" "true" +class UsedInProperty(x: Int) { + var y = x +} \ No newline at end of file diff --git a/idea/testData/quickfix/canBeParameter/usedPrivateInInitializer.kt b/idea/testData/quickfix/canBeParameter/usedPrivateInInitializer.kt new file mode 100644 index 00000000000..598b3f39034 --- /dev/null +++ b/idea/testData/quickfix/canBeParameter/usedPrivateInInitializer.kt @@ -0,0 +1,8 @@ +// "Remove 'val' from parameter" "true" +class UsedInProperty(private val x: Int) { + var y: String + + init { + y = x.toString() + } +} diff --git a/idea/testData/quickfix/canBeParameter/usedPrivateInInitializer.kt.after b/idea/testData/quickfix/canBeParameter/usedPrivateInInitializer.kt.after new file mode 100644 index 00000000000..d87fa1b3cad --- /dev/null +++ b/idea/testData/quickfix/canBeParameter/usedPrivateInInitializer.kt.after @@ -0,0 +1,8 @@ +// "Remove 'val' from parameter" "true" +class UsedInProperty(x: Int) { + var y: String + + init { + y = x.toString() + } +} diff --git a/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java index 7bb03f0a16d..5c276aa1acb 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java @@ -124,6 +124,12 @@ public class InspectionTestGenerated extends AbstractInspectionTest { doTest(fileName); } + @TestMetadata("canBeParameter/inspectionData/inspections.test") + public void testCanBeParameter_inspectionData_Inspections_test() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/canBeParameter/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 f0ff5ed4be0..060b2dc1a4b 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java @@ -680,6 +680,33 @@ public class QuickFixTestGenerated extends AbstractQuickFixTest { } } + @TestMetadata("idea/testData/quickfix/canBeParameter") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class CanBeParameter extends AbstractQuickFixTest { + public void testAllFilesPresentInCanBeParameter() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/quickfix/canBeParameter"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), true); + } + + @TestMetadata("usedInDerivedClass.kt") + public void testUsedInDerivedClass() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/canBeParameter/usedInDerivedClass.kt"); + doTest(fileName); + } + + @TestMetadata("usedInProperty.kt") + public void testUsedInProperty() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/canBeParameter/usedInProperty.kt"); + doTest(fileName); + } + + @TestMetadata("usedPrivateInInitializer.kt") + public void testUsedPrivateInInitializer() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/canBeParameter/usedPrivateInInitializer.kt"); + doTest(fileName); + } + } + @TestMetadata("idea/testData/quickfix/changeSignature") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class)