From 06f53800314f8e8dde4a9c6f8d3d1aec0cc0db0e Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Thu, 27 Sep 2012 21:15:26 +0200 Subject: [PATCH] Fix "String concatenation in loop" inspection for polyadic expressions --- .../StringConcatenationInLoopsInspection.java | 58 +++++++------------ .../siyeh/ig/psiutils/ExpressionUtils.java | 49 +++++++--------- .../StringConcatenationInLoop.java | 6 ++ .../expected.xml | 13 +++++ ...ingConcatenationInLoopsInspectionTest.java | 5 +- 5 files changed, 62 insertions(+), 69 deletions(-) diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/performance/StringConcatenationInLoopsInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/performance/StringConcatenationInLoopsInspection.java index 7df2b664452e..7cc0f5baebf7 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/performance/StringConcatenationInLoopsInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/performance/StringConcatenationInLoopsInspection.java @@ -1,5 +1,5 @@ /* - * Copyright 2003-2009 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. @@ -32,9 +32,7 @@ import javax.swing.JComponent; public class StringConcatenationInLoopsInspection extends BaseInspection { - /** - * @noinspection PublicField - */ + @SuppressWarnings("PublicField") public boolean m_ignoreUnlessAssigned = true; @Override @@ -46,23 +44,19 @@ public class StringConcatenationInLoopsInspection extends BaseInspection { @Override @NotNull public String getDisplayName() { - return InspectionGadgetsBundle.message( - "string.concatenation.in.loops.display.name"); + return InspectionGadgetsBundle.message("string.concatenation.in.loops.display.name"); } @Override @NotNull protected String buildErrorString(Object... infos) { - return InspectionGadgetsBundle.message( - "string.concatenation.in.loops.problem.descriptor"); + return InspectionGadgetsBundle.message("string.concatenation.in.loops.problem.descriptor"); } @Override public JComponent createOptionsPanel() { - return new SingleCheckboxOptionsPanel( - InspectionGadgetsBundle.message( - "string.concatenation.in.loops.only.option"), - this, "m_ignoreUnlessAssigned"); + return new SingleCheckboxOptionsPanel(InspectionGadgetsBundle.message("string.concatenation.in.loops.only.option"), + this, "m_ignoreUnlessAssigned"); } @Override @@ -70,12 +64,12 @@ public class StringConcatenationInLoopsInspection extends BaseInspection { return new StringConcatenationInLoopsVisitor(); } - private class StringConcatenationInLoopsVisitor - extends BaseInspectionVisitor { + private class StringConcatenationInLoopsVisitor extends BaseInspectionVisitor { + @Override public void visitPolyadicExpression(PsiPolyadicExpression expression) { super.visitPolyadicExpression(expression); - PsiExpression[] operands = expression.getOperands(); + final PsiExpression[] operands = expression.getOperands(); if (operands.length <= 1) { return; } @@ -107,8 +101,7 @@ public class StringConcatenationInLoopsInspection extends BaseInspection { } @Override - public void visitAssignmentExpression( - @NotNull PsiAssignmentExpression expression) { + public void visitAssignmentExpression(@NotNull PsiAssignmentExpression expression) { super.visitAssignmentExpression(expression); if (expression.getRExpression() == null) { return; @@ -137,8 +130,7 @@ public class StringConcatenationInLoopsInspection extends BaseInspection { } if (m_ignoreUnlessAssigned) { while (lhs instanceof PsiParenthesizedExpression) { - final PsiParenthesizedExpression parenthesizedExpression = - (PsiParenthesizedExpression)lhs; + final PsiParenthesizedExpression parenthesizedExpression = (PsiParenthesizedExpression)lhs; lhs = parenthesizedExpression.getExpression(); } if (!(lhs instanceof PsiReferenceExpression)) { @@ -149,52 +141,42 @@ public class StringConcatenationInLoopsInspection extends BaseInspection { } private boolean containingStatementExits(PsiElement element) { - final PsiStatement newExpressionStatement = - PsiTreeUtil.getParentOfType(element, PsiStatement.class); + final PsiStatement newExpressionStatement = PsiTreeUtil.getParentOfType(element, PsiStatement.class); if (newExpressionStatement == null) { return containingStatementExits(element); } - final PsiStatement parentStatement = - PsiTreeUtil.getParentOfType(newExpressionStatement, - PsiStatement.class); - return !ControlFlowUtils.statementMayCompleteNormally( - parentStatement); + final PsiStatement parentStatement = PsiTreeUtil.getParentOfType(newExpressionStatement, PsiStatement.class); + return !ControlFlowUtils.statementMayCompleteNormally(parentStatement); } private boolean isAppendedRepeatedly(PsiExpression expression) { PsiElement parent = expression.getParent(); - while (parent instanceof PsiParenthesizedExpression || - parent instanceof PsiPolyadicExpression) { + while (parent instanceof PsiParenthesizedExpression || parent instanceof PsiPolyadicExpression) { parent = parent.getParent(); } if (!(parent instanceof PsiAssignmentExpression)) { return false; } - final PsiAssignmentExpression assignmentExpression = - (PsiAssignmentExpression)parent; + final PsiAssignmentExpression assignmentExpression = (PsiAssignmentExpression)parent; PsiExpression lhs = assignmentExpression.getLExpression(); while (lhs instanceof PsiParenthesizedExpression) { - final PsiParenthesizedExpression parenthesizedExpression = - (PsiParenthesizedExpression)lhs; + final PsiParenthesizedExpression parenthesizedExpression = (PsiParenthesizedExpression)lhs; lhs = parenthesizedExpression.getExpression(); } if (!(lhs instanceof PsiReferenceExpression)) { return false; } - if (assignmentExpression.getOperationTokenType() == - JavaTokenType.PLUSEQ) { + if (assignmentExpression.getOperationTokenType() == JavaTokenType.PLUSEQ) { return true; } - final PsiReferenceExpression referenceExpression = - (PsiReferenceExpression)lhs; + final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)lhs; final PsiElement element = referenceExpression.resolve(); if (!(element instanceof PsiVariable)) { return false; } final PsiVariable variable = (PsiVariable)element; final PsiExpression rhs = assignmentExpression.getRExpression(); - return rhs != null && - VariableAccessUtils.variableIsUsed(variable, rhs); + return rhs != null && VariableAccessUtils.variableIsUsed(variable, rhs); } } } diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/psiutils/ExpressionUtils.java b/plugins/InspectionGadgets/src/com/siyeh/ig/psiutils/ExpressionUtils.java index e8787fa0093d..b73e4acf615f 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/psiutils/ExpressionUtils.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/psiutils/ExpressionUtils.java @@ -87,28 +87,27 @@ public class ExpressionUtils { field.hasModifierProperty(PsiModifier.FINAL); } - public static boolean isEvaluatedAtCompileTime( - @Nullable PsiExpression expression) { + public static boolean isEvaluatedAtCompileTime(@Nullable PsiExpression expression) { if (expression instanceof PsiLiteralExpression) { return true; } - if (expression instanceof PsiBinaryExpression) { - final PsiBinaryExpression binaryExpression = - (PsiBinaryExpression)expression; - final PsiExpression lhs = binaryExpression.getLOperand(); - final PsiExpression rhs = binaryExpression.getROperand(); - return isEvaluatedAtCompileTime(lhs) && - isEvaluatedAtCompileTime(rhs); + if (expression instanceof PsiPolyadicExpression) { + final PsiPolyadicExpression polyadicExpression = (PsiPolyadicExpression)expression; + final PsiExpression[] operands = polyadicExpression.getOperands(); + for (PsiExpression operand : operands) { + if (!isEvaluatedAtCompileTime(operand)) { + return false; + } + } + return true; } if (expression instanceof PsiPrefixExpression) { - final PsiPrefixExpression prefixExpression = - (PsiPrefixExpression)expression; + final PsiPrefixExpression prefixExpression = (PsiPrefixExpression)expression; final PsiExpression operand = prefixExpression.getOperand(); return isEvaluatedAtCompileTime(operand); } if (expression instanceof PsiReferenceExpression) { - final PsiReferenceExpression referenceExpression = - (PsiReferenceExpression)expression; + final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)expression; final PsiElement qualifier = referenceExpression.getQualifier(); if (qualifier instanceof PsiThisExpression) { return false; @@ -117,8 +116,7 @@ public class ExpressionUtils { if (element instanceof PsiField) { final PsiField field = (PsiField)element; final PsiExpression initializer = field.getInitializer(); - return field.hasModifierProperty(PsiModifier.FINAL) && - isEvaluatedAtCompileTime(initializer); + return field.hasModifierProperty(PsiModifier.FINAL) && isEvaluatedAtCompileTime(initializer); } if (element instanceof PsiVariable) { final PsiVariable variable = (PsiVariable)element; @@ -126,32 +124,25 @@ public class ExpressionUtils { return false; } final PsiExpression initializer = variable.getInitializer(); - return variable.hasModifierProperty(PsiModifier.FINAL) && - isEvaluatedAtCompileTime(initializer); + return variable.hasModifierProperty(PsiModifier.FINAL) && isEvaluatedAtCompileTime(initializer); } } if (expression instanceof PsiParenthesizedExpression) { - final PsiParenthesizedExpression parenthesizedExpression = - (PsiParenthesizedExpression)expression; - final PsiExpression deparenthesizedExpression = - parenthesizedExpression.getExpression(); + final PsiParenthesizedExpression parenthesizedExpression = (PsiParenthesizedExpression)expression; + final PsiExpression deparenthesizedExpression = parenthesizedExpression.getExpression(); return isEvaluatedAtCompileTime(deparenthesizedExpression); } if (expression instanceof PsiConditionalExpression) { - final PsiConditionalExpression conditionalExpression = - (PsiConditionalExpression)expression; + final PsiConditionalExpression conditionalExpression = (PsiConditionalExpression)expression; final PsiExpression condition = conditionalExpression.getCondition(); - final PsiExpression thenExpression = - conditionalExpression.getThenExpression(); - final PsiExpression elseExpression = - conditionalExpression.getElseExpression(); + final PsiExpression thenExpression = conditionalExpression.getThenExpression(); + final PsiExpression elseExpression = conditionalExpression.getElseExpression(); return isEvaluatedAtCompileTime(condition) && isEvaluatedAtCompileTime(thenExpression) && isEvaluatedAtCompileTime(elseExpression); } if (expression instanceof PsiTypeCastExpression) { - final PsiTypeCastExpression typeCastExpression = - (PsiTypeCastExpression)expression; + final PsiTypeCastExpression typeCastExpression = (PsiTypeCastExpression)expression; final PsiTypeElement castType = typeCastExpression.getCastType(); if (castType == null) { return false; diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/string_concatenation_in_loops/StringConcatenationInLoop.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/string_concatenation_in_loops/StringConcatenationInLoop.java index e05a53f56428..97a790162db6 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/string_concatenation_in_loops/StringConcatenationInLoop.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/string_concatenation_in_loops/StringConcatenationInLoop.java @@ -46,4 +46,10 @@ public class StringConcatenationInLoop s += k; } } + + void bla() { + while (true) { + System.out.println("a" + "b" + "c"); + } + } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/string_concatenation_in_loops/expected.xml b/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/string_concatenation_in_loops/expected.xml index 4b3ba72440f5..93101be39f32 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/string_concatenation_in_loops/expected.xml +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/string_concatenation_in_loops/expected.xml @@ -22,6 +22,13 @@ String concatenation <code>+</code> in loop #loc + + StringConcatenationInLoop.java + 16 + String concatenation in loop + String concatenation <code>+</code> in loop #loc + + StringConcatenationInLoop.java 46 @@ -29,4 +36,10 @@ String concatenation <code>+=</code> in loop #loc + + StringConcatenationInLoop.java + 45 + String concatenation in loop + String concatenation <code>+=</code> in loop #loc + diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/performance/StringConcatenationInLoopsInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/performance/StringConcatenationInLoopsInspectionTest.java index fce28422d2af..80310d941053 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/performance/StringConcatenationInLoopsInspectionTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/performance/StringConcatenationInLoopsInspectionTest.java @@ -5,7 +5,8 @@ import com.siyeh.ig.IGInspectionTestCase; public class StringConcatenationInLoopsInspectionTest extends IGInspectionTestCase { public void test() throws Exception { - doTest("com/siyeh/igtest/performance/string_concatenation_in_loops", - new StringConcatenationInLoopsInspection()); + final StringConcatenationInLoopsInspection tool = new StringConcatenationInLoopsInspection(); + tool.m_ignoreUnlessAssigned = false; + doTest("com/siyeh/igtest/performance/string_concatenation_in_loops", tool); } } \ No newline at end of file