From 48ac398f1121630c4cdab7f0259e9859cd2d0343 Mon Sep 17 00:00:00 2001 From: James Lin Date: Mon, 18 Dec 2023 20:23:21 +0000 Subject: [PATCH] Fix `Directory.current` setter compatibility with `IOOverrides` The `Directory.current` setter has a `dynamic` parameter so that it can accept either `String` or `Directory` arguments. (This is asymmetric with the getter, which always returns a `Directory`.) The corresponding `IOOverrides` callback, however, assumes that the argument is always a `String`, and `Directory.current` passed its `dynamic` argument through unchanged. Consequently, overriding the `Directory.current` setter would result in a `TypeError` when setting `Directory.current` to a `Directory` object. Changing `IOOverrides.setCurrentDirectory` to use a `dynamic` parameter would be a breaking change, so instead make the `Directory.current` setter check the argument's runtime type before passing it along. Bug: https://github.com/dart-lang/sdk/issues/52140 Change-Id: I3c5bba6b442b314c798bd7949dfeb5eb6251dc6e CoreLibraryReviewExempt: No API changes and VM-only. Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/336604 Reviewed-by: Brian Quinlan Reviewed-by: Lasse Nielsen Auto-Submit: James Lin Commit-Queue: Brian Quinlan --- sdk/lib/io/directory.dart | 17 +++++++++++++-- sdk/lib/io/directory_impl.dart | 25 +++++++++-------------- tests/standalone/io/io_override_test.dart | 20 ++++++++++++++---- 3 files changed, 41 insertions(+), 21 deletions(-) diff --git a/sdk/lib/io/directory.dart b/sdk/lib/io/directory.dart index e7aa6bc4fb8..88e3ef47c98 100644 --- a/sdk/lib/io/directory.dart +++ b/sdk/lib/io/directory.dart @@ -162,13 +162,26 @@ abstract interface class Directory implements FileSystemEntity { /// operations and multiple isolates. Changing the working directory, /// while asynchronous operations are pending or when other isolates /// are working with the file system, can lead to unexpected results. - static void set current(path) { + static void set current(dynamic path) { + // Disallow implicit casts to avoid bugs like + // . + // + // This can be removed if `strict-casts` is enabled. + path as Object?; + final IOOverrides? overrides = IOOverrides.current; if (overrides == null) { _Directory.current = path; return; } - overrides.setCurrentDirectory(path); + + // IOOverrides.setCurrentDirectory accepts only a [String]. + overrides.setCurrentDirectory(switch (path) { + String s => s, + Directory d => d.path, + _ => throw ArgumentError('${Error.safeToString(path)} is not a String or' + ' Directory'), + }); } /// Creates the directory if it doesn't exist. diff --git a/sdk/lib/io/directory_impl.dart b/sdk/lib/io/directory_impl.dart index c8ed9dfb14f..afdbed4db5b 100644 --- a/sdk/lib/io/directory_impl.dart +++ b/sdk/lib/io/directory_impl.dart @@ -45,23 +45,18 @@ class _Directory extends FileSystemEntity implements Directory { return new _Directory(result); } - static void set current(path) { - late Uint8List _rawPath; - if (path is _Directory) { + static void set current(Object? path) { + var _rawPath = switch (path) { // For our internal Directory implementation, go ahead and use the raw // path. - _rawPath = path._rawPath; - } else if (path is Directory) { - // FIXME(bkonyi): package:file passes in instances of classes which do - // not have _path defined, so we will fallback to using the existing - // path String for now. - _rawPath = FileSystemEntity._toUtf8Array(path.path); - } else if (path is String) { - _rawPath = FileSystemEntity._toUtf8Array(path); - } else { - throw new ArgumentError('${Error.safeToString(path)} is not a String or' - ' Directory'); - } + _Directory d => d._rawPath, + // Fall back to the String-based path. + Directory d => FileSystemEntity._toUtf8Array(d.path), + String s => FileSystemEntity._toUtf8Array(s), + _ => throw ArgumentError('${Error.safeToString(path)} is not a String or' + ' Directory') + }; + if (!_EmbedderConfig._mayChdir) { throw new UnsupportedError( "This embedder disallows setting Directory.current"); diff --git a/tests/standalone/io/io_override_test.dart b/tests/standalone/io/io_override_test.dart index ad87f00956c..79d3925478c 100644 --- a/tests/standalone/io/io_override_test.dart +++ b/tests/standalone/io/io_override_test.dart @@ -12,13 +12,16 @@ import "package:expect/expect.dart"; class DirectoryMock extends FileSystemEntity implements Directory { static final _mockUri = Uri.parse('http:///mockdir/'); - final String path = "/mockdir"; + final String path; - DirectoryMock(String path); + DirectoryMock(this.path); + + static DirectoryMock _currentDirectory = DirectoryMock(""); static DirectoryMock createDirectory(String path) => new DirectoryMock(path); - static DirectoryMock getCurrent() => new DirectoryMock(""); - static void setCurrent(String path) {} + static DirectoryMock getCurrent() => _currentDirectory; + static void setCurrent(String path) => + _currentDirectory = DirectoryMock(path); static DirectoryMock getSystemTemp() => new DirectoryMock(""); Uri get uri => _mockUri; @@ -207,6 +210,15 @@ Future ioOverridesRunTest() async { () async { Expect.isTrue(new Directory("directory") is DirectoryMock); Expect.isTrue(Directory.current is DirectoryMock); + + const mockDirectoryPath1 = "/mockDirectory1"; + const mockDirectoryPath2 = "/mockDirectory2"; + + Directory.current = mockDirectoryPath1; + Expect.isTrue(Directory.current.path == mockDirectoryPath1); + Directory.current = Directory(mockDirectoryPath2); + Expect.isTrue(Directory.current.path == mockDirectoryPath2); + Expect.identical(Uri.base, DirectoryMock._mockUri); Expect.isTrue(Directory.systemTemp is DirectoryMock); Expect.isTrue(new File("file") is FileMock);