diff --git a/mdoc-js/src/main/scala/mdoc/modifiers/JsModifier.scala b/mdoc-js/src/main/scala/mdoc/modifiers/JsModifier.scala index 63ecdefc2..e9bfd012e 100644 --- a/mdoc-js/src/main/scala/mdoc/modifiers/JsModifier.scala +++ b/mdoc-js/src/main/scala/mdoc/modifiers/JsModifier.scala @@ -2,6 +2,7 @@ package mdoc.modifiers import mdoc.{OnLoadContext, PostProcessContext, PreModifierContext} import mdoc.internal.cli.InputFile +import mdoc.internal.cli.ScalacOptions import mdoc.internal.io.ConsoleReporter import mdoc.internal.livereload.Resources import mdoc.internal.markdown.{CodeBuilder, Gensym, MarkdownCompiler} @@ -91,6 +92,7 @@ class JsModifier extends mdoc.PreModifier { reporter = ctx.reporter val compileClasspath = Classpath(classpath) val linkerClasspath = Classpath(config.classpath) + val parsedScalacOptions = ScalacOptions.parse(scalacOptions) val newClasspathHash = (classpath, linkerClasspath, scalacOptions, config.fullOpt).hashCode() @@ -98,7 +100,7 @@ class JsModifier extends mdoc.PreModifier { // to speed up unit tests by nearly 2x. if (classpathHash != newClasspathHash) { classpathHash = newClasspathHash - maybeCompiler = Some(new MarkdownCompiler(classpath, scalacOptions, target)) + maybeCompiler = Some(new MarkdownCompiler(classpath, parsedScalacOptions, target)) val loader = ScalaJSClassloader.create(linkerClasspath.entries.map(_.toURI.toURL()).toArray) diff --git a/mdoc-sbt/src/main/scala/mdoc/MdocPlugin.scala b/mdoc-sbt/src/main/scala/mdoc/MdocPlugin.scala index 5e1cad001..fc390be1c 100644 --- a/mdoc-sbt/src/main/scala/mdoc/MdocPlugin.scala +++ b/mdoc-sbt/src/main/scala/mdoc/MdocPlugin.scala @@ -198,7 +198,7 @@ object MdocPlugin extends AutoPlugin with MdocPluginCompat { if (withoutPrefix eq o) o else pluginPrefix + converter.toPath(VirtualFileRef.of(withoutPrefix)) } - .mkString(" ") + .mkString("\n") ) val classpath = ListBuffer.empty[File] // Can't use fullClasspath.value because it introduces cyclic dependency between @@ -233,7 +233,7 @@ object MdocPlugin extends AutoPlugin with MdocPluginCompat { props.put( s"js-scalac-options", - scalacOptions.mkString(" ") + scalacOptions.mkString("\n") ) props.put( s"js-classpath", diff --git a/mdoc-sbt/src/sbt-test/sbt-mdoc/scalac-options-spaces/build.sbt b/mdoc-sbt/src/sbt-test/sbt-mdoc/scalac-options-spaces/build.sbt new file mode 100644 index 000000000..3336b7953 --- /dev/null +++ b/mdoc-sbt/src/sbt-test/sbt-mdoc/scalac-options-spaces/build.sbt @@ -0,0 +1,29 @@ +ThisBuild / scalaVersion := "3.3.7" + +enablePlugins(MdocPlugin) + +// -Wconf option containing a space in the msg regex pattern. +// Previously this caused mdoc to break the option when re-splitting +// the scalacOptions string by whitespace. +scalacOptions += "-Wconf:msg=unused import:s" + +TaskKey[Unit]("check") := { + val file = mdocOut.value / "readme.md" + val obtained = IO.read(file) + IO.delete(file) + println(s"----${scalaVersion.value}----") + println(obtained) + println() + assert( + obtained.trim == + """ +# Test +```scala +val x = 1 +// x: Int = 1 +``` +""".trim, + "\"\"\"\n" + obtained + "\n\"\"\"" + ) + println("------------------") +} diff --git a/mdoc-sbt/src/sbt-test/sbt-mdoc/scalac-options-spaces/docs/readme.md b/mdoc-sbt/src/sbt-test/sbt-mdoc/scalac-options-spaces/docs/readme.md new file mode 100644 index 000000000..20b7bb005 --- /dev/null +++ b/mdoc-sbt/src/sbt-test/sbt-mdoc/scalac-options-spaces/docs/readme.md @@ -0,0 +1,4 @@ +# Test +```scala mdoc +val x = 1 +``` diff --git a/mdoc-sbt/src/sbt-test/sbt-mdoc/scalac-options-spaces/project/plugins.sbt b/mdoc-sbt/src/sbt-test/sbt-mdoc/scalac-options-spaces/project/plugins.sbt new file mode 100644 index 000000000..36ec193de --- /dev/null +++ b/mdoc-sbt/src/sbt-test/sbt-mdoc/scalac-options-spaces/project/plugins.sbt @@ -0,0 +1 @@ +addSbtPlugin("org.scalameta" % "sbt-mdoc" % sys.props("plugin.version")) diff --git a/mdoc-sbt/src/sbt-test/sbt-mdoc/scalac-options-spaces/test b/mdoc-sbt/src/sbt-test/sbt-mdoc/scalac-options-spaces/test new file mode 100644 index 000000000..ce86cda1c --- /dev/null +++ b/mdoc-sbt/src/sbt-test/sbt-mdoc/scalac-options-spaces/test @@ -0,0 +1,3 @@ +# Test that scalacOptions containing spaces (e.g. -Wconf with regex patterns) work correctly +> mdoc +> check diff --git a/mdoc/src/main/scala-2/mdoc/internal/markdown/MarkdownCompiler.scala b/mdoc/src/main/scala-2/mdoc/internal/markdown/MarkdownCompiler.scala index 7b4faa284..bcc9e5646 100644 --- a/mdoc/src/main/scala-2/mdoc/internal/markdown/MarkdownCompiler.scala +++ b/mdoc/src/main/scala-2/mdoc/internal/markdown/MarkdownCompiler.scala @@ -31,7 +31,7 @@ import mdoc.internal.worksheets.Compat._ class MarkdownCompiler( classpath: String, - val scalacOptions: String, + val scalacOptions: List[String], target: AbstractFile = new VirtualDirectory("(memory)", None) ) { private val settings = new Settings() @@ -45,7 +45,7 @@ class MarkdownCompiler( // https://github.com/scala/bug/issues/9824 // https://github.com/scalameta/mdoc/issues/124 settings.Ydelambdafy.value = "inline" - settings.processArgumentString(scalacOptions) + settings.processArguments(scalacOptions.filter(_.nonEmpty), processAll = true) def classpathEntries: Seq[Path] = global.classPath.asURLs.map(url => Paths.get(url.toURI())) diff --git a/mdoc/src/main/scala-3/mdoc/internal/markdown/MarkdownCompiler.scala b/mdoc/src/main/scala-3/mdoc/internal/markdown/MarkdownCompiler.scala index 0e93cfc08..45f1f7474 100644 --- a/mdoc/src/main/scala-3/mdoc/internal/markdown/MarkdownCompiler.scala +++ b/mdoc/src/main/scala-3/mdoc/internal/markdown/MarkdownCompiler.scala @@ -63,7 +63,7 @@ class MarkdownDriver(val settings: List[String]) extends Driver { class MarkdownCompiler( classpath: String, - val scalacOptions: String, + val scalacOptions: List[String], target: AbstractFile = new VirtualDirectory("(memory)") ) { @@ -82,7 +82,7 @@ class MarkdownCompiler( case head :: tail => head :: removeDuplicatedOptions(tail) case Nil => Nil - val options = removeDuplicatedOptions(scalacOptions.split("\\s+").filter(_.nonEmpty).toList) + val options = removeDuplicatedOptions(scalacOptions.filter(_.nonEmpty)) val settings = options ::: defaultFlags ::: "-classpath" :: classpath :: Nil diff --git a/mdoc/src/main/scala/mdoc/internal/cli/Context.scala b/mdoc/src/main/scala/mdoc/internal/cli/Context.scala index fc21806db..be00d0d37 100644 --- a/mdoc/src/main/scala/mdoc/internal/cli/Context.scala +++ b/mdoc/src/main/scala/mdoc/internal/cli/Context.scala @@ -87,7 +87,8 @@ object Context { new Context(options, reporter, compiler) } def fromOptions(options: Settings, reporter: Reporter = ConsoleReporter.default): Context = { - val compiler = MarkdownBuilder.fromClasspath(options.classpath, options.scalacOptions) + val compiler = + MarkdownBuilder.fromClasspath(options.classpath, ScalacOptions.parse(options.scalacOptions)) fromCompiler(options, reporter, compiler) } } diff --git a/mdoc/src/main/scala/mdoc/internal/cli/Dependencies.scala b/mdoc/src/main/scala/mdoc/internal/cli/Dependencies.scala index 3bd44b317..fbd80e75d 100644 --- a/mdoc/src/main/scala/mdoc/internal/cli/Dependencies.scala +++ b/mdoc/src/main/scala/mdoc/internal/cli/Dependencies.scala @@ -4,6 +4,7 @@ import scala.meta.io.Classpath import coursierapi.Dependency import mdoc.internal.markdown.MarkdownCompiler import mdoc.internal.markdown.MarkdownBuilder +import mdoc.internal.cli.ScalacOptions import scala.meta.io.AbsolutePath import coursierapi.Repository import mdoc.internal.markdown.Instrumented @@ -30,9 +31,9 @@ object Dependencies { Classpath(Classpath(settings.classpath).entries ++ jars.map(AbsolutePath(_))) val scalacOptions = instrumented.scalacOptionImports match { case Nil => - settings.scalacOptions + ScalacOptions.parse(settings.scalacOptions) case options => - s"${settings.scalacOptions} ${options.map(_.value).mkString(" ")}" + ScalacOptions.parse(settings.scalacOptions) ++ options.map(_.value) } MarkdownBuilder.fromClasspath(classpath.syntax, scalacOptions) } diff --git a/mdoc/src/main/scala/mdoc/internal/cli/ScalacOptions.scala b/mdoc/src/main/scala/mdoc/internal/cli/ScalacOptions.scala new file mode 100644 index 000000000..69eaa23f5 --- /dev/null +++ b/mdoc/src/main/scala/mdoc/internal/cli/ScalacOptions.scala @@ -0,0 +1,16 @@ +package mdoc.internal.cli + +object ScalacOptions { + + private val OptionSeparator = "\n" + + def parse(s: String): List[String] = + if (s.isEmpty) Nil + else if (s.contains(OptionSeparator)) + s.split(OptionSeparator).filter(_.nonEmpty).toList + else + s.split("\\s+").filter(_.nonEmpty).toList + + def serialize(options: Seq[String]): String = + options.mkString(OptionSeparator) +} diff --git a/mdoc/src/main/scala/mdoc/internal/markdown/MarkdownBuilder.scala b/mdoc/src/main/scala/mdoc/internal/markdown/MarkdownBuilder.scala index 0afe3aaba..e09792c8f 100644 --- a/mdoc/src/main/scala/mdoc/internal/markdown/MarkdownBuilder.scala +++ b/mdoc/src/main/scala/mdoc/internal/markdown/MarkdownBuilder.scala @@ -17,7 +17,7 @@ import java.util.concurrent.atomic.AtomicReference object MarkdownBuilder { - def default(): MarkdownCompiler = fromClasspath(classpath = "", scalacOptions = "") + def default(): MarkdownCompiler = fromClasspath(classpath = "", scalacOptions = Nil) def buildDocument( compiler: MarkdownCompiler, @@ -78,7 +78,7 @@ object MarkdownBuilder { EvaluatedDocument(doc, sectionInputs) } - def fromClasspath(classpath: String, scalacOptions: String): MarkdownCompiler = { + def fromClasspath(classpath: String, scalacOptions: List[String]): MarkdownCompiler = { val fullClasspath = if (classpath.isEmpty) defaultClasspath(_ => true) else { diff --git a/tests/jsdocs/package-lock.json b/tests/jsdocs/package-lock.json index 90633060e..f80e01c6b 100644 --- a/tests/jsdocs/package-lock.json +++ b/tests/jsdocs/package-lock.json @@ -371,6 +371,7 @@ "resolved": "https://registry.npmjs.org/ajv/-/ajv-6.12.6.tgz", "integrity": "sha512-j3fVLgvTo527anyYyJOGTYJbG+vnnQYvE0m5mmkc1TK+nxAppkCLMIL0aZ4dblVCNoGShhm+kzE4ZUykBoMg4g==", "dev": true, + "peer": true, "dependencies": { "fast-deep-equal": "^3.1.1", "fast-json-stable-stringify": "^2.0.0", @@ -775,6 +776,7 @@ "url": "https://github.com/sponsors/ai" } ], + "peer": true, "dependencies": { "caniuse-lite": "^1.0.30001646", "electron-to-chromium": "^1.5.4", @@ -5273,6 +5275,7 @@ "resolved": "https://registry.npmjs.org/webpack/-/webpack-5.24.3.tgz", "integrity": "sha512-x7lrWZ7wlWAdyKdML6YPvfVZkhD1ICuIZGODE5SzKJjqI9A4SpqGTjGJTc6CwaHqn19gGaoOR3ONJ46nYsn9rw==", "dev": true, + "peer": true, "dependencies": { "@types/eslint-scope": "^3.7.0", "@types/estree": "^0.0.46", @@ -5319,6 +5322,7 @@ "resolved": "https://registry.npmjs.org/webpack-cli/-/webpack-cli-4.5.0.tgz", "integrity": "sha512-wXg/ef6Ibstl2f50mnkcHblRPN/P9J4Nlod5Hg9HGFgSeF8rsqDGHJeVe4aR26q9l62TUJi6vmvC2Qz96YJw1Q==", "dev": true, + "peer": true, "dependencies": { "@discoveryjs/json-ext": "^0.5.0", "@webpack-cli/configtest": "^1.0.1", @@ -6169,6 +6173,7 @@ "resolved": "https://registry.npmjs.org/ajv/-/ajv-6.12.6.tgz", "integrity": "sha512-j3fVLgvTo527anyYyJOGTYJbG+vnnQYvE0m5mmkc1TK+nxAppkCLMIL0aZ4dblVCNoGShhm+kzE4ZUykBoMg4g==", "dev": true, + "peer": true, "requires": { "fast-deep-equal": "^3.1.1", "fast-json-stable-stringify": "^2.0.0", @@ -6478,6 +6483,7 @@ "resolved": "https://registry.npmjs.org/browserslist/-/browserslist-4.23.3.tgz", "integrity": "sha512-btwCFJVjI4YWDNfau8RhZ+B1Q/VLoUITrm3RlP6y1tYGWIOa+InuYiRGXUBXo8nA1qKmHMyLB/iVQg5TT4eFoA==", "dev": true, + "peer": true, "requires": { "caniuse-lite": "^1.0.30001646", "electron-to-chromium": "^1.5.4", @@ -9982,6 +9988,7 @@ "resolved": "https://registry.npmjs.org/webpack/-/webpack-5.24.3.tgz", "integrity": "sha512-x7lrWZ7wlWAdyKdML6YPvfVZkhD1ICuIZGODE5SzKJjqI9A4SpqGTjGJTc6CwaHqn19gGaoOR3ONJ46nYsn9rw==", "dev": true, + "peer": true, "requires": { "@types/eslint-scope": "^3.7.0", "@types/estree": "^0.0.46", @@ -10013,6 +10020,7 @@ "resolved": "https://registry.npmjs.org/webpack-cli/-/webpack-cli-4.5.0.tgz", "integrity": "sha512-wXg/ef6Ibstl2f50mnkcHblRPN/P9J4Nlod5Hg9HGFgSeF8rsqDGHJeVe4aR26q9l62TUJi6vmvC2Qz96YJw1Q==", "dev": true, + "peer": true, "requires": { "@discoveryjs/json-ext": "^0.5.0", "@webpack-cli/configtest": "^1.0.1", diff --git a/tests/unit/src/main/scala/tests/markdown/BaseMarkdownSuite.scala b/tests/unit/src/main/scala/tests/markdown/BaseMarkdownSuite.scala index 209d763e5..a2db9439f 100644 --- a/tests/unit/src/main/scala/tests/markdown/BaseMarkdownSuite.scala +++ b/tests/unit/src/main/scala/tests/markdown/BaseMarkdownSuite.scala @@ -48,7 +48,7 @@ abstract class BaseMarkdownSuite extends tests.BaseSuite { myStdout.reset() new ConsoleReporter(new PrintStream(myStdout)) } - protected def scalacOptions: String = "" + protected def scalacOptions: List[String] = Nil private val compiler = MarkdownBuilder.fromClasspath("", scalacOptions) private def newContext(settings: Settings, reporter: ConsoleReporter) = { Context.fromSettings(settings, reporter) diff --git a/tests/unit/src/test/scala/tests/imports/ScalacSuite.scala b/tests/unit/src/test/scala/tests/imports/ScalacSuite.scala index 0906f43bb..ffb4f9edd 100644 --- a/tests/unit/src/test/scala/tests/imports/ScalacSuite.scala +++ b/tests/unit/src/test/scala/tests/imports/ScalacSuite.scala @@ -7,7 +7,9 @@ class ScalacSuite extends BaseMarkdownSuite { for ( (name, importStyle) <- Seq( - "ammonite" -> "import $scalac.`-Wunused:imports -Xfatal-warnings`", + "ammonite" -> + """|import $scalac.`-Wunused:imports` + |import $scalac.`-Xfatal-warnings`""".stripMargin, "using" -> "//> using option -Wunused:imports -Xfatal-warnings" ) ) @@ -20,19 +22,20 @@ class ScalacSuite extends BaseMarkdownSuite { |println(42) |``` |""".stripMargin, - s"""|error: No warnings can be incurred under -Werror. - |warning: import-$name.md:4:1: Unused import - |import scala.util.Try - |^^^^^^^^^^^^^^^^^^^^^ - |""".stripMargin, - compat = Map( - Compat.Scala213 -> + name match { + case "ammonite" => + s"""|error: No warnings can be incurred under -Werror. + |warning: import-$name.md:5:19: Unused import + |import scala.util.Try + | ^^^ + |""".stripMargin + case "using" => s"""|error: No warnings can be incurred under -Werror. |warning: import-$name.md:4:19: Unused import |import scala.util.Try | ^^^ |""".stripMargin - ) + } ) check( @@ -53,7 +56,9 @@ class ScalacSuite extends BaseMarkdownSuite { for ( (name, importStyle) <- Seq( - "ammonite" -> "import $scalac.`-Wunused:imports -Xfatal-warnings`", + "ammonite" -> + """|import $scalac.`-Wunused:imports` + |import $scalac.`-Xfatal-warnings`""".stripMargin, "using" -> "//> using option -Wunused:imports -Xfatal-warnings" ) )