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.

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

Temporarily disable built-in rules of fix when --check is enabled, until fully supported #3427

Merged
merged 2 commits into from
Jan 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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)] = {
Expand Down
17 changes: 11 additions & 6 deletions modules/cli/src/main/scala/scala/cli/commands/fix/Fix.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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)
}
}

Expand Down Expand Up @@ -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 = {
Expand All @@ -148,15 +141,8 @@ trait FixScalafixRulesTestDefinitions {
|}
|""".stripMargin
}
val expectedProjectContent: String = noCrLf {
s"""// Main
|//> using options $scalafixUnusedRuleOption
|
|""".stripMargin
}

expect(updatedContent == expected)
expect(projectContent == expectedProjectContent)

}
}
Expand Down Expand Up @@ -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)
}
}

Expand Down
2 changes: 2 additions & 0 deletions website/docs/commands/fix.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down