From 3521d3aedc7c58ea0febe9a7c51b56334647f98b Mon Sep 17 00:00:00 2001 From: peter Date: Wed, 6 Jun 2012 23:09:15 +0200 Subject: [PATCH] IDEA-87070 Simplify ternary operator intention --- .../after.groovy.template | 2 + .../before.groovy.template | 2 + .../description.html | 25 + plugins/groovy/src/META-INF/plugin.xml | 467 ++++++++++++------ .../GroovyIntentionsBundle.properties | 2 + .../SimplifyTernaryOperatorIntention.java | 138 ++++++ .../SimplifyTernaryOperatorTest.groovy | 63 +++ 7 files changed, 541 insertions(+), 158 deletions(-) create mode 100644 plugins/groovy/resources/intentionDescriptions/SimplifyTernaryOperatorIntention/after.groovy.template create mode 100644 plugins/groovy/resources/intentionDescriptions/SimplifyTernaryOperatorIntention/before.groovy.template create mode 100644 plugins/groovy/resources/intentionDescriptions/SimplifyTernaryOperatorIntention/description.html create mode 100644 plugins/groovy/src/org/jetbrains/plugins/groovy/intentions/control/SimplifyTernaryOperatorIntention.java create mode 100644 plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/SimplifyTernaryOperatorTest.groovy diff --git a/plugins/groovy/resources/intentionDescriptions/SimplifyTernaryOperatorIntention/after.groovy.template b/plugins/groovy/resources/intentionDescriptions/SimplifyTernaryOperatorIntention/after.groovy.template new file mode 100644 index 000000000000..36c157bfa9a2 --- /dev/null +++ b/plugins/groovy/resources/intentionDescriptions/SimplifyTernaryOperatorIntention/after.groovy.template @@ -0,0 +1,2 @@ +def x = a && b +def y = c || d diff --git a/plugins/groovy/resources/intentionDescriptions/SimplifyTernaryOperatorIntention/before.groovy.template b/plugins/groovy/resources/intentionDescriptions/SimplifyTernaryOperatorIntention/before.groovy.template new file mode 100644 index 000000000000..bff2013b1b57 --- /dev/null +++ b/plugins/groovy/resources/intentionDescriptions/SimplifyTernaryOperatorIntention/before.groovy.template @@ -0,0 +1,2 @@ +def x = a ? b : false +def y = c ? true : d diff --git a/plugins/groovy/resources/intentionDescriptions/SimplifyTernaryOperatorIntention/description.html b/plugins/groovy/resources/intentionDescriptions/SimplifyTernaryOperatorIntention/description.html new file mode 100644 index 000000000000..818797709d78 --- /dev/null +++ b/plugins/groovy/resources/intentionDescriptions/SimplifyTernaryOperatorIntention/description.html @@ -0,0 +1,25 @@ + + + + +This intention simplifies a conditional operator ?: when one of the branches is a constant true or false. +
+ So a ? b : false gets turned into a && b and a ? true : b gets turned into a || b.
+ While you would normally not write such code, you may end up with it as a result of other refactorings. + + + diff --git a/plugins/groovy/src/META-INF/plugin.xml b/plugins/groovy/src/META-INF/plugin.xml index 7799ebd9c2c7..13f1dd3e41b5 100644 --- a/plugins/groovy/src/META-INF/plugin.xml +++ b/plugins/groovy/src/META-INF/plugin.xml @@ -31,14 +31,17 @@ - + - + - + @@ -46,10 +49,12 @@ - + - + @@ -59,7 +64,8 @@ - + @@ -67,7 +73,7 @@ + implementation="org.jetbrains.plugins.groovy.annotator.DefaultGroovyFrameworkConfigNotification"/> @@ -92,11 +98,15 @@ - - - + + + - + @@ -133,16 +143,23 @@ - + - - + + - - + + - + @@ -153,9 +170,11 @@ - + - + @@ -170,9 +189,12 @@ - - - + + + @@ -197,7 +219,8 @@ - + - + - + - - - + + + - - - - + + + + - - + + + id="groovyReferenceListWeigher" order="before openedInEditor"/> @@ -287,7 +321,7 @@ - + @@ -319,7 +353,8 @@ - + @@ -335,7 +370,7 @@ - + @@ -395,9 +430,9 @@ - + - + @@ -409,160 +444,212 @@ groupName="Declaration redundancy" enabledByDefault="true" level="WARNING" implementationClass="org.jetbrains.plugins.groovy.codeInspection.GroovyUnusedDeclarationInspection"/> - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -769,6 +909,11 @@ intention.category.groovy/intention.category.control.flow org.jetbrains.plugins.groovy.intentions.control.ReplaceTernaryWithIfElseIntention + + org.jetbrains.plugins.groovy.intentions.GroovyIntentionsBundle + intention.category.groovy/intention.category.control.flow + org.jetbrains.plugins.groovy.intentions.control.SimplifyTernaryOperatorIntention + org.jetbrains.plugins.groovy.intentions.GroovyIntentionsBundle intention.category.groovy/intention.category.control.flow @@ -882,9 +1027,9 @@ org.jetbrains.plugins.groovy.intentions.conversions.ConvertIntegerToOctalIntention - org.jetbrains.plugins.groovy.intentions.GroovyIntentionsBundle - intention.category.groovy/intention.category.conversions - org.jetbrains.plugins.groovy.intentions.conversions.ConvertIntegerToBinaryIntention + org.jetbrains.plugins.groovy.intentions.GroovyIntentionsBundle + intention.category.groovy/intention.category.conversions + org.jetbrains.plugins.groovy.intentions.conversions.ConvertIntegerToBinaryIntention org.jetbrains.plugins.groovy.intentions.GroovyIntentionsBundle @@ -1041,10 +1186,12 @@ serviceImplementation="org.jetbrains.plugins.groovy.mvc.MvcConsole"/> + serviceInterface="org.jetbrains.plugins.groovy.mvc.MvcRunTargetHistoryService"/> - - + + @@ -1052,7 +1199,8 @@ - + @@ -1074,20 +1222,22 @@ - - + + - + + internal="true"/> + internal="true"/> + text="Synchronize Griffon settings" + description="Refresh IntelliJ IDEA project structure so that it matches Griffon build settings"> aaa || bbb + GrExpression conditionExp = condExp.getCondition(); + + String conditionExpText = getStringToPutIntoOrExpression(conditionExp); + String elseExpText = getStringToPutIntoOrExpression(elseBranch); + String newExp = conditionExpText + "||" + elseExpText; + int caretOffset = conditionExpText.length() + 2; // after "||" + + GrExpression expressionFromText = groovyPsiElementFactory.createExpressionFromText(newExp, condExp.getContext()); + + expressionFromText = (GrExpression)condExp.replace(expressionFromText); + + editor.getCaretModel().moveToOffset(expressionFromText.getTextOffset() + caretOffset); // just past "||" + return; + } + + Object elseVal = GroovyConstantExpressionEvaluator.evaluate(elseBranch); + if (Boolean.FALSE.equals(elseVal) && thenBranch != null) { + // aaa ? bbb : false -> aaa && bbb + GrExpression conditionExp = condExp.getCondition(); + + String conditionExpText = getStringToPutIntoAndExpression(conditionExp); + String thenExpText = getStringToPutIntoAndExpression(thenBranch); + + + String newExp = conditionExpText + "&&" + thenExpText; + int caretOffset = conditionExpText.length() + 2; // after "&&" + GrExpression expressionFromText = groovyPsiElementFactory.createExpressionFromText(newExp, condExp.getContext()); + + expressionFromText = (GrExpression)condExp.replace(expressionFromText); + + editor.getCaretModel().moveToOffset(expressionFromText.getTextOffset() + caretOffset); // just past "&&" + } + + } + + /** + * Convert an expression into something which can be put inside ( a && b ) + * Wrap in parenthesis, if necessary + * + * @param expression + * @return a string representing the expression + */ + @NotNull + private static String getStringToPutIntoAndExpression(GrExpression expression) { + String expressionText = expression.getText(); + if (ParenthesesUtils.AND_PRECEDENCE < ParenthesesUtils.getPrecendence(expression)) { + expressionText = "(" + expressionText + ")"; + } + return expressionText; + } + + @NotNull + private static String getStringToPutIntoOrExpression(GrExpression expression) { + String expressionText = expression.getText(); + if (ParenthesesUtils.OR_PRECEDENCE < ParenthesesUtils.getPrecendence(expression)) { + expressionText = "(" + expressionText + ")"; + } + return expressionText; + } + + @NotNull + @Override + protected PsiElementPredicate getElementPredicate() { + return new PsiElementPredicate() { + @Override + public boolean satisfiedBy(PsiElement element) { + if (!(element instanceof GrConditionalExpression)) { + return false; + } + + GrConditionalExpression condExp = (GrConditionalExpression)element; + GrExpression thenBranch = condExp.getThenBranch(); + GrExpression elseBranch = condExp.getElseBranch(); + + Object thenVal = GroovyConstantExpressionEvaluator.evaluate(thenBranch); + if (Boolean.TRUE.equals(thenVal) && elseBranch != null) { + return true; + } + + Object elseVal = GroovyConstantExpressionEvaluator.evaluate(elseBranch); + if (thenBranch != null && Boolean.FALSE.equals(elseVal)) { + return true; + } + + return false; + } + }; + } +} diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/SimplifyTernaryOperatorTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/SimplifyTernaryOperatorTest.groovy new file mode 100644 index 000000000000..c288f57c3146 --- /dev/null +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/SimplifyTernaryOperatorTest.groovy @@ -0,0 +1,63 @@ +/* + * 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 org.jetbrains.plugins.groovy.intentions + +/** + * @author Niels Harremoes + * @author Oscar Toernroth + */ +class SimplifyTernaryOperatorTest extends GrIntentionTestCase { + + String intentionName = GroovyIntentionsBundle.message("simplify.ternary.operator.intention.name") + + private void doTest(String before, String after) { + + doTextTest before, intentionName, after + } + + public void testDoNotTriggerOnNormalConditional() throws Exception { + doAntiTest 'aaa ? bbb : ccc', intentionName + doAntiTest 'aaa ? false : ccc', intentionName + doAntiTest 'aaa ? bbb : true', intentionName + + } + + + public void testSimplifyWhenThenIsTrue() throws Exception { + doTest 'aaa ? true: bbb', 'aaa || bbb' + } + + + public void testSimplifyWhenThenIsTrueComplexArguments() throws Exception { + doTest 'aaa ? true : bbb ? ccc : ddd', 'aaa || (bbb ? ccc : ddd)' + } + + public void testSimplifyWhenElseIsFalseComplexArguments() throws Exception { + doTest 'aaa || bbb ? ccc || ddd : false', '(aaa || bbb) && (ccc || ddd)' + } + + public void testSimplifyWhenElseIsFalse() throws Exception { + doTest 'aaa ? bbb : false', 'aaa && bbb' + } + + // aaa ? false : bbb -> (!aaa) && bbb + + public void testResultisWrappedInParenthesesWhenNeeded() throws Exception { + doTest 'a ? b ? true : d : false', 'a && (b ? true : d)' + } + + +}