diff --git a/java/java-analysis-api/src/com/intellij/codeInspection/reference/RefJavaModule.java b/java/java-analysis-api/src/com/intellij/codeInspection/reference/RefJavaModule.java index bd072f85f38d..5c5ffd7a1371 100644 --- a/java/java-analysis-api/src/com/intellij/codeInspection/reference/RefJavaModule.java +++ b/java/java-analysis-api/src/com/intellij/codeInspection/reference/RefJavaModule.java @@ -35,9 +35,15 @@ public interface RefJavaModule extends RefElement { @NotNull Map> getExportedPackageNames(); + @NotNull + Set getServiceInterfaces(); + @NotNull Set getServiceImplementations(); + @NotNull + Set getUsedServices(); + @NotNull List getRequiredModules(); diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/java19modules/Java9ModuleEntryPoint.java b/java/java-analysis-impl/src/com/intellij/codeInspection/java19modules/Java9ModuleEntryPoint.java index 63be397acfaa..3329d6334d5f 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/java19modules/Java9ModuleEntryPoint.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/java19modules/Java9ModuleEntryPoint.java @@ -16,8 +16,8 @@ package com.intellij.codeInspection.java19modules; import com.intellij.codeInsight.daemon.impl.analysis.JavaModuleGraphUtil; -import com.intellij.codeInspection.reference.EntryPoint; -import com.intellij.codeInspection.reference.RefElement; +import com.intellij.codeInspection.reference.*; +import com.intellij.codeInspection.visibility.EntryPointWithVisibilityLevel; import com.intellij.openapi.util.InvalidDataException; import com.intellij.openapi.util.WriteExternalException; import com.intellij.pom.java.LanguageLevel; @@ -30,16 +30,20 @@ import com.intellij.util.xmlb.XmlSerializer; import gnu.trove.THashSet; import one.util.streamex.StreamEx; import org.jdom.Element; +import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.Arrays; +import java.util.List; +import java.util.Map; import java.util.Set; /** * @author Pavel.Dolgov */ -public class Java9ModuleEntryPoint extends EntryPoint { +public class Java9ModuleEntryPoint extends EntryPointWithVisibilityLevel { + public static final String ID = "moduleInfo"; public boolean ADD_EXPORTED_PACKAGES_AND_SERVICES_TO_ENTRIES = true; @NotNull @@ -56,54 +60,126 @@ public class Java9ModuleEntryPoint extends EntryPoint { @Override public boolean isEntryPoint(@NotNull PsiElement psiElement) { if (psiElement instanceof PsiClass) { - return isExported((PsiClass)psiElement); + return isServiceOrExported((PsiClass)psiElement); } if (psiElement instanceof PsiMethod) { PsiMethod method = (PsiMethod)psiElement; if (isDefaultConstructor(method) || isProviderMethod(method)) { - return isExported(method.getContainingClass()); + return isServiceOrExported(method.getContainingClass()); } } return false; } - private static boolean isDefaultConstructor(PsiMethod method) { + @Override + public int getMinVisibilityLevel(PsiMember member) { + if (member instanceof PsiClass) { + final PsiJavaModule javaModule = getJavaModule(member); + if (javaModule != null && + !isServiceClass((PsiClass)member, javaModule) && + isInExportedPackage((PsiClass)member, javaModule)) { + return PsiUtil.ACCESS_LEVEL_PACKAGE_LOCAL; + } + } + return -1; + } + + @Override + public String getTitle() { + return "Suggest package-private visibility level for classes in exported packages (Java 9+)"; + } + + @Override + public String getId() { + return ID; + } + + @Override + public boolean keepVisibilityLevel(boolean entryPointEnabled, RefJavaElement refJavaElement) { + if (refJavaElement instanceof RefClass) { + RefClass refClass = (RefClass)refJavaElement; + RefModule refModule = refClass.getModule(); + if (refModule != null) { + RefJavaModule refJavaModule = RefJavaModule.JAVA_MODULE.get(refModule); + if (refJavaModule != null) { + return isServiceClass(refClass, refJavaModule) || + !entryPointEnabled && isInExportedPackage(refClass, refJavaModule); + } + } + } + return false; + } + + private static boolean isInExportedPackage(@Nullable RefClass refClass, @NotNull RefJavaModule refJavaModule) { + RefEntity refOwner = refClass; + while (refOwner instanceof RefClass) { + String modifier = ((RefClass)refOwner).getAccessModifier(); + refOwner = PsiModifier.PUBLIC.equals(modifier) || PsiModifier.PROTECTED.equals(modifier) ? refOwner.getOwner() : null; + } + if (refOwner instanceof RefPackage) { + Map> exportedPackageNames = refJavaModule.getExportedPackageNames(); + if (exportedPackageNames.containsKey(refOwner.getQualifiedName())) { + return true; + } + } + return false; + } + + private static boolean isServiceClass(@Nullable RefClass refClass, @NotNull RefJavaModule refJavaModule) { + return refJavaModule.getServiceInterfaces().contains(refClass) || + refJavaModule.getServiceImplementations().contains(refClass) || + refJavaModule.getUsedServices().contains(refClass); + } + + + private static boolean isDefaultConstructor(@NotNull PsiMethod method) { return method.isConstructor() && method.getParameterList().getParametersCount() == 0 && method.hasModifierProperty(PsiModifier.PUBLIC); } - private static boolean isProviderMethod(PsiMethod method) { + private static boolean isProviderMethod(@NotNull PsiMethod method) { return "provider".equals(method.getName()) && method.getParameterList().getParametersCount() == 0 && method.hasModifierProperty(PsiModifier.PUBLIC) && method.hasModifierProperty(PsiModifier.STATIC); } - private static boolean isExported(@Nullable PsiClass psiClass) { - if (psiClass != null) { - String className = psiClass.getQualifiedName(); - if (className != null) { - final LanguageLevel languageLevel = PsiUtil.getLanguageLevel(psiClass); - if (languageLevel.isAtLeast(LanguageLevel.JDK_1_9)) { - PsiJavaModule javaModule = JavaModuleGraphUtil.findDescriptorByElement(psiClass); - if (javaModule != null) { - String packageName = getPublicApiPackageName(psiClass); - if (packageName != null) { - Set exportedPackageNames = getExportedPackageNames(javaModule); - if (exportedPackageNames.contains(packageName)) { - return true; - } - } - Set serviceImplementationNames = getServiceImplementationNames(javaModule); - return serviceImplementationNames.contains(className); - } - } + private static boolean isServiceOrExported(@Nullable PsiClass psiClass) { + PsiJavaModule javaModule = getJavaModule(psiClass); + return javaModule != null && (isServiceClass(psiClass, javaModule) || isInExportedPackage(psiClass, javaModule)); + } + + private static boolean isInExportedPackage(@NotNull PsiClass psiClass, @NotNull PsiJavaModule javaModule) { + String packageName = getPublicApiPackageName(psiClass); + if (packageName != null) { + Set exportedPackageNames = getExportedPackageNames(javaModule); + if (exportedPackageNames.contains(packageName)) { + return true; } } return false; } + private static boolean isServiceClass(@NotNull PsiClass psiClass, @NotNull PsiJavaModule javaModule) { + Set serviceClassNames = CachedValuesManager.getCachedValue( + javaModule, () -> CachedValueProvider.Result.create(collectServiceClassNames(javaModule), javaModule)); + + return serviceClassNames.contains(psiClass.getQualifiedName()); + } + + @Contract("null -> null") + @Nullable + private static PsiJavaModule getJavaModule(@Nullable PsiElement element) { + if (element != null) { + final LanguageLevel languageLevel = PsiUtil.getLanguageLevel(element); + if (languageLevel.isAtLeast(LanguageLevel.JDK_1_9)) { + return JavaModuleGraphUtil.findDescriptorByElement(element); + } + } + return null; + } + private static String getPublicApiPackageName(PsiClass psiClass) { if (psiClass != null && (psiClass.hasModifierProperty(PsiModifier.PUBLIC) || psiClass.hasModifierProperty(PsiModifier.PROTECTED))) { PsiElement parent = psiClass.getParent(); @@ -128,18 +204,25 @@ public class Java9ModuleEntryPoint extends EntryPoint { } @NotNull - private static Set getServiceImplementationNames(@NotNull PsiJavaModule javaModule) { - return CachedValuesManager.getCachedValue(javaModule, () -> { - Set classes = StreamEx.of(javaModule.getProvides().iterator()) - .map(PsiProvidesStatement::getImplementationList) - .nonNull() - .map(PsiReferenceList::getReferenceElements) - .flatMap(Arrays::stream) - .map(PsiJavaCodeReferenceElement::getQualifiedName) - .nonNull() - .toCollection(THashSet::new); - return CachedValueProvider.Result.create(classes, javaModule); - }); + private static Set collectServiceClassNames(@NotNull PsiJavaModule javaModule) { + Set classes = StreamEx.of(javaModule.getProvides().spliterator()) + .map(PsiProvidesStatement::getImplementationList) + .nonNull() + .map(PsiReferenceList::getReferenceElements) + .flatMap(Arrays::stream) + .map(PsiJavaCodeReferenceElement::getQualifiedName) + .nonNull() + .toCollection(THashSet::new); + + Set usages = StreamEx.of(javaModule.getUses().iterator()) + .map(PsiUsesStatement::getClassReference) + .nonNull() + .map(PsiJavaCodeReferenceElement::getQualifiedName) + .nonNull() + .toCollection(THashSet::new); + + classes.addAll(usages); + return classes; } @Override diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/reference/RefJavaModuleImpl.java b/java/java-analysis-impl/src/com/intellij/codeInspection/reference/RefJavaModuleImpl.java index 71efaf4daba5..16e81f8d8854 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/reference/RefJavaModuleImpl.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/reference/RefJavaModuleImpl.java @@ -33,7 +33,9 @@ public class RefJavaModuleImpl extends RefElementImpl implements RefJavaModule { private final RefModule myRefModule; private Map> myExportedPackageNames; + private Set myServiceInterfaces; private Set myServiceImplementations; + private Set myUsedServices; private List myRequiredModules; RefJavaModuleImpl(@NotNull PsiJavaModule javaModule, @NotNull RefManagerImpl manager) { @@ -75,104 +77,152 @@ public class RefJavaModuleImpl extends RefElementImpl implements RefJavaModule { return myExportedPackageNames != null ? myExportedPackageNames : Collections.emptyMap(); } + @NotNull + @Override + public Set getServiceInterfaces() { + return myServiceInterfaces != null ? myServiceInterfaces : Collections.emptySet(); + } + @NotNull @Override public Set getServiceImplementations() { return myServiceImplementations != null ? myServiceImplementations : Collections.emptySet(); } + @NotNull + @Override + public Set getUsedServices() { + return myUsedServices != null ? myUsedServices : Collections.emptySet(); + } + @Override @NotNull public List getRequiredModules() { return myRequiredModules != null ? myRequiredModules : Collections.emptyList(); } - @Override - public void buildReferences() { - PsiJavaModule javaModule = getElement(); - if (javaModule != null) { - for (PsiRequiresStatement statement : javaModule.getRequires()) { - PsiJavaModuleReferenceElement referenceElement = statement.getReferenceElement(); + private void buildRequiresReferences(PsiJavaModule javaModule) { + for (PsiRequiresStatement statement : javaModule.getRequires()) { + PsiJavaModuleReferenceElement referenceElement = statement.getReferenceElement(); + if (referenceElement != null) { + PsiElement element = addReference(referenceElement.getReference()); + if (element instanceof PsiJavaModule) { + PsiJavaModule requiredModule = (PsiJavaModule)element; + Map> packagesExportedByModule = getPackagesExportedByModule(requiredModule); + if (myRequiredModules == null) myRequiredModules = new ArrayList<>(1); + myRequiredModules.add(new RequiredModule(requiredModule.getName(), packagesExportedByModule, statement.hasModifierProperty(PsiModifier.TRANSITIVE))); + } + } + } + } + + private void buildExportsReferences(PsiJavaModule javaModule) { + List emptyList = Collections.emptyList(); + for (PsiPackageAccessibilityStatement statement : javaModule.getExports()) { + PsiElement element = addReference(statement.getPackageReference()); + String packageName = null; + if (element instanceof PsiPackage) { + packageName = ((PsiPackage)element).getQualifiedName(); + if (myExportedPackageNames == null) myExportedPackageNames = new THashMap<>(1); + myExportedPackageNames.put(packageName, emptyList); + } + for (PsiJavaModuleReferenceElement referenceElement : statement.getModuleReferences()) { if (referenceElement != null) { - PsiElement element = addReference(referenceElement.getReference()); - if (element instanceof PsiJavaModule) { - PsiJavaModule requiredModule = (PsiJavaModule)element; - Map> packagesExportedByModule = getPackagesExportedByModule(requiredModule); - if (myRequiredModules == null) myRequiredModules = new ArrayList<>(1); - myRequiredModules.add(new RequiredModule(requiredModule.getName(), packagesExportedByModule, statement.hasModifierProperty(PsiModifier.TRANSITIVE))); + PsiElement moduleElement = addReference(referenceElement.getReference()); + if (packageName != null && moduleElement instanceof PsiJavaModule) { + List toModuleNames = myExportedPackageNames.get(packageName); + if (toModuleNames == emptyList) myExportedPackageNames.put(packageName, toModuleNames = new ArrayList<>(1)); + toModuleNames.add(((PsiJavaModule)moduleElement).getName()); } } } - List emptyList = Collections.emptyList(); - for (PsiPackageAccessibilityStatement statement : javaModule.getExports()) { - PsiElement element = addReference(statement.getPackageReference()); - String packageName = null; - if (element instanceof PsiPackage) { - packageName = ((PsiPackage)element).getQualifiedName(); - if (myExportedPackageNames == null) myExportedPackageNames = new THashMap<>(1); - myExportedPackageNames.put(packageName, emptyList); - } - for (PsiJavaModuleReferenceElement referenceElement : statement.getModuleReferences()) { - if (referenceElement != null) { - PsiElement moduleElement = addReference(referenceElement.getReference()); - if (packageName != null && moduleElement instanceof PsiJavaModule) { - List toModuleNames = myExportedPackageNames.get(packageName); - if (toModuleNames == emptyList) myExportedPackageNames.put(packageName, toModuleNames = new ArrayList<>(1)); - toModuleNames.add(((PsiJavaModule)moduleElement).getName()); - } - } - } - } - for (PsiProvidesStatement statement : javaModule.getProvides()) { - final PsiJavaCodeReferenceElement interfaceReference = statement.getInterfaceReference(); - final PsiReferenceList implementationList = statement.getImplementationList(); - if (interfaceReference != null && implementationList != null) { - final PsiElement providerInterface = interfaceReference.resolve(); - if (providerInterface instanceof PsiClass) { - final RefElement refInterface = getRefManager().getReference(providerInterface); - if (refInterface instanceof RefJavaElementImpl) { - for (PsiJavaCodeReferenceElement implementationReference : implementationList.getReferenceElements()) { - final PsiElement implementationClass = implementationReference.resolve(); - if (implementationClass instanceof PsiClass) { - RefElement refTargetElement = null; - PsiElement targetElement = getProviderMethod((PsiClass)implementationClass); + } + } - if (targetElement == null) { - final RefElement refClass = getRefManager().getReference(implementationClass); - if (refClass instanceof RefClassImpl) { - if (myServiceImplementations == null) myServiceImplementations = new THashSet<>(); - myServiceImplementations.add((RefClass)refClass); + private void buildProvidesReferences(PsiJavaModule javaModule) { + for (PsiProvidesStatement statement : javaModule.getProvides()) { + final PsiJavaCodeReferenceElement interfaceReference = statement.getInterfaceReference(); + final PsiReferenceList implementationList = statement.getImplementationList(); + if (interfaceReference != null && implementationList != null) { + final PsiElement providerInterface = interfaceReference.resolve(); + if (providerInterface instanceof PsiClass) { + final RefElement refInterface = getRefManager().getReference(providerInterface); + if (refInterface instanceof RefClassImpl) { + if (myServiceInterfaces == null) myServiceInterfaces = new THashSet<>(); + myServiceInterfaces.add((RefClass)refInterface); - final RefMethod refConstructor = ((RefClassImpl)refClass).getDefaultConstructor(); - if (refConstructor != null) { - final PsiModifierListOwner constructorElement = refConstructor.getElement(); - if (constructorElement != null && constructorElement.hasModifierProperty(PsiModifier.PUBLIC)) { - refTargetElement = refConstructor; - targetElement = constructorElement; - } + for (PsiJavaCodeReferenceElement implementationReference : implementationList.getReferenceElements()) { + final PsiElement implementationClass = implementationReference.resolve(); + if (implementationClass instanceof PsiClass) { + RefElement refTargetElement = null; + PsiElement targetElement = getProviderMethod((PsiClass)implementationClass); + + if (targetElement == null) { + final RefElement refClass = getRefManager().getReference(implementationClass); + if (refClass instanceof RefClassImpl) { + if (myServiceImplementations == null) myServiceImplementations = new THashSet<>(); + myServiceImplementations.add((RefClass)refClass); + + final RefMethod refConstructor = ((RefClassImpl)refClass).getDefaultConstructor(); + if (refConstructor != null) { + final PsiModifierListOwner constructorElement = refConstructor.getElement(); + if (constructorElement != null && constructorElement.hasModifierProperty(PsiModifier.PUBLIC)) { + refTargetElement = refConstructor; + targetElement = constructorElement; } } } - if (targetElement == null) { - targetElement = implementationClass; - } - if (refTargetElement == null) { - refTargetElement = getRefManager().getReference(targetElement); - } - if (refTargetElement != null) { - ((RefJavaElementImpl)refInterface) - .addReference(refTargetElement, targetElement, providerInterface, false, true, null); - } + } + if (targetElement == null) { + targetElement = implementationClass; + } + if (refTargetElement == null) { + refTargetElement = getRefManager().getReference(targetElement); + } + if (refTargetElement != null) { + ((RefClassImpl)refInterface) + .addReference(refTargetElement, targetElement, providerInterface, false, true, null); } } } } } } + } + } + + private void buildUsesReferences(PsiJavaModule javaModule) { + for (PsiUsesStatement statement : javaModule.getUses()) { + final PsiJavaCodeReferenceElement reference = statement.getClassReference(); + if (reference != null) { + final PsiElement usedInterface = reference.resolve(); + if (usedInterface instanceof PsiClass) { + final RefElement refClass = getRefManager().getReference(usedInterface); + if (refClass instanceof RefClass) { + if (myUsedServices == null) myUsedServices = new THashSet<>(); + myUsedServices.add((RefClass)refClass); + } + } + } + } + } + + @Override + public void buildReferences() { + PsiJavaModule javaModule = getElement(); + if (javaModule != null) { + buildRequiresReferences(javaModule); + buildExportsReferences(javaModule); + buildProvidesReferences(javaModule); + buildUsesReferences(javaModule); + getRefManager().fireBuildReferences(this); } } + /** + * For building references between modules + */ private PsiElement addReference(PsiPolyVariantReference reference) { List resolvedElements = new ArrayList<>(); if (reference != null) { diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/visibility/EntryPointWithVisibilityLevel.java b/java/java-analysis-impl/src/com/intellij/codeInspection/visibility/EntryPointWithVisibilityLevel.java index 36a1e0ed4298..b57721487944 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/visibility/EntryPointWithVisibilityLevel.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/visibility/EntryPointWithVisibilityLevel.java @@ -16,7 +16,7 @@ package com.intellij.codeInspection.visibility; import com.intellij.codeInspection.reference.EntryPoint; -import com.intellij.openapi.extensions.ExtensionPointName; +import com.intellij.codeInspection.reference.RefJavaElement; import com.intellij.psi.PsiMember; import com.intellij.psi.util.PsiUtil; @@ -39,4 +39,11 @@ public abstract class EntryPointWithVisibilityLevel extends EntryPoint { * Id to serialize checkbox state in visibility inspection settings */ public abstract String getId(); + + /** + * Don't suggest decreasing visibility for the element, sometimes even if the entry point is disabled. + */ + public boolean keepVisibilityLevel(boolean entryPointEnabled, @SuppressWarnings("unused") RefJavaElement refJavaElement) { + return false; + } } 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 b64802ac94b2..6865cf828374 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 @@ -23,6 +23,7 @@ import com.intellij.codeInsight.daemon.impl.IdentifierUtil; import com.intellij.codeInspection.*; import com.intellij.codeInspection.ex.EntryPointsManager; import com.intellij.codeInspection.reference.*; +import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.extensions.ExtensionPoint; import com.intellij.openapi.extensions.ExtensionPointName; @@ -36,10 +37,12 @@ import com.intellij.psi.util.PsiUtil; import com.intellij.ui.components.panels.VerticalBox; import com.intellij.usageView.UsageViewTypeLocation; import com.intellij.util.VisibilityUtil; +import one.util.streamex.StreamEx; import org.jdom.Element; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.TestOnly; import javax.swing.*; import java.awt.*; @@ -101,7 +104,7 @@ public class VisibilityInspection extends GlobalJavaBatchInspectionTool { if (entryPoint instanceof EntryPointWithVisibilityLevel) { gc.gridy++; final JCheckBox checkBox = new JCheckBox(((EntryPointWithVisibilityLevel)entryPoint).getTitle()); - checkBox.setSelected(myExtensions.getOrDefault(((EntryPointWithVisibilityLevel)entryPoint).getId(), true)); + checkBox.setSelected(isEntryPointEnabled((EntryPointWithVisibilityLevel)entryPoint)); checkBox.addActionListener(e -> myExtensions.put(((EntryPointWithVisibilityLevel)entryPoint).getId(), checkBox.isSelected())); add(checkBox, gc); } @@ -181,11 +184,14 @@ public class VisibilityInspection extends GlobalJavaBatchInspectionTool { //ignore anonymous classes. They do not have access modifiers. if (refElement instanceof RefClass) { RefClass refClass = (RefClass) refElement; - if (refClass.isAnonymous() || refClass.isServlet() || refClass.isApplet() || refClass.isLocalClass() || isExported(refClass)) { + if (refClass.isAnonymous() || refClass.isServlet() || refClass.isApplet() || refClass.isLocalClass()) { return null; } } + if (keepVisibilityLevel(refElement)) { + return null; + } //ignore unreferenced code. They could be a potential entry points. if (refElement.getInReferences().isEmpty()) { @@ -215,30 +221,10 @@ public class VisibilityInspection extends GlobalJavaBatchInspectionTool { return null; } - private static boolean isExported(RefClass refClass) { - RefModule refModule = refClass.getModule(); - if (refModule != null) { - RefJavaModule refJavaModule = RefJavaModule.JAVA_MODULE.get(refModule); - if (refJavaModule != null) { - Set serviceImplementations = refJavaModule.getServiceImplementations(); - if (serviceImplementations.contains(refClass)) { - return true; - } - - RefEntity refOwner = refClass; - while (refOwner instanceof RefClass) { - String modifier = ((RefClass)refOwner).getAccessModifier(); - refOwner = PsiModifier.PUBLIC.equals(modifier) || PsiModifier.PROTECTED.equals(modifier) ? refOwner.getOwner() : null; - } - if (refOwner instanceof RefPackage) { - Map> exportedPackageNames = refJavaModule.getExportedPackageNames(); - if (exportedPackageNames.containsKey(refOwner.getQualifiedName())) { - return true; - } - } - } - } - return false; + private boolean keepVisibilityLevel(RefJavaElement refElement) { + return StreamEx.of(ExtensionPointName.create(ToolExtensionPoints.DEAD_CODE_TOOL).getExtensions()) + .select(EntryPointWithVisibilityLevel.class) + .anyMatch(point -> point.keepVisibilityLevel(isEntryPointEnabled(point), refElement)); } @NotNull @@ -273,13 +259,17 @@ public class VisibilityInspection extends GlobalJavaBatchInspectionTool { } int getMinVisibilityLevel(PsiMember member) { - return Arrays.stream(ExtensionPointName.create(ToolExtensionPoints.DEAD_CODE_TOOL).getExtensions()) - .filter(point -> point instanceof EntryPointWithVisibilityLevel && - myExtensions.getOrDefault(((EntryPointWithVisibilityLevel)point).getId(), true)) - .mapToInt(point -> ((EntryPointWithVisibilityLevel)point).getMinVisibilityLevel(member)) + return StreamEx.of(ExtensionPointName.create(ToolExtensionPoints.DEAD_CODE_TOOL).getExtensions()) + .select(EntryPointWithVisibilityLevel.class) + .filter(point -> isEntryPointEnabled(point)) + .mapToInt(point -> point.getMinVisibilityLevel(member)) .max().orElse(-1); } + private boolean isEntryPointEnabled(EntryPointWithVisibilityLevel point) { + return myExtensions.getOrDefault(point.getId(), true); + } + private int getMinVisibilityLevel(RefJavaElement refElement) { PsiElement element = refElement.getElement(); if (element instanceof PsiMember) { @@ -627,6 +617,12 @@ public class VisibilityInspection extends GlobalJavaBatchInspectionTool { } } + @TestOnly + public void setEntryPointEnabled(@NotNull String entryPointId, boolean enabled) { + LOG.assertTrue(ApplicationManager.getApplication().isUnitTestMode()); + myExtensions.put(entryPointId, enabled); + } + private static class AcceptSuggestedAccess implements LocalQuickFix{ private final RefManager myManager; @PsiModifier.ModifierConstant private final String myHint; diff --git a/java/java-tests/testData/inspection/java9Visibility/reduceVisibilityInExportedPackages/Public.java b/java/java-tests/testData/inspection/java9Visibility/reduceVisibilityInExportedPackages/Public.java new file mode 100644 index 000000000000..69dda4039633 --- /dev/null +++ b/java/java-tests/testData/inspection/java9Visibility/reduceVisibilityInExportedPackages/Public.java @@ -0,0 +1,3 @@ +package foo.bar; + +public class Public {} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/java9Visibility/reduceVisibilityInExportedPackages/ServiceApi.java b/java/java-tests/testData/inspection/java9Visibility/reduceVisibilityInExportedPackages/ServiceApi.java new file mode 100644 index 000000000000..7fd656840fcc --- /dev/null +++ b/java/java-tests/testData/inspection/java9Visibility/reduceVisibilityInExportedPackages/ServiceApi.java @@ -0,0 +1,3 @@ +package foo.bar; + +public class ServiceApi {} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/java9Visibility/reduceVisibilityInExportedPackages/ServiceImpl.java b/java/java-tests/testData/inspection/java9Visibility/reduceVisibilityInExportedPackages/ServiceImpl.java new file mode 100644 index 000000000000..16a1a3626aec --- /dev/null +++ b/java/java-tests/testData/inspection/java9Visibility/reduceVisibilityInExportedPackages/ServiceImpl.java @@ -0,0 +1,3 @@ +package foo.bar; + +public class ServiceImpl extends ServiceApi {} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/java9Visibility/reduceVisibilityInExportedPackages/UsedService.java b/java/java-tests/testData/inspection/java9Visibility/reduceVisibilityInExportedPackages/UsedService.java new file mode 100644 index 000000000000..e737a4be4fd0 --- /dev/null +++ b/java/java-tests/testData/inspection/java9Visibility/reduceVisibilityInExportedPackages/UsedService.java @@ -0,0 +1,3 @@ +package foo.bar; + +public class UsedService {} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/java9Visibility/reduceVisibilityInExportedPackages/expected.xml b/java/java-tests/testData/inspection/java9Visibility/reduceVisibilityInExportedPackages/expected.xml new file mode 100644 index 000000000000..8e068179635c --- /dev/null +++ b/java/java-tests/testData/inspection/java9Visibility/reduceVisibilityInExportedPackages/expected.xml @@ -0,0 +1,10 @@ + + + + Public.java + 3 + + Declaration access can be weaker + Can be package-private + + \ No newline at end of file diff --git a/java/java-tests/testData/inspection/java9Visibility/usedService/Api.java b/java/java-tests/testData/inspection/java9Visibility/usedService/Api.java new file mode 100644 index 000000000000..417e6f7a08f1 --- /dev/null +++ b/java/java-tests/testData/inspection/java9Visibility/usedService/Api.java @@ -0,0 +1,3 @@ +package foo.bar; + +public class Api {} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/java9Visibility/usedService/Impl.java b/java/java-tests/testData/inspection/java9Visibility/usedService/Impl.java new file mode 100644 index 000000000000..a700ddc5ed2c --- /dev/null +++ b/java/java-tests/testData/inspection/java9Visibility/usedService/Impl.java @@ -0,0 +1,4 @@ +package foo.bar.impl; +import foo.bar Api; + +public class Impl extends Api {} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/java9Visibility/usedService/Other.java b/java/java-tests/testData/inspection/java9Visibility/usedService/Other.java new file mode 100644 index 000000000000..5e7f2d1881ce --- /dev/null +++ b/java/java-tests/testData/inspection/java9Visibility/usedService/Other.java @@ -0,0 +1,4 @@ +package foo.bar.impl; +import foo.bar Api; + +public class Other extends Api {} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/java9Visibility/usedService/expected.xml b/java/java-tests/testData/inspection/java9Visibility/usedService/expected.xml new file mode 100644 index 000000000000..161b0b37a534 --- /dev/null +++ b/java/java-tests/testData/inspection/java9Visibility/usedService/expected.xml @@ -0,0 +1,10 @@ + + + + Other.java + 4 + + Declaration access can be weaker + Can be package-private + + \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/Java9VisibilityTest.kt b/java/java-tests/testSrc/com/intellij/java/codeInspection/Java9VisibilityTest.kt index 3ff360804f72..ecd67d3e589d 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/Java9VisibilityTest.kt +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/Java9VisibilityTest.kt @@ -16,6 +16,7 @@ package com.intellij.java.codeInspection import com.intellij.codeInspection.ex.GlobalInspectionToolWrapper +import com.intellij.codeInspection.java19modules.Java9ModuleEntryPoint import com.intellij.codeInspection.visibility.VisibilityInspection import com.intellij.java.testFramework.fixtures.LightJava9ModulesCodeInsightFixtureTestCase import com.intellij.openapi.application.ex.PathManagerEx @@ -33,6 +34,21 @@ class Java9VisibilityTest : LightJava9ModulesCodeInsightFixtureTestCase() { fun testInheritedService() = doTestService() fun testProvidedService() = doTestService() + fun testUsedService() { + moduleInfo("module foo.bar { exports foo.bar; uses foo.bar.Api; uses foo.bar.impl.Impl; }") + doTest("foo.bar.Api", "foo.bar.impl.Impl", "foo.bar.impl.Other") + } + + fun testReduceVisibilityInExportedPackages() { + moduleInfo("""module foo.bar { + exports foo.bar; + provides foo.bar.ServiceApi with foo.bar.ServiceImpl; + uses foo.bar.UsedService; +}""") + doTest("foo.bar.Public", "foo.bar.ServiceApi", "foo.bar.ServiceImpl", "foo.bar.UsedService", + reduceVisibilityInExportedPackages = true) + } + private fun doTestClass() { moduleInfo("module foo.bar { exports foo.bar; }") doTest("foo.bar.Api", "foo.bar.impl.Impl") @@ -43,11 +59,14 @@ class Java9VisibilityTest : LightJava9ModulesCodeInsightFixtureTestCase() { doTest("foo.bar.Api", "foo.bar.impl.Impl", "foo.bar.impl.Other") } - private fun doTest(vararg classNames: String) { + private fun doTest(vararg classNames: String, reduceVisibilityInExportedPackages: Boolean = false) { val testPath = testDataPath + getTestName(true) addJavaFiles(testPath, classNames) - val toolWrapper = GlobalInspectionToolWrapper(VisibilityInspection()) + val inspection = VisibilityInspection() + inspection.setEntryPointEnabled(Java9ModuleEntryPoint.ID, reduceVisibilityInExportedPackages) + + val toolWrapper = GlobalInspectionToolWrapper(inspection) doGlobalInspectionTest(testPath, toolWrapper) } }