From ab596f3b7b39b1e07dc8830ff18555f78872f679 Mon Sep 17 00:00:00 2001 From: Jan Lahoda Date: Fri, 22 Aug 2025 10:33:17 +0200 Subject: [PATCH] Private members are never inherited, and package-private members are only sometimes inherited, the Java import analysis needs to account for that. --- .../java/source/pretty/ImportAnalysis2.java | 10 +- .../java/source/save/ElementOverlay.java | 30 +- .../java/source/gen/ImportAnalysis2Test.java | 348 ++++++++++++++++++ 3 files changed, 375 insertions(+), 13 deletions(-) diff --git a/java/java.source.base/src/org/netbeans/modules/java/source/pretty/ImportAnalysis2.java b/java/java.source.base/src/org/netbeans/modules/java/source/pretty/ImportAnalysis2.java index 1119a4c9714a..a9d8589cd67f 100644 --- a/java/java.source.base/src/org/netbeans/modules/java/source/pretty/ImportAnalysis2.java +++ b/java/java.source.base/src/org/netbeans/modules/java/source/pretty/ImportAnalysis2.java @@ -187,7 +187,7 @@ public void classEntered(ClassTree clazz, boolean isAnonymous) { public void enterVisibleThroughClasses(ClassTree clazz) { Set visible = visibleThroughClasses.getFirst(); - visible.addAll(overlay.getAllVisibleThrough(model, elements, currentFQN.getFQN(), clazz, modle)); + visible.addAll(overlay.getAllVisibleThrough(model, elements, currentFQN.getFQN(), clazz, pack, modle)); } public void classLeft() { @@ -367,7 +367,7 @@ public ExpressionTree resolveImport(MemberSelectTree orig, final Element element return make.Identifier(element.getSimpleName()); } - if (getPackageOf(element) != null && getPackageOf(element).isUnnamed()) { + if (overlay.getPackageOf(element) != null && overlay.getPackageOf(element).isUnnamed()) { if (orig.getExpression().getKind() == Kind.MEMBER_SELECT) { return make.MemberSelect(resolveImport((MemberSelectTree) orig.getExpression(), element.getEnclosingElement()), element.getSimpleName()); @@ -452,12 +452,6 @@ private boolean checkPackagesForStarImport(String pkgName, CodeStyle cs) { return false; } - private PackageElement getPackageOf(Element el) { - while ((el != null) && (el.getKind() != ElementKind.PACKAGE)) el = el.getEnclosingElement(); - - return (PackageElement) el; - } - private Map getUsedImplicitlyImportedClasses() { if (usedImplicitlyImportedClassesCache != null) { return usedImplicitlyImportedClassesCache; diff --git a/java/java.source.base/src/org/netbeans/modules/java/source/save/ElementOverlay.java b/java/java.source.base/src/org/netbeans/modules/java/source/save/ElementOverlay.java index 3c6144151f98..2d2ffa96be07 100644 --- a/java/java.source.base/src/org/netbeans/modules/java/source/save/ElementOverlay.java +++ b/java/java.source.base/src/org/netbeans/modules/java/source/save/ElementOverlay.java @@ -389,7 +389,7 @@ public int totalMapsSize() { this.packages.size(); } - public Collection getAllVisibleThrough(ASTService ast, Elements elements, String what, ClassTree tree, ModuleElement modle) { + public Collection getAllVisibleThrough(ASTService ast, Elements elements, String what, ClassTree tree, Element currentPackage, ModuleElement modle) { Collection result = new ArrayList(); Element current = what != null ? resolve(ast, elements, what, modle) : null; @@ -408,30 +408,50 @@ public Collection getAllVisibleThrough(ASTService ast, Elemen } } } else { - result.addAll(getAllMembers(ast, elements, current)); + result.addAll(getAllMembers(ast, elements, current, currentPackage, false)); } return result; } - private Collection getAllMembers(ASTService ast, Elements elements, Element el) { + private Collection getAllMembers(ASTService ast, Elements elements, Element el, Element currentPackage, boolean inherited) { List result = new ArrayList(); - result.addAll(el.getEnclosedElements()); + el.getEnclosedElements() + .stream() + .filter(encl -> !inherited || //include all members from the current class + encl.getModifiers().contains(Modifier.PUBLIC) || //public members are inherited + encl.getModifiers().contains(Modifier.PROTECTED) || //protected memebers are inherited + (!encl.getModifiers().contains(Modifier.PRIVATE) && //private member are never inherited + resolvedPackageOf(ast, elements, encl) == currentPackage)) //package-private members are inherited only inside the same package + .forEach(result::add); for (Element parent : getAllSuperElements(ast, elements, el)) { if (!el.equals(parent)) { - result.addAll(getAllMembers(ast, elements, parent)); + result.addAll(getAllMembers(ast, elements, parent, currentPackage, true)); } } return result; } + private Element resolvedPackageOf(ASTService ast, Elements elements, Element el) { + ModuleElement modle = moduleOf(elements, el); + PackageElement pack = getPackageOf(el); + + return resolve(ast, elements, pack.getQualifiedName().toString(), modle); + } + public PackageElement unnamedPackage(ASTService ast, Elements elements, ModuleElement modle) { return (PackageElement) resolve(ast, elements, "", modle); } + public PackageElement getPackageOf(Element el) { + while ((el != null) && (el.getKind() != ElementKind.PACKAGE)) el = el.getEnclosingElement(); + + return (PackageElement) el; + } + private ModuleElement moduleOf(Elements elements, Element el) { if (el instanceof TypeElementWrapper) return moduleOf(elements, ((TypeElementWrapper) el).delegateTo); diff --git a/java/java.source.base/test/unit/src/org/netbeans/api/java/source/gen/ImportAnalysis2Test.java b/java/java.source.base/test/unit/src/org/netbeans/api/java/source/gen/ImportAnalysis2Test.java index 3671dbc99479..9bd4c9a6ecbf 100644 --- a/java/java.source.base/test/unit/src/org/netbeans/api/java/source/gen/ImportAnalysis2Test.java +++ b/java/java.source.base/test/unit/src/org/netbeans/api/java/source/gen/ImportAnalysis2Test.java @@ -1577,6 +1577,354 @@ public void run(WorkingCopy workingCopy) throws IOException { assertEquals(golden, res); } + public void testVisibleThroughClassesPrivateNotInherited() throws Exception { + beginTx(); + File testA = new File(getWorkDir(), "api/A.java"); + assertTrue(testA.getParentFile().mkdirs()); + TestUtilities.copyStringToFile(testA, + """ + package api; + public class A { + public static void test() {} + public static void test(int i) {} + } + """ + ); + testFile = new File(getWorkDir(), "hierbas/del/litoral/Test.java"); + assertTrue(testFile.getParentFile().mkdirs()); + TestUtilities.copyStringToFile(testFile, + """ + package hierbas.del.litoral; + import static api.A.*; + public class Test extends Base { + public static void t() { + } + } + class Base { + private static void test() {} + private static void test(int i) {} + } + """ + ); + String golden = + """ + package hierbas.del.litoral; + import static api.A.*; + public class Test extends Base { + public static void t() { + test(); + test(1); + } + } + class Base { + private static void test() {} + private static void test(int i) {} + } + """; + + ClasspathInfo cpInfo = ClasspathInfoAccessor.getINSTANCE().create (BootClassPathUtil.getBootClassPath(), ClassPath.EMPTY, ClassPath.EMPTY, ClassPath.EMPTY, ClassPath.EMPTY, ClassPathSupport.createClassPath(getSourcePath()), ClassPath.EMPTY, null, true, false, false, true, false, null); + JavaSource src = JavaSource.create(cpInfo, FileUtil.toFileObject(testFile)); + Task task = new Task() { + + public void run(WorkingCopy workingCopy) throws IOException { + workingCopy.toPhase(Phase.RESOLVED); + TreeMaker make = workingCopy.getTreeMaker(); + ClassTree clazz = (ClassTree) workingCopy.getCompilationUnit().getTypeDecls().get(0); + MethodTree testMethodDeclaration = (MethodTree) clazz.getMembers().get(1); + TypeElement aClass = workingCopy.getElements().getTypeElement("api.A"); + ExecutableElement testMethod = aClass.getEnclosedElements().stream().filter(el -> el.getKind() == ElementKind.METHOD).map(el -> ((ExecutableElement) el)).filter(el -> el.getSimpleName().contentEquals("test") && el.getParameters().isEmpty()).findAny().orElseThrow(); + MethodInvocationTree invocation1 = make.MethodInvocation(List.of(), make.QualIdent(testMethod), List.of()); + ExecutableElement testMethod2 = aClass.getEnclosedElements().stream().filter(el -> el.getKind() == ElementKind.METHOD).map(el -> ((ExecutableElement) el)).filter(el -> el.getSimpleName().contentEquals("test") && el.getParameters().size() == 1).findAny().orElseThrow(); + MethodInvocationTree invocation2 = make.MethodInvocation(List.of(), make.QualIdent(testMethod2), List.of(make.Literal(1))); + workingCopy.rewrite(testMethodDeclaration.getBody(), make.addBlockStatement(make.addBlockStatement(testMethodDeclaration.getBody(), make.ExpressionStatement(invocation1)), make.ExpressionStatement(invocation2))); + } + + }; + src.runModificationTask(task).commit(); + String res = TestUtilities.copyFileToString(testFile); + //System.err.println(res); + assertEquals(golden, res); + } + + public void testVisibleThroughClassesPackageNotInherited() throws Exception { + beginTx(); + File testA = new File(getWorkDir(), "api/A.java"); + assertTrue(testA.getParentFile().mkdirs()); + TestUtilities.copyStringToFile(testA, + """ + package api; + public class A { + public static void test() {} + public static void test(int i) {} + } + """ + ); + File testB = new File(getWorkDir(), "api/B.java"); + TestUtilities.copyStringToFile(testB, + """ + package api; + public class B { + static void test() {} + static void test(int i) {} + } + """ + ); + testFile = new File(getWorkDir(), "hierbas/del/litoral/Test.java"); + assertTrue(testFile.getParentFile().mkdirs()); + TestUtilities.copyStringToFile(testFile, + """ + package hierbas.del.litoral; + import static api.A.*; + public class Test extends api.B { + public static void t() { + } + } + """ + ); + String golden = + """ + package hierbas.del.litoral; + import static api.A.*; + public class Test extends api.B { + public static void t() { + test(); + test(1); + } + } + """; + + ClasspathInfo cpInfo = ClasspathInfoAccessor.getINSTANCE().create (BootClassPathUtil.getBootClassPath(), ClassPath.EMPTY, ClassPath.EMPTY, ClassPath.EMPTY, ClassPath.EMPTY, ClassPathSupport.createClassPath(getSourcePath()), ClassPath.EMPTY, null, true, false, false, true, false, null); + JavaSource src = JavaSource.create(cpInfo, FileUtil.toFileObject(testFile)); + Task task = new Task() { + + public void run(WorkingCopy workingCopy) throws IOException { + workingCopy.toPhase(Phase.RESOLVED); + TreeMaker make = workingCopy.getTreeMaker(); + ClassTree clazz = (ClassTree) workingCopy.getCompilationUnit().getTypeDecls().get(0); + MethodTree testMethodDeclaration = (MethodTree) clazz.getMembers().get(1); + TypeElement aClass = workingCopy.getElements().getTypeElement("api.A"); + ExecutableElement testMethod = aClass.getEnclosedElements().stream().filter(el -> el.getKind() == ElementKind.METHOD).map(el -> ((ExecutableElement) el)).filter(el -> el.getSimpleName().contentEquals("test") && el.getParameters().isEmpty()).findAny().orElseThrow(); + MethodInvocationTree invocation1 = make.MethodInvocation(List.of(), make.QualIdent(testMethod), List.of()); + ExecutableElement testMethod2 = aClass.getEnclosedElements().stream().filter(el -> el.getKind() == ElementKind.METHOD).map(el -> ((ExecutableElement) el)).filter(el -> el.getSimpleName().contentEquals("test") && el.getParameters().size() == 1).findAny().orElseThrow(); + MethodInvocationTree invocation2 = make.MethodInvocation(List.of(), make.QualIdent(testMethod2), List.of(make.Literal(1))); + workingCopy.rewrite(testMethodDeclaration.getBody(), make.addBlockStatement(make.addBlockStatement(testMethodDeclaration.getBody(), make.ExpressionStatement(invocation1)), make.ExpressionStatement(invocation2))); + } + + }; + src.runModificationTask(task).commit(); + String res = TestUtilities.copyFileToString(testFile); + //System.err.println(res); + assertEquals(golden, res); + } + + public void testVisibleThroughClassesPackageInherited() throws Exception { + beginTx(); + File testA = new File(getWorkDir(), "api/A.java"); + assertTrue(testA.getParentFile().mkdirs()); + TestUtilities.copyStringToFile(testA, + """ + package api; + public class A { + public static void test() {} + public static void test(int i) {} + } + """ + ); + testFile = new File(getWorkDir(), "hierbas/del/litoral/Test.java"); + assertTrue(testFile.getParentFile().mkdirs()); + TestUtilities.copyStringToFile(testFile, + """ + package hierbas.del.litoral; + import static api.A.*; + public class Test extends Base { + public static void t() { + } + } + class Base { + static void test() {} + static void test(int i) {} + } + """ + ); + String golden = + """ + package hierbas.del.litoral; + import api.A; + import static api.A.*; + public class Test extends Base { + public static void t() { + A.test(); + A.test(1); + } + } + class Base { + static void test() {} + static void test(int i) {} + } + """; + + ClasspathInfo cpInfo = ClasspathInfoAccessor.getINSTANCE().create (BootClassPathUtil.getBootClassPath(), ClassPath.EMPTY, ClassPath.EMPTY, ClassPath.EMPTY, ClassPath.EMPTY, ClassPathSupport.createClassPath(getSourcePath()), ClassPath.EMPTY, null, true, false, false, true, false, null); + JavaSource src = JavaSource.create(cpInfo, FileUtil.toFileObject(testFile)); + Task task = new Task() { + + public void run(WorkingCopy workingCopy) throws IOException { + workingCopy.toPhase(Phase.RESOLVED); + TreeMaker make = workingCopy.getTreeMaker(); + ClassTree clazz = (ClassTree) workingCopy.getCompilationUnit().getTypeDecls().get(0); + MethodTree testMethodDeclaration = (MethodTree) clazz.getMembers().get(1); + TypeElement aClass = workingCopy.getElements().getTypeElement("api.A"); + ExecutableElement testMethod = aClass.getEnclosedElements().stream().filter(el -> el.getKind() == ElementKind.METHOD).map(el -> ((ExecutableElement) el)).filter(el -> el.getSimpleName().contentEquals("test") && el.getParameters().isEmpty()).findAny().orElseThrow(); + MethodInvocationTree invocation1 = make.MethodInvocation(List.of(), make.QualIdent(testMethod), List.of()); + ExecutableElement testMethod2 = aClass.getEnclosedElements().stream().filter(el -> el.getKind() == ElementKind.METHOD).map(el -> ((ExecutableElement) el)).filter(el -> el.getSimpleName().contentEquals("test") && el.getParameters().size() == 1).findAny().orElseThrow(); + MethodInvocationTree invocation2 = make.MethodInvocation(List.of(), make.QualIdent(testMethod2), List.of(make.Literal(1))); + workingCopy.rewrite(testMethodDeclaration.getBody(), make.addBlockStatement(make.addBlockStatement(testMethodDeclaration.getBody(), make.ExpressionStatement(invocation1)), make.ExpressionStatement(invocation2))); + } + + }; + src.runModificationTask(task).commit(); + String res = TestUtilities.copyFileToString(testFile); + //System.err.println(res); + assertEquals(golden, res); + } + + public void testVisibleThroughClassesProtectedPublicInherited() throws Exception { + beginTx(); + File testA = new File(getWorkDir(), "api/A.java"); + assertTrue(testA.getParentFile().mkdirs()); + TestUtilities.copyStringToFile(testA, + """ + package api; + public class A { + public static void test() {} + public static void test(int i) {} + } + """ + ); + File testB = new File(getWorkDir(), "api/B.java"); + TestUtilities.copyStringToFile(testB, + """ + package api; + public class B { + protected static void test() {} + public static void test(int i) {} + } + """ + ); + testFile = new File(getWorkDir(), "hierbas/del/litoral/Test.java"); + assertTrue(testFile.getParentFile().mkdirs()); + TestUtilities.copyStringToFile(testFile, + """ + package hierbas.del.litoral; + import static api.A.*; + public class Test extends api.B { + public static void t() { + } + } + """ + ); + String golden = + """ + package hierbas.del.litoral; + import api.A; + import static api.A.*; + public class Test extends api.B { + public static void t() { + A.test(); + A.test(1); + } + } + """; + + ClasspathInfo cpInfo = ClasspathInfoAccessor.getINSTANCE().create (BootClassPathUtil.getBootClassPath(), ClassPath.EMPTY, ClassPath.EMPTY, ClassPath.EMPTY, ClassPath.EMPTY, ClassPathSupport.createClassPath(getSourcePath()), ClassPath.EMPTY, null, true, false, false, true, false, null); + JavaSource src = JavaSource.create(cpInfo, FileUtil.toFileObject(testFile)); + Task task = new Task() { + + public void run(WorkingCopy workingCopy) throws IOException { + workingCopy.toPhase(Phase.RESOLVED); + TreeMaker make = workingCopy.getTreeMaker(); + ClassTree clazz = (ClassTree) workingCopy.getCompilationUnit().getTypeDecls().get(0); + MethodTree testMethodDeclaration = (MethodTree) clazz.getMembers().get(1); + TypeElement aClass = workingCopy.getElements().getTypeElement("api.A"); + ExecutableElement testMethod = aClass.getEnclosedElements().stream().filter(el -> el.getKind() == ElementKind.METHOD).map(el -> ((ExecutableElement) el)).filter(el -> el.getSimpleName().contentEquals("test") && el.getParameters().isEmpty()).findAny().orElseThrow(); + MethodInvocationTree invocation1 = make.MethodInvocation(List.of(), make.QualIdent(testMethod), List.of()); + ExecutableElement testMethod2 = aClass.getEnclosedElements().stream().filter(el -> el.getKind() == ElementKind.METHOD).map(el -> ((ExecutableElement) el)).filter(el -> el.getSimpleName().contentEquals("test") && el.getParameters().size() == 1).findAny().orElseThrow(); + MethodInvocationTree invocation2 = make.MethodInvocation(List.of(), make.QualIdent(testMethod2), List.of(make.Literal(1))); + workingCopy.rewrite(testMethodDeclaration.getBody(), make.addBlockStatement(make.addBlockStatement(testMethodDeclaration.getBody(), make.ExpressionStatement(invocation1)), make.ExpressionStatement(invocation2))); + } + + }; + src.runModificationTask(task).commit(); + String res = TestUtilities.copyFileToString(testFile); + //System.err.println(res); + assertEquals(golden, res); + } + + public void testVisibleThroughClassesCurrentClassPrivate() throws Exception { + beginTx(); + File testA = new File(getWorkDir(), "api/A.java"); + assertTrue(testA.getParentFile().mkdirs()); + TestUtilities.copyStringToFile(testA, + """ + package api; + public class A { + public static void test() {} + public static void test(int i) {} + } + """ + ); + testFile = new File(getWorkDir(), "hierbas/del/litoral/Test.java"); + assertTrue(testFile.getParentFile().mkdirs()); + TestUtilities.copyStringToFile(testFile, + """ + package hierbas.del.litoral; + import static api.A.*; + public class Test extends api.B { + public static void t() { + } + private static void test() {} + private static void test(int i) {} + } + """ + ); + String golden = + """ + package hierbas.del.litoral; + import api.A; + import static api.A.*; + public class Test extends api.B { + public static void t() { + A.test(); + A.test(1); + } + private static void test() {} + private static void test(int i) {} + } + """; + + ClasspathInfo cpInfo = ClasspathInfoAccessor.getINSTANCE().create (BootClassPathUtil.getBootClassPath(), ClassPath.EMPTY, ClassPath.EMPTY, ClassPath.EMPTY, ClassPath.EMPTY, ClassPathSupport.createClassPath(getSourcePath()), ClassPath.EMPTY, null, true, false, false, true, false, null); + JavaSource src = JavaSource.create(cpInfo, FileUtil.toFileObject(testFile)); + Task task = new Task() { + + public void run(WorkingCopy workingCopy) throws IOException { + workingCopy.toPhase(Phase.RESOLVED); + TreeMaker make = workingCopy.getTreeMaker(); + ClassTree clazz = (ClassTree) workingCopy.getCompilationUnit().getTypeDecls().get(0); + MethodTree testMethodDeclaration = (MethodTree) clazz.getMembers().get(1); + TypeElement aClass = workingCopy.getElements().getTypeElement("api.A"); + ExecutableElement testMethod = aClass.getEnclosedElements().stream().filter(el -> el.getKind() == ElementKind.METHOD).map(el -> ((ExecutableElement) el)).filter(el -> el.getSimpleName().contentEquals("test") && el.getParameters().isEmpty()).findAny().orElseThrow(); + MethodInvocationTree invocation1 = make.MethodInvocation(List.of(), make.QualIdent(testMethod), List.of()); + ExecutableElement testMethod2 = aClass.getEnclosedElements().stream().filter(el -> el.getKind() == ElementKind.METHOD).map(el -> ((ExecutableElement) el)).filter(el -> el.getSimpleName().contentEquals("test") && el.getParameters().size() == 1).findAny().orElseThrow(); + MethodInvocationTree invocation2 = make.MethodInvocation(List.of(), make.QualIdent(testMethod2), List.of(make.Literal(1))); + workingCopy.rewrite(testMethodDeclaration.getBody(), make.addBlockStatement(make.addBlockStatement(testMethodDeclaration.getBody(), make.ExpressionStatement(invocation1)), make.ExpressionStatement(invocation2))); + } + + }; + src.runModificationTask(task).commit(); + String res = TestUtilities.copyFileToString(testFile); + //System.err.println(res); + assertEquals(golden, res); + } + //TODO: non-public classes not imported by module imports! String getGoldenPckg() {