From 5bacfc6e1688cab6df3b17996d66d6987ff23413 Mon Sep 17 00:00:00 2001 From: Piotr Chabelski Date: Wed, 15 Jan 2025 12:21:05 +0100 Subject: [PATCH 1/2] Don't migrate directives with `fix` for single-file projects --- .../scala/cli/commands/fix/BuiltInRules.scala | 22 +++++++++- .../FixBuiltInRulesTestDefinitions.scala | 34 ++++++++++++++++ .../FixScalafixRulesTestDefinitions.scala | 40 +++++-------------- website/docs/commands/fix.md | 2 + 4 files changed, 67 insertions(+), 31 deletions(-) diff --git a/modules/cli/src/main/scala/scala/cli/commands/fix/BuiltInRules.scala b/modules/cli/src/main/scala/scala/cli/commands/fix/BuiltInRules.scala index a855e3dd5e..81ab578d2b 100644 --- a/modules/cli/src/main/scala/scala/cli/commands/fix/BuiltInRules.scala +++ b/modules/cli/src/main/scala/scala/cli/commands/fix/BuiltInRules.scala @@ -39,6 +39,26 @@ object BuiltInRules extends CommandHelpers { .left.map(CompositeBuildException(_)) .orExit(logger) + val sourcesCount = + mainSources.paths.length + mainSources.inMemory.length + + testSources.paths.length + testSources.inMemory.length + sourcesCount match + case 0 => + logger.message("No sources to migrate directives from.") + logger.message("Nothing to do.") + case 1 => + logger.message("No need to migrate directives for a single source file project.") + logger.message("Nothing to do.") + case _ => migrateDirectives(inputs, buildOptions, mainSources, testSources, logger) + } + + private def migrateDirectives( + inputs: Inputs, + buildOptions: BuildOptions, + mainSources: Sources, + testSources: Sources, + logger: Logger + )(using ScalaCliInvokeData): Unit = { // Only initial inputs are used, new inputs discovered during processing of // CrossSources.forInput may be shared between projects val writableInputs: Seq[OnDisk] = inputs.flattened() @@ -120,8 +140,8 @@ object BuiltInRules extends CommandHelpers { directivesFromWritableTestInputs .filterNot(ttd => isProjectFile(ttd.positions)) .foreach(ttd => removeDirectivesFrom(ttd.positions, toKeep = ttd.noTestPrefixAvailable)) - } + private def getProjectSources(inputs: Inputs, logger: Logger)(using ScalaCliInvokeData ): Either[::[BuildException], (Sources, Sources)] = { 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 152225beae..ce024d2e17 100644 --- a/modules/integration/src/test/scala/scala/cli/integration/FixBuiltInRulesTestDefinitions.scala +++ b/modules/integration/src/test/scala/scala/cli/integration/FixBuiltInRulesTestDefinitions.scala @@ -447,4 +447,38 @@ trait FixBuiltInRulesTestDefinitions { _: FixTestDefinitions => expect(r.out.trim() == expectedMessage) } } + + { + val directive = "//> using dep com.lihaoyi::os-lib:0.11.3" + for { + (inputFileName, code) <- Seq( + "raw.scala" -> s"""$directive + |object Main extends App { + | println(os.pwd) + |} + |""".stripMargin, + "script.sc" -> s"""$directive + |println(os.pwd) + |""".stripMargin + ) + if !Properties.isWin // TODO: make this run on Windows CI + testInputs = TestInputs(os.rel / inputFileName -> code) + } + test( + s"dont extract directives into project.scala for a single-file project: $inputFileName" + ) { + testInputs.fromRoot { root => + val fixResult = os.proc(TestUtil.cli, "--power", "fix", ".", extraOptions) + .call(cwd = root, stderr = os.Pipe) + expect(fixResult.err.trim().contains( + "No need to migrate directives for a single source file project" + )) + expect(!os.exists(root / projectFileName)) + expect(os.read(root / inputFileName) == code) + val runResult = os.proc(TestUtil.cli, "run", ".", extraOptions) + .call(cwd = root, stderr = os.Pipe) + expect(runResult.out.trim() == root.toString) + } + } + } } 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 1055641db5..56774081fb 100644 --- a/modules/integration/src/test/scala/scala/cli/integration/FixScalafixRulesTestDefinitions.scala +++ b/modules/integration/src/test/scala/scala/cli/integration/FixScalafixRulesTestDefinitions.scala @@ -78,7 +78,8 @@ trait FixScalafixRulesTestDefinitions { os.rel / "Hello.scala" -> unusedValueInputsContent ) val expectedContent: String = noCrLf { - s"""package foo + s"""//> using options $scalafixUnusedRuleOption + |package foo | |object Hello { | def main(args: Array[String]): Unit = { @@ -88,19 +89,11 @@ trait FixScalafixRulesTestDefinitions { |} |""".stripMargin } - val expectedProjectContent: String = noCrLf { - s"""// Main - |//> using options $scalafixUnusedRuleOption - | - |""".stripMargin - } semanticRuleInputs.fromRoot { root => os.proc(TestUtil.cli, "fix", "--power", ".", scalaVersionArgs).call(cwd = root) val updatedContent = noCrLf(os.read(root / "Hello.scala")) expect(updatedContent == expectedContent) - val projectContent = noCrLf(os.read(root / "project.scala")) - expect(projectContent == expectedProjectContent) } } @@ -136,9 +129,9 @@ trait FixScalafixRulesTestDefinitions { scalaVersionArgs ).call(cwd = root) val updatedContent = noCrLf(os.read(root / "Hello.scala")) - val projectContent = noCrLf(os.read(root / "project.scala")) val expected = noCrLf { - s"""|package hello + s"""|//> using options $scalafixUnusedRuleOption + |package hello | |object Hello { | def a = { @@ -148,15 +141,8 @@ trait FixScalafixRulesTestDefinitions { |} |""".stripMargin } - val expectedProjectContent: String = noCrLf { - s"""// Main - |//> using options $scalafixUnusedRuleOption - | - |""".stripMargin - } expect(updatedContent == expected) - expect(projectContent == expectedProjectContent) } } @@ -259,24 +245,18 @@ trait FixScalafixRulesTestDefinitions { os.rel / "Hello.scala" -> original ) val expectedContent: String = noCrLf { - """|object CollectHeadOptionTest { - | def x1: Option[String] = List(1, 2, 3).collectFirst{ case n if n % 2 == 0 => n.toString } - |} - |""".stripMargin - } - val expectedProjectContent: String = noCrLf { - s"""// Main - |$directive - | - |""".stripMargin + s"""|$directive + | + |object CollectHeadOptionTest { + | def x1: Option[String] = List(1, 2, 3).collectFirst{ case n if n % 2 == 0 => n.toString } + |} + |""".stripMargin } inputs.fromRoot { root => os.proc(TestUtil.cli, "fix", ".", "--power", scalaVersionArgs).call(cwd = root) val updatedContent = noCrLf(os.read(root / "Hello.scala")) expect(updatedContent == expectedContent) - val projectContent = noCrLf(os.read(root / "project.scala")) - expect(projectContent == expectedProjectContent) } } diff --git a/website/docs/commands/fix.md b/website/docs/commands/fix.md index 4993634940..34d9496520 100644 --- a/website/docs/commands/fix.md +++ b/website/docs/commands/fix.md @@ -30,6 +30,8 @@ Files containing (experimental) `using target` directives, e.g. `//> using targe The original scope (`main` or `test`) of each extracted directive is respected. `main` scope directives are transformed them into their `test.*` equivalent when needed. +Note: directives won't be extracted for single-file projects. + ## `scalafix` integration Scala CLI is capable of running [Scalafix](https://scalacenter.github.io/scalafix/) (a refactoring and linting tool for Scala) on your project. From ed069ed446996e47db74d7b76d80504b326ae348 Mon Sep 17 00:00:00 2001 From: Piotr Chabelski Date: Thu, 16 Jan 2025 12:44:12 +0100 Subject: [PATCH 2/2] Temporarily disable built-in rules of `fix` when `--check` is enabled, until fully supported --- .../main/scala/scala/cli/commands/fix/Fix.scala | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) 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 b15c0fd738..73ef1040b6 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 @@ -30,12 +30,17 @@ object Fix extends ScalaCommand[FixOptions] { val configDb = ConfigDbUtils.configDb.orExit(logger) if options.enableBuiltInRules then { logger.message("Running built-in rules...") - BuiltInRules.runRules( - inputs = inputs, - buildOptions = buildOpts, - logger = logger - ) - logger.message("Built-in rules completed.") + if options.check then + // TODO support --check for built-in rules: https://github.com/VirtusLab/scala-cli/issues/3423 + logger.message("Skipping, '--check' is not yet supported for built-in rules.") + else { + BuiltInRules.runRules( + inputs = inputs, + buildOptions = buildOpts, + logger = logger + ) + logger.message("Built-in rules completed.") + } } if options.enableScalafix then either {