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.

Sign up for GitHub

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

fix built-in rules: don't wrap directive values in double quotes if not necessary #3414

Merged
merged 1 commit into from
Jan 13, 2025
Merged

fix built-in rules: don't wrap directive values in double quotes if not necessary #3414

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 4 additions & 1 deletion modules/directives/src/main/scala/scala/build/preprocessing/directives/StrictDirective.scala
View file
Open in desktop
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
Open in desktop
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
Gedochao marked this conversation as resolved.
Show resolved Hide resolved
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
Open in desktop
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
Open in desktop
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