Skip to content

Commit

Permalink
bugfixes for Extract method
Browse files Browse the repository at this point in the history
  • Loading branch information
@m0rkeulv
m0rkeulv committed Nov 27, 2023
1 parent 7690b76 commit 1ebf283
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 39 deletions.
8 changes: 7 additions & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
# Changelog

## 1.4.20
* Bugfix: Extract method did not include parameters from parent method
* Bugfix: Extract method parameter list was not formatted correctly
* Bugfix: Extract method was not correctly handling trailing semicolon
* Bugfix: Name suggestion for Extract Method was suggestion names from outside selection

## 1.4.19
* Quickfixes for adding/removing type tags (return types & variable types)
* Very basic Extract Method support
* Minor tweaks forIntellij 2023.3 Support
* Minor tweaks for Intellij 2023.3 Support

## 1.4.18
* Support for Intellij 2023.3
Expand Down
90 changes: 61 additions & 29 deletions ...in/java/com/intellij/plugins/haxe/ide/refactoring/extractMethod/ExtractMethodBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.jetbrains.annotations.Nullable;

import java.util.*;
import java.util.stream.Collectors;

public class ExtractMethodBuilder {

Expand All @@ -28,7 +29,7 @@ public class ExtractMethodBuilder {
private String selectedText;
private List<HaxePsiCompositeElement> expressions;
private List<HaxeAssignExpression> assignExpressions;
private List<HaxeReferenceExpression> assignExpressionsOutsideSelection;
private List<HaxeReferenceExpression> assignExpressionsToReferencesOutsideSelection;
private List<HaxeReferenceExpression> referencesUsed;
private List<HaxeReferenceExpression> referencesToParameters;
private List<HaxeLocalVarDeclaration> localVariables;
Expand Down Expand Up @@ -57,7 +58,7 @@ public ExtractMethodBuilder selectedText(@Nullable @NlsSafe String text) {
public void validateAndProcessExpressions() {

assignExpressions = findAssignExpressions();
assignExpressionsOutsideSelection = findAssignsOutsideSelection(assignExpressions);
assignExpressionsToReferencesOutsideSelection = findAssignsToReferencesOutsideSelection(assignExpressions);

referencesUsed = findReferencesUsed(expressions, assignExpressions);
referencesToParameters = findOutsideReferencesForParameterList(referencesUsed);
Expand Down Expand Up @@ -92,15 +93,15 @@ private boolean localVarDeclarationPartiallyOutsideOfSelection() {
}

private boolean requireMoreThanOneReturnValue() {
return assignExpressionsOutsideSelection.size() + localUsedOutside.size() > 1;
return assignExpressionsToReferencesOutsideSelection.size() + localUsedOutside.size() > 1;
}

private boolean moreThanOneLocalVariableUsedOutsideSelection() {
return localUsedOutside.size() > 1;
}

private boolean moreThanOneLocalVarReferencedOutsideSelection() {
return assignExpressionsOutsideSelection.size() > 1;
return assignExpressionsToReferencesOutsideSelection.size() > 1;
}

private List<HaxeAssignExpression> findAssignExpressions() {
Expand All @@ -112,7 +113,7 @@ private List<HaxeAssignExpression> findAssignExpressions() {
return assignExpressions;
}

private List<HaxeReferenceExpression> findAssignsOutsideSelection(List<HaxeAssignExpression> assignExpressions) {
private List<HaxeReferenceExpression> findAssignsToReferencesOutsideSelection(List<HaxeAssignExpression> assignExpressions) {
List<HaxeReferenceExpression> outside = new ArrayList<>();
for (HaxeAssignExpression expression : assignExpressions) {
HaxeExpression leftExpression = expression.getLeftExpression();
Expand Down Expand Up @@ -155,6 +156,8 @@ private List<HaxeReferenceExpression> findOutsideReferencesForParameterList(List
if (!resolve.getTextRange().intersects(startOffset, endOffset)) {
outside.add(referenceExpression);
}
}else if (resolve instanceof HaxeParameter) {
outside.add(referenceExpression);
}
}
return outside;
Expand Down Expand Up @@ -239,35 +242,65 @@ public HaxeMethodDeclaration buildExtractedMethod(@Nullable Project project) {
}

private String buildMethod(String suggestedName, String block, Map<String, String> parameters) {
StringBuilder builder = new StringBuilder();
builder.append("\nfunction "+suggestedName+" (");
parameters.forEach((paramName, paramType) -> {
builder.append(paramName);
String parameterList = createParameterListText(parameters);
return new StringBuilder()
.append("\n")
.append("function " + suggestedName + " (")
.append(parameterList)
.append(")")
.append(block)
.append("\n\n")
.toString();
}

private static String createParameterListText(Map<String, String> parameters) {
return parameters.entrySet().stream().map(entry -> {
String paramName = entry.getKey();
String paramType = entry.getValue();
if (paramType != null && !paramType.isBlank()) {
builder.append(":").append(paramType);
return paramName + ":" + paramType;
}
else {
return paramName;
}
});
builder.append(")");
builder.append(block);
builder.append("\n\n");
return builder.toString();
}).collect(Collectors.joining(", "));
}

private String buildMethodContent(String selectedText, boolean firstStatementReturn) {
if (firstStatementReturn) {
if (selectedText.trim().endsWith(";")) {
return "{\n return " + selectedText + "\n}\n";
}else {
return "{\n return " + selectedText + ";\n}\n";
}
}else {
return "{\n" + selectedText + "\n}\n";
boolean hasAssign = this.assignExpressionsToReferencesOutsideSelection.isEmpty();
if (!hasAssign) {
// if we need to return a value to a field outside selection
// - create var to contain value at beginning
// - create return with said var
HaxeReferenceExpression referenceExpression = assignExpressionsToReferencesOutsideSelection.get(0);
String fieldName = referenceExpression.getText();
return new StringBuilder()
.append("{\n ")
.append("var " + fieldName + ";").append("\n")
.append(selectedText).append("\n")
.append("return ").append(fieldName).append(";").append("\n")
.append("}\n")
.toString();
} else {
// if first statement should return then prefix with return
String optionalReturn = firstStatementReturn ? "return " : "";
// if we extract part of an expression we may or may not have a semicolon at the end
// we check our selection and add it if needed
String optionalSemi = selectedText.trim().endsWith(";") ? "" : ";";
return new StringBuilder()
.append("{\n ")
.append(optionalReturn)
.append(selectedText)
.append(optionalSemi)
.append("\n")
.append("}\n")
.toString();
}
}

private Map<String, String> buildParameterList(List<HaxeReferenceExpression> RefrencesForparameters) {
private Map<String, String> buildParameterList(List<HaxeReferenceExpression> referencesForParameters) {
Map<String, String> parameterNameTypeMap = new HashMap<>();
for (HaxeReferenceExpression ref : RefrencesForparameters) {
for (HaxeReferenceExpression ref : referencesForParameters) {
ResultHolder type = HaxeExpressionEvaluator.evaluate(ref, null).result;
if (type.isUnknown()) {
parameterNameTypeMap.put(ref.getReferenceName(), null);
Expand All @@ -283,16 +316,15 @@ private Map<String, String> buildParameterList(List<HaxeReferenceExpression> Ref



public PsiElement buildReplacementExpression(@Nullable Project project, HaxeMethodDeclaration methodDeclaration) {
public PsiElement buildReplacementExpressionAndUpdateMethod(@Nullable Project project, HaxeMethodDeclaration methodDeclaration) {

boolean containsReturnStatements = PsiTreeUtil.getChildOfType(methodDeclaration, HaxeReturnStatement.class) != null;

String methodName = methodDeclaration.getComponentName().getText();
String methodCall = methodName + "(" + String.join(", ", parametersMap.keySet()) + ")";

if (!assignExpressionsOutsideSelection.isEmpty()) {
if (!assignExpressionsToReferencesOutsideSelection.isEmpty()) {
// TODO find type of assign // TODO this is wrong check local vars used outside for return/assign
methodCall = assignExpressionsOutsideSelection.get(0).getText() + " = " + methodCall;
methodCall = assignExpressionsToReferencesOutsideSelection.get(0).getText() + " = " + methodCall;
}
else if (!localUsedOutside.isEmpty() && !containsReturnStatements) {
Set<PsiReference> references = localUsedOutside.keySet();
Expand Down
23 changes: 14 additions & 9 deletions ...ava/com/intellij/plugins/haxe/ide/refactoring/extractMethod/HaxeExtractMethodHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ protected void performAction(HaxeIntroduceOperation operation) {
final int selectionEnd = selectionModel.getSelectionEnd();

if (selectionModel.hasSelection()) {
startElement = getOuterMostParentWithSameTextRangeStart(file.findElementAt(selectionStart));
stopElement = file.findElementAt(selectionEnd);
startElement = getOuterMostParentWithSameTextRangeStart(file.findElementAt(selectionStart), stopElement.getTextRange().getEndOffset());
}

startElement = findStartElement(startElement);
Expand All @@ -86,10 +86,10 @@ protected void performAction(HaxeIntroduceOperation operation) {
try {
methodBuilder.validateAndProcessExpressions();

boolean partOfExpression = stopElement.getParent().getParent() instanceof HaxeExpression;

HaxeMethodDeclaration methodDeclaration = methodBuilder.buildExtractedMethod(project);
PsiElement replacementExpression = methodBuilder.buildReplacementExpression(project, methodDeclaration);

PsiElement replacementExpression = methodBuilder.buildReplacementExpressionAndUpdateMethod(project, methodDeclaration);

Document document = editor.getDocument();
HaxeMethodDeclaration parentMethod = PsiTreeUtil.getParentOfType(startElement, HaxeMethodDeclaration.class);
Expand All @@ -109,12 +109,15 @@ protected void performAction(HaxeIntroduceOperation operation) {
instance.commitDocument(document);

int number = document.getLineNumber(selectionEnd);
String documentText = document.getText(new TextRange(selectionEnd, document.getLineEndOffset(number)));
String restOfLine = document.getText(new TextRange(selectionEnd, document.getLineEndOffset(number)));

if (documentText.trim().startsWith(";")) {
document.replaceString(selectionStart, selectionEnd, replacementExpression.getText());
String replacementText = replacementExpression.getText();
boolean selectionEndsWithSemi = selectionModel.getSelectedText().endsWith(";");
boolean selectionIsFollowedBySemi = restOfLine.trim().startsWith(";");
if (!selectionEndsWithSemi || selectionIsFollowedBySemi || partOfExpression) {
document.replaceString(selectionStart, selectionEnd, replacementText);
} else {
document.replaceString(selectionStart, selectionEnd, replacementExpression.getText() + ";");
document.replaceString(selectionStart, selectionEnd, replacementText + ";");
}


Expand Down Expand Up @@ -185,14 +188,16 @@ private static PsiElement findStartElement(PsiElement startElement) {
return startElement;
}

private PsiElement getOuterMostParentWithSameTextRangeStart(PsiElement element) {
private PsiElement getOuterMostParentWithSameTextRangeStart(PsiElement element, int maxOffset) {
if (element == null) return null;
TextRange range = element.getTextRange();
int startOffset = range.getStartOffset();
PsiElement outerMost = element;
while (outerMost != null) {
PsiElement parent = outerMost.getParent();
if (parent != null && parent.getTextRange().getStartOffset() == startOffset) {
if (parent != null
&& parent.getTextRange().getStartOffset() == startOffset
&& parent.getTextRange().getEndOffset() <= maxOffset) {
outerMost = parent;
}else {
break;
Expand Down

0 comments on commit 1ebf283

Please sign in to comment.