From e36f6fa507101d47ed41e0ed271b7a735b9b460e Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Wed, 13 Apr 2011 14:58:08 +0200 Subject: [PATCH] IDEA-67323 (New inspection: StringBuffer/StringBuilder is modified but never queried) --- .../siyeh/InspectionGadgetsBundle.properties | 3 + .../com/siyeh/ig/InspectionGadgetsPlugin.java | 1 + ...hedStringBuilderQueryUpdateInspection.java | 374 ++++++++++++++++++ .../MismatchedStringBuilderQueryUpdate.html | 9 + 4 files changed, 387 insertions(+) create mode 100644 plugins/InspectionGadgets/src/com/siyeh/ig/bugs/MismatchedStringBuilderQueryUpdateInspection.java create mode 100644 plugins/InspectionGadgets/src/inspectionDescriptions/MismatchedStringBuilderQueryUpdate.html diff --git a/plugins/InspectionGadgets/src/com/siyeh/InspectionGadgetsBundle.properties b/plugins/InspectionGadgets/src/com/siyeh/InspectionGadgetsBundle.properties index 8ec733d3a713..254c1458b284 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/InspectionGadgetsBundle.properties +++ b/plugins/InspectionGadgets/src/com/siyeh/InspectionGadgetsBundle.properties @@ -1878,3 +1878,6 @@ arrays.hash.code.quickfix=Replace with 'Arrays.hashCode()' method.can.be.variable.arity.method.display.name=Method can be variable arity method method.can.be.variable.arity.method.problem.descriptor=#ref() can be converted to variable arity method convert.to.variable.arity.method.quickfix=Convert to variable arity method +mismatched.string.builder.query.update.display.name=Mismatched query and update of StringBuilder +mismatched.string.builder.updated.problem.descriptor=Contents of {0} #ref are updated, but never queried #loc +mismatched.string.builder.queried.problem.descriptor=Contents of {0} #ref are queried, but never updated #loc diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/InspectionGadgetsPlugin.java b/plugins/InspectionGadgets/src/com/siyeh/ig/InspectionGadgetsPlugin.java index 8188adaf9c1a..59217967ddfd 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/InspectionGadgetsPlugin.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/InspectionGadgetsPlugin.java @@ -552,6 +552,7 @@ public class InspectionGadgetsPlugin implements ApplicationComponent, } m_inspectionClasses.add(MismatchedArrayReadWriteInspection.class); m_inspectionClasses.add(MismatchedCollectionQueryUpdateInspection.class); + m_inspectionClasses.add(MismatchedStringBuilderQueryUpdateInspection.class); m_inspectionClasses.add(MisspelledCompareToInspection.class); m_inspectionClasses.add(MisspelledHashcodeInspection.class); m_inspectionClasses.add(MisspelledEqualsInspection.class); diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/MismatchedStringBuilderQueryUpdateInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/MismatchedStringBuilderQueryUpdateInspection.java new file mode 100644 index 000000000000..5cd627952100 --- /dev/null +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/MismatchedStringBuilderQueryUpdateInspection.java @@ -0,0 +1,374 @@ +/* + * Copyright 2011 Bas Leijdekkers + * + * 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.bugs; + +import com.intellij.psi.*; +import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.psi.util.PsiUtil; +import com.siyeh.InspectionGadgetsBundle; +import com.siyeh.ig.BaseInspection; +import com.siyeh.ig.BaseInspectionVisitor; +import com.siyeh.ig.psiutils.TypeUtils; +import com.siyeh.ig.psiutils.VariableAccessUtils; +import org.jetbrains.annotations.Nls; +import org.jetbrains.annotations.NonNls; +import org.jetbrains.annotations.NotNull; + +import java.util.HashSet; +import java.util.Set; + +public class MismatchedStringBuilderQueryUpdateInspection extends BaseInspection { + + @NonNls + private static final Set returnSelfNames = new HashSet(); + static { + returnSelfNames.add("append"); + returnSelfNames.add("appendCodePoint"); + returnSelfNames.add("delete"); + returnSelfNames.add("deleteCharAt"); + returnSelfNames.add("insert"); + returnSelfNames.add("replace"); + returnSelfNames.add("reverse"); + } + + @Override + @NotNull + public String getID(){ + return "MismatchedQueryAndUpdateOfStringBuilder"; + } + + @Nls + @NotNull + @Override + public String getDisplayName() { + return InspectionGadgetsBundle.message( + "mismatched.string.builder.query.update.display.name"); + } + + @NotNull + @Override + protected String buildErrorString(Object... infos) { + final boolean updated = ((Boolean)infos[0]).booleanValue(); + final PsiType type = (PsiType)infos[1]; //"StringBuilder"; + if(updated){ + return InspectionGadgetsBundle.message( + "mismatched.string.builder.updated.problem.descriptor", + type.getPresentableText()); + } else{ + return InspectionGadgetsBundle.message( + "mismatched.string.builder.queried.problem.descriptor", + type.getPresentableText()); + } + } + + @Override + public boolean isEnabledByDefault() { + return true; + } + + @Override + public boolean runForWholeFile() { + return true; + } + + @Override + public BaseInspectionVisitor buildVisitor() { + return new MismatchedQueryAndUpdateOfStringBuilderVisitor(); + } + + private static class MismatchedQueryAndUpdateOfStringBuilderVisitor + extends BaseInspectionVisitor { + + @Override + public void visitField(PsiField field) { + super.visitField(field); + if (!field.hasModifierProperty(PsiModifier.PRIVATE)) { + return; + } + final PsiClass containingClass = PsiUtil.getTopLevelClass(field); + if (!checkVariable(field, containingClass)) { + return; + } + final boolean queried = + stringBuilderContentsAreQueried(field, containingClass); + final boolean updated = + stringBuilderContentsAreUpdated(field, containingClass); + if (queried == updated) { + return; + } + registerFieldError(field, Boolean.valueOf(updated), + field.getType()); + } + + @Override + public void visitLocalVariable(PsiLocalVariable variable) { + super.visitLocalVariable(variable); + final PsiCodeBlock codeBlock = + PsiTreeUtil.getParentOfType(variable, PsiCodeBlock.class); + if (!checkVariable(variable, codeBlock)) { + return; + } + final boolean queried = + stringBuilderContentsAreQueried(variable, codeBlock); + final boolean updated = + stringBuilderContentsAreUpdated(variable, codeBlock); + if (queried == updated) { + return; + } + registerVariableError(variable, Boolean.valueOf(updated), + variable.getType()); + } + + private static boolean checkVariable(PsiVariable variable, + PsiElement context) { + if(context == null){ + return false; + } + if (!TypeUtils.variableHasTypeOrSubtype(variable, + "java.lang.AbstractStringBuilder")) { + return false; + } + if(VariableAccessUtils.variableIsAssigned(variable, context)){ + return false; + } + if(VariableAccessUtils.variableIsAssignedFrom(variable, context)){ + return false; + } + if(VariableAccessUtils.variableIsReturned(variable, context)){ + return false; + } + return !VariableAccessUtils.variableIsUsedInArrayInitializer( + variable, context); + } + + private static boolean stringBuilderContentsAreUpdated( + PsiVariable variable, PsiElement context) { + final PsiExpression initializer = variable.getInitializer(); + if (initializer != null && !isDefaultConstructorCall(initializer)) { + return true; + } + return isStringBuilderUpdated(variable, context); + } + + private static boolean stringBuilderContentsAreQueried( + PsiVariable variable, PsiElement context) { + return isStringBuilderQueried(variable, context); + } + + private static boolean isDefaultConstructorCall( + PsiExpression initializer) { + if (!(initializer instanceof PsiNewExpression)) { + return false; + } + final PsiNewExpression newExpression = + (PsiNewExpression) initializer; + final PsiJavaCodeReferenceElement classReference = + newExpression.getClassReference(); + if (classReference == null) { + return false; + } + final PsiElement target = classReference.resolve(); + if (!(target instanceof PsiClass)) { + return false; + } + final PsiClass aClass = (PsiClass) target; + final String qualifiedName = aClass.getQualifiedName(); + if (!"java.lang.StringBuilder".equals(qualifiedName) && + !"java.lang.StringBuffer".equals(qualifiedName)) { + return false; + } + final PsiExpressionList argumentList = + newExpression.getArgumentList(); + if (argumentList == null) { + return false; + } + final PsiExpression[] arguments = argumentList.getExpressions(); + if (arguments.length == 0) { + return true; + } + final PsiExpression argument = arguments[0]; + final PsiType argumentType = argument.getType(); + return PsiType.INT.equals(argumentType); + } + } + + public static boolean isStringBuilderUpdated(PsiVariable variable, + PsiElement context) { + final StringBuilderUpdateCalledVisitor visitor = + new StringBuilderUpdateCalledVisitor(variable); + context.accept(visitor); + return visitor.isUpdated(); + } + + private static class StringBuilderUpdateCalledVisitor + extends JavaRecursiveElementVisitor { + + @NonNls + private static final Set updateNames = new HashSet(); + static { + updateNames.add("append"); + updateNames.add("appendCodePoint"); + updateNames.add("delete"); + updateNames.add("delete"); + updateNames.add("deleteCharAt"); + updateNames.add("insert"); + updateNames.add("replace"); + updateNames.add("setCharAt"); + } + + private final PsiVariable variable; + boolean updated = false; + + public StringBuilderUpdateCalledVisitor(PsiVariable variable) { + this.variable = variable; + } + + public boolean isUpdated() { + return updated; + } + + @Override + public void visitMethodCallExpression( + PsiMethodCallExpression expression) { + super.visitMethodCallExpression(expression); + if (updated) { + return; + } + super.visitMethodCallExpression(expression); + final PsiReferenceExpression methodExpression = + expression.getMethodExpression(); + final String name = methodExpression.getReferenceName(); + if (!updateNames.contains(name)) { + return; + } + final PsiExpression qualifierExpression = + methodExpression.getQualifierExpression(); + if (hasReferenceToVariable(variable, qualifierExpression)) { + updated = true; + } + } + } + + public static boolean isStringBuilderQueried(PsiVariable variable, + PsiElement context) { + final StringBuilderQueryCalledVisitor visitor = + new StringBuilderQueryCalledVisitor(variable); + context.accept(visitor); + return visitor.isQueried(); + } + + private static class StringBuilderQueryCalledVisitor + extends JavaRecursiveElementVisitor { + + @NonNls + private static final Set queryNames = new HashSet(); + static { + queryNames.add("toString"); + queryNames.add("indexOf"); + queryNames.add("lastIndexOf"); + queryNames.add("capacity"); + queryNames.add("charAt"); + queryNames.add("codePointAt"); + queryNames.add("codePointBefore"); + queryNames.add("codePointCount"); + queryNames.add("equals"); + queryNames.add("getChars"); + queryNames.add("hashCode"); + queryNames.add("length"); + queryNames.add("offsetByCodePoints"); + queryNames.add("subSequence"); + queryNames.add("substring"); + } + + private final PsiVariable variable; + private boolean queried = false; + + private StringBuilderQueryCalledVisitor(PsiVariable variable) { + this.variable = variable; + } + + public boolean isQueried() { + return queried; + } + + @Override public void visitElement(@NotNull PsiElement element){ + if (queried) { + return; + } + super.visitElement(element); + } + + @Override + public void visitMethodCallExpression( + PsiMethodCallExpression expression) { + if (queried) { + return; + } + super.visitMethodCallExpression(expression); + final PsiReferenceExpression methodExpression = + expression.getMethodExpression(); + final String name = methodExpression.getReferenceName(); + if (!queryNames.contains(name)) { + return; + } + final PsiExpression qualifierExpression = + methodExpression.getQualifierExpression(); + if (hasReferenceToVariable(variable, qualifierExpression)) { + queried = true; + } + } + } + + private static boolean hasReferenceToVariable(PsiVariable variable, + PsiElement element) { + if (element instanceof PsiReferenceExpression) { + final PsiReferenceExpression referenceExpression = + (PsiReferenceExpression) element; + final PsiElement target = referenceExpression.resolve(); + if (variable.equals(target)) { + return true; + } + } else if (element instanceof PsiParenthesizedExpression) { + final PsiParenthesizedExpression parenthesizedExpression = + (PsiParenthesizedExpression) element; + final PsiExpression expression = + parenthesizedExpression.getExpression(); + return hasReferenceToVariable(variable, expression); + } else if (element instanceof PsiMethodCallExpression) { + final PsiMethodCallExpression methodCallExpression = + (PsiMethodCallExpression) element; + final PsiReferenceExpression methodExpression = + methodCallExpression.getMethodExpression(); + final String name = methodExpression.getReferenceName(); + if (returnSelfNames.contains(name)) { + return hasReferenceToVariable(variable, + methodExpression.getQualifierExpression()); + } + } else if (element instanceof PsiConditionalExpression) { + final PsiConditionalExpression conditionalExpression = + (PsiConditionalExpression) element; + final PsiExpression thenExpression = + conditionalExpression.getThenExpression(); + if (hasReferenceToVariable(variable, thenExpression)) { + return true; + } + final PsiExpression elseExpression = + conditionalExpression.getElseExpression(); + return hasReferenceToVariable(variable, elseExpression); + } + return false; + } +} diff --git a/plugins/InspectionGadgets/src/inspectionDescriptions/MismatchedStringBuilderQueryUpdate.html b/plugins/InspectionGadgets/src/inspectionDescriptions/MismatchedStringBuilderQueryUpdate.html new file mode 100644 index 000000000000..e90bc982f551 --- /dev/null +++ b/plugins/InspectionGadgets/src/inspectionDescriptions/MismatchedStringBuilderQueryUpdate.html @@ -0,0 +1,9 @@ + + +This inspection reports any StringBuilder or StringBuffer fields or variables whose contents are read but not written, +or written but not read. Such mismatched reads and writes are pointless, and probably indicate +dead, incomplete or erroneous code. +

+New in 10.5, Powered by InspectionGadgets + + \ No newline at end of file