IDEA-163874 "Replace inefficient Stream API call chains ending with count" removes comments

IDEA-163875 Inspection "Replace inefficient Stream API call chains ending with count()" should respect alignment settings
This commit is contained in:
Tagir Valeev
2016-11-22 12:14:21 +07:00
parent 7f8d10752e
commit ec864dc55a
23 changed files with 281 additions and 90 deletions
@@ -21,6 +21,7 @@ import com.intellij.openapi.project.Project;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiUtil;
import com.intellij.psi.util.RedundantCastUtil;
import com.siyeh.ig.psiutils.CommentTracker;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -168,8 +169,10 @@ public class ReplaceInefficientStreamCountInspection extends BaseJavaBatchLocalI
PsiMethod qualifier,
PsiElementFactory factory) {
if (!isCallOf(qualifier, CommonClassNames.JAVA_UTIL_COLLECTION, STREAM_METHOD, 0)) return;
PsiExpression qualifierExpression = qualifierCall.getMethodExpression().getQualifierExpression();
PsiReferenceExpression methodExpression = qualifierCall.getMethodExpression();
PsiExpression qualifierExpression = methodExpression.getQualifierExpression();
if(qualifierExpression == null) return;
methodExpression.handleElementRename(SIZE_METHOD);
boolean addCast = true;
PsiElement toReplace = countCall;
PsiElement parent = PsiUtil.skipParenthesizedExprUp(countCall.getParent());
@@ -184,8 +187,9 @@ public class ReplaceInefficientStreamCountInspection extends BaseJavaBatchLocalI
}
}
}
String replacementText = (addCast ? "(long) " : "")+qualifierExpression.getText()+"."+SIZE_METHOD+"()";
PsiElement replacement = toReplace.replace(factory.createExpressionFromText(replacementText, countCall));
CommentTracker ct = new CommentTracker();
String replacementText = (addCast ? "(long) " : "") + ct.text(qualifierCall);
PsiElement replacement = ct.replaceAndRestoreComments(toReplace, factory.createExpressionFromText(replacementText, countCall));
if (replacement instanceof PsiTypeCastExpression && RedundantCastUtil.isCastRedundant((PsiTypeCastExpression)replacement)) {
RedundantCastUtil.removeCast((PsiTypeCastExpression)replacement);
}
@@ -32,7 +32,7 @@ import com.intellij.psi.tree.IElementType;
import com.intellij.psi.util.InheritanceUtil;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.intellij.util.containers.ContainerUtil;
import com.siyeh.ig.psiutils.CommentTracker;
import com.siyeh.ig.psiutils.ControlFlowUtils;
import one.util.streamex.MoreCollectors;
import one.util.streamex.StreamEx;
@@ -41,8 +41,6 @@ import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.Collection;
/**
* @author Tagir Valeev
*/
@@ -169,13 +167,13 @@ public class Java8CollectionRemoveIfInspection extends BaseJavaBatchLocalInspect
PsiElementFactory factory = JavaPsiFacade.getElementFactory(project);
String replacement = null;
if (!FileModificationService.getInstance().preparePsiElementForWrite(element)) return;
CommentTracker ct = new CommentTracker();
if (statements.length == 2 && statements[1] instanceof PsiIfStatement) {
PsiVariable variable = declaration.getNextElementVariable(statements[0]);
if (variable == null) return;
PsiExpression condition = ((PsiIfStatement)statements[1]).getCondition();
if (condition == null) return;
replacement = (declaration.myCollection == null ? "" : declaration.myCollection.getText() + ".") +
"removeIf(" + LambdaUtil.createLambda(variable, condition) + ");";
replacement = generateRemoveIf(declaration, ct, condition, variable.getName());
}
else if (statements.length == 1 && statements[0] instanceof PsiIfStatement){
PsiExpression condition = ((PsiIfStatement)statements[0]).getCondition();
@@ -191,19 +189,22 @@ public class Java8CollectionRemoveIfInspection extends BaseJavaBatchLocalInspect
info = javaCodeStyleManager.suggestVariableName(VariableKind.PARAMETER, "value", null, type);
}
String paramName = javaCodeStyleManager.suggestUniqueVariableName(info, condition, true).names[0];
call.replace(factory.createIdentifier(paramName));
replacement = (declaration.myCollection == null ? "" : declaration.myCollection.getText() + ".") +
"removeIf(" + paramName + "->"+condition.getText() + ");";
ct.replace(call, factory.createIdentifier(paramName));
replacement = generateRemoveIf(declaration, ct, condition, paramName);
}
}
if(replacement == null) return;
Collection<PsiComment> comments = ContainerUtil.map(PsiTreeUtil.findChildrenOfType(loop, PsiComment.class),
comment -> (PsiComment)comment.copy());
PsiElement result = loop.replace(factory.createStatementFromText(replacement, loop));
if (previous != null) previous.delete();
if (replacement == null) return;
if (previous != null) ct.delete(previous);
PsiElement result = ct.replaceAndRestoreComments(loop, factory.createStatementFromText(replacement, loop));
LambdaCanBeMethodReferenceInspection.replaceAllLambdasWithMethodReferences(result);
CodeStyleManager.getInstance(project).reformat(result);
comments.forEach(comment -> result.getParent().addBefore(comment, result));
}
@NotNull
private static String generateRemoveIf(IteratorDeclaration declaration, CommentTracker ct,
PsiExpression condition, String paramName) {
return (declaration.myCollection == null ? "" : ct.text(declaration.myCollection) + ".") +
"removeIf(" + paramName + "->" + ct.text(condition) + ");";
}
}
@@ -29,7 +29,7 @@ import com.intellij.psi.codeStyle.CodeStyleManager;
import com.intellij.psi.codeStyle.JavaCodeStyleManager;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.intellij.util.containers.ContainerUtil;
import com.siyeh.ig.psiutils.CommentTracker;
import com.siyeh.ig.psiutils.ControlFlowUtils;
import com.siyeh.ig.psiutils.EquivalenceChecker;
import com.siyeh.ig.psiutils.ExpressionUtils;
@@ -39,7 +39,6 @@ import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import javax.swing.*;
import java.util.Collection;
/**
* @author Tagir Valeev
@@ -236,33 +235,29 @@ public class Java8ReplaceMapGetInspection extends BaseJavaBatchLocalInspectionTo
PsiElement statement = PsiTreeUtil.skipSiblingsBackward(ifStatement, PsiWhiteSpace.class, PsiComment.class);
PsiMethodCallExpression getCall = tryExtractMapGetCall(value, statement);
if(getCall == null || !Java8CollectionsApiInspection.isJavaUtilMapMethodWithName(getCall, "get")) return;
PsiElement nameElement = getCall.getMethodExpression().getReferenceNameElement();
if(nameElement == null) return;
PsiExpression[] args = getCall.getArgumentList().getExpressions();
if(args.length != 1) return;
PsiStatement thenBranch = ControlFlowUtils.stripBraces(ifStatement.getThenBranch());
Collection<PsiComment> comments = ContainerUtil.map(PsiTreeUtil.findChildrenOfType(ifStatement, PsiComment.class),
comment -> (PsiComment)comment.copy());
PsiElementFactory factory = JavaPsiFacade.getElementFactory(project);
PsiAssignmentExpression assignment = ExpressionUtils.getAssignment(thenBranch);
if (!FileModificationService.getInstance().preparePsiElementForWrite(element)) return;
CommentTracker ct = new CommentTracker();
PsiReferenceExpression methodExpression = getCall.getMethodExpression();
if(assignment != null) {
PsiExpression defaultValue = assignment.getRExpression();
if (!ExpressionUtils.isSimpleExpression(defaultValue)) return;
nameElement.replace(factory.createIdentifier("getOrDefault"));
getCall.getArgumentList().add(defaultValue);
methodExpression.handleElementRename("getOrDefault");
getCall.getArgumentList().add(ct.markUsed(defaultValue));
} else {
PsiExpression lambdaCandidate =
extractLambdaCandidate(thenBranch, getCall.getMethodExpression().getQualifierExpression(), args[0], value);
PsiExpression lambdaCandidate = extractLambdaCandidate(thenBranch, methodExpression.getQualifierExpression(), args[0], value);
if (lambdaCandidate == null) return;
nameElement.replace(factory.createIdentifier("computeIfAbsent"));
methodExpression.handleElementRename("computeIfAbsent");
String varName = JavaCodeStyleManager.getInstance(project).suggestUniqueVariableName("k", lambdaCandidate, true);
PsiExpression lambda = factory.createExpressionFromText(varName + " -> " + lambdaCandidate.getText(), lambdaCandidate);
PsiExpression lambda = factory.createExpressionFromText(varName + " -> " + ct.text(lambdaCandidate), lambdaCandidate);
getCall.getArgumentList().add(lambda);
}
ifStatement.delete();
ct.deleteAndRestoreComments(ifStatement);
CodeStyleManager.getInstance(project).reformat(statement);
comments.forEach(comment -> statement.getParent().addBefore(comment, statement));
}
}
}
@@ -4,6 +4,7 @@ import java.util.Arrays;
class Test {
long cnt() {
return (long) Arrays.asList('d', 'e', 'f').size();
/*count*/
return (long) Arrays.asList('d', 'e', 'f')./*stream*/size()/*after*/;
}
}
@@ -4,6 +4,6 @@ import java.util.Arrays;
class Test {
int cnt() {
return Arrays.asList('d', 'e', 'f').size();
return Arrays.asList('d', 'e', 'f').size(/*inside*/);
}
}
@@ -3,7 +3,9 @@
import java.util.Arrays;
class Test {
short cnt() {
return (short) Arrays.asList('d', 'e', 'f').size();
}
short cnt() {
/*before dot*/
//after dot
return (short) Arrays.asList('d', 'e', 'f').size();
}
}
@@ -0,0 +1,8 @@
// "Replace Collection.stream().count() with Collection.size()" "true"
import java.util.Arrays;
class Test {
/*count*/
long cnt = (long) Arrays.asList('d', 'e', 'f')./*stream*/size()/*after*/;
}
@@ -4,6 +4,6 @@ import java.util.Arrays;
class Test {
long cnt() {
return Arrays.asList('d', 'e', 'f').stream().c<caret>ount();
return Arrays.asList('d', 'e', 'f')./*stream*/stream()./*count*/c<caret>ount()/*after*/;
}
}
@@ -4,6 +4,6 @@ import java.util.Arrays;
class Test {
int cnt() {
return (int) Arrays.asList('d', 'e', 'f').stream().c<caret>ount();
return (int) Arrays.asList('d', 'e', 'f').stream(/*inside*/).c<caret>ount();
}
}
@@ -3,7 +3,8 @@
import java.util.Arrays;
class Test {
short cnt() {
return (short) Arrays.asList('d', 'e', 'f').stream().c<caret>ount();
}
short cnt() {
return (short) Arrays.asList('d', 'e', 'f').stream()/*before dot*/.//after dot
c<caret>ount();
}
}
@@ -0,0 +1,7 @@
// "Replace Collection.stream().count() with Collection.size()" "true"
import java.util.Arrays;
class Test {
long cnt = Arrays.asList('d', 'e', 'f')./*stream*/stream()./*count*/c<caret>ount()/*after*/;
}
@@ -2,10 +2,12 @@
import java.util.*;
public class Main {
public void removeEmpty(List<String> list) throws Exception {
// iterate over list
// if it's empty
/* remove! */
list.removeIf(String::isEmpty);
}
public void removeEmpty(List<String> list) throws Exception {
/*here's list*/
// iterate over list
// if it's empty
/* remove! */
/*empty?*/
list.removeIf(String::isEmpty);
}
}
@@ -4,6 +4,6 @@ import java.util.List;
public class Main {
public void testIterator(List<List<String>> data, boolean b) {
data.removeIf(strings -> strings.isEmpty() && b);
data.removeIf(strings -> strings.isEmpty() && /* also check the flag */ b);
}
}
@@ -2,8 +2,10 @@
import java.util.*;
public class Main {
public void removeEmpty(List<String> list) throws Exception {
// remove empty
list.removeIf(String::isEmpty);
}
public void removeEmpty(List<String> list) throws Exception {
// remove empty
// everything is ok!
list.removeIf(str -> str.trim()/*trimmed empty*/
.isEmpty());
}
}
@@ -2,15 +2,15 @@
import java.util.*;
public class Main {
public void removeEmpty(List<String> list) throws Exception {
f<caret>or(Iterator<String> it = list.iterator(); it.hasNext();) {
// iterate over list
String str = it.next();
// if it's empty
if(str.isEmpty()) {
/* remove! */
it.remove();
}
public void removeEmpty(List<String> list) throws Exception {
f<caret>or(Iterator<String> it = list/*here's list*/.iterator(); it.hasNext();) {
// iterate over list
String str = it.next();
// if it's empty
if(str./*empty?*/isEmpty()) {
/* remove! */
it.remove();
}
}
}
}
}
@@ -5,7 +5,7 @@ import java.util.List;
public class Main {
public void testIterator(List<List<String>> data, boolean b) {
for(Ite<caret>rator<List<String>> iter = data.iterator(); iter.hasNext();) {
if(iter.next().isEmpty() && b) {
if(iter.next().isEmpty() && /* also check the flag */ b) {
iter.remove();
}
}
@@ -2,14 +2,16 @@
import java.util.*;
public class Main {
public void removeEmpty(List<String> list) throws Exception {
Iterator<String> it = list.iterator();
while<caret>(it.hasNext()) {
String str = it.next();
// remove empty
if(str.isEmpty()) {
it.remove();
}
public void removeEmpty(List<String> list) throws Exception {
Iterator<String> it = list.iterator();
while<caret>(it.hasNext()) {
String str = it.next();
// remove empty
if(str.trim()/*trimmed empty*/
.isEmpty()) {
it.remove();
}
// everything is ok!
}
}
}
}
@@ -5,7 +5,8 @@ import java.util.Map;
public class Main {
public void testMap(Map<String, List<String>> map, String key, String value) {
List<String> list = map.computeIfAbsent(key, k -> new ArrayList<>());
List<String> list = map.computeIfAbsent(key, k -> new /*create new ArrayList*/ ArrayList<>());
// and put it
list.add(value);
}
}
@@ -7,10 +7,10 @@ public class Main {
private String str;
public void testGetOrDefault(Map<String, String> map, String key, Main other) {
/*
str = map.getOrDefault(key, NONE);
/*
block comment
*/
str = map.getOrDefault(key, NONE);
System.out.println(str);
}
}
@@ -2,10 +2,11 @@
import java.util.Map;
public class Main {
public void testGetOrDefault(Map<String, String> map, String key, Main other) {
// comment
String a = null, str = map.getOrDefault(key, "");
System.out.println(str);
}
public void testGetOrDefault(Map<String, String> map, String key, Main other) {
String a = null, str = map.getOrDefault(key, "");
// before if
// comment
/* after comment */
System.out.println(str);
}
}
@@ -7,7 +7,8 @@ public class Main {
public void testMap(Map<String, List<String>> map, String key, String value) {
List<String> list = map.get(key);
if(list == nul<caret>l) {
list = new ArrayList<>();
list = new /*create new ArrayList*/ ArrayList<>();
// and put it
map.put(key, list);
}
list.add(value);
@@ -2,13 +2,14 @@
import java.util.Map;
public class Main {
public void testGetOrDefault(Map<String, String> map, String key, Main other) {
String a = null, str = map.get(key);
if(str == nu<caret>ll) {
// comment
str = "";
public void testGetOrDefault(Map<String, String> map, String key, Main other) {
String a = null, str = map.get(key);
// before if
if(str == nu<caret>ll) {
// comment
str = "";
/* after comment */
}
System.out.println(str);
}
System.out.println(str);
}
}
@@ -0,0 +1,162 @@
/*
* Copyright 2000-2016 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.siyeh.ig.psiutils;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
import org.jetbrains.annotations.NotNull;
import java.util.ArrayList;
import java.util.List;
/**
* A helper class to implement quick-fix which collects removed comments from the PSI and can restore them at once.
*
* After this object restores comments, it becomes unusable.
*
* @author Tagir Valeev
*/
public class CommentTracker {
private List<PsiElement> ignoredParents = new ArrayList<>();
private List<PsiComment> comments = new ArrayList<>();
/**
* Marks the element as used and returns its text. The comments from used elements will not be extracted.
*
* @param element element to return the text
* @return a text to be inserted into refactored code
*/
public @NotNull String text(@NotNull PsiElement element) {
checkState();
ignoredParents.add(element);
return element.getText();
}
/**
* Marks the element as used and returns it. The comments from used elements will not be extracted.
*
* @param element element to mark
* @param <T> the type of the element
* @return the passed argument
*/
public @NotNull <T extends PsiElement> T markUsed(@NotNull T element) {
checkState();
ignoredParents.add(element);
return element;
}
/**
* Deletes given PsiElement collecting all the comments inside it.
*
* @param element element to delete
*/
public void delete(@NotNull PsiElement element) {
grabComments(element);
element.delete();
}
/**
* Deletes given PsiElement replacing it with the comments including comments inside the deleted element
* and previously gathered comments.
*
* <p>After calling this method the tracker cannot be used anymore.</p>
*
* @param element element to delete
*/
public void deleteAndRestoreComments(@NotNull PsiElement element) {
grabComments(element);
insertCommentsBefore(element);
element.delete();
}
/**
* Replaces given PsiElement collecting all the comments inside it.
*
* @param element element to replace
* @param replacement replacement element
* @return the element which was actually inserted in the tree (either <code>replacement</code> or its copy)
*/
public @NotNull PsiElement replace(@NotNull PsiElement element, @NotNull PsiElement replacement) {
grabComments(element);
return element.replace(replacement);
}
/**
* Replaces given PsiElement collecting all the comments inside it and restore comments putting them
* to the appropriate place before replaced element.
*
* <p>After calling this method the tracker cannot be used anymore.</p>
*
* @param element element to replace
* @param replacement replacement element
* @return the element which was actually inserted in the tree (either <code>replacement</code> or its copy)
*/
public @NotNull PsiElement replaceAndRestoreComments(@NotNull PsiElement element, @NotNull PsiElement replacement) {
PsiElement result = replace(element, replacement);
PsiElement anchor = PsiTreeUtil.getNonStrictParentOfType(result, PsiStatement.class, PsiLambdaExpression.class, PsiVariable.class);
if(anchor instanceof PsiLambdaExpression && anchor != result) {
anchor = ((PsiLambdaExpression)anchor).getBody();
}
if(anchor == null) anchor = result;
insertCommentsBefore(anchor);
return result;
}
/**
* Inserts gathered comments just before given anchor element
*
* <p>After calling this method the tracker cannot be used anymore.</p>
*
* @param anchor
*/
public void insertCommentsBefore(@NotNull PsiElement anchor) {
checkState();
if(!comments.isEmpty()) {
PsiElement parent = anchor.getParent();
PsiElementFactory factory = JavaPsiFacade.getElementFactory(anchor.getProject());
for(PsiComment comment : comments) {
PsiElement added = parent.addBefore(factory.createCommentFromText(comment.getText(), anchor), anchor);
PsiElement prevSibling = added.getPrevSibling();
if(prevSibling instanceof PsiWhiteSpace) {
PsiWhiteSpace whiteSpaceBefore = (PsiWhiteSpace)prevSibling;
PsiElement prev = anchor.getPrevSibling();
if (prev instanceof PsiWhiteSpace) {
prev.replace(whiteSpaceBefore);
}
else {
parent.addBefore(whiteSpaceBefore, anchor);
}
}
}
}
comments = null;
}
private void grabComments(PsiElement element) {
checkState();
for(PsiComment comment : PsiTreeUtil.collectElementsOfType(element, PsiComment.class)) {
if(ignoredParents.stream().noneMatch(parent -> PsiTreeUtil.isAncestor(parent, comment, false))) {
comments.add(comment);
}
}
}
private void checkState() {
if(comments == null) {
throw new IllegalStateException(getClass().getSimpleName()+" has been already used");
}
}
}