Skip to content

Commit

Permalink
Improve detection of nested Extract and Move Method refactorings
Browse files Browse the repository at this point in the history 
pmd/pmd@72edaee
  • Loading branch information
@tsantalis
tsantalis committed Jan 10, 2025
1 parent d6dd518 commit 0437482
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 19 deletions.
24 changes: 24 additions & 0 deletions src/main/java/gr/uom/java/xmi/UMLOperation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
import gr.uom.java.xmi.decomposition.VariableDeclaration;
import gr.uom.java.xmi.diff.CodeRange;
import gr.uom.java.xmi.diff.StringDistance;
import gr.uom.java.xmi.diff.UMLClassBaseDiff;
import gr.uom.java.xmi.diff.UMLModelDiff;
import gr.uom.java.xmi.diff.UMLTypeParameterDiff;
import gr.uom.java.xmi.diff.UMLTypeParameterListDiff;

Expand Down Expand Up @@ -625,6 +627,28 @@ public AbstractCall isDelegate() {
return null;
}

public AbstractCall isDelegateToAnotherClass(UMLModelDiff modelDiff) {
if(getBody() != null && modelDiff != null) {
List<AbstractStatement> statements = getBody().getCompositeStatement().getStatements();
if(statements.size() == 1 && statements.get(0) instanceof StatementObject) {
StatementObject statement = (StatementObject)statements.get(0);
for(AbstractCall operationInvocation : statement.getMethodInvocations()) {
if(operationInvocation.getExpression() != null) {
UMLClassBaseDiff classDiff = modelDiff.getUMLClassDiff(this.getClassName());
if(classDiff != null) {
for(UMLImport imp : classDiff.getNextClass().getImportedTypes()) {
if(imp.getName().endsWith(operationInvocation.getExpression())) {
return operationInvocation;
}
}
}
}
}
}
}
return null;
}

public boolean isGetter() {
if(getBody() != null) {
List<AbstractStatement> statements = getBody().getCompositeStatement().getStatements();
Expand Down
45 changes: 26 additions & 19 deletions src/main/java/gr/uom/java/xmi/diff/UMLModelDiff.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4105,7 +4105,8 @@ else if(replacement instanceof VariableReplacementWithMethodInvocation) {
return true;
}
}
else if(replacement.getAfter().equals(addedOperation.getNonQualifiedClassName())) {
else if(replacement.getAfter().equals(addedOperation.getNonQualifiedClassName()) ||
replacement.getAfter().startsWith(addedOperation.getNonQualifiedClassName() + ".")) {
return true;
}
}
Expand Down Expand Up @@ -4243,7 +4244,7 @@ private void checkForExtractedAndMovedLambdas(List<UMLOperationBodyMapper> mappe
!conflictingExpression(addedOperationInvocation, addedOperation, variableDeclarationMap, childFieldDeclarationMap) &&
operationBodyMapper.getMappings().size() >= lambdaStatementCount - castingStatements &&
extractAndMoveMatchCondition(operationBodyMapper, mapper, addedOperationInvocation)) {
createExtractAndMoveMethodRefactoring(addedOperation, mapper, addedOperationInvocations, operationBodyMapper);
createExtractAndMoveMethodRefactoring(addedOperation, mapper, addedOperationInvocations, operationBodyMapper, false);
for(AbstractCodeMapping mapping : operationBodyMapper.getMappings()) {
if(fragment.getLambdas().get(0).getLocationInfo().subsumes(mapping.getFragment1().getLocationInfo())) {
mapper.getNonMappedLeavesT1().remove(fragment);
Expand Down Expand Up @@ -4282,7 +4283,7 @@ else if(lambdaStatementCount < addedOperationStatementCount) {
!conflictingExpression(addedOperationInvocation, addedOperation, variableDeclarationMap, childFieldDeclarationMap) &&
operationBodyMapper.getMappings().size() >= lambdaStatementCount - castingStatements - possiblyInlinedOperations.size() &&
extractAndMoveMatchCondition(operationBodyMapper, mapper, addedOperationInvocation)) {
createExtractAndMoveMethodRefactoring(addedOperation, mapper, addedOperationInvocations, operationBodyMapper);
createExtractAndMoveMethodRefactoring(addedOperation, mapper, addedOperationInvocations, operationBodyMapper, false);
for(UMLOperation inlinedOperation : possiblyInlinedOperations.keySet()) {
AbstractCall inlinedOperationInvocation = possiblyInlinedOperations.get(inlinedOperation);
List<String> arguments = inlinedOperationInvocation.arguments();
Expand Down Expand Up @@ -4344,6 +4345,9 @@ private void checkForExtractedAndMovedOperations(List<UMLOperationBodyMapper> ma
for(Iterator<UMLOperation> addedOperationIterator = addedOperations.iterator(); addedOperationIterator.hasNext();) {
UMLOperation addedOperation = addedOperationIterator.next();
AbstractCall delegateCall = addedOperation.isDelegate();
if(delegateCall == null) {
delegateCall = addedOperation.isDelegateToAnotherClass(this);
}
if(!getterOrSetterCorrespondingToRenamedAttribute(addedOperation)) {
for(UMLOperationBodyMapper mapper : mappers) {
Pair<VariableDeclarationContainer, VariableDeclarationContainer> pair = Pair.of(mapper.getContainer1(), addedOperation);
Expand Down Expand Up @@ -4401,7 +4405,7 @@ else if(childFieldDeclarationMap != null) {
mapper, className, addedOperationInvocation, Optional.empty());
if(extractAndMoveMatchCondition(operationBodyMapper, mapper, addedOperationInvocation) && !skipRefactoring(operationBodyMapper.getMappings())) {
createExtractAndMoveMethodRefactoringBasedOnClassName(addedOperation, mapper,
className, addedOperationInvocations, operationBodyMapper);
className, addedOperationInvocations, operationBodyMapper, false);
}
else if(delegateCall != null) {
//find added operation that delegates to
Expand All @@ -4417,7 +4421,7 @@ else if(delegateCall != null) {
mapper, className, delegateCall, Optional.empty());
if(extractAndMoveMatchCondition(nestedOperationBodyMapper, mapper, delegateCall)) {
createExtractAndMoveMethodRefactoringBasedOnClassName(addedOperation, mapper,
className, addedOperationInvocations, nestedOperationBodyMapper);
className, addedOperationInvocations, nestedOperationBodyMapper, true);
}
}
}
Expand All @@ -4442,27 +4446,27 @@ else if(mapping instanceof CompositeStatementObjectMapping && !mapper.getNonMapp

private void createExtractAndMoveMethodRefactoringBasedOnClassName(UMLOperation addedOperation,
UMLOperationBodyMapper mapper, String className, List<AbstractCall> addedOperationInvocations,
UMLOperationBodyMapper operationBodyMapper) throws RefactoringMinerTimedOutException {
UMLOperationBodyMapper operationBodyMapper, boolean nested) throws RefactoringMinerTimedOutException {
if(className.equals(addedOperation.getClassName())) {
//extract inside moved or renamed class
createExtractAndMoveMethodRefactoring(addedOperation, mapper, addedOperationInvocations, operationBodyMapper);
createExtractAndMoveMethodRefactoring(addedOperation, mapper, addedOperationInvocations, operationBodyMapper, nested);
}
else if(isSubclassOf(className, addedOperation.getClassName())) {
createExtractAndMoveMethodRefactoring(addedOperation, mapper, addedOperationInvocations, operationBodyMapper);
createExtractAndMoveMethodRefactoring(addedOperation, mapper, addedOperationInvocations, operationBodyMapper, nested);
}
else if(isSubclassOf(addedOperation.getClassName(), className)) {
createExtractAndMoveMethodRefactoring(addedOperation, mapper, addedOperationInvocations, operationBodyMapper);
createExtractAndMoveMethodRefactoring(addedOperation, mapper, addedOperationInvocations, operationBodyMapper, nested);
}
else if(addedOperation.getClassName().startsWith(className + ".")) {
createExtractAndMoveMethodRefactoring(addedOperation, mapper, addedOperationInvocations, operationBodyMapper);
createExtractAndMoveMethodRefactoring(addedOperation, mapper, addedOperationInvocations, operationBodyMapper, nested);
}
else if(className.startsWith(addedOperation.getClassName() + ".")) {
createExtractAndMoveMethodRefactoring(addedOperation, mapper, addedOperationInvocations, operationBodyMapper);
createExtractAndMoveMethodRefactoring(addedOperation, mapper, addedOperationInvocations, operationBodyMapper, nested);
}
else if(sourceClassImportsTargetClass(className, addedOperation.getClassName()) ||
sourceClassImportsSuperclassOfTargetClass(className, addedOperation.getClassName()) ||
targetClassImportsSourceClass(className, addedOperation.getClassName())) {
createExtractAndMoveMethodRefactoring(addedOperation, mapper, addedOperationInvocations, operationBodyMapper);
createExtractAndMoveMethodRefactoring(addedOperation, mapper, addedOperationInvocations, operationBodyMapper, nested);
}
for(CandidateAttributeRefactoring candidate : operationBodyMapper.getCandidateAttributeRenames()) {
String before = PrefixSuffixUtils.normalize(candidate.getOriginalVariableName());
Expand Down Expand Up @@ -4543,20 +4547,23 @@ private Set<UMLOperationBodyMapper> findMappersWithTheSameFragment1(Set<Abstract
}

private void createExtractAndMoveMethodRefactoring(UMLOperation addedOperation, UMLOperationBodyMapper mapper,
List<AbstractCall> addedOperationInvocations, UMLOperationBodyMapper operationBodyMapper)
List<AbstractCall> addedOperationInvocations, UMLOperationBodyMapper operationBodyMapper, boolean nested)
throws RefactoringMinerTimedOutException {
ExtractOperationRefactoring extractOperationRefactoring =
new ExtractOperationRefactoring(operationBodyMapper, mapper.getContainer2(), addedOperationInvocations);
refactorings.add(extractOperationRefactoring);
refactorings.addAll(operationBodyMapper.getRefactorings());
deleteAddedOperation(addedOperation);
mapper.addChildMapper(operationBodyMapper);
MappingOptimizer optimizer = new MappingOptimizer(mapper.getClassDiff());
optimizer.optimizeDuplicateMappingsForExtract(mapper, refactorings);
Set<UMLOperationBodyMapper> mappers = findMappersWithTheSameFragment1(operationBodyMapper.getMappings());
if(mappers.size() > 0) {
mappers.add(operationBodyMapper);
optimizer.optimizeDuplicateMappingsForMoveCode(new ArrayList<>(mappers), refactorings);
if(!nested) {
MappingOptimizer optimizer = new MappingOptimizer(mapper.getClassDiff());
optimizer.optimizeDuplicateMappingsForExtract(mapper, refactorings);

Set<UMLOperationBodyMapper> mappers = findMappersWithTheSameFragment1(operationBodyMapper.getMappings());
if(mappers.size() > 0) {
mappers.add(operationBodyMapper);
optimizer.optimizeDuplicateMappingsForMoveCode(new ArrayList<>(mappers), refactorings);
}
}
}

Expand Down

0 comments on commit 0437482

Please sign in to comment.