[groovy] 'change to operator' inspection updates (IDEA-95818):

- inline getMessage() (one-line method);
- inline registerFIxIfValid() (small one-usage method);
- get rid of unused `ReplacementData.getExpressionText()`;
- rename bundle properties from 'convert.to...' to 'change.to...';
- introduce separate property for highlight message  and include expression text in message;
- set default severity to 'Weak Warning', do not force 'Weak Warning';
- make test extend `LightGroovyTestCase`.
This commit is contained in:
Daniil Ovchinnikov
2016-03-28 14:08:28 +03:00
parent 4fbe817c82
commit 5153f72ee6
5 changed files with 30 additions and 39 deletions

View File

@@ -46,9 +46,10 @@ parameter.can.be.final.tooltip=Parameter ''{0}'' can be final
groovy.probable.bugs=Probable bugs
equals.between.inconvertible.types='equals()' between objects of inconvertible types
equals.between.inconvertible.types.tooltip=<code>{0}</code> between objects of inconvertible types ''{1}'' and ''{2}''
convert.to.operator=Change to operator
convert.to.operator.double.negation.option=Use double negation (i.e. !!)
convert.to.operator.compareto.equality.option=Change compareTo equality to equals (i.e. ==)
change.to.operator.message=''{0}'' can be changed to operator
change.to.operator.fix=Change ''{0}'' to operator
change.to.operator.double.negation.option=Use double negation (i.e. !!)
change.to.operator.compareTo.equality.option=Change compareTo equality to equals (i.e. ==)
unassigned.access=Variable Not Assigned
unassigned.access.short.name=VariableNotAssigned
unassigned.access.tooltip=Variable ''{0}'' might not be assigned

View File

@@ -23,7 +23,6 @@ import com.intellij.psi.PsiMethod;
import com.intellij.psi.PsiModifier;
import com.intellij.psi.PsiModifierListOwner;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.jetbrains.plugins.groovy.codeInspection.BaseInspection;
import org.jetbrains.plugins.groovy.codeInspection.BaseInspectionVisitor;
import org.jetbrains.plugins.groovy.codeInspection.GroovyFix;
@@ -37,7 +36,7 @@ import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.path.GrM
import javax.swing.*;
import java.util.Map;
import static com.intellij.codeInspection.ProblemHighlightType.WEAK_WARNING;
import static com.intellij.codeInspection.ProblemHighlightType.GENERIC_ERROR_OR_WARNING;
import static org.jetbrains.plugins.groovy.codeInspection.GroovyInspectionBundle.message;
public class ChangeToOperatorInspection extends BaseInspection {
@@ -65,23 +64,20 @@ public class ChangeToOperatorInspection extends BaseInspection {
if (transformation == null) return;
// TODO apply transformation recursively
ReplacementData replacement = transformation.transform(methodCallExpression,
new OptionsData(useDoubleNegation, shouldChangeCompareToEqualityToEquals));
registerFixIfValid(replacement);
OptionsData optionsData = new OptionsData(useDoubleNegation, shouldChangeCompareToEqualityToEquals);
ReplacementData replacement = transformation.transform(methodCallExpression, optionsData);
if (replacement == null) return;
GrExpression expression = replacement.getExpression();
String text = expression.getText();
GroovyFix fix = getFix(message("change.to.operator.fix", text), replacement.getReplacement());
registerError(expression, message("change.to.operator.message", text), new LocalQuickFix[]{fix}, GENERIC_ERROR_OR_WARNING);
}
private boolean isValid(PsiModifierListOwner method) {
return ((method != null) && !method.hasModifierProperty(PsiModifier.STATIC));
}
private void registerFixIfValid(@Nullable ReplacementData replacement) {
if (replacement == null) return;
String message = getMessage();
LocalQuickFix[] quickFixes = {getFix(message, replacement.getReplacement())};
registerError(replacement.getExpression(), message, quickFixes, WEAK_WARNING);
}
private GroovyFix getFix(@NotNull final String message, final String replacement) {
return new GroovyFix() {
@NotNull
@@ -99,15 +95,11 @@ public class ChangeToOperatorInspection extends BaseInspection {
};
}
public String getMessage() {
return message("convert.to.operator");
}
@Override
public JComponent createOptionsPanel() {
MultipleCheckboxOptionsPanel optionsPanel = new MultipleCheckboxOptionsPanel(this);
optionsPanel.addCheckbox(message("convert.to.operator.double.negation.option"), "useDoubleNegation");
optionsPanel.addCheckbox(message("convert.to.operator.compareto.equality.option"), "shouldChangeCompareToEqualityToEquals");
optionsPanel.addCheckbox(message("change.to.operator.double.negation.option"), "useDoubleNegation");
optionsPanel.addCheckbox(message("change.to.operator.compareTo.equality.option"), "shouldChangeCompareToEqualityToEquals");
return optionsPanel;
}
}

View File

@@ -31,10 +31,6 @@ public final class ReplacementData {
return element;
}
public String getExpressionText() {
return element.getText();
}
public String getReplacement() {
return replacement;
}

View File

@@ -801,7 +801,7 @@
implementationClass="org.jetbrains.plugins.groovy.codeInspection.style.JavaStylePropertiesInvocationInspection"/>
<localInspection language="Groovy" groupPath="Groovy" shortName="ChangeToOperator"
displayName="Change to operator"
groupName="Style" enabledByDefault="true" level="WARNING"
groupName="Style" enabledByDefault="true" level="WEAK WARNING"
implementationClass="org.jetbrains.plugins.groovy.codeInspection.changeToOperator.ChangeToOperatorInspection"/>
<localInspection language="Groovy" groupPath="Groovy" shortName="GroovyAccessToStaticFieldLockedOnInstance"
displayName="Access to static field locked on instance data"

View File

@@ -15,8 +15,10 @@
*/
package org.jetbrains.plugins.groovy.inspections
import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase
import com.intellij.testFramework.LightProjectDescriptor
import org.intellij.lang.annotations.Language
import org.jetbrains.plugins.groovy.GroovyLightProjectDescriptor
import org.jetbrains.plugins.groovy.LightGroovyTestCase
import org.jetbrains.plugins.groovy.codeInspection.changeToOperator.ChangeToOperatorInspection
import java.util.regex.Pattern
@@ -24,7 +26,11 @@ import java.util.regex.Pattern
import static org.jetbrains.plugins.groovy.GroovyFileType.GROOVY_FILE_TYPE
import static org.jetbrains.plugins.groovy.util.TestUtils.CARET_MARKER
public class GrChangeToOperatorTest extends LightCodeInsightFixtureTestCase {
public class GrChangeToOperatorTest extends LightGroovyTestCase {
final String basePath = null
final LightProjectDescriptor projectDescriptor = GroovyLightProjectDescriptor.GROOVY_2_3_9
def inspection = new ChangeToOperatorInspection()
void testSimpleUnaryExpression() {
@@ -44,8 +50,7 @@ public class GrChangeToOperatorTest extends LightCodeInsightFixtureTestCase {
assertValid(/if (${_}!a.asBoolean()${_});/, /!a/)
}
// TODO
void notYetTestable_testComplexNegatableUnaryExpression() {
void testComplexNegatableUnaryExpression() {
assertValid(/if (${_}'a'.intern().asBoolean()${_});/, /'a'.intern()/)
}
@@ -59,8 +64,7 @@ public class GrChangeToOperatorTest extends LightCodeInsightFixtureTestCase {
assertValid(/if (${_}!a.asBoolean()${_});/, /!a/)
}
// TODO
void notYetTestable_testComplexNegatedOption() {
void testComplexNegatedOption() {
inspection.useDoubleNegation = false
assertValid(/if (${_}'a'.intern().asBoolean()${_});/, /'a'.intern()/)
@@ -89,10 +93,9 @@ public class GrChangeToOperatorTest extends LightCodeInsightFixtureTestCase {
assertValid(/${_}a.xor((a.b+1) == b)${_} == a/, /(a ^ ((a.b + 1) == b))/)
}
// TODO
void notYetTestable_testComplexBinaryExpression() {
void testComplexBinaryExpression() {
assertValid(/b.isCase(a)/, /a in b/)
assertValid(/if (${_}[1,2,3].isCase(2-1)${_});/, /(2 - 1) in [1,2,3]/)
assertValid(/if (${_}[1, 2, 3].isCase(2-1)${_});/, /(2 - 1) in [1, 2, 3]/)
assertValid(/def x = ${_}"1".plus(1)${_}/, /"1" + 1/)
assertValid(/("1" + 1).plus(1)/, /("1" + 1) + 1/)
assertValid(/!a.toString().asBoolean()/, /!a.toString()/)
@@ -103,8 +106,7 @@ public class GrChangeToOperatorTest extends LightCodeInsightFixtureTestCase {
assertValid(/!a.equals(b)/, /a != b/)
}
// TODO
void notYetTestable_testComplexNegatableBinaryExpression() {
void testComplexNegatableBinaryExpression() {
assertValid(/!(1.toString().replace('1', '2')+"").equals(2.toString())/, /(1.toString().replace('1', '2') + "") != 2.toString()/)
}
@@ -171,7 +173,7 @@ public class GrChangeToOperatorTest extends LightCodeInsightFixtureTestCase {
private void assertValid(@Language('Groovy') String text, @Language('Groovy') String methodReplacement) {
def (prefix, method, suffix) = getMessage(text)
configure("${prefix}${CARET_MARKER}${method}${suffix}")
myFixture.launchAction(myFixture.findSingleIntention(inspection.getMessage()))
myFixture.launchAction(myFixture.findSingleIntention("Change '"))
myFixture.checkResult("${DECLARATIONS} ${prefix}${methodReplacement}${suffix}")
}