From b3b3efe4b1f800790a85b4165dea404dceae1b88 Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Wed, 25 Oct 2017 13:39:25 +0200 Subject: [PATCH] IG: warn on '==' if no common subclass is found (IDEA-178449) --- ...lsBetweenInconvertibleTypesInspection.java | 77 +++++++++++++------ ...tweenInconvertibleTypesInspectionTest.java | 28 ++++--- 2 files changed, 65 insertions(+), 40 deletions(-) diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/EqualsBetweenInconvertibleTypesInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/EqualsBetweenInconvertibleTypesInspection.java index bfdbcafc82b7..6e3147a3dcac 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/EqualsBetweenInconvertibleTypesInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/EqualsBetweenInconvertibleTypesInspection.java @@ -16,10 +16,8 @@ package com.siyeh.ig.bugs; import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel; -import com.intellij.psi.PsiClass; -import com.intellij.psi.PsiElement; -import com.intellij.psi.PsiReferenceExpression; -import com.intellij.psi.PsiType; +import com.intellij.psi.*; +import com.intellij.psi.tree.IElementType; import com.intellij.psi.util.PsiUtil; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.BaseInspection; @@ -76,27 +74,56 @@ public class EqualsBetweenInconvertibleTypesInspection extends BaseInspection { @Override public BaseInspectionVisitor buildVisitor() { - return new BaseEqualsVisitor() { - void checkTypes(@NotNull PsiReferenceExpression expression, @NotNull PsiType leftType, @NotNull PsiType rightType) { - boolean convertible = TypeUtils.areConvertible(leftType, rightType); - if (convertible) { - if (!WARN_IF_NO_MUTUAL_SUBCLASS_FOUND) return; - if (leftType.isAssignableFrom(rightType) || rightType.isAssignableFrom(leftType)) return; - PsiClass leftClass = PsiUtil.resolveClassInClassTypeOnly(leftType); - PsiClass rightClass = PsiUtil.resolveClassInClassTypeOnly(rightType); - if (leftClass == null || rightClass == null) return; - if (!leftClass.isInterface() && !rightClass.isInterface()) return; - if (!rightClass.isInterface()) { - PsiClass tmp = leftClass; - leftClass = rightClass; - rightClass = tmp; - } - if (InheritanceUtil.existsMutualSubclass(leftClass, rightClass, isOnTheFly())) return; - } - if (TypeUtils.mayBeEqualByContract(leftType, rightType)) return; - PsiElement name = expression.getReferenceNameElement(); - registerError(name == null ? expression : name, leftType, rightType, convertible); + return new EqualsBetweenInconvertibleTypesVisitor(); + } + + private class EqualsBetweenInconvertibleTypesVisitor extends BaseEqualsVisitor { + + @Override + public void visitBinaryExpression(PsiBinaryExpression expression) { + super.visitBinaryExpression(expression); + if (!WARN_IF_NO_MUTUAL_SUBCLASS_FOUND) return; + final IElementType tokenType = expression.getOperationTokenType(); + if (!tokenType.equals(JavaTokenType.EQEQ) && !tokenType.equals(JavaTokenType.NE)) { + return; } - }; + final PsiExpression lhs = expression.getLOperand(); + final PsiType lhsType = lhs.getType(); + final PsiExpression rhs = expression.getROperand(); + if (rhs == null) { + return; + } + final PsiType rhsType = rhs.getType(); + if (lhsType == null || rhsType == null || !TypeUtils.areConvertible(lhsType, rhsType)) { + // red code + return; + } + if (existsSharedSubclass(lhsType, rhsType)) { + return; + } + registerError(expression.getOperationSign(), lhsType, rhsType, true); + } + + void checkTypes(@NotNull PsiReferenceExpression expression, @NotNull PsiType leftType, @NotNull PsiType rightType) { + boolean convertible = TypeUtils.areConvertible(leftType, rightType); + if (convertible && (!WARN_IF_NO_MUTUAL_SUBCLASS_FOUND || existsSharedSubclass(leftType, rightType))) return; + if (TypeUtils.mayBeEqualByContract(leftType, rightType)) return; + PsiElement name = expression.getReferenceNameElement(); + registerError(name == null ? expression : name, leftType, rightType, convertible); + } + + private boolean existsSharedSubclass(@NotNull PsiType leftType, @NotNull PsiType rightType) { + if (leftType.isAssignableFrom(rightType) || rightType.isAssignableFrom(leftType)) return true; + PsiClass leftClass = PsiUtil.resolveClassInClassTypeOnly(leftType); + PsiClass rightClass = PsiUtil.resolveClassInClassTypeOnly(rightType); + if (leftClass == null || rightClass == null) return true; + if (!leftClass.isInterface() && !rightClass.isInterface()) return true; + if (!rightClass.isInterface()) { + PsiClass tmp = leftClass; + leftClass = rightClass; + rightClass = tmp; + } + return InheritanceUtil.existsMutualSubclass(leftClass, rightClass, isOnTheFly()); + } } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/EqualsBetweenInconvertibleTypesInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/EqualsBetweenInconvertibleTypesInspectionTest.java index e024c70ef152..b7441f59a552 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/EqualsBetweenInconvertibleTypesInspectionTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/EqualsBetweenInconvertibleTypesInspectionTest.java @@ -1,18 +1,4 @@ -/* - * Copyright 2000-2013 JetBrains s.r.o. - * - * 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. - */ +// Copyright 2000-2017 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package com.siyeh.ig.bugs; import com.intellij.codeInspection.InspectionProfileEntry; @@ -102,6 +88,18 @@ public class EqualsBetweenInconvertibleTypesInspectionTest extends LightInspecti "}"); } + public void testNoCommonSubclassEqualityComparison() { + doTest("import java.util.Date;\n" + + "import java.util.Map;\n" + + "import java.util.Objects;\n" + + "\n" + + "class X {\n" + + " public static boolean foo(Date date, Map map) {\n" + + " return map /*No class found which is a subtype of both 'Map' and 'Date'*/==/**/ date;\n" + + " }\n" + + "}"); + } + public void testCommonSubclass() { doTest("import java.util.Date;\n" + "import java.util.Map;\n" +