Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8266631: StandardJavaFileManager: getJavaFileObjects() impl violates the spec #4360

Closed
wants to merge 2 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -190,10 +190,10 @@ Iterable<? extends JavaFileObject> getJavaFileObjectsFromFiles(
* Returns file objects representing the given paths.
*
* @implSpec
* The default implementation converts each path to a file and calls
* {@link #getJavaFileObjectsFromFiles getJavaObjectsFromFiles}.
* The default implementation lazily converts each path to a file and calls
* {@link #getJavaFileObjectsFromFiles(Iterable) getJavaFileObjectsFromFiles}.
* IllegalArgumentException will be thrown if any of the paths
Copy link
Contributor

@jonathan-gibbons jonathan-gibbons Jun 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we're editing lines in this region ... use {@code} or {@link} around IllegalArgumentException.

* cannot be converted to a file.
* cannot be converted to a file at the point the conversion happens.
*
* @param paths a list of paths
* @return a list of file objects
Copy link
Contributor

@jonathan-gibbons jonathan-gibbons Jun 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the following @throws, remove the trailing "."

@@ -212,10 +212,10 @@ default Iterable<? extends JavaFileObject> getJavaFileObjectsFromPaths(
* Returns file objects representing the given paths.
*
* @implSpec
* The default implementation converts each path to a file and calls
* {@link #getJavaFileObjectsFromFiles getJavaObjectsFromFiles}.
* The default implementation lazily converts each path to a file and calls
* {@link #getJavaFileObjectsFromPaths(Collection) getJavaFileObjectsFromPaths}.
* IllegalArgumentException will be thrown if any of the paths
Copy link
Contributor

@jonathan-gibbons jonathan-gibbons Jun 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we're editing lines in this region ... use {@code} or {@link} around IllegalArgumentException.

* cannot be converted to a file.
* cannot be converted to a file at the point the conversion happens.
*
* @param paths a list of paths
* @return a list of file objects
@@ -267,7 +267,8 @@ default Iterable<? extends JavaFileObject> getJavaFileObjectsFromPaths(
* @throws IllegalArgumentException if the array of files includes
* a directory
Copy link
Contributor

@jonathan-gibbons jonathan-gibbons Jun 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider adding or if this file manager does not support any of the * given paths in line with similar methods

* @throws NullPointerException if the given array contains null
* elements
* elements and the given element is used by
* {@linkplain #getJavaFileObjectsFromPaths(Collection)}.
Copy link
Contributor

@jonathan-gibbons jonathan-gibbons Jun 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new condition should be in an @implSpec.

The package declaration contains a general specification for nulls and NPE, so it's not clear that we need the @throws NPE at all, but it's not wrong to leave it as was.

*
* @since 9
*/
@@ -332,10 +333,11 @@ void setLocation(Location location, Iterable<? extends File> files)
* will be cancelled.
*
* @implSpec
* The default implementation converts each path to a file and calls
* {@link #getJavaFileObjectsFromFiles getJavaObjectsFromFiles}.
* The default implementation lazily converts each path to a file and calls
* {@link #setLocation setLocation}.
* {@linkplain IllegalArgumentException IllegalArgumentException}
* will be thrown if any of the paths cannot be converted to a file.
* will be thrown if any of the paths cannot be converted to a file at
* the point the conversion happens.
*
* @param location a location
* @param paths a list of paths, if {@code null} use the default