diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/deadCode/UnusedDeclarationInspectionBase.java b/java/java-analysis-impl/src/com/intellij/codeInspection/deadCode/UnusedDeclarationInspectionBase.java index 389d4ee0588f..86e1303d191f 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/deadCode/UnusedDeclarationInspectionBase.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/deadCode/UnusedDeclarationInspectionBase.java @@ -706,7 +706,6 @@ public class UnusedDeclarationInspectionBase extends GlobalInspectionTool { } } - @TestOnly public List getExtensions() { return myExtensions; } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/visibility/AccessCanBeTightenedInspection.java b/java/java-analysis-impl/src/com/intellij/codeInspection/visibility/AccessCanBeTightenedInspection.java index 567762335763..1927c84e3dc8 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/visibility/AccessCanBeTightenedInspection.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/visibility/AccessCanBeTightenedInspection.java @@ -168,16 +168,24 @@ class AccessCanBeTightenedInspection extends BaseJavaBatchLocalInspectionTool { final PsiFile memberFile = member.getContainingFile(); Project project = memberFile.getProject(); - if (myDeadCodeInspection.isEntryPoint(member)) { - log(member.getName() +" is entry point"); - return currentLevel; + int minLevel = PsiUtil.ACCESS_LEVEL_PRIVATE; + boolean entryPoint = myDeadCodeInspection.isEntryPoint(member); + if (entryPoint) { + int level = VisibilityInspection.getMinVisibilityLevel(member, myDeadCodeInspection.getExtensions().stream()); + if (level <= 0) { + log(member.getName() +" is entry point"); + return currentLevel; + } + else { + minLevel = level; + } } PsiDirectory memberDirectory = memberFile.getContainingDirectory(); final PsiPackage memberPackage = memberDirectory == null ? null : JavaDirectoryService.getInstance().getPackage(memberDirectory); log(member.getName()+ ": checking effective level for "+member); - AtomicInteger maxLevel = new AtomicInteger(PsiUtil.ACCESS_LEVEL_PRIVATE); + AtomicInteger maxLevel = new AtomicInteger(minLevel); AtomicBoolean foundUsage = new AtomicBoolean(); boolean proceed = UnusedSymbolUtil.processUsages(project, memberFile, member, new EmptyProgressIndicator(), null, info -> { PsiElement element = info.getElement(); @@ -195,7 +203,7 @@ class AccessCanBeTightenedInspection extends BaseJavaBatchLocalInspectionTool { return handleUsage(member, memberClass, memberFile, maxLevel, memberPackage, functionalExpression, psiFile, foundUsage); }); } - if (!foundUsage.get()) { + if (!foundUsage.get() && !entryPoint) { log(member.getName() + " unused; ignore"); return currentLevel; // do not propose private for unused method } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/visibility/EntryPointWithModifiableVisibilityLevel.java b/java/java-analysis-impl/src/com/intellij/codeInspection/visibility/EntryPointWithModifiableVisibilityLevel.java new file mode 100644 index 000000000000..71dd61cc48d6 --- /dev/null +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/visibility/EntryPointWithModifiableVisibilityLevel.java @@ -0,0 +1,34 @@ +/* + * 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. + */ +package com.intellij.codeInspection.visibility; + +import com.intellij.codeInspection.reference.EntryPoint; +import com.intellij.psi.PsiMember; +import com.intellij.psi.util.PsiUtil; + +/** + * Register entry points which visibility can be decreased, + * e.g. package private test methods in junit 5 + */ +public abstract class EntryPointWithModifiableVisibilityLevel extends EntryPoint { + + /** + * @return minimum accepted modifier (see {@link PsiUtil.AccessLevel}) or -1 when not applicable + */ + public int getMinVisibilityLevel(PsiMember member) { + return -1; + } +} diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/visibility/VisibilityInspection.java b/java/java-analysis-impl/src/com/intellij/codeInspection/visibility/VisibilityInspection.java index 8cfc64c79121..d6700d3c8831 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/visibility/VisibilityInspection.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/visibility/VisibilityInspection.java @@ -48,7 +48,9 @@ import org.jetbrains.annotations.Nullable; import javax.swing.*; import java.awt.*; +import java.util.Arrays; import java.util.List; +import java.util.stream.Stream; public class VisibilityInspection extends GlobalJavaBatchInspectionTool { private static final Logger LOG = Logger.getInstance("#com.intellij.codeInspection.visibility.VisibilityInspection"); @@ -146,8 +148,12 @@ public class VisibilityInspection extends GlobalJavaBatchInspectionTool { if (refElement instanceof RefParameter) return null; if (refElement.isSyntheticJSP()) return null; + int minLevel = -1; //ignore entry points. - if (refElement.isEntry()) return null; + if (refElement.isEntry()) { + minLevel = getMinVisibilityLevel(refElement); + if (minLevel <= 0) return null; + } //ignore implicit constructors. User should not be able to see them. if (refElement instanceof RefImplicitConstructor) return null; @@ -170,15 +176,19 @@ public class VisibilityInspection extends GlobalJavaBatchInspectionTool { if (isTopLevelClass(refClass) && !SUGGEST_PACKAGE_LOCAL_FOR_TOP_CLASSES) return null; } + //ignore unreferenced code. They could be a potential entry points. - if (refElement.getInReferences().isEmpty()) return null; + if (refElement.getInReferences().isEmpty() && minLevel <= 0) { + minLevel = getMinVisibilityLevel(refElement); + if (minLevel <= 0) return null; + } //ignore interface members. They always have public access modifier. if (refElement.getOwner() instanceof RefClass) { RefClass refClass = (RefClass) refElement.getOwner(); if (refClass.isInterface()) return null; } - String access = getPossibleAccess(refElement); + String access = getPossibleAccess(refElement, minLevel <= 0 ? PsiUtil.ACCESS_LEVEL_PRIVATE : minLevel); if (access != refElement.getAccessModifier() && access != null) { final PsiElement element = refElement.getElement(); final PsiElement nameIdentifier = element != null ? IdentifierUtil.getNameIdentifier(element) : null; @@ -208,11 +218,29 @@ public class VisibilityInspection extends GlobalJavaBatchInspectionTool { return null; } + static int getMinVisibilityLevel(PsiMember member, + Stream stream) { + return stream + .filter(point -> point instanceof EntryPointWithModifiableVisibilityLevel) + .mapToInt(extension -> ((EntryPointWithModifiableVisibilityLevel)extension).getMinVisibilityLevel(member)) + .max().orElse(-1); + } + + private static int getMinVisibilityLevel(RefJavaElement refElement) { + ExtensionPoint point = Extensions.getRootArea().getExtensionPoint(ToolExtensionPoints.DEAD_CODE_TOOL); + PsiElement element = refElement.getElement(); + if (element instanceof PsiMember) { + Stream stream = Arrays.stream(point.getExtensions()); + return getMinVisibilityLevel((PsiMember)element, stream); + } + return -1; + } + @Nullable @PsiModifier.ModifierConstant - private String getPossibleAccess(@NotNull RefJavaElement refElement) { + private String getPossibleAccess(@NotNull RefJavaElement refElement, int minLevel) { String curAccess = refElement.getAccessModifier(); - String weakestAccess = PsiModifier.PRIVATE; + String weakestAccess = PsiUtil.getAccessModifier(minLevel); if (isTopLevelClass(refElement) || isCalledOnSubClasses(refElement)) { weakestAccess = PsiModifier.PACKAGE_LOCAL; diff --git a/plugins/InspectionGadgets/testsrc/com/intellij/codeInspection/visibility/AccessCanBeTightenedInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/intellij/codeInspection/visibility/AccessCanBeTightenedInspectionTest.java index 9041e181abec..d9467fb37b38 100644 --- a/plugins/InspectionGadgets/testsrc/com/intellij/codeInspection/visibility/AccessCanBeTightenedInspectionTest.java +++ b/plugins/InspectionGadgets/testsrc/com/intellij/codeInspection/visibility/AccessCanBeTightenedInspectionTest.java @@ -15,8 +15,22 @@ */ package com.intellij.codeInspection.visibility; +import com.intellij.ToolExtensionPoints; import com.intellij.codeInspection.LocalInspectionTool; +import com.intellij.codeInspection.reference.RefElement; +import com.intellij.openapi.extensions.ExtensionPointName; +import com.intellij.openapi.extensions.Extensions; +import com.intellij.openapi.util.InvalidDataException; +import com.intellij.openapi.util.WriteExternalException; +import com.intellij.psi.PsiClass; +import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiMember; +import com.intellij.psi.PsiMethod; +import com.intellij.psi.util.PsiUtil; +import com.intellij.testFramework.PlatformTestUtil; import com.siyeh.ig.LightInspectionTestCase; +import org.jdom.Element; +import org.jetbrains.annotations.NotNull; @SuppressWarnings("WeakerAccess") public class AccessCanBeTightenedInspectionTest extends LightInspectionTestCase { @@ -277,4 +291,50 @@ public class AccessCanBeTightenedInspectionTest extends LightInspectionTestCase myFixture.configureByFiles("x/Outer.java", "x/Consumer.java"); myFixture.checkHighlighting(); } + + public void testSuggestPackagePrivateForEntryPoint() { + myFixture.addFileToProject("x/MyTest.java", + "package x;\n" + + "public class MyTest {\n" + + " public void foo() {}\n" + + "}"); + PlatformTestUtil.registerExtension(Extensions.getRootArea(), ExtensionPointName.create(ToolExtensionPoints.DEAD_CODE_TOOL), new EntryPointWithModifiableVisibilityLevel() { + @Override + public void readExternal(Element element) throws InvalidDataException {} + + @Override + public void writeExternal(Element element) throws WriteExternalException {} + + @NotNull + @Override + public String getDisplayName() { + return "accepted visibility"; + } + + @Override + public boolean isEntryPoint(@NotNull RefElement refElement, @NotNull PsiElement psiElement) { + return isEntryPoint(psiElement); + } + + @Override + public boolean isEntryPoint(@NotNull PsiElement psiElement) { + return psiElement instanceof PsiMethod && "foo".equals(((PsiMethod)psiElement).getName()) || psiElement instanceof PsiClass; + } + + @Override + public int getMinVisibilityLevel(PsiMember member) { + return member instanceof PsiMethod && isEntryPoint(member) ? PsiUtil.ACCESS_LEVEL_PROTECTED : -1; + } + + @Override + public boolean isSelected() { + return true; + } + + @Override + public void setSelected(boolean selected) {} + }, getTestRootDisposable()); + myFixture.configureByFiles("x/MyTest.java"); + myFixture.checkHighlighting(); + } } \ No newline at end of file diff --git a/plugins/junit/src/com/intellij/execution/junit2/inspection/JUnitEntryPoint.java b/plugins/junit/src/com/intellij/execution/junit2/inspection/JUnitEntryPoint.java index fe97e8d570ab..5aaa66855448 100644 --- a/plugins/junit/src/com/intellij/execution/junit2/inspection/JUnitEntryPoint.java +++ b/plugins/junit/src/com/intellij/execution/junit2/inspection/JUnitEntryPoint.java @@ -21,8 +21,8 @@ package com.intellij.execution.junit2.inspection; import com.intellij.codeInsight.AnnotationUtil; -import com.intellij.codeInspection.reference.EntryPoint; import com.intellij.codeInspection.reference.RefElement; +import com.intellij.codeInspection.visibility.EntryPointWithModifiableVisibilityLevel; import com.intellij.execution.junit.JUnitUtil; import com.intellij.openapi.util.DefaultJDOMExternalizer; import com.intellij.openapi.util.InvalidDataException; @@ -31,11 +31,12 @@ import com.intellij.psi.*; import com.intellij.psi.search.searches.ClassInheritorsSearch; import com.intellij.psi.util.PsiClassUtil; import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.psi.util.PsiUtil; import com.intellij.util.CommonProcessors; import org.jdom.Element; import org.jetbrains.annotations.NotNull; -public class JUnitEntryPoint extends EntryPoint { +public class JUnitEntryPoint extends EntryPointWithModifiableVisibilityLevel { public boolean ADD_JUNIT_TO_ENTRIES = true; @NotNull @@ -82,6 +83,22 @@ public class JUnitEntryPoint extends EntryPoint { return false; } + @Override + public int getMinVisibilityLevel(PsiMember member) { + PsiClass container = null; + if (member instanceof PsiClass) { + container = (PsiClass)member; + } + else if (member instanceof PsiMethod) { + container = member.getContainingClass(); + } + if (container != null && JUnitUtil.isJUnit5TestClass(container, false)) { + return PsiUtil.ACCESS_LEVEL_PACKAGE_LOCAL; + } + + return super.getMinVisibilityLevel(member); + } + public boolean isSelected() { return ADD_JUNIT_TO_ENTRIES; }