From 2aea6d36cdcd373d6415a64272a252e653c2421c Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Fri, 22 Jun 2012 14:03:59 +0200 Subject: [PATCH] IDEA-85927 (Quick-fix for 'Manual array copy' inspection changes semantic if condition is 'i >= 0') --- .../ManualArrayCopyInspection.java | 117 +++++++----------- .../Decrement.after.java | 7 ++ .../Decrement.java | 9 ++ .../Simple.after.java | 7 ++ .../replace_with_system_arraycopy/Simple.java | 9 ++ .../performance/ManualArrayCopyFixTest.java | 19 +++ 6 files changed, 97 insertions(+), 71 deletions(-) create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/replace_with_system_arraycopy/Decrement.after.java create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/replace_with_system_arraycopy/Decrement.java create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/replace_with_system_arraycopy/Simple.after.java create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/replace_with_system_arraycopy/Simple.java create mode 100644 plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/performance/ManualArrayCopyFixTest.java diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/performance/ManualArrayCopyInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/performance/ManualArrayCopyInspection.java index 80953c044bc1..f01ef60ca5d5 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/performance/ManualArrayCopyInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/performance/ManualArrayCopyInspection.java @@ -1,5 +1,5 @@ /* - * Copyright 2003-2011 Dave Griffith, Bas Leijdekkers + * Copyright 2003-2012 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. @@ -72,18 +72,16 @@ public class ManualArrayCopyInspection extends BaseInspection { this.decrement = decrement; } + @Override @NotNull public String getName() { - return InspectionGadgetsBundle.message( - "manual.array.copy.replace.quickfix"); + return InspectionGadgetsBundle.message("manual.array.copy.replace.quickfix"); } @Override - public void doFix(Project project, ProblemDescriptor descriptor) - throws IncorrectOperationException { + public void doFix(Project project, ProblemDescriptor descriptor) throws IncorrectOperationException { final PsiElement forElement = descriptor.getPsiElement(); - final PsiForStatement forStatement = - (PsiForStatement)forElement.getParent(); + final PsiForStatement forStatement = (PsiForStatement)forElement.getParent(); final String newExpression = buildSystemArrayCopyText(forStatement); if (newExpression == null) { return; @@ -92,20 +90,15 @@ public class ManualArrayCopyInspection extends BaseInspection { } @Nullable - private String buildSystemArrayCopyText(PsiForStatement forStatement) - throws IncorrectOperationException { + private String buildSystemArrayCopyText(PsiForStatement forStatement) throws IncorrectOperationException { final PsiExpression condition = forStatement.getCondition(); - final PsiBinaryExpression binaryExpression = - (PsiBinaryExpression)ParenthesesUtils.stripParentheses( - condition); + final PsiBinaryExpression binaryExpression = (PsiBinaryExpression)ParenthesesUtils.stripParentheses(condition); if (binaryExpression == null) { return null; } - final IElementType tokenType = - binaryExpression.getOperationTokenType(); + final IElementType tokenType = binaryExpression.getOperationTokenType(); final PsiExpression limit; - if (decrement ^ JavaTokenType.LT.equals(tokenType) || - JavaTokenType.LE.equals(tokenType)) { + if (decrement ^ JavaTokenType.LT.equals(tokenType) || JavaTokenType.LE.equals(tokenType)) { limit = binaryExpression.getROperand(); } else { @@ -114,18 +107,15 @@ public class ManualArrayCopyInspection extends BaseInspection { if (limit == null) { return null; } - final PsiStatement initialization = - forStatement.getInitialization(); + final PsiStatement initialization = forStatement.getInitialization(); if (initialization == null) { return null; } if (!(initialization instanceof PsiDeclarationStatement)) { return null; } - final PsiDeclarationStatement declaration = - (PsiDeclarationStatement)initialization; - final PsiElement[] declaredElements = - declaration.getDeclaredElements(); + final PsiDeclarationStatement declaration = (PsiDeclarationStatement)initialization; + final PsiElement[] declaredElements = declaration.getDeclaredElements(); if (declaredElements.length != 1) { return null; } @@ -137,33 +127,28 @@ public class ManualArrayCopyInspection extends BaseInspection { final String lengthText; final PsiExpression initializer = variable.getInitializer(); if (decrement) { - lengthText = buildLengthText(initializer, limit, false); + lengthText = buildLengthText(initializer, limit, JavaTokenType.LE.equals(tokenType) || JavaTokenType.GE.equals(tokenType)); } else { - lengthText = buildLengthText(limit, initializer, - JavaTokenType.LE.equals(tokenType) || - JavaTokenType.GE.equals(tokenType)); + lengthText = buildLengthText(limit, initializer, JavaTokenType.LE.equals(tokenType) || JavaTokenType.GE.equals(tokenType)); } if (lengthText == null) { return null; } - final PsiArrayAccessExpression lhs = - getLhsArrayAccessExpression(forStatement); + final PsiArrayAccessExpression lhs = getLhsArrayAccessExpression(forStatement); if (lhs == null) { return null; } final PsiExpression lArray = lhs.getArrayExpression(); final String toArrayText = lArray.getText(); - final PsiArrayAccessExpression rhs = - getRhsArrayAccessExpression(forStatement); + final PsiArrayAccessExpression rhs = getRhsArrayAccessExpression(forStatement); if (rhs == null) { return null; } final PsiExpression rArray = rhs.getArrayExpression(); final String fromArrayText = rArray.getText(); final PsiExpression rhsIndexExpression = rhs.getIndexExpression(); - final PsiExpression strippedRhsIndexExpression = - ParenthesesUtils.stripParentheses(rhsIndexExpression); + final PsiExpression strippedRhsIndexExpression = ParenthesesUtils.stripParentheses(rhsIndexExpression); final PsiExpression limitExpression; if (decrement) { limitExpression = limit; @@ -171,19 +156,12 @@ public class ManualArrayCopyInspection extends BaseInspection { else { limitExpression = initializer; } - final String fromOffsetText = - buildOffsetText(strippedRhsIndexExpression, variable, - limitExpression, decrement && - (JavaTokenType.LT.equals(tokenType) || - JavaTokenType.GT.equals(tokenType))); + final String fromOffsetText = buildOffsetText(strippedRhsIndexExpression, variable, limitExpression, decrement && + (JavaTokenType.LT.equals(tokenType) || JavaTokenType.GT.equals(tokenType))); final PsiExpression lhsIndexExpression = lhs.getIndexExpression(); - final PsiExpression strippedLhsIndexExpression = - ParenthesesUtils.stripParentheses(lhsIndexExpression); - final String toOffsetText = - buildOffsetText(strippedLhsIndexExpression, variable, - limitExpression, decrement && - (JavaTokenType.LT.equals(tokenType) || - JavaTokenType.GT.equals(tokenType))); + final PsiExpression strippedLhsIndexExpression = ParenthesesUtils.stripParentheses(lhsIndexExpression); + final String toOffsetText = buildOffsetText(strippedLhsIndexExpression, variable, + limitExpression, decrement && (JavaTokenType.LT.equals(tokenType) || JavaTokenType.GT.equals(tokenType))); @NonNls final StringBuilder buffer = new StringBuilder(60); buffer.append("System.arraycopy("); buffer.append(fromArrayText); @@ -199,6 +177,7 @@ public class ManualArrayCopyInspection extends BaseInspection { return buffer.toString(); } + @Nullable private static PsiArrayAccessExpression getLhsArrayAccessExpression( PsiForStatement forStatement) { PsiStatement body = forStatement.getBody(); @@ -240,6 +219,7 @@ public class ManualArrayCopyInspection extends BaseInspection { return (PsiArrayAccessExpression)deparenthesizedExpression; } + @Nullable private static PsiArrayAccessExpression getRhsArrayAccessExpression( PsiForStatement forStatement) { PsiStatement body = forStatement.getBody(); @@ -297,9 +277,7 @@ public class ManualArrayCopyInspection extends BaseInspection { @NonNls @Nullable - private static String buildLengthText(PsiExpression max, - PsiExpression min, - boolean plusOne) { + private static String buildLengthText(PsiExpression max, PsiExpression min, boolean plusOne) { max = ParenthesesUtils.stripParentheses(max); if (max == null) { return null; @@ -308,10 +286,9 @@ public class ManualArrayCopyInspection extends BaseInspection { if (min == null) { return buildExpressionText(max, plusOne, false); } - final Object constant = - ExpressionUtils.computeConstantExpression(min); - if (constant instanceof Number) { - final Number minNumber = (Number)constant; + final Object minConstant = ExpressionUtils.computeConstantExpression(min); + if (minConstant instanceof Number) { + final Number minNumber = (Number)minConstant; final int minValue; if (plusOne) { minValue = minNumber.intValue() - 1; @@ -319,14 +296,20 @@ public class ManualArrayCopyInspection extends BaseInspection { else { minValue = minNumber.intValue(); } - final String maxText = - buildExpressionText(max, false, false); + if (minValue == 0) { + return buildExpressionText(max, false, false); + } + if (max instanceof PsiLiteralExpression) { + final Object maxConstant = ExpressionUtils.computeConstantExpression(max); + if (maxConstant instanceof Number) { + final Number number = (Number)maxConstant; + return String.valueOf(number.intValue() - minValue); + } + } + final String maxText = buildExpressionText(max, false, false); if (minValue > 0) { return maxText + '-' + minValue; } - else if (minValue == 0) { - return maxText; - } else { return maxText + '+' + -minValue; } @@ -343,39 +326,31 @@ public class ManualArrayCopyInspection extends BaseInspection { return maxText + '-' + minText; } - private static String buildExpressionText(PsiExpression expression, - boolean plusOne, - boolean parenthesize) { + private static String buildExpressionText(PsiExpression expression, boolean plusOne, boolean parenthesize) { if (!plusOne) { - final int precedence = - ParenthesesUtils.getPrecedence(expression); + final int precedence = ParenthesesUtils.getPrecedence(expression); if (precedence > ParenthesesUtils.ADDITIVE_PRECEDENCE) { return '(' + expression.getText() + ')'; } else { - if (parenthesize && precedence >= - ParenthesesUtils.ADDITIVE_PRECEDENCE) { + if (parenthesize && precedence >= ParenthesesUtils.ADDITIVE_PRECEDENCE) { return '(' + expression.getText() + ')'; } return expression.getText(); } } if (expression instanceof PsiBinaryExpression) { - final PsiBinaryExpression binaryExpression = - (PsiBinaryExpression)expression; - final IElementType tokenType = - binaryExpression.getOperationTokenType(); + final PsiBinaryExpression binaryExpression = (PsiBinaryExpression)expression; + final IElementType tokenType = binaryExpression.getOperationTokenType(); if (tokenType == JavaTokenType.MINUS) { - final PsiExpression rhs = - binaryExpression.getROperand(); + final PsiExpression rhs = binaryExpression.getROperand(); if (ExpressionUtils.isOne(rhs)) { return binaryExpression.getLOperand().getText(); } } } else if (expression instanceof PsiLiteralExpression) { - final PsiLiteralExpression literalExpression = - (PsiLiteralExpression)expression; + final PsiLiteralExpression literalExpression = (PsiLiteralExpression)expression; final Object value = literalExpression.getValue(); if (value instanceof Integer) { final Integer integer = (Integer)value; diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/replace_with_system_arraycopy/Decrement.after.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/replace_with_system_arraycopy/Decrement.after.java new file mode 100644 index 000000000000..8f20bd3539e6 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/replace_with_system_arraycopy/Decrement.after.java @@ -0,0 +1,7 @@ +package com.siyeh.igfixes.performance.replace_with_system_arraycopy; + +class Decrement { + void foo(int[] a, int[] b) { + System.arraycopy(a, 0, b, 0, 2); + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/replace_with_system_arraycopy/Decrement.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/replace_with_system_arraycopy/Decrement.java new file mode 100644 index 000000000000..7ab369e16516 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/replace_with_system_arraycopy/Decrement.java @@ -0,0 +1,9 @@ +package com.siyeh.igfixes.performance.replace_with_system_arraycopy; + +class Decrement { + void foo(int[] a, int[] b) { + for (int i = 1; i >= 0; i--) { + b[i] = a[i]; + } + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/replace_with_system_arraycopy/Simple.after.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/replace_with_system_arraycopy/Simple.after.java new file mode 100644 index 000000000000..fb1921f60a1f --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/replace_with_system_arraycopy/Simple.after.java @@ -0,0 +1,7 @@ +package com.siyeh.igfixes.performance.replace_with_system_arraycopy; + +class Simple { + void foo(String[] source, Object[] target) { + System.arraycopy(source, 0, target, 0, 5); + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/replace_with_system_arraycopy/Simple.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/replace_with_system_arraycopy/Simple.java new file mode 100644 index 000000000000..9ad83104ed83 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/replace_with_system_arraycopy/Simple.java @@ -0,0 +1,9 @@ +package com.siyeh.igfixes.performance.replace_with_system_arraycopy; + +class Simple { + void foo(String[] source, Object[] target) { + for (int k = 0; k < 5; k++) { // can be converted to System.arraycopy() + target[k] = source[k]; + } + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/performance/ManualArrayCopyFixTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/performance/ManualArrayCopyFixTest.java new file mode 100644 index 000000000000..fee9dd5bb28f --- /dev/null +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/performance/ManualArrayCopyFixTest.java @@ -0,0 +1,19 @@ +package com.siyeh.ig.fixes.performance; + +import com.siyeh.InspectionGadgetsBundle; +import com.siyeh.ig.IGQuickFixesTestCase; +import com.siyeh.ig.performance.ManualArrayCopyInspection; + +public class ManualArrayCopyFixTest extends IGQuickFixesTestCase { + + @Override + protected void setUp() throws Exception { + super.setUp(); + myFixture.enableInspections(new ManualArrayCopyInspection()); + myRelativePath = "performance/replace_with_system_arraycopy"; + myDefaultHint = InspectionGadgetsBundle.message("manual.array.copy.replace.quickfix"); + } + + public void testSimple() { doTest(); } + public void testDecrement() { doTest(); } +} \ No newline at end of file