8177650: JShell tool: packages in classpath don't appear in completions

Reviewed-by: asotona

src/jdk.jshell/share/classes/jdk/jshell/SourceCodeAnalysisImpl.java

@@ -92,7 +92,6 @@ import javax.lang.model.type.TypeMirror;
 import static jdk.internal.jshell.debug.InternalDebugControl.DBG_COMPA;
 
 import java.io.IOException;
-import java.net.URI;
 import java.nio.file.DirectoryStream;
 import java.nio.file.FileSystem;
 import java.nio.file.FileSystems;
@@ -101,6 +100,7 @@ import java.nio.file.FileVisitor;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.nio.file.ProviderNotFoundException;
 import java.nio.file.attribute.BasicFileAttributes;
 import java.util.Arrays;
 import java.util.Collection;
@@ -2002,12 +2002,22 @@ class SourceCodeAnalysisImpl extends SourceCodeAnalysis {
     //update indexes, either initially or after a classpath change:
     private void refreshIndexes(int version) {
         try {
-            Collection<Path> paths = new ArrayList<>();
-            MemoryFileManager fm = proc.taskFactory.fileManager();
-
-            appendPaths(fm, StandardLocation.PLATFORM_CLASS_PATH, paths);
-            appendPaths(fm, StandardLocation.CLASS_PATH, paths);
-            appendPaths(fm, StandardLocation.SOURCE_PATH, paths);
+            Collection<Path> paths = proc.taskFactory.parse("", task -> {
+                MemoryFileManager fm = proc.taskFactory.fileManager();
+                Collection<Path> _paths = new ArrayList<>();
+                try {
+                    appendPaths(fm, StandardLocation.PLATFORM_CLASS_PATH, _paths);
+                    appendPaths(fm, StandardLocation.CLASS_PATH, _paths);
+                    appendPaths(fm, StandardLocation.SOURCE_PATH, _paths);
+                    appendModulePaths(fm, StandardLocation.SYSTEM_MODULES, _paths);
+                    appendModulePaths(fm, StandardLocation.UPGRADE_MODULE_PATH, _paths);
+                    appendModulePaths(fm, StandardLocation.MODULE_PATH, _paths);
+                    return _paths;
+                } catch (Exception ex) {
+                    proc.debug(ex, "SourceCodeAnalysisImpl.refreshIndexes(" + version + ")");
+                    return List.of();
+                }
+            });
 
             Map<Path, ClassIndex> newIndexes = new HashMap<>();
 
@@ -2060,27 +2070,24 @@ class SourceCodeAnalysisImpl extends SourceCodeAnalysis {
         }
     }
 
+    private void appendModulePaths(MemoryFileManager fm, Location loc, Collection<Path> paths) throws IOException {
+        for (Set<Location> moduleLocations : fm.listLocationsForModules(loc)) {
+            for (Location moduleLocation : moduleLocations) {
+                Iterable<? extends Path> modulePaths = fm.getLocationAsPaths(moduleLocation);
+
+                if (modulePaths == null) {
+                    continue;
+                }
+
+                modulePaths.forEach(paths::add);
+            }
+        }
+    }
+
     //create/update index a given JavaFileManager entry (which may be a JDK installation, a jar/zip file or a directory):
     //if an index exists for the given entry, the existing index is kept unless the timestamp is modified
     private ClassIndex indexForPath(Path path) {
-        if (isJRTMarkerFile(path)) {
-            FileSystem jrtfs = FileSystems.getFileSystem(URI.create("jrt:/"));
-            Path modules = jrtfs.getPath("modules");
-            return PATH_TO_INDEX.compute(path, (p, index) -> {
-                try {
-                    long lastModified = Files.getLastModifiedTime(modules).toMillis();
-                    if (index == null || index.timestamp != lastModified) {
-                        try (DirectoryStream<Path> stream = Files.newDirectoryStream(modules)) {
-                            index = doIndex(lastModified, path, stream);
-                        }
-                    }
-                    return index;
-                } catch (IOException ex) {
-                    proc.debug(ex, "SourceCodeAnalysisImpl.indexesForPath(" + path.toString() + ")");
-                    return new ClassIndex(-1, path, Collections.emptySet(), Collections.emptyMap());
-                }
-            });
-        } else if (!Files.isDirectory(path)) {
+        if (!Files.isDirectory(path)) {
             if (Files.exists(path)) {
                 return PATH_TO_INDEX.compute(path, (p, index) -> {
                     try {
@@ -2093,7 +2100,7 @@ class SourceCodeAnalysisImpl extends SourceCodeAnalysis {
                             }
                         }
                         return index;
-                    } catch (IOException ex) {
+                    } catch (IOException | ProviderNotFoundException ex) {
                         proc.debug(ex, "SourceCodeAnalysisImpl.indexesForPath(" + path.toString() + ")");
                         return new ClassIndex(-1, path, Collections.emptySet(), Collections.emptyMap());
                     }
@@ -2112,10 +2119,6 @@ class SourceCodeAnalysisImpl extends SourceCodeAnalysis {
         }
     }
 
-    static boolean isJRTMarkerFile(Path path) {
-        return path.equals(Paths.get(System.getProperty("java.home"), "lib", "modules"));
-    }
-
     //create an index based on the content of the given dirs; the original JavaFileManager entry is originalPath.
     private ClassIndex doIndex(long timestamp, Path originalPath, Iterable<? extends Path> dirs) {
         Set<String> packages = new HashSet<>();
@@ -2200,13 +2203,17 @@ class SourceCodeAnalysisImpl extends SourceCodeAnalysis {
             upToDate = classpathVersion == indexVersion;
         }
         while (!upToDate) {
-            INDEXER.submit(() -> {}).get();
+            waitCurrentBackgroundTasksFinished();
             synchronized (currentIndexes) {
                 upToDate = classpathVersion == indexVersion;
             }
         }
     }
 
+    public static void waitCurrentBackgroundTasksFinished() throws Exception {
+        INDEXER.submit(() -> {}).get();
+    }
+
     /**
      * A candidate for continuation of the given user's input.
      */

test/langtools/jdk/jshell/Compiler.java

@@ -72,11 +72,17 @@ public class Compiler {
     }
 
     public void jar(Path directory, String jarName, String...files) {
+        Path classDirPath = getClassDir();
+        Path baseDir = classDirPath.resolve(directory);
+        Path jarPath = baseDir.resolve(jarName);
+        jar(directory, jarPath, files);
+    }
+
+    public void jar(Path directory, Path jarPath, String...files) {
         Manifest manifest = new Manifest();
         manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0");
         Path classDirPath = getClassDir();
         Path baseDir = classDirPath.resolve(directory);
-        Path jarPath = baseDir.resolve(jarName);
         new JarTask(tb, jarPath.toString())
                 .manifest(manifest)
                 .baseDir(baseDir.toString())

test/langtools/jdk/jshell/CompletionSuggestionTest.java

@@ -834,4 +834,20 @@ public class CompletionSuggestionTest extends KullaTesting {
         assertCompletionIncludesExcludes("import module ja|", Set.of("java.base"), Set.of("jdk.compiler"));
         assertCompletion("import module java/*c*/./*c*/ba|", "java.base");
     }
+
+    public void testCustomClassPathIndexing() {
+        Path p1 = outDir.resolve("dir1");
+        compiler.compile(p1,
+                "package p1.p2;\n" +
+                "public class Test {\n" +
+                "}",
+                "package p1.p3;\n" +
+                "public class Test {\n" +
+                "}");
+        String jarName = "test.jar";
+        compiler.jar(p1, jarName, "p1/p2/Test.class", "p1/p3/Test.class");
+        addToClasspath(compiler.getPath(p1.resolve(jarName)));
+
+        assertCompletion("p1.|", "p2.", "p3.");
+    }
 }

test/langtools/jdk/jshell/ReplToolTesting.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2025, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -25,6 +25,7 @@ import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.io.PrintStream;
+import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -566,6 +567,36 @@ public class ReplToolTesting {
         }
     }
 
+    public void assertCompletions(boolean after, String input, String expectedCompletionsPattern) {
+        if (!after) {
+            try {
+                Class<?> sourceCodeAnalysisImpl = Class.forName("jdk.jshell.SourceCodeAnalysisImpl");
+                Method waitBackgroundTaskFinished = sourceCodeAnalysisImpl.getDeclaredMethod("waitCurrentBackgroundTasksFinished");
+
+                waitBackgroundTaskFinished.setAccessible(true);
+                waitBackgroundTaskFinished.invoke(null);
+            } catch (ReflectiveOperationException ex) {
+                throw new AssertionError(ex.getMessage(), ex);
+            }
+
+            setCommandInput(input + "\t");
+        } else {
+            assertOutput(getCommandOutput().trim(), "", "command output: " + input);
+            assertOutput(getCommandErrorOutput(), "", "command error: " + input);
+            assertOutput(getUserOutput(), "", "user output: " + input);
+            assertOutput(getUserErrorOutput(), "", "user error: " + input);
+            String actualOutput = getTerminalOutput();
+            Pattern compiledPattern =
+                    Pattern.compile(expectedCompletionsPattern, Pattern.DOTALL);
+            if (!compiledPattern.asMatchPredicate().test(actualOutput)) {
+                throw new AssertionError("Actual output:\n" +
+                                         actualOutput + "\n" +
+                                         "does not match expected pattern: " +
+                                         expectedCompletionsPattern);
+            }
+        }
+    }
+
     private String normalizeLineEndings(String text) {
         return normalizeLineEndings(System.getProperty("line.separator"), text);
     }

test/langtools/jdk/jshell/ToolCompletionTest.java

@@ -0,0 +1,257 @@
+/*
+ * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8177650
+ * @summary Verify JShell tool code completion
+ * @library /tools/lib
+ * @modules jdk.compiler/com.sun.tools.javac.api
+ *          jdk.compiler/com.sun.tools.javac.main
+ *          jdk.jdeps/com.sun.tools.javap
+ *          jdk.jshell/jdk.jshell:+open
+ *          jdk.jshell/jdk.internal.jshell.tool
+ *          java.desktop
+ * @build toolbox.ToolBox toolbox.JarTask toolbox.JavacTask
+ * @build ReplToolTesting TestingInputStream Compiler
+ * @run testng ToolCompletionTest
+ */
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+import org.testng.annotations.Test;
+
+public class ToolCompletionTest extends ReplToolTesting {
+
+    private final Compiler compiler = new Compiler();
+    private final Path outDir = Paths.get("tool_completion_test");
+
+    @Test
+    public void testClassPathOnCmdLineIndexing() {
+        Path p1 = outDir.resolve("dir1");
+        compiler.compile(p1,
+                """
+                package p1.p2;
+                public class Test {
+                }
+                """,
+                """
+                package p1.p3;
+                public class Test {
+                }
+                """);
+        String jarName = "test.jar";
+        compiler.jar(p1, jarName, "p1/p2/Test.class", "p1/p3/Test.class");
+
+        test(false, new String[]{"--no-startup", "--class-path", compiler.getPath(p1.resolve(jarName)).toString()},
+                (a) -> assertCompletions(a, "p1.", ".*p2\\..*p3\\..*"),
+                 //cancel the input, so that JShell can be finished:
+                (a) -> assertCommand(a, "\003", null)
+                );
+    }
+
+    @Test
+    public void testClassPathViaEnvIndexing() {
+        Path p1 = outDir.resolve("dir1");
+        compiler.compile(p1,
+                """
+                package p1.p2;
+                public class Test {
+                }
+                """,
+                """
+                package p1.p3;
+                public class Test {
+                }
+                """);
+        String jarName = "test.jar";
+        compiler.jar(p1, jarName, "p1/p2/Test.class", "p1/p3/Test.class");
+
+        test(false, new String[]{"--no-startup"},
+                (a) -> assertCommand(a, "/env --class-path " + compiler.getPath(p1.resolve(jarName)).toString(), null),
+                (a) -> assertCompletions(a, "p1.", ".*p2\\..*p3\\..*"),
+                 //cancel the input, so that JShell can be finished:
+                (a) -> assertCommand(a, "\003", null)
+                );
+    }
+
+    @Test
+    public void testClassPathChangeIndexing() {
+        //verify that changing the classpath has effect:
+        Path dir1 = outDir.resolve("dir1");
+        compiler.compile(dir1,
+                """
+                package p1.p2;
+                public class Test {
+                }
+                """,
+                """
+                package p1.p3;
+                public class Test {
+                }
+                """);
+        String jarName1 = "test1.jar";
+        compiler.jar(dir1, jarName1, "p1/p2/Test.class", "p1/p3/Test.class");
+
+        Path dir2 = outDir.resolve("dir2");
+        compiler.compile(dir2,
+                """
+                package p1.p5;
+                public class Test {
+                }
+                """,
+                """
+                package p1.p6;
+                public class Test {
+                }
+                """);
+        String jarName2 = "test2.jar";
+        compiler.jar(dir2, jarName2, "p1/p5/Test.class", "p1/p6/Test.class");
+
+        test(false, new String[]{"--no-startup", "--class-path", compiler.getPath(dir1.resolve(jarName1)).toString()},
+                (a) -> assertCommand(a, "1", null),
+                (a) -> assertCommand(a, "/env --class-path " + compiler.getPath(dir2.resolve(jarName2)).toString(), null),
+                (a) -> assertCompletions(a, "p1.", ".*p5\\..*p6\\..*"),
+                 //cancel the input, so that JShell can be finished:
+                (a) -> assertCommand(a, "\003", null)
+                );
+    }
+
+    @Test
+    public void testModulePathOnCmdLineIndexing() {
+        Path p1 = outDir.resolve("dir1");
+        compiler.compile(p1,
+                """
+                module m {
+                    exports p1.p2;
+                    exports p1.p3;
+                }
+                """,
+                """
+                package p1.p2;
+                public class Test {
+                }
+                """,
+                """
+                package p1.p3;
+                public class Test {
+                }
+                """);
+        String jarName = "test.jar";
+        compiler.jar(p1, jarName, "p1/p2/Test.class", "p1/p3/Test.class");
+
+        test(false, new String[]{"--no-startup", "--module-path", compiler.getPath(p1.resolve(jarName)).toString()},
+                (a) -> assertCompletions(a, "p1.", ".*p2\\..*p3\\..*"),
+                 //cancel the input, so that JShell can be finished:
+                (a) -> assertCommand(a, "\003", null)
+                );
+    }
+
+    @Test
+    public void testModulePathOnCmdLineIndexing2() throws IOException {
+        Path p1 = outDir.resolve("dir1");
+        compiler.compile(p1,
+                """
+                module m {
+                    exports p1.p2;
+                    exports p1.p3;
+                }
+                """,
+                """
+                package p1.p2;
+                public class Test {
+                }
+                """,
+                """
+                package p1.p3;
+                public class Test {
+                }
+                """);
+        String jarName = "test.jar";
+        Path lib = outDir.resolve("lib");
+        Files.createDirectories(lib);
+        compiler.jar(p1, lib.resolve(jarName), "p1/p2/Test.class", "p1/p3/Test.class");
+
+        test(false, new String[]{"--no-startup", "--module-path", lib.toString()},
+                (a) -> assertCompletions(a, "p1.", ".*p2\\..*p3\\..*"),
+                 //cancel the input, so that JShell can be finished:
+                (a) -> assertCommand(a, "\003", null)
+                );
+    }
+
+    @Test
+    public void testUpgradeModulePathIndexing() {
+        Path p1 = outDir.resolve("dir1");
+        compiler.compile(p1,
+                """
+                module m {
+                    exports p1.p2;
+                    exports p1.p3;
+                }
+                """,
+                """
+                package p1.p2;
+                public class Test {
+                }
+                """,
+                """
+                package p1.p3;
+                public class Test {
+                }
+                """);
+        String jarName = "test.jar";
+        compiler.jar(p1, jarName, "p1/p2/Test.class", "p1/p3/Test.class");
+
+        test(false, new String[]{"--no-startup", "-C--upgrade-module-path", "-C" + compiler.getPath(p1.resolve(jarName)).toString()},
+                (a) -> assertCompletions(a, "p1.", ".*p2\\..*p3\\..*"),
+                 //cancel the input, so that JShell can be finished:
+                (a) -> assertCommand(a, "\003", null)
+                );
+    }
+
+    @Test
+    public void testBootClassPathPrepend() {
+        Path p1 = outDir.resolve("dir1");
+        compiler.compile(p1,
+                """
+                package p1.p2;
+                public class Test {
+                }
+                """,
+                """
+                package p1.p3;
+                public class Test {
+                }
+                """);
+        String jarName = "test.jar";
+        compiler.jar(p1, jarName, "p1/p2/Test.class", "p1/p3/Test.class");
+
+        test(false, new String[]{"--no-startup", "-C-Xbootclasspath/p:" + compiler.getPath(p1.resolve(jarName)).toString(), "-C--source=8"},
+                (a) -> assertCompletions(a, "p1.", ".*p2\\..*p3\\..*"),
+                 //cancel the input, so that JShell can be finished:
+                (a) -> assertCommand(a, "\003", null)
+                );
+    }
+}