IDEA-259107 extract method: don't rely on text offsets to link generated elements

GitOrigin-RevId: 382b49a3927e84a463350abf9ed0d36ad76ab633
This commit is contained in:
Alexandr Suhinin
2021-03-03 11:38:41 +00:00
committed by intellij-monorepo-bot
parent 2b2ad50680
commit dff4a758f6
7 changed files with 107 additions and 45 deletions
@@ -8,7 +8,6 @@ import com.intellij.psi.util.PsiTreeUtil
import com.intellij.psi.util.PsiUtil
import com.intellij.psi.util.TypeConversionUtil
import com.intellij.refactoring.extractMethod.newImpl.ExtractMethodHelper.createDeclaration
import com.intellij.refactoring.extractMethod.newImpl.ExtractMethodHelper.findInCopy
import com.intellij.refactoring.extractMethod.newImpl.ExtractMethodHelper.findTopmostParenthesis
import com.intellij.refactoring.extractMethod.newImpl.structures.DataOutput
import com.intellij.refactoring.extractMethod.newImpl.structures.DataOutput.*
@@ -81,11 +80,7 @@ class BodyBuilder(private val factory: PsiElementFactory) {
}
private fun wrapExpression(expression: PsiExpression, shouldBeReturned: Boolean): Pair<PsiStatement, PsiExpression> {
val statement = if (shouldBeReturned) {
factory.createStatementFromText("return ${expression.text};", expression.context)
} else {
factory.createStatementFromText("${expression.text};", expression.context)
}
val statement = if (shouldBeReturned) createReturnStatement(expression) else createExpressionStatement(expression)
val block = factory.createCodeBlockFromText("{\n}", expression.context)
val addedStatement = block.add(statement) as PsiStatement
val inCopyExpression = when (addedStatement) {
@@ -96,12 +91,16 @@ class BodyBuilder(private val factory: PsiElementFactory) {
return Pair(addedStatement, inCopyExpression)
}
private fun findInputParameterInCopy(source: PsiElement, copy: PsiElement, parameter: InputParameter): InputParameter {
return InputParameter(
references = parameter.references.map { reference -> findInCopy(source, copy, reference) },
name = parameter.name,
type = parameter.type
)
private fun createReturnStatement(returnExpression: PsiExpression): PsiReturnStatement {
val statement = factory.createStatementFromText("return dummy;", returnExpression.context) as PsiReturnStatement
statement.returnValue?.replace(returnExpression)
return statement
}
private fun createExpressionStatement(callExpression: PsiExpression): PsiExpressionStatement {
val expressionStatement = factory.createStatementFromText("dummy();", callExpression.context) as PsiExpressionStatement
expressionStatement.expression.replace(callExpression)
return expressionStatement
}
fun copyOf(elements: List<PsiElement>): List<PsiElement> {
@@ -125,11 +124,15 @@ class BodyBuilder(private val factory: PsiElementFactory) {
val normalizedExpression = PsiUtil.skipParenthesizedExprDown(expression)
if (normalizedExpression != null) {
require(dataOutput is ExpressionOutput)
val (wrappedStatement, wrappedExpression) = wrapExpression(normalizedExpression, dataOutput.type != PsiType.VOID)
val wrappedInputParameters = inputParameters.map { parameter -> findInputParameterInCopy(normalizedExpression, wrappedExpression, parameter) }
val parameterMarkers = inputParameters.associateWith { parameter -> PsiElementMark.createMarkers(parameter.references) }
val needsReturnStatement = dataOutput.type != PsiType.VOID
val (wrappedStatement, wrappedExpression) = wrapExpression(normalizedExpression, needsReturnStatement)
val wrappedParameters = parameterMarkers.entries.map { (parameter, markers) ->
parameter.copy(references = PsiElementMark.releaseMarkers(wrappedStatement, markers))
}
val wrappedFlowOutput = UnconditionalFlow(listOf(wrappedStatement), true)
val wrappedDataOutput = dataOutput.copy(returnExpressions = listOf(wrappedExpression))
return build(listOf(wrappedStatement), wrappedFlowOutput, wrappedDataOutput, wrappedInputParameters, disabledParameters, missedDeclarations)
return build(listOf(wrappedStatement), wrappedFlowOutput, wrappedDataOutput, wrappedParameters, disabledParameters, missedDeclarations)
}
val blockStatement = elements.singleOrNull() as? PsiBlockStatement
@@ -139,22 +142,28 @@ class BodyBuilder(private val factory: PsiElementFactory) {
.dropWhile { it is PsiWhiteSpace }
.dropLastWhile { it is PsiWhiteSpace }
val copy = copyOf(normalizedElements)
val exitStatementsMarkers = PsiElementMark.createMarkers(flowOutput.statements)
val parameterMarkers = inputParameters.associateWith { parameter -> PsiElementMark.createMarkers(parameter.references) }
val exitCopies = flowOutput.statements.map { statement -> findInCopy(normalizedElements.first(), copy.first(), statement) }
val copy = copyOf(normalizedElements)
val block = copy.first().parent as PsiCodeBlock
val exitStatementsInCopy = PsiElementMark.releaseMarkers(block, exitStatementsMarkers)
val inCopyFlowOutput = when (flowOutput) {
is ConditionalFlow -> ConditionalFlow(exitCopies)
is UnconditionalFlow -> UnconditionalFlow(exitCopies, flowOutput.isDefaultExit)
is ConditionalFlow -> ConditionalFlow(exitStatementsInCopy)
is UnconditionalFlow -> UnconditionalFlow(exitStatementsInCopy, flowOutput.isDefaultExit)
EmptyFlow -> EmptyFlow
}
val inCopyInputGroups = inputParameters.map { parameter -> findInputParameterInCopy(normalizedElements.first(), copy.first(), parameter) }
val inCopyInputGroups = parameterMarkers.entries.map { (parameter, marks) ->
parameter.copy(references = PsiElementMark.releaseMarkers(block, marks))
}
val exitSubstitution = findExitReplacements(inCopyFlowOutput, dataOutput)
val inputReplacements = inCopyInputGroups.map { createInputReplacements(it) }.flatten()
val requiredDeclarations = missedDeclarations.map { createDeclaration(it) }
(inputReplacements + exitSubstitution).forEach { (source, target) -> source.replace(target) }
val block = copy.first().parent as PsiCodeBlock
castNumericReturns(block, dataOutput.type)
val defaultReturn = findDefaultReturn(dataOutput, flowOutput)
@@ -1,7 +1,6 @@
// Copyright 2000-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
package com.intellij.refactoring.extractMethod.newImpl
import com.intellij.codeInsight.CodeInsightUtil
import com.intellij.codeInsight.Nullability
import com.intellij.codeInsight.NullableNotNullManager
import com.intellij.codeInsight.PsiEquivalenceUtil
@@ -23,7 +22,6 @@ import com.intellij.refactoring.extractMethod.newImpl.structures.DataOutput.*
import com.intellij.refactoring.extractMethod.newImpl.structures.ExtractOptions
import com.intellij.refactoring.extractMethod.newImpl.structures.InputParameter
import com.intellij.refactoring.util.RefactoringUtil
import java.util.LinkedHashSet
object ExtractMethodHelper {
@@ -99,13 +97,6 @@ object ExtractMethodHelper {
return references.asSequence().mapNotNull { reference -> (reference.resolve() as? PsiVariable) }
}
fun <T: PsiElement> findInCopy(firstInSource: PsiElement, firstInCopy: PsiElement, element: T): T {
val sourceStartOffset: Int = firstInSource.textRange.startOffset
val copyStartOffset: Int = firstInCopy.textRange.startOffset
val range = element.textRange.shiftRight(copyStartOffset - sourceStartOffset)
return CodeInsightUtil.findElementInRange(firstInCopy.containingFile, range.startOffset, range.endOffset, element.javaClass)
}
fun hasConflictResolve(name: String?, scopeToIgnore: List<PsiElement>): Boolean {
require(scopeToIgnore.isNotEmpty())
if (name == null) return false
@@ -237,3 +228,30 @@ object ExtractMethodHelper {
.map { propertyName -> suggestGetterName(propertyName) }
}
}
/**
* Tracks [PsiElement] inside the copied or modified tree.
*/
class PsiElementMark<T: PsiElement> {
companion object {
fun <T: PsiElement> createMarkers(elements: List<T>): List<PsiElementMark<T>> {
return elements.map(::createMarker)
}
fun <T: PsiElement> releaseMarkers(root: PsiElement, marks: List<PsiElementMark<T>>): List<T> {
return marks.map { mark -> releaseMarker(root, mark) }
}
fun <T: PsiElement> createMarker(element: T): PsiElementMark<T> {
val mark = PsiElementMark<T>()
PsiTreeUtil.mark(element, mark)
return mark
}
@Suppress("UNCHECKED_CAST")
fun <T: PsiElement> releaseMarker(root: PsiElement, mark: PsiElementMark<T>): T {
return PsiTreeUtil.releaseMark(root, mark) as T
}
}
}
@@ -18,22 +18,22 @@ public class AnnotationArgConverter {
private void newMethod(PsiAnnotationMemberValue value, final StringBuilder buffer) {
value.accept(new JavaElementVisitor() {
@Override
public void visitExpression(PsiExpression expression) {
buffer.append(expression.getText());
@Override
public void visitExpression(PsiExpression expression) {
buffer.append(expression.getText());
}
@Override
public void visitNewExpression(PsiNewExpression expression) {
PsiArrayInitializerExpression arrayInitializer = expression.getArrayInitializer();
if (arrayInitializer == null) {
super.visitNewExpression(expression);
}
@Override
public void visitNewExpression(PsiNewExpression expression) {
PsiArrayInitializerExpression arrayInitializer = expression.getArrayInitializer();
if (arrayInitializer == null) {
super.visitNewExpression(expression);
}
else {
buffer.append(")");
}
else {
buffer.append(")");
}
}
});
}
}
@@ -11,7 +11,7 @@ class Test {
@NotNull
private Supplier newMethod() {
return (s) -> {
System.out.println(s);
System.out.println(s);
};
}
@@ -0,0 +1,13 @@
class Test {
static class B {
int getX(){
return 42;
}
}
void test(){
<selection>int x = new Test.B().getX();</selection>
System.out.println(x);
}
}
@@ -0,0 +1,18 @@
class Test {
static class B {
int getX(){
return 42;
}
}
void test(){
int x = newMethod();
System.out.println(x);
}
private int newMethod() {
int x = new B().getX();
return x;
}
}
@@ -1426,6 +1426,10 @@ public class ExtractMethodNewTest extends LightJavaCodeInsightTestCase {
}
}
public void testNestedReference() throws Exception {
doTest();
}
public void testQualifyWhenConflictingNamePresent() throws Exception {
final CommonCodeStyleSettings settings = CodeStyle.getSettings(getProject()).getCommonSettings(JavaLanguage.INSTANCE);
settings.ELSE_ON_NEW_LINE = true;