8220816: (fs) Files.createDirectory should make it more obvious that it fails when the directory already exists #28378

bplb · 2025-11-18T17:41:24Z

Slightly changes the specification of Files.createDirectory to highlight that it fails if a filesystem entity already exists at the given location.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8220816: (fs) Files.createDirectory should make it more obvious that it fails when the directory already exists (Enhancement - P5)

Reviewers

Alan Bateman (@AlanBateman - Reviewer)
Jaikiran Pai (@jaikiran - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/28378/head:pull/28378
$ git checkout pull/28378

Update a local copy of the PR:
$ git checkout pull/28378
$ git pull https://git.openjdk.org/jdk.git pull/28378/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 28378

View PR using the GUI difftool:
$ git pr show -t 28378

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/28378.diff

Using Webrev

Link to Webrev Comment

…it fails when the directory already exists

bridgekeeper · 2025-11-18T17:42:51Z

👋 Welcome back bpb! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-11-18T17:43:12Z

@bplb This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8220816: (fs) Files.createDirectory should make it more obvious that it fails when the directory already exists

Reviewed-by: alanb, jpai

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 57 new commits pushed to the master branch:

b41146c: 8367531: Template Framework: use scopes and tokens instead of misbehaving immediate-return-queries
6fc8e49: 8372097: C2: PhasePrintLevel requires setting PrintPhaseLevel explicitly to be active
852141b: 8372004: Have SSLLogger implement System.Logger
... and 54 more: https://git.openjdk.org/jdk/compare/9ec773ad27773f5813c79ae33ac1d2393c2e0cc8...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

openjdk · 2025-11-18T17:43:58Z

@bplb The following label will be automatically applied to this pull request:

nio

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2025-11-18T17:47:15Z

Webrevs

justin-curtis-lu

Wording looks reasonable to me. I presume the CSR will be filed after wording is finalized.

justin-curtis-lu · 2025-11-18T21:23:19Z

src/java.base/share/classes/java/nio/file/Files.java

     *          if the array contains an attribute that cannot be set atomically
     *          when creating the directory
     * @throws  FileAlreadyExistsException
     *          if a directory could not otherwise be created because a file of


Is it worth clarifying ..."file or directory"... in the throws tag as well?

Is it worth clarifying ..."file or directory"... in the throws tag as well?

Yes, I think so, thanks.

Changed in commit 3af3490.

bplb · 2025-11-18T22:02:14Z

I presume the CSR will be filed after wording is finalized.

Yes. I generally wait for the discussion to converge before filing the CSR.

bplb · 2025-11-18T22:06:12Z

Please note that this was discussed before on nio-dev.

RogerRiggs · 2025-11-19T01:35:29Z

src/java.base/share/classes/java/nio/file/Files.java

     * @throws  FileAlreadyExistsException
-     *          if a directory could not otherwise be created because a file of
-     *          that name already exists <i>(optional specific exception)</i>
+     *          if a directory could not otherwise be created because a file or


It is pre-existing wording, but I don't know that "otherwise" has any meaning here.
It reads the same whether "otherwise" is present or not.

I don't know that "otherwise" has any meaning here.

No, I also don't think it does.

AlanBateman · 2025-11-19T10:32:17Z

I think the @throws FileAlreadyExistsException description could be improved as "because a file of that name already exists" is too subtle. If we do change it then it will need to be done for all the methods that throw this exception, not just Files.createDirectory. What do you think of keeping it as simple as "If dir locates a file that already exists" ?

Files.createFile has "Creates a new and empty file, failing if the file already exists", which I think is clear. It's less easy to create a sentence for createDirectory, createSymbolicFile and other methods. "Creates a directory, failing if the file already exists" is accurate but might not be clear to everyone that "file" means any file (sym link, directory, ...). I wonder if we could keep simple like this: "Creates a directory, failing if dir locates an existing file". I would be hesitate to using "file or directory" as proposed because that's just two of many cases.

bplb · 2025-11-19T16:41:59Z

What do you think of keeping it as simple as "If dir locates a file that already exists" ?

That's fine. I previously suggested {@code dir} already exists here.

"Creates a directory, failing if dir locates an existing file".

That might work.

bplb · 2025-11-19T17:54:14Z

Commit 5c62cd5 does not (yet) address throws FileAlreadyExistsException verbiage for these cases:

If a file of that name already exists and the CREATE_NEW option is specified;
if the target file exists but cannot be replaced because the REPLACE_EXISTING option is not specified.

AlanBateman · 2025-11-19T18:50:39Z

src/java.base/share/classes/java/nio/file/Files.java

     * @throws  FileAlreadyExistsException
-     *          if {@code dir} exists but is not a directory <i>(optional specific
-     *          exception)</i>
+     *          if {@code dir} locates an existing file but is not a directory


Maybe "that is not a directory" might be better.

Fixed in 9cf6ede.

AlanBateman · 2025-11-19T18:51:21Z

src/java.base/share/classes/java/nio/file/Files.java

    /**
-     * Creates a new and empty file, failing if the file already exists. The
+     * Creates a new and empty file, failing if {@code path} locates an existing
+     * file. The


That works. I assume you'll re-flow this a bit to avoid "file. The" be on its own.

I assume you'll re-flow this a bit

I left it that way intentionally to ease comparing the diffs in progress.

Fixed in 9cf6ede.

AlanBateman · 2025-11-19T18:52:08Z

src/java.base/share/classes/java/nio/file/Files.java

    /**
-     * Creates a new link (directory entry) for an existing file <i>(optional
+     * Creates a new link (directory entry) for an existing file,
+    * failing if {@code link} locates an existing file <i>(optional


Minor nit, I think the comment has got mis-aligned.

Fixed in 9cf6ede.

AlanBateman

Good, thanks for the updates.

A coin toss as to whether it needs a CSR as there isn't anything new or changed.

bplb · 2025-11-19T19:26:06Z

Good, thanks for the updates.

Thanks for the review.

Are we not concerned about the other FAEEs mentioned above?

AlanBateman · 2025-11-19T19:46:17Z

Are we not concerned about the other FAEEs mentioned above?

Some of them are okay, e.g. copy has "if the target file exists but ...".

If you have cycles, there are 7 cases that have "If a file of that name already exists and the CREATE_NEW ..." that could be improved if changed to "If the path locates an existing file and the CREATE_NEW ...".

bplb · 2025-11-19T20:01:04Z

If you have cycles, there are 7 cases [...] that could be improved [...]

Yes, I will change them.

bplb · 2025-11-19T20:08:17Z

If you have cycles, there are 7 cases that have "If a file of that name already exists and the CREATE_NEW ..." that could be improved if changed to "If the path locates an existing file and the CREATE_NEW ...".

Addressed in commit 3684b8c.

jaikiran · 2025-11-20T09:23:03Z

Hello Brian, the changes look reasonable to me.

Having said that, the JBS issue description states that the usage of the word "file" makes it sound like the exception is raised only if a regular file exists at the Path. Specifically, the previous wording was:

If a file of that name already exists

and with the proposed change we now have:

If the path locates an existing file

So, I think, that wording will still leave readers wondering whether existing directory at that Path are considered when throwing that exception.

I am not proposing further changes though (I can't think of something that's more concise). Files.exists(Path) too uses the word "file":

Tests whether a file exists.

and it checks even for directories, so I think it's all consistent with the rest of the API docs.

AlanBateman · 2025-11-20T09:31:42Z

Having said that, the JBS issue description states that the usage of the word "file" makes it sound like the exception is raised only if a regular file exists

I think it's best to keep the API docs simple and consistent. I'm sure there are developers that don't think of a directory or sym files as "files" but once you attempt to clarify things then it gets complicated and it could potentially confusing a wider set of developers.

8220816: (fs) Files.createDirectory should make it more obvious that …

4ab0ccb

…it fails when the directory already exists

openjdk bot added the nio nio-dev@openjdk.org label Nov 18, 2025

openjdk bot added the rfr Pull request is ready for review label Nov 18, 2025

8220816: "directory" -> "file or directory"

358bea7

justin-curtis-lu reviewed Nov 18, 2025

View reviewed changes

8220816: Update throws verbiage to match

3af3490

RogerRiggs reviewed Nov 19, 2025

View reviewed changes

8220816: Improve consistency per review comments

5c62cd5

AlanBateman reviewed Nov 19, 2025

View reviewed changes

8220816: Address more review comments

9cf6ede

AlanBateman approved these changes Nov 19, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Nov 19, 2025

8220816: Change FAEE for CREATE_NEW cases

3684b8c

AlanBateman approved these changes Nov 19, 2025

View reviewed changes

openjdk bot removed the ready Pull request is ready to be integrated label Nov 19, 2025

openjdk bot added the ready Pull request is ready to be integrated label Nov 19, 2025

jaikiran approved these changes Nov 20, 2025

View reviewed changes

8220816: (fs) Files.createDirectory should make it more obvious that it fails when the directory already exists #28378

Are you sure you want to change the base?

8220816: (fs) Files.createDirectory should make it more obvious that it fails when the directory already exists #28378

Conversation

bplb commented Nov 18, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Nov 18, 2025

Uh oh!

openjdk bot commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mlbridge bot commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

justin-curtis-lu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bplb commented Nov 18, 2025

Uh oh!

bplb commented Nov 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlanBateman commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bplb commented Nov 19, 2025

Uh oh!

bplb commented Nov 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlanBateman left a comment

Choose a reason for hiding this comment

Uh oh!

bplb commented Nov 19, 2025

Uh oh!

AlanBateman commented Nov 19, 2025

Uh oh!

bplb commented Nov 19, 2025

Uh oh!

bplb commented Nov 19, 2025

Uh oh!

jaikiran commented Nov 20, 2025

Uh oh!

AlanBateman commented Nov 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

bplb commented Nov 18, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Nov 18, 2025 •

edited

Loading

openjdk bot commented Nov 18, 2025 •

edited

Loading

mlbridge bot commented Nov 18, 2025 •

edited

Loading

AlanBateman commented Nov 19, 2025 •

edited

Loading