From e66d0f71ac0e2dfcdefe34a28ce096e54e848922 Mon Sep 17 00:00:00 2001 From: Valentin Kipyatkov Date: Tue, 11 Aug 2015 19:07:10 +0300 Subject: [PATCH] Implemented more general scheme of post-processing with intentions in J2K + added replacement of get/set with property access there --- .../UsePropertyAccessSyntaxIntention.kt | 6 +- .../kotlin/idea/j2k/J2kPostProcessings.kt | 93 +++++++++++ .../kotlin/idea/j2k/J2kPostProcessor.kt | 153 +++++++----------- j2k/testData/JavaApi.java | 3 + .../IfNullReturnToElvis.kt | 2 +- .../SyntheticExtensionPropertyAccess.java | 15 ++ .../SyntheticExtensionPropertyAccess.kt | 15 ++ ...otlinConverterForWebDemoTestGenerated.java | 6 + ...otlinConverterSingleFileTestGenerated.java | 6 + 9 files changed, 204 insertions(+), 95 deletions(-) create mode 100644 idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessings.kt create mode 100644 j2k/testData/fileOrElement/codeSimplifications/SyntheticExtensionPropertyAccess.java create mode 100644 j2k/testData/fileOrElement/codeSimplifications/SyntheticExtensionPropertyAccess.kt diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/UsePropertyAccessSyntaxIntention.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/UsePropertyAccessSyntaxIntention.kt index f162840b7f2..b51dd074be5 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/UsePropertyAccessSyntaxIntention.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/UsePropertyAccessSyntaxIntention.kt @@ -16,7 +16,6 @@ package org.jetbrains.kotlin.idea.intentions -import com.intellij.codeInspection.ProblemHighlightType import com.intellij.openapi.editor.Editor import org.jetbrains.kotlin.descriptors.DeclarationDescriptor import org.jetbrains.kotlin.descriptors.DeclarationDescriptorWithVisibility @@ -28,7 +27,6 @@ import org.jetbrains.kotlin.idea.codeInsight.ReferenceVariantsHelper import org.jetbrains.kotlin.idea.core.getResolutionScope import org.jetbrains.kotlin.idea.core.isVisible import org.jetbrains.kotlin.idea.inspections.IntentionBasedInspection -import org.jetbrains.kotlin.load.java.descriptors.JavaClassDescriptor import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.psiUtil.getQualifiedExpressionForSelector @@ -49,6 +47,10 @@ class UsePropertyAccessSyntaxIntention : JetSelfTargetingOffsetIndependentIntent } override fun applyTo(element: JetCallExpression, editor: Editor) { + applyTo(element) + } + + public fun applyTo(element: JetCallExpression) { val propertyName = findExtensionPropertyToUse(element)!!.getName() val arguments = element.getValueArguments() when (arguments.size()) { diff --git a/idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessings.kt b/idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessings.kt new file mode 100644 index 00000000000..e48c021e5ea --- /dev/null +++ b/idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessings.kt @@ -0,0 +1,93 @@ +/* + * Copyright 2010-2015 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.j2k + +import org.jetbrains.kotlin.idea.intentions.* +import org.jetbrains.kotlin.idea.intentions.branchedTransformations.intentions.IfThenToElvisIntention +import org.jetbrains.kotlin.idea.intentions.branchedTransformations.intentions.IfThenToSafeAccessIntention +import org.jetbrains.kotlin.psi.* +import java.util.ArrayList + +interface J2kPostProcessing { + fun createAction(element: JetElement): (() -> Unit)? +} + +object J2KPostProcessingRegistrar { + private val _processings = ArrayList() + + val processings: Collection + get() = _processings + + init { + _processings.add(RemoveExplicitTypeArgumentsProcessing()) + _processings.add(MoveLambdaOutsideParenthesesProcessing()) + _processings.add(ConvertToStringTemplateProcessing()) + + registerTypicalIntentionBasedProcessing(UsePropertyAccessSyntaxIntention()) { applyTo(it) } + registerTypicalIntentionBasedProcessing(IfThenToSafeAccessIntention()) { applyTo(it) } + registerTypicalIntentionBasedProcessing(IfThenToElvisIntention()) { applyTo(it) } + registerTypicalIntentionBasedProcessing(IfNullToElvisIntention()) { applyTo(it) } + registerTypicalIntentionBasedProcessing(SimplifyNegatedBinaryExpressionIntention()) { applyTo(it) } + } + + private inline fun > registerTypicalIntentionBasedProcessing( + intention: TIntention, + inlineOptions(InlineOption.ONLY_LOCAL_RETURN) apply: TIntention.(TElement) -> Unit + ) { + _processings.add(object : J2kPostProcessing { + override fun createAction(element: JetElement): (() -> Unit)? { + if (!javaClass().isInstance(element)) return null + @suppress("UNCHECKED_CAST") + if (intention.applicabilityRange(element as TElement) == null) return null + return { intention.apply(element) } + } + }) + } + + private class RemoveExplicitTypeArgumentsProcessing : J2kPostProcessing { + override fun createAction(element: JetElement): (() -> Unit)? { + if (element !is JetTypeArgumentList || !RemoveExplicitTypeArgumentsIntention.isApplicableTo(element, approximateFlexible = true)) return null + + return { element.delete() } + } + } + + private class MoveLambdaOutsideParenthesesProcessing : J2kPostProcessing { + private val intention = MoveLambdaOutsideParenthesesIntention() + + override fun createAction(element: JetElement): (() -> Unit)? { + if (element !is JetCallExpression) return null + val literalArgument = element.valueArguments.lastOrNull()?.getArgumentExpression()?.unpackFunctionLiteral() ?: return null + if (!intention.isApplicableTo(element, literalArgument.textOffset)) return null + return { intention.applyTo(element) } + } + } + + private class ConvertToStringTemplateProcessing : J2kPostProcessing { + private val intention = ConvertToStringTemplateIntention() + + override fun createAction(element: JetElement): (() -> Unit)? { + if (element is JetBinaryExpression && intention.isApplicableTo(element) && intention.isConversionResultSimple(element)) { + return { intention.applyTo(element) } + } + else { + return null + } + } + } + +} \ No newline at end of file diff --git a/idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessor.kt b/idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessor.kt index 04a3759a5fc..404210c81b1 100644 --- a/idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessor.kt +++ b/idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessor.kt @@ -19,16 +19,15 @@ package org.jetbrains.kotlin.idea.j2k import com.intellij.openapi.editor.RangeMarker import com.intellij.openapi.util.TextRange import com.intellij.psi.PsiElement +import com.intellij.psi.PsiRecursiveElementVisitor import com.intellij.psi.codeStyle.CodeStyleManager import com.intellij.psi.search.LocalSearchScope import com.intellij.psi.search.searches.ReferencesSearch +import com.intellij.util.SmartList import org.jetbrains.kotlin.diagnostics.Diagnostic import org.jetbrains.kotlin.diagnostics.Errors import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade import org.jetbrains.kotlin.idea.caches.resolve.resolveImportReference -import org.jetbrains.kotlin.idea.intentions.* -import org.jetbrains.kotlin.idea.intentions.branchedTransformations.intentions.IfThenToElvisIntention -import org.jetbrains.kotlin.idea.intentions.branchedTransformations.intentions.IfThenToSafeAccessIntention import org.jetbrains.kotlin.idea.quickfix.RemoveModifierFix import org.jetbrains.kotlin.idea.quickfix.RemoveRightPartOfBinaryExpressionFix import org.jetbrains.kotlin.idea.references.mainReference @@ -38,7 +37,8 @@ import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.psiUtil.elementsInRange import org.jetbrains.kotlin.resolve.BindingContext -import java.util.ArrayList +import java.util.HashSet +import java.util.LinkedHashMap public class J2kPostProcessor(private val formatCode: Boolean) : PostProcessor { override fun analyzeFile(file: JetFile, range: TextRange?): BindingContext { @@ -90,101 +90,25 @@ public class J2kPostProcessor(private val formatCode: Boolean) : PostProcessor { } override fun doAdditionalProcessing(file: JetFile, rangeMarker: RangeMarker?) { - fun rangeFilter(element: PsiElement): RangeFilterResult { - if (rangeMarker == null) return RangeFilterResult.PROCESS - if (!rangeMarker.isValid()) return RangeFilterResult.SKIP - val range = TextRange(rangeMarker.getStartOffset(), rangeMarker.getEndOffset()) - val elementRange = element.getTextRange() - return when { - range.contains(elementRange) -> RangeFilterResult.PROCESS - range.intersects(elementRange) -> RangeFilterResult.GO_INSIDE - else -> RangeFilterResult.SKIP - } - } + var elementToActions = collectAvailableActions(file, rangeMarker) - val redundantTypeArgs = ArrayList() - file.accept(object : JetTreeVisitorVoid(){ - override fun visitElement(element: PsiElement) { - if (rangeFilter(element) != RangeFilterResult.SKIP) { - super.visitElement(element) - } - } + while (elementToActions.isNotEmpty()) { + val processingsToRerun = HashSet() - override fun visitTypeArgumentList(typeArgumentList: JetTypeArgumentList) { - if (rangeFilter(typeArgumentList) == RangeFilterResult.PROCESS && RemoveExplicitTypeArgumentsIntention.isApplicableTo(typeArgumentList, approximateFlexible = true)) { - redundantTypeArgs.add(typeArgumentList) - return - } - - super.visitTypeArgumentList(typeArgumentList) - } - - override fun visitPrefixExpression(expression: JetPrefixExpression) { - super.visitPrefixExpression(expression) - - val intention = SimplifyNegatedBinaryExpressionIntention() - if (rangeFilter(expression) == RangeFilterResult.PROCESS && intention.isApplicableTo(expression)) { - intention.applyTo(expression) - } - } - - override fun visitIfExpression(expression: JetIfExpression) { - super.visitIfExpression(expression) - - if (rangeFilter(expression) == RangeFilterResult.PROCESS) { - run { - val intention = IfThenToSafeAccessIntention() - if (intention.isApplicableTo(expression)) { - intention.applyTo(expression) - return - } + for ((element, actions) in elementToActions) { + for ((action, processing) in actions) { + if (element.isValid) { + action() } - - run { - val intention = IfThenToElvisIntention() - if (intention.isApplicableTo(expression)) { - intention.applyTo(expression) - return - } - } - - run { - val intention = IfNullToElvisIntention() - if (intention.applicabilityRange(expression) != null) { - intention.applyTo(expression) - return - } - } - - } - } - - override fun visitBinaryExpression(expression: JetBinaryExpression) { - val intention = ConvertToStringTemplateIntention() - if (intention.isApplicableTo(expression) && intention.isConversionResultSimple(expression)) { - val result = intention.applyTo(expression) - result.accept(this) - } - else { - super.visitBinaryExpression(expression) - } - } - - override fun visitCallExpression(expression: JetCallExpression) { - super.visitCallExpression(expression) - - val literalArgument = expression.valueArguments.lastOrNull()?.getArgumentExpression()?.unpackFunctionLiteral() - if (literalArgument != null) { - val intention = MoveLambdaOutsideParenthesesIntention() - if (intention.isApplicableTo(expression, literalArgument.textOffset)) { - intention.applyTo(expression) + else { + processingsToRerun.add(processing) } } } - }) - for (typeArgs in redundantTypeArgs) { - typeArgs.delete() + if (processingsToRerun.isEmpty()) break + //TODO: it looks like there are no such cases currently, add tests later on or drop this + elementToActions = collectAvailableActions(file, rangeMarker, processingFilter = { it in processingsToRerun }) } if (formatCode) { @@ -200,5 +124,50 @@ public class J2kPostProcessor(private val formatCode: Boolean) : PostProcessor { } } + private data class ActionData(val action: () -> Unit, val processing: J2kPostProcessing) + + private fun collectAvailableActions( + file: JetFile, + rangeMarker: RangeMarker?, + processingFilter: (J2kPostProcessing) -> Boolean = { true } + ): LinkedHashMap> { + val processings = J2KPostProcessingRegistrar.processings.filter(processingFilter) + val elementToActions = LinkedHashMap>() + + file.accept(object : PsiRecursiveElementVisitor(){ + override fun visitElement(element: PsiElement) { + if (element is JetElement) { + val rangeResult = rangeFilter(element, rangeMarker) + if (rangeResult == RangeFilterResult.SKIP) return + + super.visitElement(element) + + if (rangeResult == RangeFilterResult.PROCESS) { + processings.forEach { processing -> + val action = processing.createAction(element) + if (action != null) { + elementToActions.getOrPut(element) { SmartList() }.add(ActionData(action, processing)) + } + } + } + } + } + }) + + return elementToActions + } + + private fun rangeFilter(element: PsiElement, rangeMarker: RangeMarker?): RangeFilterResult { + if (rangeMarker == null) return RangeFilterResult.PROCESS + if (!rangeMarker.isValid) return RangeFilterResult.SKIP + val range = TextRange(rangeMarker.startOffset, rangeMarker.endOffset) + val elementRange = element.textRange + return when { + range.contains(elementRange) -> RangeFilterResult.PROCESS + range.intersects(elementRange) -> RangeFilterResult.GO_INSIDE + else -> RangeFilterResult.SKIP + } + } + override fun simpleNameReference(nameExpression: JetSimpleNameExpression) = nameExpression.mainReference } diff --git a/j2k/testData/JavaApi.java b/j2k/testData/JavaApi.java index 27e97d9e614..98f8f923624 100644 --- a/j2k/testData/JavaApi.java +++ b/j2k/testData/JavaApi.java @@ -49,6 +49,9 @@ public enum E { public class Base { public @Nullable String foo(@Nullable String s) { return s; } + + public int getProperty() { return 1; } + public void setProperty(int value) {} } public class Derived extends Base { diff --git a/j2k/testData/fileOrElement/codeSimplifications/IfNullReturnToElvis.kt b/j2k/testData/fileOrElement/codeSimplifications/IfNullReturnToElvis.kt index f5c66c3c793..82659db5ca9 100644 --- a/j2k/testData/fileOrElement/codeSimplifications/IfNullReturnToElvis.kt +++ b/j2k/testData/fileOrElement/codeSimplifications/IfNullReturnToElvis.kt @@ -2,7 +2,7 @@ import java.io.File class C { fun foo(file: File): String { - val parent = file.getParentFile() ?: return "" + val parent = file.parentFile ?: return "" return parent.getName() } } diff --git a/j2k/testData/fileOrElement/codeSimplifications/SyntheticExtensionPropertyAccess.java b/j2k/testData/fileOrElement/codeSimplifications/SyntheticExtensionPropertyAccess.java new file mode 100644 index 00000000000..3242942c688 --- /dev/null +++ b/j2k/testData/fileOrElement/codeSimplifications/SyntheticExtensionPropertyAccess.java @@ -0,0 +1,15 @@ +import javaApi.Base; + +class C extends Base { + public void f() { + Base other = Base(); + int value = other.getProperty() + getProperty(); + other.setProperty(1); + setProperty(other.getProperty() + value); + getBase(getProperty()).setProperty(0); + } + + private Base getBase(int i) { + return new Base(); + } +} diff --git a/j2k/testData/fileOrElement/codeSimplifications/SyntheticExtensionPropertyAccess.kt b/j2k/testData/fileOrElement/codeSimplifications/SyntheticExtensionPropertyAccess.kt new file mode 100644 index 00000000000..ce07aff8581 --- /dev/null +++ b/j2k/testData/fileOrElement/codeSimplifications/SyntheticExtensionPropertyAccess.kt @@ -0,0 +1,15 @@ +import javaApi.Base + +class C : Base() { + public fun f() { + val other = Base() + val value = other.property + property + other.property = 1 + property = other.property + value + getBase(property).property = 0 + } + + private fun getBase(i: Int): Base { + return Base() + } +} diff --git a/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterForWebDemoTestGenerated.java b/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterForWebDemoTestGenerated.java index 69fd68d7840..af0b109d05b 100644 --- a/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterForWebDemoTestGenerated.java +++ b/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterForWebDemoTestGenerated.java @@ -990,6 +990,12 @@ public class JavaToKotlinConverterForWebDemoTestGenerated extends AbstractJavaTo String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/codeSimplifications/RedundantTypeCastAndInline.java"); doTest(fileName); } + + @TestMetadata("SyntheticExtensionPropertyAccess.java") + public void testSyntheticExtensionPropertyAccess() throws Exception { + String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/codeSimplifications/SyntheticExtensionPropertyAccess.java"); + doTest(fileName); + } } @TestMetadata("j2k/testData/fileOrElement/comments") diff --git a/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterSingleFileTestGenerated.java b/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterSingleFileTestGenerated.java index 2982b198bdf..18d36d159c2 100644 --- a/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterSingleFileTestGenerated.java +++ b/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterSingleFileTestGenerated.java @@ -990,6 +990,12 @@ public class JavaToKotlinConverterSingleFileTestGenerated extends AbstractJavaTo String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/codeSimplifications/RedundantTypeCastAndInline.java"); doTest(fileName); } + + @TestMetadata("SyntheticExtensionPropertyAccess.java") + public void testSyntheticExtensionPropertyAccess() throws Exception { + String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/codeSimplifications/SyntheticExtensionPropertyAccess.java"); + doTest(fileName); + } } @TestMetadata("j2k/testData/fileOrElement/comments")