Skip to content

Commit

Permalink
fix built-in rules: don't wrap directive values in double quotes if…
Browse files Browse the repository at this point in the history 
… not necessary (#3414)
  • Loading branch information
@Gedochao
Gedochao authored Jan 13, 2025
1 parent 1e7ef4e commit da025a0
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 37 deletions.
5 changes: 4 additions & 1 deletion modules/directives/src/main/scala/scala/build/preprocessing/directives/StrictDirective.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
78 changes: 50 additions & 28 deletions ...les/integration/src/test/scala/scala/cli/integration/FixBuiltInRulesTestDefinitions.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package scala.cli.integration

import com.eed3si9n.expecty.Expecty.expect

import scala.util.Properties

trait FixBuiltInRulesTestDefinitions { _: FixTestDefinitions =>
test("basic built-in rules") {
val mainFileName = "Main.scala"
Expand All @@ -17,7 +19,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
)

Expand Down Expand Up @@ -52,11 +54,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
)

Expand Down Expand Up @@ -89,7 +87,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
)

Expand Down Expand Up @@ -125,11 +123,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
)

Expand Down Expand Up @@ -217,11 +211,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
)

Expand Down Expand Up @@ -366,23 +360,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
)

Expand Down Expand Up @@ -425,4 +419,32 @@ trait FixBuiltInRulesTestDefinitions { _: FixTestDefinitions =>
)
}
}

if (!Properties.isWin) // TODO: fix this test for Windows CI
test("using directives with boolean values are handled correctly") {
val expectedMessage = "Hello, world!"
def maybeScalapyPrefix =
if (actualScalaVersion.startsWith("2.13.")) ""
else "import me.shadaj.scalapy.py" + System.lineSeparator()
TestInputs(
os.rel / "Messages.scala" ->
s"""object Messages {
| def hello: String = "$expectedMessage"
|}
|""".stripMargin,
os.rel / "Main.scala" ->
s"""//> using python true
|$maybeScalapyPrefix
|object Main extends App {
| py.Dynamic.global.print(Messages.hello, flush = true)
|}
|""".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)
}
}
}
6 changes: 3 additions & 3 deletions ...es/integration/src/test/scala/scala/cli/integration/FixScalafixRulesTestDefinitions.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ trait FixScalafixRulesTestDefinitions {
}
val expectedProjectContent: String = noCrLf {
s"""// Main
|//> using options "$scalafixUnusedRuleOption"
|//> using options $scalafixUnusedRuleOption
|
|""".stripMargin
}
Expand Down Expand Up @@ -150,7 +150,7 @@ trait FixScalafixRulesTestDefinitions {
}
val expectedProjectContent: String = noCrLf {
s"""// Main
|//> using options "$scalafixUnusedRuleOption"
|//> using options $scalafixUnusedRuleOption
|
|""".stripMargin
}
Expand Down Expand Up @@ -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
|
Expand Down
10 changes: 5 additions & 5 deletions modules/integration/src/test/scala/scala/cli/integration/FixTestDefinitions.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit da025a0

Please sign in to comment.