Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move JDK shims to munit.internal #441

Merged
merged 7 commits into from
Apr 10, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .jvmopts
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,4 @@
-Xmx2G
-XX:ReservedCodeCacheSize=1024m
-XX:+TieredCompilation
-XX:+CMSClassUnloadingEnabled
-Dfile.encoding=UTF-8
28 changes: 0 additions & 28 deletions munit/js/src/main/scala/java/nio/file/Path.scala

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
package java.io
package munit.internal

import java.net.URI
import java.nio.file.Path
import munit.internal.JSIO
import munit.internal.NodeNIOPath

// obtained implementation by experimentation on the JDK.
class File(path: String) {
Expand All @@ -20,7 +18,7 @@ class File(path: String) {
}
)
def toPath: Path =
NodeNIOPath(path)
Path(path)
def toURI: URI = {
val file = getAbsoluteFile.toString
val uripath =
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package java.nio.file
package munit.internal

import scala.scalajs.js
import java.{util => ju}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
package java.lang.reflect
package munit.internal

class InvocationTargetException extends java.lang.RuntimeException
Original file line number Diff line number Diff line change
@@ -1,70 +1,63 @@
package munit.internal

import java.io.File
import java.net.URI
import java.nio.file.WatchEvent.{Kind, Modifier}
import java.nio.file._
import java.util

import scala.collection.JavaConverters._

// Rough implementation of java.nio.Path, should work similarly for the happy
// path but has undefined behavior for error handling.
case class NodeNIOPath(filename: String) extends Path {
case class Path(filename: String) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's call this MUnitPath instead of Path. Or something else if you have suggestions. I don't like it when libraries use generic names for internal classes because it pollutes the namespace when you're auto-completing symbols.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, done.

private[this] val escapedSeparator =
java.util.regex.Pattern.quote(File.separator)
def getFileSystem(): FileSystem = ???
def toRealPath(x: LinkOption*): Path = ???
def register(x: WatchService, y: Array[Kind[_]], z: Modifier*): WatchKey = ???
def register(x: WatchService, y: Kind[_]*): WatchKey = ???
def compareTo(x: Path): Int = ???

private def adjustIndex(idx: Int): Int =
if (isAbsolute) idx + 1 else idx
override def subpath(beginIndex: Int, endIndex: Int): Path =
NodeNIOPath(
def subpath(beginIndex: Int, endIndex: Int): Path =
Path(
filename
.split(escapedSeparator)
.slice(adjustIndex(beginIndex), adjustIndex(endIndex))
.mkString
)
override def toFile: File =
def toFile: File =
new File(filename)
override def isAbsolute: Boolean = JSIO.path match {
def isAbsolute: Boolean = JSIO.path match {
case Some(path) => path.isAbsolute(filename).asInstanceOf[Boolean]
case None => filename.startsWith(File.separator)
}
override def getName(index: Int): Path =
NodeNIOPath(
def getName(index: Int): Path =
Path(
filename
.split(escapedSeparator)
.lift(adjustIndex(index))
.getOrElse(throw new IllegalArgumentException)
)
override def getParent: Path =
def getParent: Path =
JSIO.path match {
case Some(path) =>
NodeNIOPath(path.dirname(filename).asInstanceOf[String])
Path(path.dirname(filename).asInstanceOf[String])
case None =>
throw new UnsupportedOperationException(
"Path.getParent() is only supported in Node.js"
)
}

override def toAbsolutePath: Path =
def toAbsolutePath: Path =
if (isAbsolute) this
else NodeNIOPath.workingDirectory.resolve(this)
override def relativize(other: Path): Path =
else Path.workingDirectory.resolve(this)
def relativize(other: Path): Path =
JSIO.path match {
case Some(path) =>
NodeNIOPath(
Path(
path.relative(filename, other.toString()).asInstanceOf[String]
)
case None =>
throw new UnsupportedOperationException(
"Path.relativize() is only supported in Node.js"
)
}
override def getNameCount: Int = {
def getNameCount: Int = {
val strippeddrive =
if ((filename.length > 1) && (filename(1) == ':')) filename.substring(2)
else filename
Expand All @@ -73,44 +66,44 @@ case class NodeNIOPath(filename: String) extends Path {
if (remaining.isEmpty) first.length
else remaining.length
}
override def toUri: URI = toFile.toURI
override def getFileName: Path =
def toUri: URI = toFile.toURI
def getFileName: Path =
JSIO.path match {
case Some(path) =>
NodeNIOPath(path.basename(filename).asInstanceOf[String])
Path(path.basename(filename).asInstanceOf[String])
case None =>
throw new UnsupportedOperationException(
"Path.getFileName() is only supported in Node.js"
)
}
override def getRoot: Path =
def getRoot: Path =
if (!isAbsolute) null
else NodeNIOPath(File.separator)
override def normalize(): Path =
else Path(File.separator)
def normalize(): Path =
JSIO.path match {
case Some(path) =>
NodeNIOPath(path.normalize(filename).asInstanceOf[String])
Path(path.normalize(filename).asInstanceOf[String])
case None =>
throw new UnsupportedOperationException(
"Path.normalize() is only supported in Node.js"
)
}
override def endsWith(other: Path): Boolean =
def endsWith(other: Path): Boolean =
endsWith(other.toString)
override def endsWith(other: String): Boolean =
def endsWith(other: String): Boolean =
paths(filename).endsWith(paths(other))
// JSPath.resolve(relpath, relpath) produces an absolute path from cwd.
// This method turns the generated absolute path back into a relative path.
private def adjustResolvedPath(resolved: Path): Path =
if (isAbsolute) resolved
else NodeNIOPath.workingDirectory.relativize(resolved)
override def resolveSibling(other: Path): Path =
else Path.workingDirectory.relativize(resolved)
def resolveSibling(other: Path): Path =
resolveSibling(other.toString)
override def resolveSibling(other: String): Path =
def resolveSibling(other: String): Path =
JSIO.path match {
case Some(path) =>
adjustResolvedPath(
NodeNIOPath(
Path(
path
.resolve(path.dirname(filename).asInstanceOf[String], other)
.asInstanceOf[String]
Expand All @@ -121,36 +114,36 @@ case class NodeNIOPath(filename: String) extends Path {
"Path.normalize() is only supported in Node.js"
)
}
override def resolve(other: Path): Path =
def resolve(other: Path): Path =
resolve(other.toString)
override def resolve(other: String): Path =
def resolve(other: String): Path =
JSIO.path match {
case Some(path) =>
adjustResolvedPath(
NodeNIOPath(path.resolve(filename, other).asInstanceOf[String])
Path(path.resolve(filename, other).asInstanceOf[String])
)
case None =>
throw new UnsupportedOperationException(
"Path.normalize() is only supported in Node.js"
)
}
override def startsWith(other: Path): Boolean =
def startsWith(other: Path): Boolean =
startsWith(other.toString)
override def startsWith(other: String): Boolean =
def startsWith(other: String): Boolean =
paths(filename).startsWith(paths(other))
private def paths(name: String) =
name.split(escapedSeparator)
override def toString: String =
filename
override def iterator(): util.Iterator[Path] =
def iterator(): util.Iterator[Path] =
filename
.split(File.separator)
.iterator
.map(name => NodeNIOPath(name): Path)
.map(name => Path(name): Path)
.asJava
}

object NodeNIOPath {
def workingDirectory: NodeNIOPath =
NodeNIOPath(PlatformPathIO.workingDirectoryString)
object Path {
def workingDirectory: Path =
Path(PlatformPathIO.workingDirectoryString)
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
package java.nio.file
package munit.internal
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's introduce a new munit.internal.io package. The general structure I try to following is

  • flat list of classes/objects inside munit._ for public API
  • flat list of packages inside munit.internal._

The reason is mostly historical because MUnit includes a reimplementation of the java difflib and it didn't feel right to have all of those diff classes inside munit.internal._.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


import java.io.File
import java.net.URI

import munit.internal.NodeNIOPath

object Paths {
// NOTE: We can't use Scala-style varargs since those have a different jvm
// signature than Java-style varargs. The boot classpath contains nio.file.Path
Expand All @@ -14,7 +11,7 @@ object Paths {
val path =
if (more.isEmpty) first
else first + File.separator + more.mkString(File.separator)
NodeNIOPath(path)
Path(path)
}

def get(uri: URI): Path = {
Expand All @@ -25,8 +22,8 @@ object Paths {
val (leading, trailing) = parts.span(_ == "")
trailing match {
case drive :: path if (drive.length == 2 && drive(1) == ':') =>
NodeNIOPath(trailing.mkString("\\"))
case _ => NodeNIOPath(uripath)
Path(trailing.mkString("\\"))
case _ => Path(uripath)
}
}
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
package java.lang.reflect
package munit.internal

class UndeclaredThrowableException extends java.lang.RuntimeException
13 changes: 13 additions & 0 deletions munit/jvm/src/main/scala/munit/internal/package.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package munit

package object internal {
type File = java.io.File
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add an object called PlatformIO that has these methods and aliases instead? I try to avoid package objects when possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved the io-related ones to PlatformIO. I moved the reflect ones to PlatformCompat.

object Files {
def readAllLines(path: Path) = java.nio.file.Files.readAllLines(path)
}

type Path = java.nio.file.Path
object Paths {
def get(path: String) = java.nio.file.Paths.get(path)
}
}
13 changes: 13 additions & 0 deletions munit/native/src/main/scala/munit/internal/package.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package munit

package object internal {
type File = java.io.File
object Files {
def readAllLines(path: Path) = java.nio.file.Files.readAllLines(path)
}

type Path = java.nio.file.Path
object Paths {
def get(path: String) = java.nio.file.Paths.get(path)
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package munit.internal.console

import java.nio.file.{Files, Path, Paths}

import munit.internal.{Files, Path, Paths}
import munit.Location

import scala.collection.JavaConverters._
Expand Down