diff --git a/idea/src/org/jetbrains/jet/plugin/refactoring/JetRefactoringBundle.properties b/idea/src/org/jetbrains/jet/plugin/refactoring/JetRefactoringBundle.properties index 8d3e9f1c21c..697d86843c6 100644 --- a/idea/src/org/jetbrains/jet/plugin/refactoring/JetRefactoringBundle.properties +++ b/idea/src/org/jetbrains/jet/plugin/refactoring/JetRefactoringBundle.properties @@ -36,4 +36,5 @@ refactoring.move.non.kotlin.file=Target must be a Kotlin file package.private.0.will.no.longer.be.accessible.from.1=Package-private {0} will no longer be accessible from {1} 0.uses.package.private.1={0} uses package-private {1} -0.will.no.longer.be.accessible.after.extraction={0} will no longer be accessible after extraction \ No newline at end of file +0.will.no.longer.be.accessible.after.extraction={0} will no longer be accessible after extraction +0.will.become.invisible.after.extraction={0} will become invisible after extraction \ No newline at end of file diff --git a/idea/src/org/jetbrains/jet/plugin/refactoring/extractFunction/ExtractionData.kt b/idea/src/org/jetbrains/jet/plugin/refactoring/extractFunction/ExtractionData.kt index 5931e4baff0..7199e2d28ac 100644 --- a/idea/src/org/jetbrains/jet/plugin/refactoring/extractFunction/ExtractionData.kt +++ b/idea/src/org/jetbrains/jet/plugin/refactoring/extractFunction/ExtractionData.kt @@ -106,10 +106,6 @@ class ExtractionData( } fun getBrokenReferencesInfo(body: JetBlockExpression): List { - fun compareDescriptors(d1: DeclarationDescriptor?, d2: DeclarationDescriptor?): Boolean { - return d1 == d2 || (d1 != null && d2 != null && DescriptorRenderer.FQNAMES_IN_TYPES.render(d1) == DescriptorRenderer.FQNAMES_IN_TYPES.render(d2)) - } - val startOffset = body.getStatements().first!!.getTextRange()!!.getStartOffset() val referencesInfo = ArrayList() @@ -137,3 +133,9 @@ class ExtractionData( fun getInferredResultType(): JetType? = getExpressions().last?.let { AnalyzerFacadeWithCache.getContextForElement(it)[BindingContext.EXPRESSION_TYPE, it] } } + +private fun compareDescriptors(d1: DeclarationDescriptor?, d2: DeclarationDescriptor?): Boolean { + return d1 == d2 || + (d1 != null && d2 != null && + DescriptorRenderer.FQNAMES_IN_TYPES.render(d1) == DescriptorRenderer.FQNAMES_IN_TYPES.render(d2)) +} \ No newline at end of file diff --git a/idea/src/org/jetbrains/jet/plugin/refactoring/extractFunction/ExtractionDescriptor.kt b/idea/src/org/jetbrains/jet/plugin/refactoring/extractFunction/ExtractionDescriptor.kt index cc79c53694a..62f9eedc8a8 100644 --- a/idea/src/org/jetbrains/jet/plugin/refactoring/extractFunction/ExtractionDescriptor.kt +++ b/idea/src/org/jetbrains/jet/plugin/refactoring/extractFunction/ExtractionDescriptor.kt @@ -27,6 +27,8 @@ import org.jetbrains.jet.lang.psi.JetThisExpression import org.jetbrains.jet.plugin.references.JetSimpleNameReference import org.jetbrains.jet.lang.resolve.name.FqName import org.jetbrains.jet.plugin.references.JetSimpleNameReference.ShorteningMode +import org.jetbrains.jet.lang.psi.psiUtil.replaced +import org.jetbrains.jet.lang.psi.JetQualifiedExpression data class Parameter( val argumentText: String, @@ -38,7 +40,7 @@ data class Parameter( val nameForRef: String get() = mirrorVarName ?: name } -trait Replacement: Function1 +trait Replacement: Function1 trait ParameterReplacement : Replacement { val parameter: Parameter @@ -49,9 +51,9 @@ class RenameReplacement(override val parameter: Parameter): ParameterReplacement override fun copy(parameter: Parameter) = RenameReplacement(parameter) [suppress("PARAMETER_NAME_CHANGED_ON_OVERRIDE")] - override fun invoke(e: JetElement) { + override fun invoke(e: JetElement): JetElement { val thisExpr = e.getParent() as? JetThisExpression - (thisExpr ?: e).replace(JetPsiFactory.createSimpleName(e.getProject(), parameter.nameForRef)) + return (thisExpr ?: e).replaced(JetPsiFactory.createSimpleName(e.getProject(), parameter.nameForRef)) } } @@ -59,16 +61,21 @@ class AddPrefixReplacement(override val parameter: Parameter): ParameterReplacem override fun copy(parameter: Parameter) = AddPrefixReplacement(parameter) [suppress("PARAMETER_NAME_CHANGED_ON_OVERRIDE")] - override fun invoke(e: JetElement) { + override fun invoke(e: JetElement): JetElement { val selector = (e.getParent() as? JetCallExpression) ?: e - selector.replace(JetPsiFactory.createExpression(e.getProject(), "${parameter.nameForRef}.${selector.getText()}")) + val newExpr = selector.replace( + JetPsiFactory.createExpression(e.getProject(), "${parameter.nameForRef}.${selector.getText()}") + ) as JetQualifiedExpression + + return with(newExpr.getSelectorExpression()!!) { if (this is JetCallExpression) getCalleeExpression()!! else this } } } class FqNameReplacement(val fqName: FqName): Replacement { [suppress("PARAMETER_NAME_CHANGED_ON_OVERRIDE")] - override fun invoke(e: JetElement) { - (e.getReference() as? JetSimpleNameReference)?.bindToFqName(fqName, ShorteningMode.NO_SHORTENING) + override fun invoke(e: JetElement): JetElement { + val newExpr = (e.getReference() as? JetSimpleNameReference)?.bindToFqName(fqName, ShorteningMode.NO_SHORTENING) as JetElement + return if (newExpr is JetQualifiedExpression) newExpr.getSelectorExpression()!! else newExpr } } diff --git a/idea/src/org/jetbrains/jet/plugin/refactoring/extractFunction/extractFunctionUtils.kt b/idea/src/org/jetbrains/jet/plugin/refactoring/extractFunction/extractFunctionUtils.kt index 9d23b0385ca..3063783e710 100644 --- a/idea/src/org/jetbrains/jet/plugin/refactoring/extractFunction/extractFunctionUtils.kt +++ b/idea/src/org/jetbrains/jet/plugin/refactoring/extractFunction/extractFunctionUtils.kt @@ -82,6 +82,16 @@ import org.jetbrains.jet.lang.psi.JetTreeVisitorVoid import org.jetbrains.jet.lang.types.checker.JetTypeChecker import org.jetbrains.jet.lang.resolve.name.FqName import org.jetbrains.jet.lang.resolve.DescriptorUtils +import com.intellij.refactoring.util.RefactoringUIUtil +import org.jetbrains.jet.plugin.references.JetReference +import org.jetbrains.jet.lang.psi.psiUtil.replaced +import org.jetbrains.jet.lang.psi.psiUtil.isAncestor +import java.util.Collections +import com.intellij.psi.PsiNamedElement +import org.jetbrains.jet.lang.diagnostics.Severity +import org.jetbrains.jet.lang.diagnostics.Errors +import org.jetbrains.jet.lang.descriptors.impl.LocalVariableDescriptor +import org.jetbrains.jet.lang.types.ErrorUtils private val DEFAULT_FUNCTION_NAME = "myFun" private val DEFAULT_RETURN_TYPE = KotlinBuiltIns.getInstance().getUnitType() @@ -337,9 +347,6 @@ private fun ExtractionData.inferParametersInfo( replacementMap[refInfo.offsetInBody] = if (hasThisReceiver && extractThis) AddPrefixReplacement(parameter) else RenameReplacement(parameter) } - else if (originalDeclaration is JetProperty) { - return MaybeError(JetRefactoringBundle.message("cannot.extract.non.local.declaration.ref")!!) - } } } @@ -457,8 +464,57 @@ fun ExtractionData.performAnalysis(): Maybe { } fun ExtractionDescriptor.validate(): ExtractionDescriptorWithConflicts { - // todo: add name conflict checks (parameters, function, etc.) - return ExtractionDescriptorWithConflicts(this, MultiMap()) + val conflicts = MultiMap() + + val nameByOffset = HashMap() + val function = generateFunction(true, nameByOffset) + + val bindingContext = AnalyzerFacadeWithCache.getContextForElement(function.getBodyExpression()!!) + + for ((originalOffset, resolveResult) in extractionData.refOffsetToDeclaration) { + if (resolveResult.declaration in extractionData.originalElements) continue + + val currentRefExpr = nameByOffset[originalOffset] as JetSimpleNameExpression? + if (currentRefExpr == null) continue + + if (currentRefExpr.getParent() is JetThisExpression) continue + + val diagnostics = bindingContext.getDiagnostics().forElement(currentRefExpr) + + val currentDescriptor = bindingContext[BindingContext.REFERENCE_TARGET, currentRefExpr] + val currentTarget = currentDescriptor?.let { + DescriptorToDeclarationUtil.getDeclaration(extractionData.project, it, bindingContext) + } as? PsiNamedElement + if (currentTarget is JetParameter && currentTarget.getParent() == function.getValueParameterList()) continue + if (currentDescriptor is LocalVariableDescriptor + && parameters.any { it.mirrorVarName == currentDescriptor.getName().asString() }) continue + + if (diagnostics.any { it.getFactory() == Errors.UNRESOLVED_REFERENCE } + || (currentDescriptor != null + && !ErrorUtils.isError(currentDescriptor) + && !compareDescriptors(currentDescriptor, resolveResult.descriptor))) { + conflicts.putValue( + currentRefExpr, + JetRefactoringBundle.message( + "0.will.no.longer.be.accessible.after.extraction", + RefactoringUIUtil.getDescription(resolveResult.declaration, true) + ) + ) + continue + } + + diagnostics.firstOrNull { it.getFactory() == Errors.INVISIBLE_MEMBER }?.let { + conflicts.putValue( + currentRefExpr, + JetRefactoringBundle.message( + "0.will.become.invisible.after.extraction", + RefactoringUIUtil.getDescription(resolveResult.declaration, true) + ) + ) + } + } + + return ExtractionDescriptorWithConflicts(this, conflicts) } fun ExtractionDescriptor.getFunctionText( @@ -488,7 +544,29 @@ fun ExtractionDescriptor.getFunctionText( } } -fun ExtractionDescriptor.generateFunction(inTempFile: Boolean = false): JetNamedFunction { +fun createNameCounterpartMap(from: JetElement, to: JetElement): Map { + val map = HashMap() + + val fromOffset = from.getTextRange()!!.getStartOffset() + from.accept( + object: JetTreeVisitorVoid() { + override fun visitSimpleNameExpression(expression: JetSimpleNameExpression) { + val offset = expression.getTextRange()!!.getStartOffset() - fromOffset + val newExpression = to.findElementAt(offset)?.getParentByType(javaClass()) + assert(newExpression!= null, "Couldn't find expression at $offset in '${to.getText()}'") + + map[expression] = newExpression!! + } + } + ) + + return map +} + +fun ExtractionDescriptor.generateFunction( + inTempFile: Boolean = false, + nameByOffset: MutableMap = HashMap() +): JetNamedFunction { val project = extractionData.project fun createFunction(): JetNamedFunction { @@ -509,7 +587,8 @@ fun ExtractionDescriptor.generateFunction(inTempFile: Boolean = false): JetNamed fun adjustFunctionBody(function: JetNamedFunction) { val body = function.getBodyExpression() as JetBlockExpression - val exprReplacementMap = HashMap Unit>() + val exprReplacementMap = HashMap JetElement>() + val originalOffsetByExpr = HashMap() val range = body.getStatements().first?.getTextRange() if (range == null) return @@ -517,31 +596,46 @@ fun ExtractionDescriptor.generateFunction(inTempFile: Boolean = false): JetNamed val bodyOffset = range.getStartOffset() val file = body.getContainingFile()!! - for ((offsetInBody, replacement) in replacementMap) { - if (replacement is ParameterReplacement && replacement.parameter == receiverParameter) continue - + for ((offsetInBody, resolveResult) in extractionData.refOffsetToDeclaration) { val expr = file.findElementAt(bodyOffset + offsetInBody)?.getParentByType(javaClass()) assert(expr != null, "Couldn't find expression at $offsetInBody in '${body.getText()}'") - exprReplacementMap[expr!!] = replacement + originalOffsetByExpr[expr!!] = offsetInBody + + replacementMap[offsetInBody]?.let { replacement -> + if (replacement !is ParameterReplacement || replacement.parameter != receiverParameter) { + exprReplacementMap[expr] = replacement + } + } } + val replacingReturn: JetExpression? + val expressionsToReplaceWithReturn: List if (controlFlow is JumpBasedControlFlow) { - val replacementExpr = JetPsiFactory.createExpression( - project, - if (controlFlow is ConditionalJump) "return true" else "return" - ) - for (jumpElement in controlFlow.elementsToReplace) { + replacingReturn = JetPsiFactory.createExpression(project, if (controlFlow is ConditionalJump) "return true" else "return") + expressionsToReplaceWithReturn = controlFlow.elementsToReplace.map { jumpElement -> val offsetInBody = jumpElement.getTextRange()!!.getStartOffset() - extractionData.originalStartOffset!! val expr = file.findElementAt(bodyOffset + offsetInBody)?.getParentByType(jumpElement.javaClass) assert(expr != null, "Couldn't find expression at $offsetInBody in '${body.getText()}'") - exprReplacementMap[expr!!] = { it.replace(replacementExpr) } + expr!! + } + } + else { + replacingReturn = null + expressionsToReplaceWithReturn = Collections.emptyList() + } + + if (replacingReturn != null) { + for (expr in expressionsToReplaceWithReturn) { + expr.replace(replacingReturn) } } - for ((expr, replacement) in exprReplacementMap) { - replacement(expr) + for ((expr, originalOffset) in originalOffsetByExpr) { + if (expr.isValid()) { + nameByOffset.put(originalOffset, exprReplacementMap[expr]?.invoke(expr) ?: expr) + } } for (param in parameters) { @@ -558,7 +652,11 @@ fun ExtractionDescriptor.generateFunction(inTempFile: Boolean = false): JetNamed body.appendElement(JetPsiFactory.createReturn(project, "false")) is ExpressionEvaluation -> - body.getStatements().last?.let { it.replace(JetPsiFactory.createReturn(project, it.getText()!!)) } + body.getStatements().last?.let { + val newExpr = it.replaced(JetPsiFactory.createReturn(project, it.getText()!!)).getReturnedExpression()!! + val counterpartMap = createNameCounterpartMap(it, newExpr) + nameByOffset.entrySet().forEach { it.setValue(counterpartMap[it.getValue()]!!) } + } } } diff --git a/idea/testData/refactoring/extractFunction/basic/localClassFunctionRef.kt b/idea/testData/refactoring/extractFunction/basic/localClassFunctionRef.kt new file mode 100644 index 00000000000..90b8f164720 --- /dev/null +++ b/idea/testData/refactoring/extractFunction/basic/localClassFunctionRef.kt @@ -0,0 +1,8 @@ +// NEXT_SIBLING: +fun foo(a: Int): Int { + class A { + fun bar(): Int = a + 10 + } + + return A().bar() +} \ No newline at end of file diff --git a/idea/testData/refactoring/extractFunction/basic/localClassFunctionRef.kt.conflicts b/idea/testData/refactoring/extractFunction/basic/localClassFunctionRef.kt.conflicts new file mode 100644 index 00000000000..92c4bca0bb9 --- /dev/null +++ b/idea/testData/refactoring/extractFunction/basic/localClassFunctionRef.kt.conflicts @@ -0,0 +1 @@ +class foo.A will no longer be accessible after extraction \ No newline at end of file diff --git a/idea/testData/refactoring/extractFunction/basic/localClassPropertyRef.kt b/idea/testData/refactoring/extractFunction/basic/localClassPropertyRef.kt new file mode 100644 index 00000000000..ad6e1f21306 --- /dev/null +++ b/idea/testData/refactoring/extractFunction/basic/localClassPropertyRef.kt @@ -0,0 +1,8 @@ +// NEXT_SIBLING: +fun foo(a: Int): Int { + class A { + val bar: Int = a + 10 + } + + return A().bar +} \ No newline at end of file diff --git a/idea/testData/refactoring/extractFunction/basic/localClassPropertyRef.kt.conflicts b/idea/testData/refactoring/extractFunction/basic/localClassPropertyRef.kt.conflicts new file mode 100644 index 00000000000..92c4bca0bb9 --- /dev/null +++ b/idea/testData/refactoring/extractFunction/basic/localClassPropertyRef.kt.conflicts @@ -0,0 +1 @@ +class foo.A will no longer be accessible after extraction \ No newline at end of file diff --git a/idea/testData/refactoring/extractFunction/basic/localFunctionRef.kt b/idea/testData/refactoring/extractFunction/basic/localFunctionRef.kt new file mode 100644 index 00000000000..1c2d7bc01a3 --- /dev/null +++ b/idea/testData/refactoring/extractFunction/basic/localFunctionRef.kt @@ -0,0 +1,6 @@ +// NEXT_SIBLING: +fun foo(a: Int): Int { + fun bar(): Int = a + 10 + + return bar() +} \ No newline at end of file diff --git a/idea/testData/refactoring/extractFunction/basic/localFunctionRef.kt.conflicts b/idea/testData/refactoring/extractFunction/basic/localFunctionRef.kt.conflicts new file mode 100644 index 00000000000..b22b8f15e81 --- /dev/null +++ b/idea/testData/refactoring/extractFunction/basic/localFunctionRef.kt.conflicts @@ -0,0 +1 @@ +function foo.bar will no longer be accessible after extraction \ No newline at end of file diff --git a/idea/testData/refactoring/extractFunction/basic/misdirectedRef.kt b/idea/testData/refactoring/extractFunction/basic/misdirectedRef.kt new file mode 100644 index 00000000000..48da22b0002 --- /dev/null +++ b/idea/testData/refactoring/extractFunction/basic/misdirectedRef.kt @@ -0,0 +1,8 @@ +// NEXT_SIBLING: +fun bar(): Int = 100 + +fun foo(a: Int): Int { + fun bar(): Int = a + 10 + + return bar() +} \ No newline at end of file diff --git a/idea/testData/refactoring/extractFunction/basic/misdirectedRef.kt.conflicts b/idea/testData/refactoring/extractFunction/basic/misdirectedRef.kt.conflicts new file mode 100644 index 00000000000..b22b8f15e81 --- /dev/null +++ b/idea/testData/refactoring/extractFunction/basic/misdirectedRef.kt.conflicts @@ -0,0 +1 @@ +function foo.bar will no longer be accessible after extraction \ No newline at end of file diff --git a/idea/testData/refactoring/extractFunction/basic/privateMemberRef.kt b/idea/testData/refactoring/extractFunction/basic/privateMemberRef.kt new file mode 100644 index 00000000000..3e706cc35de --- /dev/null +++ b/idea/testData/refactoring/extractFunction/basic/privateMemberRef.kt @@ -0,0 +1,8 @@ +// NEXT_SIBLING: +class A { + private val t: Int = 1 + + fun foo(): Int { + return t + } +} \ No newline at end of file diff --git a/idea/testData/refactoring/extractFunction/basic/privateMemberRef.kt.conflicts b/idea/testData/refactoring/extractFunction/basic/privateMemberRef.kt.conflicts new file mode 100644 index 00000000000..a5ca9d5fe14 --- /dev/null +++ b/idea/testData/refactoring/extractFunction/basic/privateMemberRef.kt.conflicts @@ -0,0 +1 @@ +property A.t will become invisible after extraction \ No newline at end of file diff --git a/idea/testData/refactoring/extractFunction/basic/refInReturn.kt b/idea/testData/refactoring/extractFunction/basic/refInReturn.kt new file mode 100644 index 00000000000..0b3fab7895e --- /dev/null +++ b/idea/testData/refactoring/extractFunction/basic/refInReturn.kt @@ -0,0 +1,6 @@ +// NEXT_SIBLING: +fun foo(a: Int): Int { + println(a) + if (a > 0) return a + return -a +} \ No newline at end of file diff --git a/idea/testData/refactoring/extractFunction/basic/refInReturn.kt.after b/idea/testData/refactoring/extractFunction/basic/refInReturn.kt.after new file mode 100644 index 00000000000..87b310757f1 --- /dev/null +++ b/idea/testData/refactoring/extractFunction/basic/refInReturn.kt.after @@ -0,0 +1,11 @@ +// NEXT_SIBLING: +fun b(a: Int): Boolean { + println(a) + if (a > 0) return true + return false +} + +fun foo(a: Int): Int { + if (b(a)) return a + return -a +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/jet/plugin/refactoring/introduce/introduceVariable/JetExtractionTestGenerated.java b/idea/tests/org/jetbrains/jet/plugin/refactoring/introduce/introduceVariable/JetExtractionTestGenerated.java index 61013b5988c..27195742b07 100644 --- a/idea/tests/org/jetbrains/jet/plugin/refactoring/introduce/introduceVariable/JetExtractionTestGenerated.java +++ b/idea/tests/org/jetbrains/jet/plugin/refactoring/introduce/introduceVariable/JetExtractionTestGenerated.java @@ -198,6 +198,21 @@ public class JetExtractionTestGenerated extends AbstractJetExtractionTest { JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.TestsPackage", new File("idea/testData/refactoring/extractFunction/basic"), Pattern.compile("^(.+)\\.kt$"), true); } + @TestMetadata("localClassFunctionRef.kt") + public void testLocalClassFunctionRef() throws Exception { + doExtractFunctionTest("idea/testData/refactoring/extractFunction/basic/localClassFunctionRef.kt"); + } + + @TestMetadata("localClassPropertyRef.kt") + public void testLocalClassPropertyRef() throws Exception { + doExtractFunctionTest("idea/testData/refactoring/extractFunction/basic/localClassPropertyRef.kt"); + } + + @TestMetadata("localFunctionRef.kt") + public void testLocalFunctionRef() throws Exception { + doExtractFunctionTest("idea/testData/refactoring/extractFunction/basic/localFunctionRef.kt"); + } + @TestMetadata("malformedExpression.kt") public void testMalformedExpression() throws Exception { doExtractFunctionTest("idea/testData/refactoring/extractFunction/basic/malformedExpression.kt"); @@ -208,6 +223,21 @@ public class JetExtractionTestGenerated extends AbstractJetExtractionTest { doExtractFunctionTest("idea/testData/refactoring/extractFunction/basic/malformedStatements.kt"); } + @TestMetadata("misdirectedRef.kt") + public void testMisdirectedRef() throws Exception { + doExtractFunctionTest("idea/testData/refactoring/extractFunction/basic/misdirectedRef.kt"); + } + + @TestMetadata("privateMemberRef.kt") + public void testPrivateMemberRef() throws Exception { + doExtractFunctionTest("idea/testData/refactoring/extractFunction/basic/privateMemberRef.kt"); + } + + @TestMetadata("refInReturn.kt") + public void testRefInReturn() throws Exception { + doExtractFunctionTest("idea/testData/refactoring/extractFunction/basic/refInReturn.kt"); + } + } @TestMetadata("idea/testData/refactoring/extractFunction/controlFlow")