feat: honor sbt resolvers in addition to scalafixResolvers

[GreyPlane]

scalacenter/scalafix#1944

Use code from here and adapt it to courisierapi.

There is still some issues or decisions that I'm not very confident to deal with.

1. Since sbt.Credentials is a task, so it cannot be included in ScalaCompletion.
2. I assume that repo's credential was resolved by comparing the host of credential's field and repo's root url, which I'm not 100% sure if it is OK.
3. Due to the reason above, I don't know how to find credential for IvyRepository.
4. I don't know how to test this through scripted test, since the logic has to be deferred to when scalafixTask actually ran.

Would you mind to provide some help? @bjaglin

[bjaglin]

Thanks for the PR! I will look at that this week-end, and see if I can advise.

[byrek3d]

It would be super cool if this change were to be approved :)

[bjaglin]

Sorry for the delay, I'll look at it this week end

[GreyPlane]

@bjaglin friendly ping :)

Also I did some investigation and find out that during the sbt.Credentials task, it actually call Ivy API to register all credentials, that's why it's a task at first place I guess, so I guess the question 3 isn't a problem.

[bjaglin]

Thanks for your patience on that initial pass, I knew it would require some focus time and I couldn't find it lately due to personal commitments.

(1) (2) (3)

Supporting credentials really brings a lot of complexity. Do you need that feature for your own use-case? If not, we could keep it simple for the first iteration, and just redefine the default scalafixResolvers to include sbt resolvers. This way, we wouldn't have all the challenges about extra memory consumption, inconsistent behavior of the completions, need for memoization to avoid duplicated warns, etc.

Also, to follow-up on (2) specifically, I wouldn't know how to reliably and easily test this... Starting a nexus repository via docker for integration tests in CI? That looks like a lot of work...

(4)

scalafix being a task, it actually makes it easy to write a scripted test, as we can add/remove sbt resolvers over time in the test file and assert how the task succeeds/fails when it tries to fetch a dependency from a non-standard resolver.

I would suggest trying to run a scalafix rule available in sonatype snapshots. Let me know if that helps. I can also take care of that and push a commit to your branch if you prefer.

[bjaglin]

FTR, scala 2.13.14 is coming out very soon, so I'll most likely release a version of scalafix & sbt-scalafix this week-end. I have a bit of free time this week, so happy to help you push this forward in order for this to be part of that release.

[bjaglin]

@adpi2 in case you have some bandwidth to review this and make sbt-scalafix more corporate-friendly, that would be great!

[GreyPlane]

Supporting credentials really brings a lot of complexity. Do you need that feature for your own use-case?

Unfortunately yes :(, Also I think if rules was hosted on some kind of public repo, chances are very high that default sbt-scalafix resolvers were able to find it, otherwise for corpo private repo, it's almost always behind some kind of authentication IMO.

we could keep it simple for the first iteration, and just redefine the default scalafixResolvers to include sbt resolvers.

I think the iteration plan sounds reasonable, if there's anyone could be helped by the initial version of it, I'm perfectly fine to do this, but unfortunately it's not my case...

Also, to follow-up on (2) specifically

Even though not 100% sure, I think it could cover the most cases though, alternately, I think we can ask other more experienced people in the community, public channel or something, since it's more like a know/not know knowledge-wise thing.

I can also take care of that and push a commit to your branch if you prefer.

I'm not very experienced on sbt scripted test, though I can learn to how to do it, but if you can provide extra help it will be great!

[GreyPlane]

Some of the changes could be a lit a bit time consuming, I will do these at off-work time, or at least I can dedicate my weekend on these.

[bjaglin]

I think if rules was hosted on some kind of public repo, chances are very high that default sbt-scalafix resolvers were able to find it, otherwise for corpo private repo, it's almost always behind some kind of authentication IMO.

My personal experience at work was that our nexus was accessible through a VPN, thus my question.

I think the iteration plan sounds reasonable, if there's anyone could be helped by the initial version of it, I'm perfectly fine to do this, but unfortunately it's not my case...

OK,never mind then, let's brainstorm to find a solution for your use-case then!

[bjaglin]

Also, to follow-up on (2) specifically, I wouldn't know how to reliably and easily test this... Starting a nexus repository via docker for integration tests in CI? That looks like a lot of work...

Even though not 100% sure, I think it could cover the most cases though, alternately, I think we can ask other more experienced people in the community, public channel or something, since it's more like a know/not know knowledge-wise thing.

Let's abandon the idea of testing credentials in CI for now. I don't expect this to change much, so relying on your manual testing for now should be OK.

[bjaglin]

I'm not very experienced on sbt scripted test, though I can learn to how to do it, but if you can provide extra help it will be great!

I am thinking about something like that (untested so please bear with me)

src/sbt-test/sbt-scalafix/resolvers/src/main/scala/A.scala

object A

src/sbt-test/sbt-scalafix/resolvers/build.sbt

// https://github.com/scalacenter/example-scalafix-rule/blob/e9850db/scalafix/rules/src/main/resources/META-INF/services/scalafix.v1.Rule#L1C1-L1C14
// https://github.com/scalacenter/example-scalafix-rule/blob/e9850db/scalafix/rules/src/main/scala/fix/Examplescalafixrule_v1.scala#L23-L28
ThisBuild / scalafixDependencies += "ch.epfl.scala" %% "example-scalafix-rule" % "4.0.0+3-e9850db5-SNAPSHOT"

src/sbt-test/sbt-scalafix/resolvers/test

# without the proper resolver/repository, invocation of the community rule fails
-> scalafix fix.Syntactic

# once the proper resolver is setup via sbt, dependency fetching works fine and scalafix can be used normally
> set resolvers ++= Resolver.sonatypeOssRepos("snapshots")
-> scalafix --check fix.Syntactic
> scalafix fix.Syntactic
> scalafix --check fix.Syntactic

[GreyPlane]

Scripted test is lots easier than I was thought it would be :), Thanks for your help.

Though set resolvers ++= Resolver.sonatypeOssRepos("snapshots") seems wouldn't reevaluate scalafixAdaptSbtResolvers, even after I moved it to buildSettings.

Is there something wrong with the settings I put scalafixAdaptSbtResolvers in or there's some limitation about scripted test?

[bjaglin]

Though set resolvers ++= Resolver.sonatypeOssRepos("snapshots") seems wouldn't reevaluate scalafixAdaptSbtResolvers, even after I moved it to buildSettings.

I just tested locally, and it seems to work.

> set ThisBuild / resolvers += Resolver.sonatypeRepo("snapshots")

...
[info] [info] Defining ThisBuild / resolvers
[info] [info] The new value will be used by ThisBuild / scalafixAdaptSbtResolvers, externalResolvers
...

[bjaglin]

You should rebase, it looks like you have conflicts against #405 which was merged after you started your branch.

[bjaglin]

I think the only real outstanding comment is the error recovery in the parser, we are getting close 👍

[bjaglin]

I am looking at the scripted failure. Once you have confirmed credentials work, we are good to go!

[GreyPlane]

Thanks for the tremendous effort you put into guiding me through this.