From 1638590aadd091a998b70c2a9dfea46ac16ef47e Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Fri, 13 Jan 2017 13:18:42 +0700 Subject: [PATCH] ExpressionUtils#getQualifierOrThis; ForCanBeForeachInspection & WhileCanBeForeachInspection fixed to handle unqualified calls in nested classes; cosmetics; tests for ForCanBeForeachInspection quick fix --- .../forCanBeForEach/afterForOuterClass.java | 15 +++ .../afterForOuterClassIterator.java | 15 +++ .../forCanBeForEach/afterForThisClass.java | 10 ++ .../forCanBeForEach/beforeForOuterClass.java | 15 +++ .../beforeForOuterClassIterator.java | 15 +++ .../forCanBeForEach/beforeForThisClass.java | 10 ++ .../siyeh/ig/fixes/AddThisQualifierFix.java | 40 +------ .../ForCanBeForeachInspectionBase.java | 56 ++++----- .../WhileCanBeForeachInspectionBase.java | 51 ++------ .../siyeh/ig/psiutils/ExpressionUtils.java | 38 +++++- .../siyeh/ig/psiutils/ParenthesesUtils.java | 3 +- .../migration/ForCanBeForeachInspection.java | 109 ++---------------- .../WhileCanBeForeachInspection.java | 32 ++--- .../ForCanBeForeachInspectionFixTest.java | 35 ++++++ 14 files changed, 206 insertions(+), 238 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/forCanBeForEach/afterForOuterClass.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/forCanBeForEach/afterForOuterClassIterator.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/forCanBeForEach/afterForThisClass.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/forCanBeForEach/beforeForOuterClass.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/forCanBeForEach/beforeForOuterClassIterator.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/forCanBeForEach/beforeForThisClass.java create mode 100644 plugins/InspectionGadgets/testsrc/com/siyeh/ig/migration/ForCanBeForeachInspectionFixTest.java diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/forCanBeForEach/afterForOuterClass.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/forCanBeForEach/afterForOuterClass.java new file mode 100644 index 000000000000..efa968405de3 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/forCanBeForEach/afterForOuterClass.java @@ -0,0 +1,15 @@ +// "Replace with 'foreach'" "true" +import java.util.*; + +public class Test extends ArrayList { + public void print() { + new Runnable() { + @Override + public void run() { + for (String s : Test.this) { + System.out.println(s); + } + } + }; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/forCanBeForEach/afterForOuterClassIterator.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/forCanBeForEach/afterForOuterClassIterator.java new file mode 100644 index 000000000000..efa968405de3 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/forCanBeForEach/afterForOuterClassIterator.java @@ -0,0 +1,15 @@ +// "Replace with 'foreach'" "true" +import java.util.*; + +public class Test extends ArrayList { + public void print() { + new Runnable() { + @Override + public void run() { + for (String s : Test.this) { + System.out.println(s); + } + } + }; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/forCanBeForEach/afterForThisClass.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/forCanBeForEach/afterForThisClass.java new file mode 100644 index 000000000000..224dddf6edb1 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/forCanBeForEach/afterForThisClass.java @@ -0,0 +1,10 @@ +// "Replace with 'foreach'" "true" +import java.util.*; + +public class Test extends ArrayList { + public void print() { + for (String s : this) { + System.out.println(s); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/forCanBeForEach/beforeForOuterClass.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/forCanBeForEach/beforeForOuterClass.java new file mode 100644 index 000000000000..c0e66865c819 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/forCanBeForEach/beforeForOuterClass.java @@ -0,0 +1,15 @@ +// "Replace with 'foreach'" "true" +import java.util.*; + +public class Test extends ArrayList { + public void print() { + new Runnable() { + @Override + public void run() { + for (int i = 0; i < size(); i++) { + System.out.println(get(i)); + } + } + }; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/forCanBeForEach/beforeForOuterClassIterator.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/forCanBeForEach/beforeForOuterClassIterator.java new file mode 100644 index 000000000000..912fcadcc8d5 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/forCanBeForEach/beforeForOuterClassIterator.java @@ -0,0 +1,15 @@ +// "Replace with 'foreach'" "true" +import java.util.*; + +public class Test extends ArrayList { + public void print() { + new Runnable() { + @Override + public void run() { + for (Iterator it = iterator(); it.hasNext(); ) { + System.out.println(it.next()); + } + } + }; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/forCanBeForEach/beforeForThisClass.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/forCanBeForEach/beforeForThisClass.java new file mode 100644 index 000000000000..f2d81bf12076 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/forCanBeForEach/beforeForThisClass.java @@ -0,0 +1,10 @@ +// "Replace with 'foreach'" "true" +import java.util.*; + +public class Test extends ArrayList { + public void print() { + for (int i = 0; i < size(); i++) { + System.out.println(get(i)); + } + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/fixes/AddThisQualifierFix.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/fixes/AddThisQualifierFix.java index f3fbd591fef0..e55dfc3afaac 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/fixes/AddThisQualifierFix.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/fixes/AddThisQualifierFix.java @@ -17,16 +17,13 @@ package com.siyeh.ig.fixes; import com.intellij.codeInspection.ProblemDescriptor; import com.intellij.openapi.project.Project; -import com.intellij.psi.PsiClass; -import com.intellij.psi.PsiElement; -import com.intellij.psi.PsiMember; +import com.intellij.psi.PsiExpression; import com.intellij.psi.PsiReferenceExpression; -import com.intellij.psi.util.InheritanceUtil; import com.intellij.util.IncorrectOperationException; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.InspectionGadgetsFix; import com.siyeh.ig.PsiReplacementUtil; -import com.siyeh.ig.psiutils.ClassUtils; +import com.siyeh.ig.psiutils.ExpressionUtils; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; @@ -44,37 +41,8 @@ public class AddThisQualifierFix extends InspectionGadgetsFix { if (expression.getQualifierExpression() != null) { return; } - final PsiElement target = expression.resolve(); - if (!(target instanceof PsiMember)) { - return; - } - final PsiMember member = (PsiMember)target; - final PsiClass memberClass = member.getContainingClass(); - if (memberClass == null) { - return; - } - PsiClass containingClass = ClassUtils.getContainingClass(expression); - @NonNls final String newExpression; - if (InheritanceUtil.isInheritorOrSelf(containingClass, memberClass, true)) { - newExpression = "this." + expression.getText(); - } - else { - containingClass = ClassUtils.getContainingClass(containingClass); - if (containingClass == null) { - return; - } - while (!InheritanceUtil.isInheritorOrSelf(containingClass, memberClass, true)) { - containingClass = ClassUtils.getContainingClass(containingClass); - if (containingClass == null) { - return; - } - } - final String qualifiedName = containingClass.getQualifiedName(); - if (qualifiedName == null) { - return; - } - newExpression = qualifiedName + ".this." + expression.getText(); - } + final PsiExpression thisQualifier = ExpressionUtils.getQualifierOrThis(expression); + @NonNls final String newExpression = thisQualifier.getText() + "." + expression.getText(); PsiReplacementUtil.replaceExpressionAndShorten(expression, newExpression); } } diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/ForCanBeForeachInspectionBase.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/ForCanBeForeachInspectionBase.java index 8158a57b2386..48c994ccd518 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/ForCanBeForeachInspectionBase.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/ForCanBeForeachInspectionBase.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2016 JetBrains s.r.o. + * Copyright 2000-2017 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. @@ -24,7 +24,10 @@ import com.siyeh.HardcodedMethodConstants; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; -import com.siyeh.ig.psiutils.*; +import com.siyeh.ig.psiutils.ExpressionUtils; +import com.siyeh.ig.psiutils.ParenthesesUtils; +import com.siyeh.ig.psiutils.TypeUtils; +import com.siyeh.ig.psiutils.VariableAccessUtils; import org.intellij.lang.annotations.Pattern; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; @@ -215,39 +218,22 @@ public class ForCanBeForeachInspectionBase extends BaseInspection { if (arguments.length != 0) { return false; } - final PsiExpression qualifier = initialMethodExpression.getQualifierExpression(); - final PsiClass qualifierClass; - if (qualifier == null) { - qualifierClass = ClassUtils.getContainingClass(initialMethodExpression); - if (ignoreUntypedCollections) { - final PsiClassType type = (PsiClassType)variable.getType(); - final PsiType[] parameters = type.getParameters(); - if (parameters.length == 0) { - return false; - } - } - } - else { - final PsiType qualifierType = qualifier.getType(); - if (!(qualifierType instanceof PsiClassType)) { - return false; - } - final PsiClassType classType = (PsiClassType)qualifierType; - qualifierClass = classType.resolve(); - if (ignoreUntypedCollections) { - final PsiClassType type = (PsiClassType)variable.getType(); - final PsiType[] parameters = type.getParameters(); - final PsiType[] parameters1 = classType.getParameters(); - if (parameters.length == 0 && parameters1.length == 0) { - return false; - } - } - } - if (qualifierClass == null) { + final PsiExpression qualifier = ExpressionUtils.getQualifierOrThis(initialMethodExpression); + final PsiType qualifierType = qualifier.getType(); + if (!(qualifierType instanceof PsiClassType)) { return false; } - if (!InheritanceUtil.isInheritor(qualifierClass, CommonClassNames.JAVA_LANG_ITERABLE) && - !InheritanceUtil.isInheritor(qualifierClass, CommonClassNames.JAVA_UTIL_COLLECTION)) { + final PsiClassType classType = (PsiClassType)qualifierType; + final PsiClass qualifierClass = classType.resolve(); + if (ignoreUntypedCollections) { + final PsiClassType type = (PsiClassType)variable.getType(); + final PsiType[] parameters = type.getParameters(); + final PsiType[] parameters1 = classType.getParameters(); + if (parameters.length == 0 && parameters1.length == 0) { + return false; + } + } + if (!InheritanceUtil.isInheritor(qualifierClass, CommonClassNames.JAVA_LANG_ITERABLE)) { return false; } final PsiExpression condition = forStatement.getCondition(); @@ -289,8 +275,8 @@ public class ForCanBeForeachInspectionBase extends BaseInspection { return visitor.isMethodCalled(); } - private static boolean isHasNext(PsiExpression condition, - PsiVariable iterator) { + static boolean isHasNext(PsiExpression condition, + PsiVariable iterator) { if (!(condition instanceof PsiMethodCallExpression)) { return false; } diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/WhileCanBeForeachInspectionBase.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/WhileCanBeForeachInspectionBase.java index 771419f79891..6df76a97cfff 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/WhileCanBeForeachInspectionBase.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/WhileCanBeForeachInspectionBase.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2015 JetBrains s.r.o. + * Copyright 2000-2017 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. @@ -23,8 +23,10 @@ import com.siyeh.HardcodedMethodConstants; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; +import com.siyeh.ig.psiutils.ExpressionUtils; import com.siyeh.ig.psiutils.TypeUtils; import com.siyeh.ig.psiutils.VariableAccessUtils; +import org.intellij.lang.annotations.Pattern; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -39,6 +41,7 @@ public class WhileCanBeForeachInspectionBase extends BaseInspection { return (PsiStatement)prevStatement; } + @Pattern(VALID_ID_PATTERN) @Override @NotNull public String getID() { @@ -118,29 +121,20 @@ public class WhileCanBeForeachInspectionBase extends BaseInspection { if (!"iterator".equals(initialCallName) && !"listIterator".equals(initialCallName)) { return false; } - final PsiExpression qualifier = initialMethodExpression.getQualifierExpression(); + final PsiExpression qualifier = ExpressionUtils.getQualifierOrThis(initialMethodExpression); if (qualifier instanceof PsiSuperExpression) { return false; } - final PsiClass qualifierClass; - if (qualifier != null) { - final PsiType qualifierType = qualifier.getType(); - if (!(qualifierType instanceof PsiClassType)) { - return false; - } - qualifierClass = ((PsiClassType)qualifierType).resolve(); - } - else { - qualifierClass = PsiTreeUtil.getParentOfType(whileStatement, PsiClass.class); - } - if (qualifierClass == null) { + final PsiType qualifierType = qualifier.getType(); + if (!(qualifierType instanceof PsiClassType)) { return false; } + final PsiClass qualifierClass = ((PsiClassType)qualifierType).resolve(); if (!InheritanceUtil.isInheritor(qualifierClass, CommonClassNames.JAVA_LANG_ITERABLE)) { return false; } final PsiExpression condition = whileStatement.getCondition(); - if (!isHasNextCalled(variable, condition)) { + if (!ForCanBeForeachInspectionBase.isHasNext(condition, variable)) { return false; } final PsiStatement body = whileStatement.getBody(); @@ -173,33 +167,6 @@ public class WhileCanBeForeachInspectionBase extends BaseInspection { return true; } - private static boolean isHasNextCalled(PsiVariable iterator, PsiExpression condition) { - if (!(condition instanceof PsiMethodCallExpression)) { - return false; - } - final PsiMethodCallExpression call = (PsiMethodCallExpression)condition; - final PsiExpressionList argumentList = call.getArgumentList(); - final PsiExpression[] arguments = argumentList.getExpressions(); - if (arguments.length != 0) { - return false; - } - final PsiReferenceExpression methodExpression = call.getMethodExpression(); - @NonNls final String methodName = methodExpression.getReferenceName(); - if (!HardcodedMethodConstants.HAS_NEXT.equals(methodName)) { - return false; - } - final PsiExpression qualifier = methodExpression.getQualifierExpression(); - if (qualifier == null) { - return true; - } - if (!(qualifier instanceof PsiReferenceExpression)) { - return false; - } - final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)qualifier; - final PsiElement target = referenceExpression.resolve(); - return iterator.equals(target); - } - private static int calculateCallsToIteratorNext(PsiVariable iterator, PsiElement context) { final NumberCallsToIteratorNextVisitor visitor = new NumberCallsToIteratorNextVisitor(iterator); context.accept(visitor); diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java index e8cf91d9b707..45c3985a115f 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java @@ -20,11 +20,10 @@ import com.intellij.codeInsight.NullableNotNullManager; import com.intellij.openapi.project.Project; import com.intellij.psi.*; import com.intellij.psi.tree.IElementType; -import com.intellij.psi.util.ConstantExpressionUtil; -import com.intellij.psi.util.PsiTreeUtil; -import com.intellij.psi.util.PsiUtil; -import com.intellij.psi.util.TypeConversionUtil; +import com.intellij.psi.util.*; +import com.intellij.psi.util.InheritanceUtil; import com.intellij.util.ArrayUtil; +import com.intellij.util.ObjectUtils; import com.siyeh.HardcodedMethodConstants; import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NonNls; @@ -885,4 +884,35 @@ public class ExpressionUtils { if (type == null || type.getArrayDimensions() <= 0) return null; return qualifier; } + + /** + * Returns a qualifier for reference or creates a corresponding {@link PsiThisExpression} statement if + * a qualifier is null + * + * @param ref a reference expression to get a qualifier from + * @return a qualifier or created (non-physical) {@link PsiThisExpression}. + */ + @NotNull + public static PsiExpression getQualifierOrThis(@NotNull PsiReferenceExpression ref) { + PsiExpression qualifier = ref.getQualifierExpression(); + if (qualifier != null) return qualifier; + PsiElementFactory factory = JavaPsiFacade.getElementFactory(ref.getProject()); + PsiMember member = ObjectUtils.tryCast(ref.resolve(), PsiMember.class); + if (member != null) { + PsiClass memberClass = member.getContainingClass(); + if (memberClass != null) { + PsiClass containingClass = ClassUtils.getContainingClass(ref); + if (!InheritanceUtil.isInheritorOrSelf(containingClass, memberClass, true)) { + containingClass = ClassUtils.getContainingClass(containingClass); + while (containingClass != null && !InheritanceUtil.isInheritorOrSelf(containingClass, memberClass, true)) { + containingClass = ClassUtils.getContainingClass(containingClass); + } + if (containingClass != null) { + return factory.createExpressionFromText(containingClass.getQualifiedName() + "." + PsiKeyword.THIS, ref); + } + } + } + } + return factory.createExpressionFromText(PsiKeyword.THIS, ref); + } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ParenthesesUtils.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ParenthesesUtils.java index 89412e503bcf..f41db80c3050 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ParenthesesUtils.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ParenthesesUtils.java @@ -18,6 +18,7 @@ package com.siyeh.ig.psiutils; import com.intellij.psi.*; import com.intellij.psi.tree.IElementType; import com.intellij.psi.util.PsiTreeUtil; +import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -104,7 +105,7 @@ public class ParenthesesUtils { return parent; } - @Nullable + @Contract("null -> null") public static PsiExpression stripParentheses(@Nullable PsiExpression expression) { while (expression instanceof PsiParenthesizedExpression) { final PsiParenthesizedExpression parenthesizedExpression = (PsiParenthesizedExpression)expression; diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/migration/ForCanBeForeachInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/migration/ForCanBeForeachInspection.java index 4e0056a16d0e..86a522be6df8 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/migration/ForCanBeForeachInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/migration/ForCanBeForeachInspection.java @@ -21,7 +21,6 @@ import com.intellij.openapi.project.Project; import com.intellij.psi.*; import com.intellij.psi.codeStyle.*; import com.intellij.psi.tree.IElementType; -import com.intellij.psi.util.PsiTreeUtil; import com.siyeh.HardcodedMethodConstants; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.InspectionGadgetsFix; @@ -133,7 +132,7 @@ public class ForCanBeForeachInspection extends ForCanBeForeachInspectionBase { return null; } final PsiReferenceExpression listLengthExpression = methodCallExpression.getMethodExpression(); - final PsiExpression qualifier = ParenthesesUtils.stripParentheses(listLengthExpression.getQualifierExpression()); + final PsiExpression qualifier = ParenthesesUtils.stripParentheses(ExpressionUtils.getQualifierOrThis(listLengthExpression)); final PsiReferenceExpression listReference; if (qualifier instanceof PsiReferenceExpression) { listReference = (PsiReferenceExpression)qualifier; @@ -142,16 +141,11 @@ public class ForCanBeForeachInspection extends ForCanBeForeachInspectionBase { listReference = null; } PsiType parameterType; - if (listReference == null) { - parameterType = extractListTypeFromContainingClass(forStatement); - } - else { - final PsiType type = listReference.getType(); - if (type == null) { - return null; - } - parameterType = extractContentTypeFromType(type); + final PsiType type = qualifier.getType(); + if (type == null) { + return null; } + parameterType = WhileCanBeForeachInspection.getContentType(type, CommonClassNames.JAVA_UTIL_COLLECTION); if (parameterType == null) { parameterType = TypeUtils.getObjectType(forStatement); } @@ -202,86 +196,14 @@ public class ForCanBeForeachInspection extends ForCanBeForeachInspectionBase { finalString = ""; statementToSkip = null; } - @NonNls final StringBuilder out = new StringBuilder("for("); - out.append(finalString).append(typeString).append(' ').append(contentVariableName).append(": "); - @NonNls final String listName; - if (listReference == null) { - listName = "this"; - } - else { - listName = getVariableReferenceText(listReference, listVariable, forStatement); - } - out.append(listName).append(')'); + @NonNls final StringBuilder out = new StringBuilder( + "for(" + finalString + typeString + ' ' + contentVariableName + ": " + qualifier.getText() + ')'); if (body != null) { replaceCollectionGetAccess(body, contentVariableName, listVariable, indexName, statementToSkip, out); } return out.toString(); } - @Nullable - private PsiType extractContentTypeFromType(PsiType collectionType) { - if (!(collectionType instanceof PsiClassType)) { - return null; - } - final PsiType[] parameterTypes = ((PsiClassType)collectionType).getParameters(); - if (parameterTypes.length == 0) { - return null; - } - return GenericsUtil.getVariableTypeByExpressionType(parameterTypes[0]); - } - - @Nullable - private PsiType extractListTypeFromContainingClass( - PsiElement element) { - PsiClass listClass = PsiTreeUtil.getParentOfType(element, - PsiClass.class); - if (listClass == null) { - return null; - } - final PsiMethod[] getMethods = - listClass.findMethodsByName("get", true); - if (getMethods.length == 0) { - return null; - } - final PsiType type = getMethods[0].getReturnType(); - if (!(type instanceof PsiClassType)) { - return null; - } - final PsiClassType classType = (PsiClassType)type; - final PsiClass parameterClass = classType.resolve(); - if (parameterClass == null) { - return null; - } - PsiClass subClass = null; - while (listClass != null && !listClass.hasTypeParameters()) { - subClass = listClass; - listClass = listClass.getSuperClass(); - } - if (listClass == null || subClass == null) { - return TypeUtils.getObjectType(element); - } - final PsiTypeParameter[] typeParameters = - listClass.getTypeParameters(); - if (!parameterClass.equals(typeParameters[0])) { - return TypeUtils.getObjectType(element); - } - final PsiReferenceList extendsList = subClass.getExtendsList(); - if (extendsList == null) { - return null; - } - final PsiJavaCodeReferenceElement[] referenceElements = - extendsList.getReferenceElements(); - if (referenceElements.length == 0) { - return null; - } - final PsiType[] types = - referenceElements[0].getTypeParameters(); - if (types.length == 0) { - return TypeUtils.getObjectType(element); - } - return types[0]; - } - @Nullable private String createCollectionIterationText(@NotNull PsiForStatement forStatement) { final PsiStatement body = forStatement.getBody(); @@ -308,14 +230,13 @@ public class ForCanBeForeachInspection extends ForCanBeForeachInspectionBase { if (iteratorType == null) { return null; } - final PsiType iteratorContentType = - extractContentTypeFromType(iteratorType); + final PsiType iteratorContentType = WhileCanBeForeachInspection.getContentType(iteratorType, CommonClassNames.JAVA_UTIL_ITERATOR); final PsiType iteratorVariableType = iteratorVariable.getType(); final PsiType contentType; final PsiClassType javaLangObject = TypeUtils.getObjectType(forStatement); if (iteratorContentType == null) { final PsiType iteratorVariableContentType = - extractContentTypeFromType(iteratorVariableType); + WhileCanBeForeachInspection.getContentType(iteratorVariableType, CommonClassNames.JAVA_UTIL_ITERATOR); if (iteratorVariableContentType == null) { contentType = javaLangObject; } @@ -328,8 +249,7 @@ public class ForCanBeForeachInspection extends ForCanBeForeachInspectionBase { } final PsiReferenceExpression methodExpression = initializer.getMethodExpression(); - final PsiExpression collection = - methodExpression.getQualifierExpression(); + final PsiExpression collection = ExpressionUtils.getQualifierOrThis(methodExpression); final boolean isDeclaration = isIteratorNextDeclaration(firstStatement, iteratorVariable, contentType); final PsiStatement statementToSkip; @NonNls final String finalString; @@ -388,12 +308,7 @@ public class ForCanBeForeachInspection extends ForCanBeForeachInspectionBase { if (!contentType.equals(javaLangObject) && iteratorContentType == null) { out.append('(').append("java.lang.Iterable<").append(contentTypeString).append('>').append(')'); } - if (collection == null) { - out.append("this"); - } - else { - out.append(collection.getText()); - } + out.append(collection.getText()); out.append(')'); replaceIteratorNext(body, contentVariableName, iteratorVariable, contentType, statementToSkip, out); return out.toString(); @@ -876,7 +791,7 @@ public class ForCanBeForeachInspection extends ForCanBeForeachInspectionBase { } @NotNull - private String getVariableReferenceText(PsiReferenceExpression reference, PsiVariable variable, PsiElement context) { + private static String getVariableReferenceText(PsiReferenceExpression reference, PsiVariable variable, PsiElement context) { final String text = reference.getText(); final PsiResolveHelper resolveHelper = PsiResolveHelper.SERVICE.getInstance(context.getProject()); final PsiVariable target = resolveHelper.resolveReferencedVariable(text, context); diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/migration/WhileCanBeForeachInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/migration/WhileCanBeForeachInspection.java index 71fc396a7603..8142da445115 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/migration/WhileCanBeForeachInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/migration/WhileCanBeForeachInspection.java @@ -27,6 +27,7 @@ import com.intellij.util.Query; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.InspectionGadgetsFix; import com.siyeh.ig.PsiReplacementUtil; +import com.siyeh.ig.psiutils.ExpressionUtils; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -73,19 +74,8 @@ public class WhileCanBeForeachInspection extends WhileCanBeForeachInspectionBase return; } final PsiReferenceExpression methodExpression = initializer.getMethodExpression(); - final PsiExpression collection = methodExpression.getQualifierExpression(); - final PsiType collectionType; - if (collection == null) { - final PsiClass aClass = PsiTreeUtil.getParentOfType(whileStatement, PsiClass.class); - if (aClass == null) { - return; - } - final PsiElementFactory factory = JavaPsiFacade.getElementFactory(whileStatement.getProject()); - collectionType = factory.createType(aClass); - } - else { - collectionType = collection.getType(); - } + final PsiExpression collection = ExpressionUtils.getQualifierOrThis(methodExpression); + final PsiType collectionType = collection.getType(); if (collectionType == null) { return; } @@ -130,11 +120,7 @@ public class WhileCanBeForeachInspection extends WhileCanBeForeachInspectionBase if (!TypeConversionUtil.isAssignable(iteratorContentType, contentType)) { out.append("(java.lang.Iterable<").append(iteratorContentType.getCanonicalText()).append(">)"); } - if (collection == null) { - out.append("this"); - } else { - out.append(collection.getText()); - } + out.append(collection.getText()); out.append(')'); ForCanBeForeachInspection.replaceIteratorNext(body, contentVariableName, iterator, contentType, statementToSkip, out); @@ -169,11 +155,11 @@ public class WhileCanBeForeachInspection extends WhileCanBeForeachInspectionBase final String result = out.toString(); PsiReplacementUtil.replaceStatementAndShortenClassNames(whileStatement, result); } + } - @Nullable - private static PsiType getContentType(PsiType type, String containerClassName) { - PsiType parameterType = PsiUtil.substituteTypeParameter(type, containerClassName, 0, true); - return GenericsUtil.getVariableTypeByExpressionType(parameterType); - } + @Nullable + static PsiType getContentType(PsiType type, String containerClassName) { + PsiType parameterType = PsiUtil.substituteTypeParameter(type, containerClassName, 0, true); + return GenericsUtil.getVariableTypeByExpressionType(parameterType); } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/migration/ForCanBeForeachInspectionFixTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/migration/ForCanBeForeachInspectionFixTest.java new file mode 100644 index 000000000000..944cc280bcaf --- /dev/null +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/migration/ForCanBeForeachInspectionFixTest.java @@ -0,0 +1,35 @@ +/* + * Copyright 2000-2017 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.ig.migration; + +import com.intellij.codeInsight.daemon.quickFix.LightQuickFixParameterizedTestCase; +import com.intellij.codeInspection.LocalInspectionTool; +import org.jetbrains.annotations.NotNull; + +public class ForCanBeForeachInspectionFixTest extends LightQuickFixParameterizedTestCase { + @NotNull + @Override + protected LocalInspectionTool[] configureLocalInspectionTools() { + return new LocalInspectionTool[]{new ForCanBeForeachInspection()}; + } + + public void test() throws Exception { doAllTests(); } + + @Override + protected String getBasePath() { + return "/codeInsight/daemonCodeAnalyzer/quickFix/forCanBeForEach"; + } +} \ No newline at end of file