RFR: 8262742: (fs) Add Path::resolve with varargs string [v2]

Brian Burkhalter bpb at openjdk.org
Wed Jul 12 00:53:18 UTC 2023


On Tue, 11 Jul 2023 21:09:15 GMT, Roger Riggs <rriggs at openjdk.org> wrote:

>> Brian Burkhalter has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - 8262742: Fix typo
>>  - 8262742: Remove invalid "throws InvalidPathException"; correct typo
>
> src/java.base/share/classes/java/nio/file/Path.java line 520:
> 
>> 518: 
>> 519:     /**
>> 520:      * Resolves a path or a sequence of paths against this path.
> 
> "or"  -> "and then"

Like in `Path.of`, the first parameter may be considered element zero of the sequence which was the intent here.

> src/java.base/share/classes/java/nio/file/Path.java line 574:
> 
>> 572:      * Path result = resolve(first);
>> 573:      * for (String s : more) {
>> 574:      *     result = result.resolve(s);
> 
> The code here doesn't match the description, converts to Paths and then resolves.
> Though it might not be detectable.

Description updated in 1b0c5228e05cc4a685f18b949be99f73f5e1b208.

> src/java.base/share/classes/java/nio/file/Path.java line 586:
> 
>> 584:      * @throws  InvalidPathException
>> 585:      *          if the path string cannot be converted to a Path.
>> 586:      *
> 
> When an InvalidPathException is thrown, what information is available to identify the illegal string?
> 
> "if the path" -> "if a path" or "if any of the paths".

The message of `InvalidPathException` is most often created in `UnixPath`, `WindowsPath`, and `WindowsPathParser` and indicates the problem, e.g., `nul` character, unmappable character, etc.

Exception doc updated in 1b0c5228e05cc4a685f18b949be99f73f5e1b208.

> test/jdk/java/nio/file/Path/PathOps.java line 1728:
> 
>> 1726:             .resolve("/foo", "", "foo")
>> 1727:             .resolve("/bar", "foo", "", "/bar");
>> 1728: 
> 
> Should there be test cases that include multiple directories in a path:  "abc/def/xyz"?
> Also cases with "..", or is that only relevant if canonicalizing.

Multiple directories would be good but I don't know about `..`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14805#discussion_r1260431320
PR Review Comment: https://git.openjdk.org/jdk/pull/14805#discussion_r1260431419
PR Review Comment: https://git.openjdk.org/jdk/pull/14805#discussion_r1260430902
PR Review Comment: https://git.openjdk.org/jdk/pull/14805#discussion_r1260431676


More information about the nio-dev mailing list