From 346c62ab491e833d992262f8052e6ed03bc13747 Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Thu, 5 Jan 2012 16:28:51 +0100 Subject: [PATCH] IDEA-78287 (EqualsWhichDoesntCheckParameterClass should check super call either) --- ...chDoesntCheckParameterClassInspection.java | 102 ++++-------------- .../ig/bugs/ParameterClassCheckVisitor.java | 60 +++++++---- .../EqualsWhichDoesntCheckParameterClass.java | 17 +++ 3 files changed, 76 insertions(+), 103 deletions(-) diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/EqualsWhichDoesntCheckParameterClassInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/EqualsWhichDoesntCheckParameterClassInspection.java index d98f7f52c219..8a592c0ed48f 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/EqualsWhichDoesntCheckParameterClassInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/EqualsWhichDoesntCheckParameterClassInspection.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. @@ -16,7 +16,6 @@ package com.siyeh.ig.bugs; import com.intellij.psi.*; -import com.siyeh.HardcodedMethodConstants; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; @@ -25,21 +24,18 @@ import com.siyeh.ig.psiutils.MethodUtils; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; -public class EqualsWhichDoesntCheckParameterClassInspection - extends BaseInspection { +public class EqualsWhichDoesntCheckParameterClassInspection extends BaseInspection { @Override @NotNull public String getDisplayName() { - return InspectionGadgetsBundle.message( - "equals.doesnt.check.class.parameter.display.name"); + return InspectionGadgetsBundle.message("equals.doesnt.check.class.parameter.display.name"); } @Override @NotNull public String buildErrorString(Object... infos) { - return InspectionGadgetsBundle.message( - "equals.doesnt.check.class.parameter.problem.descriptor"); + return InspectionGadgetsBundle.message("equals.doesnt.check.class.parameter.problem.descriptor"); } @Override @@ -52,8 +48,7 @@ public class EqualsWhichDoesntCheckParameterClassInspection return new EqualsWhichDoesntCheckParameterClassVisitor(); } - private static class EqualsWhichDoesntCheckParameterClassVisitor - extends BaseInspectionVisitor { + private static class EqualsWhichDoesntCheckParameterClassVisitor extends BaseInspectionVisitor { @Override public void visitMethod(@NotNull PsiMethod method) { @@ -71,14 +66,13 @@ public class EqualsWhichDoesntCheckParameterClassInspection if (isParameterChecked(body, parameter)) { return; } - if (isParameterCheckNotNeeded(body, parameter)) { + if (isParameterCheckNotNeeded(body)) { return; } registerMethodError(method); } - private static boolean isParameterCheckNotNeeded( - PsiCodeBlock body, PsiParameter parameter) { + private static boolean isParameterCheckNotNeeded(PsiCodeBlock body) { final PsiStatement[] statements = body.getStatements(); if (statements.length == 0) { return true; @@ -90,66 +84,22 @@ public class EqualsWhichDoesntCheckParameterClassInspection if (!(statement instanceof PsiReturnStatement)) { return false; } - final PsiReturnStatement returnStatement = - (PsiReturnStatement)statement; - final PsiExpression returnValue = - returnStatement.getReturnValue(); - final Object constant = - ExpressionUtils.computeConstantExpression(returnValue); - if (Boolean.FALSE.equals(constant)) { - return true; - } - if (!(returnValue instanceof PsiMethodCallExpression)) { - return false; - } - final PsiMethodCallExpression methodCallExpression = - (PsiMethodCallExpression)returnValue; - return isCallToSuperEquals(methodCallExpression, parameter); + final PsiReturnStatement returnStatement = (PsiReturnStatement)statement; + final PsiExpression returnValue = returnStatement.getReturnValue(); + final Object constant = ExpressionUtils.computeConstantExpression(returnValue); + return Boolean.FALSE.equals(constant); } - private static boolean isCallToSuperEquals( - PsiMethodCallExpression methodCallExpression, - PsiParameter parameter) { - final PsiReferenceExpression methodExpression = - methodCallExpression.getMethodExpression(); - final PsiExpression qualifierExpression = - methodExpression.getQualifierExpression(); - if (!(qualifierExpression instanceof PsiSuperExpression)) { - return false; - } - final String name = methodExpression.getReferenceName(); - if (!HardcodedMethodConstants.EQUALS.equals(name)) { - return false; - } - final PsiExpressionList argumentList = - methodCallExpression.getArgumentList(); - final PsiExpression[] arguments = argumentList.getExpressions(); - if (arguments.length != 1) { - return false; - } - final PsiExpression argument = arguments[0]; - if (!(argument instanceof PsiReferenceExpression)) { - return false; - } - final PsiReferenceExpression referenceExpression = - (PsiReferenceExpression)argument; - final PsiElement target = referenceExpression.resolve(); - return parameter.equals(target); - } - - private static boolean isParameterChecked(PsiCodeBlock body, - PsiParameter parameter) { + private static boolean isParameterChecked(PsiCodeBlock body, PsiParameter parameter) { if (usesEqualsBuilderReflectionEquals(body)) { return true; } - final ParameterClassCheckVisitor visitor = - new ParameterClassCheckVisitor(parameter); + final ParameterClassCheckVisitor visitor = new ParameterClassCheckVisitor(parameter); body.accept(visitor); return visitor.isChecked(); } - private static boolean usesEqualsBuilderReflectionEquals( - PsiCodeBlock body) { + private static boolean usesEqualsBuilderReflectionEquals(PsiCodeBlock body) { final PsiStatement[] statements = body.getStatements(); if (statements.length != 1) { return false; @@ -158,37 +108,29 @@ public class EqualsWhichDoesntCheckParameterClassInspection if (!(statement instanceof PsiReturnStatement)) { return false; } - final PsiReturnStatement returnStatement = - (PsiReturnStatement)statement; - final PsiExpression returnValue = - returnStatement.getReturnValue(); + final PsiReturnStatement returnStatement = (PsiReturnStatement)statement; + final PsiExpression returnValue = returnStatement.getReturnValue(); if (!(returnValue instanceof PsiMethodCallExpression)) { return false; } - final PsiMethodCallExpression methodCallExpression = - (PsiMethodCallExpression)returnValue; - final PsiReferenceExpression methodExpression = - methodCallExpression.getMethodExpression(); - @NonNls final String referenceName = - methodExpression.getReferenceName(); + final PsiMethodCallExpression methodCallExpression = (PsiMethodCallExpression)returnValue; + final PsiReferenceExpression methodExpression = methodCallExpression.getMethodExpression(); + @NonNls final String referenceName = methodExpression.getReferenceName(); if (!"reflectionEquals".equals(referenceName)) { return false; } - final PsiExpression qualifier = - methodExpression.getQualifierExpression(); + final PsiExpression qualifier = methodExpression.getQualifierExpression(); if (!(qualifier instanceof PsiReferenceExpression)) { return false; } - final PsiReferenceExpression referenceExpression = - (PsiReferenceExpression)qualifier; + final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)qualifier; final PsiElement target = referenceExpression.resolve(); if (!(target instanceof PsiClass)) { return false; } final PsiClass aClass = (PsiClass)target; final String className = aClass.getQualifiedName(); - return "org.apache.commons.lang.builder.EqualsBuilder".equals( - className); + return "org.apache.commons.lang.builder.EqualsBuilder".equals(className); } } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/ParameterClassCheckVisitor.java b/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/ParameterClassCheckVisitor.java index 06cf86997485..77fa225c1e72 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/ParameterClassCheckVisitor.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/ParameterClassCheckVisitor.java @@ -1,5 +1,5 @@ /* - * Copyright 2003-2010 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. @@ -26,7 +26,7 @@ class ParameterClassCheckVisitor extends JavaRecursiveElementVisitor { private boolean checked = false; - ParameterClassCheckVisitor(PsiParameter parameter) { + ParameterClassCheckVisitor(@NotNull PsiParameter parameter) { super(); this.parameter = parameter; } @@ -45,15 +45,13 @@ class ParameterClassCheckVisitor extends JavaRecursiveElementVisitor { return; } super.visitMethodCallExpression(expression); - if (isGetClassCall(expression) || isGetInstanceCall(expression)) { + if (isGetClassCall(expression) || isGetInstanceCall(expression) || isCallToSuperEquals(expression)) { checked = true; } } - private boolean isGetInstanceCall( - PsiMethodCallExpression methodCallExpression) { - final PsiReferenceExpression methodExpression = - methodCallExpression.getMethodExpression(); + private boolean isGetInstanceCall(PsiMethodCallExpression methodCallExpression) { + final PsiReferenceExpression methodExpression = methodCallExpression.getMethodExpression(); final String methodName = methodExpression.getReferenceName(); if (!HardcodedMethodConstants.IS_INSTANCE.equals(methodName)) { return false; @@ -70,8 +68,7 @@ class ParameterClassCheckVisitor extends JavaRecursiveElementVisitor { if (!CommonClassNames.JAVA_LANG_CLASS.equals(className)) { return false; } - final PsiExpressionList argumentList = - methodCallExpression.getArgumentList(); + final PsiExpressionList argumentList = methodCallExpression.getArgumentList(); final PsiExpression[] expressions = argumentList.getExpressions(); if (expressions.length != 1) { return false; @@ -80,16 +77,13 @@ class ParameterClassCheckVisitor extends JavaRecursiveElementVisitor { return isParameterReference(expression); } - private boolean isGetClassCall( - PsiMethodCallExpression methodCallExpression) { - final PsiReferenceExpression methodExpression = - methodCallExpression.getMethodExpression(); + private boolean isGetClassCall(PsiMethodCallExpression methodCallExpression) { + final PsiReferenceExpression methodExpression = methodCallExpression.getMethodExpression(); final String methodName = methodExpression.getReferenceName(); if (!HardcodedMethodConstants.GET_CLASS.equals(methodName)) { return false; } - final PsiExpressionList argumentList = - methodCallExpression.getArgumentList(); + final PsiExpressionList argumentList = methodCallExpression.getArgumentList(); final PsiExpression[] arguments = argumentList.getExpressions(); if (arguments.length != 0) { return false; @@ -106,14 +100,36 @@ class ParameterClassCheckVisitor extends JavaRecursiveElementVisitor { if (!CommonClassNames.JAVA_LANG_OBJECT.equals(className)) { return false; } - final PsiExpression qualifier = - methodExpression.getQualifierExpression(); + final PsiExpression qualifier = methodExpression.getQualifierExpression(); return isParameterReference(qualifier); } + private boolean isCallToSuperEquals(PsiMethodCallExpression methodCallExpression) { + final PsiReferenceExpression methodExpression = methodCallExpression.getMethodExpression(); + final PsiExpression qualifierExpression = methodExpression.getQualifierExpression(); + if (!(qualifierExpression instanceof PsiSuperExpression)) { + return false; + } + final String name = methodExpression.getReferenceName(); + if (!HardcodedMethodConstants.EQUALS.equals(name)) { + return false; + } + final PsiExpressionList argumentList = methodCallExpression.getArgumentList(); + final PsiExpression[] arguments = argumentList.getExpressions(); + if (arguments.length != 1) { + return false; + } + final PsiExpression argument = arguments[0]; + if (!(argument instanceof PsiReferenceExpression)) { + return false; + } + final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)argument; + final PsiElement target = referenceExpression.resolve(); + return parameter.equals(target); + } + @Override - public void visitInstanceOfExpression( - @NotNull PsiInstanceOfExpression expression) { + public void visitInstanceOfExpression(@NotNull PsiInstanceOfExpression expression) { if (checked) { return; } @@ -135,8 +151,7 @@ class ParameterClassCheckVisitor extends JavaRecursiveElementVisitor { if (!isParameterReference(operand)) { return; } - final PsiTryStatement statement = - PsiTreeUtil.getParentOfType(expression, PsiTryStatement.class); + final PsiTryStatement statement = PsiTreeUtil.getParentOfType(expression, PsiTryStatement.class); if (statement == null) { return; } @@ -173,8 +188,7 @@ class ParameterClassCheckVisitor extends JavaRecursiveElementVisitor { if (!(operand instanceof PsiReferenceExpression)) { return false; } - final PsiReferenceExpression expression = - (PsiReferenceExpression)operand; + final PsiReferenceExpression expression = (PsiReferenceExpression)operand; final PsiElement referent = expression.resolve(); return referent != null && referent.equals(parameter); } diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/equals_which_doesnt_check_parameter/EqualsWhichDoesntCheckParameterClass.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/equals_which_doesnt_check_parameter/EqualsWhichDoesntCheckParameterClass.java index f31cceebc9fb..c7519ff0f566 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/equals_which_doesnt_check_parameter/EqualsWhichDoesntCheckParameterClass.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/equals_which_doesnt_check_parameter/EqualsWhichDoesntCheckParameterClass.java @@ -79,3 +79,20 @@ class Five { return super.equals(obj); } } +class Parent { + private String parentField; + + public boolean equals(Object o) { + if (! getClass().isAssignableFrom(o.getClass()) {return false;} + return ((Parent)o).parentField.equals(parentField); + } +} + +class Child { + private String childField; + + public boolean equals(Object o) { + if (! super.equals(o)) {return false;} + return ((Child)o).childField.equals(childField); + } +}