Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8220695

Add StandardJavaFileManager.getJavaFileObjectsFromPaths overload

    Details

    • Type: CSR
    • Status: Closed
    • Priority: P4
    • Resolution: Approved
    • Fix Version/s: 13
    • Component/s: tools
    • Labels:
      None
    • Subcomponent:
    • Compatibility Kind:
      source
    • Compatibility Risk:
      low
    • Compatibility Risk Description:
      Hide
      This change deprecates but does not remove the existing method, and the new method has a default implementation, so there is no binary compatibility impact and low source compatibility impact.

      One source compatibility risk of adding an overload is that it may not be possible to disambiguate the new overload in all cases.

      Selecting the new overload at existing call sites does not pose a compatibility risk, as both methods have identical semantics.
      Show
      This change deprecates but does not remove the existing method, and the new method has a default implementation, so there is no binary compatibility impact and low source compatibility impact. One source compatibility risk of adding an overload is that it may not be possible to disambiguate the new overload in all cases. Selecting the new overload at existing call sites does not pose a compatibility risk, as both methods have identical semantics.
    • Interface Kind:
      Java API
    • Scope:
      SE

      Description

      Summary

      Add a new method StandardJavaFileManager#getJavaFileObjectsFromPaths(Collection<? extends Path>) and deprecate getJavaFileObjectsFromPaths(Iterable<? extends Path>) with a migration path to the new method.

      Problem

      Using StandardJavaFileManager#getJavaFileObjectsFromPaths(Iterable<? extends Path>) is error-prone, since Path itself implements Iterable<Path>. This allows accidentally passing a single path when the intent was to pass a singleton collection, i.e.: getJavaFileObjectsFromPaths(path) vs. getJavaFileObjectsFromPaths(List.of(Path)).

      Solution

      The solution is to add a new method that accepts a Collection<? extends Path>, which cannot be confused with an individual Path. The existing Iterable-accepting method is deprecated, and its default implementation delegates to the new method for migration compatibility.

      A similar issue affected setLocationFromPaths, but was fixed before that method was released in JDK 9; see JDK-8150111.

      Specification

      The javadoc specification for the new method is identical to previous iterable-accepting version. (The @param javadoc refers informally to a 'list of paths' and does not need updating.)

      diff -r eed9f74eab87 src/java.compiler/share/classes/javax/tools/StandardJavaFileManager.java
      --- a/src/java.compiler/share/classes/javax/tools/StandardJavaFileManager.java  Fri Mar 15 09:57:42 2019 +0100
      +++ b/src/java.compiler/share/classes/javax/tools/StandardJavaFileManager.java  Thu Mar 21 20:08:52 2019 -0700
      @@ -28,9 +28,11 @@
       import java.io.File;
       import java.io.IOException;
       import java.nio.file.Path;
      +import java.util.ArrayList;
       import java.util.Arrays;
       import java.util.Collection;
       import java.util.Iterator;
      +import java.util.List;
      
       /**
        * File manager based on {@linkplain File java.io.File} and {@linkplain Path java.nio.file.Path}.
      @@ -199,11 +201,40 @@
            * a directory or if this file manager does not support any of the
            * given paths.
            *
      -     * @since 9
      +     * @since 13
            */
           default Iterable<? extends JavaFileObject> getJavaFileObjectsFromPaths(
      +            Collection<? extends Path> paths) {
      +        return getJavaFileObjectsFromFiles(asFiles(paths));
      +    }
      +
      +    /**
      +     * Returns file objects representing the given paths.
      +     *
      +     * @implSpec
      +     * The default implementation converts each path to a file and calls
      +     * {@link #getJavaFileObjectsFromFiles getJavaObjectsFromFiles}.
      +     * IllegalArgumentException will be thrown if any of the paths
      +     * cannot be converted to a file.
      +     *
      +     * @param paths a list of paths
      +     * @return a list of file objects
      +     * @throws IllegalArgumentException if the list of paths includes
      +     * a directory or if this file manager does not support any of the
      +     * given paths.
      +     *
      +     * @since 9
      +     * @deprecated use {@link #getJavaFileObjectsFromPaths(Collection)} instead,
      +     * to prevent the possibility of accidentally calling the method with a
      +     * single {@code Path} as such an argument. Although {@code Path} implements
      +     * {@code Iterable<Path>}, it would almost never be correct to pass a single
      +     * {@code Path} and have it be treated as an {@code Iterable} of its
      +     * components.
      +     */
      +    @Deprecated(since = "13")
      +    default Iterable<? extends JavaFileObject> getJavaFileObjectsFromPaths(
                   Iterable<? extends Path> paths) {
      -        return getJavaFileObjectsFromFiles(asFiles(paths));
      +        return getJavaFileObjectsFromPaths(asCollection(paths));
           }
      
           /**
      @@ -484,4 +515,13 @@
                   }
               };
           }
      +
      +    private static <T> Collection<T> asCollection(Iterable<T> iterable) {
      +        if (iterable instanceof Collection) {
      +            return (Collection<T>) iterable;
      +        }
      +        List<T> result = new ArrayList<>();
      +        for (T item : iterable) result.add(item);
      +        return result;
      +    }
       }

      webrev: http://cr.openjdk.java.net/~cushon/8220687/webrev.03/src/java.compiler/share/classes/javax/tools/StandardJavaFileManager.java.sdiff.html

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                cushon Liam Miller-Cushon
                Reporter:
                cushon Liam Miller-Cushon
                Reviewed By:
                Jonathan Gibbons
              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: