[java] report access to package-private members declared in production code from tests (IDEA-372858)

GitOrigin-RevId: 6f046186e0f0a587e1aaab31dd55b9f76a28db96
This commit is contained in:
Nikolay Chashnikov
2025-05-19 10:03:24 +00:00
committed by intellij-monorepo-bot
parent 4e72b9064c
commit d039e59e88
7 changed files with 80 additions and 15 deletions
@@ -2437,6 +2437,7 @@ extend.exception.fix.family.name=Make class extend 'Exception'
inspection.use.of.private.field.inner.classes.option=Ignore accesses from inner classes
refused.bequest.fix.family.name=Insert call to super method
inspection.suspicious.package.private.access.description={0} is {1}, but declared in a different module ''{2}''
inspection.suspicious.package.private.access.from.tests.description={0} is {1} and used in tests, but declared in the production source
replace.case.default.with.default=Replace 'case default' with 'default'
replace.case.default.null.with.null.default=Replace 'case default, null' with 'case null, default'
create.default.branch.fix.family.name=Create 'default' branch
@@ -2497,6 +2498,7 @@ inspection.test.method.without.assertions.exceptions.option=Ignore test methods
inspection.collection.must.have.initial.capacity.initializers.option=Don't report field initializers
utility.class.without.private.constructor.cant.generate.constructor.message=Utility class has instantiations, private constructor will not be created
inspection.suspicious.package.private.access.problem={0} overrides a package-private method from {1} which is declared in a different module ''{2}''
inspection.suspicious.package.private.access.from.tests.problem={0} in tests overrides a package-private method from {1} which is declared in the production source
inspection.condition.covered.by.further.condition.descr=Condition ''{0}'' covered by subsequent {1, choice, 1#condition ''''{2}''''|2#conditions}
create.missing.switch.branch=Create missing branch {0}
create.missing.switch.branches=Create missing branches {0}
@@ -1,6 +1,7 @@
<html>
<body>
Reports usages of package-private members declared in the same package but in a different module.
Reports usages of package-private members declared in the same package but in a different module, and usages of package-private members
declared in production from tests.
<p>If the declaring classes are loaded by different loaders, the code that accesses a package-private member will fail with
<code>IllegalAccessError</code> at runtime.</p>
<p>If a method overrides a package-private method from a class that is loaded by a different loader, it won't be invoked when the super
@@ -29,6 +29,7 @@ import com.intellij.util.xmlb.annotations.Tag;
import com.intellij.util.xmlb.annotations.XCollection;
import com.siyeh.InspectionGadgetsBundle;
import com.siyeh.ig.psiutils.ClassUtils;
import com.siyeh.ig.psiutils.TestUtils;
import one.util.streamex.StreamEx;
import org.jdom.Element;
import org.jetbrains.annotations.NotNull;
@@ -133,9 +134,8 @@ public final class SuspiciousPackagePrivateAccessInspection extends AbstractBase
private void checkPackageLocalAccess(@NotNull UElement sourceNode, PsiJvmMember targetElement, final String accessType) {
PsiElement sourcePsi = sourceNode.getSourcePsi();
if (sourcePsi != null) {
Module targetModule = ModuleUtilCore.findModuleForPsiElement(targetElement);
Module sourceModule = ModuleUtilCore.findModuleForPsiElement(sourcePsi);
if (isPackageLocalAccessSuspicious(sourceModule, targetModule) &&
SuspiciousPackagePrivateAccess suspiciousAccess = verifyPackagePrivateAccess(sourcePsi, targetElement);
if (suspiciousAccess != null &&
PsiTreeUtil.getParentOfType(sourcePsi, PsiComment.class) == null) {
List<IntentionAction> fixes =
JvmElementActionFactories.createModifierActions(targetElement, MemberRequestsKt.modifierRequest(JvmModifier.PUBLIC, true));
@@ -143,8 +143,16 @@ public final class SuspiciousPackagePrivateAccessInspection extends AbstractBase
StringUtil.removeHtmlTags(StringUtil.capitalize(RefactoringUIUtil.getDescription(targetElement, true)));
LocalQuickFix[] quickFixes =
IntentionWrapper.wrapToQuickFixes(fixes.toArray(IntentionAction.EMPTY_ARRAY), targetElement.getContainingFile());
myProblemsHolder.registerProblem(sourcePsi, InspectionGadgetsBundle.message("inspection.suspicious.package.private.access.description", elementDescription, accessType, targetModule.getName()),
quickFixes);
String message;
if (suspiciousAccess.accessedFromTests) {
message = InspectionGadgetsBundle.message("inspection.suspicious.package.private.access.from.tests.description",
elementDescription, accessType);
}
else {
message = InspectionGadgetsBundle.message("inspection.suspicious.package.private.access.description", elementDescription,
accessType, suspiciousAccess.targetModule.getName());
}
myProblemsHolder.registerProblem(sourcePsi, message, quickFixes);
}
}
}
@@ -153,9 +161,8 @@ public final class SuspiciousPackagePrivateAccessInspection extends AbstractBase
PsiElement sourcePsi = sourceNode.getSourcePsi();
PsiElement nameIdentifier = UElementKt.getSourcePsiElement(sourceNode.getUastAnchor());
if (sourcePsi != null && nameIdentifier != null && targetElement.hasModifier(JvmModifier.PACKAGE_LOCAL)) {
Module targetModule = ModuleUtilCore.findModuleForPsiElement(targetElement);
Module sourceModule = ModuleUtilCore.findModuleForPsiElement(sourcePsi);
if (isPackageLocalAccessSuspicious(sourceModule, targetModule)) {
SuspiciousPackagePrivateAccess accessResult = verifyPackagePrivateAccess(sourcePsi, targetElement);
if (accessResult != null) {
List<IntentionAction> fixes =
JvmElementActionFactories.createModifierActions(targetElement, MemberRequestsKt.modifierRequest(JvmModifier.PUBLIC, true));
String elementDescription =
@@ -164,21 +171,41 @@ public final class SuspiciousPackagePrivateAccessInspection extends AbstractBase
StringUtil.removeHtmlTags(RefactoringUIUtil.getDescription(targetElement.getParent(), false));
LocalQuickFix[] quickFixes =
IntentionWrapper.wrapToQuickFixes(fixes.toArray(IntentionAction.EMPTY_ARRAY), targetElement.getContainingFile());
String problem = InspectionGadgetsBundle
.message("inspection.suspicious.package.private.access.problem", elementDescription, classDescription, targetModule.getName());
String problem;
if (accessResult.accessedFromTests) {
problem = InspectionGadgetsBundle.message("inspection.suspicious.package.private.access.from.tests.problem", elementDescription,
classDescription);
}
else {
problem = InspectionGadgetsBundle.message("inspection.suspicious.package.private.access.problem", elementDescription,
classDescription, accessResult.targetModule.getName());
}
myProblemsHolder.registerProblem(nameIdentifier, problem, quickFixes);
}
}
}
private boolean isPackageLocalAccessSuspicious(Module sourceModule, Module targetModule) {
if (targetModule == null || sourceModule == null || targetModule.equals(sourceModule)) {
return false;
private @Nullable SuspiciousPackagePrivateAccess verifyPackagePrivateAccess(@NotNull PsiElement sourceElement, @NotNull PsiElement targetElement) {
Module targetModule = ModuleUtilCore.findModuleForPsiElement(targetElement);
Module sourceModule = ModuleUtilCore.findModuleForPsiElement(sourceElement);
if (targetModule == null || sourceModule == null) {
return null;
}
if (targetModule.equals(sourceModule)) {
if (TestUtils.isInTestSourceContent(sourceElement) && !TestUtils.isInTestSourceContent(targetElement)) {
return new SuspiciousPackagePrivateAccess(targetModule, true);
}
return null;
}
ModulesSet sourceGroup = myModuleNameToModulesSet.get(sourceModule.getName());
ModulesSet targetGroup = myModuleNameToModulesSet.get(targetModule.getName());
return sourceGroup == null || sourceGroup != targetGroup;
if (sourceGroup == null || sourceGroup != targetGroup) {
return new SuspiciousPackagePrivateAccess(targetModule, false);
}
return null;
}
private record SuspiciousPackagePrivateAccess(Module targetModule, boolean accessedFromTests) {}
}
private static boolean canAccessProtectedMember(UElement sourceNode, PsiMember member, PsiClass accessObjectType) {
@@ -0,0 +1,3 @@
package xxx;
class AnotherPackagePrivateClass {}
@@ -0,0 +1,15 @@
package xxx;
public class PackagePrivateClassTest extends PublicClass {
public void test1() {
new <warning descr="Class xxx.PackagePrivateClass is package-private and used in tests, but declared in the production source">PackagePrivateClass</warning>();
}
public void test2() {
new AnotherPackagePrivateClass();
}
@Override
public void <warning descr="Method packagePrivateMethod() in tests overrides a package-private method from class xxx.PublicClass which is declared in the production source">packagePrivateMethod</warning>(){
}
}
@@ -4,6 +4,12 @@ package com.siyeh.ig.dependency;
public class SuspiciousPackagePrivateAccessInspectionTest extends SuspiciousPackagePrivateAccessInspectionTestCase {
public SuspiciousPackagePrivateAccessInspectionTest() {super("java");}
@Override
protected void setUp() throws Exception {
super.setUp();
myFixture.copyDirectoryToProject("depTests", "../depTests");
}
public void testAccessingPackagePrivateMembers() {
doTestWithDependency();
}
@@ -23,4 +29,9 @@ public class SuspiciousPackagePrivateAccessInspectionTest extends SuspiciousPack
public void testOverridePackagePrivateMethod() {
doTestWithDependency();
}
public void testPackagePrivateClassTest() {
myFixture.configureByFile("../depTests/xxx/PackagePrivateClassTest.java");
myFixture.testHighlighting(true, false, false);
}
}
@@ -67,6 +67,7 @@ public abstract class SuspiciousPackagePrivateAccessInspectionTestCase extends L
private static class ProjectWithDepModuleDescriptor extends ProjectDescriptor {
private static final String DEP_MODULE_SOURCE_ROOT = "dep-module-src";
private VirtualFile mySourceRoot;
private VirtualFile myTestSourceRoot;
ProjectWithDepModuleDescriptor(@NotNull LanguageLevel languageLevel) {
super(languageLevel);
@@ -84,6 +85,8 @@ public abstract class SuspiciousPackagePrivateAccessInspectionTestCase extends L
model.setSdk(getSdk());
mySourceRoot = createSourceRoot(depModule, DEP_MODULE_SOURCE_ROOT);
model.addContentEntry(mySourceRoot).addSourceFolder(mySourceRoot, JavaSourceRootType.SOURCE);
myTestSourceRoot = createSourceRoot(depModule, "depTests");
model.addContentEntry(myTestSourceRoot).addSourceFolder(myTestSourceRoot, JavaSourceRootType.TEST_SOURCE);
});
ModuleRootModificationUtil.addDependency(mainModule, depModule);
});
@@ -93,6 +96,9 @@ public abstract class SuspiciousPackagePrivateAccessInspectionTestCase extends L
if (mySourceRoot != null) {
WriteAction.run(() -> mySourceRoot.delete(this));
}
if (myTestSourceRoot != null) {
WriteAction.run(() -> myTestSourceRoot.delete(this));
}
}
@NotNull