diff --git a/modules/cli/src/main/scala/scala/cli/commands/fix/Fix.scala b/modules/cli/src/main/scala/scala/cli/commands/fix/Fix.scala index 7ce7639101..935bd1d6c4 100644 --- a/modules/cli/src/main/scala/scala/cli/commands/fix/Fix.scala +++ b/modules/cli/src/main/scala/scala/cli/commands/fix/Fix.scala @@ -10,6 +10,7 @@ import scala.build.preprocessing.{ExtractedDirectives, SheBang} import scala.build.{CrossSources, Logger, Position, Sources} import scala.cli.commands.shared.SharedOptions import scala.cli.commands.{ScalaCommand, SpecificationLevel} +import scala.collection.immutable.HashMap import scala.util.chaining.scalaUtilChainingOps object Fix extends ScalaCommand[FixOptions] { @@ -28,30 +29,9 @@ object Fix extends ScalaCommand[FixOptions] { val newLine = System.lineSeparator() override def runCommand(options: FixOptions, args: RemainingArgs, logger: Logger): Unit = { - val buildOptions = BuildOptions() - val inputs = options.shared.inputs(args.remaining, () => Inputs.default()).orExit(logger) + val inputs = options.shared.inputs(args.remaining, () => Inputs.default()).orExit(logger) - val (crossSources, _) = CrossSources.forInputs( - inputs, - preprocessors = Sources.defaultPreprocessors( - buildOptions.archiveCache, - buildOptions.internal.javaClassNameVersionOpt, - () => buildOptions.javaHome().value.javaCommand - ), - logger = logger, - suppressWarningOptions = SuppressWarningOptions( - Some(true), - Some(true), - Some(true) - ), - exclude = buildOptions.internal.exclude - ).orExit(logger) - - val sharedOptions = crossSources.sharedOptions(buildOptions) - val scopedSources = crossSources.scopedSources(sharedOptions).orExit(logger) - - val mainSources = scopedSources.sources(Scope.Main, sharedOptions) - val testSources = scopedSources.sources(Scope.Test, sharedOptions) + val (mainSources, testSources) = getProjectSources(inputs) // Only initial inputs are used, new inputs discovered during processing of // CrossSources.forInput may be shared between projects @@ -92,11 +72,7 @@ object Fix extends ScalaCommand[FixOptions] { .append("// Main") .append(newLine) - allDirectives - .flatMap(_.explodeToStrings) - .distinct - .sorted - .foreach(projectFileContents.append(_).append(newLine)) + createFormattedLinesAndAppend(allDirectives, projectFileContents, isTest = false) } // Deal with directives from the Test scope @@ -124,16 +100,41 @@ object Fix extends ScalaCommand[FixOptions] { directive <- directivesWithTestPrefix } yield directive - allDirectives - .flatMap(_.explodeToStrings) - .distinct - .sorted - .foreach(projectFileContents.append(_).append(newLine)) + createFormattedLinesAndAppend(allDirectives, projectFileContents, isTest = true) } + projectFileContents.append(newLine) os.write.over(inputs.workspace / Constants.projectFileName, projectFileContents.toString) } + def getProjectSources(inputs: Inputs): (Sources, Sources) = { + val buildOptions = BuildOptions() + + val (crossSources, _) = CrossSources.forInputs( + inputs, + preprocessors = Sources.defaultPreprocessors( + buildOptions.archiveCache, + buildOptions.internal.javaClassNameVersionOpt, + () => buildOptions.javaHome().value.javaCommand + ), + logger = logger, + suppressWarningOptions = SuppressWarningOptions( + Some(true), + Some(true), + Some(true) + ), + exclude = buildOptions.internal.exclude + ).orExit(logger) + + val sharedOptions = crossSources.sharedOptions(buildOptions) + val scopedSources = crossSources.scopedSources(sharedOptions).orExit(logger) + + val mainSources = scopedSources.sources(Scope.Main, sharedOptions) + val testSources = scopedSources.sources(Scope.Test, sharedOptions) + + (mainSources, testSources) + } + def getExtractedDirectives(sources: Sources)( using loggingUtilities: LoggingUtilities ): Seq[ExtractedDirectives] = { @@ -244,6 +245,36 @@ object Fix extends ScalaCommand[FixOptions] { } } + def createFormattedLinesAndAppend( + strictDirectives: Seq[StrictDirective], + projectFileContents: StringBuilder, + isTest: Boolean + ): Unit = { + strictDirectives + // group by key to merge values + .groupBy(_.key) + .map { (key, directives) => + StrictDirective(key, directives.flatMap(_.values)) + } + // group by key prefixes to create splits between groups + .groupBy(dir => (if (isTest) dir.key.stripPrefix("test.") else dir.key).takeWhile(_ != '.')) + .map { (_, directives) => + directives.flatMap(_.explodeToStringsWithColLimit()).toSeq.sorted + } + .toSeq + .filter(_.nonEmpty) + .sortBy(_.head)(using directivesOrdering) + // append groups to the StringBuilder, add new lines between groups that are bigger than one line + .foldLeft(0) { (lastSize, directiveLines) => + val newSize = directiveLines.size + if (lastSize > 1 || (lastSize != 0 && newSize > 1)) projectFileContents.append(newLine) + + directiveLines.foreach(projectFileContents.append(_).append(newLine)) + + newSize + } + } + case class TransformedTestDirectives( withTestPrefix: Seq[StrictDirective], noTestPrefixAvailable: Seq[StrictDirective], @@ -256,4 +287,34 @@ object Fix extends ScalaCommand[FixOptions] { ) { def relativePath(path: os.Path) = path.relativeTo(workspacePath) } + + private val directivesOrdering: Ordering[String] = { + val directivesOrder: Map[String, Int] = HashMap( + "scala" -> 0, + "platforms" -> 1, + "jvm" -> 2, + "native" -> 3, + "js" -> 4, + "options" -> 5, + "java" -> 6, + "mainClass" -> 7, + "objectWrapper" -> 8, + "toolkit" -> 9, + "dependency" -> 10, + "publish" -> 20 + ) + + Ordering.by { directiveLine => + val key = directiveLine + .stripPrefix("//> using") + .stripLeading() + .stripPrefix("test.") + .takeWhile(!_.isWhitespace) + val orderFromMap = directivesOrder.get(key).orElse { + directivesOrder.find((keyPrefix, _) => key.startsWith(keyPrefix)).map(_._2) + } + + orderFromMap.getOrElse(15) + } + } } 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 67e628c9d2..2f43db24c3 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 @@ -15,9 +15,34 @@ case class StrictDirective( s"//> using $key$suffix" } - /** Explodes this directive into a sequence of directives, each with a single value. */ - def explodeToStrings: Seq[String] = - values.map(v => StrictDirective(key, Seq(v)).toString) + /** Checks whether the directive with the sequence of values will fit into the given column limit, + * if it does then the function returns the single directive with all the values. If the + * directive does not fit then the function explodes it into a sequence of directives with + * distinct values, each with a single value. + */ + def explodeToStringsWithColLimit(colLimit: Int = 100): Seq[String] = { + val validValues = values.filter { + case _: EmptyValue => false + case _ => true + } + + val usingKeyString = s"//> using $key" + + if (validValues.isEmpty) + Seq(usingKeyString) + else { + val distinctValuesStrings = validValues + .map(v => s"\"${v.toString}\"") + .distinct + .sorted + + if (distinctValuesStrings.map(_.length).sum + usingKeyString.length < colLimit) + Seq(s"$usingKeyString ${distinctValuesStrings.mkString(" ")}") + else + distinctValuesStrings.map(v => s"$usingKeyString $v") + } + } + def stringValuesCount: Int = values.count { case _: StringValue => true diff --git a/modules/integration/src/test/scala/scala/cli/integration/FixTests.scala b/modules/integration/src/test/scala/scala/cli/integration/FixTests.scala index f41821c290..65109fef61 100644 --- a/modules/integration/src/test/scala/scala/cli/integration/FixTests.scala +++ b/modules/integration/src/test/scala/scala/cli/integration/FixTests.scala @@ -24,7 +24,7 @@ class FixTests extends ScalaCliSuite { |} |""".stripMargin, os.rel / projectFileName -> - s"""//> using lib com.lihaoyi::pprint:0.6.6 + s"""//> using lib "com.lihaoyi::pprint:0.6.6" |""".stripMargin ) @@ -33,31 +33,38 @@ class FixTests extends ScalaCliSuite { val fixOutput = os.proc(TestUtil.cli, "--power", "fix", ".", "-v", "-v", extraOptions) .call(cwd = root, mergeErrIntoOut = true).out.trim() - expect(fixOutput == + assertNoDiff( + fixOutput, """Extracting directives from Main.scala |Extracting directives from project.scala |Removing directives from Main.scala - |Removing directives from project.scala""".stripMargin) + |Removing directives from project.scala""".stripMargin + ) val projectFileContents = os.read(root / projectFileName) val mainFileContents = os.read(root / mainFileName) - expect(projectFileContents.contains( + assertNoDiff( + projectFileContents, """// Main - |//> 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 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" + | |""".stripMargin - )) + ) - expect(mainFileContents == + assertNoDiff( + mainFileContents, """package com.foo.main | |object Main extends App { | println(os.pwd) |} - |""".stripMargin) + |""".stripMargin + ) val runProc = os.proc(TestUtil.cli, "--power", "compile", ".", extraOptions) .call(cwd = root, stderr = os.Pipe) @@ -72,7 +79,7 @@ class FixTests extends ScalaCliSuite { val inputs = TestInputs( mainSubPath -> s"""//> using objectWrapper - |//> using dep com.lihaoyi::os-lib:0.9.1 + |//> using dep "com.lihaoyi::os-lib:0.9.1" | |package com.foo.main | @@ -82,7 +89,7 @@ class FixTests extends ScalaCliSuite { |""".stripMargin, testSubPath -> s"""//> using options -Xasync, -Xfatal-warnings - |//> using dep "org.scalameta::munit::0.7.29" + |//> using dep org.scalameta::munit::0.7.29 | |package com.foo.test.bar | @@ -103,39 +110,44 @@ class FixTests extends ScalaCliSuite { val fixOutput = os.proc(TestUtil.cli, "--power", "fix", ".", "-v", "-v", extraOptions) .call(cwd = root, mergeErrIntoOut = true).out.trim() - expect(fixOutput == + assertNoDiff( + fixOutput, """Extracting directives from project.scala |Extracting directives from src/Main.scala |Removing directives from project.scala |Removing directives from src/Main.scala |Extracting directives from test/MyTests.scala - |Removing directives from test/MyTests.scala""".stripMargin) + |Removing directives from test/MyTests.scala""".stripMargin + ) val projectFileContents = os.read(root / projectFileName) val mainFileContents = os.read(root / mainSubPath) val testFileContents = os.read(root / testSubPath) - expect(projectFileContents == + assertNoDiff( + projectFileContents, """// Main - |//> using dependency com.lihaoyi::os-lib:0.9.1 - |//> using dependency com.lihaoyi::pprint:0.6.6 |//> using objectWrapper + |//> using dependency "com.lihaoyi::os-lib:0.9.1" "com.lihaoyi::pprint:0.6.6" | |// Test - |//> using test.dependency org.scalameta::munit::0.7.29 - |//> using test.options -Xasync - |//> using test.options -Xfatal-warnings - |""".stripMargin) + |//> using test.options "-Xasync" "-Xfatal-warnings" + |//> using test.dependency "org.scalameta::munit::0.7.29" + |""".stripMargin + ) - expect(mainFileContents == + assertNoDiff( + mainFileContents, """package com.foo.main | |object Main extends App { | println(os.pwd) |} - |""".stripMargin) + |""".stripMargin + ) - expect(testFileContents == + assertNoDiff( + testFileContents, """package com.foo.test.bar | |class MyTests extends munit.FunSuite { @@ -144,7 +156,8 @@ class FixTests extends ScalaCliSuite { | println("Hello from " + "tests") | } |} - |""".stripMargin) + |""".stripMargin + ) val runProc = os.proc(TestUtil.cli, "--power", "compile", ".", extraOptions) .call(cwd = root, stderr = os.Pipe) @@ -172,8 +185,11 @@ class FixTests extends ScalaCliSuite { val inputs = TestInputs( mainSubPath -> - s"""//> using objectWrapper - |//> using dep com.lihaoyi::os-lib:0.9.1 + s"""//> using platform jvm + |//> using jvm 17 + |//> using objectWrapper + |//> using dep "com.lihaoyi::os-lib:0.9.1" + |//> using scala 3.3.0 | |package com.foo.main | @@ -185,7 +201,7 @@ class FixTests extends ScalaCliSuite { withUnusedTargetSubPath -> withUnusedTargetContents, testSubPath -> s"""//> using options -Xasync, -Xfatal-warnings - |//> using dep "org.scalameta::munit::0.7.29" + |//> using dep org.scalameta::munit::0.7.29 |//> using scala 3.2.2 | |package com.foo.test.bar @@ -199,6 +215,11 @@ class FixTests extends ScalaCliSuite { |""".stripMargin, os.rel / projectFileName -> s"""//> using lib 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 |""".stripMargin ) @@ -215,7 +236,8 @@ class FixTests extends ScalaCliSuite { extraOptions ).call(cwd = root, stderr = os.Pipe) - expect(res.err.trim() == + assertNoDiff( + res.err.trim(), """Extracting directives from project.scala |Extracting directives from src/Main.scala |Extracting directives from src/UsedTarget.scala @@ -225,7 +247,8 @@ class FixTests extends ScalaCliSuite { |Extracting directives from test/MyTests.scala |Removing directives from test/MyTests.scala | Keeping: - | //> using scala 3.2.2""".stripMargin) + | //> using scala 3.2.2""".stripMargin + ) val projectFileContents = os.read(root / projectFileName) val mainFileContents = os.read(root / mainSubPath) @@ -233,29 +256,40 @@ class FixTests extends ScalaCliSuite { val withUsedTargetContentsRead = os.read(root / withUsedTargetSubPath) val withUnusedTargetContentsRead = os.read(root / withUnusedTargetSubPath) - expect(projectFileContents == + assertNoDiff( + projectFileContents, """// Main - |//> using dependency com.lihaoyi::os-lib:0.9.1 - |//> using dependency com.lihaoyi::pprint:0.6.6 + |//> using scala "3.3.0" + |//> using platforms "jvm" + |//> using jvm "17" |//> using objectWrapper - |//> using toolkit latest + |//> using toolkit "latest" + |//> 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" | |// Test - |//> using test.dependency org.scalameta::munit::0.7.29 - |//> using test.options -Xasync - |//> using test.options -Xfatal-warnings - |""".stripMargin) + |//> using test.options "-Xasync" "-Xfatal-warnings" + |//> using test.dependency "org.scalameta::munit::0.7.29" + |""".stripMargin + ) - expect(mainFileContents == + assertNoDiff( + mainFileContents, """package com.foo.main | |object Main extends App { | println(os.pwd) |} - |""".stripMargin) + |""".stripMargin + ) // Directives with no 'test.' equivalent are retained - expect(testFileContents == + assertNoDiff( + testFileContents, """//> using scala 3.2.2 | |package com.foo.test.bar @@ -266,10 +300,11 @@ class FixTests extends ScalaCliSuite { | println("Hello from " + "tests") | } |} - |""".stripMargin) + |""".stripMargin + ) - expect(withUsedTargetContents == withUsedTargetContentsRead) - expect(withUnusedTargetContents == withUnusedTargetContentsRead) + assertNoDiff(withUsedTargetContents, withUsedTargetContentsRead) + assertNoDiff(withUnusedTargetContents, withUnusedTargetContentsRead) val runProc = os.proc(TestUtil.cli, "--power", "compile", ".", extraOptions) .call(cwd = root, stderr = os.Pipe)