MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-04-19 13:02:30 +07:00
Code Issues Packages Projects Releases Wiki Activity

UnrollLoopAction: fixes according to review IDEA-CR-24513

Browse Source 
1. Check that loop parameter is not written
2. Support List.of, Collections.singleton, Collections.singletonList
3. Support if w/o braces
4. Support break not in last position
This commit is contained in:
Tagir Valeev
2017-09-18 16:36:27 +07:00
parent 8928ef54a4
commit 7345e8b0ef
19 changed files with 264 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

66
java/java-impl/src/com/intellij/codeInsight/intention/impl/UnrollLoopAction.java
View File

@@ -33,8 +33,15 @@ import org.jetbrains.annotations.NotNull;
import java.util.Arrays;
import java.util.Objects;
import static com.siyeh.ig.callMatcher.CallMatcher.anyOf;
import static com.siyeh.ig.callMatcher.CallMatcher.staticCall;
public class UnrollLoopAction extends PsiElementBaseIntentionAction {
private static final CallMatcher LIST_CONSTRUCTOR = CallMatcher.staticCall(CommonClassNames.JAVA_UTIL_ARRAYS, "asList");
private static final CallMatcher LIST_CONSTRUCTOR = anyOf(staticCall(CommonClassNames.JAVA_UTIL_ARRAYS, "asList"),
staticCall(CommonClassNames.JAVA_UTIL_LIST, "of"));
private static final CallMatcher SINGLETON_CONSTRUCTOR =
anyOf(staticCall(CommonClassNames.JAVA_UTIL_COLLECTIONS, "singleton", "singletonList").parameterCount(1),
staticCall(CommonClassNames.JAVA_UTIL_LIST, "of").parameterTypes("E"));
@Override
public boolean isAvailable(@NotNull Project project, Editor editor, @NotNull final PsiElement element) {
@@ -42,14 +49,17 @@ public class UnrollLoopAction extends PsiElementBaseIntentionAction {
if (loop == null) return false;
if (!(loop.getParent() instanceof PsiCodeBlock)) return false;
PsiExpression iteratedValue = ExpressionUtils.resolveExpression(loop.getIteratedValue());
PsiParameter iterationParameter = loop.getIterationParameter();
if (extractExpressions(iteratedValue).length == 0) return false;
PsiStatement[] statements = ControlFlowUtils.unwrapBlock(loop.getBody());
if (statements.length == 0) return false;
if (Arrays.stream(statements).anyMatch(PsiDeclarationStatement.class::isInstance)) return false;
if (isBreakChain(loop)) {
statements = Arrays.copyOfRange(statements, 0, statements.length - 1);
}
if (VariableAccessUtils.variableIsAssigned(iterationParameter, loop)) return false;
//if (isBreakChain(loop)) {
// statements = Arrays.copyOfRange(statements, 0, statements.length - 1);
//}
for (PsiStatement statement : statements) {
if (isLoopBreak(statement)) continue;
boolean acceptable = PsiTreeUtil.processElements(statement, e -> {
if (e instanceof PsiBreakStatement && ((PsiBreakStatement)e).findExitedStatement() == loop) return false;
if (e instanceof PsiContinueStatement && ((PsiContinueStatement)e).findContinuedStatement() == loop) return false;
@@ -72,9 +82,15 @@ public class UnrollLoopAction extends PsiElementBaseIntentionAction {
}
if (expression instanceof PsiMethodCallExpression) {
PsiMethodCallExpression call = (PsiMethodCallExpression)expression;
if (LIST_CONSTRUCTOR.test(call) && MethodCallUtils.isVarArgCall(call)) {
if (SINGLETON_CONSTRUCTOR.test(call)) {
return call.getArgumentList().getExpressions();
}
if (LIST_CONSTRUCTOR.test(call)) {
PsiExpression[] args = call.getArgumentList().getExpressions();
if (args.length > 1 || MethodCallUtils.isVarArgCall(call)) {
return args;
}
}
}
return PsiExpression.EMPTY_ARRAY;
}
@@ -96,7 +112,6 @@ public class UnrollLoopAction extends PsiElementBaseIntentionAction {
PsiForeachStatement loop = PsiTreeUtil.getParentOfType(element, PsiForeachStatement.class);
if (loop == null) return;
if (!(loop.getParent() instanceof PsiCodeBlock)) return;
boolean breakChain = isBreakChain(loop);
PsiExpression iteratedValue = loop.getIteratedValue();
PsiExpression[] expressions = extractExpressions(ExpressionUtils.resolveExpression(iteratedValue));
if (expressions.length == 0) return;
@@ -115,22 +130,23 @@ public class UnrollLoopAction extends PsiElementBaseIntentionAction {
}
PsiStatement body = copy.getBody();
assert body != null;
PsiElement[] children;
if (body instanceof PsiBlockStatement) {
PsiElement[] children = ((PsiBlockStatement)body).getCodeBlock().getChildren();
PsiElement parent = anchor.getParent();
PsiElement currentAnchor = anchor;
children = ((PsiBlockStatement)body).getCodeBlock().getChildren();
// Skip {braces}
Arrays.stream(children, 1, children.length - 1).forEach(child -> parent.addBefore(child, currentAnchor));
children = Arrays.copyOfRange(children, 1, children.length-1);
} else {
children = new PsiElement[]{body};
}
if (breakChain) {
PsiStatement lastStatement = PsiTreeUtil.getPrevSiblingOfType(anchor, PsiStatement.class);
if (lastStatement instanceof PsiIfStatement) {
PsiIfStatement ifStatement = (PsiIfStatement)lastStatement;
for(PsiElement child : children) {
PsiElement added = anchor.getParent().addBefore(child, anchor);
if (added instanceof PsiIfStatement && isLoopBreak((PsiStatement)added)) {
PsiIfStatement ifStatement = (PsiIfStatement)added;
PsiExpression condition = Objects.requireNonNull(ifStatement.getCondition());
PsiStatement thenBranch = Objects.requireNonNull(ifStatement.getThenBranch());
String negated = BoolUtils.getNegatedExpressionText(condition);
condition.replace(factory.createExpressionFromText(negated, condition));
PsiBlockStatement block = (PsiBlockStatement)thenBranch.replace(factory.createStatementFromText("{}", lastStatement));
PsiBlockStatement block = (PsiBlockStatement)thenBranch.replace(factory.createStatementFromText("{}", added));
anchor = block.getCodeBlock().getLastChild();
}
}
@@ -140,19 +156,11 @@ public class UnrollLoopAction extends PsiElementBaseIntentionAction {
ct.deleteAndRestoreComments(loop);
}
/**
* @param loop loop to test
* @return true if the last statement is "if(...) break"
*/
private static boolean isBreakChain(PsiForeachStatement loop) {
PsiStatement lastStatement = loop.getBody();
if (lastStatement instanceof PsiBlockStatement) {
lastStatement = ControlFlowUtils.getLastStatementInBlock(((PsiBlockStatement)lastStatement).getCodeBlock());
}
if (!(lastStatement instanceof PsiIfStatement)) return false;
PsiIfStatement ifStatement = (PsiIfStatement)lastStatement;
return ifStatement.getElseBranch() == null &&
ifStatement.getCondition() != null &&
ControlFlowUtils.statementBreaksLoop(ControlFlowUtils.stripBraces(ifStatement.getThenBranch()), loop);
private static boolean isLoopBreak(PsiStatement statement) {
if (!(statement instanceof PsiIfStatement)) return false;
PsiIfStatement ifStatement = (PsiIfStatement)statement;
if (ifStatement.getElseBranch() != null || ifStatement.getCondition() == null) return false;
PsiStatement thenBranch = ControlFlowUtils.stripBraces(ifStatement.getThenBranch());
return thenBranch instanceof PsiBreakStatement && ((PsiBreakStatement)thenBranch).getLabelIdentifier() == null;
}
}

19
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unrollLoop/afterUnrollBreak.java Normal file
View File

@@ -0,0 +1,19 @@
// "Unroll loop" "true"
class Test {
void test() {
if (!(Math.random() > 0.5)) {
System.out.println((Object) "one");
if (!(Math.random() > 0.5)) {
System.out.println((Object) 1);
if (!(Math.random() > 0.5)) {
System.out.println((Object) 1.0);
if (!(Math.random() > 0.5)) {
System.out.println((Object) 1.0f);
}
}
}
}
}
void foo(boolean b) {}
}

25
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unrollLoop/afterUnrollBreakMultiple.java Normal file
View File

@@ -0,0 +1,25 @@
// "Unroll loop" "true"
class Test {
void test(String s1, String s2, String s3) {
if (s1.length() <= 5) {
System.out.println("Long string: " + s1);
if (s1.length() <= 20) {
System.out.println("Very long string: " + s1);
if (s2.length() <= 5) {
System.out.println("Long string: " + s2);
if (s2.length() <= 20) {
System.out.println("Very long string: " + s2);
if (s3.length() <= 5) {
System.out.println("Long string: " + s3);
if (s3.length() <= 20) {
System.out.println("Very long string: " + s3);
}
}
}
}
}
}
}
void foo(boolean b) {}
}

14
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unrollLoop/afterUnrollBreakNoBraces.java Normal file
View File

@@ -0,0 +1,14 @@
// "Unroll loop" "true"
class Test {
void test(String s1, String s2, String s3) {
if ((i += s1.length()) <= 10) {
if ((i += s2.length()) <= 10) {
if ((i += s3.length()) <= 10) {
}
}
}
System.out.println(i);
}
void foo(boolean b) {}
}

19
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unrollLoop/afterUnrollListJava9.java Normal file
View File

@@ -0,0 +1,19 @@
// "Unroll loop" "true"
import java.util.*;
class Test {
void test() {
if (!"foo".isEmpty()) {
System.out.println("foo");
}
if (!"bar".isEmpty()) {
System.out.println("bar");
}
if (!"baz".isEmpty()) {
System.out.println("baz");
}
if (!"".isEmpty()) {
System.out.println("");
}
}
}

10
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unrollLoop/afterUnrollListOneJava9.java Normal file
View File

@@ -0,0 +1,10 @@
// "Unroll loop" "true"
import java.util.*;
class Test {
void test() {
if (!"foo".isEmpty()) {
System.out.println("foo");
}
}
}

11
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unrollLoop/afterUnrollNoBraces.java Normal file
View File

@@ -0,0 +1,11 @@
// "Unroll loop" "true"
import java.util.Arrays;
class Test {
void test() {
System.out.println("foo");
System.out.println("bar");
}
void foo(boolean b) {}
}

10
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unrollLoop/afterUnrollSingleton.java Normal file
View File

@@ -0,0 +1,10 @@
// "Unroll loop" "true"
import java.util.*;
class Test {
void test() {
if (!"xyz".isEmpty()) {
System.out.println("xyz");
}
}
}

2
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unrollLoop/beforeUnrollBreak.java
View File

@@ -1,4 +1,4 @@
// "Unroll loop" "false"
// "Unroll loop" "true"
class Test {
void test() {
fo<caret>r(Object x : new Object[] {"one", 1, 1.0, 1.0f}) {

15
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unrollLoop/beforeUnrollBreakDeeper.java Normal file
View File

@@ -0,0 +1,15 @@
// "Unroll loop" "false"
class Test {
void test(String s1, String s2, String s3) {
fo<caret>r(String s : new String[] {s1, s2, s3}) {
if (!s.isEmpty()) {
if(s.length() > 5) break;
}
System.out.println("Long string: "+s);
if(s.length() > 20) break;
System.out.println("Very long string: "+s);
}
}
void foo(boolean b) {}
}

13
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unrollLoop/beforeUnrollBreakMultiple.java Normal file
View File

@@ -0,0 +1,13 @@
// "Unroll loop" "true"
class Test {
void test(String s1, String s2, String s3) {
fo<caret>r(String s : new String[] {s1, s2, s3}) {
if(s.length() > 5) break;
System.out.println("Long string: "+s);
if(s.length() > 20) break;
System.out.println("Very long string: "+s);
}
}
void foo(boolean b) {}
}

10
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unrollLoop/beforeUnrollBreakNoBraces.java Normal file
View File

@@ -0,0 +1,10 @@
// "Unroll loop" "true"
class Test {
void test(String s1, String s2, String s3) {
fo<caret>r(String s : new String[] {s1, s2, s3})
if((i+=s.length()) > 10) break;
System.out.println(i);
}
void foo(boolean b) {}
}

12
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unrollLoop/beforeUnrollListArray.java Normal file
View File

@@ -0,0 +1,12 @@
// "Unroll loop" "false"
import java.util.*;
class Test {
void test(String[] data) {
fo<caret>r(String s : Arrays.asList(data)) {
if(!s.isEmpty()) {
System.out.println(s);
}
}
}
}

12
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unrollLoop/beforeUnrollListJava9.java Normal file
View File

@@ -0,0 +1,12 @@
// "Unroll loop" "true"
import java.util.*;
class Test {
void test() {
fo<caret>r(String s : List.of("foo", "bar", "baz", "")) {
if(!s.isEmpty()) {
System.out.println(s);
}
}
}
}

12
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unrollLoop/beforeUnrollListOneJava9.java Normal file
View File

@@ -0,0 +1,12 @@
// "Unroll loop" "true"
import java.util.*;
class Test {
void test() {
fo<caret>r(String s : List.of("foo")) {
if(!s.isEmpty()) {
System.out.println(s);
}
}
}
}

10
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unrollLoop/beforeUnrollNoBraces.java Normal file
View File

@@ -0,0 +1,10 @@
// "Unroll loop" "true"
import java.util.Arrays;
class Test {
void test() {
fo<caret>r(String s : Arrays.asList("foo", "bar")) System.out.println(s);
}
void foo(boolean b) {}
}

12
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unrollLoop/beforeUnrollSingleton.java Normal file
View File

@@ -0,0 +1,12 @@
// "Unroll loop" "true"
import java.util.*;
class Test {
void test() {
fo<caret>r(String s : Collections.singleton("xyz")) {
if(!s.isEmpty()) {
System.out.println(s);
}
}
}
}

13
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unrollLoop/beforeUnrollWritesVar.java Normal file
View File

@@ -0,0 +1,13 @@
// "Unroll loop" "false"
class Test {
void test() {
f<caret>or(int x : new int[]{1, 2, 3}) {
System.out.println(x);
x = 6;
x++;
System.out.println(x);
}
}
void foo(boolean b) {}
}

9
java/java-tests/testSrc/com/intellij/java/codeInsight/intention/UnrollLoopActionTest.java
View File

@@ -16,11 +16,20 @@
package com.intellij.java.codeInsight.intention;
import com.intellij.codeInsight.daemon.LightIntentionActionTestCase;
import com.intellij.testFramework.LightProjectDescriptor;
import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase;
import org.jetbrains.annotations.NotNull;
public class UnrollLoopActionTest extends LightIntentionActionTestCase {
public void test() { doAllTests(); }
@NotNull
@Override
protected LightProjectDescriptor getProjectDescriptor() {
return LightCodeInsightFixtureTestCase.JAVA_9;
}
@Override
protected String getBasePath() {
return "/codeInsight/daemonCodeAnalyzer/quickFix/unrollLoop";