RFR: 8245956: JavaCompiler still uses files API instead of Path API in a specific case

[Jonathan Gibbons]

On Wed, 2 Dec 2020 05:10:12 GMT, Guoxiong Li <github.com+13688759+lgxbslgx at openjdk.org> wrote:

> Hi all,
> 
> `JavacFileManager.getClassLoader` uses `getLocation` which causes exception in some situations.
> This patch uses `getLocationAsPaths` instead of `getLocation` to solve it.
> Thanks you for taking the time to review.
> 
> Best Regards.

Nice.

Some minor suggestions inline for your consideration.

src/jdk.compiler/share/classes/com/sun/tools/javac/file/JavacFileManager.java line 744:

> 742:     public ClassLoader getClassLoader(Location location) {
> 743:         checkNotModuleOrientedLocation(location);
> 744:         Collection<? extends Path> paths = getLocationAsPaths(location);

Minor suggestion:  the use of the simple name `path` or `paths` can be confusing in contexts like this, when the name may refer to an instance of `java.nio.file.Path` or a collection of such items.

You did good by renaming the variable from `path` to `paths`.  A more explicit name would be to use `searchPath` as the name for a series of file system objects.

test/langtools/tools/javac/T8245956/T8245956.java line 27:

> 25:  * @test
> 26:  * @bug 8245956
> 27:  * @summary JavaCompiler still uses files API instead of Path API in a specific case

I suggest changing `files API` to `File API`.
I have made the corresponding change in the JBS issue.

test/langtools/tools/javac/T8245956/T8245956.java line 62:

> 60:                 classPath.add(fsPath);
> 61:                 fileManager.setLocationFromPaths(StandardLocation.CLASS_PATH, classPath);
> 62:                 fileManager.getClassLoader(StandardLocation.CLASS_PATH);  // Should not generate any exceptions.

Minor suggestion: add something like a print statement after the `getClassLoader` call to verify that the `getClassLoader` call terminated normally.

-------------

Marked as reviewed by jjg (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/1553