feature: Accept plain String paths on IO/FileSystem#550
Conversation
Wrapping every file path with IO.path(...) before calling IO.readString
and friends was pure boilerplate — most callers just have a path string,
and the wrapper littered every code sample in the docs.
Add String-accepting overloads on FileSystemBase, Gzip, and Zip that
delegate to the existing IOPath methods via IOPath.parse. The overloads
cascade automatically through `export FileSystem.{...}` on IO. IO.path
remains for composition (`/`), normalization, and path-part queries.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces String-accepting overloads for path-based operations in the IO, FileSystem, Gzip, and Zip APIs, allowing for more concise code by removing the requirement for explicit IOPath wrapping. The update includes extensive documentation revisions and new test cases to validate the overloads. Feedback identifies a redundant DummyImplicit parameter in the Zip.create method that should be removed to clean up the API signature.
| def create(target: IOPath, sources: Seq[IOPath]): Unit | ||
|
|
||
| /** String-path overload of [[create]]. */ | ||
| def create(target: String, sources: Seq[String])(using DummyImplicit): Unit = create( |
There was a problem hiding this comment.
The DummyImplicit parameter appears to be redundant here. Since the first parameter target is a String (whereas the original method takes an IOPath), the erased signatures of the two create overloads are already distinct (create(String, Seq) vs create(IOPath, Seq)). DummyImplicit is typically only necessary when the signatures are identical after erasure, which usually happens when only the generic type parameters differ.
def create(target: String, sources: Seq[String]): Unit = create(There was a problem hiding this comment.
Good catch — removed in 9214c18. Verified the two overloads compile and resolve correctly on JVM/JS/Native because the first parameter keeps the erased signatures distinct (IOPath vs String).
Codex review flagged that the filesystem.md "every IO operation that takes a path accepts a String" claim overpromised: - IO.watch still required IOPath - createTempFile(directory = Option[IOPath]) still requires IOPath Add a String-path overload for IOWatch.watch (with a new test) so the common watch case follows the same pattern as read/write/list. Narrow the docs wording to acknowledge the remaining Option[IOPath] case. Also add a comment on Zip.create(String, Seq[String])(using DummyImplicit) explaining why the implicit parameter is needed (JVM erasure clash with the IOPath, Seq[IOPath] overload). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Switch the new String-path tests from s"\${dir.posixPath}/file.txt" to
(dir / "file.txt").posixPath. Same behavior, but reuses IOPath's path
joining semantics instead of hand-rolling separators in the tests.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
/gemini review |
Gemini pointed out the DummyImplicit isn't needed — IOPath and String are distinct concrete types, so create(IOPath, Seq[IOPath]) and create(String, Seq[String]) already have distinct erased signatures (Seq[T] erases to Seq, but the first parameter is IOPath vs String). Verified by removing DummyImplicit: compiles on JVM/JS/Native and all ZipTest cases still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces String-accepting overloads for path-based operations across the IO, FileSystem, Gzip, and Zip APIs, reducing the need for explicit IOPath wrapping. The changes include extensive updates to documentation and examples to reflect the simplified usage, along with new test cases verifying the overloads. Regarding the implementation in Zip.scala, it is recommended to use the @targetName annotation instead of DummyImplicit to resolve JVM erasure clashes, as it is the idiomatic Scala 3 approach.
| IOPath.parse(target), | ||
| sources.map(IOPath.parse) | ||
| ) | ||
|
|
There was a problem hiding this comment.
Resolves conflict in uni/src/main/scala/wvlet/uni/control/IO.scala: - main (#548): removed FileSystemInit.init() in favor of auto-init - this branch: removed IOPath() wrap since FileSystem.readString now accepts String directly - Result: neither import is needed; keep only wvlet.uni.io.FileSystem. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
IO.readString,IO.writeString, and the rest of theIO/FileSystemsurface now accept a plainStringpath in addition toIOPath, removing theIO.path(...)wrapper from the common case.Gzip.compressFile/decompressFile,Zip.create/extract/list, andIOWatch.watch.IO.path(...)stays — it remains the canonical way to build anIOPathfor composition (/), normalization, or part-queries (fileName,parent,extension).docs/core/filesystem.md,docs/core/io.md,docs/uni-walkthrough.md) rewritten to show plain-string usage. The doc note narrows the claim so it doesn't overpromise for the remainingOption[IOPath]cases (e.g.createTempFile'sdirectoryparameter).Why overloads, not
given Conversion[String, IOPath]?A Scala 3
Conversionstill requiresimport scala.language.implicitConversionsat each call site, which defeats the simpler-docs goal. Overloads are zero-ceremony and IDE-discoverable; the cost is ~30 one-line delegates concentrated inFileSystemBase.Example
Test plan
./sbt scalafmtAll./sbt coreJVM/test uniJVM/test— 1613 tests pass, 4 pending./sbt uniJS/testOnly *IO*— 28 JS tests pass./sbt coreJS/compile coreNative/compile uniJS/test:compile uniNative/test:compileIOFileSystemTestcases covering read/write, write modes, list/delete, copy/move withStringpathsGzipTest,ZipTest, andIOWatchJvmTestcases exercising the newStringoverloads🤖 Generated with Claude Code