From 44790eccaff0807b3ce72dc150408aeb770fd3b7 Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Tue, 18 Jul 2017 16:37:07 +0300 Subject: [PATCH] Use "unnecessary variable" inspection in J2K Unused variables are no more treated as unnecessary Related to KT-15958 --- .../UnnecessaryVariableInspection.kt | 85 +++++++++++-------- .../kotlin/idea/j2k/J2kPostProcessings.kt | 16 ++++ .../unnecessaryVariable/copyOfValUnused.kt | 6 ++ .../LocalInspectionTestGenerated.java | 6 ++ .../assignmentExpression/nullability.kt | 3 +- .../fileOrElement/ifStatement/withBlocks.kt | 7 +- .../fileOrElement/ifStatement/withoutElse.kt | 3 +- .../RedundantTypeCastAndInline.kt | 5 +- .../toKotlinClasses/iterableAndIterator.kt | 6 +- .../toKotlinClasses/iterableAndIterator2.kt | 6 +- .../toKotlinClasses/iterableAndIterator3.kt | 3 +- .../tryWithResource/WithReturnAtEnd.kt | 3 +- 12 files changed, 91 insertions(+), 58 deletions(-) create mode 100644 idea/testData/inspectionsLocal/unnecessaryVariable/copyOfValUnused.kt diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/UnnecessaryVariableInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/UnnecessaryVariableInspection.kt index c19e124dad5..a74fbbbfe36 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/UnnecessaryVariableInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/UnnecessaryVariableInspection.kt @@ -22,14 +22,13 @@ import com.intellij.codeInspection.ProblemHighlightType import com.intellij.codeInspection.ProblemsHolder import com.intellij.openapi.project.Project import com.intellij.psi.PsiElementVisitor +import com.intellij.psi.search.LocalSearchScope +import com.intellij.psi.search.searches.ReferencesSearch import org.jetbrains.kotlin.descriptors.FunctionDescriptor import org.jetbrains.kotlin.descriptors.VariableDescriptor import org.jetbrains.kotlin.idea.caches.resolve.analyze import org.jetbrains.kotlin.idea.refactoring.inline.KotlinInlineValHandler -import org.jetbrains.kotlin.psi.KtNameReferenceExpression -import org.jetbrains.kotlin.psi.KtProperty -import org.jetbrains.kotlin.psi.KtReturnExpression -import org.jetbrains.kotlin.psi.KtVisitorVoid +import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.psiUtil.getNextSiblingIgnoringWhitespaceAndComments import org.jetbrains.kotlin.psi.psiUtil.getParentOfType import org.jetbrains.kotlin.resolve.BindingContext.DECLARATION_TO_DESCRIPTOR @@ -42,45 +41,61 @@ class UnnecessaryVariableInspection : AbstractKotlinInspection() { override fun visitProperty(property: KtProperty) { super.visitProperty(property) - if (!property.isLocal) return - val initializer = property.initializer ?: return val nameIdentifier = property.nameIdentifier ?: return + val status = statusFor(property) ?: return + holder.registerProblem( + nameIdentifier, + when (status) { + UnnecessaryVariableInspection.Companion.Status.RETURN_ONLY -> + "Variable used only in following return and can be inlined" + UnnecessaryVariableInspection.Companion.Status.EXACT_COPY -> + "Variable is an exact copy of another variable and can be inlined" + }, + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, + InlineVariableFix() + ) + } + } - if (!property.isVar && initializer is KtNameReferenceExpression && property.typeReference == null) { - val context = property.analyze() - val initializerDescriptor = context[REFERENCE_TARGET, initializer] - if (initializerDescriptor is VariableDescriptor) { - if (!initializerDescriptor.isVar && - initializerDescriptor.containingDeclaration is FunctionDescriptor) { - holder.registerProblem( - nameIdentifier, - "Variable is an exact copy of another variable and can be inlined", - ProblemHighlightType.GENERIC_ERROR_OR_WARNING, - InlineVariableFix() - ) - return - } - } - } + companion object { + private enum class Status { + RETURN_ONLY, + EXACT_COPY + } - val nextStatement = property.getNextSiblingIgnoringWhitespaceAndComments() - if (nextStatement is KtReturnExpression) { - val returned = nextStatement.returnedExpression - if (returned is KtNameReferenceExpression) { - val context = nextStatement.analyze() - if (context[REFERENCE_TARGET, returned] == context[DECLARATION_TO_DESCRIPTOR, property]) { - holder.registerProblem( - nameIdentifier, - "Variable used only in following return and can be inlined", - ProblemHighlightType.GENERIC_ERROR_OR_WARNING, - InlineVariableFix() - ) - } + private fun statusFor(property: KtProperty): Status? { + val enclosingElement = KtPsiUtil.getEnclosingElementForLocalDeclaration(property) ?: return null + val initializer = property.initializer ?: return null + + if (!property.isVar && initializer is KtNameReferenceExpression && property.typeReference == null) { + val context = property.analyze() + val initializerDescriptor = context[REFERENCE_TARGET, initializer] + if (initializerDescriptor is VariableDescriptor) { + if (!initializerDescriptor.isVar && initializerDescriptor.containingDeclaration is FunctionDescriptor) { + if (ReferencesSearch.search(property, LocalSearchScope(enclosingElement)).findFirst() != null) { + return Status.EXACT_COPY } } } } + val nextStatement = property.getNextSiblingIgnoringWhitespaceAndComments() + if (nextStatement is KtReturnExpression) { + val returned = nextStatement.returnedExpression + if (returned is KtNameReferenceExpression) { + val context = nextStatement.analyze() + if (context[REFERENCE_TARGET, returned] == context[DECLARATION_TO_DESCRIPTOR, property]) { + return Status.RETURN_ONLY + } + } + } + + return null + } + + fun isActiveFor(property: KtProperty) = statusFor(property) != null + } + class InlineVariableFix : LocalQuickFix { override fun getName() = "Inline variable" diff --git a/idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessings.kt b/idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessings.kt index 212f72a67d3..d7277a2aa86 100644 --- a/idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessings.kt +++ b/idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessings.kt @@ -26,7 +26,9 @@ import org.jetbrains.kotlin.idea.caches.resolve.analyze import org.jetbrains.kotlin.idea.core.replaced import org.jetbrains.kotlin.idea.core.setVisibility import org.jetbrains.kotlin.idea.inspections.RedundantSamConstructorInspection +import org.jetbrains.kotlin.idea.inspections.UnnecessaryVariableInspection import org.jetbrains.kotlin.idea.inspections.UseExpressionBodyInspection +import org.jetbrains.kotlin.idea.inspections.findExistingEditor import org.jetbrains.kotlin.idea.intentions.* import org.jetbrains.kotlin.idea.intentions.branchedTransformations.intentions.FoldIfToReturnAsymmetricallyIntention import org.jetbrains.kotlin.idea.intentions.branchedTransformations.intentions.FoldIfToReturnIntention @@ -37,6 +39,7 @@ import org.jetbrains.kotlin.idea.intentions.conventionNameCalls.ReplaceGetOrSetI import org.jetbrains.kotlin.idea.intentions.conventionNameCalls.ReplaceGetOrSetIntention import org.jetbrains.kotlin.idea.quickfix.RemoveModifierFix import org.jetbrains.kotlin.idea.quickfix.RemoveUselessCastFix +import org.jetbrains.kotlin.idea.refactoring.inline.KotlinInlineValHandler import org.jetbrains.kotlin.idea.references.mainReference import org.jetbrains.kotlin.idea.util.application.runWriteAction import org.jetbrains.kotlin.lexer.KtTokens @@ -80,6 +83,7 @@ object J2KPostProcessingRegistrar { _processings.add(RemoveRedundantSamAdaptersProcessing()) _processings.add(RemoveRedundantCastToNullableProcessing()) _processings.add(UseExpressionBodyProcessing()) + _processings.add(UnnecessaryVariableProcessing()) registerIntentionBasedProcessing(FoldInitializerAndIfToElvisIntention()) @@ -269,6 +273,18 @@ object J2KPostProcessingRegistrar { } } + private class UnnecessaryVariableProcessing : J2kPostProcessing { + override fun createAction(element: KtElement, diagnostics: Diagnostics): (() -> Unit)? { + if (element !is KtProperty) return null + + if (!UnnecessaryVariableInspection.isActiveFor(element)) return null + + return { + KotlinInlineValHandler().inlineElement(element.project, element.findExistingEditor(), element) + } + } + } + private class RemoveRedundantCastToNullableProcessing : J2kPostProcessing { override fun createAction(element: KtElement, diagnostics: Diagnostics): (() -> Unit)? { if (element !is KtBinaryExpressionWithTypeRHS) return null diff --git a/idea/testData/inspectionsLocal/unnecessaryVariable/copyOfValUnused.kt b/idea/testData/inspectionsLocal/unnecessaryVariable/copyOfValUnused.kt new file mode 100644 index 00000000000..0848917cfe2 --- /dev/null +++ b/idea/testData/inspectionsLocal/unnecessaryVariable/copyOfValUnused.kt @@ -0,0 +1,6 @@ +// PROBLEM: none + +fun test() { + val x = 1 + val y = x +} \ 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 f10157ac78f..8e51c12381e 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java @@ -1472,6 +1472,12 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { doTest(fileName); } + @TestMetadata("copyOfValUnused.kt") + public void testCopyOfValUnused() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/unnecessaryVariable/copyOfValUnused.kt"); + doTest(fileName); + } + @TestMetadata("copyOfValWithExplicitType.kt") public void testCopyOfValWithExplicitType() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/unnecessaryVariable/copyOfValWithExplicitType.kt"); diff --git a/j2k/testData/fileOrElement/assignmentExpression/nullability.kt b/j2k/testData/fileOrElement/assignmentExpression/nullability.kt index 013c2ca9b26..94224d39719 100644 --- a/j2k/testData/fileOrElement/assignmentExpression/nullability.kt +++ b/j2k/testData/fileOrElement/assignmentExpression/nullability.kt @@ -2,8 +2,7 @@ import java.util.HashSet internal class Foo { fun foo(o: HashSet<*>) { - val o2 = o var foo = 0 - foo = o2.size + foo = o.size } } \ No newline at end of file diff --git a/j2k/testData/fileOrElement/ifStatement/withBlocks.kt b/j2k/testData/fileOrElement/ifStatement/withBlocks.kt index 803d3a232c7..9dc70a7c55f 100644 --- a/j2k/testData/fileOrElement/ifStatement/withBlocks.kt +++ b/j2k/testData/fileOrElement/ifStatement/withBlocks.kt @@ -1,6 +1,5 @@ -if (1 > 0) { - val n = 1 - return n +return if (1 > 0) { + 1 } else { - return 0 + 0 } \ No newline at end of file diff --git a/j2k/testData/fileOrElement/ifStatement/withoutElse.kt b/j2k/testData/fileOrElement/ifStatement/withoutElse.kt index be2db5323dd..d8a9fc504f4 100644 --- a/j2k/testData/fileOrElement/ifStatement/withoutElse.kt +++ b/j2k/testData/fileOrElement/ifStatement/withoutElse.kt @@ -1,4 +1,3 @@ if (1 > 0) { - val n = 1 - return n + return 1 } \ No newline at end of file diff --git a/j2k/testData/fileOrElement/postProcessing/RedundantTypeCastAndInline.kt b/j2k/testData/fileOrElement/postProcessing/RedundantTypeCastAndInline.kt index a998433e7e6..cb186d8a672 100644 --- a/j2k/testData/fileOrElement/postProcessing/RedundantTypeCastAndInline.kt +++ b/j2k/testData/fileOrElement/postProcessing/RedundantTypeCastAndInline.kt @@ -1,9 +1,8 @@ internal class C { fun foo(o: Any) { if (o is String) { - val s = o - val l = s.length - val substring = s.substring(l - 2) + val l = o.length + val substring = o.substring(l - 2) } } } \ No newline at end of file diff --git a/j2k/testData/fileOrElement/toKotlinClasses/iterableAndIterator.kt b/j2k/testData/fileOrElement/toKotlinClasses/iterableAndIterator.kt index 9804efbdfb0..921057ecf88 100644 --- a/j2k/testData/fileOrElement/toKotlinClasses/iterableAndIterator.kt +++ b/j2k/testData/fileOrElement/toKotlinClasses/iterableAndIterator.kt @@ -8,8 +8,7 @@ internal class Test : Iterable { } fun push(i: Iterator): Iterator { - val j = i - return j + return i } } @@ -19,7 +18,6 @@ internal class FullTest : Iterable { } fun push(i: Iterator): Iterator { - val j = i - return j + return i } } \ No newline at end of file diff --git a/j2k/testData/fileOrElement/toKotlinClasses/iterableAndIterator2.kt b/j2k/testData/fileOrElement/toKotlinClasses/iterableAndIterator2.kt index 20031caea66..e15f8279a43 100644 --- a/j2k/testData/fileOrElement/toKotlinClasses/iterableAndIterator2.kt +++ b/j2k/testData/fileOrElement/toKotlinClasses/iterableAndIterator2.kt @@ -10,8 +10,7 @@ internal class Test : Iterable { } fun push(i: Iterator): Iterator { - val j = i - return j + return i } } @@ -21,7 +20,6 @@ internal class FullTest : Iterable { } fun push(i: Iterator): Iterator { - val j = i - return j + return i } } \ No newline at end of file diff --git a/j2k/testData/fileOrElement/toKotlinClasses/iterableAndIterator3.kt b/j2k/testData/fileOrElement/toKotlinClasses/iterableAndIterator3.kt index 9063c437fc2..71ee936671d 100644 --- a/j2k/testData/fileOrElement/toKotlinClasses/iterableAndIterator3.kt +++ b/j2k/testData/fileOrElement/toKotlinClasses/iterableAndIterator3.kt @@ -7,7 +7,6 @@ internal class Test : Iterable { } fun push(i: Iterator): Iterator { - val j = i - return j + return i } } diff --git a/j2k/testData/fileOrElement/tryWithResource/WithReturnAtEnd.kt b/j2k/testData/fileOrElement/tryWithResource/WithReturnAtEnd.kt index db3b330b694..e9f4625d9e3 100644 --- a/j2k/testData/fileOrElement/tryWithResource/WithReturnAtEnd.kt +++ b/j2k/testData/fileOrElement/tryWithResource/WithReturnAtEnd.kt @@ -5,8 +5,7 @@ class C { try { ByteArrayInputStream(ByteArray(10)).use { stream -> // reading something - val c = stream.read() - return c + return stream.read() } } catch (e: IOException) { println(e)