BREAKING CHANGE: remove Deno.OpenMode (#4884)

This commit removes Deno.OpenMode along with overloaded variants
of Deno.open() and Deno.openSync() that used OpenMode.

cli/js/deno.ts

@@ -35,7 +35,6 @@ export {
   seek,
   seekSync,
   OpenOptions,
-  OpenMode,
 } from "./files.ts";
 export { read, readSync, write, writeSync } from "./ops/io.ts";
 export { FsEvent, watchFs } from "./ops/fs_events.ts";

cli/js/files.ts

@@ -18,60 +18,43 @@ import {
   open as opOpen,
   openSync as opOpenSync,
   OpenOptions,
-  OpenMode,
 } from "./ops/fs/open.ts";
-export { OpenOptions, OpenMode } from "./ops/fs/open.ts";
+export { OpenOptions } from "./ops/fs/open.ts";
 
-export function openSync(path: string, options?: OpenOptions): File;
-export function openSync(path: string, openMode?: OpenMode): File;
-
-/**@internal*/
 export function openSync(
   path: string,
-  modeOrOptions: OpenOptions | OpenMode = "r"
+  options: OpenOptions = { read: true }
 ): File {
-  let openMode = undefined;
-  let options = undefined;
-
-  if (typeof modeOrOptions === "string") {
-    openMode = modeOrOptions;
-  } else {
-    checkOpenOptions(modeOrOptions);
-    options = modeOrOptions as OpenOptions;
-  }
-
-  const rid = opOpenSync(path, openMode as OpenMode, options);
+  checkOpenOptions(options);
+  const rid = opOpenSync(path, options);
   return new File(rid);
 }
 
-export async function open(path: string, options?: OpenOptions): Promise<File>;
-export async function open(path: string, openMode?: OpenMode): Promise<File>;
-
-/**@internal*/
 export async function open(
   path: string,
-  modeOrOptions: OpenOptions | OpenMode = "r"
+  options: OpenOptions = { read: true }
 ): Promise<File> {
-  let openMode = undefined;
-  let options = undefined;
-
-  if (typeof modeOrOptions === "string") {
-    openMode = modeOrOptions;
-  } else {
-    checkOpenOptions(modeOrOptions);
-    options = modeOrOptions as OpenOptions;
-  }
-
-  const rid = await opOpen(path, openMode as OpenMode, options);
+  checkOpenOptions(options);
+  const rid = await opOpen(path, options);
   return new File(rid);
 }
 
 export function createSync(path: string): File {
-  return openSync(path, "w+");
+  return openSync(path, {
+    read: true,
+    write: true,
+    truncate: true,
+    create: true,
+  });
 }
 
 export function create(path: string): Promise<File> {
-  return open(path, "w+");
+  return open(path, {
+    read: true,
+    write: true,
+    truncate: true,
+    create: true,
+  });
 }
 
 export class File

cli/js/lib.deno.ns.d.ts

@@ -653,18 +653,6 @@ declare namespace Deno {
    */
   export function openSync(path: string, options?: OpenOptions): File;
 
-  /** Synchronously open a file and return an instance of `Deno.File`.  The file
-   * may be created depending on the mode passed in.  It is the callers responsibility
-   * to close the file when finished with it.
-   *
-   *       const file = Deno.openSync("/foo/bar.txt", "r");
-   *       // Do work with file
-   *       Deno.close(file.rid);
-   *
-   * Requires `allow-read` and/or `allow-write` permissions depending on openMode.
-   */
-  export function openSync(path: string, openMode?: OpenMode): File;
-
   /** Open a file and resolve to an instance of `Deno.File`.  The
    * file does not need to previously exist if using the `create` or `createNew`
    * open options.  It is the callers responsibility to close the file when finished
@@ -678,18 +666,6 @@ declare namespace Deno {
    */
   export function open(path: string, options?: OpenOptions): Promise<File>;
 
-  /** Open a file and resolve to an instance of `Deno.File`.  The file may be
-   * created depending on the mode passed in.  It is the callers responsibility
-   * to close the file when finished with it.
-   *
-   *       const file = await Deno.open("/foo/bar.txt", "w+");
-   *       // Do work with file
-   *       Deno.close(file.rid);
-   *
-   * Requires `allow-read` and/or `allow-write` permissions depending on openMode.
-   */
-  export function open(path: string, openMode?: OpenMode): Promise<File>;
-
   /** Creates a file if none exists or truncates an existing file and returns
    *  an instance of `Deno.File`.
    *
@@ -890,21 +866,6 @@ declare namespace Deno {
     mode?: number;
   }
 
-  /** A set of string literals which specify how to open a file.
-   *
-   * |Value |Description                                                                                       |
-   * |------|--------------------------------------------------------------------------------------------------|
-   * |`"r"` |Read-only. Default. Starts at beginning of file.                                                  |
-   * |`"r+"`|Read-write. Start at beginning of file.                                                           |
-   * |`"w"` |Write-only. Opens and truncates existing file or creates new one for writing only.                |
-   * |`"w+"`|Read-write. Opens and truncates existing file or creates new one for writing and reading.         |
-   * |`"a"` |Write-only. Opens existing file or creates new one. Each write appends content to the end of file.|
-   * |`"a+"`|Read-write. Behaves like `"a"` and allows to read from file.                                      |
-   * |`"x"` |Write-only. Exclusive create - creates new file only if one doesn't exist already.                |
-   * |`"x+"`|Read-write. Behaves like `x` and allows reading from file.                                        |
-   */
-  export type OpenMode = "r" | "r+" | "w" | "w+" | "a" | "a+" | "x" | "x+";
-
   /** **UNSTABLE**: new API, yet to be vetted
    *
    *  Check if a given resource id (`rid`) is a TTY.

cli/js/ops/fs/open.ts

@@ -15,27 +15,16 @@ export interface OpenOptions {
   mode?: number;
 }
 
-export type OpenMode = "r" | "r+" | "w" | "w+" | "a" | "a+" | "x" | "x+";
-
-export function openSync(
-  path: string,
-  openMode: OpenMode | undefined,
-  options: OpenOptions | undefined
-): number {
+export function openSync(path: string, options: OpenOptions): number {
   const mode: number | undefined = options?.mode;
-  return sendSync("op_open", { path, options, openMode, mode });
+  return sendSync("op_open", { path, options, mode });
 }
 
-export function open(
-  path: string,
-  openMode: OpenMode | undefined,
-  options: OpenOptions | undefined
-): Promise<number> {
+export function open(path: string, options: OpenOptions): Promise<number> {
   const mode: number | undefined = options?.mode;
   return sendAsync("op_open", {
     path,
     options,
-    openMode,
     mode,
   });
 }

cli/js/tests/files_test.ts

@@ -201,11 +201,11 @@ unitTest(
   { perms: { write: false } },
   async function writePermFailure(): Promise<void> {
     const filename = "tests/hello.txt";
-    const writeModes: Deno.OpenMode[] = ["w", "a", "x"];
-    for (const mode of writeModes) {
+    const openOptions: Deno.OpenOptions[] = [{ write: true }, { append: true }];
+    for (const options of openOptions) {
       let err;
       try {
-        await Deno.open(filename, mode);
+        await Deno.open(filename, options);
       } catch (e) {
         err = e;
       }
@@ -266,8 +266,8 @@ unitTest({ perms: { read: false } }, async function readPermFailure(): Promise<
 > {
   let caughtError = false;
   try {
-    await Deno.open("package.json", "r");
-    await Deno.open("cli/tests/fixture.json", "r");
+    await Deno.open("package.json", { read: true });
+    await Deno.open("cli/tests/fixture.json", { read: true });
   } catch (e) {
     caughtError = true;
     assert(e instanceof Deno.errors.PermissionDenied);
@@ -308,7 +308,12 @@ unitTest(
   async function readNullBufferFailure(): Promise<void> {
     const tempDir = Deno.makeTempDirSync();
     const filename = tempDir + "hello.txt";
-    const file = await Deno.open(filename, "w+");
+    const file = await Deno.open(filename, {
+      read: true,
+      write: true,
+      truncate: true,
+      create: true,
+    });
 
     // reading into an empty buffer should return 0 immediately
     const bytesRead = await file.read(new Uint8Array(0));
@@ -334,18 +339,15 @@ unitTest(
   { perms: { write: false, read: false } },
   async function readWritePermFailure(): Promise<void> {
     const filename = "tests/hello.txt";
-    const writeModes: Deno.OpenMode[] = ["r+", "w+", "a+", "x+"];
-    for (const mode of writeModes) {
-      let err;
-      try {
-        await Deno.open(filename, mode);
-      } catch (e) {
-        err = e;
-      }
-      assert(!!err);
-      assert(err instanceof Deno.errors.PermissionDenied);
-      assertEquals(err.name, "PermissionDenied");
+    let err;
+    try {
+      await Deno.open(filename, { read: true });
+    } catch (e) {
+      err = e;
     }
+    assert(!!err);
+    assert(err instanceof Deno.errors.PermissionDenied);
+    assertEquals(err.name, "PermissionDenied");
   }
 );
 
@@ -377,7 +379,11 @@ unitTest(
     const encoder = new TextEncoder();
     const filename = tempDir + "hello.txt";
     const data = encoder.encode("Hello world!\n");
-    let file = await Deno.open(filename, "w");
+    let file = await Deno.open(filename, {
+      create: true,
+      write: true,
+      truncate: true,
+    });
     // assert file was created
     let fileInfo = Deno.statSync(filename);
     assert(fileInfo.isFile);
@@ -398,7 +404,10 @@ unitTest(
     }
     file.close();
     // assert that existing file is truncated on open
-    file = await Deno.open(filename, "w");
+    file = await Deno.open(filename, {
+      write: true,
+      truncate: true,
+    });
     file.close();
     const fileSize = Deno.statSync(filename).size;
     assertEquals(fileSize, 0);
@@ -414,7 +423,12 @@ unitTest(
     const filename = tempDir + "hello.txt";
     const data = encoder.encode("Hello world!\n");
 
-    const file = await Deno.open(filename, "w+");
+    const file = await Deno.open(filename, {
+      write: true,
+      truncate: true,
+      create: true,
+      read: true,
+    });
     const seekPosition = 0;
     // assert file was created
     let fileInfo = Deno.statSync(filename);

cli/js/tests/process_test.ts

@@ -229,7 +229,10 @@ unitTest(
   async function runRedirectStdoutStderr(): Promise<void> {
     const tempDir = await makeTempDir();
     const fileName = tempDir + "/redirected_stdio.txt";
-    const file = await open(fileName, "w");
+    const file = await open(fileName, {
+      create: true,
+      write: true,
+    });
 
     const p = run({
       cmd: [
@@ -261,7 +264,7 @@ unitTest(
     const fileName = tempDir + "/redirected_stdio.txt";
     const encoder = new TextEncoder();
     await writeFile(fileName, encoder.encode("hello"));
-    const file = await open(fileName, "r");
+    const file = await open(fileName);
 
     const p = run({
       cmd: ["python", "-c", "import sys; assert 'hello' == sys.stdin.read();"],

cli/js/write_file.ts

@@ -24,8 +24,10 @@ export function writeFileSync(
     }
   }
 
-  const openMode = !!options.append ? "a" : "w";
-  const file = openSync(path, openMode);
+  const openOptions = !!options.append
+    ? { write: true, create: true, append: true }
+    : { write: true, create: true, truncate: true };
+  const file = openSync(path, openOptions);
 
   if (
     options.mode !== undefined &&
@@ -52,8 +54,10 @@ export async function writeFile(
     }
   }
 
-  const openMode = !!options.append ? "a" : "w";
-  const file = await open(path, openMode);
+  const openOptions = !!options.append
+    ? { write: true, create: true, append: true }
+    : { write: true, create: true, truncate: true };
+  const file = await open(path, openOptions);
 
   if (
     options.mode !== undefined &&

cli/ops/fs.rs

@@ -50,8 +50,7 @@ fn into_string(s: std::ffi::OsString) -> Result<String, OpError> {
 struct OpenArgs {
   promise_id: Option<u64>,
   path: String,
-  options: Option<OpenOptions>,
-  open_mode: Option<String>,
+  options: OpenOptions,
   mode: Option<u32>,
 }
 
@@ -91,76 +90,22 @@ fn op_open(
     let _ = mode; // avoid unused warning
   }
 
-  if let Some(options) = args.options {
-    if options.read {
-      state.check_read(&path)?;
-    }
+  let options = args.options;
+  if options.read {
+    state.check_read(&path)?;
+  }
 
-    if options.write || options.append {
-      state.check_write(&path)?;
-    }
+  if options.write || options.append {
+    state.check_write(&path)?;
+  }
 
-    open_options
-      .read(options.read)
-      .create(options.create)
-      .write(options.write)
-      .truncate(options.truncate)
-      .append(options.append)
-      .create_new(options.create_new);
-  } else if let Some(open_mode) = args.open_mode {
-    let open_mode = open_mode.as_ref();
-    match open_mode {
-      "r" => {
-        state.check_read(&path)?;
-      }
-      "w" | "a" | "x" => {
-        state.check_write(&path)?;
-      }
-      &_ => {
-        state.check_read(&path)?;
-        state.check_write(&path)?;
-      }
-    };
-
-    match open_mode {
-      "r" => {
-        open_options.read(true);
-      }
-      "r+" => {
-        open_options.read(true).write(true);
-      }
-      "w" => {
-        open_options.create(true).write(true).truncate(true);
-      }
-      "w+" => {
-        open_options
-          .read(true)
-          .create(true)
-          .write(true)
-          .truncate(true);
-      }
-      "a" => {
-        open_options.create(true).append(true);
-      }
-      "a+" => {
-        open_options.read(true).create(true).append(true);
-      }
-      "x" => {
-        open_options.create_new(true).write(true);
-      }
-      "x+" => {
-        open_options.create_new(true).read(true).write(true);
-      }
-      &_ => {
-        // TODO: this should be type error
-        return Err(OpError::other("Unknown open mode.".to_string()));
-      }
-    }
-  } else {
-    return Err(OpError::other(
-      "Open requires either openMode or options.".to_string(),
-    ));
-  };
+  open_options
+    .read(options.read)
+    .create(options.create)
+    .write(options.write)
+    .truncate(options.truncate)
+    .append(options.append)
+    .create_new(options.create_new);
 
   let is_sync = args.promise_id.is_none();
 

cli/tests/044_bad_resource.ts

@@ -1,5 +1,5 @@
 async function main(): Promise<void> {
-  const file = await Deno.open("044_bad_resource.ts", "r");
+  const file = await Deno.open("044_bad_resource.ts", { read: true });
   file.close();
   await file.seek(10, 0);
 }

std/archive/tar.ts

@@ -39,15 +39,15 @@ const ustar = "ustar\u000000";
 class FileReader implements Deno.Reader {
   private file?: Deno.File;
 
-  constructor(private filePath: string, private mode: Deno.OpenMode = "r") {}
+  constructor(private filePath: string) {}
 
   public async read(p: Uint8Array): Promise<number | Deno.EOF> {
     if (!this.file) {
-      this.file = await Deno.open(this.filePath, this.mode);
+      this.file = await Deno.open(this.filePath, { read: true });
     }
     const res = await Deno.read(this.file.rid, p);
     if (res === Deno.EOF) {
-      await Deno.close(this.file.rid);
+      Deno.close(this.file.rid);
       this.file = undefined;
     }
     return res;

std/fs/walk_test.ts

@@ -1,4 +1,4 @@
-const { cwd, chdir, makeTempDir, mkdir, open, symlink } = Deno;
+const { cwd, chdir, makeTempDir, mkdir, create, symlink } = Deno;
 const { remove } = Deno;
 import { walk, walkSync, WalkOptions, WalkEntry } from "./walk.ts";
 import { assert, assertEquals, assertThrowsAsync } from "../testing/asserts.ts";
@@ -46,7 +46,7 @@ export async function walkArray(
 }
 
 export async function touch(path: string): Promise<void> {
-  const f = await open(path, "w");
+  const f = await create(path);
   f.close();
 }
 

std/io/util.ts

@@ -47,6 +47,11 @@ export async function tempFile(
     `${dir}/${opts.prefix || ""}${r}${opts.postfix || ""}`
   );
   await mkdir(path.dirname(filepath), { recursive: true });
-  const file = await open(filepath, "a");
+  const file = await open(filepath, {
+    create: true,
+    read: true,
+    write: true,
+    append: true,
+  });
   return { file, filepath };
 }

std/log/handlers.ts

@@ -2,6 +2,7 @@
 const { open, openSync, close, renameSync, statSync } = Deno;
 type File = Deno.File;
 type Writer = Deno.Writer;
+type OpenOptions = Deno.OpenOptions;
 import { getLevelByName, LogLevel } from "./levels.ts";
 import { LogRecord } from "./logger.ts";
 import { red, yellow, blue, bold } from "../fmt/colors.ts";
@@ -101,6 +102,7 @@ export class FileHandler extends WriterHandler {
   protected _file!: File;
   protected _filename: string;
   protected _mode: LogMode;
+  protected _openOptions: OpenOptions;
   #encoder = new TextEncoder();
 
   constructor(levelName: string, options: FileHandlerOptions) {
@@ -108,10 +110,17 @@ export class FileHandler extends WriterHandler {
     this._filename = options.filename;
     // default to append mode, write only
     this._mode = options.mode ? options.mode : "a";
+    this._openOptions = {
+      createNew: this._mode === "x",
+      create: this._mode !== "x",
+      append: this._mode === "a",
+      truncate: this._mode !== "a",
+      write: true,
+    };
   }
 
   async setup(): Promise<void> {
-    this._file = await open(this._filename, this._mode);
+    this._file = await open(this._filename, this._openOptions);
     this._writer = this._file;
   }
 
@@ -119,8 +128,9 @@ export class FileHandler extends WriterHandler {
     Deno.writeSync(this._file.rid, this.#encoder.encode(msg + "\n"));
   }
 
-  async destroy(): Promise<void> {
-    await this._file.close();
+  destroy(): Promise<void> {
+    this._file.close();
+    return Promise.resolve();
   }
 }
 
@@ -193,7 +203,7 @@ export class RotatingFileHandler extends FileHandler {
       }
     }
 
-    this._file = openSync(this._filename, this._mode);
+    this._file = openSync(this._filename, this._openOptions);
     this._writer = this._file;
   }
 }

std/mime/multipart_test.ts

@@ -93,7 +93,9 @@ test(async function multipartMultipartWriter(): Promise<void> {
   const mw = new MultipartWriter(buf);
   await mw.writeField("foo", "foo");
   await mw.writeField("bar", "bar");
-  const f = await open(path.resolve("./mime/testdata/sample.txt"), "r");
+  const f = await open(path.resolve("./mime/testdata/sample.txt"), {
+    read: true,
+  });
   await mw.writeFile("file", "sample.txt", f);
   await mw.close();
   f.close();