MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-09-27 10:03:11 +07:00
Code Issues Packages Projects Releases Wiki Activity

IDEA-174151 'Replace with single Map method' suggests incorrect rewrites to computeIfAbsent

Browse Source 
Option added (on by default) to disable warning if side effects are possible.
This commit is contained in:
Tagir Valeev
2017-06-08 17:41:44 +07:00
parent 24f54f7b00
commit c0f0d3c636
10 changed files with 31 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
java/java-impl/src/com/intellij/codeInspection/java18api/Java8MapApiInspection.java
+19 -9
View File
@@ -52,6 +52,7 @@ public class Java8MapApiInspection extends BaseJavaBatchLocalInspectionTool {
public boolean mySuggestMapPutIfAbsent = true;
public boolean mySuggestMapMerge = true;
public boolean myTreatGetNullAsContainsKey = false;
public boolean mySideEffects = false;
@Nullable
@Override
@@ -62,6 +63,7 @@ public class Java8MapApiInspection extends BaseJavaBatchLocalInspectionTool {
panel.addCheckbox("Suggest conversion to Map.putIfAbsent", "mySuggestMapPutIfAbsent");
panel.addCheckbox("Suggest conversion to Map.merge", "mySuggestMapMerge");
panel.addCheckbox("Treat 'get(k) != null' the same as 'containsKey(k)' (may change semantics)", "myTreatGetNullAsContainsKey");
panel.addCheckbox("Suggest replacement even if lambda may have side effects", "mySideEffects");
return panel;
}
@@ -121,7 +123,9 @@ public class Java8MapApiInspection extends BaseJavaBatchLocalInspectionTool {
if (PsiTreeUtil.collectElements(presentValue, e -> PsiEquivalenceUtil.areElementsEquivalent(e, absentValue)).length == 0) {
return;
}
condition.register(holder, new ReplaceWithSingleMapOperation("merge", PsiTreeUtil
boolean informationLevel =
!mySideEffects && SideEffectChecker.mayHaveSideEffects(presentValue, ex -> condition.extractGetCall(ex) != null);
condition.register(holder, informationLevel, new ReplaceWithSingleMapOperation("merge", PsiTreeUtil
.getParentOfType(absentValue, PsiMethodCallExpression.class), presentValue, noneBranch));
}
}
@@ -139,11 +143,11 @@ public class Java8MapApiInspection extends BaseJavaBatchLocalInspectionTool {
condition.isMap(putCall.getMethodExpression().getQualifierExpression())) {
PsiExpression[] putArgs = putCall.getArgumentList().getExpressions();
if (putArgs.length != 2 || !condition.isKey(putArgs[0]) || !ExpressionUtils.isSimpleExpression(putArgs[1])) return;
condition.register(holder, new ReplaceWithSingleMapOperation("putIfAbsent", getCall, putArgs[1], result));
condition.register(holder, false, new ReplaceWithSingleMapOperation("putIfAbsent", getCall, putArgs[1], result));
}
if (mySuggestMapGetOrDefault && condition.isContainsKey() && ExpressionUtils.isSimpleExpression(noneExpression) &&
condition.isMapValueType(noneExpression.getType())) {
condition.register(holder, new ReplaceWithSingleMapOperation("getOrDefault", getCall, noneExpression, result));
condition.register(holder, false, new ReplaceWithSingleMapOperation("getOrDefault", getCall, noneExpression, result));
}
}
@@ -159,7 +163,7 @@ public class Java8MapApiInspection extends BaseJavaBatchLocalInspectionTool {
PsiExpression rValue = assignment.getRExpression();
if (ExpressionUtils.isSimpleExpression(rValue) && condition.isValueReference(assignment.getLExpression()) &&
!condition.isValueReference(rValue) && condition.isMapValueType(rValue.getType())) {
condition.register(holder, ReplaceWithSingleMapOperation.fromIf("getOrDefault", condition, rValue));
condition.register(holder, false, ReplaceWithSingleMapOperation.fromIf("getOrDefault", condition, rValue));
}
} else if (condition.isGetNull()) {
/*
@@ -171,25 +175,30 @@ public class Java8MapApiInspection extends BaseJavaBatchLocalInspectionTool {
*/
PsiExpression lambdaCandidate = extractLambdaCandidate(condition, noneBranch);
if (lambdaCandidate != null && mySuggestMapComputeIfAbsent) {
condition.register(holder, ReplaceWithSingleMapOperation.fromIf("computeIfAbsent", condition, lambdaCandidate));
boolean informationLevel = !mySideEffects && SideEffectChecker.mayHaveSideEffects(lambdaCandidate);
condition
.register(holder, informationLevel, ReplaceWithSingleMapOperation.fromIf("computeIfAbsent", condition, lambdaCandidate));
}
if (lambdaCandidate == null) {
PsiExpression expression = extractPutValue(condition, noneBranch);
if(expression != null) {
String replacement = null;
boolean informationLevel = false;
if (mySuggestMapPutIfAbsent && ExpressionUtils.isSimpleExpression(expression) && !condition.isValueReference(expression)) {
replacement = "putIfAbsent";
}
else if (mySuggestMapComputeIfAbsent && !condition.hasVariable()) {
informationLevel = !mySideEffects && SideEffectChecker.mayHaveSideEffects(expression);
replacement = "computeIfAbsent";
}
if(replacement != null) {
if(condition.hasVariable()) {
condition.register(holder, ReplaceWithSingleMapOperation.fromIf(replacement, condition, expression));
condition.register(holder, informationLevel, ReplaceWithSingleMapOperation.fromIf(replacement, condition, expression));
} else {
PsiMethodCallExpression call = PsiTreeUtil.getParentOfType(expression, PsiMethodCallExpression.class);
LOG.assertTrue(call != null);
condition.register(holder, new ReplaceWithSingleMapOperation(replacement, call, expression, noneBranch));
condition
.register(holder, informationLevel, new ReplaceWithSingleMapOperation(replacement, call, expression, noneBranch));
}
}
}
@@ -554,9 +563,10 @@ public class Java8MapApiInspection extends BaseJavaBatchLocalInspectionTool {
return myFullCondition;
}
public void register(ProblemsHolder holder, ReplaceWithSingleMapOperation fix) {
public void register(ProblemsHolder holder, boolean informationLevel, ReplaceWithSingleMapOperation fix) {
//noinspection DialogTitleCapitalization
holder.registerProblem(getFullCondition(), QuickFixBundle.message("java.8.map.api.inspection.description", fix.myMethodName), fix);
holder.registerProblem(getFullCondition(), QuickFixBundle.message("java.8.map.api.inspection.description", fix.myMethodName),
informationLevel ? ProblemHighlightType.INFORMATION : ProblemHighlightType.GENERIC_ERROR_OR_WARNING, fix);
}
public boolean isMapValueType(@Nullable PsiType type) {
java/java-tests/testData/inspection/java8MapApi/afterComputeIfAbsentSingleLine.java
+1 -1
View File
@@ -1,4 +1,4 @@
// "Replace with 'computeIfAbsent' method call" "true"
// "Replace with 'computeIfAbsent' method call" "GENERIC_ERROR_OR_WARNING"
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
java/java-tests/testData/inspection/java8MapApi/afterComputeIfAbsentUsedKey.java
+1 -1
View File
@@ -1,4 +1,4 @@
// "Replace with 'computeIfAbsent' method call" "true"
// "Replace with 'computeIfAbsent' method call" "INFORMATION"
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
java/java-tests/testData/inspection/java8MapApi/afterMergeSimple.java
+1 -1
View File
@@ -1,4 +1,4 @@
// "Replace with 'merge' method call" "true"
// "Replace with 'merge' method call" "GENERIC_ERROR_OR_WARNING"
import java.util.Map;
public class Main {
java/java-tests/testData/inspection/java8MapApi/beforeComputeIfAbsentSingleLine.java
+1 -1
View File
@@ -1,4 +1,4 @@
// "Replace with 'computeIfAbsent' method call" "true"
// "Replace with 'computeIfAbsent' method call" "GENERIC_ERROR_OR_WARNING"
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
java/java-tests/testData/inspection/java8MapApi/beforeComputeIfAbsentUsedKey.java
+1 -1
View File
@@ -1,4 +1,4 @@
// "Replace with 'computeIfAbsent' method call" "true"
// "Replace with 'computeIfAbsent' method call" "INFORMATION"
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
java/java-tests/testData/inspection/java8MapApi/beforeMergeSimple.java
+1 -1
View File
@@ -1,4 +1,4 @@
// "Replace with 'merge' method call" "true"
// "Replace with 'merge' method call" "GENERIC_ERROR_OR_WARNING"
import java.util.Map;
public class Main {
plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/MismatchedStringBuilderQueryUpdateInspection.java
+1 -1
View File
@@ -270,7 +270,7 @@ public class MismatchedStringBuilderQueryUpdateInspection extends BaseInspection
if (hasReferenceToVariable(variable, qualifierExpression)) {
PsiElement parent = PsiTreeUtil.getParentOfType(expression, PsiStatement.class, PsiLambdaExpression.class);
if (parent instanceof PsiStatement &&
!SideEffectChecker.mayHaveSideEffects((PsiStatement)parent, this::isSideEffectFreeBuilderMethodCall)) {
!SideEffectChecker.mayHaveSideEffects(parent, this::isSideEffectFreeBuilderMethodCall)) {
return;
}
queried = true;
plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/SideEffectChecker.java
+2 -2
View File
@@ -65,9 +65,9 @@ public class SideEffectChecker {
return visitor.mayHaveSideEffects();
}
public static boolean mayHaveSideEffects(@NotNull PsiStatement statement, Predicate<PsiMethodCallExpression> shouldIgnoreCall) {
public static boolean mayHaveSideEffects(@NotNull PsiElement element, Predicate<PsiMethodCallExpression> shouldIgnoreCall) {
final SideEffectsVisitor visitor = new SideEffectsVisitor(null, shouldIgnoreCall);
statement.accept(visitor);
element.accept(visitor);
return visitor.mayHaveSideEffects();
}
resources-en/src/inspectionDescriptions/Java8MapApi.html
+3
View File
@@ -28,6 +28,9 @@ Reports calls to <code>Map.get()</code> which could be replaced with <code>getOr
</li>
</ul>
<p>Note that replacement with <code>computeIfAbsent()</code> or <code>merge()</code> may work incorrectly for some <code>Map</code>
implementations if the code extracted to lambda expression modifies the same <code>Map</code>. By default,
warning is not issued if this code may have side effects. If desired, use the last checkbox to issue warning always.</p>
<!-- tooltip end -->
<p>This inspection only reports if the project or module is configured to use a language level of 8 or higher.</p>
<p><small>New in 2016.3</small></p>