testParenthesesDontChangeIntention: fixes according to review IDEA-CR-34618

1. Support deparenthesizing in UnnecessaryConstantArrayCreationExpressionInspection; remove from exceptions
2. Link to IDEA-179081
3. Pull up shouldSkipByFamilyName
4. Explanatory comment on skipping elements with start offset equals to caret position
This commit is contained in:
Tagir Valeev
2018-07-06 17:03:12 +07:00
parent c802e28988
commit 069b588cde
7 changed files with 48 additions and 29 deletions
@@ -16,9 +16,6 @@
package com.intellij.java.propertyBased;
import com.intellij.codeInsight.intention.IntentionAction;
import com.intellij.codeInsight.intention.IntentionActionDelegate;
import com.intellij.codeInspection.LocalQuickFix;
import com.intellij.codeInspection.ex.QuickFixWrapper;
import com.intellij.openapi.editor.Editor;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
@@ -63,8 +60,8 @@ class JavaIntentionPolicy extends IntentionPolicy {
actionText.startsWith("Create missing 'switch' branches") || // if all existing branches do 'return something', we don't automatically generate compilable code for new branches
actionText.matches("Make .* default") || // can make interface non-functional and its lambdas incorrect
actionText.startsWith("Unimplement") || // e.g. leaves red references to the former superclass methods
actionText.equals("Make 'static'") || // from Non-'static' initializer inspection; it does not care if initializer refers instance members
actionText.equals("Split into declaration and initialization") || // constant field will not be compile-time constant anymore, so if used in annotation or switch label, a new error will appear
actionText.equals("Make 'static'") || // from Non-'static' initializer inspection; it does not care if initializer refers instance members, see IDEA-195165
actionText.equals("Split into declaration and initialization") || // TODO: remove when IDEA-179081 is fixed
actionText.equals("Replace with 'while'"); // TODO: remove when IDEA-195157 is fixed
}
@@ -139,32 +136,14 @@ class JavaParenthesesPolicy extends JavaIntentionPolicy {
super.shouldSkipIntention(actionText);
}
private static boolean shouldSkipByFamilyName(String familyName) {
return // int[] x = (new int[] {0}) -- correctly becomes not available
familyName.equals("Replace with array initializer expression") ||
// if((a && b)) -- extract "a" doesn't work, seems legit, remove parentheses first
@Override
protected boolean shouldSkipByFamilyName(@NotNull String familyName) {
return // if((a && b)) -- extract "a" doesn't work, seems legit, remove parentheses first
familyName.equals("Extract If Condition") ||
// TODO: sometimes DFA warning issued for parenthesized expression: fix and remove exception after merging dfa_refactoring branch
familyName.equals("Simplify boolean expression");
}
@Override
public boolean mayInvokeIntention(@NotNull IntentionAction action) {
IntentionAction original = action;
while (original instanceof IntentionActionDelegate) {
original = ((IntentionActionDelegate)original).getDelegate();
}
String familyName;
if (original instanceof QuickFixWrapper) {
LocalQuickFix fix = ((QuickFixWrapper)original).getFix();
familyName = fix.getFamilyName();
}
else {
familyName = original.getFamilyName();
}
return !shouldSkipByFamilyName(familyName) && super.mayInvokeIntention(action);
}
@NotNull
@Override
public List<PsiElement> getElementsToWrap(@NotNull PsiElement element) {
@@ -16,6 +16,9 @@
package com.intellij.testFramework.propertyBased;
import com.intellij.codeInsight.intention.IntentionAction;
import com.intellij.codeInsight.intention.IntentionActionDelegate;
import com.intellij.codeInspection.LocalQuickFix;
import com.intellij.codeInspection.ex.QuickFixWrapper;
import com.intellij.openapi.editor.Editor;
import com.intellij.psi.PsiComment;
import com.intellij.psi.PsiElement;
@@ -41,7 +44,22 @@ public class IntentionPolicy {
* </li>
*/
public boolean mayInvokeIntention(@NotNull IntentionAction action) {
return action.startInWriteAction() && !shouldSkipIntention(action.getText());
if (!action.startInWriteAction() || shouldSkipIntention(action.getText())) {
return false;
}
IntentionAction original = action;
while (original instanceof IntentionActionDelegate) {
original = ((IntentionActionDelegate)original).getDelegate();
}
String familyName;
if (original instanceof QuickFixWrapper) {
LocalQuickFix fix = ((QuickFixWrapper)original).getFix();
familyName = fix.getFamilyName();
}
else {
familyName = original.getFamilyName();
}
return shouldSkipByFamilyName(familyName);
}
protected boolean shouldSkipIntention(@NotNull String actionText) {
@@ -50,6 +68,10 @@ public class IntentionPolicy {
actionText.startsWith("Convert to project line separators"); // changes VFS, not document
}
protected boolean shouldSkipByFamilyName(@NotNull String familyName) {
return false;
}
/**
* Controls whether the given intention (already approved by {@link #mayInvokeIntention}) is allowed to
* introduce new highlighting errors into the code. It's recommended to return false by default,
@@ -169,6 +169,12 @@ public class InvokeIntention extends ActionOnFile {
List<IntentionAction> intentions) {
if (currentElement == null) return intentions;
int offset = editor.getCaretModel().getOffset();
/*
* When start offset of the element exactly equals to offset in the editor, we have a dubious situation
* which we'd like to avoid: sometimes intention looks what's on the left of caret, but we add a parenthesis there and things changed.
* E.g. "a" + <caret>"b" allows to join plus, but "a" + (<caret>"b") does not, and this looks legit as the intention reacts on plus,
* not on literal.
*/
List<PsiElement> elementsToWrap = ContainerUtil.filter(myPolicy.getElementsToWrap(currentElement),
e -> e.getTextRange().getStartOffset() != offset);
if (elementsToWrap.isEmpty()) return intentions;
@@ -19,6 +19,7 @@ import com.intellij.codeInspection.CleanupLocalInspectionTool;
import com.intellij.codeInspection.ProblemDescriptor;
import com.intellij.openapi.project.Project;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiUtil;
import com.siyeh.InspectionGadgetsBundle;
import com.siyeh.ig.BaseInspection;
import com.siyeh.ig.BaseInspectionVisitor;
@@ -84,7 +85,11 @@ public class UnnecessaryConstantArrayCreationExpressionInspection extends BaseIn
if (arrayInitializer == null) {
return;
}
new CommentTracker().replaceAndRestoreComments(newExpression, arrayInitializer);
PsiExpression target = newExpression;
while(target.getParent() instanceof PsiParenthesizedExpression) {
target = (PsiExpression)target.getParent();
}
new CommentTracker().replaceAndRestoreComments(target, arrayInitializer);
}
}
@@ -102,7 +107,7 @@ public class UnnecessaryConstantArrayCreationExpressionInspection extends BaseIn
if (!(parent instanceof PsiNewExpression)) {
return;
}
final PsiElement grandParent = parent.getParent();
final PsiElement grandParent = PsiUtil.skipParenthesizedExprUp(parent.getParent());
if (!(grandParent instanceof PsiVariable)) {
return;
}
@@ -0,0 +1,3 @@
class C {
int[] a = (new int[<caret>]{42});
}
@@ -22,6 +22,7 @@ import com.siyeh.ig.style.UnnecessaryConstantArrayCreationExpressionInspection;
public class UnnecessaryConstantArrayCreationExpressionFixTest extends IGQuickFixesTestCase {
public void testPrimitive() { doTest("int[]"); }
public void testParenthesized() { doTest("int[]"); }
public void testTwoDimension() { doTest("Map[][]"); }
public void testInitializerWithoutNew() { assertQuickfixNotAvailable(); }