diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/migration/ForCanBeForeachInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/migration/ForCanBeForeachInspection.java index b484244e62ab..c130cc858206 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/migration/ForCanBeForeachInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/migration/ForCanBeForeachInspection.java @@ -498,18 +498,33 @@ public class ForCanBeForeachInspection extends BaseInspection { if (tokenType.equals(JavaTokenType.LT)) { arrayLengthExpression = (PsiReferenceExpression)ParenthesesUtils.stripParentheses(rhs); indexName = lhs.getText(); - } else if (tokenType.equals(JavaTokenType.GT)) { + } + else if (tokenType.equals(JavaTokenType.GT)) { arrayLengthExpression = (PsiReferenceExpression)ParenthesesUtils.stripParentheses(lhs); indexName = rhs.getText(); - } else { + } + else { return null; } if (arrayLengthExpression == null) { return null; } - final PsiReferenceExpression arrayReference = (PsiReferenceExpression)arrayLengthExpression.getQualifierExpression(); + PsiReferenceExpression arrayReference = (PsiReferenceExpression)arrayLengthExpression.getQualifierExpression(); if (arrayReference == null) { - return null; + final PsiElement target = arrayLengthExpression.resolve(); + if (!(target instanceof PsiVariable)) { + return null; + } + final PsiVariable variable = (PsiVariable)target; + final PsiExpression initializer = variable.getInitializer(); + if (!(initializer instanceof PsiReferenceExpression)) { + return null; + } + final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)initializer; + arrayReference = (PsiReferenceExpression)referenceExpression.getQualifierExpression(); + if (arrayReference == null) { + return null; + } } final PsiArrayType arrayType = (PsiArrayType)arrayReference.getType(); if (arrayType == null) { @@ -524,33 +539,24 @@ public class ForCanBeForeachInspection extends BaseInspection { final PsiVariable arrayVariable = (PsiVariable)target; final PsiStatement body = forStatement.getBody(); final PsiStatement firstStatement = getFirstStatement(body); - final boolean isDeclaration = - isArrayElementDeclaration(firstStatement, arrayVariable, - indexName); + final boolean isDeclaration = isArrayElementDeclaration(firstStatement, arrayVariable, indexName); final String contentVariableName; @NonNls final String finalString; final PsiStatement statementToSkip; if (isDeclaration) { - final PsiDeclarationStatement declarationStatement = - (PsiDeclarationStatement)firstStatement; + final PsiDeclarationStatement declarationStatement = (PsiDeclarationStatement)firstStatement; assert declarationStatement != null; - final PsiElement[] declaredElements = - declarationStatement.getDeclaredElements(); + final PsiElement[] declaredElements = declarationStatement.getDeclaredElements(); final PsiElement declaredElement = declaredElements[0]; if (!(declaredElement instanceof PsiVariable)) { return null; } - final PsiVariable variable = - (PsiVariable)declaredElement; - if (VariableAccessUtils.variableIsAssigned(variable, - forStatement)) { - final String collectionName = - arrayReference.getReferenceName(); - contentVariableName = createNewVariableName(forStatement, - componentType, collectionName); + final PsiVariable variable = (PsiVariable)declaredElement; + if (VariableAccessUtils.variableIsAssigned(variable, forStatement)) { + final String collectionName = arrayReference.getReferenceName(); + contentVariableName = createNewVariableName(forStatement, componentType, collectionName); final Project project = forStatement.getProject(); - final CodeStyleSettings codeStyleSettings = - CodeStyleSettingsManager.getSettings(project); + final CodeStyleSettings codeStyleSettings = CodeStyleSettingsManager.getSettings(project); if (codeStyleSettings.GENERATE_FINAL_LOCALS) { finalString = "final "; } @@ -571,13 +577,10 @@ public class ForCanBeForeachInspection extends BaseInspection { } } else { - final String collectionName = - arrayReference.getReferenceName(); - contentVariableName = createNewVariableName(forStatement, - componentType, collectionName); + final String collectionName = arrayReference.getReferenceName(); + contentVariableName = createNewVariableName(forStatement, componentType, collectionName); final Project project = forStatement.getProject(); - final CodeStyleSettings codeStyleSettings = - CodeStyleSettingsManager.getSettings(project); + final CodeStyleSettings codeStyleSettings = CodeStyleSettingsManager.getSettings(project); if (codeStyleSettings.GENERATE_FINAL_LOCALS) { finalString = "final "; } @@ -597,8 +600,7 @@ public class ForCanBeForeachInspection extends BaseInspection { out.append(arrayName); out.append(')'); if (body != null) { - replaceArrayAccess(body, contentVariableName, arrayVariable, - indexName, statementToSkip, out); + replaceArrayAccess(body, contentVariableName, arrayVariable, indexName, statementToSkip, out); } return out.toString(); } @@ -991,7 +993,14 @@ public class ForCanBeForeachInspection extends BaseInspection { } final PsiDeclarationStatement declaration = (PsiDeclarationStatement)initialization; final PsiElement[] declaredElements = declaration.getDeclaredElements(); - if (declaredElements.length < 1) { + final PsiElement secondDeclaredElement; + if (declaredElements.length == 1) { + secondDeclaredElement = null; + } + else if (declaredElements.length == 2) { + secondDeclaredElement = declaredElements[1]; + } + else { return false; } final PsiElement declaredElement = declaredElements[0]; @@ -1012,7 +1021,7 @@ public class ForCanBeForeachInspection extends BaseInspection { return false; } final PsiExpression condition = forStatement.getCondition(); - final Holder collectionHolder = getCollectionFromSizeComparison(condition, indexVariable); + final Holder collectionHolder = getCollectionFromSizeComparison(condition, indexVariable, secondDeclaredElement); if (collectionHolder == null) { return false; } @@ -1041,10 +1050,16 @@ public class ForCanBeForeachInspection extends BaseInspection { if (!(initialization instanceof PsiDeclarationStatement)) { return false; } - final PsiDeclarationStatement declaration = - (PsiDeclarationStatement)initialization; + final PsiDeclarationStatement declaration = (PsiDeclarationStatement)initialization; final PsiElement[] declaredElements = declaration.getDeclaredElements(); - if (declaredElements.length != 1) { + final PsiElement secondDeclaredElement; + if (declaredElements.length == 1) { + secondDeclaredElement = null; + } + else if (declaredElements.length == 2) { + secondDeclaredElement = declaredElements[1]; + } + else { return false; } final PsiElement declaredElement = declaredElements[0]; @@ -1065,15 +1080,12 @@ public class ForCanBeForeachInspection extends BaseInspection { if (integer.intValue() != 0) { return false; } - //if (!isArrayLengthComparison(condition, indexVariable)) { - // return false; - //} final PsiStatement update = forStatement.getUpdate(); if (!VariableAccessUtils.variableIsIncremented(indexVariable, update)) { return false; } final PsiExpression condition = forStatement.getCondition(); - final PsiReferenceExpression arrayReference = getVariableReferenceFromCondition(condition, indexVariable); + final PsiReferenceExpression arrayReference = getVariableReferenceFromCondition(condition, indexVariable, secondDeclaredElement); if (arrayReference == null) { return false; } @@ -1083,17 +1095,10 @@ public class ForCanBeForeachInspection extends BaseInspection { } final PsiVariable arrayVariable = (PsiVariable)element; final PsiStatement body = forStatement.getBody(); - if (body == null) { - return true; - } - if (!isIndexVariableOnlyUsedAsIndex(arrayVariable, indexVariable, body)) { - return false; - } - if (VariableAccessUtils.variableIsAssigned(arrayVariable, body)) { - return false; - } - return !VariableAccessUtils.arrayContentsAreAssigned(arrayVariable, - body); + return body == null || + isIndexVariableOnlyUsedAsIndex(arrayVariable, indexVariable, body) && + !VariableAccessUtils.variableIsAssigned(arrayVariable, body) && + !VariableAccessUtils.arrayContentsAreAssigned(arrayVariable, body); } private static boolean isIndexVariableOnlyUsedAsIndex( @@ -1280,8 +1285,9 @@ public class ForCanBeForeachInspection extends BaseInspection { } @Nullable - private static PsiReferenceExpression getVariableReferenceFromCondition(PsiExpression condition, PsiVariable variable) { - System.out.println("ForCanBeForeachInspection.getVariableReferenceFromCondition(" + condition + ")"); + private static PsiReferenceExpression getVariableReferenceFromCondition(PsiExpression condition, + PsiVariable variable, + PsiElement secondDeclaredElement) { condition = ParenthesesUtils.stripParentheses(condition); if (!(condition instanceof PsiBinaryExpression)) { return null; @@ -1293,40 +1299,51 @@ public class ForCanBeForeachInspection extends BaseInspection { if (rhs == null) { return null; } - final PsiReferenceExpression referenceExpression; + PsiReferenceExpression referenceExpression; if (tokenType.equals(JavaTokenType.LT)) { - if (!VariableAccessUtils.evaluatesToVariable(lhs, variable) || !expressionIsArrayLengthLookup(rhs)) { - return null; - } - if (rhs instanceof PsiMethodCallExpression) { - final PsiMethodCallExpression expression = (PsiMethodCallExpression)rhs; - referenceExpression = expression.getMethodExpression(); - } - else if (rhs instanceof PsiReferenceExpression) { - referenceExpression = (PsiReferenceExpression)rhs; - } - else { + if (!VariableAccessUtils.evaluatesToVariable(lhs, variable) || !(rhs instanceof PsiReferenceExpression)) { return null; } + referenceExpression = (PsiReferenceExpression)rhs; } else if (tokenType.equals(JavaTokenType.GT)) { - if (!VariableAccessUtils.evaluatesToVariable(rhs, variable) || !expressionIsArrayLengthLookup(lhs)) { - return null; - } - if (lhs instanceof PsiMethodCallExpression) { - final PsiMethodCallExpression expression = (PsiMethodCallExpression)lhs; - referenceExpression = expression.getMethodExpression(); - } - else if (lhs instanceof PsiReferenceExpression) { - referenceExpression = (PsiReferenceExpression)lhs; - } - else { + if (!VariableAccessUtils.evaluatesToVariable(rhs, variable) || !(lhs instanceof PsiReferenceExpression)) { return null; } + referenceExpression = (PsiReferenceExpression)lhs; } else { return null; } + if (!expressionIsArrayLengthLookup(referenceExpression)) { + final PsiElement target = referenceExpression.resolve(); + if (secondDeclaredElement != null && !secondDeclaredElement.equals(target)) { + return null; + } + if (target instanceof PsiVariable) { + final PsiVariable maxVariable = (PsiVariable)target; + final PsiCodeBlock context = PsiTreeUtil.getParentOfType(maxVariable, PsiCodeBlock.class); + if (context == null) { + return null; + } + if (VariableAccessUtils.variableIsAssigned(maxVariable, context)) { + return null; + } + final PsiExpression expression = ParenthesesUtils.stripParentheses(maxVariable.getInitializer()); + if (!(expression instanceof PsiReferenceExpression)) { + return null; + } + referenceExpression = (PsiReferenceExpression)expression; + if (!expressionIsArrayLengthLookup(referenceExpression)) { + return null; + } + } + } + else { + if (secondDeclaredElement != null) { + return null; + } + } final PsiExpression qualifierExpression = referenceExpression.getQualifierExpression(); if (qualifierExpression instanceof PsiReferenceExpression) { return (PsiReferenceExpression)qualifierExpression; @@ -1341,27 +1358,8 @@ public class ForCanBeForeachInspection extends BaseInspection { } } - private static boolean isArrayLengthComparison(PsiExpression condition, PsiVariable variable) { - System.out.println("ForCanBeForeachInspection.isArrayLengthComparison(" + condition + ", " + variable + ")"); - condition = ParenthesesUtils.stripParentheses(condition); - if (!(condition instanceof PsiBinaryExpression)) { - return false; - } - final PsiBinaryExpression binaryExpression = (PsiBinaryExpression)condition; - final IElementType tokenType = binaryExpression.getOperationTokenType(); - final PsiExpression lhs = binaryExpression.getLOperand(); - final PsiExpression rhs = binaryExpression.getROperand(); - if (tokenType.equals(JavaTokenType.LT)) { - return VariableAccessUtils.evaluatesToVariable(lhs, variable) && rhs != null && expressionIsArrayLengthLookup(rhs); - } - else if (tokenType.equals(JavaTokenType.GT)) { - return VariableAccessUtils.evaluatesToVariable(rhs, variable) && expressionIsArrayLengthLookup(lhs); - } - return false; - } - - private static Holder getCollectionFromSizeComparison( - PsiExpression condition, PsiVariable variable) { + @Nullable + private static Holder getCollectionFromSizeComparison(PsiExpression condition, PsiVariable variable, PsiElement secondDeclaredElement) { condition = ParenthesesUtils.stripParentheses(condition); if (!(condition instanceof PsiBinaryExpression)) { return null; @@ -1374,13 +1372,13 @@ public class ForCanBeForeachInspection extends BaseInspection { if (!VariableAccessUtils.evaluatesToVariable(lhs, variable)) { return null; } - return getCollectionFromListMethodCall(rhs, HardcodedMethodConstants.SIZE); + return getCollectionFromListMethodCall(rhs, HardcodedMethodConstants.SIZE, secondDeclaredElement); } else if (tokenType.equals(JavaTokenType.GT)) { if (!VariableAccessUtils.evaluatesToVariable(rhs, variable)) { return null; } - return getCollectionFromListMethodCall(lhs, HardcodedMethodConstants.SIZE); + return getCollectionFromListMethodCall(lhs, HardcodedMethodConstants.SIZE, secondDeclaredElement); } return null; } @@ -1407,23 +1405,31 @@ public class ForCanBeForeachInspection extends BaseInspection { CommonClassNames.JAVA_UTIL_LIST); } - private static Holder getCollectionFromListMethodCall( - PsiExpression expression, String methodName) { + @Nullable + private static Holder getCollectionFromListMethodCall(PsiExpression expression, String methodName, PsiElement secondDeclaredElement) { expression = ParenthesesUtils.stripParentheses(expression); if (expression instanceof PsiReferenceExpression) { final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)expression; final PsiElement target = referenceExpression.resolve(); - if (target instanceof PsiVariable) { - final PsiVariable variable = (PsiVariable)target; - final PsiCodeBlock context = PsiTreeUtil.getParentOfType(variable, PsiCodeBlock.class); - if (context != null) { - if (!VariableAccessUtils.variableIsAssigned(variable, context)) { - expression = ParenthesesUtils.stripParentheses(variable.getInitializer()); - } - } + if (secondDeclaredElement != null && !secondDeclaredElement.equals(target)) { + return null; } + if (!(target instanceof PsiVariable)) { + return null; + } + final PsiVariable variable = (PsiVariable)target; + final PsiCodeBlock context = PsiTreeUtil.getParentOfType(variable, PsiCodeBlock.class); + if (context == null) { + return null; + } + if (VariableAccessUtils.variableIsAssigned(variable, context)) { + return null; + } + expression = ParenthesesUtils.stripParentheses(variable.getInitializer()); + } + else if (secondDeclaredElement != null) { + return null; } - if (!(expression instanceof PsiMethodCallExpression)) { return null; } @@ -1466,7 +1472,6 @@ public class ForCanBeForeachInspection extends BaseInspection { } private static boolean expressionIsArrayLengthLookup(PsiExpression expression) { - System.out.println("ForCanBeForeachInspection.expressionIsArrayLengthLookup(" + expression + ")"); expression = ParenthesesUtils.stripParentheses(expression); if (!(expression instanceof PsiReferenceExpression)) { return false; diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/migration/foreach/ForCanBeForEach.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/migration/foreach/ForCanBeForEach.java index 4e335443e285..ceb0034f6b37 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/migration/foreach/ForCanBeForEach.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/migration/foreach/ForCanBeForEach.java @@ -258,5 +258,13 @@ public class ForCanBeForEach { public void food(int[] is) { for (int i = 0; is.length > i; i++) { } + for (int i = 0, j = 10; i < is.length; i++) { + } + } + + void foo(List l) { + for (int i = 0, j = 10; i < l.size(); i++) { + System.out.println(j); + } } } diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/migration/foreach/expected.xml b/plugins/InspectionGadgets/test/com/siyeh/igtest/migration/foreach/expected.xml index ba0d20f5e815..c544dc2bbb1f 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/migration/foreach/expected.xml +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/migration/foreach/expected.xml @@ -106,6 +106,13 @@ <code>for</code> loop replaceable with 'for each' #loc + + ForCanBeForEach.java + 252 + 'for' loop replaceable with 'for each' + <code>for</code> loop replaceable with 'for each' #loc + + ForCanBeForEach.java 259