Skip to content

Commit a9f1fa5

Browse files
sid-srinisid-srini
committed
Addressing review comments
1 parent c32194e commit a9f1fa5

File tree

4 files changed

+214
-72
lines changed

4 files changed

+214
-72
lines changed

‎java/java.hints/src/org/netbeans/modules/java/hints/OrganizeImports.java‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -233,12 +233,8 @@ public int compare(ImportTree o1, ImportTree o2) {
233233
}
234234
CompilationUnitTree cut = maker.CompilationUnit(cu.getPackageAnnotations(), cu.getPackageName(), imps, cu.getTypeDecls(), cu.getSourceFile());
235235
((JCCompilationUnit)cut).packge = ((JCCompilationUnit)cu).packge;
236-
if (!starImports.isEmpty() || !staticStarImports.isEmpty()) {
237-
((JCCompilationUnit)cut).starImportScope = ((JCCompilationUnit)cu).starImportScope;
238-
}
239-
if (!moduleImports.isEmpty()) {
240-
((JCCompilationUnit)cut).moduleImportScope = ((JCCompilationUnit)cu).moduleImportScope;
241-
}
236+
((JCCompilationUnit)cut).starImportScope = ((JCCompilationUnit)cu).starImportScope;
237+
((JCCompilationUnit)cut).moduleImportScope = ((JCCompilationUnit)cu).moduleImportScope;
242238
CompilationUnitTree ncu = toImport.isEmpty() ? cut : GeneratorUtilities.get(copy).addImports(cut, toImport);
243239
copy.rewrite(cu, ncu);
244240
}

‎java/java.source.base/src/org/netbeans/api/java/source/GeneratorUtilities.java‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1315,7 +1315,7 @@ private CompilationUnitTree addImports(CompilationUnitTree cut, List<? extends I
13151315
// Add modulesToImport to the importScope, to check name clashes, if any
13161316
if (!modulesToImport.isEmpty()) {
13171317
for (ModuleElement m : modulesToImport) {
1318-
for (Element e : m.getEnclosedElements()) {
1318+
for (PackageElement e : elementUtilities.transitivelyExportedPackages(m)) {
13191319
if (e instanceof Symbol p) {
13201320
importScope.appendSubScope(p.members());
13211321
}
@@ -1412,10 +1412,19 @@ private CompilationUnitTree addImports(CompilationUnitTree cut, List<? extends I
14121412
el = currentToImportElement.getEnclosingElement();
14131413
break;
14141414
}
1415-
final int cnt;
1415+
final int cnt; // Count of imports that may replace the currentToImportElement
14161416
if (explicitNamedImports.contains(currentToImportElement)) {
1417-
cnt = 0;
1417+
cnt = 0; // Explicit named imports must not be replaced by another
14181418
} else {
1419+
/// Check if the import is replaced by a star import or module import
1420+
///
1421+
/// Logic:
1422+
/// - This refers to the count of imports of the enclosing element, if any.
1423+
/// - A replacing star import or module import is indicated by a negative count.
1424+
/// - If the replacement has already been included in the imports, the count is set as -2.
1425+
/// - If the replacement is yet to be included in the imports, the count is set as -1.
1426+
/// - Otherwise, a non-negative count indicates that the import is below the threshold for replacement.
1427+
14191428
Integer enclosingStarCnt = el == null ? null : isStatic ? typeCounts.get((TypeElement)el) : pkgCounts.get((PackageElement)el);
14201429
if (isStatic) {
14211430
cnt = enclosingStarCnt == null ? 0 : enclosingStarCnt;
@@ -1425,9 +1434,15 @@ private CompilationUnitTree addImports(CompilationUnitTree cut, List<? extends I
14251434
ModuleElement m = elements.getModuleOf(currentToImportElement);
14261435
Integer mc = m == null ? null : modCounts.get(m);
14271436
if (mc != null && mc < 0) {
1428-
// No need to add an import unless a self import
1429-
modCount = mc == -1 && m == currentToImportElement ? -1 : -2;
1430-
if (el != null) pkgCounts.replace((PackageElement)el, -2);
1437+
modCount = mc;
1438+
if (modCount == -1) {
1439+
// Import the module
1440+
currentToImportElement = m;
1441+
el = null;
1442+
// Mark to skip importing this module subsequently.
1443+
modCounts.put(m, -2);
1444+
}
1445+
// Else, no need to add the current import as its module has already been imported.
14311446
}
14321447
}
14331448
cnt = modCount < 0 ? modCount : enclosingStarCnt == null ? 0 : enclosingStarCnt;

‎java/java.source.base/test/unit/src/org/netbeans/api/java/source/GeneratorUtilitiesTest.java‎

Lines changed: 102 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -611,83 +611,125 @@ public void validate(CompilationInfo info) {
611611
}
612612

613613
public void testAddImportsWithModule1() throws Exception {
614-
performTest("test/Process.java", "package test;\nimport java.util.Collections;\npublic class Process {\npublic static void main(String... args) {\n" +
615-
"Collections.singleton(Process.class).forEach(System.out::println);\n}\n}", "25", new AddImportsTask(false, List.of("java.base"), "test.Process"), new Validator() {
616-
public void validate(CompilationInfo info) {
617-
assertEquals(0, info.getDiagnostics().size());
618-
List<? extends ImportTree> imports = info.getCompilationUnit().getImports();
619-
assertEquals(2, imports.size());
620-
assertEquals("java.util.Collections", imports.get(0).getQualifiedIdentifier().toString());
621-
assertTrue(imports.get(1).isModule());
622-
assertEquals("java.base", imports.get(1).getQualifiedIdentifier().toString());
623-
}
624-
}, false);
614+
performTest("test/Process.java",
615+
"""
616+
package test;
617+
import java.util.Collections;
618+
public class Process {
619+
public static void main(String... args) {
620+
Collections.singleton(Process.class).forEach(System.out::println);
621+
}
622+
}
623+
""",
624+
"25", new AddImportsTask(false, List.of("java.base"), "test.Process"),
625+
(CompilationInfo info) -> {
626+
assertEquals(0, info.getDiagnostics().size());
627+
List<? extends ImportTree> imports = info.getCompilationUnit().getImports();
628+
assertEquals(2, imports.size());
629+
assertEquals("java.util.Collections", imports.get(0).getQualifiedIdentifier().toString());
630+
assertTrue(imports.get(1).isModule());
631+
assertEquals("java.base", imports.get(1).getQualifiedIdentifier().toString());
632+
}, false);
625633
}
626634

627635
public void testAddImportsWithModule2() throws Exception {
628-
performTest("package test;\nimport java.util.List;\nimport javax.swing.JLabel;\npublic class Test { }\n", "25", new AddImportsTask(false, List.of("java.base", "java.desktop"), "javax.swing.JTable", "java.util.ArrayList"), new Validator() {
629-
public void validate(CompilationInfo info) {
630-
assertEquals(0, info.getDiagnostics().size());
631-
List<? extends ImportTree> imports = info.getCompilationUnit().getImports();
632-
assertEquals(4, imports.size());
633-
assertEquals("java.util.List", imports.get(0).getQualifiedIdentifier().toString());
634-
assertEquals("javax.swing.JLabel", imports.get(1).getQualifiedIdentifier().toString());
635-
assertTrue(imports.get(2).isModule());
636-
assertEquals("java.base", imports.get(2).getQualifiedIdentifier().toString());
637-
assertTrue(imports.get(3).isModule());
638-
assertEquals("java.desktop", imports.get(3).getQualifiedIdentifier().toString());
639-
}
640-
}, false);
636+
performTest(
637+
"""
638+
package test;
639+
import java.util.List;
640+
import javax.swing.JLabel;
641+
public class Test { }
642+
""",
643+
"25", new AddImportsTask(false, List.of("java.base", "java.desktop"), "javax.swing.JTable", "java.util.ArrayList"),
644+
(CompilationInfo info) -> {
645+
assertEquals(0, info.getDiagnostics().size());
646+
List<? extends ImportTree> imports = info.getCompilationUnit().getImports();
647+
assertEquals(4, imports.size());
648+
assertEquals("java.util.List", imports.get(0).getQualifiedIdentifier().toString());
649+
assertEquals("javax.swing.JLabel", imports.get(1).getQualifiedIdentifier().toString());
650+
assertTrue(imports.get(2).isModule());
651+
assertEquals("java.base", imports.get(2).getQualifiedIdentifier().toString());
652+
assertTrue(imports.get(3).isModule());
653+
assertEquals("java.desktop", imports.get(3).getQualifiedIdentifier().toString());
654+
}, false);
641655
}
642656

643657
public void testAddImportsWithModule3() throws Exception {
644-
performTest("package test;\nimport javax.swing.JLabel;\nimport module java.base;\npublic class Test { }\n", "25", new AddImportsTask(false, List.of("java.base", "java.desktop"), "java.util.List", "java.util.ArrayList"), new Validator() {
645-
public void validate(CompilationInfo info) {
646-
assertEquals(0, info.getDiagnostics().size());
647-
List<? extends ImportTree> imports = info.getCompilationUnit().getImports();
648-
assertEquals(4, imports.size());
649-
assertEquals("java.util.List", imports.get(0).getQualifiedIdentifier().toString());
650-
assertEquals("javax.swing.JLabel", imports.get(1).getQualifiedIdentifier().toString());
651-
assertTrue(imports.get(2).isModule());
652-
assertEquals("java.base", imports.get(2).getQualifiedIdentifier().toString());
653-
assertTrue(imports.get(3).isModule());
654-
assertEquals("java.desktop", imports.get(3).getQualifiedIdentifier().toString());
655-
}
656-
}, false);
658+
performTest(
659+
"""
660+
package test;
661+
import javax.swing.JLabel;
662+
import module java.base;
663+
public class Test { }
664+
""",
665+
"25", new AddImportsTask(false, List.of("java.base", "java.desktop"), "java.util.List", "java.util.ArrayList"),
666+
(CompilationInfo info) -> {
667+
assertEquals(0, info.getDiagnostics().size());
668+
List<? extends ImportTree> imports = info.getCompilationUnit().getImports();
669+
assertEquals(4, imports.size());
670+
assertEquals("java.util.List", imports.get(0).getQualifiedIdentifier().toString());
671+
assertEquals("javax.swing.JLabel", imports.get(1).getQualifiedIdentifier().toString());
672+
assertTrue(imports.get(2).isModule());
673+
assertEquals("java.base", imports.get(2).getQualifiedIdentifier().toString());
674+
assertTrue(imports.get(3).isModule());
675+
assertEquals("java.desktop", imports.get(3).getQualifiedIdentifier().toString());
676+
}, false);
657677
}
658678

659679
public void testAddImportsWithModule4() throws Exception {
660680
Preferences preferences = MimeLookup.getLookup(JavaTokenId.language().mimeType()).lookup(Preferences.class);
661681
preferences.putBoolean("allowConvertToStarImport", true);
662-
performTest("package test;\nimport java.awt.*;\nimport java.util.List;\nimport module java.base;\npublic class Test {\nprivate void op(List list) {\nint size = list.size();\n}\n}\n", "25", new AddImportsTask("java.util.AbstractList", "java.util.ArrayList", "java.util.Collection", "java.util.Collections"), new Validator() {
663-
public void validate(CompilationInfo info) {
664-
assertEquals(0, info.getDiagnostics().size());
665-
List<? extends ImportTree> imports = info.getCompilationUnit().getImports();
666-
assertEquals(3, imports.size());
667-
assertEquals("java.awt.*", imports.get(0).getQualifiedIdentifier().toString());
668-
assertEquals("java.util.List", imports.get(1).getQualifiedIdentifier().toString());
669-
assertTrue(imports.get(2).isModule());
670-
assertEquals("java.base", imports.get(2).getQualifiedIdentifier().toString());
671-
}
672-
}, false);
682+
performTest(
683+
"""
684+
package test;
685+
import java.awt.*;
686+
import java.util.List;
687+
import module java.base;
688+
public class Test {
689+
private void op(List list) {
690+
int size = list.size();
691+
}
692+
}
693+
""",
694+
"25", new AddImportsTask("java.util.AbstractList", "java.util.ArrayList", "java.util.Collection", "java.util.Collections"),
695+
(CompilationInfo info) -> {
696+
assertEquals(0, info.getDiagnostics().size());
697+
List<? extends ImportTree> imports = info.getCompilationUnit().getImports();
698+
assertEquals(3, imports.size());
699+
assertEquals("java.awt.*", imports.get(0).getQualifiedIdentifier().toString());
700+
assertEquals("java.util.List", imports.get(1).getQualifiedIdentifier().toString());
701+
assertTrue(imports.get(2).isModule());
702+
assertEquals("java.base", imports.get(2).getQualifiedIdentifier().toString());
703+
}, false);
673704
preferences.putBoolean("allowConvertToStarImport", false);
674705
}
675706

676707
public void testAddImportsWithModule5() throws Exception {
677708
Preferences preferences = MimeLookup.getLookup(JavaTokenId.language().mimeType()).lookup(Preferences.class);
678709
preferences.putBoolean("allowConvertToStarImport", true);
679-
performTest("package test;\nimport java.util.List;\npublic class Test {\nprivate List l = new ArrayList<>();\nprivate Button b;\n private void op(List list) {\nint size = list.size();\n}\n}\n", "25", new AddImportsTask(false, List.of("java.desktop", "java.base"), "java.awt.Button", "java.util.AbstractList", "java.util.ArrayList", "java.util.Collection", "java.util.Collections"), new Validator() {
680-
public void validate(CompilationInfo info) {
681-
assertEquals(0, info.getDiagnostics().size());
682-
List<? extends ImportTree> imports = info.getCompilationUnit().getImports();
683-
assertEquals(3, imports.size());
684-
assertEquals("java.util.List", imports.get(0).getQualifiedIdentifier().toString());
685-
assertTrue(imports.get(1).isModule());
686-
assertEquals("java.base", imports.get(1).getQualifiedIdentifier().toString());
687-
assertTrue(imports.get(2).isModule());
688-
assertEquals("java.desktop", imports.get(2).getQualifiedIdentifier().toString());
689-
}
690-
}, false);
710+
performTest(
711+
"""
712+
package test;
713+
import java.util.List;
714+
public class Test {
715+
private List l = new ArrayList<>();
716+
private Button b;
717+
private void op(List list) {
718+
int size = list.size();
719+
}
720+
}
721+
""",
722+
"25", new AddImportsTask(false, List.of("java.desktop", "java.base"), "java.awt.Button", "java.util.AbstractList", "java.util.ArrayList", "java.util.Collection", "java.util.Collections"),
723+
(CompilationInfo info) -> {
724+
assertEquals(0, info.getDiagnostics().size());
725+
List<? extends ImportTree> imports = info.getCompilationUnit().getImports();
726+
assertEquals(3, imports.size());
727+
assertEquals("java.util.List", imports.get(0).getQualifiedIdentifier().toString());
728+
assertTrue(imports.get(1).isModule());
729+
assertEquals("java.base", imports.get(1).getQualifiedIdentifier().toString());
730+
assertTrue(imports.get(2).isModule());
731+
assertEquals("java.desktop", imports.get(2).getQualifiedIdentifier().toString());
732+
}, false);
691733
preferences.putBoolean("allowConvertToStarImport", false);
692734
}
693735

0 commit comments

Comments
 (0)