From c3bab7ca4e7de830b578cdaab152c436009ca796 Mon Sep 17 00:00:00 2001 From: Piotr Chabelski Date: Fri, 10 Jan 2025 16:41:10 +0100 Subject: [PATCH] `fix` built-in rules: don't wrap directive values in double quotes if not necessary --- .../directives/StrictDirective.scala | 5 +- .../FixBuiltInRulesTestDefinitions.scala | 72 +++++++++++-------- .../FixScalafixRulesTestDefinitions.scala | 6 +- .../cli/integration/FixTestDefinitions.scala | 10 +-- 4 files changed, 56 insertions(+), 37 deletions(-) diff --git a/modules/directives/src/main/scala/scala/build/preprocessing/directives/StrictDirective.scala b/modules/directives/src/main/scala/scala/build/preprocessing/directives/StrictDirective.scala index 82cc135760..37f154cf1e 100644 --- a/modules/directives/src/main/scala/scala/build/preprocessing/directives/StrictDirective.scala +++ b/modules/directives/src/main/scala/scala/build/preprocessing/directives/StrictDirective.scala @@ -46,7 +46,10 @@ case class StrictDirective( Seq(usingKeyString) else { val distinctValuesStrings = validValues - .map(v => s"\"${v.toString}\"") + .map { + case s if s.toString.exists(_.isWhitespace) => s"\"$s\"" + case s => s.toString + } .distinct .sorted diff --git a/modules/integration/src/test/scala/scala/cli/integration/FixBuiltInRulesTestDefinitions.scala b/modules/integration/src/test/scala/scala/cli/integration/FixBuiltInRulesTestDefinitions.scala index b0921d5884..423e2ab7b9 100644 --- a/modules/integration/src/test/scala/scala/cli/integration/FixBuiltInRulesTestDefinitions.scala +++ b/modules/integration/src/test/scala/scala/cli/integration/FixBuiltInRulesTestDefinitions.scala @@ -17,7 +17,7 @@ trait FixBuiltInRulesTestDefinitions { _: FixTestDefinitions => |} |""".stripMargin, os.rel / projectFileName -> - s"""//> using deps "com.lihaoyi::pprint:0.6.6" + s"""//> using deps com.lihaoyi::pprint:0.6.6 |""".stripMargin ) @@ -52,11 +52,7 @@ trait FixBuiltInRulesTestDefinitions { _: FixTestDefinitions => projectFileContents, """// Main |//> using objectWrapper - | - |//> using dependency "com.lihaoyi::os-lib:0.9.1" - |//> using dependency "com.lihaoyi::pprint:0.6.6" - |//> using dependency "com.lihaoyi::upickle:3.1.2" - | + |//> using dependency com.lihaoyi::os-lib:0.9.1 com.lihaoyi::pprint:0.6.6 com.lihaoyi::upickle:3.1.2 |""".stripMargin ) @@ -89,7 +85,7 @@ trait FixBuiltInRulesTestDefinitions { _: FixTestDefinitions => |println(os.pwd) |""".stripMargin, os.rel / projectFileName -> - s"""//> using deps "com.lihaoyi::pprint:0.6.6" + s"""//> using deps com.lihaoyi::pprint:0.6.6 |""".stripMargin ) @@ -125,11 +121,7 @@ trait FixBuiltInRulesTestDefinitions { _: FixTestDefinitions => projectFileContents, """// Main |//> using objectWrapper - | - |//> using dependency "com.lihaoyi::os-lib:0.9.1" - |//> using dependency "com.lihaoyi::pprint:0.6.6" - |//> using dependency "com.lihaoyi::upickle:3.1.2" - | + |//> using dependency com.lihaoyi::os-lib:0.9.1 com.lihaoyi::pprint:0.6.6 com.lihaoyi::upickle:3.1.2 |""".stripMargin ) @@ -217,11 +209,11 @@ trait FixBuiltInRulesTestDefinitions { _: FixTestDefinitions => projectFileContents, """// Main |//> using objectWrapper - |//> using dependency "com.lihaoyi::os-lib:0.9.1" "com.lihaoyi::pprint:0.6.6" + |//> using dependency com.lihaoyi::os-lib:0.9.1 com.lihaoyi::pprint:0.6.6 | |// Test - |//> using test.options "-Xasync" "-Xfatal-warnings" - |//> using test.dependency "org.scalameta::munit::0.7.29" "org.typelevel::cats-core:2.9.0" + |//> using test.options -Xasync -Xfatal-warnings + |//> using test.dependency org.scalameta::munit::0.7.29 org.typelevel::cats-core:2.9.0 |""".stripMargin ) @@ -366,23 +358,23 @@ trait FixBuiltInRulesTestDefinitions { _: FixTestDefinitions => assertNoDiff( projectFileContents, s"""// Main - |//> using scala "3.3.0" - |//> using platforms "jvm" - |//> using jvm "17" - |//> using options "-Werror" - |//> using files "$includePath" + |//> using scala 3.3.0 + |//> using platforms jvm + |//> using jvm 17 + |//> using options -Werror + |//> using files $includePath |//> using objectWrapper - |//> using toolkit "default" - |//> using dependency "com.lihaoyi::os-lib:0.9.1" "com.lihaoyi::pprint:0.6.6" + |//> using toolkit default + |//> using dependency com.lihaoyi::os-lib:0.9.1 com.lihaoyi::pprint:0.6.6 | - |//> using publish.ci.password "env:PUBLISH_PASSWORD" - |//> using publish.ci.secretKey "env:PUBLISH_SECRET_KEY" - |//> using publish.ci.secretKeyPassword "env:PUBLISH_SECRET_KEY_PASSWORD" - |//> using publish.ci.user "env:PUBLISH_USER" + |//> using publish.ci.password env:PUBLISH_PASSWORD + |//> using publish.ci.secretKey env:PUBLISH_SECRET_KEY + |//> using publish.ci.secretKeyPassword env:PUBLISH_SECRET_KEY_PASSWORD + |//> using publish.ci.user env:PUBLISH_USER | |// Test - |//> using test.options "-Xasync" "-Xfatal-warnings" - |//> using test.dependency "org.scalameta::munit::0.7.29" "org.typelevel::cats-core:2.9.0" + |//> using test.options -Xasync -Xfatal-warnings + |//> using test.dependency org.scalameta::munit::0.7.29 org.typelevel::cats-core:2.9.0 |""".stripMargin ) @@ -425,4 +417,28 @@ trait FixBuiltInRulesTestDefinitions { _: FixTestDefinitions => ) } } + + test("using directives with boolean values are handled correctly") { + val expectedMessage = "Hello, world!" + TestInputs( + os.rel / "Messages.scala" -> + s"""object Messages { + | def hello: String = "$expectedMessage" + |} + |""".stripMargin, + os.rel / "Main.scala" -> + """//> using nativeMultithreading true + |//> using platform native + |object Main extends App { + | println(Messages.hello) + |} + |""".stripMargin + ).fromRoot { root => + os.proc(TestUtil.cli, "--power", "fix", ".", extraOptions) + .call(cwd = root, stderr = os.Pipe) + val r = os.proc(TestUtil.cli, "--power", "run", ".", extraOptions) + .call(cwd = root, stderr = os.Pipe) + expect(r.out.trim() == expectedMessage) + } + } } diff --git a/modules/integration/src/test/scala/scala/cli/integration/FixScalafixRulesTestDefinitions.scala b/modules/integration/src/test/scala/scala/cli/integration/FixScalafixRulesTestDefinitions.scala index 2e6c91d78a..1055641db5 100644 --- a/modules/integration/src/test/scala/scala/cli/integration/FixScalafixRulesTestDefinitions.scala +++ b/modules/integration/src/test/scala/scala/cli/integration/FixScalafixRulesTestDefinitions.scala @@ -90,7 +90,7 @@ trait FixScalafixRulesTestDefinitions { } val expectedProjectContent: String = noCrLf { s"""// Main - |//> using options "$scalafixUnusedRuleOption" + |//> using options $scalafixUnusedRuleOption | |""".stripMargin } @@ -150,7 +150,7 @@ trait FixScalafixRulesTestDefinitions { } val expectedProjectContent: String = noCrLf { s"""// Main - |//> using options "$scalafixUnusedRuleOption" + |//> using options $scalafixUnusedRuleOption | |""".stripMargin } @@ -242,7 +242,7 @@ trait FixScalafixRulesTestDefinitions { } test("external rule") { - val directive = s"//> using scalafixDependency \"com.github.xuwei-k::scalafix-rules:0.5.1\"" + val directive = s"//> using scalafixDependency com.github.xuwei-k::scalafix-rules:0.5.1" val original: String = s"""|$directive | diff --git a/modules/integration/src/test/scala/scala/cli/integration/FixTestDefinitions.scala b/modules/integration/src/test/scala/scala/cli/integration/FixTestDefinitions.scala index 020f7629ec..2f429552f0 100644 --- a/modules/integration/src/test/scala/scala/cli/integration/FixTestDefinitions.scala +++ b/modules/integration/src/test/scala/scala/cli/integration/FixTestDefinitions.scala @@ -22,13 +22,13 @@ abstract class FixTestDefinitions test("built-in + scalafix rules") { val mainFileName = "Main.scala" val unusedValName = "unused" - val directive1 = "//> using dep \"com.lihaoyi::os-lib:0.11.3\"" - val directive2 = "//> using dep \"com.lihaoyi::pprint:0.9.0\"" + val directive1 = "//> using dep com.lihaoyi::os-lib:0.11.3" + val directive2 = "//> using dep com.lihaoyi::pprint:0.9.0" val mergedDirective1And2 = - "using dependency \"com.lihaoyi::os-lib:0.11.3\" \"com.lihaoyi::pprint:0.9.0\"" + "using dependency com.lihaoyi::os-lib:0.11.3 com.lihaoyi::pprint:0.9.0" val directive3 = - if (actualScalaVersion.startsWith("2")) "//> using options \"-Xlint:unused\"" - else "//> using options \"-Wunused:all\"" + if (actualScalaVersion.startsWith("2")) "//> using options -Xlint:unused" + else "//> using options -Wunused:all" TestInputs( os.rel / "Foo.scala" -> s"""$directive1