Simplify Demorgans intention, don't keep unnecessary parentheses, add tests

This commit is contained in:
Bas Leijdekkers
2012-01-31 17:06:18 +01:00
parent 7638e45ab3
commit 55ee5dcb50
10 changed files with 182 additions and 163 deletions
@@ -1,5 +1,5 @@
/*
* Copyright 2003-2011 Dave Griffith, Bas Leijdekkers
* Copyright 2003-2012 Dave Griffith, Bas Leijdekkers
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -24,7 +24,7 @@ import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.psi.*;
import com.intellij.psi.codeStyle.CodeStyleManager;
import com.intellij.psi.codeStyle.JavaCodeStyleManager;
import com.intellij.psi.util.PsiUtil;
import com.intellij.psi.util.PsiUtilCore;
import com.intellij.util.IncorrectOperationException;
import com.siyeh.IntentionPowerPackBundle;
import com.siyeh.ipp.psiutils.BoolUtils;
@@ -46,8 +46,7 @@ public abstract class Intention extends PsiElementBaseIntentionAction {
}
@Override
public void invoke(Project project, Editor editor, PsiElement element)
throws IncorrectOperationException {
public void invoke(Project project, Editor editor, PsiElement element) throws IncorrectOperationException {
if (!isWritable(project, element)) {
return;
}
@@ -58,33 +57,25 @@ public abstract class Intention extends PsiElementBaseIntentionAction {
processIntention(matchingElement);
}
protected abstract void processIntention(@NotNull PsiElement element)
throws IncorrectOperationException;
protected abstract void processIntention(@NotNull PsiElement element) throws IncorrectOperationException;
@NotNull
protected abstract PsiElementPredicate getElementPredicate();
protected static void replaceExpression(@NotNull String newExpression,
@NotNull PsiExpression expression)
protected static void replaceExpression(@NotNull String newExpression, @NotNull PsiExpression expression)
throws IncorrectOperationException {
final Project project = expression.getProject();
final PsiElementFactory factory =
JavaPsiFacade.getElementFactory(project);
final PsiExpression newCall =
factory.createExpressionFromText(newExpression, expression);
final PsiElementFactory factory = JavaPsiFacade.getElementFactory(project);
final PsiExpression newCall = factory.createExpressionFromText(newExpression, expression);
final PsiElement insertedElement = expression.replace(newCall);
final CodeStyleManager codeStyleManager =
CodeStyleManager.getInstance(project);
final CodeStyleManager codeStyleManager = CodeStyleManager.getInstance(project);
codeStyleManager.reformat(insertedElement);
}
protected static void replaceExpressionWithNegatedExpression(
@NotNull PsiExpression newExpression,
@NotNull PsiExpression expression)
protected static void replaceExpressionWithNegatedExpression(@NotNull PsiExpression newExpression, @NotNull PsiExpression expression)
throws IncorrectOperationException {
final Project project = expression.getProject();
final PsiElementFactory factory =
JavaPsiFacade.getElementFactory(project);
final PsiElementFactory factory = JavaPsiFacade.getElementFactory(project);
PsiExpression expressionToReplace = expression;
final String newExpressionText = newExpression.getText();
final String expString;
@@ -93,36 +84,29 @@ public abstract class Intention extends PsiElementBaseIntentionAction {
expString = newExpressionText;
}
else if (ComparisonUtils.isComparison(newExpression)) {
final PsiBinaryExpression binaryExpression =
(PsiBinaryExpression)newExpression;
final String negatedComparison =
ComparisonUtils.getNegatedComparison(binaryExpression.getOperationTokenType());
final PsiBinaryExpression binaryExpression = (PsiBinaryExpression)newExpression;
final String negatedComparison = ComparisonUtils.getNegatedComparison(binaryExpression.getOperationTokenType());
final PsiExpression lhs = binaryExpression.getLOperand();
final PsiExpression rhs = binaryExpression.getROperand();
assert rhs != null;
expString = lhs.getText() + negatedComparison + rhs.getText();
}
else {
if (ParenthesesUtils.getPrecedence(newExpression) >
ParenthesesUtils.PREFIX_PRECEDENCE) {
if (ParenthesesUtils.getPrecedence(newExpression) > ParenthesesUtils.PREFIX_PRECEDENCE) {
expString = "!(" + newExpressionText + ')';
}
else {
expString = '!' + newExpressionText;
}
}
final PsiExpression newCall =
factory.createExpressionFromText(expString, expression);
final PsiExpression newCall = factory.createExpressionFromText(expString, expression);
assert expressionToReplace != null;
final PsiElement insertedElement = expressionToReplace.replace(newCall);
final CodeStyleManager codeStyleManager =
CodeStyleManager.getInstance(project);
final CodeStyleManager codeStyleManager = CodeStyleManager.getInstance(project);
codeStyleManager.reformat(insertedElement);
}
protected static void replaceExpressionWithNegatedExpressionString(
@NotNull String newExpression,
@NotNull PsiExpression expression)
protected static void replaceExpressionWithNegatedExpressionString(@NotNull String newExpression, @NotNull PsiExpression expression)
throws IncorrectOperationException {
final Project project = expression.getProject();
final JavaPsiFacade psiFacade = JavaPsiFacade.getInstance(project);
@@ -130,52 +114,44 @@ public abstract class Intention extends PsiElementBaseIntentionAction {
PsiExpression expressionToReplace = expression;
final String expString;
if (BoolUtils.isNegated(expression)) {
expressionToReplace = BoolUtils.findNegation(expression);
expressionToReplace = BoolUtils.findNegation(expressionToReplace);
expString = newExpression;
}
else {
PsiElement parent = expressionToReplace.getParent();
while (parent instanceof PsiParenthesizedExpression) {
expressionToReplace = (PsiExpression)parent;
parent = parent.getParent();
}
expString = "!(" + newExpression + ')';
}
final PsiExpression newCall =
factory.createExpressionFromText(expString, expression);
final PsiExpression newCall = factory.createExpressionFromText(expString, expression);
assert expressionToReplace != null;
final PsiElement insertedElement = expressionToReplace.replace(newCall);
final CodeStyleManager codeStyleManager =
CodeStyleManager.getInstance(project);
final CodeStyleManager codeStyleManager = CodeStyleManager.getInstance(project);
codeStyleManager.reformat(insertedElement);
}
protected static void replaceStatement(
@NonNls @NotNull String newStatementText,
@NonNls @NotNull PsiStatement statement)
protected static void replaceStatement(@NonNls @NotNull String newStatementText, @NonNls @NotNull PsiStatement statement)
throws IncorrectOperationException {
final Project project = statement.getProject();
final PsiElementFactory factory =
JavaPsiFacade.getElementFactory(project);
final PsiStatement newStatement =
factory.createStatementFromText(newStatementText, statement);
final PsiElementFactory factory = JavaPsiFacade.getElementFactory(project);
final PsiStatement newStatement = factory.createStatementFromText(newStatementText, statement);
final PsiElement insertedElement = statement.replace(newStatement);
final CodeStyleManager codeStyleManager =
CodeStyleManager.getInstance(project);
final CodeStyleManager codeStyleManager = CodeStyleManager.getInstance(project);
codeStyleManager.reformat(insertedElement);
}
protected static void replaceStatementAndShorten(
@NonNls @NotNull String newStatementText,
@NonNls @NotNull PsiStatement statement)
protected static void replaceStatementAndShorten(@NonNls @NotNull String newStatementText, @NonNls @NotNull PsiStatement statement)
throws IncorrectOperationException {
final Project project = statement.getProject();
final JavaPsiFacade psiFacade = JavaPsiFacade.getInstance(project);
final PsiElementFactory factory = psiFacade.getElementFactory();
final PsiStatement newStatement =
factory.createStatementFromText(newStatementText, statement);
final PsiStatement newStatement = factory.createStatementFromText(newStatementText, statement);
final PsiElement insertedElement = statement.replace(newStatement);
final JavaCodeStyleManager javaCodeStyleManager =
JavaCodeStyleManager.getInstance(project);
final PsiElement shortenedElement =
javaCodeStyleManager.shortenClassReferences(insertedElement);
final CodeStyleManager codeStyleManager =
CodeStyleManager.getInstance(project);
final JavaCodeStyleManager javaCodeStyleManager = JavaCodeStyleManager.getInstance(project);
final PsiElement shortenedElement = javaCodeStyleManager.shortenClassReferences(insertedElement);
final CodeStyleManager codeStyleManager = CodeStyleManager.getInstance(project);
codeStyleManager.reformat(shortenedElement);
}
@@ -202,8 +178,7 @@ public abstract class Intention extends PsiElementBaseIntentionAction {
}
@Override
public boolean isAvailable(@NotNull Project project, Editor editor,
@NotNull PsiElement element) {
public boolean isAvailable(@NotNull Project project, Editor editor, @NotNull PsiElement element) {
return findMatchingElement(element, editor) != null;
}
@@ -213,14 +188,12 @@ public abstract class Intention extends PsiElementBaseIntentionAction {
}
private static boolean isWritable(Project project, PsiElement element) {
final VirtualFile virtualFile = PsiUtil.getVirtualFile(element);
final VirtualFile virtualFile = PsiUtilCore.getVirtualFile(element);
if (virtualFile == null) {
return true;
}
final ReadonlyStatusHandler readonlyStatusHandler =
ReadonlyStatusHandler.getInstance(project);
final ReadonlyStatusHandler.OperationStatus operationStatus =
readonlyStatusHandler.ensureFilesWritable(virtualFile);
final ReadonlyStatusHandler readonlyStatusHandler = ReadonlyStatusHandler.getInstance(project);
final ReadonlyStatusHandler.OperationStatus operationStatus = readonlyStatusHandler.ensureFilesWritable(virtualFile);
return !operationStatus.hasReadonlyFiles();
}
@@ -1,5 +1,5 @@
/*
* Copyright 2003-2006 Dave Griffith, Bas Leijdekkers
* Copyright 2003-2012 Dave Griffith, Bas Leijdekkers
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -29,8 +29,7 @@ import org.jetbrains.annotations.NotNull;
public class DemorgansIntention extends MutablyNamedIntention {
protected String getTextForElement(PsiElement element) {
final PsiPolyadicExpression binaryExpression =
(PsiPolyadicExpression)element;
final PsiPolyadicExpression binaryExpression = (PsiPolyadicExpression)element;
final IElementType tokenType = binaryExpression.getOperationTokenType();
if (tokenType.equals(JavaTokenType.ANDAND)) {
return IntentionPowerPackBundle.message("demorgans.intention.name1");
@@ -45,84 +44,55 @@ public class DemorgansIntention extends MutablyNamedIntention {
return new ConjunctionPredicate();
}
public void processIntention(@NotNull PsiElement element)
throws IncorrectOperationException {
PsiPolyadicExpression exp =
(PsiPolyadicExpression)element;
final IElementType tokenType = exp.getOperationTokenType();
PsiElement parent = exp.getParent();
while (isConjunctionExpression(parent, tokenType)) {
exp = (PsiPolyadicExpression)parent;
assert exp != null;
parent = exp.getParent();
}
final String newExpression =
convertConjunctionExpression(exp, tokenType);
replaceExpressionWithNegatedExpressionString(newExpression,
exp);
public void processIntention(@NotNull PsiElement element) throws IncorrectOperationException {
final PsiPolyadicExpression polyadicExpression = (PsiPolyadicExpression)element;
final String newExpression = convertConjunctionExpression(polyadicExpression);
replaceExpressionWithNegatedExpressionString(newExpression, polyadicExpression);
}
private static String convertConjunctionExpression(PsiPolyadicExpression exp,
IElementType tokenType) {
private static String convertConjunctionExpression(PsiPolyadicExpression polyadicExpression) {
final IElementType tokenType = polyadicExpression.getOperationTokenType();
final String flippedConjunction;
if (tokenType.equals(JavaTokenType.ANDAND)) {
flippedConjunction = "||";
final boolean tokenTypeAndAnd = tokenType.equals(JavaTokenType.ANDAND);
flippedConjunction = tokenTypeAndAnd ? "||" : "&&";
final StringBuilder result = new StringBuilder();
for (PsiExpression operand : polyadicExpression.getOperands()) {
if (result.length() != 0) {
result.append(flippedConjunction);
}
result.append(convertLeafExpression(operand, tokenTypeAndAnd));
}
else {
flippedConjunction = "&&";
}
String result = null;
for (PsiExpression expression : exp.getOperands()) {
String lhsText = convertLeafExpression(expression);
result = result == null ? lhsText : result + flippedConjunction + lhsText;
}
return result;
return result.toString();
}
private static String convertLeafExpression(PsiExpression condition) {
if (BoolUtils.isNegation(condition)) {
final PsiExpression negated = BoolUtils.getNegated(condition);
if (negated == null) {
private static String convertLeafExpression(PsiExpression expression, boolean tokenTypeAndAnd) {
if (BoolUtils.isNegation(expression)) {
final PsiExpression negatedExpression = BoolUtils.getNegated(expression);
if (negatedExpression == null) {
return "";
}
if (ParenthesesUtils.getPrecedence(negated) >
ParenthesesUtils.OR_PRECEDENCE) {
return '(' + negated.getText() + ')';
if (tokenTypeAndAnd) {
if (ParenthesesUtils.getPrecedence(negatedExpression) > ParenthesesUtils.OR_PRECEDENCE) {
return '(' + negatedExpression.getText() + ')';
}
} else if (ParenthesesUtils.getPrecedence(negatedExpression) > ParenthesesUtils.AND_PRECEDENCE) {
return '(' + negatedExpression.getText() + ')';
}
final PsiElement conditionParent = condition.getParent();
if (conditionParent instanceof PsiExpression &&
ParenthesesUtils.getPrecedence(negated) > ParenthesesUtils.AND_PRECEDENCE &&
ParenthesesUtils.getPrecedence((PsiExpression)conditionParent) > ParenthesesUtils.AND_PRECEDENCE) {
return '(' + negated.getText() + ')';
}
return negated.getText();
return negatedExpression.getText();
}
else if (ComparisonUtils.isComparison(condition)) {
final PsiBinaryExpression binaryExpression =
(PsiBinaryExpression)condition;
final String negatedComparison =
ComparisonUtils.getNegatedComparison(binaryExpression.getOperationTokenType());
else if (ComparisonUtils.isComparison(expression)) {
final PsiBinaryExpression binaryExpression = (PsiBinaryExpression)expression;
final String negatedComparison = ComparisonUtils.getNegatedComparison(binaryExpression.getOperationTokenType());
final PsiExpression lhs = binaryExpression.getLOperand();
final PsiExpression rhs = binaryExpression.getROperand();
assert rhs != null;
return lhs.getText() + negatedComparison + rhs.getText();
}
else if (ParenthesesUtils.getPrecedence(condition) >
ParenthesesUtils.PREFIX_PRECEDENCE) {
return "!(" + condition.getText() + ')';
else if (ParenthesesUtils.getPrecedence(expression) > ParenthesesUtils.PREFIX_PRECEDENCE) {
return "!(" + expression.getText() + ')';
}
else {
return '!' + condition.getText();
return '!' + expression.getText();
}
}
private static boolean isConjunctionExpression(PsiElement exp,
IElementType conjunctionType) {
if (!(exp instanceof PsiPolyadicExpression)) {
return false;
}
final PsiPolyadicExpression binExp = (PsiPolyadicExpression)exp;
final IElementType tokenType = binExp.getOperationTokenType();
return tokenType.equals(conjunctionType);
}
}
@@ -1,5 +1,5 @@
/*
* Copyright 2003-2011 Dave Griffith, Bas Leijdekkers
* Copyright 2003-2012 Dave Griffith, Bas Leijdekkers
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -22,17 +22,17 @@ import org.jetbrains.annotations.Nullable;
public class BoolUtils {
private BoolUtils() {
}
private BoolUtils() {}
public static boolean isNegated(PsiExpression exp) {
PsiExpression ancestor = exp;
while (ancestor.getParent() instanceof PsiParenthesizedExpression) {
ancestor = (PsiExpression)ancestor.getParent();
PsiElement parent = ancestor.getParent();
while (parent instanceof PsiParenthesizedExpression) {
ancestor = (PsiExpression)parent;
parent = ancestor.getParent();
}
if (ancestor.getParent() instanceof PsiPrefixExpression) {
final PsiPrefixExpression prefixAncestor =
(PsiPrefixExpression)ancestor.getParent();
if (parent instanceof PsiPrefixExpression) {
final PsiPrefixExpression prefixAncestor = (PsiPrefixExpression)parent;
final IElementType tokenType = prefixAncestor.getOperationTokenType();
if (tokenType.equals(JavaTokenType.EXCL)) {
return true;
@@ -42,14 +42,15 @@ public class BoolUtils {
}
@Nullable
public static PsiExpression findNegation(PsiExpression exp) {
PsiExpression ancestor = exp;
while (ancestor.getParent() instanceof PsiParenthesizedExpression) {
ancestor = (PsiExpression)ancestor.getParent();
public static PsiExpression findNegation(PsiExpression expression) {
PsiExpression ancestor = expression;
PsiElement parent = ancestor.getParent();
while (parent instanceof PsiParenthesizedExpression) {
ancestor = (PsiExpression)parent;
parent = ancestor.getParent();
}
if (ancestor.getParent() instanceof PsiPrefixExpression) {
final PsiPrefixExpression prefixAncestor =
(PsiPrefixExpression)ancestor.getParent();
if (parent instanceof PsiPrefixExpression) {
final PsiPrefixExpression prefixAncestor = (PsiPrefixExpression)parent;
if (JavaTokenType.EXCL.equals(prefixAncestor.getOperationTokenType())) {
return prefixAncestor;
}
@@ -67,12 +68,16 @@ public class BoolUtils {
}
@Nullable
public static PsiExpression getNegated(PsiExpression exp) {
final PsiPrefixExpression prefixExp = (PsiPrefixExpression)exp;
final PsiExpression operand = prefixExp.getOperand();
if (operand == null) {
public static PsiExpression getNegated(PsiExpression expression) {
if (!(expression instanceof PsiPrefixExpression)) {
return null;
}
final PsiPrefixExpression prefixExpression = (PsiPrefixExpression)expression;
final IElementType tokenType = prefixExpression.getOperationTokenType();
if (!JavaTokenType.EXCL.equals(tokenType)) {
return null;
}
final PsiExpression operand = prefixExpression.getOperand();
return ParenthesesUtils.stripParentheses(operand);
}
@@ -80,23 +85,18 @@ public class BoolUtils {
if (!(expression instanceof PsiLiteralExpression)) {
return false;
}
final PsiLiteralExpression literalExpression =
(PsiLiteralExpression)expression;
final PsiLiteralExpression literalExpression = (PsiLiteralExpression)expression;
@NonNls final String text = literalExpression.getText();
return PsiKeyword.TRUE.equals(text) ||
PsiKeyword.FALSE.equals(text);
return PsiKeyword.TRUE.equals(text) || PsiKeyword.FALSE.equals(text);
}
public static String getNegatedExpressionText(
@Nullable PsiExpression condition) {
public static String getNegatedExpressionText(@Nullable PsiExpression condition) {
if (condition == null) {
return "";
}
if (condition instanceof PsiParenthesizedExpression) {
final PsiParenthesizedExpression parenthesizedExpression =
(PsiParenthesizedExpression)condition;
final PsiExpression expression =
parenthesizedExpression.getExpression();
final PsiParenthesizedExpression parenthesizedExpression = (PsiParenthesizedExpression)condition;
final PsiExpression expression = parenthesizedExpression.getExpression();
return '(' + getNegatedExpressionText(expression) + ')';
}
else if (isNegation(condition)) {
@@ -107,10 +107,8 @@ public class BoolUtils {
return negated.getText();
}
else if (ComparisonUtils.isComparison(condition)) {
final PsiBinaryExpression binaryExpression =
(PsiBinaryExpression)condition;
final String negatedComparison =
ComparisonUtils.getNegatedComparison(binaryExpression.getOperationTokenType());
final PsiBinaryExpression binaryExpression = (PsiBinaryExpression)condition;
final String negatedComparison = ComparisonUtils.getNegatedComparison(binaryExpression.getOperationTokenType());
final PsiExpression lhs = binaryExpression.getLOperand();
final PsiExpression rhs = binaryExpression.getROperand();
if (rhs == null) {
@@ -118,8 +116,7 @@ public class BoolUtils {
}
return lhs.getText() + negatedComparison + rhs.getText();
}
else if (ParenthesesUtils.getPrecedence(condition) >
ParenthesesUtils.PREFIX_PRECEDENCE) {
else if (ParenthesesUtils.getPrecedence(condition) > ParenthesesUtils.PREFIX_PRECEDENCE) {
return "!(" + condition.getText() + ')';
}
else {
@@ -0,0 +1,7 @@
package com.siyeh.ipp.bool.demorgans;
class NeedsMoreParentheses {
void foo(boolean a, boolean b, boolean c, boolean d) {
boolean f = !(!(a || b) |<caret>| !(c || d));
}
}
@@ -0,0 +1,7 @@
package com.siyeh.ipp.bool.demorgans;
class NeedsMoreParentheses {
void foo(boolean a, boolean b, boolean c, boolean d) {
boolean f = (a || b) && (c || d);
}
}
@@ -0,0 +1,8 @@
package com.siyeh.ipp.bool.demorgans;
class NeedsParentheses {
void foo(boolean a, boolean b) {
if (!(!a || !b) <caret>|| !(a || b)){}
}
}
@@ -0,0 +1,8 @@
package com.siyeh.ipp.bool.demorgans;
class NeedsParentheses {
void foo(boolean a, boolean b) {
if (!((!a || !b) && (a || b))){}
}
}
@@ -0,0 +1,7 @@
package com.siyeh.ipp.bool.demorgans;
class NotTooManyParentheses {
void foo(boolean a, boolean b, boolean c) {
if (a && (b ||<caret> c)) {}
}
}
@@ -0,0 +1,7 @@
package com.siyeh.ipp.bool.demorgans;
class NotTooManyParentheses {
void foo(boolean a, boolean b, boolean c) {
if (a && !(!b && !c)) {}
}
}
@@ -0,0 +1,35 @@
/*
* Copyright 2000-2012 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.ipp.bool;
import com.siyeh.IntentionPowerPackBundle;
import com.siyeh.ipp.IPPTestCase;
public class DemorgansIntentionTest extends IPPTestCase {
public void testNeedsParentheses() { doTest(); }
public void testNeedsMoreParentheses() { doTest(); }
public void testNotTooManyParentheses() { doTest(); }
@Override
protected String getIntentionName() {
return IntentionPowerPackBundle.message("demorgans.intention.name2");
}
@Override
protected String getRelativePath() {
return "bool/demorgans";
}
}