Java: Don't create parameters for effectively local variables in extracted fragment (IDEA-180864)

This commit is contained in:
Pavel Dolgov
2017-10-24 17:23:19 +03:00
parent 5fab484adf
commit f2eb220e2b
9 changed files with 188 additions and 14 deletions
@@ -50,6 +50,7 @@ public class DuplicatesFinder {
private boolean myMultipleExitPoints;
@Nullable private final ReturnValue myReturnValue;
private final boolean myWithExtractedParameters;
private final Set<PsiVariable> myEffectivelyLocal;
private ComplexityHolder myPatternComplexityHolder;
private ComplexityHolder myCandidateComplexityHolder;
@@ -57,7 +58,8 @@ public class DuplicatesFinder {
InputVariables parameters,
@Nullable ReturnValue returnValue,
@NotNull List<? extends PsiVariable> outputParameters,
boolean withExtractedParameters) {
boolean withExtractedParameters,
@Nullable Set<PsiVariable> effectivelyLocal) {
myReturnValue = returnValue;
LOG.assertTrue(pattern.length > 0);
myPattern = pattern;
@@ -65,6 +67,7 @@ public class DuplicatesFinder {
myParameters = parameters;
myOutputParameters = outputParameters;
myWithExtractedParameters = withExtractedParameters;
myEffectivelyLocal = effectivelyLocal != null ? effectivelyLocal : Collections.emptySet();
final PsiElement codeFragment = ControlFlowUtil.findCodeFragment(pattern[0]);
try {
@@ -99,7 +102,7 @@ public class DuplicatesFinder {
InputVariables parameters,
@Nullable ReturnValue returnValue,
@NotNull List<? extends PsiVariable> outputParameters) {
this(pattern, parameters, returnValue, outputParameters, false);
this(pattern, parameters, returnValue, outputParameters, false, null);
}
public DuplicatesFinder(final PsiElement[] pattern,
@@ -384,6 +387,10 @@ public class DuplicatesFinder {
traverseParameter(resolveResult1, resolveResult2, match);
return match.putDeclarationCorrespondence(resolveResult1, resolveResult2);
}
if (resolveResult1 instanceof PsiVariable && myEffectivelyLocal.contains((PsiVariable)resolveResult1)) {
return (resolveResult2 instanceof PsiLocalVariable || resolveResult2 instanceof PsiParameter) &&
match.putDeclarationCorrespondence(resolveResult1, resolveResult2);
}
final PsiElement qualifier2 = ((PsiJavaCodeReferenceElement)candidate).getQualifier();
if (!equivalentResolve(resolveResult1, resolveResult2, qualifier2)) {
return matchExtractableVariable(pattern, candidate, match);
@@ -1757,19 +1757,30 @@ public class ExtractMethodProcessor implements MatchProvider {
return applyChosenClassAndExtract(inputVariables, extractPass);
}
private void declareNecessaryVariablesInsideBody(PsiCodeBlock body) throws IncorrectOperationException {
@NotNull
protected Set<PsiVariable> getEffectivelyLocalVariables() {
Set<PsiVariable> effectivelyLocal = new LinkedHashSet<>();
List<PsiVariable> usedVariables = myControlFlowWrapper.getUsedVariablesInBody(ControlFlowUtil.findCodeFragment(myElements[0]), myOutputVariables);
for (PsiVariable variable : usedVariables) {
boolean toDeclare = !isDeclaredInside(variable) && myInputVariables.toDeclareInsideBody(variable);
if (toDeclare) {
String name = variable.getName();
PsiDeclarationStatement statement = myElementFactory.createVariableDeclarationStatement(name, variable.getType(), null);
body.add(statement);
effectivelyLocal.add(variable);
}
}
if (myArtificialOutputVariable instanceof PsiField && !myIsChainedConstructor) {
body.add(myElementFactory.createVariableDeclarationStatement(myArtificialOutputVariable.getName(), myArtificialOutputVariable.getType(), null));
effectivelyLocal.add(myArtificialOutputVariable);
}
return effectivelyLocal;
}
private void declareNecessaryVariablesInsideBody(PsiCodeBlock body) throws IncorrectOperationException {
Set<PsiVariable> effectivelyLocal = getEffectivelyLocalVariables();
for (PsiVariable variable : effectivelyLocal) {
String name = variable.getName();
LOG.assertTrue(name != null, "variable name is null");
PsiDeclarationStatement statement = myElementFactory.createVariableDeclarationStatement(name, variable.getType(), null);
body.add(statement);
}
}
@@ -171,7 +171,8 @@ public class JavaDuplicatesExtractMethodProcessor extends ExtractMethodProcessor
public DuplicatesFinder createDuplicatesFinder() {
ReturnValue returnValue = myOutputVariables.length == 1 ? new VariableReturnValue(myOutputVariables[0]) : null;
return new DuplicatesFinder(myElements, myInputVariables, returnValue, Collections.emptyList(), true);
Set<PsiVariable> effectivelyLocal = getEffectivelyLocalVariables();
return new DuplicatesFinder(myElements, myInputVariables, returnValue, Collections.emptyList(), true, effectivelyLocal);
}
private void relaxMethodVisibility(Match match) {
@@ -57,7 +57,8 @@ public class ParametrizedDuplicates {
private PsiMethodCallExpression myParametrizedCall;
private VariableData[] myVariableDatum;
private ParametrizedDuplicates(@NotNull PsiElement[] pattern) {
private ParametrizedDuplicates(@NotNull PsiElement[] pattern,
@NotNull ExtractMethodProcessor originalProcessor) {
LOG.assertTrue(pattern.length != 0, "pattern length");
if (pattern[0] instanceof PsiStatement) {
PsiElement[] copy = copyElements(pattern);
@@ -65,7 +66,7 @@ public class ParametrizedDuplicates {
}
else if (pattern[0] instanceof PsiExpression) {
PsiElement[] copy = copyElements(pattern);
PsiExpression wrapped = wrapExpressionWithCodeBlock(copy);
PsiExpression wrapped = wrapExpressionWithCodeBlock(copy, originalProcessor);
myElements = wrapped != null ? new PsiElement[]{wrapped} : PsiElement.EMPTY_ARRAY;
}
else {
@@ -89,7 +90,7 @@ public class ParametrizedDuplicates {
return null;
}
ParametrizedDuplicates duplicates = new ParametrizedDuplicates(pattern);
ParametrizedDuplicates duplicates = new ParametrizedDuplicates(pattern, originalProcessor);
if (!duplicates.initMatches(matches)) {
return null;
}
@@ -103,13 +104,14 @@ public class ParametrizedDuplicates {
@NotNull
private static List<Match> findOriginalDuplicates(@NotNull ExtractMethodProcessor processor) {
PsiElement[] elements = getFilteredElements(processor.myElements);
Set<PsiVariable> effectivelyLocal = processor.getEffectivelyLocalVariables();
List<PsiVariable> variables = ContainerUtil.map(processor.myInputVariables.getInputVariables(), iv -> iv.variable);
InputVariables inputVariables = new InputVariables(variables, processor.myProject, new LocalSearchScope(processor.myElements), false);
DuplicatesFinder finder = new DuplicatesFinder(elements, inputVariables,
processor.myOutputVariable != null
? new VariableReturnValue(processor.myOutputVariable) : null,
Collections.emptyList(), true) {
Collections.emptyList(), true, effectivelyLocal) {
@Override
protected boolean isSelf(@NotNull PsiElement candidate) {
for (PsiElement element : elements) {
@@ -165,7 +167,7 @@ public class ParametrizedDuplicates {
matches.removeAll(badMatches);
}
myMatches = matches;
if (myMatches.isEmpty() || myUsagesList.isEmpty()) {
if (myMatches.isEmpty()) {
return false;
}
@@ -303,7 +305,8 @@ public class ParametrizedDuplicates {
}
@Nullable
private static PsiExpression wrapExpressionWithCodeBlock(@NotNull PsiElement[] copy) {
private static PsiExpression wrapExpressionWithCodeBlock(@NotNull PsiElement[] copy,
@NotNull ExtractMethodProcessor originalProcessor) {
if (copy.length != 1 || !(copy[0] instanceof PsiExpression)) return null;
PsiExpression expression = (PsiExpression)copy[0];
@@ -335,6 +338,14 @@ public class ParametrizedDuplicates {
LOG.assertTrue(statements.length == 1, "wrapper class method's body statement count");
PsiStatement bodyStatement = statements[0];
Set<PsiVariable> effectivelyLocal = originalProcessor.getEffectivelyLocalVariables();
for (PsiVariable variable : effectivelyLocal) {
String name = variable.getName();
LOG.assertTrue(name != null, "effectively local variable's name is null");
PsiDeclarationStatement declaration = factory.createVariableDeclarationStatement(name, variable.getType(), null);
body.addBefore(declaration, bodyStatement);
}
PsiExpression wrapped = null;
if (PsiType.VOID.equals(type) && bodyStatement instanceof PsiExpressionStatement) {
wrapped = ((PsiExpressionStatement)bodyStatement).getExpression();
@@ -0,0 +1,48 @@
import java.io.File;
import java.io.FileNotFoundException;
import java.util.Scanner;
import static org.junit.jupiter.api.Assertions.assertEquals;
public class EffectivelyLocalVariables {
void test() {
Scanner inputStreamScanner = null;
String theFirstLineFromDestinationFile;
String originContent = "aaa";
String fileName = "bbb";
ddd(originContent, fileName);
}
private void ddd(String originContent, String fileName) {
Scanner inputStreamScanner;
String theFirstLineFromDestinationFile;
<selection>try {
inputStreamScanner =
new Scanner(new File(fileName));
theFirstLineFromDestinationFile = inputStreamScanner.nextLine();
assertEquals(theFirstLineFromDestinationFile, originContent);
} catch (FileNotFoundException e) {
e.printStackTrace();
}</selection>
}
void dup() {
Scanner inputStreamScanner = null;
String theFirstLineFromDestinationFile;
String originContent = "";
String fileName = "";
try {
inputStreamScanner =
new Scanner(
new File(fileName));
theFirstLineFromDestinationFile = inputStreamScanner.nextLine();
// destination should contain original file's content
assertEquals(theFirstLineFromDestinationFile, originContent);
} catch (FileNotFoundException e) {
e.printStackTrace();
}
}
}
@@ -0,0 +1,45 @@
import java.io.File;
import java.io.FileNotFoundException;
import java.util.Scanner;
import static org.junit.jupiter.api.Assertions.assertEquals;
public class EffectivelyLocalVariables {
void test() {
Scanner inputStreamScanner = null;
String theFirstLineFromDestinationFile;
String originContent = "aaa";
String fileName = "bbb";
ddd(originContent, fileName);
}
private void ddd(String originContent, String fileName) {
Scanner inputStreamScanner;
String theFirstLineFromDestinationFile;
newMethod(originContent, fileName);
}
private void newMethod(String originContent, String fileName) {
Scanner inputStreamScanner;
String theFirstLineFromDestinationFile;
try {
inputStreamScanner =
new Scanner(new File(fileName));
theFirstLineFromDestinationFile = inputStreamScanner.nextLine();
assertEquals(theFirstLineFromDestinationFile, originContent);
} catch (FileNotFoundException e) {
e.printStackTrace();
}
}
void dup() {
Scanner inputStreamScanner = null;
String theFirstLineFromDestinationFile;
String originContent = "";
String fileName = "";
newMethod(originContent, fileName);
}
}
@@ -0,0 +1,19 @@
public class EffectivelyLocalWithinExpression {
void foo() {
int n;
if (<selection>(n = z()) > 0 && n < 100</selection>) {
System.out.println();
}
}
void bar() {
int n;
if ((n = z()) > 1 && n < 100) {
System.out.println();
}
}
int z() {
return 1;
}
}
@@ -0,0 +1,24 @@
public class EffectivelyLocalWithinExpression {
void foo() {
int n;
if (newMethod(0)) {
System.out.println();
}
}
private boolean newMethod(int i) {
int n;
return (n = z()) > i && n < 100;
}
void bar() {
int n;
if (newMethod(1)) {
System.out.println();
}
}
int z() {
return 1;
}
}
@@ -860,6 +860,14 @@ public class ExtractMethodTest extends LightCodeInsightTestCase {
doDuplicatesTest();
}
public void testEffectivelyLocalVariables() throws Exception {
doDuplicatesTest();
}
public void testEffectivelyLocalWithinExpression() throws Exception {
doDuplicatesTest();
}
public void testSuggestChangeSignatureWithChangedParameterName() throws Exception {
configureByFile(BASE_PATH + getTestName(false) + ".java");
boolean success = performExtractMethod(true, true, getEditor(), getFile(), getProject(), false, null, false, "p");