Scalafix command for scala-cli with basic options and tests #2968

Vigorge · 2024-06-17T10:10:04Z

Resolves Issue #647

modules/cli/src/main/scala/scala/cli/commands/scalafix/Scalafix.scala

Gedochao · 2024-06-18T13:37:54Z

Hey, thanks for the contribution!
I'll make sure to review this as soon as I am available, but given the volume of code included, I might need a couple days to get to it.

modules/cli/src/main/scala/scala/cli/commands/scalafix/Scalafix.scala

modules/integration/src/test/scala/scala/cli/integration/ScalafixTests.scala

Gedochao · 2024-06-25T10:00:49Z

NOTE: After this is cleaned up and merged, we'll want to merge the scalafix sub-command with the fix sub-command (there's no point to keep them separate).
Individual fix features would probably be enabled/disabled by flags.
We'll do that as a follow-up, no need to worry about it in this PR.

bjaglin

Thanks for starting this 🤩

I am not familiar with scala-cli at all, but as the maintainer of sbt-scalafix and scalafix, here is some feedback FWIW!

modules/cli/src/main/scala/scala/cli/commands/scalafix/ScalafixOptions.scala

project/deps.sc

modules/cli/src/main/scala/scala/cli/commands/scalafix/Scalafix.scala

modules/integration/src/test/scala/scala/cli/integration/ScalafixTests.scala

modules/cli/src/main/scala/scala/cli/commands/scalafix/Scalafix.scala

modules/cli/src/main/scala/scala/cli/commands/scalafix/ScalafixOptions.scala

tgodzik · 2024-06-26T12:21:50Z

Thanks @bjaglin!

@Vigorge if you are at any point overwhelmed by amount of comments, we are here to help. You already did a lot of great work 😍

Vigorge · 2024-07-04T13:35:16Z

Thanks @bjaglin!

@Vigorge if you are at any point overwhelmed by amount of comments, we are here to help. You already did a lot of great work 😍

Thanks a lot for such an impactful feedback! These comments are really helpful and I'll surely take them into account. I really hope that I will resolve them in a few days and add some more features. Unfortunately, I had to take a little break, but now am fully committed to work more on this PR now

modules/cli/src/main/scala/scala/cli/commands/scalafix/ScalafixOptions.scala

Gedochao · 2024-08-08T09:18:50Z

Not sure why didn't the CI run, but a rebase from main seems to be necessary, anyway.

SethTisue · 2024-08-08T17:12:02Z

Great to see activity on this — I think this could be big for Scalafix, in terms of people being able to use it more easily.

modules/integration/src/test/scala/scala/cli/integration/ScalafixTests.scala

Vigorge · 2024-08-12T13:45:24Z

Hi, sorry for taking such a long time to resolve all those threads and thanks a lot again for such feedback.

The PR is ready for review.

bjaglin · 2024-08-12T14:50:03Z

Thanks for the follow-ups @Vigorge 👍 I will take a look at the PR tomorrow.

bjaglin

Hey @Vigorge, thanks for all the follow-ups/additions. Having a semantic rule tested against all Scala versions is a great achievement!

I went through a thorough second review to avoid any surprise in the third/final round. From my very own perspective (again, I am not familiar with scala-cli), here are the outstanding comments to be addressed before this could be released

workspace / current directory injection seems broken - I did not manage to use the CLI
diagnostics/linters are ignored under --check
using scalafix-interfaces with native-image is impossible and raises a cryptic error - a more actionnable error message should be added until this is sorted out in a further PR

modules/cli/src/main/scala/scala/cli/commands/scalafix/Scalafix.scala

modules/integration/src/test/scala/scala/cli/integration/ScalafixTests213.scala

modules/cli/src/main/scala/scala/cli/commands/scalafix/ScalafixOptions.scala

modules/cli/src/main/scala/scala/cli/commands/scalafix/Scalafix.scala

Gedochao · 2024-09-11T08:50:40Z

@Vigorge how's the progress on this? Can we unblock you somehow?

Vigorge · 2024-09-17T15:56:39Z

@Vigorge how's the progress on this? Can we unblock you somehow?

HI, still am working on the native image solution and gonna add feature in few days.

Thanks for support! I will comment here if some help will be needed.

dos65 · 2024-10-16T13:58:54Z

@Gedochao Hi, we did changes in MR to make scalafix work both in jvm and native builds. Could you approve CI run?

dos65 · 2024-10-18T08:49:12Z

Update. I need one additionial iteration to go trough comments from @bjaglin
(add a bit more docs and tests) but in general I hope the PR is close to completion.

bjaglin

Wow, great effort @dos65 👏

I added some comments as potential follow-ups/simplifications, but based on a review (I can't test right now), that's looking more than enough for an initial release from a Scalafix perspective 👍

modules/integration/src/test/scala/scala/cli/integration/ScalafixTestDefinitions.scala

modules/build/src/main/scala/scala/build/ScalafixArtifacts.scala

modules/integration/src/test/scala/scala/cli/integration/ScalafixTestDefinitions.scala

dos65 · 2024-11-06T21:28:35Z

@Gedochao could you approve ci run?

dos65 · 2024-11-07T20:14:01Z

@Gedochao There are some failed native-macos tests. I hope they are flaky. Otherwise I have no idea what might be wrong with them

dos65 · 2024-11-08T09:55:36Z

Great, now CI is green.

tgodzik · 2024-11-08T10:11:29Z

Great work @dos65 ! @Gedochao is out seek, so we can wait for him to take a look next week.

Gedochao

LGTM
I'll merge this once Scala CLI 1.5.3 is out.

scarf005 · 2024-11-13T12:49:15Z

Sorry for nitpicking, but the title seems to have a typo:

- Scalafix command for scala-cli with basic optionns and tests
+ Scalafix command for scala-cli with basic options and tests

Really looking forward to this PR!

Gedochao · 2024-11-15T07:52:27Z

I'll merge this once Scala CLI 1.5.3 is out.

...make that 1.5.4 😩 (#3273 (comment))

Vigorge marked this pull request as draft June 17, 2024 10:12

tgodzik reviewed Jun 17, 2024

View reviewed changes

modules/cli/src/main/scala/scala/cli/commands/scalafix/Scalafix.scala Outdated Show resolved Hide resolved

Gedochao reviewed Jun 17, 2024

View reviewed changes

modules/cli/src/main/scala/scala/cli/commands/scalafix/Scalafix.scala Outdated Show resolved Hide resolved

Gedochao reviewed Jun 25, 2024

View reviewed changes

modules/cli/src/main/scala/scala/cli/commands/scalafix/Scalafix.scala Outdated Show resolved Hide resolved

Gedochao reviewed Jun 25, 2024

View reviewed changes

modules/integration/src/test/scala/scala/cli/integration/ScalafixTests.scala Outdated Show resolved Hide resolved

bishabosha mentioned this pull request Jun 25, 2024

scalafix command to run library migration & hygiene rules #647

Closed

Gedochao linked an issue Jun 25, 2024 that may be closed by this pull request

scalafix command to run library migration & hygiene rules #647

Closed

bjaglin reviewed Jun 26, 2024

View reviewed changes

Gedochao reviewed Aug 8, 2024

View reviewed changes

Vigorge force-pushed the scalafix-command branch from 2848d16 to 9da5f2a Compare August 12, 2024 12:23

Vigorge marked this pull request as ready for review August 12, 2024 12:27

dos65 reviewed Aug 12, 2024

View reviewed changes

modules/integration/src/test/scala/scala/cli/integration/ScalafixTests.scala Outdated Show resolved Hide resolved

Vigorge requested review from Gedochao and bjaglin August 12, 2024 13:42

bjaglin reviewed Aug 13, 2024

View reviewed changes

dos65 force-pushed the scalafix-command branch from a409057 to 52f6591 Compare October 16, 2024 14:16

dos65 force-pushed the scalafix-command branch from 9d908a2 to d1a7ae9 Compare October 21, 2024 20:34

bjaglin approved these changes Oct 22, 2024

View reviewed changes

Vigorge and others added 14 commits November 6, 2024 23:49

Refactor documentation and minor inconveniences

717080b

Make scalac options and scala version configurable

a73beb1

Remove fixed scalac options

b2eaee5

Correct ScalafixOptions tags to experimental

4adb5c3

Add more tests and fix external rule dependencies forwarding to scalafix

0f61c24

Differentiate tests for 2.12, 2.13 and 3+ scala versions

97a0161

Fix undeleted printlns

f9f8429

Fmt and fix

1875834

Native image support

17fac70

scalafix: fix native image support

d685d39

scalafix: run fix + fix tests

5829f89

scalafix: add scalafix.dep, more tests

632c6e1

scalafix: gen doc

512b309

scalafix: support ExplicitResultTypes for scala3 + minor fixes

789f70a

dos65 force-pushed the scalafix-command branch from 6bb8c5b to 789f70a Compare November 6, 2024 20:49

scalafix: fix docs tests

78384d2

dos65 force-pushed the scalafix-command branch from ae7f0c9 to 78384d2 Compare November 7, 2024 13:22

Gedochao approved these changes Nov 13, 2024

View reviewed changes

Gedochao added the needs-minor-release This change should require a minor version bump. label Nov 13, 2024

Gedochao changed the title ~~Scalafix command for scala-cli with basic optionns and tests~~ Scalafix command for scala-cli with basic options and tests Nov 13, 2024

Gedochao merged commit eec8bde into VirtusLab:main Nov 21, 2024
78 checks passed

bjaglin mentioned this pull request Nov 21, 2024

scala-agnostic launcher scalacenter/scalafix#2127

Open

fthomas mentioned this pull request Nov 24, 2024

Support Scalafix migrations in Scala CLI projects scala-steward-org/scala-steward#3486

Open

Gedochao mentioned this pull request Nov 28, 2024

Merge scalafix sub-command into fix #3327

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Scalafix command for scala-cli with basic options and tests #2968

Scalafix command for scala-cli with basic options and tests #2968

Vigorge commented Jun 17, 2024

Gedochao commented Jun 18, 2024

Gedochao commented Jun 25, 2024 •

edited

Loading

bjaglin left a comment

tgodzik commented Jun 26, 2024

Vigorge commented Jul 4, 2024

Gedochao commented Aug 8, 2024

SethTisue commented Aug 8, 2024

Vigorge commented Aug 12, 2024 •

edited

Loading

bjaglin commented Aug 12, 2024

bjaglin left a comment •

edited

Loading

Gedochao commented Sep 11, 2024

Vigorge commented Sep 17, 2024

dos65 commented Oct 16, 2024

dos65 commented Oct 18, 2024

bjaglin left a comment

dos65 commented Nov 6, 2024

dos65 commented Nov 7, 2024 •

edited

Loading

dos65 commented Nov 8, 2024

tgodzik commented Nov 8, 2024

Gedochao left a comment

scarf005 commented Nov 13, 2024

Gedochao commented Nov 15, 2024

Scalafix command for scala-cli with basic options and tests #2968

Scalafix command for scala-cli with basic options and tests #2968

Conversation

Vigorge commented Jun 17, 2024

Gedochao commented Jun 18, 2024

Gedochao commented Jun 25, 2024 • edited Loading

bjaglin left a comment

Choose a reason for hiding this comment

tgodzik commented Jun 26, 2024

Vigorge commented Jul 4, 2024

Gedochao commented Aug 8, 2024

SethTisue commented Aug 8, 2024

Vigorge commented Aug 12, 2024 • edited Loading

bjaglin commented Aug 12, 2024

bjaglin left a comment • edited Loading

Choose a reason for hiding this comment

Gedochao commented Sep 11, 2024

Vigorge commented Sep 17, 2024

dos65 commented Oct 16, 2024

dos65 commented Oct 18, 2024

bjaglin left a comment

Choose a reason for hiding this comment

dos65 commented Nov 6, 2024

dos65 commented Nov 7, 2024 • edited Loading

dos65 commented Nov 8, 2024

tgodzik commented Nov 8, 2024

Gedochao left a comment

Choose a reason for hiding this comment

scarf005 commented Nov 13, 2024

Gedochao commented Nov 15, 2024

Gedochao commented Jun 25, 2024 •

edited

Loading

Vigorge commented Aug 12, 2024 •

edited

Loading

bjaglin left a comment •

edited

Loading

dos65 commented Nov 7, 2024 •

edited

Loading