[extract method] clean up: rename changed expressions to parametrized expressions

GitOrigin-RevId: aa1c0b2d4f2c44f9fb823900c50851b2269c1ac6
This commit is contained in:
Alexandr Suhinin
2023-10-13 11:28:52 +03:00
committed by intellij-monorepo-bot
parent 101994d98e
commit 3f0fa71a39
2 changed files with 43 additions and 34 deletions

View File

@@ -6,11 +6,18 @@ import com.intellij.openapi.util.TextRange
import com.intellij.psi.*
import com.intellij.psi.util.PsiTreeUtil
data class ChangedExpression(val pattern: PsiExpression, val candidate: PsiExpression)
/**
* Represents a pair of expressions which are located in the same place inside the initial code and duplicate
* and which can be extracted into the same new parameter.
*
* @property pattern Parametrized expression inside the initial source code.
* @property candidate Parametrized expression inside the duplicate.
*/
data class ParametrizedExpression(val pattern: PsiExpression, val candidate: PsiExpression)
data class Duplicate(val pattern: List<PsiElement>, val candidate: List<PsiElement>, val changedExpressions: List<ChangedExpression>)
data class Duplicate(val pattern: List<PsiElement>, val candidate: List<PsiElement>, val parametrizedExpressions: List<ParametrizedExpression>)
class JavaDuplicatesFinder(pattern: List<PsiElement>, private val predefinedChanges: Set<PsiExpression> = emptySet()) {
class JavaDuplicatesFinder(pattern: List<PsiElement>, private val parametrizedExpressions: Set<PsiExpression> = emptySet()) {
companion object {
private val ELEMENT_IN_PHYSICAL_FILE = Key<PsiElement>("ELEMENT_IN_PHYSICAL_FILE")
@@ -32,8 +39,8 @@ class JavaDuplicatesFinder(pattern: List<PsiElement>, private val predefinedChan
private val pattern: List<PsiElement> = pattern.filterNot(::isNoise)
fun withPredefinedChanges(predefinedChanges: Set<PsiExpression>): JavaDuplicatesFinder {
return JavaDuplicatesFinder(pattern, this.predefinedChanges + predefinedChanges)
fun withParametrizedExpressions(parametrizedExpressions: Set<PsiExpression>): JavaDuplicatesFinder {
return JavaDuplicatesFinder(pattern, this.parametrizedExpressions + parametrizedExpressions)
}
fun findDuplicates(scope: PsiElement): List<Duplicate> {
@@ -94,9 +101,9 @@ class JavaDuplicatesFinder(pattern: List<PsiElement>, private val predefinedChan
}
fun tryExtractDuplicate(pattern: List<PsiElement>, candidate: List<PsiElement>): Duplicate? {
val changedExpressions = ArrayList<ChangedExpression>()
if (!traverseAndCollectChanges(pattern, candidate, changedExpressions)) return null
return removeInternalReferences(Duplicate(pattern, candidate, changedExpressions))
val parametrizedExpressions = ArrayList<ParametrizedExpression>()
if (!traverseAndCollectChanges(pattern, candidate, parametrizedExpressions)) return null
return removeInternalReferences(Duplicate(pattern, candidate, parametrizedExpressions))
}
private fun removeInternalReferences(duplicate: Duplicate): Duplicate? {
@@ -104,7 +111,7 @@ class JavaDuplicatesFinder(pattern: List<PsiElement>, private val predefinedChan
val candidateDeclarations = duplicate.candidate.flatMap { PsiTreeUtil.findChildrenOfType(it, PsiVariable::class.java) }
val declarationsMapping = patternDeclarations.zip(candidateDeclarations).toMap()
val changedExpressions = duplicate.changedExpressions.filterNot { (pattern, candidate) ->
val parametrizedExpressions = duplicate.parametrizedExpressions.filterNot { (pattern, candidate) ->
val patternVariable = (pattern as? PsiReferenceExpression)?.resolve()
val candidateVariable = (candidate as? PsiReferenceExpression)?.resolve()
if (patternVariable !in declarationsMapping.keys && candidateVariable !in declarationsMapping.values) return@filterNot false
@@ -112,30 +119,30 @@ class JavaDuplicatesFinder(pattern: List<PsiElement>, private val predefinedChan
return@filterNot true
}
if (ExtractMethodHelper.hasReferencesToScope(duplicate.pattern, changedExpressions.map{ change -> change.pattern }) ||
ExtractMethodHelper.hasReferencesToScope(duplicate.candidate, changedExpressions.map { change -> change.candidate })){
if (ExtractMethodHelper.hasReferencesToScope(duplicate.pattern, parametrizedExpressions.map{ change -> change.pattern }) ||
ExtractMethodHelper.hasReferencesToScope(duplicate.candidate, parametrizedExpressions.map { change -> change.candidate })){
return null
}
return duplicate.copy(changedExpressions = changedExpressions)
return duplicate.copy(parametrizedExpressions = parametrizedExpressions)
}
/**
* Does recursive equivalence check for [pattern] and [candidate] nodes.
* Puts parametrized expressions into [changedExpressions] list.
* Puts parametrized expressions into [parametrizedExpressions] list.
* @return false if [pattern] and [candidate] are not parametrized duplicates.
*/
private fun traverseAndCollectChanges(pattern: List<PsiElement>,
candidate: List<PsiElement>,
changedExpressions: MutableList<ChangedExpression>): Boolean {
parametrizedExpressions: MutableList<ParametrizedExpression>): Boolean {
if (candidate.size != pattern.size) return false
val notEqualElements = pattern.zip(candidate).filterNot { (pattern, candidate) ->
pattern !in predefinedChanges &&
pattern !in this.parametrizedExpressions &&
areNodesEquivalent(pattern, candidate) &&
traverseAndCollectChanges(childrenOf(pattern), childrenOf(candidate), changedExpressions)
traverseAndCollectChanges(childrenOf(pattern), childrenOf(candidate), parametrizedExpressions)
}
if (notEqualElements.any { (pattern, candidate) -> ! canBeReplaced(pattern, candidate) }) return false
changedExpressions += notEqualElements.map { (pattern, candidate) -> ChangedExpression(pattern as PsiExpression, candidate as PsiExpression) }
parametrizedExpressions += notEqualElements.map { (pattern, candidate) -> ParametrizedExpression(pattern as PsiExpression, candidate as PsiExpression) }
return true
}
@@ -183,7 +190,7 @@ class JavaDuplicatesFinder(pattern: List<PsiElement>, private val predefinedChan
}
private fun isOvercomplicated(duplicate: Duplicate): Boolean {
val singleChangedExpression = duplicate.changedExpressions.singleOrNull()?.pattern ?: return false
val singleChangedExpression = duplicate.parametrizedExpressions.singleOrNull()?.pattern ?: return false
val singleDeclaration = duplicate.pattern.singleOrNull() as? PsiDeclarationStatement
val variable = singleDeclaration?.declaredElements?.singleOrNull() as? PsiVariable
return variable?.initializer == singleChangedExpression

View File

@@ -82,7 +82,7 @@ class DuplicatesMethodExtractor(val extractOptions: ExtractOptions, val targetCl
val file = PsiDocumentManager.getInstance(project).getPsiFile(editor.document) ?: return
val calls = callsToReplace?.map { it.element!! } ?: return
val parameterExpressions = extractOptions.inputParameters.flatMap { parameter -> parameter.references }.toSet()
val finder = JavaDuplicatesFinder(extractOptions.elements).withPredefinedChanges(parameterExpressions)
val finder = JavaDuplicatesFinder(extractOptions.elements).withParametrizedExpressions(parameterExpressions)
var duplicates = finder
.findDuplicates(method.containingClass ?: file)
.filterNot { duplicate ->
@@ -92,8 +92,8 @@ class DuplicatesMethodExtractor(val extractOptions: ExtractOptions, val targetCl
//TODO check same data output
//TODO check same flow output (+ same return values)
val changedExpressions = duplicates.flatMap { it.changedExpressions.map(ChangedExpression::pattern) }
val duplicatesFinder = finder.withPredefinedChanges(changedExpressions.toSet())
val parametrizedExpressions = duplicates.flatMap { it.parametrizedExpressions.map(ParametrizedExpression::pattern) }
val duplicatesFinder = finder.withParametrizedExpressions(parametrizedExpressions.toSet())
val duplicatesWithUnifiedParameters = duplicates.mapNotNull { duplicatesFinder.tryExtractDuplicate(it.pattern, it.candidate) }
@@ -105,7 +105,7 @@ class DuplicatesMethodExtractor(val extractOptions: ExtractOptions, val targetCl
//TODO clean up
val initialParameters = extractOptions.inputParameters.flatMap(InputParameter::references).toSet()
val exactDuplicates = duplicates.filter { duplicate ->
duplicate.changedExpressions.all { changedExpression -> changedExpression.pattern in initialParameters }
duplicate.parametrizedExpressions.all { changedExpression -> changedExpression.pattern in initialParameters }
}
val oldMethodCall = findMethodCallInside(calls.firstOrNull())
val newMethodCall = findMethodCallInside(parametrizedExtraction.callElements.firstOrNull())
@@ -134,15 +134,7 @@ class DuplicatesMethodExtractor(val extractOptions: ExtractOptions, val targetCl
false -> emptyList()
}
val duplicatesExtractOptions = duplicates.map { duplicate ->
val expressionMap = duplicate.changedExpressions.associate { (pattern, candidate) -> pattern to candidate }
fun getMappedParameter(parameter: InputParameter): InputParameter {
val references = parameter.references.map { expression -> expressionMap[expression] ?: expression }
return parameter.copy(references = references)
}
findExtractOptions(duplicate.candidate).copy(inputParameters = parameters.map(::getMappedParameter))
}
val duplicatesExtractOptions = duplicates.map { duplicate -> createExtractDescriptor(duplicate, parameters) }
if (duplicatesExtractOptions.any { options -> options.isStatic }) {
runWriteAction {
@@ -164,6 +156,16 @@ class DuplicatesMethodExtractor(val extractOptions: ExtractOptions, val targetCl
}
}
private fun createExtractDescriptor(duplicate: Duplicate, parameters: List<InputParameter>): ExtractOptions {
val expressionMap = duplicate.parametrizedExpressions.associate { (pattern, candidate) -> pattern to candidate }
fun getMappedParameter(parameter: InputParameter): InputParameter {
val references = parameter.references.map { expression -> expressionMap[expression] ?: expression }
return parameter.copy(references = references)
}
return findExtractOptions(duplicate.candidate).copy(inputParameters = parameters.map(::getMappedParameter))
}
private fun isGoodSignatureChange(callBefore: List<PsiElement>, initialParameters: List<InputParameter>,
callAfter: List<PsiElement>, updatedParameters: List<InputParameter>): Boolean {
val sizeAfter = callAfter.sumOf(::calculateCodeLeafs)
@@ -208,7 +210,7 @@ class DuplicatesMethodExtractor(val extractOptions: ExtractOptions, val targetCl
private fun findNewParameters(parameters: List<InputParameter>, duplicates: List<Duplicate>): List<InputParameter> {
if (duplicates.isEmpty()) return parameters
val updatedParameters = duplicates.fold(parameters) { updatedParameters, duplicate ->
updateParameters(updatedParameters, duplicate.changedExpressions)
updateParameters(updatedParameters, duplicate.parametrizedExpressions)
}
return ExtractMethodPipeline.foldParameters(updatedParameters, LocalSearchScope(duplicates.first().pattern.toTypedArray()))
}
@@ -235,9 +237,9 @@ class DuplicatesMethodExtractor(val extractOptions: ExtractOptions, val targetCl
return confirmedDuplicates
}
private fun updateParameters(parameters: List<InputParameter>, changes: List<ChangedExpression>): List<InputParameter> {
private fun updateParameters(parameters: List<InputParameter>, changes: List<ParametrizedExpression>): List<InputParameter> {
val changeMap = changes.associate { (pattern, candidate) -> pattern to candidate }
val expressionGroups = groupEquivalentExpressions(changes.map(ChangedExpression::pattern))
val expressionGroups = groupEquivalentExpressions(changes.map(ParametrizedExpression::pattern))
val parametersGroupedByPatternExpressions = expressionGroups.map { expressionGroup ->
val parameter = parameters.firstOrNull { expressions -> isEqual(expressionGroup.first(), expressions.references.first()) }
parameter?.copy(references = expressionGroup) ?: inputParameterOf(expressionGroup)