Skip to content

Commit

Permalink
Support Extract and Move Method to Newly Added class
Browse files Browse the repository at this point in the history 
Refactoring oracle update
Pending fix for UMLModelDiff.conflictingExpression()
eclipse-jgit/jgit@afb013b
  • Loading branch information
@tsantalis
tsantalis committed Dec 31, 2024
1 parent a526d2a commit 5592bea
Show file tree
Hide file tree
Showing 9 changed files with 242 additions and 38 deletions.
43 changes: 43 additions & 0 deletions src/main/java/gr/uom/java/xmi/decomposition/AbstractCall.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1277,6 +1277,49 @@ private static boolean equalsIgnoringExtraParenthesis(String s1, String s2) {
return true;
}
}
String reservedTokens1 = ReplacementUtil.keepReservedTokens(s1);
String reservedTokens2 = ReplacementUtil.keepReservedTokens(s2);
String[] tokens1 = LeafType.CAMEL_CASE_SPLIT_PATTERN.split(s1);
String[] tokens2 = LeafType.CAMEL_CASE_SPLIT_PATTERN.split(s2);
List<String> tokenList1 = new ArrayList<String>();
for(String token : tokens1) {
if(token.contains("(") && !token.contains("()")) {
String prefix = token.substring(0, token.indexOf("("));
String suffix = token.substring(token.indexOf("(")+1, token.length());
tokenList1.add(prefix);
tokenList1.add(suffix);
}
else {
tokenList1.add(token);
}
}
List<String> tokenList2 = new ArrayList<String>();
for(String token : tokens2) {
if(token.contains("(") && !token.contains("()")) {
String prefix = token.substring(0, token.indexOf("("));
String suffix = token.substring(token.indexOf("(")+1, token.length());
tokenList2.add(prefix);
tokenList2.add(suffix);
}
else {
tokenList2.add(token);
}
}
int commonTokens = 0;
for(int i=0; i<tokenList1.size(); i++) {
String tokenI = tokenList1.get(i);
for(int j=i; j<tokenList2.size(); j++) {
String tokenJ = tokenList2.get(j);
if(tokenI.equals(tokenJ) || tokenI.toLowerCase().equals(tokenJ.toLowerCase())) {
commonTokens++;
break;
}
}
}
if(commonTokens >= Math.min(tokenList1.size(), tokenList2.size()) &&
!reservedTokens1.isEmpty() && reservedTokens1.equals(reservedTokens2)) {
return true;
}
return false;
}

Expand Down
10 changes: 10 additions & 0 deletions src/main/java/gr/uom/java/xmi/decomposition/StringBasedHeuristics.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2156,6 +2156,16 @@ else if(replacement.getType().equals(ReplacementType.CLASS_INSTANCE_CREATION) &&
assignment1.equals(replacement.getBefore()) &&
assignment2.equals(replacement.getAfter()))
classInstanceCreationReplacement = true;
else if(replacement instanceof MethodInvocationReplacement) {
AbstractCall invocationBefore = ((MethodInvocationReplacement)replacement).getInvokedOperationBefore();
AbstractCall invocationAfter = ((MethodInvocationReplacement)replacement).getInvokedOperationAfter();
if((assignment1.equals(replacement.getBefore()) || assignment1.equals(invocationBefore.actualString())) &&
(assignment2.equals(replacement.getAfter()) || assignment2.equals(invocationAfter.actualString()))) {
if(!invocationBefore.getName().contains(invocationAfter.getName()) && !invocationAfter.getName().contains(invocationBefore.getName())) {
rightHandSideReplacement = true;
}
}
}
}
if(inv1 != null && inv2 != null) {
equalArguments = inv1.equalArguments(inv2) && inv1.arguments().size() > 0;
Expand Down
24 changes: 23 additions & 1 deletion src/main/java/gr/uom/java/xmi/decomposition/UMLOperationBodyMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2587,6 +2587,14 @@ public UMLOperationBodyMapper(UMLOperationBodyMapper operationBodyMapper, UMLOpe
CompositeStatementObject composite2 = addedOperationBody.getCompositeStatement();
List<AbstractCodeFragment> leaves1 = leaves1Sublist.isPresent() ? leaves1Sublist.get() : operationBodyMapper.getNonMappedLeavesT1();
List<CompositeStatementObject> innerNodes1 = leaves1Sublist.isPresent() ? new ArrayList<>() : operationBodyMapper.getNonMappedInnerNodesT1();
for(Pair<AbstractCodeFragment, UMLComment> pair : operationBodyMapper.getCommentedCode()) {
if(leaves1.contains(pair.getLeft())) {
leaves1.remove(pair.getLeft());
}
if(innerNodes1.contains(pair.getLeft())) {
innerNodes1.remove(pair.getLeft());
}
}
//adding leaves that were mapped with replacements
Set<AbstractCodeFragment> addedLeaves1 = new LinkedHashSet<AbstractCodeFragment>();
Set<CompositeStatementObject> addedInnerNodes1 = new LinkedHashSet<CompositeStatementObject>();
Expand Down Expand Up @@ -3596,6 +3604,16 @@ public List<CompositeReplacement> getCompositeReplacements() {
return composites;
}

public boolean containsOnlyBlockMappings() {
int count = 0;
for(AbstractCodeMapping mapping : getMappings()) {
if(mapping.getFragment1().getLocationInfo().getCodeElementType().equals(CodeElementType.BLOCK) && mapping.getFragment2().getLocationInfo().getCodeElementType().equals(CodeElementType.BLOCK)) {
count++;
}
}
return count > 0 && count == getMappings().size();
}

public int mappingsWithoutBlocks() {
int count = 0;
Set<LeafMapping> subExpressionMappings = new LinkedHashSet<>();
Expand Down Expand Up @@ -10494,7 +10512,11 @@ public boolean containsParentMapping(AbstractCodeMapping mapping) {
if(parent1 != null && parent2 != null) {
for(AbstractCodeMapping previousMapping : this.mappings) {
if(previousMapping.getFragment1().equals(parent1) && previousMapping.getFragment2().equals(parent2)) {
return true;
boolean isMethodBodyBlock1 = parent1.getParent() == null;
boolean isMethodBodyBlock2 = parent2.getParent() == null;
if(isMethodBodyBlock1 == isMethodBodyBlock2) {
return true;
}
}
}
if(parent1.getLocationInfo().getCodeElementType().equals(CodeElementType.BLOCK) && parent2.getLocationInfo().getCodeElementType().equals(CodeElementType.BLOCK)) {
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/gr/uom/java/xmi/diff/InlineOperationDetection.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -323,11 +323,12 @@ private boolean inlineMatchCondition(UMLOperationBodyMapper operationBodyMapper,
}
}
List<AbstractCodeMapping> exactMatchList = operationBodyMapper.getExactMatches();
boolean exactMatchForInt = exactMatchList.size() == 1 && exactMatchList.get(0).getFragment1().getString().startsWith("for(int i=0;");
List<AbstractCodeMapping> exactMatchListWithoutMatchesInNestedContainers = operationBodyMapper.getExactMatchesWithoutMatchesInNestedContainers();
int exactMatches = exactMatchList.size();
int exactMatchesWithoutMatchesInNestedContainers = exactMatchListWithoutMatchesInNestedContainers.size();
return mappings > 0 && (mappings > nonMappedElementsT1 ||
(exactMatches >= mappings && nonMappedElementsT2 == 0) ||
(exactMatches >= mappings && !exactMatchForInt && nonMappedElementsT2 == 0) ||
(exactMatchesWithoutMatchesInNestedContainers == 1 && !exactMatchListWithoutMatchesInNestedContainers.get(0).getFragment1().throwsNewException() && nonMappedElementsT1-exactMatchesWithoutMatchesInNestedContainers < 10) ||
(exactMatches > 1 && nonMappedElementsT1-exactMatches < 20));
}
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/gr/uom/java/xmi/diff/MappingOptimizer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ else if(r instanceof ReplaceConditionalWithTernaryRefactoring) {
if(ref instanceof ExtractOperationRefactoring) {
ExtractOperationRefactoring refactoring = (ExtractOperationRefactoring)ref;
if(updatedMappers.contains(refactoring.getBodyMapper())) {
if(refactoring.getBodyMapper().getMappings().size() == 0) {
if(refactoring.getBodyMapper().getMappings().size() == 0 || refactoring.getBodyMapper().containsOnlyBlockMappings()) {
refactoringsToBeRemoved.add(refactoring);
}
else {
Expand All @@ -558,7 +558,7 @@ else if(r instanceof ReplaceConditionalWithTernaryRefactoring) {
else if(ref instanceof InlineOperationRefactoring) {
InlineOperationRefactoring refactoring = (InlineOperationRefactoring)ref;
if(updatedMappers.contains(refactoring.getBodyMapper())) {
if(refactoring.getBodyMapper().getMappings().size() == 0) {
if(refactoring.getBodyMapper().getMappings().size() == 0 || refactoring.getBodyMapper().containsOnlyBlockMappings()) {
refactoringsToBeRemoved.add(refactoring);
}
else {
Expand All @@ -569,7 +569,7 @@ else if(ref instanceof InlineOperationRefactoring) {
else if(ref instanceof MoveCodeRefactoring) {
MoveCodeRefactoring refactoring = (MoveCodeRefactoring)ref;
if(updatedMappers.contains(refactoring.getBodyMapper())) {
if(refactoring.getBodyMapper().getMappings().size() == 0) {
if(refactoring.getBodyMapper().getMappings().size() == 0 || refactoring.getBodyMapper().containsOnlyBlockMappings()) {
refactoringsToBeRemoved.add(refactoring);
}
else {
Expand Down
47 changes: 42 additions & 5 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 @@ -1948,7 +1948,22 @@ public List<UMLAttribute> getRemovedAttributesInCommonClasses() {
private List<UMLOperation> getOperationsInAddedClasses() {
List<UMLOperation> addedOperations = new ArrayList<UMLOperation>();
for(UMLClass addedClass : addedClasses) {
addedOperations.addAll(addedClass.getOperations());
for(UMLOperation operation : addedClass.getOperations()) {
//check if the method is moved
boolean movedMethod = false;
for(Refactoring r : refactorings) {
if(r instanceof MoveOperationRefactoring) {
MoveOperationRefactoring move = (MoveOperationRefactoring)r;
if(move.getMovedOperation().equals(operation)) {
movedMethod = true;
break;
}
}
}
if(!movedMethod) {
addedOperations.add(operation);
}
}
}
return addedOperations;
}
Expand Down Expand Up @@ -3120,6 +3135,9 @@ else if(!diffs2.isEmpty()) {
}
List<UMLOperation> allOperationsInAddedClasses = getOperationsInAddedClasses();
checkForExtractedAndMovedLambdas(getOperationBodyMappersInMovedAndRenamedClasses(), allOperationsInAddedClasses);
if(allOperationsInAddedClasses.size() <= MAXIMUM_NUMBER_OF_COMPARED_METHODS) {
checkForExtractedAndMovedOperations(getOperationBodyMappersInCommonClasses(), allOperationsInAddedClasses);
}
List<MoveAttributeRefactoring> moveAttributeRefactorings = new ArrayList<MoveAttributeRefactoring>();
moveAttributeRefactorings.addAll(checkForAttributeMovesBetweenCommonClasses(renameMap, refactorings));
moveAttributeRefactorings.addAll(checkForAttributeMovesIncludingAddedClasses(renameMap, refactorings));
Expand Down Expand Up @@ -4163,6 +4181,9 @@ private void checkForExtractedAndMovedLambdas(List<UMLOperationBodyMapper> mappe
String className = mapper.getContainer2().getClassName();
if(!className.equals(addedOperation.getClassName()) && (mapper.nonMappedElementsT1() > 0 || includesReplacementInvolvingAddedMethod(mapper.getReplacementsInvolvingMethodInvocation(), addedOperation, mapper.getContainer2(), mapper.getClassDiff())) && !mapper.containsExtractOperationRefactoring(addedOperation) && !processedOperationPairs.contains(pair)) {
processedOperationPairs.add(pair);
Map<String, Set<VariableDeclaration>> variableDeclarationMap = mapper.getContainer2().variableDeclarationMap();
UMLAbstractClass childCallerClass = this.findClassInChildModel(mapper.getContainer2().getClassName());
Map<String, VariableDeclaration> childFieldDeclarationMap = childCallerClass != null ? childCallerClass.getFieldDeclarationMap() : null;
for(AbstractCodeFragment fragment : new ArrayList<>(mapper.getNonMappedLeavesT1())) {
if(fragment.getLambdas().size() > 0 && fragment.getLambdas().get(0).getBody() != null) {
int lambdaStatementCount = fragment.getLambdas().get(0).getBody().statementCountIncludingBlocks() - 1;
Expand Down Expand Up @@ -4190,7 +4211,7 @@ private void checkForExtractedAndMovedLambdas(List<UMLOperationBodyMapper> mappe
UMLOperationBodyMapper operationBodyMapper = createMapperForExtractAndMove(addedOperation,
mapper, className, addedOperationInvocation, Optional.of(subList));
if(!anotherAddedMethodExistsWithBetterMatchingInvocationExpression(addedOperationInvocation, addedOperation, addedOperations) &&
!conflictingExpression(addedOperationInvocation, addedOperation, mapper.getContainer2().variableDeclarationMap()) &&
!conflictingExpression(addedOperationInvocation, addedOperation, variableDeclarationMap, childFieldDeclarationMap) &&
operationBodyMapper.getMappings().size() >= lambdaStatementCount - castingStatements &&
extractAndMoveMatchCondition(operationBodyMapper, mapper, addedOperationInvocation)) {
createExtractAndMoveMethodRefactoring(addedOperation, mapper, addedOperationInvocations, operationBodyMapper);
Expand Down Expand Up @@ -4229,7 +4250,7 @@ else if(lambdaStatementCount < addedOperationStatementCount) {
UMLOperationBodyMapper operationBodyMapper = createMapperForExtractAndMove(addedOperation,
mapper, className, addedOperationInvocation, Optional.of(subList));
if(!anotherAddedMethodExistsWithBetterMatchingInvocationExpression(addedOperationInvocation, addedOperation, addedOperations) &&
!conflictingExpression(addedOperationInvocation, addedOperation, mapper.getContainer2().variableDeclarationMap()) &&
!conflictingExpression(addedOperationInvocation, addedOperation, variableDeclarationMap, childFieldDeclarationMap) &&
operationBodyMapper.getMappings().size() >= lambdaStatementCount - castingStatements - possiblyInlinedOperations.size() &&
extractAndMoveMatchCondition(operationBodyMapper, mapper, addedOperationInvocation)) {
createExtractAndMoveMethodRefactoring(addedOperation, mapper, addedOperationInvocations, operationBodyMapper);
Expand Down Expand Up @@ -4348,7 +4369,7 @@ else if(childFieldDeclarationMap != null) {
UMLOperationBodyMapper operationBodyMapper = createMapperForExtractAndMove(addedOperation,
mapper, className, addedOperationInvocation, Optional.empty());
if(!anotherAddedMethodExistsWithBetterMatchingInvocationExpression(addedOperationInvocation, addedOperation, addedOperations) &&
!conflictingExpression(addedOperationInvocation, addedOperation, mapper.getContainer2().variableDeclarationMap()) &&
!conflictingExpression(addedOperationInvocation, addedOperation, variableDeclarationMap, childFieldDeclarationMap) &&
extractAndMoveMatchCondition(operationBodyMapper, mapper, addedOperationInvocation)) {
if(className.equals(addedOperation.getClassName())) {
//extract inside moved or renamed class
Expand Down Expand Up @@ -4500,7 +4521,7 @@ private boolean isAnonymousClassName(String className) {
return StringDistance.isNumeric(anonymousID) || Character.isLowerCase(anonymousID.charAt(0));
}

private boolean conflictingExpression(AbstractCall invocation, UMLOperation addedOperation, Map<String, Set<VariableDeclaration>> variableDeclarationMap) {
private boolean conflictingExpression(AbstractCall invocation, UMLOperation addedOperation, Map<String, Set<VariableDeclaration>> variableDeclarationMap, Map<String, VariableDeclaration> childFieldDeclarationMap) {
String expression = invocation.getExpression();
if(expression != null && variableDeclarationMap.containsKey(expression)) {
Set<VariableDeclaration> variableDeclarations = variableDeclarationMap.get(expression);
Expand All @@ -4517,6 +4538,19 @@ private boolean conflictingExpression(AbstractCall invocation, UMLOperation adde
}
}
}
/*else if(expression != null && childFieldDeclarationMap != null && childFieldDeclarationMap.containsKey(expression)) {
VariableDeclaration variableDeclaration = childFieldDeclarationMap.get(expression);
UMLClassBaseDiff classDiff = getUMLClassDiff(addedOperation.getClassName());
UMLType type = variableDeclaration.getType();
boolean superclassRelationship = false;
if(classDiff != null && type != null && classDiff.getNewSuperclass() != null &&
classDiff.getNewSuperclass().equals(type)) {
superclassRelationship = true;
}
if(type != null && !addedOperation.getNonQualifiedClassName().equals(type.getClassType()) && !superclassRelationship) {
return true;
}
}*/
return false;
}

Expand Down Expand Up @@ -4612,6 +4646,9 @@ private boolean extractAndMoveMatchCondition(UMLOperationBodyMapper operationBod
}
}
}
if(operationBodyMapper.getContainer2().getBodyHashCode() == parentMapper.getContainer2().getBodyHashCode()) {
return false;
}
int mappings = operationBodyMapper.mappingsWithoutBlocks();
int nonMappedElementsT1 = operationBodyMapper.nonMappedElementsT1();
int nonMappedElementsT2 = operationBodyMapper.nonMappedElementsT2();
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/org/refactoringminer/test/TestAllRefactorings.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@ public void testAllRefactorings() throws Exception {
GitHistoryRefactoringMinerImpl detector = new GitHistoryRefactoringMinerImpl();
TestBuilder test = new TestBuilder(detector, REPOS, Refactorings.All.getValue());
RefactoringPopulator.feedRefactoringsInstances(Refactorings.All.getValue(), Systems.FSE.getValue(), test);
test.assertExpectationsWithGitHubAPI(12377, 20, 233);
test.assertExpectationsWithGitHubAPI(12403, 22, 226);
}
}
Loading

0 comments on commit 5592bea

Please sign in to comment.