Merge scalafix into fix

[Gedochao]

Closes #3327

This merges the scalafix sub-command into fix.

scala-cli fix . --power
# The `fix` sub-command is experimental
# Please bear in mind that non-ideal user experience should be expected.
# If you encounter any bugs or have feedback to share, make sure to reach out to the maintenance team at https://github.com/VirtusLab/scala-cli
# Running built-in rules...
# Writing project.scala
# Removing directives from Smth.scala
# Built-in rules completed.
# Running scalafix rules...
# 
# Starting compilation server
# Compiling project (Scala 3.6.2, JVM (zulu:23))
# [warn] ./Main.scala:2:7
# [warn] unused local definition
# [warn]   val unused = "unused"
# [warn]       ^^^^^^
# Compiled project (Scala 3.6.2, JVM (zulu:23))
# scalafix rules completed.

Former fix functionalities are now referred to in the code as the built-in rules.
Effectively, fix now runes 2 separate sets of rules (both enabled by default): built-in and scalafix.
They can be controlled via the --enable-scalafix and --enable-built-in command line options.

[tgodzik]

LGTM, just one minor comment

[bjaglin]

Not sure if this PR is the right forum to open a discussion (happy to continue wherever is more suited, just let me know!), but I was surprised by this UX change that I found out while reading the release notes, particularly because scalafix already has a concept built-rules.

While I share the concern about having several fix commands, I am not sure that users will understand what "built-in rules" refer to and how they can be opted-in/out. As far as I can tell, at the moment, there is no way to opt-out from the scala-cli built-in rule, while all scalafix built-in rules are opt-in (until #3330 at least).

Maybe a way to bring some clarity for users would be to rewrite the scala-cli built-in rule as a scalafix rule (potentially maintained in this repo together with the build), publish it as a regular scalafix external rule but load by default when scalafix is run through scala-cli, allowing users to opt-in/out via .scalafix.conf just like any other scalafix rule?

(1) from #2309 looks non-trivial to implement as a Scalafix rule as the Scalafix API is very file-centric, but it should be doable by

• hacking some state persistence in the rule accumulating directive information collected in source files
• creating an empty project.scala
• invoke the scalafix CLI with target files explicitly (as positional args) to control the order of evaluation and make sure project.scala always gets evaluated last

I am obviously biased as the Scalafix core maintainer, but if this turns out to work without any change in Scalafix (as I hope), we would have another impactful community rule that rule authors can get inspiration from for their own use-case, and if we hit some limitations, we could extend the Scalafix API, which would also benefit rule authors. Maybe that would also be an opportunity to parse/prettyprint scala-cli directives in Scalameta, to reduce the string parsing/building logic here?

[tgodzik]

there is no way to opt-out from the scala-cli built-in rule,

You should be able to do scala-cli fix --enable-built-in=false . which should only run scalafix rules. I think we could change the defaults not to run build in ones by default, but we would need to discuss it further. We can for sure rename those flags as it's rather experimental still.

Having a scalafix rules that would do the same would also be quite useful, since it is much more complex I am not sure when we would be able to write it.

[bjaglin]

there is no way to opt-out from the scala-cli built-in rule,

You should be able to do scala-cli fix --enable-built-in=false . which should only run scalafix rules.

Indeed, since there is only one scala-cli built-in rule at the moment, this flag does the job, but as soon as we have more than one built-in rule, we would likely need a toggle per rule.

we don't need third-party library that only rely on TASTy (e.g. https://github.com/scalacenter/tasty-query) for type inspection and inference, TASTy as a serialiser is good enough

To start with, we could have a no-op Scalafix rule just for the sake of enabling .scalafix.conf and corresponding CLI args to opt-in/out for a consistent UI - scala-cli could still use custom code when detecting the presence of it.

[tgodzik]

Indeed, since there is only one scala-cli built-in rule at the moment, this flag does the job, but as soon as we have more than one built-in rule, we would likely need a toggle per rule.

I don't think we plan on having more and I would prefer to try to use scalafix API next time. If we had more than one rule, then maybe we could migrate to scala build-fix command instead 🤔 Or we could run build in rules by name with a more general --rules option.

To start with, we could have a no-op Scalafix rule just for the sake of enabling .scalafix.conf and corresponding CLI args to opt-in/out for a consistent UI - scala-cli could still use custom code when detecting the presence of it.

I think I am a bit lost here. Why would we need it?

[bjaglin]

To start with, we could have a no-op Scalafix rule just for the sake of enabling .scalafix.conf and corresponding CLI args to opt-in/out for a consistent UI - scala-cli could still use custom code when detecting the presence of it.

I think I am a bit lost here. Why would we need it?

• Going back to my original comment, I am concerned about having 2 separate ways of enabling/disabling rules, and the ambiguous name of "built-in", therefore I was suggesting to converge on Scalafix as a distribution/configuration channel, in order using .scalafix.conf or the scalafix-specific CLI args for all of them.
• You replied that it's unlikely that the effort of porting the existing scala-cli built-in rule ever gets prioritized.
• As a result, I was suggesting a tradeoff: keep the scala-cli built-in rule logic as-is (i.e. not built against Scalafix APIs), but execute only when a rule of specific name is requested to Scalafix.
  • The actual implementation of that Scalafix rule would be a no-op, just to avoid failing the Scalafix invocation.
  • That way we get a consistent user-facing UI, with the mental model that scala-cli fix is a pure wrapper of Scalafix (preloaded with scala-cli-specific rules), potentially with default rules (including the scala-cli one) if none is explicitly requested.

[bjaglin]

If we had more than one rule, then maybe we could migrate to scala build-fix command instead

That would definitely reduce the ambiguity. I guess this is a decision that needs to be taken if/when fix gets out of experimental mode.

Or we could run build in rules by name with a more general --rules option.

I think this is where my concern is back: will users understand that the same command can control rules of different type/distribution channels via different CLI options?

[tgodzik]

I think this is where my concern is back: will users understand that the same command can control rules of different type/distribution channels via different CLI options?

Yeah, I guess it's a tough one. Though I wonder if from user perspective it makes any difference and should we make that difference visible or go the other way?