From f21f77bb58aff23cb23013102aa7a132d9844985 Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Sat, 30 Aug 2014 11:37:57 +0200 Subject: [PATCH] Make "Create JUnit assertion" intention applicable in more cases and test --- .../siyeh/IntentionPowerPackBundle.properties | 2 +- .../ipp/junit/CreateAssertIntention.java | 133 +++++++++--------- .../ipp/junit/CreateAssertPredicate.java | 59 ++------ .../create_assert/AnonymousClassJUnit3.java | 12 ++ .../AnonymousClassJUnit3_after.java | 12 ++ .../create_assert/AnonymousClassJUnit4.java | 11 ++ .../AnonymousClassJUnit4_after.java | 13 ++ .../ipp/junit/create_assert/AssertFalse.java | 12 ++ .../create_assert/AssertFalse_after.java | 12 ++ .../create_assert/StaticImportJUnit4.java | 9 ++ .../StaticImportJUnit4_after.java | 10 ++ .../ipp/junit/CreateAssertIntentionTest.java | 60 ++++++++ 12 files changed, 227 insertions(+), 118 deletions(-) create mode 100644 plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/AnonymousClassJUnit3.java create mode 100644 plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/AnonymousClassJUnit3_after.java create mode 100644 plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/AnonymousClassJUnit4.java create mode 100644 plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/AnonymousClassJUnit4_after.java create mode 100644 plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/AssertFalse.java create mode 100644 plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/AssertFalse_after.java create mode 100644 plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/StaticImportJUnit4.java create mode 100644 plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/StaticImportJUnit4_after.java create mode 100644 plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/junit/CreateAssertIntentionTest.java diff --git a/plugins/IntentionPowerPak/src/com/siyeh/IntentionPowerPackBundle.properties b/plugins/IntentionPowerPak/src/com/siyeh/IntentionPowerPackBundle.properties index 30e64929f8b2..952afa890e96 100644 --- a/plugins/IntentionPowerPak/src/com/siyeh/IntentionPowerPackBundle.properties +++ b/plugins/IntentionPowerPak/src/com/siyeh/IntentionPowerPackBundle.properties @@ -40,7 +40,7 @@ replace.switch.with.if.intention.family.name=Replace Switch with If simplify.variable.intention.name=Replace with Java-style array declaration simplify.variable.intention.family.name=Replace with Java Style Array Declaration constant.expression.intention.family.name=Compute Constant Value -create.assert.intention.name=Create JUnit Assertion +create.assert.intention.name=Create JUnit assertion create.assert.intention.family.name=Create JUnit Assertion simplify.if.else.intention.name=Simplify 'if else' simplify.if.else.intention.family.name=Simplify If Else diff --git a/plugins/IntentionPowerPak/src/com/siyeh/ipp/junit/CreateAssertIntention.java b/plugins/IntentionPowerPak/src/com/siyeh/ipp/junit/CreateAssertIntention.java index 55ae45e3f3bf..4be5442d03c1 100644 --- a/plugins/IntentionPowerPak/src/com/siyeh/ipp/junit/CreateAssertIntention.java +++ b/plugins/IntentionPowerPak/src/com/siyeh/ipp/junit/CreateAssertIntention.java @@ -1,5 +1,5 @@ /* - * Copyright 2003-2006 Dave Griffith, Bas Leijdekkers + * Copyright 2003-2014 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. @@ -16,16 +16,15 @@ package com.siyeh.ipp.junit; import com.intellij.psi.*; -import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.tree.IElementType; -import com.intellij.util.IncorrectOperationException; -import com.intellij.codeInsight.AnnotationUtil; import com.siyeh.ig.PsiReplacementUtil; +import com.siyeh.ig.psiutils.ExpressionUtils; +import com.siyeh.ig.psiutils.ImportUtils; import com.siyeh.ipp.base.Intention; import com.siyeh.ipp.base.PsiElementPredicate; import com.siyeh.ipp.psiutils.BoolUtils; -import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.NonNls; +import org.jetbrains.annotations.NotNull; public class CreateAssertIntention extends Intention { @@ -34,28 +33,12 @@ public class CreateAssertIntention extends Intention { return new CreateAssertPredicate(); } - public void processIntention(PsiElement element) - throws IncorrectOperationException { - final PsiExpressionStatement statement = - (PsiExpressionStatement)element; - assert statement != null; + public void processIntention(PsiElement element) { + final PsiExpressionStatement statement = (PsiExpressionStatement)element; final PsiExpression expression = statement.getExpression(); - final PsiMethod containingMethod = - PsiTreeUtil.getParentOfType(statement, PsiMethod.class); - final String specifierString; - if (containingMethod != null && - AnnotationUtil.isAnnotated(containingMethod, - "org.junit.Test", true)) { - specifierString = "org.junit.Assert."; - } - else { - specifierString = ""; - } + final String newStatement; if (BoolUtils.isNegation(expression)) { - @NonNls final String newExpression = - specifierString + "assertFalse(" + - BoolUtils.getNegatedExpressionText(expression) + ");"; - PsiReplacementUtil.replaceStatementAndShortenClassNames(statement, newExpression); + newStatement = buildNewStatement("assertFalse", element, BoolUtils.getNegatedExpressionText(expression)); } else if (isNullComparison(expression)) { final PsiBinaryExpression binaryExpression = @@ -63,16 +46,19 @@ public class CreateAssertIntention extends Intention { final PsiExpression lhs = binaryExpression.getLOperand(); final PsiExpression rhs = binaryExpression.getROperand(); final PsiExpression comparedExpression; - if (isNull(lhs)) { + if (ExpressionUtils.isNullLiteral(lhs)) { comparedExpression = rhs; } else { comparedExpression = lhs; } assert comparedExpression != null; - @NonNls final String newExpression = specifierString + - "assertNull(" + comparedExpression.getText() + ");"; - PsiReplacementUtil.replaceStatementAndShortenClassNames(statement, newExpression); + if (JavaTokenType.EQEQ.equals(binaryExpression.getOperationTokenType())) { + newStatement = buildNewStatement("assertNull", element, comparedExpression.getText()); + } + else { + newStatement = buildNewStatement("assertNotNull", element, comparedExpression.getText()); + } } else if (isEqualityComparison(expression)) { final PsiBinaryExpression binaryExpression = @@ -91,23 +77,16 @@ public class CreateAssertIntention extends Intention { } assert comparingExpression != null; final PsiType type = lhs.getType(); - @NonNls final String newExpression; if (PsiType.DOUBLE.equals(type) || PsiType.FLOAT.equals(type)) { - newExpression = specifierString + "assertEquals(" + - comparedExpression.getText() + ", " + - comparingExpression.getText() + ", 0.0);"; + newStatement = buildNewStatement("assertEquals", + element, comparedExpression.getText(), comparingExpression.getText(), "0.0"); } else if (type instanceof PsiPrimitiveType) { - newExpression = specifierString + "assertEquals(" + - comparedExpression.getText() + ", " + - comparingExpression.getText() + ");"; + newStatement = buildNewStatement("assertEquals", element, comparedExpression.getText(), comparingExpression.getText()); } else { - newExpression = specifierString + "assertSame(" + - comparedExpression.getText() + ", " + - comparingExpression.getText() + ");"; + newStatement = buildNewStatement("assertSame", element, comparedExpression.getText(), comparingExpression.getText()); } - PsiReplacementUtil.replaceStatementAndShortenClassNames(statement, newExpression); } else if (isEqualsExpression(expression)) { final PsiMethodCallExpression call = @@ -118,26 +97,55 @@ public class CreateAssertIntention extends Intention { methodExpression.getQualifierExpression(); assert comparedExpression != null; final PsiExpressionList argList = call.getArgumentList(); - final PsiExpression comparingExpression = - argList.getExpressions()[0]; - @NonNls final String newExpression; + final PsiExpression comparingExpression = argList.getExpressions()[0]; if (comparingExpression instanceof PsiLiteralExpression) { - newExpression = specifierString + "assertEquals(" + - comparingExpression.getText() + ", " + - comparedExpression.getText() + ");"; + newStatement = buildNewStatement("assertEquals", element, comparingExpression.getText(), comparedExpression.getText()); } else { - newExpression = specifierString + "assertEquals(" + - comparedExpression.getText() + ", " + - comparingExpression.getText() + ");"; + newStatement = buildNewStatement("assertEquals", element, comparedExpression.getText(), comparingExpression.getText()); } - PsiReplacementUtil.replaceStatementAndShortenClassNames(statement, newExpression); } else { - @NonNls final String newExpression = - specifierString + "assertTrue(" + expression.getText() + ");"; - PsiReplacementUtil.replaceStatementAndShortenClassNames(statement, newExpression); + newStatement = buildNewStatement("assertTrue", element, expression.getText()); } + PsiReplacementUtil.replaceStatementAndShortenClassNames(statement, newStatement); + } + + @NonNls + private static String buildNewStatement(@NonNls String memberName, PsiElement context, String... argumentTexts) { + final PsiElementFactory factory = JavaPsiFacade.getElementFactory(context.getProject()); + final StringBuilder builder = new StringBuilder(memberName).append('('); + boolean comma = false; + for (String argumentText : argumentTexts) { + if (comma) { + builder.append(','); + } + else { + comma = true; + } + builder.append(argumentText); + } + builder.append(')'); + final String text = builder.toString(); + + final PsiMethodCallExpression methodCallExpression = (PsiMethodCallExpression)factory.createExpressionFromText(text, context); + final PsiMethod method = methodCallExpression.resolveMethod(); + if (method != null || hasStaticImports(context) && ImportUtils.addStaticImport("org.junit.Assert", memberName, context)) { + return text + ';'; + } + else { + return "org.junit.Assert." + text + ';'; + } + } + + private static boolean hasStaticImports(PsiElement element) { + final PsiFile file = element.getContainingFile(); + if (!(file instanceof PsiJavaFile)) { + return false; + } + final PsiJavaFile javaFile = (PsiJavaFile)file; + final PsiImportList importList = javaFile.getImportList(); + return importList != null && importList.getImportStaticStatements().length > 0; } private static boolean isEqualsExpression(PsiExpression expression) { @@ -176,25 +184,16 @@ public class CreateAssertIntention extends Intention { if (!(expression instanceof PsiBinaryExpression)) { return false; } - final PsiBinaryExpression binaryExpression = - (PsiBinaryExpression)expression; + final PsiBinaryExpression binaryExpression = (PsiBinaryExpression)expression; final IElementType tokenType = binaryExpression.getOperationTokenType(); - if (!JavaTokenType.EQEQ.equals(tokenType)) { + if (!JavaTokenType.EQEQ.equals(tokenType) && !JavaTokenType.NE.equals(tokenType)) { return false; } final PsiExpression lhs = binaryExpression.getLOperand(); - if (isNull(lhs)) { + if (ExpressionUtils.isNullLiteral(lhs)) { return true; } - final PsiExpression Rhs = binaryExpression.getROperand(); - return isNull(Rhs); - } - - private static boolean isNull(PsiExpression expression) { - if (!(expression instanceof PsiLiteralExpression)) { - return false; - } - @NonNls final String text = expression.getText(); - return PsiKeyword.NULL.equals(text); + final PsiExpression rhs = binaryExpression.getROperand(); + return ExpressionUtils.isNullLiteral(rhs); } } diff --git a/plugins/IntentionPowerPak/src/com/siyeh/ipp/junit/CreateAssertPredicate.java b/plugins/IntentionPowerPak/src/com/siyeh/ipp/junit/CreateAssertPredicate.java index 27f09f9c3b80..c21bb76aa1bc 100644 --- a/plugins/IntentionPowerPak/src/com/siyeh/ipp/junit/CreateAssertPredicate.java +++ b/plugins/IntentionPowerPak/src/com/siyeh/ipp/junit/CreateAssertPredicate.java @@ -1,5 +1,5 @@ /* - * Copyright 2003-2009 Dave Griffith, Bas Leijdekkers + * Copyright 2003-2014 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. @@ -15,14 +15,10 @@ */ package com.siyeh.ipp.junit; -import com.intellij.codeInsight.AnnotationUtil; -import com.intellij.openapi.project.Project; import com.intellij.psi.*; -import com.intellij.psi.search.GlobalSearchScope; -import com.intellij.psi.util.InheritanceUtil; import com.intellij.psi.util.PsiTreeUtil; +import com.siyeh.ig.psiutils.TestUtils; import com.siyeh.ipp.base.PsiElementPredicate; -import org.jetbrains.annotations.NonNls; class CreateAssertPredicate implements PsiElementPredicate { @@ -41,50 +37,13 @@ class CreateAssertPredicate implements PsiElementPredicate { if (!PsiType.BOOLEAN.equals(type)) { return false; } - final PsiMethod containingMethod = - PsiTreeUtil.getParentOfType(expression, PsiMethod.class); - return isTestMethod(containingMethod); - } - - private static boolean isTestMethod(PsiMethod method) { - if (method == null) { - return false; + PsiMethod containingMethod = PsiTreeUtil.getParentOfType(expression, PsiMethod.class); + while (containingMethod != null) { + if (TestUtils.isJUnitTestMethod(containingMethod)) { + return true; + } + containingMethod = PsiTreeUtil.getParentOfType(containingMethod, PsiMethod.class); } - if (AnnotationUtil.isAnnotated(method, "org.junit.Test", true)) { - return true; - } - if (method.hasModifierProperty(PsiModifier.ABSTRACT) || - !method.hasModifierProperty(PsiModifier.PUBLIC)) { - return false; - } - final PsiType returnType = method.getReturnType(); - if (returnType == null) { - return false; - } - if (!returnType.equals(PsiType.VOID)) { - return false; - } - final PsiParameterList parameterList = method.getParameterList(); - final PsiParameter[] parameters = parameterList.getParameters(); - if (parameters.length != 0) { - return false; - } - @NonNls final String methodName = method.getName(); - if (!methodName.startsWith("test")) { - return false; - } - final PsiClass containingClass = method.getContainingClass(); - return isTestClass(containingClass); - } - - private static boolean isTestClass(PsiClass aClass) { - if (aClass == null) { - return false; - } - final Project project = aClass.getProject(); - final GlobalSearchScope scope = GlobalSearchScope.allScope(project); - final JavaPsiFacade psiFacade = JavaPsiFacade.getInstance(project); - final PsiClass ancestorClass = psiFacade.findClass("junit.framework.TestCase", scope); - return InheritanceUtil.isInheritorOrSelf(aClass, ancestorClass, true); + return false; } } diff --git a/plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/AnonymousClassJUnit3.java b/plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/AnonymousClassJUnit3.java new file mode 100644 index 000000000000..d6c02fcfbe66 --- /dev/null +++ b/plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/AnonymousClassJUnit3.java @@ -0,0 +1,12 @@ +import junit.framework.TestCase; + +public class AnonymousClassJUnit3 extends TestCase { + + public void test2BiggerThan1() { + new Object() { + void foo() { + 2 > 1 + } + }; + } +} \ No newline at end of file diff --git a/plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/AnonymousClassJUnit3_after.java b/plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/AnonymousClassJUnit3_after.java new file mode 100644 index 000000000000..9821c9431820 --- /dev/null +++ b/plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/AnonymousClassJUnit3_after.java @@ -0,0 +1,12 @@ +import junit.framework.TestCase; + +public class AnonymousClassJUnit3 extends TestCase { + + public void test2BiggerThan1() { + new Object() { + void foo() { + assertTrue(2 > 1); + } + }; + } +} \ No newline at end of file diff --git a/plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/AnonymousClassJUnit4.java b/plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/AnonymousClassJUnit4.java new file mode 100644 index 000000000000..e7b11eef3a87 --- /dev/null +++ b/plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/AnonymousClassJUnit4.java @@ -0,0 +1,11 @@ +public class AnonymousClassJUnit4 { + + @org.junit.Test + public void test2BiggerThan1() { + new Object() { + void foo() { + 2 > 1 + } + } + } +} \ No newline at end of file diff --git a/plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/AnonymousClassJUnit4_after.java b/plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/AnonymousClassJUnit4_after.java new file mode 100644 index 000000000000..787d0e6d6115 --- /dev/null +++ b/plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/AnonymousClassJUnit4_after.java @@ -0,0 +1,13 @@ +import org.junit.Assert; + +public class AnonymousClassJUnit4 { + + @org.junit.Test + public void test2BiggerThan1() { + new Object() { + void foo() { + Assert.assertTrue(2 > 1); + } + } + } +} \ No newline at end of file diff --git a/plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/AssertFalse.java b/plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/AssertFalse.java new file mode 100644 index 000000000000..76bddf465b6f --- /dev/null +++ b/plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/AssertFalse.java @@ -0,0 +1,12 @@ +import junit.framework.TestCase; + +public class AssertFalse extends TestCase { + + public void testOne() { + !result() + } + + boolean result() { + return false; + } +} \ No newline at end of file diff --git a/plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/AssertFalse_after.java b/plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/AssertFalse_after.java new file mode 100644 index 000000000000..6c1ad31f57a6 --- /dev/null +++ b/plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/AssertFalse_after.java @@ -0,0 +1,12 @@ +import junit.framework.TestCase; + +public class AssertFalse extends TestCase { + + public void testOne() { + assertFalse(result()); + } + + boolean result() { + return false; + } +} \ No newline at end of file diff --git a/plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/StaticImportJUnit4.java b/plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/StaticImportJUnit4.java new file mode 100644 index 000000000000..24abc14797f4 --- /dev/null +++ b/plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/StaticImportJUnit4.java @@ -0,0 +1,9 @@ +import static java.util.Collections.EMPTY_LIST; + +public class AnonymousClassJUnit4 { + + @org.junit.Test + public void testNotNull() { + EMPTY_LIST != null + } +} \ No newline at end of file diff --git a/plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/StaticImportJUnit4_after.java b/plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/StaticImportJUnit4_after.java new file mode 100644 index 000000000000..792a4f778816 --- /dev/null +++ b/plugins/IntentionPowerPak/test/com/siyeh/ipp/junit/create_assert/StaticImportJUnit4_after.java @@ -0,0 +1,10 @@ +import static java.util.Collections.EMPTY_LIST; +import static org.junit.Assert.assertNotNull; + +public class AnonymousClassJUnit4 { + + @org.junit.Test + public void testNotNull() { + assertNotNull(EMPTY_LIST); + } +} \ No newline at end of file diff --git a/plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/junit/CreateAssertIntentionTest.java b/plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/junit/CreateAssertIntentionTest.java new file mode 100644 index 000000000000..b67a451ce947 --- /dev/null +++ b/plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/junit/CreateAssertIntentionTest.java @@ -0,0 +1,60 @@ +/* + * Copyright 2000-2014 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.junit; + +import com.siyeh.IntentionPowerPackBundle; +import com.siyeh.ipp.IPPTestCase; + +/** + * @see CreateAssertIntention + * @author Bas Leijdekkers + */ +public class CreateAssertIntentionTest extends IPPTestCase { + + public void testAnonymousClassJUnit3() { doTest(); } + public void testAnonymousClassJUnit4() { doTest(); } + public void testStaticImportJUnit4() { doTest(); } + public void testAssertFalse() { doTest(); } + + @Override + protected String getRelativePath() { + return "junit/create_assert"; + } + + @Override + protected String getIntentionName() { + return IntentionPowerPackBundle.message("create.assert.intention.name"); + } + + @Override + protected void setUp() throws Exception { + super.setUp(); + myFixture.addClass("package org.junit;" + + "class Assert {" + + " public static void assertTrue(java.lang.String message, boolean condition) {}" + + " public static void assertNotNull(boolean condition) {}" + + "}"); + myFixture.addClass("package org.junit;" + + "@Retention(RetentionPolicy.RUNTIME)" + + "@Target({ElementType.METHOD})" + + "public @interface Test {}"); + myFixture.addClass("package junit.framework;" + + "public abstract class TestCase {" + + " static public void assertTrue(boolean condition) {}" + + " static public void assertFalse(boolean condition) {}" + ); + } +}