allow visibility inspection to warn on entry points (IDEA-165181)

java/java-analysis-impl/src/com/intellij/codeInspection/deadCode/UnusedDeclarationInspectionBase.java

@@ -706,7 +706,6 @@ public class UnusedDeclarationInspectionBase extends GlobalInspectionTool {
     }
   }
 
-  @TestOnly
   public List<EntryPoint> getExtensions() {
     return myExtensions;
   }

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
       }

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;
+  }
+}

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<EntryPoint> stream) {
+    return stream
+      .filter(point -> point instanceof EntryPointWithModifiableVisibilityLevel)
+      .mapToInt(extension -> ((EntryPointWithModifiableVisibilityLevel)extension).getMinVisibilityLevel(member))
+      .max().orElse(-1);
+  }
+
+  private static int getMinVisibilityLevel(RefJavaElement refElement) {
+    ExtensionPoint<EntryPoint> point = Extensions.getRootArea().getExtensionPoint(ToolExtensionPoints.DEAD_CODE_TOOL);
+    PsiElement element = refElement.getElement();
+    if (element instanceof PsiMember) {
+      Stream<EntryPoint> 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;

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" +
+      "    <warning descr=\"Access can be protected\">public</warning> 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();
+  }
 }

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;
   }