Java: Added settings in ClassEscapesItsScopeInspection, enabled the inspection by default, corrected precedence of 'protected' and 'package-local' visibility (IDEA-166535)

This commit is contained in:
Pavel Dolgov
2017-02-08 15:53:28 +03:00
parent 93cbfd924f
commit 28b553bcdb
5 changed files with 150 additions and 35 deletions
@@ -2764,7 +2764,7 @@
implementationClass="com.siyeh.ig.visibility.AnonymousClassVariableHidesContainingMethodVariableInspection"/>
<localInspection groupPath="Java" language="JAVA" suppressId="ClassEscapesDefinedScope" shortName="ClassEscapesItsScope" bundle="com.siyeh.InspectionGadgetsBundle"
key="class.escapes.defined.scope.display.name" groupBundle="messages.InspectionsBundle"
groupKey="group.names.visibility.issues" enabledByDefault="false" level="WARNING"
groupKey="group.names.visibility.issues" enabledByDefault="true" level="WARNING"
implementationClass="com.siyeh.ig.visibility.ClassEscapesItsScopeInspection"/>
<localInspection groupPath="Java" language="JAVA" suppressId="FieldNameHidesFieldInSuperclass" shortName="FieldHidesSuperclassField"
bundle="com.siyeh.InspectionGadgetsBundle" key="field.name.hides.in.superclass.display.name"
@@ -18,7 +18,7 @@ package com.siyeh.ig.visibility;
import com.intellij.codeInsight.daemon.impl.analysis.JavaModuleGraphUtil;
import com.intellij.codeInspection.BaseJavaBatchLocalInspectionTool;
import com.intellij.codeInspection.ProblemsHolder;
import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel;
import com.intellij.codeInspection.ui.MultipleCheckboxOptionsPanel;
import com.intellij.openapi.module.Module;
import com.intellij.openapi.roots.ModuleFileIndex;
import com.intellij.openapi.roots.ModuleRootManager;
@@ -42,10 +42,9 @@ import java.util.Set;
public class ClassEscapesItsScopeInspection extends BaseJavaBatchLocalInspectionTool {
/**
* @noinspection PublicField
*/
public boolean onlyJava9Modules = true;
@SuppressWarnings("PublicField") public boolean checkModuleApi = true; // public & protected fields & methods within expoted packages
@SuppressWarnings("PublicField") public boolean checkPublicApi; // All public & protected fields & methods
@SuppressWarnings("PublicField") public boolean checkPackageLocal;
@Pattern(VALID_ID_PATTERN)
@Override
@@ -63,38 +62,43 @@ public class ClassEscapesItsScopeInspection extends BaseJavaBatchLocalInspection
@Nullable
@Override
public JComponent createOptionsPanel() {
return new SingleCheckboxOptionsPanel(
InspectionGadgetsBundle.message("class.escapes.defined.scope.java9.modules.option"), this, "onlyJava9Modules");
MultipleCheckboxOptionsPanel panel = new MultipleCheckboxOptionsPanel(this);
panel.addCheckbox("Module's API exposes not exported classes (Java 9+)", "checkModuleApi");
panel.addCheckbox("Public API exposes non-accessible classes", "checkPublicApi");
panel.addCheckbox("Package-local API exposes private classes", "checkPackageLocal");
return panel;
}
@NotNull
@Override
public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) {
List<VisibilityChecker> checkers = new ArrayList<>(2);
PsiFile file = holder.getFile();
if (file instanceof PsiJavaFile) {
PsiJavaFile javaFile = (PsiJavaFile)file;
if (javaFile.getLanguageLevel().isAtLeast(LanguageLevel.JDK_1_9)) {
PsiJavaModule psiModule = JavaModuleGraphUtil.findDescriptorByElement(file);
if (psiModule != null) {
VirtualFile vFile = file.getVirtualFile();
if (vFile != null) {
Module module = ProjectFileIndex.SERVICE.getInstance(holder.getProject()).getModuleForFile(vFile);
if (module != null) {
Set<String> exportedPackageNames =
new THashSet<>(ContainerUtil.mapNotNull(psiModule.getExports(), PsiExportsStatement::getPackageName));
if (exportedPackageNames.contains(javaFile.getPackageName())) {
checkers.add(new Java9NonAccessibleTypeExposedVisitor(holder, module, exportedPackageNames));
if (checkModuleApi) {
PsiFile file = holder.getFile();
if (file instanceof PsiJavaFile) {
PsiJavaFile javaFile = (PsiJavaFile)file;
if (javaFile.getLanguageLevel().isAtLeast(LanguageLevel.JDK_1_9)) {
PsiJavaModule psiModule = JavaModuleGraphUtil.findDescriptorByElement(file);
if (psiModule != null) {
VirtualFile vFile = file.getVirtualFile();
if (vFile != null) {
Module module = ProjectFileIndex.SERVICE.getInstance(holder.getProject()).getModuleForFile(vFile);
if (module != null) {
Set<String> exportedPackageNames =
new THashSet<>(ContainerUtil.mapNotNull(psiModule.getExports(), PsiExportsStatement::getPackageName));
if (exportedPackageNames.contains(javaFile.getPackageName())) {
checkers.add(new Java9NonAccessibleTypeExposedVisitor(holder, module, exportedPackageNames));
}
}
}
}
}
}
}
if (!onlyJava9Modules) {
if (checkPublicApi || checkPackageLocal) {
checkers.add(new ClassEscapesItsScopeVisitor(holder));
}
return new VisibilityVisitor(checkers.toArray(VisibilityChecker.EMPTY_ARRAY));
return !checkers.isEmpty() ? new VisibilityVisitor(checkers.toArray(VisibilityChecker.EMPTY_ARRAY)) : PsiElementVisitor.EMPTY_VISITOR;
}
private static class VisibilityVisitor extends JavaElementVisitor {
@@ -153,21 +157,26 @@ public class ClassEscapesItsScopeInspection extends BaseJavaBatchLocalInspection
abstract boolean checkVisibilityIssue(PsiMember member, PsiClass psiClass, PsiJavaCodeReferenceElement reference);
}
private static class ClassEscapesItsScopeVisitor extends VisibilityChecker {
private class ClassEscapesItsScopeVisitor extends VisibilityChecker {
public ClassEscapesItsScopeVisitor(ProblemsHolder holder) {
super(holder);
}
@Override
boolean checkVisibilityIssue(PsiMember member, PsiClass psiClass, PsiJavaCodeReferenceElement reference) {
if (isLessRestrictiveScope(member, psiClass)) {
if (needToCheck(member) && isLessRestrictiveScope(member, psiClass)) {
myHolder.registerProblem(reference, InspectionGadgetsBundle.message("class.escapes.defined.scope.problem.descriptor"));
return true;
}
return false;
}
private static boolean isLessRestrictiveScope(@NotNull PsiMember member, @NotNull PsiClass aClass) {
private boolean needToCheck(PsiMember member) {
return checkPublicApi && (member.hasModifierProperty(PsiModifier.PUBLIC) || member.hasModifierProperty(PsiModifier.PROTECTED)) ||
checkPackageLocal && member.hasModifierProperty(PsiModifier.PACKAGE_LOCAL);
}
private boolean isLessRestrictiveScope(@NotNull PsiMember member, @NotNull PsiClass aClass) {
final int methodScopeOrder = getScopeOrder(member);
final int classScopeOrder = getScopeOrder(aClass);
final PsiClass containingClass = member.getContainingClass();
@@ -179,7 +188,7 @@ public class ClassEscapesItsScopeInspection extends BaseJavaBatchLocalInspection
return methodScopeOrder > classScopeOrder && containingClassScopeOrder > classScopeOrder;
}
private static int getScopeOrder(@NotNull PsiModifierListOwner element) {
private int getScopeOrder(@NotNull PsiModifierListOwner element) {
if (element.hasModifierProperty(PsiModifier.PUBLIC)) {
return 4;
}
@@ -187,10 +196,10 @@ public class ClassEscapesItsScopeInspection extends BaseJavaBatchLocalInspection
return 1;
}
else if (element.hasModifierProperty(PsiModifier.PROTECTED)) {
return 2;
return 3;
}
else {
return 3;
return 2;
}
}
}
@@ -0,0 +1,44 @@
import java.util.List;
public class ExposedByPackageLocal {
public static class NestedPublic { }
protected static class NestedProtected { }
static class NestedPackageLocal { }
private static class NestedPrivate { }
public NestedPublic withPublic1(
List<NestedPublic> list) { return list.get(0);}
protected NestedPublic withPublic2(
List<NestedPublic> list) { return list.get(0);}
NestedPublic withPublic3(
List<NestedPublic> list) { return list.get(0);}
private NestedPublic withPublic4(
List<NestedPublic> list) { return list.get(0);}
public NestedProtected withProtected1(
List<NestedProtected> list) { return list.get(0);}
protected NestedProtected withProtected2(
List<NestedProtected> list) { return list.get(0);}
NestedProtected withProtected3(
List<NestedProtected> list) { return list.get(0);}
private NestedProtected withProtected4(
List<NestedProtected> list) { return list.get(0);}
public NestedPackageLocal withPackageLocal1(
List<NestedPackageLocal> list) { return list.get(0);}
protected NestedPackageLocal withPackageLocal2(
List<NestedPackageLocal> list) { return list.get(0);}
NestedPackageLocal withPackageLocal3(
List<NestedPackageLocal> list) { return list.get(0);}
private NestedPackageLocal withPackageLocal4(
List<NestedPackageLocal> list) { return list.get(0);}
public NestedPrivate withPrivate1(
List<NestedPrivate> list) { return list.get(0);}
protected NestedPrivate withPrivate2(
List<NestedPrivate> list) { return list.get(0);}
<warning descr="Class 'NestedPrivate' is exposed outside its defined scope">NestedPrivate</warning> withPrivate3(
List<<warning descr="Class 'NestedPrivate' is exposed outside its defined scope">NestedPrivate</warning>> list) { return list.get(0);}
private NestedPrivate withPrivate4(
List<NestedPrivate> list) { return list.get(0);}
}
@@ -0,0 +1,44 @@
import java.util.List;
public class ExposedByPublic {
public static class NestedPublic { }
protected static class NestedProtected { }
static class NestedPackageLocal { }
private static class NestedPrivate { }
public NestedPublic withPublic1(
List<NestedPublic> list) { return list.get(0);}
protected NestedPublic withPublic2(
List<NestedPublic> list) { return list.get(0);}
NestedPublic withPublic3(
List<NestedPublic> list) { return list.get(0);}
private NestedPublic withPublic4(
List<NestedPublic> list) { return list.get(0);}
public <warning descr="Class 'NestedProtected' is exposed outside its defined scope">NestedProtected</warning> withProtected1(
List<<warning descr="Class 'NestedProtected' is exposed outside its defined scope">NestedProtected</warning>> list) { return list.get(0);}
protected NestedProtected withProtected2(
List<NestedProtected> list) { return list.get(0);}
NestedProtected withProtected3(
List<NestedProtected> list) { return list.get(0);}
private NestedProtected withProtected4(
List<NestedProtected> list) { return list.get(0);}
public <warning descr="Class 'NestedPackageLocal' is exposed outside its defined scope">NestedPackageLocal</warning> withPackageLocal1(
List<<warning descr="Class 'NestedPackageLocal' is exposed outside its defined scope">NestedPackageLocal</warning>> list) { return list.get(0);}
protected <warning descr="Class 'NestedPackageLocal' is exposed outside its defined scope">NestedPackageLocal</warning> withPackageLocal2(
List<<warning descr="Class 'NestedPackageLocal' is exposed outside its defined scope">NestedPackageLocal</warning>> list) { return list.get(0);}
NestedPackageLocal withPackageLocal3(
List<NestedPackageLocal> list) { return list.get(0);}
private NestedPackageLocal withPackageLocal4(
List<NestedPackageLocal> list) { return list.get(0);}
public <warning descr="Class 'NestedPrivate' is exposed outside its defined scope">NestedPrivate</warning> withPrivate1(
List<<warning descr="Class 'NestedPrivate' is exposed outside its defined scope">NestedPrivate</warning>> list) { return list.get(0);}
protected <warning descr="Class 'NestedPrivate' is exposed outside its defined scope">NestedPrivate</warning> withPrivate2(
List<<warning descr="Class 'NestedPrivate' is exposed outside its defined scope">NestedPrivate</warning>> list) { return list.get(0);}
NestedPrivate withPrivate3(
List<NestedPrivate> list) { return list.get(0);}
private NestedPrivate withPrivate4(
List<NestedPrivate> list) { return list.get(0);}
}
@@ -23,16 +23,34 @@ import org.jetbrains.annotations.Nullable;
* @author Bas Leijdekkers
*/
public class ClassEscapesItsScopeInspectionTest extends LightInspectionTestCase {
private ClassEscapesItsScopeInspection myInspection = new ClassEscapesItsScopeInspection();
public void testClassEscapesItsScope() { doTest(); }
public void testClassEscapesItsScope() { doTest(true, true); }
public void testGenericParameterEscapesItsScope() { doTest(); }
public void testGenericParameterEscapesItsScope() { doTest(true, true); }
public void testExposedByPublic() {
doTest(true, false);
}
public void testExposedByPackageLocal() {
doTest(false, true);
}
private void doTest(boolean checkPublicApi, boolean checkPackageLocal) {
myInspection.checkPublicApi = checkPublicApi;
myInspection.checkPackageLocal = checkPackageLocal;
try {
doTest();
}
finally {
myInspection.checkPublicApi = myInspection.checkPackageLocal = false;
}
}
@Nullable
@Override
protected InspectionProfileEntry getInspection() {
ClassEscapesItsScopeInspection inspection = new ClassEscapesItsScopeInspection();
inspection.onlyJava9Modules = false;
return inspection;
return myInspection;
}
}