From 764452dba1b7fe34336a2eda92991180f07ac8d7 Mon Sep 17 00:00:00 2001 From: Pavel Dolgov Date: Thu, 19 Jan 2017 17:33:01 +0300 Subject: [PATCH] Java: Look for classes which escape their scope in all parts of the method's signature (IDEA-166535) --- .../ClassEscapesItsScopeInspection.java | 86 ++++++++----------- .../ClassEscapesItsScope.java | 8 ++ .../GenericParameterEscapesItsScope.java | 59 +++++++++++++ .../ClassEscapesItsScopeInspectionTest.java | 2 + 4 files changed, 106 insertions(+), 49 deletions(-) create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igtest/visibility/class_escapes_its_scope/GenericParameterEscapesItsScope.java diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/visibility/ClassEscapesItsScopeInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/visibility/ClassEscapesItsScopeInspection.java index 2231a0a9dc9c..04e047379599 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/visibility/ClassEscapesItsScopeInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/visibility/ClassEscapesItsScopeInspection.java @@ -16,13 +16,16 @@ package com.siyeh.ig.visibility; import com.intellij.psi.*; +import com.intellij.psi.util.PsiTreeUtil; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; +import org.intellij.lang.annotations.Pattern; import org.jetbrains.annotations.NotNull; public class ClassEscapesItsScopeInspection extends BaseInspection { + @Pattern(VALID_ID_PATTERN) @Override @NotNull public String getID() { @@ -47,70 +50,55 @@ public class ClassEscapesItsScopeInspection extends BaseInspection { } private static class ClassEscapesItsScopeVisitor extends BaseInspectionVisitor { - @Override - public void visitMethod(@NotNull PsiMethod method) { - //no call to super, so we don't drill into anonymous classes - if (method.isConstructor()) { - return; + public void visitReferenceElement(PsiJavaCodeReferenceElement reference) { + super.visitReferenceElement(reference); + PsiElement parent = reference.getParent(); + if (parent instanceof PsiTypeElement || parent instanceof PsiReferenceList) { + PsiElement grandParent = PsiTreeUtil.skipParentsOfType(reference, PsiTypeElement.class, PsiReferenceList.class, + PsiParameter.class, PsiParameterList.class, + PsiReferenceParameterList.class, PsiJavaCodeReferenceElement.class, + PsiTypeParameter.class, PsiTypeParameterList.class); + if (grandParent instanceof PsiField || grandParent instanceof PsiMethod) { + PsiMember member = (PsiMember)grandParent; + if (!isPrivate(member)) { + PsiElement resolved = reference.resolve(); + if (resolved instanceof PsiClass && !(resolved instanceof PsiTypeParameter)) { + PsiClass psiClass = (PsiClass)resolved; + if (isLessRestrictiveScope(member, psiClass)) { + registerError(reference); + } + } + } + } } - if (method.hasModifierProperty(PsiModifier.PRIVATE)) { - return; - } - checkForEscaping(method, method.getReturnType(), method.getReturnTypeElement()); } - @Override - public void visitField(@NotNull PsiField field) { - //no call to super, so we don't drill into anonymous classes - if (field.hasModifierProperty(PsiModifier.PRIVATE)) { - return; + private static boolean isPrivate(@NotNull PsiMember member) { + if (member.hasModifierProperty(PsiModifier.PRIVATE)) { + return true; } - final PsiClass containingClass = field.getContainingClass(); - if (containingClass == null) { - return; + PsiClass containingClass = member.getContainingClass(); + if (containingClass != null && isPrivate(containingClass)) { + return true; } - if (containingClass.hasModifierProperty(PsiModifier.PRIVATE)) { - return; - } - checkForEscaping(field, field.getType(), field.getTypeElement()); + + return false; } - private void checkForEscaping(PsiMember member, PsiType type, PsiTypeElement typeElement) { - if (type == null || typeElement == null) { - return; - } - final PsiType componentType = type.getDeepComponentType(); - if (!(componentType instanceof PsiClassType)) { - return; - } - final PsiClass fieldClass = ((PsiClassType)componentType).resolve(); - if (fieldClass == null || fieldClass instanceof PsiTypeParameter) { - return; - } - if (!isLessRestrictiveScope(member, fieldClass)) { - return; - } - - final PsiJavaCodeReferenceElement baseTypeElement = typeElement.getInnermostComponentReferenceElement(); - if (baseTypeElement == null) { - return; - } - registerError(baseTypeElement); - } - - private static boolean isLessRestrictiveScope(PsiMember method, PsiClass aClass) { - final int methodScopeOrder = getScopeOrder(method); + private static boolean isLessRestrictiveScope(@NotNull PsiMember member, @NotNull PsiClass aClass) { + final int methodScopeOrder = getScopeOrder(member); final int classScopeOrder = getScopeOrder(aClass); - final PsiClass containingClass = method.getContainingClass(); - if (containingClass != null && containingClass.getQualifiedName() == null) { + final PsiClass containingClass = member.getContainingClass(); + if (containingClass == null || + containingClass.getQualifiedName() == null) { return false; } final int containingClassScopeOrder = getScopeOrder(containingClass); return methodScopeOrder > classScopeOrder && containingClassScopeOrder > classScopeOrder; } - private static int getScopeOrder(PsiModifierListOwner element) { + private static int getScopeOrder(@NotNull PsiModifierListOwner element) { if (element.hasModifierProperty(PsiModifier.PUBLIC)) { return 4; } diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/visibility/class_escapes_its_scope/ClassEscapesItsScope.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/visibility/class_escapes_its_scope/ClassEscapesItsScope.java index 923bb756e4cf..b389ffbc07b9 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/visibility/class_escapes_its_scope/ClassEscapesItsScope.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/visibility/class_escapes_its_scope/ClassEscapesItsScope.java @@ -5,7 +5,15 @@ public class ClassEscapesItsScope { public A giveMeA() { return new A(); } + void printA(A a) { + System.out.println(a); + } private class A {} + + void throwsE() throws E { + throw new E(); + } + private static class E extends Exception {} } class BarInside { diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/visibility/class_escapes_its_scope/GenericParameterEscapesItsScope.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/visibility/class_escapes_its_scope/GenericParameterEscapesItsScope.java new file mode 100644 index 000000000000..ac43bfcf4fc9 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/visibility/class_escapes_its_scope/GenericParameterEscapesItsScope.java @@ -0,0 +1,59 @@ +/* + * Copyright 2000-2017 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. + */ +import java.util.*; +public class GenericParameterEscapesItsScope { + public List as; + public List<B> bs; + + public List<B> getBs() { return bs; } + public void setBs(List<B> bs) { this.bs = bs; } + + public List getAs() { return as; } + public void setAs(List as) { this.as = as; } + + public class Inner extends B implements Getter, Setter { + public B b; + @Override + public B get() { + return b; + } + @Override + public void set(B b) { + this.b = b; + } + } + public Data<B> foo() { + class Local extends B implements Data { + public B b; + @Override + public B get() { + return b; + } + @Override + public void set(B b) { + this.b = b; + } + } + return new Local(); + } + + public static class A {} + static class B extends A {} + + public interface Getter { T get(); } + interface Setter { void set(T t); } + public interface Data extends Getter, Setter { } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/visibility/ClassEscapesItsScopeInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/visibility/ClassEscapesItsScopeInspectionTest.java index de5b8d59fffd..65d8775382b2 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/visibility/ClassEscapesItsScopeInspectionTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/visibility/ClassEscapesItsScopeInspectionTest.java @@ -27,6 +27,8 @@ public class ClassEscapesItsScopeInspectionTest extends LightInspectionTestCase public void testClassEscapesItsScope() { doTest(); } + public void testGenericParameterEscapesItsScope() { doTest(); } + @Nullable @Override protected InspectionProfileEntry getInspection() {